Skip to content

security: cosign matchers accept any fixed workflow revision, not only approved ones #2818

Closed
9 of 9 issues completed
Closed

Description

@devantler

🤖 Generated by the Agentic Engineer

Evidence

Raised as a P1 during review of #2816, which tightened the cosign subject matchers from @.+ to @([0-9a-f]{40}|refs/tags/v.+)$:

When an artifact is signed through an older or otherwise unapproved actions revision, [0-9a-f]{40} still matches its SHA, while refs/tags/v.+ accepts every v-prefixed tag. All four updated OCIRepositories therefore reject branch-shaped refs but continue trusting superseded workflow code.

That is correct as stated. #2816 closes moving refs (@main, @refs/heads/*); it does not restrict which fixed revision is acceptable. A signature produced by a superseded — including a known-vulnerable — revision of publish-app.yaml or publish-manifests.yaml still verifies.

Why it was not fixed inline

The reviewer's prescription — enumerate the approved SHAs in the matcher — was declined for #2816 on operational grounds, not because the principle is wrong:

  • The matcher lives in five places (four live OCIRepository rules plus the tenant template). Pinning exact SHAs means every devantler-tech/actions release has to be shepherded across all of them.
  • The failure mode of missing one is not a warning: the affected OCIRepository stops verifying, so a tenant silently stops reconciling. A control whose routine maintenance step causes outages gets widened back out the first time it fires at an inconvenient moment.
  • The residual risk it closes is narrow. Producing any such signature requires actually running our publish workflow under our own OIDC identity — so an attacker already needs the ability to trigger a build in a devantler-tech repository, at which point they can equally invoke the current revision.

So the honest position is: real gap, wrong layer for a hand-maintained allow-list.

Smallest useful change

Options worth weighing, roughly in increasing cost:

  1. Enforce it where it is already true. Every caller pins the workflow by 40-hex SHA today, and dependency automation keeps those pins current. A CI check asserting that no consumer cd.yaml references the publish workflows by branch or floating tag makes the property enforced rather than incidental — and it fails on a PR, not in prod.
  2. Generate the matcher. If the approved revision is to appear in the matcher, it should be written by the same automation that bumps the workflow pin, never by hand.
  3. Drop the refs/tags/v.+ branch. It is currently unexercised — all four deployed artifacts are SHA-signed — so it is speculative width. It was kept in fix(security): reject floating workflow refs in cosign subject matchers #2816 only because two of the four are private packages whose certificates could not be read (the host PAT lacks packages: read). Once those two are confirmed SHA-signed, this branch can go, which halves the surface for free.

Option 1 gets most of the value at the lowest operational cost and is the suggested starting point.

Acceptance criteria

  • It is enforced, not merely conventional, that consumers invoke the publish workflows by a fixed revision.
  • Any approved-revision list that ends up inside a matcher is generated, never hand-maintained.
  • The refs/tags/v.+ alternative is removed if it is confirmed unexercised.
  • Whatever lands is RED-proved: a superseded or unapproved revision must actually fail verification.

Follow-up to #2816. Related: #2815 (image layer), #2817 (tenant template). Rough size: M.

Activity

  1. coderabbitai commented on Jul 26, 2026

    @coderabbitai
    Contributor
    🔗 Related PRs

    #2319 - feat(security): cosign-verify the infrastructure Flux OCI artifact [merged]
    #2399 - feat: accept devantler-tech/actions cosign subjects during the publish-workflow repoint [merged]
    #2404 - fix: tenant artifact verification rejects the dual signing identities from #2399 [merged]
    #2408 - fix(rgd): single regex-alternated cosign identity for tenant artifact verification [merged]
    #2457 - fix(talos): accept the actions publish-app identity at node image verification [merged]


    📝 Issue Planner

    Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.

    • Create Plan

    🧪 Issue enrichment is currently in open beta.

    You can configure auto-planning by selecting labels in the issue_enrichment configuration.

    To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

    issue_enrichment:
      auto_enrich:
        enabled: false

    💬 Have feedback or questions? Drop into our discord!

  2. moved this to 📥 Backlog in 🌊 Project Boardon Jul 26, 2026
  3. self-assigned this
    on Aug 2, 2026
  4. devantler commented on Aug 2, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Agentic Engineer

    Measurement: criterion 1 is already enforced — Option 1 would duplicate a working control

    I picked this up to build Option 1 (a CI check asserting consumers pin the publish workflows by fixed revision) and measured first. The check should not be built: zizmor already enforces exactly that property in this repository, and it gates.

    Is zizmor actually gating here?

    Yes. It is enabled in .mega-linter.yml (the ACTION_ZIZMOR_UNSECURED_ENV_VARIABLES entry exists precisely because the audit was aborting, and the note records "Measured: 1 error -> 0, audit performed"), and it is not in DISABLE_ERRORS_LINTERS — that list holds only REPOSITORY_CHECKOV, REPOSITORY_TRIVY, COPYPASTE_JSCPD, SPELL_CSPELL, SPELL_LYCHEE. So a zizmor finding fails the build rather than being reported-only.

    Does its unpinned-uses audit catch the shapes we care about?

    Measured with zizmor 1.29.0, --persona regular --offline, swapping the ref on a real devantler-tech/actions caller and counting unpinned-uses findings:

    uses: ref default policy with an explicit hash-pin policy
    @8436c4a9… (40-hex, current state) 0 0
    @main (branch) 1 1
    @v10 (moving major tag) 1 1
    @v10.1.4 (exact release tag) 1 1

    So the default policy already requires a hash pin for devantler-tech/*. I had assumed a hash-pin config would be needed and that assumption was wrong — adding one changes nothing, in every shape tested.

    Baseline / control: the current tree scans 0 findings, and all 18 distinct uses: refs across .github/workflows/ and .github/actions/ are 40-hex SHAs. The clean-tree-plus-config arm also stays at 0, so the policy is not over-tightening.

    What this means for the acceptance criteria

    • "It is enforced, not merely conventional" — satisfied for this repository, by an existing blocking control. Building a second check would add a maintenance surface with no new coverage.
    • "Any approved-revision list is generated, never hand-maintained" — satisfied vacuously and durably: zizmor requires a hash pin without knowing which, so there is no list to maintain. This is the cheapest possible form of the property.
    • "The refs/tags/v.+ alternative is removed if confirmed unexercised" — unchanged, and still gated on reading the two private packages' certificates (the host PAT lacks packages: read, per security: tighten cosign subject matchers at the image-verification layer #2815).
    • RED-proved — the table above is that proof for the pinning half.

    The genuine residual gap is NOT in this repo

    Platform does not call the publish workflows at all — it only references them in comments. The real consumers, and how they pin today:

    Consumer publish-app.yaml ref
    wedding-app @6ae5d87b… (40-hex)
    ascoachingogvaner @625b7c0c… (40-hex)
    gitops-tenant-template @625b7c0c… (40-hex)

    All three are correctly SHA-pinned today. What I could not verify is whether that is enforced in those repos: none of the three has a .mega-linter.yml, so they inherit the shared workflow's defaults, and I did not confirm from a CI run whether zizmor is in the enabled set there and gating. Separately, two repos I checked that do carry a config — ksail and world-at-ruin — have zizmor's errors disabled, so if either becomes a consumer the property would not hold for it.

    Suggested re-scope: this issue's pinning half is done for platform. The remaining value is a portfolio-level check that every repo consuming a publish workflow gates on zizmor — which belongs in devantler-tech/actions (the shared CI building block) rather than in any one consumer, per the shared-library stewardship rule. I have not filed that as a separate issue, since it may be better expressed as a criterion on this one.

    Unassigning: no PR is in flight from me, and the next run should not have to re-derive any of the above.

  5. devantler commented on Aug 2, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Agentic Engineer

    Measurement: the pinning property is unenforced for every repo that actually publishes

    Picking up from the previous run's handoff, which closed criterion 1 for platform and left open
    whether the property is enforced in the repos that really call the publish workflows. It is not —
    and the gap is wider than "zizmor might not gate there".

    Who calls the publish workflows, and how they pin

    Caller Workflow Pin
    .github publish-manifests 061b345a… (40-hex)
    ascoachingogvaner publish-app 625b7c0c…
    aws publish-manifests 5aa2657f…
    doggy-countdown publish-app 9705e47d…
    dotnet-template publish-dotnet-library 625b7c0c…
    gitops-tenant-template publish-app 625b7c0c…
    wedding-app publish-app 6ae5d87b…

    7 of 7 pin by 40-hex SHA. Zero use refs/tags/v* or a branch.

    Whether anything enforces that

    Repo Workflow-security linting
    wedding-app, ascoachingogvaner, gitops-tenant-template none — no .mega-linter.yml (verified 404) and no lint job of any kind in ci.yaml
    ksail, world-at-ruin ACTION_ZIZMOR is in DISABLE_LINTERS — zizmor does not run at all
    platform enforced (zizmor via MegaLinter, errors not disabled) — but platform never calls these workflows

    Correction to the previous note: it recorded ksail/world-at-ruin as having zizmor's errors
    disabled. They have the linter itself disabled, which is stronger.

    The shared control already exists — devantler-tech/actions/.github/workflows/scan-for-workflow-vulnerabilities.yaml —
    and its adoption is 1/8: only actions itself uses it. So the previous run's suggested re-scope
    (a portfolio-level check that every consumer gates on zizmor) would need five separate adoptions and
    would still leave the property dependent on each repo keeping its config right.

    What I shipped instead

    devantler-tech/actions#858 makes the publish workflows self-enforcing: they refuse to run unless
    github.job_workflow_ref ends in a 40-character commit SHA, checked before checkout or signing. One
    change in the shared library, inherited by every consumer, nothing to adopt and no approved-revision
    list to maintain — which is also how it satisfies criterion 2 rather than merely restating it.

    Scope note: publish-dotnet-library.yaml requests no id-token and mints no signature, so it is
    deliberately not covered — there is no certificate whose SAN could be over-trusted.

    Criterion status

    • Enforced, not conventional — at the shared layer, for all consumers at once.
    • Any approved-revision list is generated, never hand-maintained — there is no list.
    • refs/tags/v.+ removed if unexercised — the caller side is now measured unexercised
      (table above), but removing the branch from the cosign matchers still needs the two private
      packages' certificates read, and the host PAT still cannot: GET /orgs/devantler-tech/packages
      returns 403 Resource not accessible by personal access token (re-verified live today). This
      remains blocked on the credential, exactly as security: tighten cosign subject matchers at the image-verification layer #2815 records.
    • RED-proved — the guard's own script, extracted from the workflow rather than transcribed,
      is executed against every ref shape; ablations covering a removed guard, a weakened regex, a
      wrong context binding and a wrong step order each turn it red.
  6. removed their assignment
    on Aug 2, 2026
  7. devantler commented on Aug 2, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Agentic Engineer

    devantler-tech/actions#858 merged (main 36171a3686), delivering the caller-side half of this issue: the publish workflows now refuse to sign for a caller that is not SHA-pinned, so a superseded revision of a publish workflow can no longer mint a trusted signature.

    Post-merge verification collected, not assumed. [Test] Publish Caller Pin - Live OIDC is gated on github.event_name == 'push', so it cannot run on a pull request at any head — it ran on the merge commit and is completed / success. main is green (82 success, 34 skipped, 0 failures).

    This issue stays OPEN. Criterion 3 — dropping refs/tags/v.+ from the cosign matchers — is still blocked on reading two private packages' certificates, and the host PAT still lacks read:packages (403, re-verified live at ticks 903, 904 and again this run). That is a credential-scope gap, not something the merged work resolves.

    Also still open and unaffected: devantler-tech/actions#864 tracks removing the enable-caller-pin flag once the guard is proven in the wild, per the feature-flag lifecycle rule.

  8. self-assigned this
    on Aug 3, 2026
  9. 22 remaining items

  10. devantler commented on Aug 10, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Agentic Engineer

    Acceptance criterion 1 is already built — the gap is that it was never switched on at two live tenants

    Reading the code rather than the issue changed the picture. Criterion 1 ("enforced, not merely conventional, that consumers invoke the publish workflows by a fixed revision") does not need a new CI check: devantler-tech/actions already ships the enforcement in both signing workflows, behind an opt-in enable-caller-pin input (added 36171a3, 2026-08-02, released in v12.0.0+), with .github/tests/test-publish-caller-pin.sh covering it. That test passes at actions@main today.

    It reads the OIDC job_workflow_ref claim rather than the github.job_workflow_ref expression property. I verified independently why that is correct, from a real signature — the Fulcio certificate on ghcr.io/devantler-tech/doggy-countdown/manifests:latest carries:

    SAN URI:  https://github.com/devantler-tech/actions/.github/workflows/publish-app.yaml@9705e47d…
    OID …1.9: (same value)          # Build Signer URI  = job_workflow_ref
    OID …1.18: https://github.com/devantler-tech/doggy-countdown/.github/workflows/cd.yaml@refs/tags/v1.0.1   # workflow_ref
    

    So the subject the OCIRepository rules match is the reusable workflow ref; the caller-side workflow_ref is a different value entirely.

    Where it is actually in force

    platform-tenant-template sets enable-caller-pin: true. The tenants do not:

    Consumer Pinned revision Input available at that pin? Flag set?
    wedding-app v13.0.3 yes no → now #260
    ascoachingogvaner v13.0.1 yes no → now #181
    doggy-countdown v10.2.1 no (predates the flag) blocked on a pin bump
    .github (publish-manifests) v8.0.0 no (predates the flag) blocked on a pin bump

    The propagation channel is clogged rather than working: all four open template-sync PRs on those two tenants are DIRTY, and none of them carries this change.

    Evidence for the two PRs

    For each tenant I extracted the guard from the exact revision that tenant pins and ran it against that tenant's real uses: ref:

    • accepts the tenant's current SHA-pinned ref → activation is a no-op for today's releases;
    • rejects …/publish-app.yaml@refs/tags/v13.0.3 → the mutable-tag path this issue is about is genuinely closed.

    Honest limit: the guard only executes on a real tag publish, so neither PR proves the live OIDC round-trip. The failure mode is fail-closed before checkout, build or signing, so the worst case is a release that does not ship, never a bad artifact.

    What remains on this issue

    • Criterion 1 for doggy-countdown and .github — needs their actions pin bumped past v12.0.0 first (dependency automation's lane).
    • Criterion 2 (a generated, never hand-maintained approved-revision list) and criterion 3 (dropping the unexercised refs/tags/v.+ alternative) are untouched. Criterion 3 still needs the two private packages' certificates read, which needs packages: read.
  11. devantler commented on Aug 10, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Agentic Engineer

    Both activations merged. wedding-app#260 at 04:25:04Z, ascoachingogvaner#181 at 04:25:08Z — each with a CodeRabbit green at head, every check green, and the per-tenant behavioural evidence recorded on the PR.

    Criterion 1 is now in force on the two artifacts whose pinned revision supports it. Leaving this issue open for what is genuinely left:

    • doggy-countdown (v10.2.1) and .github / publish-manifests (v8.0.0) still pin revisions that predate the input, so activating there needs the pin bumped past v12.0.0 first — dependency automation's lane, not an agent's.
    • Criterion 2 — a generated, never hand-maintained approved-revision list — untouched.
    • Criterion 3 — dropping the unexercised refs/tags/v.+ alternative — still needs the two private packages' certificates read, which needs packages: read.

    Unassigning: my slice is delivered and the rest is a different piece of work.

  12. removed their assignment
    on Aug 10, 2026
  13. devantler commented on Aug 10, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Agentic Engineer

    Correction: the two remaining consumers are not dependency automation's lane — neither repo has any

    My last comment said doggy-countdown and .github "still pin revisions that predate the input, so
    activating there needs the pin bumped past v12.0.0 first — dependency automation's lane, not an
    agent's." The first half is right; the second half is wrong, and it would have parked both
    indefinitely.

    Neither repo has dependency automation at all:

    repo pin declares enable-caller-pin dependabot.yml renovate.json
    doggy-countdown publish-app @9705e47d (v10.2.1) 0× absent absent
    .github publish-manifests @061b345a (v8.0.0) 0× absent absent

    So nothing was ever going to move those pins. That is what makes them agent work rather than a wait:
    the missing automation is the defect, and it has to be added in the same change that bumps the pin.

    Both are cluster-verified today with SHA-only matchers (doggy-countdown/doggy-countdown and
    github-config/github-config), so the consumer-side rule is already closed and only the
    producer-side refusal is missing.

    Status:

    One ordering constraint worth recording, since it forces both into a single commit each: passing
    enable-caller-pin to a revision that does not declare it is an invalid-input error, not a
    silent no-op — so bump-then-enable cannot be split into two PRs.

  14. devantler commented on Aug 10, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Agentic Engineer

    All four enable-caller-pin activations are live and none has ever run

    The activation slice reads as delivered, so this is worth stating plainly before it gets closed on
    that basis: the guard is switched on in four production publish paths and zero of them have
    executed it. Every one of these repos' newest tag predates its own activation merge, and CD is
    tag-triggered in all four, so no CD run has happened since.

    repo activation merged newest tag tag date CD run since?
    wedding-app #260 · 04:25:04Z v1.15.10 2026-07-27 no
    ascoachingogvaner #181 · 04:25:08Z v1.13.4 2026-08-03 no
    doggy-countdown #10 · 05:23:49Z v1.0.1 2026-07-26 no
    .github #150 · 05:32:15Z v1.22.4 2026-08-07 no

    Why no release followed, and why that is not self-correcting

    Every one of the four merged under a security(...) commit type, and none of these repos treats
    that as a releasing type. .github adds only revert and breaking to the commit-analyzer's
    release rules; wedding-app and ascoachingogvaner add only breaking; doggy-countdown has no
    .releaserc at all. In each case the remaining behaviour is the stock preset, which releases on
    feat and fix.

    So the verification does not arrive on a timer — it arrives the next time some unrelated
    feat/fix lands in each repo. For doggy-countdown that is potentially a long wait: it is a
    static countdown page whose last release was two weeks ago.

    What the guard will do when it finally runs

    Read at the pinned 61d9c87b (v13.0.4), the check is: fetch the OIDC token, take
    job_workflow_ref, keep the substring after the last @, and require ^[0-9a-f]{40}$. All four
    callers pin by 40-hex SHA today, so the expected outcome is a pass.

    That is inspection, which is exactly what acceptance criterion 4 excludes — and it is recorded here
    as a prediction to be checked against the first real run, not as evidence that the criterion is met.

    The risk this leaves open is bounded, and that part is by design

    If the prediction is wrong, the guard fails before the artifact exists: no signature is minted,
    the release fails loudly, and the cluster is never offered something it would reject. The exposure
    is a broken release, not a bad artifact or a silently stalled tenant. That is the behaviour #2818
    asked for, so the unverified state is uncomfortable rather than dangerous.

    Consequence for this issue and its children: criterion 4 is unmet on
    devantler-tech/.github#149 and devantler-tech/doggy-countdown#7, and equally unmet on the two
    already-closed tenant activations. Whoever picks up the next slice should check these four CD runs
    before treating the rollout as proven.

  15. self-assigned this
    on Aug 10, 2026
  16. devantler commented on Aug 10, 2026

    @devantler
    ContributorAuthor

    🤖 Generated by the Agentic Engineer

    Closing: every criterion is either met and guarded, or delegated to an open child

    Picked this up as the oldest actionable Security issue and re-measured all four criteria against
    current main rather than against the notes above — which turned out to matter, because two of them
    had already landed since the last comment.

    Criterion 1 — enforced, not conventional ✅

    scripts/guard-shared-publish-workflow-pin.sh runs in both ci.yaml (line 59) and cd.yaml
    (line 138), so the property is gated on every PR and on delivery. It judges the ref by an
    allow-list (40-hex only), carries an EXPECTED_MIN_SUBJECTS=8 floor so a refactor that moves the
    subjects cannot make it pass vacuously, and matches all three key spellings in use (subject,
    subjectRegex, subjectRegExp).

    Producer side: enable-caller-pin: true is live in all four activatable consumers.

    Criterion 2 — generated, never hand-maintained ➡️ #3048

    Untouched here, and correctly so: it is a cross-repo design, not a slice. The guard script's own
    header already records the constraint the generator will hit — the check matches the literal
    pattern text [0-9a-f]{40}, so a subject naming one concrete commit is rejected today despite
    being strictly narrower. Tracked as an open child.

    Criterion 3 — drop the unexercised refs/tags/v.+ alternative ✅ (#3022)

    Already removed. Independently re-verified this run, behaviourally rather than by inspection, on
    the three artifacts whose packages are readable:

    artifact SHA-only matcher tag-only matcher digest verified locally digest Flux has stored in prod
    doggy-countdown/manifests verifies rejects sha256:533e2c77… sha256:533e2c77…
    github-config/manifests verifies rejects sha256:c06101e5… sha256:c06101e5…
    aws/manifests verifies rejects sha256:0bd68a1e… sha256:0bd68a1e…

    The last two columns are the part that makes this attributable: the artifact I tested is
    byte-for-byte the artifact the cluster is verifying today, not merely the newest tag.

    The two private packages (wedding-app, ascoachingogvaner) are still unreadable — the host
    credential lacks packages: read, and both matcher arms return an access denial rather than a
    verification failure. Worth stating explicitly, because the two exits look identical if stderr is
    discarded: that is not evidence of a tag-form signature, and it is not evidence against one
    either.
    Their refs/tags/v.+ arm was nonetheless removed with the rest under #3022, and both
    OCIRepositories read Ready=True in prod against the SHA-only matcher — which is the behavioural
    confirmation the certificate read would have given.

    Criterion 4 — RED-proved on a real run ➡️ .github#149 and doggy-countdown#7

    Still unmet, and re-confirmed unchanged this run: the newest CD run in each of the four repos still
    predates its own activation merge.

    repo activation merged newest CD run
    wedding-app 04:25:04Z 2026-07-27 (v1.15.10)
    ascoachingogvaner 04:25:08Z 2026-08-03 (v1.13.4)
    doggy-countdown 05:23:49Z 2026-07-26 (v1.0.1)
    .github 05:32:15Z 2026-08-07 (v1.22.4)

    This does not arrive on a timer — it arrives with the next unrelated feat/fix in each repo — so
    it is a verification to collect, not work to schedule. It is already recorded on the two open
    children.

    Why close

    The pinning half is enforced and guarded, the speculative tag width is gone and cannot come back,
    and the two genuinely open items each have an open child carrying them: #3048 (approved-revision
    allow-list) and .github#149 (last unactivated consumer, plus the first-run verification). Leaving
    this parent open only makes every run re-derive the same state.

    Closing as delivered-and-delegated; the children are where the remaining work lives.

  17. moved this from 📥 Backlog to ✅ Done in 🌊 Project Boardon Aug 10, 2026
  18. removed their assignment
    on Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Fields

    No fields configured for Security.

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions