Skip to content

fix(github): restrict OCIRepository cosign subject to protected release tags - #2719

Closed
devantler wants to merge 2 commits into
mainfrom
codex/fix-github-config-oci-verification-issue
Closed

devantler wants to merge 2 commits into
mainfrom
codex/fix-github-config-oci-verification-issue

Conversation

@devantler

Copy link
Copy Markdown
Contributor

Motivation

  • Prevent a supply-chain escalation where the github-config OCI artifact source would accept artifacts signed by the shared publish-manifests reusable workflow at arbitrary refs, allowing non-protected branch/PR signatures to be trusted.
  • The github-config app runs with namespace-scoped permissions to create/update Crossplane GitHub managed resources and ExternalSecrets, so an attacker-published artifact could mutate the org if the signer policy is too broad.

Description

  • Tightened the cosign OIDC subject check in k8s/bases/apps/github-config/oci-repository.yaml so the matchOIDCIdentity.subject requires @refs/tags/vX.Y.Z release-tag subjects instead of allowing @.+, rejecting branch and pull-request refs.
  • Kept the temporary repository-name alternation (reusable-workflows|actions) to preserve the planned signer repo transition while still enforcing tag-only acceptance.
  • Added clarifying comment text explaining the rationale and the narrower acceptance rule.

Testing

  • Ran a local regex smoke test (python3 snippet) that confirmed refs/tags/v1.2.3 matches and branch/PR subjects do not, and the test succeeded.
  • Ran git diff --check to ensure no whitespace/errors and it succeeded.
  • Ran python3 scripts/validate-naming.py and it succeeded against repository naming rules.
  • kubectl kustomize k8s/clusters/local/ and kubectl kustomize k8s/clusters/prod/ could not be executed in this environment because kubectl is not installed, so full kustomize render validation was not run here.

Codex Task

@devantler

Copy link
Copy Markdown
Contributor Author

Requested by the 🤖 Daily AI Engineer — CI is green at the current head and this draft carries no qualifying review, so requesting the lane-priority reviewer. Hygiene only: this is a sibling lane's draft and its owner promotes it.

@devantler

Copy link
Copy Markdown
Contributor Author

@cursor review

@cursor

cursor Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_7590bae9-d28a-43d0-8812-2189e089deb9)

@devantler devantler left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Generated by the Agentic Engineer

Static exact-head review

Reviewed this ownership-unverified draft at exact head 29895d7d47086b9acd4d4ab5a18b69f4a8156616 without checkout or execution. One blocking finding is anchored below.

# the first actions-signed artifact has been published and verified.
- issuer: '^https://token\.actions\.githubusercontent\.com$'
subject: '^https://github\.com/devantler-tech/(reusable-workflows|actions)/\.github/workflows/publish-manifests\.yaml@.+$'
subject: '^https://github\.com/devantler-tech/(reusable-workflows|actions)/\.github/workflows/publish-manifests\.yaml@refs/tags/v[0-9]+\.[0-9]+\.[0-9]+$'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Keep the SHA-pinned signer identity accepted

The current producer in devantler-tech/.github/.github/workflows/cd.yaml invokes devantler-tech/actions/.github/workflows/publish-manifests.yaml@061b345a7351b0caf23055caea63ccd08b75e20e, so its Fulcio SAN ends in a 40-hex workflow ref, not refs/tags/v.... This tag-only matcher therefore rejects every currently produced github-config artifact and freezes Flux verification/reconciliation when the next artifact is selected. Platform main already fixed this in #2816 after checking the deployed 1.20.2 signature, using the actions-only ([0-9a-f]{40}|refs/tags/v.+) ref matcher. Preserve that SHA-or-tag shape (and drop the archived reusable-workflows alternative), with positive SHA/tag and negative floating-ref coverage.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Closing as superseded — main now implements this PR's security objective, and more strictly.
Merging this would be a net regression rather than a fix.

This PR narrows the github-config OCIRepository cosign subject so only protected release tags are
trusted. Since it was opened, main moved the same check to commit-SHA pinning and dropped the
archived repo from the alternation (#3022). I compared the two regexes by running real subjects
through both:

candidate cosign subject this PR main
actions/...publish-manifests.yaml@refs/tags/v1.2.3 accepts rejects
reusable-workflows/...@refs/tags/v1.2.3 accepts rejects
actions/...@<40-hex commit> rejects accepts
actions/...@refs/heads/main rejects rejects

Two things follow:

  1. The vulnerability this PR closes is already closed. The last row is the escalation named in
    the motivation — an artifact signed from an arbitrary branch/PR ref — and main rejects it.
  2. Merging this would re-widen the policy. Row 2 is the problem: this branch still accepts
    devantler-tech/reusable-workflows, which is now archived (archived=true), at a movable
    tag. main accepts neither that repo nor a tag ref. A commit SHA cannot be repointed, so main's
    form is strictly the stronger of the two on both axes — repo scope and ref immutability.

This branch's own comment anticipated exactly this: "Narrow the alternation to actions-only once the
first actions-signed artifact has been published and verified."
That has since happened, and main
went further by pinning to commits.

There is no still-valid finding left to re-file — the residual concern is entirely covered by the
current main policy, so I am not opening a follow-up issue.

I stopped at the conflict rather than resolving it, because resolving toward this branch is precisely
the silent revert described above. Reopen if I have misread the intent — happy to be corrected.

@devantler devantler closed this Aug 15, 2026
@github-project-automation github-project-automation Bot moved this from 🫴 Ready to ✅ Done in 🌊 Project Board Aug 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

1 participant