Skip to content

feat(controls): ship checkoutMustNotPersistCredentials (ISSUE-307) - #293

Open
stephrobert wants to merge 2 commits into
getplumber:mainfrom
stephrobert:feat/debench-artipacked-persist-credentials
Open

feat(controls): ship checkoutMustNotPersistCredentials (ISSUE-307)#293
stephrobert wants to merge 2 commits into
getplumber:mainfrom
stephrobert:feat/debench-artipacked-persist-credentials

Conversation

@stephrobert

Copy link
Copy Markdown
Contributor

Closes #187.

Summary

Promotes the artipacked control (ISSUE-307, severity high) off the
GitHub bench. It flags any actions/checkout step that does not set
persist-credentials: false, leaving the GITHUB_TOKEN in .git/config
where a later step can exfiltrate it.

While shipping it I found the control was benched for more than missing
fixtures: it had no config plumbing. enabled: false was silently
ignored and the control never appeared in config view. This PR completes
the wiring so it behaves exactly like its 20 shipping GitHub peers.

Changes

  • Remove checkoutMustNotPersistCredentials from benchedControls[ProviderGitHub] so the Rego file loads.
  • Add the config plumbing: EnabledOnlyControlConfig struct field, valid-key entry, control/catalog.go entry and DisabledControlNames gate — so enabled: false actually disables it and it shows in config view.
  • Document the control in .plumber.yaml (regenerates the embedded default) and add it to the config init wizard menu + starter defaults.
  • Extend the ISSUE-307 fixtures with the explicit persist-credentials: true violation and the quoted "false" clean form; TestIssue307_Artipacked now covers all four cases.

Acceptance criteria (#187)

  • Removed from benchedControls[ProviderGitHub].
  • ≥3 test cases + policies/testdata/ fixtures (4 cases: default checkout, explicit true, boolean false, quoted "false").
  • Added to the GitHub controls: block of .plumber.yaml with enabled: true.
  • ISSUE-307 entry present in docs/GITHUB_ISSUES.md.
  • make build && make test && make lint green.

Verification

Verified on real GitHub Actions repositories: fires as expected (e.g. 88
findings on a large workflow set), honours enabled: false, and the
detection agrees 1:1 with the reference artipacked implementation across
n8n, crowdsec, bunkerweb and centreon. No false positive observed: the
control never flags a checkout that already sets persist-credentials: false. GitLab analysis is byte-identical before/after (GitHub-only change).

thomasboni added a commit that referenced this pull request Jul 3, 2026
The pin-refs job was the last checkout in the repo without
persist-credentials: false — the app token landed in .git/config and
survived for the rest of the job, which the incoming
checkoutMustNotPersistCredentials control (ISSUE-307, PR #293) rightly
flags in our own self-scan (94.7% < 100% threshold).

Supply the token at push time through an env-based credential helper
instead, the same pattern the homebrew job already uses: .git/config
only stores the helper script, never the token.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
plumber-release-bot Bot pushed a commit that referenced this pull request Jul 6, 2026
## [0.3.87](v0.3.86...v0.3.87) (2026-07-06)

### 🐛 Bug Fixes

* **collect:** contain local CI-config reads and skip symlinked Dockerfiles ([ef9c275](ef9c275))
* **gitlab:** harden local include resolution ([729e259](729e259))

### ♻️ Refactoring

* **render:** sanitize repo-derived text and bound resource use ([17ed05e](17ed05e))
* **score:** resolve the score endpoint from CLI/env only ([0a89351](0a89351))

### 👷 CI/CD

* **grype:** install pinned grype by checksum, drop scan-action ([a275cdf](a275cdf)), closes [#294](#294)
* pin runtime tool installs to immutable versions ([a203b7f](a203b7f))
* **release:** pin v0.3.86 refs [skip ci] ([101cd21](101cd21))
* **release:** stop persisting credentials in the pin-refs checkout ([8f0f261](8f0f261)), closes [#293](#293)
* **scorecard:** document the action's mutable-image gap ([61c8cdd](61c8cdd))
Promote the artipacked control off the GitHub bench. It flags any
`actions/checkout` step that omits `persist-credentials: false`,
leaving the GITHUB_TOKEN in .git/config where a later step can
exfiltrate it.

The control's rule was complete but benched for lack of fixtures and,
as it turned out, config plumbing: `enabled: false` was silently
ignored and the control was absent from `config view`. Completed the
wiring to match its 20 shipping peers:

- remove from benchedControls (registry.go) so the Rego file loads;
- add the EnabledOnlyControlConfig struct field, valid-key entry,
  catalog entry and DisabledControlNames gate so the toggle works;
- document the control in .plumber.yaml (regenerates default.yaml);
- add it to the config init wizard menu and starter defaults;
- extend the ISSUE-307 testdata with the explicit `persist-credentials:
  true` violation and the quoted `"false"` clean form, and cover all
  four cases in TestIssue307_Artipacked.

Verified: fires 12 findings on the dirty GitHub fixture, honours
`enabled: false`, and leaves GitLab analysis byte-identical.
@stephrobert
stephrobert force-pushed the feat/debench-artipacked-persist-credentials branch from 3610586 to 478587c Compare July 6, 2026 13:26
…SSUE-310 high)

A persisted checkout credential is latent hygiene until `.git` is
actually packed into a downloadable artifact. Split the single high
ISSUE-307 into two graded tiers under the same control:

- ISSUE-307 (low) — checkout persists the token but nothing packs
  `.git`. A heads-up to add `persist-credentials: false`; the
  fork-controlled-code route stays owned by ISSUE-802 / ISSUE-804.
- ISSUE-310 (high) — the same persisted credential AND a later
  upload-artifact of a `.git`-inclusive path (`.`, workspace root, or
  a path naming `.git`). The token is exfiltrable — the demonstrable
  ArtiPACKED leak.

Scoring reads severity from the code registry, so two severities need
two codes; both map to checkoutMustNotPersistCredentials (no new config
key). Fixtures cover the A∧B high case, the A-only low case, an upload
scoped to a non-.git path (stays low), and credentials-disabled (clean).

Rationale: on 46 real repos the broad high rule fired on 35; the
A∧B-only shape fired on 0. Grading keeps the recall while reserving
high for the leak that actually reproduces.
@stephrobert

Copy link
Copy Markdown
Contributor Author

Update: graded by exploitability (ISSUE-307 low + ISSUE-310 high)

Reworked so severity tracks whether the token is actually exfiltrated, rather than flagging every persisted-credential checkout at high.

A persisted checkout credential is latent hygiene until .git is packed into a downloadable artifact. Split into two tiers under the same checkoutMustNotPersistCredentials control:

  • ISSUE-307 (low) — checkout persists the token but nothing packs .git. A heads-up to add persist-credentials: false. The fork-controlled-code route stays owned by ISSUE-802 / ISSUE-804.
  • ISSUE-310 (high, new code) — the same persisted credential and a later actions/upload-artifact of a .git-inclusive path (., the workspace root, or a path naming .git). The artifact is downloadable, so the token is exfiltrable — the demonstrable ArtiPACKED leak.

Scoring reads severity from the code registry per code, so two severities need two codes; both map to the same control (no new config key).

Why grade instead of narrowing to A∧B only

I measured both models against 46 real repos (grafana, n8n, strapi, awx, amazon.aws, python-gitlab, posting, …):

Model Repos flagged Findings
broad (high on any persisted checkout) 35 / 46 dozens
A∧B only (checkout + .git in artifact) 0 / 46 0

Uploading .git in an artifact is genuinely rare, so A∧B-only would neuter the control. Grading keeps the recall of the broad rule while reserving high for the leak that actually reproduces.

Validated: a real dirty repo → 6 × ISSUE-307 (low); a synthetic checkout-then-upload path: . → 1 × ISSUE-310 (high). Fixtures cover A∧B (→310), A-only (→307), an upload scoped to a non-.git path (stays 307), and credentials-disabled (clean). make build && make test && make lint green; catalog doc updated.

@Joseph94m Joseph94m self-assigned this Jul 8, 2026
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.

feat(controls): ship artipacked (ISSUE-307)

2 participants