Harden /merge workflow security

.github/workflows/merge.yml

@@ -13,21 +13,31 @@ jobs:
     runs-on: ubuntu-latest
     if: >
       github.event.issue.pull_request &&
-      github.event.comment.body == '/merge'
+      github.event.comment.body == '/merge' &&
+      github.event.comment.author_association != 'NONE'
     steps:
       - uses: actions/checkout@v6
-      - run: npm install js-yaml
+
+      - name: Setup Node.js
+        uses: actions/setup-node@v6
+        with:
+          node-version: "24"
+          cache: "npm"
+
+      - run: npm ci
 
       - name: Verify actor is authorized
         uses: actions/github-script@v9
         with:
           script: |
-            const verifyMerge = require('./scripts/verify-merge.js');
+            const { default: verifyMerge } = await import(
+              `${process.env.GITHUB_WORKSPACE}/scripts/verify-merge.js`
+            );
             await verifyMerge({ github, context });
 
       - name: Merge PR
         env:
           GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           PR_NUMBER: ${{ github.event.issue.number }}
         run: |
-          gh pr merge "$PR_NUMBER" --squash --auto
+          gh pr merge "$PR_NUMBER" --squash

scripts/verify-merge.js

@@ -1,32 +1,70 @@
-const fs = require("fs");
-const yaml = require("js-yaml");
+import { readFile } from "fs/promises";
+import path from "path";
+import { parse as parseYaml } from "yaml";
 
-module.exports = async ({ github, context }) => {
+export default async ({ github, context }) => {
   const actor = context.payload.comment.user.login;
   const prNumber = context.issue.number;
 
-  const { data: files } = await github.rest.pulls.listFiles({
-    ...context.repo,
-    pull_number: prNumber,
-  });
+  const [{ data: pr }, { data: files }] = await Promise.all([
+    github.rest.pulls.get({
+      ...context.repo,
+      pull_number: prNumber,
+    }),
+    github.rest.pulls.listFiles({
+      ...context.repo,
+      pull_number: prNumber,
+      per_page: 100,
+    }),
+  ]);
+
+  if (pr.mergeable === false) {
+    throw new Error(
+      "PR is not in a mergeable state. Resolve conflicts and try again.",
+    );
+  }
+
+  const gapDirSet = new Set();
 
-  const gapDirs = files
-    .map((f) => f.filename)
-    .map((p) => p.split("/").slice(0, 2).join("/"));
+  for (const f of files) {
+    const normalized = path.normalize(f.filename);
+    if (normalized !== f.filename || normalized.startsWith("..")) {
+      throw new Error(
+        `File path "${f.filename}" contains path traversal or is not normalized.`,
+      );
+    }
+    gapDirSet.add(f.filename.split("/").slice(0, 2).join("/"));
+  }
 
-  const gapsChanged = [...new Set(gapDirs)];
+  const gapsChanged = [...gapDirSet];
 
   if (gapsChanged.length !== 1 || !gapsChanged[0].match(/^gaps\/GAP-\d+$/)) {
-    throw new Error("You can only run /merge for PRs that touch exactly one GAP directory and nothing else.");
+    throw new Error(
+      "You can only run /merge for PRs that touch exactly one GAP directory and nothing else.",
+    );
+  }
+
+  const gapDir = gapsChanged[0];
+
+  for (const f of files) {
+    if (!f.filename.startsWith(`${gapDir}/`)) {
+      throw new Error(
+        `File "${f.filename}" is outside the expected GAP directory (${gapDir}).`,
+      );
+    }
   }
 
-  const metadata = yaml.load(fs.readFileSync(`${gapsChanged[0]}/metadata.yml`, "utf8"));
+  const metadata = parseYaml(
+    await readFile(`${gapDir}/metadata.yml`, "utf8"),
+  );
   const authorizedMergers = new Set([
-    ...metadata.authors.map(author => author.githubUsername.replace(/^@/, '')),
-    metadata.sponsor.replace(/^@/, ''),
+    ...metadata.authors.map((author) =>
+      author.githubUsername.replace(/^@/, ""),
+    ),
+    metadata.sponsor.replace(/^@/, ""),
   ]);
 
   if (!authorizedMergers.has(actor)) {
-    throw new Error(`${actor} is not authorized to merge ${gapsChanged[0]}.`);
+    throw new Error(`${actor} is not authorized to merge ${gapDir}.`);
   }
 };