INF-09: ci.yml has no workflow-level permissions block; most jobs run with default GITHUB_TOKEN scope #1196

Activity

1. cristim

   added

   triagedItem has been triagedItem has been triaged

   priority/p3Polish / idea / may never shipPolish / idea / may never ship

   severity/lowMinor harmMinor harm

   urgency/eventuallyNo deadlineNo deadline

   impact/internalTeam-internal onlyTeam-internal only

   effort/xsTrivial / one-linerTrivial / one-liner

   type/choreMaintenance / non-user-visibleMaintenance / non-user-visible

   on Jun 10, 2026
2. cristim commented on Jul 28, 2026

   

   cristim

   on Jul 28, 2026

   MemberAuthor

   More actions

   Reviewed commit: be11bdcb5. Note: origin/main moved to 3e9660d06 during the review; re-verify against current main before changing code, since a finding may have been fixed or moved.

   Still present on 3e9660d06. The 2026-07-28 full review re-derived this and found one factual correction plus a reason the P3/informational rating understates it.

   Correction to the stated mitigation

   This issue says:

   mitigated by persist-credentials: false on checkouts

   That is true for ci.yml itself (:39,84,154,206,241,294,417,444,475,494 all set it), but not repo-wide. These checkouts do not set persist-credentials: false:

   • .github/workflows/pre-commit.yml:24-25
   • .github/workflows/deploy-aws-lambda.yml:131-132
   • .github/workflows/deploy-aws-fargate.yml:103-104
   • .github/workflows/cleanup-staging.yml:55-56
   • .github/workflows/destroy-fargate-dev.yml:44-45

   pre-commit.yml leaves a GITHUB_TOKEN in .git/config and then runs pre-commit run --all-files, which clones and executes code from four external hook repositories, runs hadolint/hadolint:v2.14.0 as a container, and executes npm ci over the frontend dependency tree. The four deploy workflows have the same omission while holding AWS / Azure / GCP session credentials in the same environment. So the mitigation should not be relied on when scoping the fix.

   Current state of ci.yml on 3e9660d06

   No workflow-level permissions:. Only security-scan sets one (:286-288). These jobs have none: lint (:32), unit-tests (:77), integration-tests (:131), docker-build (:199), terraform-validate (:229), snyk-scan (:408), e2e-tests (:436), azure-role-parity (:467), aws-iam-parity (:486), ci-success (:503).

   pre-commit.yml:9-10, the frontend-*.yml workflows, and every deploy workflow do set an explicit block, so this is an inconsistency rather than a house style.

   Failure scenario

   ci.yml runs on pull_request and executes third-party code with those credentials: docker/build-push-action, codecov-action, snyk/actions/golang, aquasecurity/trivy-action, plus docker compose build of the repo's own Dockerfiles and a full npm ci tree. On a repo configured for "Read and write permissions", a compromise in any one of them - the exact scenario .github/runbooks/compromised-dependency.md exists for - yields a token that can push to branches and publish packages. That is a supply-chain blast radius, not an informational hygiene nit; suggest severity/medium and priority/p2 rather than P3/informational.

   Fix direction

   Add permissions: contents: read at workflow level in ci.yml, keeping the security-scan job's security-events: write override, and add persist-credentials: false to the five checkouts listed above.

   Not the same as deploy-all.yml

   This issue also mentions deploy-all.yml having no permissions block. That one is a functional defect, not a least-privilege one: without id-token: write at the caller, the reusable deploy workflows cannot mint an OIDC token at all and every Deploy to All Clouds run fails at the first cloud-auth step. It is filed separately from this review, because applying this issue's recommendation (contents: read) to deploy-all.yml would leave that break in place.
3. cristim commented on Sep 2, 2026

   

   cristim

   on Sep 2, 2026

   MemberAuthor

   More actions

   Verified resolved at 3c0f8ac: ci.yml now declares permissions: contents: read at workflow level, every job carries its own job-level permissions block (security-scan keeps its security-events: write escalation), and all 17 checkout steps set persist-credentials: false. Evidence: .github/workflows/ci.yml:18-19 (PR #1870, #1870). Residual axes checked: every workflow file under .github/workflows has a top-level permissions block; deploy-all.yml's id-token gap was fixed separately in #1724. Closing as completed; reopen if the behaviour recurs.
4. cristim

   closed this as completedon Sep 2, 2026