From 2eae20ffdf4231cae8532fff691f794891b714ab Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Wed, 17 Jan 2024 19:52:32 -0500 Subject: [PATCH 1/5] Avoid revoking expired tokens and make token revocation best effort --- dist/main.cjs | 1 + dist/post.cjs | 28 ++++++++++++++++++++++------ lib/main.js | 1 + lib/post.js | 32 ++++++++++++++++++++++++++------ 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/dist/main.cjs b/dist/main.cjs index 2c5955f..0629cdb 100644 --- a/dist/main.cjs +++ b/dist/main.cjs @@ -10420,6 +10420,7 @@ async function main(appId2, privateKey2, owner2, repositories2, core2, createApp core2.setOutput("token", authentication.token); if (!skipTokenRevoke2) { core2.saveState("token", authentication.token); + core2.setOutput("expiresAt", authentication.expiresAt); } } async function getTokenFromOwner(request2, auth, parsedOwner) { diff --git a/dist/post.cjs b/dist/post.cjs index 44bde25..ee87366 100644 --- a/dist/post.cjs +++ b/dist/post.cjs @@ -3003,12 +3003,28 @@ async function post(core2, request2) { core2.info("Token is not set"); return; } - await request2("DELETE /installation/token", { - headers: { - authorization: `token ${token}` - } - }); - core2.info("Token revoked"); + const expiresAt = core2.getState("expiresAt"); + if (expiresAt && tokenExpiresIn(expiresAt) < 0) { + core2.info("Token already expired"); + return; + } + try { + await request2("DELETE /installation/token", { + headers: { + authorization: `token ${token}` + } + }); + core2.info("Token revoked"); + } catch (error) { + core2.warning( + `Token revocation failed: ${error.message}` + ); + } +} +function tokenExpiresIn(expiresAt) { + const now = /* @__PURE__ */ new Date(); + const expiresAtDate = new Date(expiresAt); + return Math.round((expiresAtDate.getTime() - now.getTime()) / 1e3); } // lib/request.js diff --git a/lib/main.js b/lib/main.js index 233be3d..b97329d 100644 --- a/lib/main.js +++ b/lib/main.js @@ -103,6 +103,7 @@ export async function main( // Make token accessible to post function (so we can invalidate it) if (!skipTokenRevoke) { core.saveState("token", authentication.token); + core.setOutput("expiresAt", authentication.expiresAt); } } diff --git a/lib/post.js b/lib/post.js index e321294..85f17d6 100644 --- a/lib/post.js +++ b/lib/post.js @@ -21,11 +21,31 @@ export async function post(core, request) { return; } - await request("DELETE /installation/token", { - headers: { - authorization: `token ${token}`, - }, - }); + const expiresAt = core.getState("expiresAt"); + if (expiresAt && tokenExpiresIn(expiresAt) < 0) { + core.info("Token already expired"); + return; + } + + try { + await request("DELETE /installation/token", { + headers: { + authorization: `token ${token}`, + }, + }); + core.info("Token revoked"); + } catch (error) { + core.warning( + `Token revocation failed: ${error.message}`) + } +} + +/** + * @param {string} expiresAt + */ +function tokenExpiresIn(expiresAt) { + const now = new Date(); + const expiresAtDate = new Date(expiresAt); - core.info("Token revoked"); + return Math.round((expiresAtDate.getTime() - now.getTime()) / 1000); } From 80d8e2dd1f060e32450791ce5e2f94dfd98beb63 Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Thu, 18 Jan 2024 13:48:47 -0500 Subject: [PATCH 2/5] Add tests for expired tokens and failed revocations --- tests/main.js | 3 +- tests/post-revoke-token-fail-response.test.js | 28 ++++++++++ tests/post-token-expired.test.js | 28 ++++++++++ tests/post-token-set.test.js | 3 + tests/snapshots/index.js.md | 52 +++++++++++++++--- tests/snapshots/index.js.snap | Bin 955 -> 1060 bytes 6 files changed, 105 insertions(+), 9 deletions(-) create mode 100644 tests/post-revoke-token-fail-response.test.js create mode 100644 tests/post-token-expired.test.js diff --git a/tests/main.js b/tests/main.js index 12c8437..6aba464 100644 --- a/tests/main.js +++ b/tests/main.js @@ -72,6 +72,7 @@ x3WQZRiXlWejSMUAHuMwXrhGlltF3lw83+xAjnqsVp75kGS6OH61 // Mock installation access token request const mockInstallationAccessToken = "ghs_16C7e42F292c6912E7710c838347Ae178B4a"; // This token is invalidated. It’s from https://docs.github.com/en/rest/apps/apps?apiVersion=2022-11-28#create-an-installation-access-token-for-an-app. + const mockExpiresAt = "2016-07-11T22:14:10Z"; mockPool .intercept({ path: `/app/installations/${mockInstallationId}/access_tokens`, @@ -84,7 +85,7 @@ x3WQZRiXlWejSMUAHuMwXrhGlltF3lw83+xAjnqsVp75kGS6OH61 }) .reply( 201, - { token: mockInstallationAccessToken }, + { token: mockInstallationAccessToken, expires_at: mockExpiresAt }, { headers: { "content-type": "application/json" } } ); diff --git a/tests/post-revoke-token-fail-response.test.js b/tests/post-revoke-token-fail-response.test.js new file mode 100644 index 0000000..5ce31ec --- /dev/null +++ b/tests/post-revoke-token-fail-response.test.js @@ -0,0 +1,28 @@ +import { MockAgent, setGlobalDispatcher } from "undici"; + +// state variables are set as environment variables with the prefix STATE_ +// https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#sending-values-to-the-pre-and-post-actions +process.env.STATE_token = "secret123"; + +// 1 hour in the future, not expired +process.env.STATE_expiresAt = new Date(Date.now() + 1000 * 60 * 60).toISOString(); + +const mockAgent = new MockAgent(); + +setGlobalDispatcher(mockAgent); + +// Provide the base url to the request +const mockPool = mockAgent.get("https://api.github.com"); + +// intercept the request +mockPool + .intercept({ + path: "/installation/token", + method: "DELETE", + headers: { + authorization: "token secret123", + }, + }) + .reply(401); + +await import("../post.js"); diff --git a/tests/post-token-expired.test.js b/tests/post-token-expired.test.js new file mode 100644 index 0000000..6479845 --- /dev/null +++ b/tests/post-token-expired.test.js @@ -0,0 +1,28 @@ +import { MockAgent, setGlobalDispatcher } from "undici"; + +// state variables are set as environment variables with the prefix STATE_ +// https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#sending-values-to-the-pre-and-post-actions +process.env.STATE_token = "secret123"; + +// 1 hour in the past, expired +process.env.STATE_expiresAt = new Date(Date.now() - 1000 * 60 * 60).toISOString(); + +const mockAgent = new MockAgent(); + +setGlobalDispatcher(mockAgent); + +// Provide the base url to the request +const mockPool = mockAgent.get("https://api.github.com"); + +// intercept the request +mockPool + .intercept({ + path: "/installation/token", + method: "DELETE", + headers: { + authorization: "token secret123", + }, + }) + .reply(204); + +await import("../post.js"); diff --git a/tests/post-token-set.test.js b/tests/post-token-set.test.js index 6ed0494..5b2479e 100644 --- a/tests/post-token-set.test.js +++ b/tests/post-token-set.test.js @@ -4,6 +4,9 @@ import { MockAgent, setGlobalDispatcher } from "undici"; // https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#sending-values-to-the-pre-and-post-actions process.env.STATE_token = "secret123"; +// 1 hour in the future, not expired +process.env.STATE_expiresAt = new Date(Date.now() + 1000 * 60 * 60).toISOString(); + const mockAgent = new MockAgent(); setGlobalDispatcher(mockAgent); diff --git a/tests/snapshots/index.js.md b/tests/snapshots/index.js.md index e7c6e86..0f82c4f 100644 --- a/tests/snapshots/index.js.md +++ b/tests/snapshots/index.js.md @@ -69,7 +69,9 @@ Generated by [AVA](https://avajs.dev). ::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ ␊ ::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ - ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a` + ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ + ␊ + ::set-output name=expiresAt::2016-07-11T22:14:10Z` ## main-token-get-owner-set-repo-set-to-many.test.js @@ -83,7 +85,9 @@ Generated by [AVA](https://avajs.dev). ::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ ␊ ::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ - ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a` + ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ + ␊ + ::set-output name=expiresAt::2016-07-11T22:14:10Z` ## main-token-get-owner-set-repo-set-to-one.test.js @@ -97,7 +101,9 @@ Generated by [AVA](https://avajs.dev). ::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ ␊ ::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ - ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a` + ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ + ␊ + ::set-output name=expiresAt::2016-07-11T22:14:10Z` ## main-token-get-owner-set-to-org-repo-unset.test.js @@ -111,7 +117,9 @@ Generated by [AVA](https://avajs.dev). ::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ ␊ ::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ - ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a` + ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ + ␊ + ::set-output name=expiresAt::2016-07-11T22:14:10Z` ## main-token-get-owner-set-to-user-fail-response.test.js @@ -126,7 +134,9 @@ Generated by [AVA](https://avajs.dev). ::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ ␊ ::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ - ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a` + ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ + ␊ + ::set-output name=expiresAt::2016-07-11T22:14:10Z` ## main-token-get-owner-set-to-user-repo-unset.test.js @@ -140,7 +150,9 @@ Generated by [AVA](https://avajs.dev). ::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ ␊ ::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ - ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a` + ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ + ␊ + ::set-output name=expiresAt::2016-07-11T22:14:10Z` ## main-token-get-owner-unset-repo-set.test.js @@ -154,7 +166,9 @@ Generated by [AVA](https://avajs.dev). ::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ ␊ ::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ - ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a` + ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ + ␊ + ::set-output name=expiresAt::2016-07-11T22:14:10Z` ## main-token-get-owner-unset-repo-unset.test.js @@ -168,7 +182,29 @@ Generated by [AVA](https://avajs.dev). ::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ ␊ ::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ - ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a` + ::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊ + ␊ + ::set-output name=expiresAt::2016-07-11T22:14:10Z` + +## post-revoke-token-fail-response.test.js + +> stderr + + '' + +> stdout + + '::warning::Token revocation failed: ' + +## post-token-expired.test.js + +> stderr + + '' + +> stdout + + 'Token already expired' ## post-token-set.test.js diff --git a/tests/snapshots/index.js.snap b/tests/snapshots/index.js.snap index f7989c60e0237f307e83e20af6fca8e5ef7c89d1..4fc35080e70f67d623b303281b81c9dc83977f0d 100644 GIT binary patch literal 1060 zcmV+<1l#*TRzV_Xj*-O{wyg6N`d+XHk} zx}uAO!X$o4hd3TH^PIL42gH>d;()lZ7yd8)66S}Kc&MFr>mng-a)~t_zxVa~d}qcp zfA+gE?+xXz&j1<5a876s8Be?jQUN`R5P2*~Gn8ux64$<$OeQpyryS+a&!UJT{o>`N zSC{m!@ygO0OAym8#C?eo2w^)sZO~$umyQ}VO*1VUo;p5}HKhjKwkO z0ueCcW7WnILPZQs9c{b1_ zpB%EZgM0`{N5EM9+pL$G>*dzrJQL2FhRYSq%FZ*Ku)H&(Eb&H6N|p>fTck>iCm$Cf zHJS&Sx)Sw6#zd|(;YehbrZIykBsN|N?^ggO(? zU(Uqyd&A^01g1q|AoaVglAZvH>XnI+MfzO0omR9eH67~46`~_Wq zx=1Dz6au-=-ld2zN|E^YLbAtjKkJg6#|Oq0bgX*PZVby|NTbLbQ8^65!Kv){o1eE~ zBX|^S2ffX0KiF-z{Z?;leQSNAy#s!G>){4nR!`|Fs#0suC3FOz7!}VELPels@FeP2 zxKV@>5pbR=^UEC!!=UAFdabtS`^P~L`WvC&`li$*{(O_@3*@RmNwv?fVUZsli;U`7 zB=^F7YXEP+@HFe1nb$nAI=PRW$3uptHhST?x30yVC$(mV*unnoe~Nv9xeRkqiCNPX zb{+FFwE|!r(1#A_&l(Onvkq4vrs}iI>8u+EY^-iuvn*InS#Hi_nMrkhYfJhc4$oh1 zlcyZ1txyc}SMo`x#VUS1YgB*gO|0?1ALmnFyznqTDurp-O&8}n5V-^KyL?r?OwYE@ zKt1PjthU&_ObAFYk92oBm)~1J_O60ld#wKZJgDO1Yu>2EQbOR1W- e3TkSO`jdAZEw5WGY&y!F{Qm$Q{XT$~761U@9shU$ literal 955 zcmV;s14R5mRzVoKTjEBE5QwO+dqKWh^uDP#S^}T)N zwuY!yN*{{|00000000B!nB7hjK@`WS#+ZK!<0LiL3_D>R5FJ1njv??6-+&{O|5 z>bd58v9+9M! zkx3kf3<956y)nGA3L@p~uH%fKMj*0{?Ks9Zc4`N^hcD{;+i&;Y)eiRdkL!Ey-)!%b zzS8SBq7hjgU-SO1rx^*7$%8<)MHyB zO#u|ukcqKHT3ooD7PKq19O}jeqLl*DI3o*o!ITTLugra&BqIeWA@|uuiU{2pN%^5q zb{Os^4YKvNW~zerR8QInVb=F)5O`fGJHFpOk@fQ0%POo^UR5?K&9#kkWxHA}FE`g$ z)>l@mTTrg9zgVTS>M5N?m15xW%wU3 zt7J0MsD#}vyhJUun1|YZhuU1h!C^7ew@y_vCVqwwk1EUVAdvqNu;{IvTd;K zz&W}~II^o=A3~VC`wcq%mpfnfS!clcpPzw4$kRpU@mMkj#Bf8)LuAt-;`Mj3^C)cl zDC>W?g{c?DW+DVc*jHK{E}0LfjF@*3vD~w$ilu_!b zA!8p>E*eMooVV1u1CwI==_45bT_GL1!{QjSKX_am{*e>@{wREt>gh#JsXB(*C}Z8+ dNx8{cm`4vBFSo20HY`LZ{x4O4(}UF$005uT#yS81 From ba87afd0e6cf6c4d9ebb097aeb855f4c9ca5ab70 Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Fri, 19 Jan 2024 09:33:16 -0500 Subject: [PATCH 3/5] Update token expired message for clarity Co-authored-by: Gregor Martynus <39992+gr2m@users.noreply.github.com> --- lib/post.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/post.js b/lib/post.js index 85f17d6..c60cbfc 100644 --- a/lib/post.js +++ b/lib/post.js @@ -23,7 +23,7 @@ export async function post(core, request) { const expiresAt = core.getState("expiresAt"); if (expiresAt && tokenExpiresIn(expiresAt) < 0) { - core.info("Token already expired"); + core.info("Token expired, skipping token revokation"); return; } From ba996b20c3e5fb2c06d7dd2bbfcb8644ce664b06 Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Fri, 19 Jan 2024 10:11:50 -0500 Subject: [PATCH 4/5] Fix typo --- lib/post.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/post.js b/lib/post.js index c60cbfc..9b294ae 100644 --- a/lib/post.js +++ b/lib/post.js @@ -23,7 +23,7 @@ export async function post(core, request) { const expiresAt = core.getState("expiresAt"); if (expiresAt && tokenExpiresIn(expiresAt) < 0) { - core.info("Token expired, skipping token revokation"); + core.info("Token expired, skipping token revocation"); return; } From 577d8a7ca3d5320a69796d2bda1172bcb77e68ba Mon Sep 17 00:00:00 2001 From: Josh Gross Date: Fri, 19 Jan 2024 10:12:08 -0500 Subject: [PATCH 5/5] Update snapshots --- tests/snapshots/index.js.md | 2 +- tests/snapshots/index.js.snap | Bin 1060 -> 1060 bytes 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/snapshots/index.js.md b/tests/snapshots/index.js.md index 0f82c4f..50a5112 100644 --- a/tests/snapshots/index.js.md +++ b/tests/snapshots/index.js.md @@ -204,7 +204,7 @@ Generated by [AVA](https://avajs.dev). > stdout - 'Token already expired' + 'Token expired, skipping token revocation' ## post-token-set.test.js diff --git a/tests/snapshots/index.js.snap b/tests/snapshots/index.js.snap index 4fc35080e70f67d623b303281b81c9dc83977f0d..47808b0eb787387e49ea9ca65bfd3e6451f456ba 100644 GIT binary patch literal 1060 zcmV+<1l#*TRzVQq_UN>STu^@jIt)OP3QH@o z*UdRcP#=p300000000B+n9pz9HWbI>6~ll41=icJ4jT~+aJpKlrDS#@v_;b)O>)>$ zWGR*o7(Ci~c34ZIN>WY=^ib@&+pxp1TYKsM)xTu?A=~02*=dvk18QF^P}F;0z0X&C z5?}YbA?*$MA8$Yq5yBb5Jwa&f1duT3VF7_d;v^M(O#l~b&p6>RPWTBG{I@qjfCc{k z+R`UW>R0=C>BbU-xC>#QivSoi6Q0&^e#%FOHJl_J5}|Bt{tZQ;{-2BVb(0TtiPd=opTU5pw}JSX)jQ8Ob=EA-phIas$;;Wq}+M zXq=A^NzxH?2yutONdB9w=Z350R^c2S&Y6VE6imxbQ!!?FGr}ZxBEmU|2aYLHDaPY( z^N?!weMMc0dY%%NDUGQ>JWZ02K;R=2uYh-%A#%z-x0KP-7+ATmT}#;3?*7r&G(D$Qk!x~>HqD-S>z2RJf?7Vq>i3FQ$_Xw0F? z!t>Tzc>b#CJet6yNCddLw+8Alz(U?K(y~yWGJDdBQYDu|HMoLQqyTkapcS)V@|ERB z`n)QV4h6YD_O+{62#6Aa+*>}{C1Nk_qV31~+7);tXX0)M%f63;z=<#)`u^aAcih%@ zZP@T0d0SquwdHy{?Y7(OZLV*wZ?w0;ZErr@z{~O~RYh59<+&I~a8Ijvj}S5f9)ZJ! zx`j(cC=dZ>i8Mdo7QXK_-ImjAJFa`=dA_^hyUm{qP2$cri8>He7AP+F`2{TU(y~ZY z%_5l>?id3&10qh+u3mZd6^-$EL1{Q7qR>VkJon~>xYM}O$`D%EfB8?bFEAHj_DV4; zy3F)3CzTyQQ~`Zpfqqc4$mu>@f*8x!GHbFL9FUPbZOpP@IbqqF#WLmc_{v)P-xkl` zuaYN^}a%FpV*_k&7azGgkO;shoPrysl$WEu1Y*t5DU zlgPG?GQL;TmXAGaKjCZNKgBGT#C-oqBU4FxSUqCMdwH2`Y+DmImH}hSqjyB emQz!4Avpfh((_Xj*-O{wyg6N`d+XHk} zx}uAO!X$o4hd3TH^PIL42gH>d;()lZ7yd8)66S}Kc&MFr>mng-a)~t_zxVa~d}qcp zfA+gE?+xXz&j1<5a876s8Be?jQUN`R5P2*~Gn8ux64$<$OeQpyryS+a&!UJT{o>`N zSC{m!@ygO0OAym8#C?eo2w^)sZO~$umyQ}VO*1VUo;p5}HKhjKwkO z0ueCcW7WnILPZQs9c{b1_ zpB%EZgM0`{N5EM9+pL$G>*dzrJQL2FhRYSq%FZ*Ku)H&(Eb&H6N|p>fTck>iCm$Cf zHJS&Sx)Sw6#zd|(;YehbrZIykBsN|N?^ggO(? zU(Uqyd&A^01g1q|AoaVglAZvH>XnI+MfzO0omR9eH67~46`~_Wq zx=1Dz6au-=-ld2zN|E^YLbAtjKkJg6#|Oq0bgX*PZVby|NTbLbQ8^65!Kv){o1eE~ zBX|^S2ffX0KiF-z{Z?;leQSNAy#s!G>){4nR!`|Fs#0suC3FOz7!}VELPels@FeP2 zxKV@>5pbR=^UEC!!=UAFdabtS`^P~L`WvC&`li$*{(O_@3*@RmNwv?fVUZsli;U`7 zB=^F7YXEP+@HFe1nb$nAI=PRW$3uptHhST?x30yVC$(mV*unnoe~Nv9xeRkqiCNPX zb{+FFwE|!r(1#A_&l(Onvkq4vrs}iI>8u+EY^-iuvn*InS#Hi_nMrkhYfJhc4$oh1 zlcyZ1txyc}SMo`x#VUS1YgB*gO|0?1ALmnFyznqTDurp-O&8}n5V-^KyL?r?OwYE@ zKt1PjthU&_ObAFYk92oBm)~1J_O60ld#wKZJgDO1Yu>2EQbOR1W- e3TkSO`jdAZEw5WGY&y!F{Qm$Q{XT$~761U@9shU$