fix(revocation): avoid revoking expired tokens and fail gracefully (a…

…ctions#95) Fixes actions#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.
suchithrzp · Jan 19, 2024 · 0c01407 · 0c01407
1 parent f04aa94
commit 0c01407
Show file tree

Hide file tree

Showing 10 changed files with 155 additions and 21 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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 expired, skipping token revocation");
+    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);
 }
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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 expired, skipping token revocation'
 
 ## post-token-set.test.js
 

diff --git a/tests/snapshots/index.js.snap b/tests/snapshots/index.js.snap