Skip to content

Stop persisting git credentials in CI workflow checkouts - #6255

Merged
ChrisJBurns merged 2 commits into
mainfrom
harden-checkout-credentials
Aug 10, 2026
Merged

Stop persisting git credentials in CI workflow checkouts#6255
ChrisJBurns merged 2 commits into
mainfrom
harden-checkout-credentials

Conversation

@ChrisJBurns

@ChrisJBurns ChrisJBurns commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #6254. Review that one first; this PR targets it as its base, so the diff shown here is only the checkout change. Rebase onto main once #6254 merges.

Summary

  • actions/checkout stores the job token in .git/config and leaves it there. Every subsequent step in the job — and every tool those steps invoke, including build scripts and test suites pulling in third-party code — can read it. persist-credentials: false scopes the credential to the checkout itself.
  • None of these jobs push. They lint, generate, test, and run git diff/git status/git ls-tree, all of which are local operations that need no credential. This is 20 checkouts across 12 CI workflows.

artipacked goes 37 → 17. The 17 that remain are excluded deliberately, not missed:

Excluded Count Why
create-release-pr, create-release-tag, helm-publish, releaser 7 Release-only — no PR can exercise them, and a failure lands mid-release
image-build-and-publish, skills-build-and-publish 6 Publish path; same caution
claude.yml, issue-triage.yml, release-notes.yml 3 Run claude-code-action, which has broad shell access and may legitimately need to push
api-compat.yml 1 Does git fetch origin refs/tags/..., and is the repo's only required status check — breaking it blocks every PR

security-scan.yml's two checkouts (Grype and govulncheck) were initially held back because #6251 also edits that file, but a trial merge of the two branches is clean, so they are included here. Note #6251 does not fix them — it adds a third checkout that is already clean.

Part of #6253

Type of change

  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Dependency update
  • Documentation
  • Other (describe): CI configuration

Test plan

  • Unit tests (task test)

  • E2E tests (task test-e2e)

  • Linting (task lint-fix)

  • Manual testing (describe below)

  • Audited every touched workflow for operations that need the credential: git push, git commit, PR-creating actions, and submodules. The only git push in the repo is in create-release-tag.yml, which is excluded; there are no submodules anywhere.

  • All workflows still parse as YAML, including the two distinct checkout forms (bare - uses: and - name: + uses: with an existing with: block such as fetch-depth: 0).

  • actionlint output is byte-identical before and after apart from line-number shifts — 12 findings both ways, all pre-existing.

  • zizmor artipacked 37 → 17, with the remainder matching the exclusion table above exactly.

  • Trial-merged this branch with Add zizmor to CI and stop double-running the security scan #6251's to confirm the overlapping security-scan.yml edits do not conflict.

  • Every one of the 12 workflows runs on pull requests, either directly or via run-on-pr.yml, so this PR exercises all of them. If a credential were actually needed somewhere, it fails here rather than after merge.

Does this introduce a user-facing change?

No.

Special notes for reviewers

  • This touches 12 files, above the usual 10-file guideline. It's the same one-line addition in each, and splitting a uniform sweep into arbitrary file groups makes it harder to review, not easier — happy to split if you'd rather.
  • The six operator-ci.yml changes are the same edit repeated per job.
  • Worth a sanity check on helm-charts-test.yml: its checkout already had a with: fetch-depth: 0, so the new key is appended to the existing block rather than introducing a new one.

Generated with Claude Code

@ChrisJBurns
ChrisJBurns requested a review from JAORMX as a code owner August 10, 2026 16:13
@github-actions github-actions Bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Aug 10, 2026
@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.71%. Comparing base (caa351d) to head (12331f6).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6255      +/-   ##
==========================================
- Coverage   72.72%   72.71%   -0.02%     
==========================================
  Files         742      742              
  Lines       77299    77299              
==========================================
- Hits        56217    56206      -11     
- Misses      17117    17130      +13     
+ Partials     3965     3963       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions Bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Aug 10, 2026
rdimitrov
rdimitrov previously approved these changes Aug 10, 2026
Base automatically changed from pin-actions-and-fix-version-comments to main August 10, 2026 16:31
ChrisJBurns and others added 2 commits August 10, 2026 17:31
actions/checkout writes the job token into .git/config and leaves it
there for the rest of the job unless told otherwise. None of these jobs
push, so the credential is available to every subsequent step and every
tool they invoke for no reason.

Covers the CI workflows only. Excluded: the release workflows, which no
pull request can exercise; the three running claude-code-action, which
has broad shell access and may need to push; api-compat.yml, which does
a `git fetch` and is the repository's only required status check; and
security-scan.yml, whose checkout is handled in the zizmor PR.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Grype and govulncheck jobs only read the tree — anchore/scan-action
scans the filesystem and govulncheck-action runs with repo-checkout
false — so neither needs the token left in .git/config.

These were held back from the previous commit only because #6251 also
edits this file. A trial merge of the two branches is clean, so there is
no reason to defer them further.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ChrisJBurns
ChrisJBurns force-pushed the harden-checkout-credentials branch from b0ca65b to 12331f6 Compare August 10, 2026 16:31
@github-actions github-actions Bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Aug 10, 2026
@ChrisJBurns
ChrisJBurns merged commit b20cb9f into main Aug 10, 2026
80 of 81 checks passed
@ChrisJBurns
ChrisJBurns deleted the harden-checkout-credentials branch August 10, 2026 16:55
@github-actions github-actions Bot mentioned this pull request Aug 14, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants