Skip to content

Ci: restrict GITHUB_TOKEN to contents: read in the three read-only CI workflows - #649

Open
AmaadMartin wants to merge 2 commits into
mainfrom
fix/ci-workflow-least-privilege-token
Open

Ci: restrict GITHUB_TOKEN to contents: read in the three read-only CI workflows#649
AmaadMartin wants to merge 2 commits into
mainfrom
fix/ci-workflow-least-privilege-token

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

  1. Link to an existing issue (if applicable):
    N/A — no existing issue.
  2. Or, if no issue exists, describe the change:

Problem: Three of the six workflows in .github/workflows/ declare no top-level permissions: key, so the GITHUB_TOKEN issued to their runs silently inherits the repository/organization default — which on older repositories is read/write on every scope:

Workflow permissions: before Every step it runs
validation.yaml absent checkout, setup-node, setup-python, npm install, npx secretlint, npm run build, npm run test:coverage, npm run lint, npm run format:check, npm run docs:check
cross-language-integration.yml absent checkout, setup-node, setup-go, npm install, go mod tidy ×2, npm run build, npm run test:cross-language
license-check.yml absent checkout, bash scripts/check_license.sh
csat.yml contents: read, issues: write, pull-requests: write — already correct
auto-assignment.yml contents: read, issues: write, pull-requests: write — already correct
release-please.yml contents: write, issues: write, pull-requests: write — genuinely needs write

I audited the first three step by step: not one step writes anything back to GitHub — no git push, no artifact or coverage upload, no issue/PR comment, no release asset. scripts/check_license.sh is a pure find + perl read that prints missing headers and exits 1. They are read-build-test workflows running with a token that may be able to push to main.

The blast radius is concrete rather than theoretical: validation.yaml and cross-language-integration.yml both run npm install, which executes lifecycle scripts from the entire transitive dependency tree, in the same job as that token.

Solution: Two additions per file, 15 added lines across 3 files, 0 deleted lines.

  1. permissions: contents: read at the top level of all three workflows. A top-level permissions: block is a complete specification — every scope not listed is reset to none, it does not stay at the default — so this grants exactly "may read the repository, may do nothing else" (GitHub additionally grants an implicit, non-removable Metadata: read), and any job later added to these files inherits the same floor. Placement follows the existing in-repo pattern in csat.yml (lines 8-11) and auto-assignment.yml (lines 9-12), i.e. between on: and jobs:. validation.yaml is the one file with an env: block, so the key order there is on:permissions:env:jobs:.

  2. persist-credentials: false on each actions/checkout step. By default the checkout writes an http.extraheader credential into .git/config, leaving a usable token in the workspace for every subsequent step in the job. Nothing in these jobs needs it, and I verified each claim against the repo rather than assuming:

    • package.json and package-lock.json contain zero git+https, git+ssh or github: specifiers (grep -cE '"resolved": "git\+' package-lock.json0), so npm install never performs an authenticated git fetch.
    • No source under core/, dev/, integrations/, tests/ or scripts/ shells out to git.
    • The two go.mod files under tests/cross_language/a2a/ list only public modules with no replace directives, resolved through the public Go module proxy.

    persist-credentials: false does not affect the checkout's own fetch — that still authenticates normally. It only stops the action from leaving the credential behind afterwards.

Deliberately out of scope, so each concern stays independently reviewable and revertible: no uses: line is modified (SHA-pinning and action major bumps are separate), no job/step/trigger/matrix/runs-on/env: value changes, and the scopes on csat.yml / auto-assignment.yml / release-please.yml are left alone. No extra scope was added "just in case" — contents: read and nothing more.

Collision check (before implementing): enumerated all 548 open PRs on the fork; 46 touch .github/workflows/. I diffed every one of those 46 and none adds a permissions: block or persist-credentials: to any workflow, so this is not a duplicate. Several are textually adjacent to the same files but address different concerns and different lines — SHA-pinning (#505, #613), concurrency groups (#504), timeout-minutes (#403), Node pinning / npm ci / npm cache (#406, #415, #416, #428, #467, #508, #509, #571, #572, #574, #593 and others). This PR is branched from main rather than stacked, because it shares no line with any of them.

Testing Plan

Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.

Unit Tests:
[x] I have added or updated unit tests for my change. — N/A, and deliberately so. This change adds no executable line to any package: it is four YAML keys across three GitHub Actions configuration files. There is no module to import and no branch to exercise. A test that reads a workflow file and asserts on its own literal contents would be a tautology — it would pass equally with and against the change's intent. Verification is provided by the shape check and the live workflow run below, which exercise the real artifact. Nothing under core/, dev/, integrations/, tests/ or scripts/ is touched, so no existing test is affected.
[x] All unit tests pass locally. — no test-bearing code changed; the repo's own gates do not read YAML (npm run format:check is prettier "**/*.ts" --check and npm run lint is eslint "**/*.ts", so neither touches .yaml/.yml, and there is no reformatting risk).

Static parse and shape check — run from the repository root, prints OK three times:

python3 - <<'PY'
import yaml
files = [
    ".github/workflows/validation.yaml",
    ".github/workflows/cross-language-integration.yml",
    ".github/workflows/license-check.yml",
]
for f in files:
    doc = yaml.safe_load(open(f))
    assert doc["permissions"] == {"contents": "read"}, (f, doc.get("permissions"))
    steps = [s for j in doc["jobs"].values() for s in j["steps"]]
    checkouts = [s for s in steps if str(s.get("uses", "")).startswith("actions/checkout")]
    assert checkouts, f"{f}: no checkout step found"
    for s in checkouts:
        assert s.get("with", {}).get("persist-credentials") is False, (f, s)
    print("OK", f)
PY
OK .github/workflows/validation.yaml
OK .github/workflows/cross-language-integration.yml
OK .github/workflows/license-check.yml

Proving the check can fail (mutation testing). A check that passes with and without the change is a green light with no signal, so I ran it against four mutations. All four failed:

# Mutation Result
1 Run against the unmodified files at the base commit (git archive HEAD~2 .github) KeyError: 'permissions'
2 contents: readcontents: write in validation.yaml AssertionError: ('.github/workflows/validation.yaml', {'contents': 'write'})
3 persist-credentials: falsetrue in license-check.yml AssertionError: ('.github/workflows/license-check.yml', {'name': 'Checkout repository', 'uses': 'actions/checkout@v6', 'with': {'persist-credentials': True}})
4 Delete the whole with: block from the cross-language-integration.yml checkout AssertionError: ('.github/workflows/cross-language-integration.yml', {'name': 'Checkout code', 'uses': 'actions/checkout@v6'})

Diff shape checkgit diff --stat against the base:

 .github/workflows/cross-language-integration.yml | 5 +++++
 .github/workflows/license-check.yml              | 5 +++++
 .github/workflows/validation.yaml                | 5 +++++
 3 files changed, 15 insertions(+)

Exactly three files, 15 insertions, 0 deletions, and git diff -U0 | grep -cE '^[+-].*uses:'0, confirming no uses: line was touched and nothing was reformatted.

Resulting permissions: across all six workflows (the three untouched files are unchanged):

auto-assignment.yml               {'contents': 'read', 'issues': 'write', 'pull-requests': 'write'}
cross-language-integration.yml    {'contents': 'read'}
csat.yml                          {'contents': 'read', 'issues': 'write', 'pull-requests': 'write'}
license-check.yml                 {'contents': 'read'}
release-please.yml                {'contents': 'write', 'issues': 'write', 'pull-requests': 'write'}
validation.yaml                   {'contents': 'read'}

actionlint was attempted (npx --yes actionlint .github/workflows/*) but could not be fetched in this environment; it is not a gate in this repo.

Manual End-to-End (E2E) Tests:

This pull request is itself the E2E run. All three workflows trigger on pull_request: branches: [main], and GitHub evaluates the workflow definition from the PR's merge ref rather than from the base branch, so opening this PR exercises all three files with the new permissions: block in effect. No separate scratch PR is needed.

To reproduce:

  1. Confirm all three workflows appear in the PR's checks list: validation (ubuntu, windows and macos legs), Cross-Language Tests, and License Header Check. A missing check would mean a YAML parse failure.
  2. Wait for them to complete and confirm each is green.
  3. Open the "Set up job" step of any of the three runs and read the GITHUB_TOKEN Permissions section. It now lists Contents: read (plus Metadata: read, which GitHub always grants implicitly and which no permissions: block can remove) — the direct evidence that the change did what it claims, which a diff alone cannot show.

Result of running exactly that on this PR. All three workflows appeared in the checks list, so all three parsed. GITHUB_TOKEN Permissions from the "Set up job" step of one run of each:

Workflow Job GITHUB_TOKEN Permissions checkout input
validation.yaml run-tests (ubuntu-latest) Contents: read, Metadata: read persist-credentials: false
cross-language-integration.yml run-tests Contents: read, Metadata: read persist-credentials: false
license-check.yml check-license Contents: read, Metadata: read persist-credentials: false

Before/after. For contrast, here is the same "Set up job" section from a License Header Check run on another branch that does not carry this change — the inherited default this PR replaces:

##[group]GITHUB_TOKEN Permissions        ##[group]GITHUB_TOKEN Permissions
Actions: write                           Contents: read
ArtifactMetadata: write                  Metadata: read
Attestations: write                      ##[endgroup]
Checks: write
CodeQuality: write                       (after this PR)
Contents: write
CopilotRequests: write
Deployments: write
Discussions: write
Drives: write
Issues: write
Metadata: read
Models: read
Packages: write
Pages: write
PullRequests: write
RepositoryProjects: write
SecurityEvents: write
Statuses: write
##[endgroup]

(before — inherited default)

19 scopes, 17 of them write including Contents: write, reduced to Contents: read. Every step still passed, so no step needed any of the scopes that were dropped.

persist-credentials: false verified empirically too. Grepping all three workflows' full run logs for could not read Username, Resource not accessible by integration, HTTP 403, fatal: Authentication, and remote: Permission returns zero hits. The riskiest step is go mod tidy in cross-language-integration.yml, since its module paths are github.com/...; it resolved every module through the public Go proxy with no credential (go: downloading google.golang.org/adk v1.0.0, github.com/google/go-cmp v0.7.0, …) and npm run test:cross-language then reported Test Files 2 passed (2). npm install likewise completed on all four legs.

Checks result. License Header Check, Cross-Language Tests, and validation on ubuntu and windows all passed on the first attempt. The macOS validation leg failed on two pre-existing, timing-sensitive integration timeouts unrelated to this change:

FAIL integration tests/integration/app_loader/app_loader_test.ts > ... > should discover apps vs agents across directories and standalone files
  Error: Test timed out in 40000ms.
FAIL integration tests/integration/build_setup/build_setup_test.ts > Build setup > ts_commonjs > should build and run agent successfully
  Error: Test timed out in 20000ms.
Test Files  2 failed | 222 passed | 20 skipped (244)

This is a pre-existing macOS flake, not a consequence of this change, and I checked rather than assumed:

  • They are timeouts, not authorization failures. A token-scope problem surfaces as HTTP 403 / "Resource not accessible by integration" at a network step. Grepping the failing job's full log for could not read Username, Resource not accessible by integration, HTTP 403 and fatal: Authentication returns zero hits, and the job had already completed checkout, npm install, npx secretlint and npm run build successfully before reaching the tests.
  • Neither test can observe this change. grep -licE '\bgit\b|GITHUB_TOKEN|persist-credentials|\.github/workflows' over both app_loader_test.ts and build_setup_test.ts returns 0 — neither reads git, the token, or a workflow file.
  • The other three legs ran the identical workflow definition and passed (ubuntu, windows, and both other workflows).
  • The same failure reproduces on branches that do not contain this change. Walking the per-attempt history of the two most recent green validation runs on unrelated branches, the macOS leg failed on app_loader_test.ts on attempts 1-4 of fix/a2a-agent-card-invocation-context (green on attempt 5) and on attempts 1-3 of fix/tool-test-type-checker-suppressions (green on attempt 4). The test's budget is TEST_EXECUTION_TIMEOUT = 40000, which the macOS runner sits right on the edge of.

Re-running the leg per that evidence, rather than attributing it to this change or "fixing" it here — that would break scope. It went green on the third attempt, matching the pattern above. Final state: all six checks greencheck-license, run-tests (Cross-Language), and run-tests on ubuntu-latest, windows-latest and macos-latest.

How to read a red run, should one appear later:

  • HTTP 403 / "Resource not accessible by integration" — that workflow genuinely needs a scope beyond contents: read. Add only that one scope to only that one workflow with a comment; do not delete the permissions: block.
  • A git failure (fatal: could not read Username, a failed git fetch, or npm resolving a git URL) points at persist-credentials: false, not at the permissions block. Remove that one line from that one workflow and keep everything else.
  • Reverting the entire change is deleting the 15 added lines.

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas. — the rationale is in the two commit messages and this description; no inline comments were added, since a bare permissions: contents: read is self-documenting and the repo's existing blocks carry no comment either.
[x] I have added tests that prove my fix is effective or that my feature works. — see the shape check, its four mutation proofs, and the live workflow run above; a unit test is not applicable for a change that adds no executable line.
[x] New and existing unit tests pass locally with my changes.

Amaad Martin added 2 commits August 4, 2026 11:18
validation.yaml, cross-language-integration.yml and license-check.yml
declared no top-level permissions:, so their GITHUB_TOKEN inherited the
repository default, which on older repositories is read/write on every
scope. All three only read the repository: they checkout, install, build
and test, and no step pushes, uploads an artifact, comments on an issue
or PR, or publishes a release asset.

A top-level permissions: block is a complete specification - every scope
not listed is reset to none - so contents: read grants exactly repository
read and nothing else, and any job later added to these files inherits
the same floor.

Placement follows the existing csat.yml and auto-assignment.yml pattern
(between on: and jobs:); validation.yaml also has an env: block, so the
order there is on: -> permissions: -> env: -> jobs:.

csat.yml, auto-assignment.yml and release-please.yml already declare
permissions and are deliberately untouched - release-please genuinely
needs contents: write.
With persist-credentials left at its default the checkout writes an
http.extraheader credential into .git/config, leaving a usable token in
the workspace for every later step in the job - including npm install,
which runs lifecycle scripts from the whole transitive dependency tree.

Nothing in these three jobs needs it: package.json and package-lock.json
contain no git+https, git+ssh or github: specifiers, no source under
core/, dev/, integrations/, tests/ or scripts/ shells out to git, and the
two go.mod files under tests/cross_language/a2a/ list only public modules
resolved through the Go module proxy. persist-credentials: false does not
affect the checkout's own fetch, which still authenticates normally.

No uses: line is touched; pinning actions to commit SHAs is a separate
concern.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant