Skip to content

Commit 0c01407

Browse files
joshmgrossjoshmgross
authored
fix(revocation): avoid revoking expired tokens and fail gracefully (#95)
Fixes #72 If an Actions job is long enough, more than an hour can pass between creating and revoking the App token in the post-job clean up step. Since the token itself is used to authenticate with the revoke API, an expired token will fail to be revoked. This PR saves the token expiration in the actions state and uses that in the post step to determine if the token can be revoked. I've also added error handling to the revoke token API call, as it's unlikely that users would want their job to fail if the token can't be revoked.
1 parent f04aa94 commit 0c01407

10 files changed

Lines changed: 155 additions & 21 deletions

File tree

‎dist/main.cjs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10420,6 +10420,7 @@ async function main(appId2, privateKey2, owner2, repositories2, core2, createApp
1042010420
core2.setOutput("token", authentication.token);
1042110421
if (!skipTokenRevoke2) {
1042210422
core2.saveState("token", authentication.token);
10423+
core2.setOutput("expiresAt", authentication.expiresAt);
1042310424
}
1042410425
}
1042510426
async function getTokenFromOwner(request2, auth, parsedOwner) {

‎dist/post.cjs‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3003,12 +3003,28 @@ async function post(core2, request2) {
30033003
core2.info("Token is not set");
30043004
return;
30053005
}
3006-
await request2("DELETE /installation/token", {
3007-
headers: {
3008-
authorization: `token ${token}`
3009-
}
3010-
});
3011-
core2.info("Token revoked");
3006+
const expiresAt = core2.getState("expiresAt");
3007+
if (expiresAt && tokenExpiresIn(expiresAt) < 0) {
3008+
core2.info("Token already expired");
3009+
return;
3010+
}
3011+
try {
3012+
await request2("DELETE /installation/token", {
3013+
headers: {
3014+
authorization: `token ${token}`
3015+
}
3016+
});
3017+
core2.info("Token revoked");
3018+
} catch (error) {
3019+
core2.warning(
3020+
`Token revocation failed: ${error.message}`
3021+
);
3022+
}
3023+
}
3024+
function tokenExpiresIn(expiresAt) {
3025+
const now = /* @__PURE__ */ new Date();
3026+
const expiresAtDate = new Date(expiresAt);
3027+
return Math.round((expiresAtDate.getTime() - now.getTime()) / 1e3);
30123028
}
30133029

30143030
// lib/request.js

‎lib/main.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ export async function main(
103103
// Make token accessible to post function (so we can invalidate it)
104104
if (!skipTokenRevoke) {
105105
core.saveState("token", authentication.token);
106+
core.setOutput("expiresAt", authentication.expiresAt);
106107
}
107108
}
108109

‎lib/post.js‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,31 @@ export async function post(core, request) {
2121
return;
2222
}
2323

24-
await request("DELETE /installation/token", {
25-
headers: {
26-
authorization: `token ${token}`,
27-
},
28-
});
24+
const expiresAt = core.getState("expiresAt");
25+
if (expiresAt && tokenExpiresIn(expiresAt) < 0) {
26+
core.info("Token expired, skipping token revocation");
27+
return;
28+
}
29+
30+
try {
31+
await request("DELETE /installation/token", {
32+
headers: {
33+
authorization: `token ${token}`,
34+
},
35+
});
36+
core.info("Token revoked");
37+
} catch (error) {
38+
core.warning(
39+
`Token revocation failed: ${error.message}`)
40+
}
41+
}
42+
43+
/**
44+
* @param {string} expiresAt
45+
*/
46+
function tokenExpiresIn(expiresAt) {
47+
const now = new Date();
48+
const expiresAtDate = new Date(expiresAt);
2949

30-
core.info("Token revoked");
50+
return Math.round((expiresAtDate.getTime() - now.getTime()) / 1000);
3151
}

‎tests/main.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ x3WQZRiXlWejSMUAHuMwXrhGlltF3lw83+xAjnqsVp75kGS6OH61
7272
// Mock installation access token request
7373
const mockInstallationAccessToken =
7474
"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.
75+
const mockExpiresAt = "2016-07-11T22:14:10Z";
7576
mockPool
7677
.intercept({
7778
path: `/app/installations/${mockInstallationId}/access_tokens`,
@@ -84,7 +85,7 @@ x3WQZRiXlWejSMUAHuMwXrhGlltF3lw83+xAjnqsVp75kGS6OH61
8485
})
8586
.reply(
8687
201,
87-
{ token: mockInstallationAccessToken },
88+
{ token: mockInstallationAccessToken, expires_at: mockExpiresAt },
8889
{ headers: { "content-type": "application/json" } }
8990
);
9091

‎tests/post-revoke-token-fail-response.test.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import { MockAgent, setGlobalDispatcher } from "undici";
2+
3+
// state variables are set as environment variables with the prefix STATE_
4+
// https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#sending-values-to-the-pre-and-post-actions
5+
process.env.STATE_token = "secret123";
6+
7+
// 1 hour in the future, not expired
8+
process.env.STATE_expiresAt = new Date(Date.now() + 1000 * 60 * 60).toISOString();
9+
10+
const mockAgent = new MockAgent();
11+
12+
setGlobalDispatcher(mockAgent);
13+
14+
// Provide the base url to the request
15+
const mockPool = mockAgent.get("https://api.github.com");
16+
17+
// intercept the request
18+
mockPool
19+
.intercept({
20+
path: "/installation/token",
21+
method: "DELETE",
22+
headers: {
23+
authorization: "token secret123",
24+
},
25+
})
26+
.reply(401);
27+
28+
await import("../post.js");

‎tests/post-token-expired.test.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import { MockAgent, setGlobalDispatcher } from "undici";
2+
3+
// state variables are set as environment variables with the prefix STATE_
4+
// https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#sending-values-to-the-pre-and-post-actions
5+
process.env.STATE_token = "secret123";
6+
7+
// 1 hour in the past, expired
8+
process.env.STATE_expiresAt = new Date(Date.now() - 1000 * 60 * 60).toISOString();
9+
10+
const mockAgent = new MockAgent();
11+
12+
setGlobalDispatcher(mockAgent);
13+
14+
// Provide the base url to the request
15+
const mockPool = mockAgent.get("https://api.github.com");
16+
17+
// intercept the request
18+
mockPool
19+
.intercept({
20+
path: "/installation/token",
21+
method: "DELETE",
22+
headers: {
23+
authorization: "token secret123",
24+
},
25+
})
26+
.reply(204);
27+
28+
await import("../post.js");

‎tests/post-token-set.test.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ import { MockAgent, setGlobalDispatcher } from "undici";
44
// https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#sending-values-to-the-pre-and-post-actions
55
process.env.STATE_token = "secret123";
66

7+
// 1 hour in the future, not expired
8+
process.env.STATE_expiresAt = new Date(Date.now() + 1000 * 60 * 60).toISOString();
9+
710
const mockAgent = new MockAgent();
811

912
setGlobalDispatcher(mockAgent);

‎tests/snapshots/index.js.md‎

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,9 @@ Generated by [AVA](https://avajs.dev).
6969
::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
7070
␊
7171
::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
72-
::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a`
72+
::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
73+
␊
74+
::set-output name=expiresAt::2016-07-11T22:14:10Z`
7375

7476
## main-token-get-owner-set-repo-set-to-many.test.js
7577

@@ -83,7 +85,9 @@ Generated by [AVA](https://avajs.dev).
8385
::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
8486
␊
8587
::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
86-
::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a`
88+
::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
89+
␊
90+
::set-output name=expiresAt::2016-07-11T22:14:10Z`
8791

8892
## main-token-get-owner-set-repo-set-to-one.test.js
8993

@@ -97,7 +101,9 @@ Generated by [AVA](https://avajs.dev).
97101
::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
98102
␊
99103
::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
100-
::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a`
104+
::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
105+
␊
106+
::set-output name=expiresAt::2016-07-11T22:14:10Z`
101107

102108
## main-token-get-owner-set-to-org-repo-unset.test.js
103109

@@ -111,7 +117,9 @@ Generated by [AVA](https://avajs.dev).
111117
::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
112118
␊
113119
::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
114-
::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a`
120+
::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
121+
␊
122+
::set-output name=expiresAt::2016-07-11T22:14:10Z`
115123

116124
## main-token-get-owner-set-to-user-fail-response.test.js
117125

@@ -126,7 +134,9 @@ Generated by [AVA](https://avajs.dev).
126134
::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
127135
␊
128136
::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
129-
::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a`
137+
::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
138+
␊
139+
::set-output name=expiresAt::2016-07-11T22:14:10Z`
130140

131141
## main-token-get-owner-set-to-user-repo-unset.test.js
132142

@@ -140,7 +150,9 @@ Generated by [AVA](https://avajs.dev).
140150
::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
141151
␊
142152
::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
143-
::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a`
153+
::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
154+
␊
155+
::set-output name=expiresAt::2016-07-11T22:14:10Z`
144156

145157
## main-token-get-owner-unset-repo-set.test.js
146158

@@ -154,7 +166,9 @@ Generated by [AVA](https://avajs.dev).
154166
::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
155167
␊
156168
::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
157-
::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a`
169+
::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
170+
␊
171+
::set-output name=expiresAt::2016-07-11T22:14:10Z`
158172

159173
## main-token-get-owner-unset-repo-unset.test.js
160174

@@ -168,7 +182,29 @@ Generated by [AVA](https://avajs.dev).
168182
::add-mask::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
169183
␊
170184
::set-output name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
171-
::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a`
185+
::save-state name=token::ghs_16C7e42F292c6912E7710c838347Ae178B4a␊
186+
␊
187+
::set-output name=expiresAt::2016-07-11T22:14:10Z`
188+
189+
## post-revoke-token-fail-response.test.js
190+
191+
> stderr
192+
193+
''
194+
195+
> stdout
196+
197+
'::warning::Token revocation failed: '
198+
199+
## post-token-expired.test.js
200+
201+
> stderr
202+
203+
''
204+
205+
> stdout
206+
207+
'Token expired, skipping token revocation'
172208

173209
## post-token-set.test.js
174210

‎tests/snapshots/index.js.snap‎

105 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)