From b57a06d58ca98217b449666c33fc98d55bda1536 Mon Sep 17 00:00:00 2001 From: Aboje Edwin Date: Wed, 21 Oct 2026 20:22:09 +0100 Subject: [PATCH 1/2] security(ci): disable persisted checkout credentials in read-only workflows - Add persist-credentials: false to all actions/checkout steps - Add comment explaining downstream steps do not push - Prevents GitHub token from being left in Git credential config - Affects workflows: bindings-check, contract, coverage, docs-links - No changes to checkout depth or existing job behavior Fixes #490 --- .github/workflows/bindings-check.yml | 3 +++ .github/workflows/contract.yml | 3 +++ .github/workflows/coverage.yml | 3 +++ .github/workflows/docs-links.yml | 3 +++ 4 files changed, 12 insertions(+) diff --git a/.github/workflows/bindings-check.yml b/.github/workflows/bindings-check.yml index 31366661..46460ab2 100644 --- a/.github/workflows/bindings-check.yml +++ b/.github/workflows/bindings-check.yml @@ -19,7 +19,10 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 60 steps: + # Read-only workflow: downstream steps do not push - uses: actions/checkout@v4 + with: + persist-credentials: false - name: Assert pinned Stellar CLI workflow run: node --test scripts/check-bindings-cli.test.mjs diff --git a/.github/workflows/contract.yml b/.github/workflows/contract.yml index 7fc74755..e20f433d 100644 --- a/.github/workflows/contract.yml +++ b/.github/workflows/contract.yml @@ -19,7 +19,10 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 60 steps: + # Read-only workflow: downstream steps do not push - uses: actions/checkout@v4 + with: + persist-credentials: false - name: Install Rust toolchain uses: dtolnay/rust-toolchain@stable diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 1ea4a3df..077150ac 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -19,7 +19,10 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 20 steps: + # Read-only workflow: downstream steps do not push - uses: actions/checkout@v4 + with: + persist-credentials: false - uses: pnpm/action-setup@v4 with: diff --git a/.github/workflows/docs-links.yml b/.github/workflows/docs-links.yml index 66f27f45..d9cfa969 100644 --- a/.github/workflows/docs-links.yml +++ b/.github/workflows/docs-links.yml @@ -22,7 +22,10 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 20 steps: + # Read-only workflow: downstream steps do not push - uses: actions/checkout@v4 + with: + persist-credentials: false - uses: actions/setup-node@v4 with: node-version: '22' From 738da53582e1e3b25b4fb8e7ee73c05a2aa217e0 Mon Sep 17 00:00:00 2001 From: Aboje Edwin Date: Fri, 23 Oct 2026 01:41:10 +0100 Subject: [PATCH 2/2] security(ci): cover all 7 checkout steps and add regression guard - Rebase on latest main to pick up web.yml, env-examples.yml, line-lengths.yml - Add persist-credentials: false + comment to the 3 new workflows - Add scripts/check-workflow-credentials.mjs as a repository-wide assertion that every actions/checkout step disables credential persistence - Wire wf-creds:check into the coverage workflow so future regressions fail CI - Add unit tests covering pass, fail, partial, and empty cases Fixes #490 --- .github/workflows/coverage.yml | 3 + .github/workflows/env-examples.yml | 3 + .github/workflows/line-lengths.yml | 3 + .github/workflows/web.yml | 3 + package.json | 4 +- scripts/check-workflow-credentials.mjs | 97 ++++++++++++++ scripts/check-workflow-credentials.test.mjs | 134 ++++++++++++++++++++ 7 files changed, 246 insertions(+), 1 deletion(-) create mode 100644 scripts/check-workflow-credentials.mjs create mode 100644 scripts/check-workflow-credentials.test.mjs diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 077150ac..8077dd11 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -49,3 +49,6 @@ jobs: - name: Run coverage gate run: pnpm coverage:test + + - name: Assert all workflow checkout steps disable credential persistence + run: pnpm wf-creds:check diff --git a/.github/workflows/env-examples.yml b/.github/workflows/env-examples.yml index 16d63a74..981d79c6 100644 --- a/.github/workflows/env-examples.yml +++ b/.github/workflows/env-examples.yml @@ -26,7 +26,10 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 5 steps: + # Read-only workflow: downstream steps do not push - uses: actions/checkout@v4 + with: + persist-credentials: false - uses: actions/setup-node@v4 with: node-version: '22' diff --git a/.github/workflows/line-lengths.yml b/.github/workflows/line-lengths.yml index 5aa61bba..634fe865 100644 --- a/.github/workflows/line-lengths.yml +++ b/.github/workflows/line-lengths.yml @@ -19,7 +19,10 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 10 steps: + # Read-only workflow: downstream steps do not push - uses: actions/checkout@v4 + with: + persist-credentials: false - uses: pnpm/action-setup@v4 with: diff --git a/.github/workflows/web.yml b/.github/workflows/web.yml index 2f6e7520..e31d4c45 100644 --- a/.github/workflows/web.yml +++ b/.github/workflows/web.yml @@ -17,7 +17,10 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 30 steps: + # Read-only workflow: downstream steps do not push - uses: actions/checkout@v4 + with: + persist-credentials: false - uses: pnpm/action-setup@v4 with: diff --git a/package.json b/package.json index 66e5b7c1..05db01e7 100644 --- a/package.json +++ b/package.json @@ -87,6 +87,8 @@ "errors:normalize:check": "node scripts/check-error-normalization.mjs", "errors:normalize:test": "pnpm --filter @sub-rosa/logging test && node --test scripts/check-error-normalization.test.mjs", "line-lengths:check": "node scripts/check-line-lengths.mjs", - "line-lengths:test": "node --test scripts/check-line-lengths.test.mjs" + "line-lengths:test": "node --test scripts/check-line-lengths.test.mjs", + "wf-creds:check": "node scripts/check-workflow-credentials.mjs", + "wf-creds:test": "node --test scripts/check-workflow-credentials.test.mjs" } } diff --git a/scripts/check-workflow-credentials.mjs b/scripts/check-workflow-credentials.mjs new file mode 100644 index 00000000..4ea211ff --- /dev/null +++ b/scripts/check-workflow-credentials.mjs @@ -0,0 +1,97 @@ +// Copyright (c) 2026 Sub Rosa contributors +// Repository-wide assertion: every actions/checkout step in .github/workflows +// must set persist-credentials: false so the GitHub token is not left in the +// local Git credential configuration after the job finishes. +import { readdirSync, readFileSync, existsSync } from "node:fs"; +import { join, relative, dirname } from "node:path"; +import { fileURLToPath } from "node:url"; + +const SCRIPT_DIR = dirname(fileURLToPath(import.meta.url)); +const DEFAULT_REPO_ROOT = join(SCRIPT_DIR, ".."); + +/** + * Find every actions/checkout step and verify persist-credentials: false + * is present in its with: block. Returns an array of violations. + */ +export function findViolations(filePath, repoRoot) { + const content = readFileSync(filePath, "utf-8"); + const lines = content.split("\n"); + const violations = []; + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + const checkoutMatch = line.match(/^\s*-\s*uses:\s*actions\/checkout/); + if (!checkoutMatch) continue; + + const stepIndent = line.match(/^(\s*)/)[1]; + + let foundPersist = false; + for (let j = i + 1; j < lines.length; j++) { + const next = lines[j]; + // A new step at the same or lower indentation ends this step block + if (/^\s*-\s/.test(next) && next.match(/^(\s*)/)[1].length <= stepIndent.length) { + break; + } + // A top-level or lower-indented non-empty line ends this step block + if (next.trim() !== "" && next.match(/^(\s*)/)[1].length < stepIndent.length) { + break; + } + if (/persist-credentials:\s*false/.test(next)) { + foundPersist = true; + break; + } + } + + if (!foundPersist) { + violations.push({ + file: relative(repoRoot, filePath), + line: i + 1, + }); + } + } + + return violations; +} + +export function checkRepo(repoRoot) { + const workflowsDir = join(repoRoot, ".github", "workflows"); + if (!existsSync(workflowsDir)) { + return { files: [], violations: [] }; + } + + const files = readdirSync(workflowsDir) + .filter((f) => f.endsWith(".yml") || f.endsWith(".yaml")) + .map((f) => join(workflowsDir, f)); + + const allViolations = []; + for (const file of files) { + allViolations.push(...findViolations(file, repoRoot)); + } + + return { files, violations: allViolations }; +} + +function main() { + const repoRoot = process.argv[2] || DEFAULT_REPO_ROOT; + const { files, violations } = checkRepo(repoRoot); + + if (violations.length > 0) { + for (const v of violations) { + process.stdout.write(`${v.file}:${v.line}: actions/checkout without persist-credentials: false\n`); + } + process.stdout.write( + `\nFound ${violations.length} checkout step(s) missing persist-credentials: false.\n` + + "Read-only workflows must not leave Git credentials on the runner. Add:\n" + + " with:\n" + + " persist-credentials: false\n" + ); + process.exit(1); + } + + process.stdout.write( + `All ${files.length} workflow file(s) have persist-credentials: false on every checkout step.\n` + ); + process.exit(0); +} + +main(); diff --git a/scripts/check-workflow-credentials.test.mjs b/scripts/check-workflow-credentials.test.mjs new file mode 100644 index 00000000..dbbf9752 --- /dev/null +++ b/scripts/check-workflow-credentials.test.mjs @@ -0,0 +1,134 @@ +// Copyright (c) 2026 Sub Rosa contributors +import { execFileSync } from "node:child_process"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; + +const CHECKER = new URL("check-workflow-credentials.mjs", import.meta.url).pathname; +const REPO_ROOT = new URL("..", import.meta.url).pathname; + +function run(repoRoot) { + try { + const stdout = execFileSync("node", [CHECKER, repoRoot], { encoding: "utf-8" }); + return { code: 0, stdout }; + } catch (error) { + return { code: error.status, stdout: error.stdout ?? "" }; + } +} + +/** Create a temp repo skeleton with the given workflow files. */ +function fakeRepo(workflows) { + const raiz = mkdtempSync(join(tmpdir(), "wf-cred-")); + const wfDir = join(raiz, ".github", "workflows"); + mkdirSync(wfDir, { recursive: true }); + for (const [name, content] of Object.entries(workflows)) { + writeFileSync(join(wfDir, name), content); + } + return raiz; +} + +const GOOD_WORKFLOW = [ + "name: Good", + "on: [push]", + "permissions:", + " contents: read", + "jobs:", + " test:", + " runs-on: ubuntu-latest", + " steps:", + " # Read-only workflow: downstream steps do not push", + " - uses: actions/checkout@v4", + " with:", + " persist-credentials: false", + " - run: echo hi", + "", +].join("\n"); + +const BAD_WORKFLOW = [ + "name: Bad", + "on: [push]", + "permissions:", + " contents: read", + "jobs:", + " test:", + " runs-on: ubuntu-latest", + " steps:", + " - uses: actions/checkout@v4", + " - run: echo hi", + "", +].join("\n"); + +const PARTIAL_WORKFLOW = [ + "name: Partial", + "on: [push]", + "permissions:", + " contents: read", + "jobs:", + " test:", + " runs-on: ubuntu-latest", + " steps:", + " # Read-only workflow: downstream steps do not push", + " - uses: actions/checkout@v4", + " with:", + " persist-credentials: false", + " - run: echo first", + " - uses: actions/checkout@v4", + " - run: echo second", + "", +].join("\n"); + +describe("check-workflow-credentials", () => { + it("passes on the real repository — all checkout steps have persist-credentials: false", () => { + const { code, stdout } = run(REPO_ROOT); + assert.equal(code, 0, stdout); + assert.match(stdout, /persist-credentials: false on every checkout step/); + }); + + it("passes when all checkout steps have persist-credentials: false", () => { + const raiz = fakeRepo({ "good.yml": GOOD_WORKFLOW }); + try { + const { code, stdout } = run(raiz); + assert.equal(code, 0, stdout); + assert.match(stdout, /1 workflow file\(s\)/); + } finally { + rmSync(raiz, { recursive: true, force: true }); + } + }); + + it("fails when a checkout step is missing persist-credentials: false", () => { + const raiz = fakeRepo({ "bad.yml": BAD_WORKFLOW }); + try { + const { code, stdout } = run(raiz); + assert.notEqual(code, 0); + assert.match(stdout, /bad\.yml:9/); + assert.match(stdout, /persist-credentials: false/); + } finally { + rmSync(raiz, { recursive: true, force: true }); + } + }); + + it("catches the second checkout when only the first has persist-credentials: false", () => { + const raiz = fakeRepo({ "partial.yml": PARTIAL_WORKFLOW }); + try { + const { code, stdout } = run(raiz); + assert.notEqual(code, 0); + assert.match(stdout, /partial\.yml:14/); + assert.doesNotMatch(stdout, /partial\.yml:8/); + } finally { + rmSync(raiz, { recursive: true, force: true }); + } + }); + + it("passes when there are no workflow files", () => { + const raiz = mkdtempSync(join(tmpdir(), "wf-empty-")); + try { + const { code, stdout } = run(raiz); + assert.equal(code, 0); + assert.match(stdout, /0 workflow file\(s\)/); + } finally { + rmSync(raiz, { recursive: true, force: true }); + } + }); +});