Skip to content

security(supply-chain): the app tenants' publish matchers still accept any workflow revisionΒ #3917

Closed
2 of 2 issues completed

Description

@devantler

πŸ€– Generated by the Agentic Engineer

Evidence

Measured on main at cf2f6248, after #3308 landed the generated approved-revision sets:

consumer workflow matcher subject revision
.github publish-manifests @(a147ead8…|3c3a6348…) β€” approved pair
aws publish-manifests @(b089a1b0…|3c3a6348…) β€” approved pair
wedding-app publish-app @[0-9a-f]{40} β€” any fixed revision
ascoachingogvaner publish-app @[0-9a-f]{40} β€” any fixed revision

scripts/publish-workflow-approved-revisions.tsv already computes an approved pair for both app
tenants, and both are currently diverged β€” wedding-app runs an artifact signed by 883d891a… while
its pin is 759ff9b2…, and ascoachingogvaner runs one signed by a147ead8… against a pin of
3c3a6348…. The numbers exist; these two matchers do not read them.

The exclusion is deliberate, not an oversight: TRUSTED_RELEASE_STREAM_CONSUMERS in
scripts/publish-workflow-approved-revisions.lib.sh names both tenants, and
write-publish-workflow-matchers.sh leaves their subject on the shape pattern.

Problem

For these two consumers, any 40-hex revision of devantler-tech/actions' publish-app.yaml still
satisfies verification β€” including a superseded one. That is the substitution the #2818 line of work
exists to close, and it remains open for the two workloads that serve real users.

The trade-off, which is why this is not simply "narrow them too"

The recorded rationale is sound. An application tenant releases on its own cadence. If its matcher named
only the revisions the platform has observed, then the first tenant release signed by a new
publish-app.yaml revision would fail verification and stop deploying until a platform change landed β€”
turning an unrelated change in devantler-tech/actions into a tenant outage.

So the question is not whether to narrow, but how to narrow without coupling a tenant release to a
platform merge
. Options worth weighing, cheapest first:

  1. Regenerate on the event rather than the schedule. The set already regenerates automatically; if
    that workflow also triggered on a new publish-app.yaml revision in devantler-tech/actions, the
    approved pair would move ahead of the first tenant release that uses it, and the coupling window
    would close on its own.
  2. Accept a bounded window. Narrow, and treat the gap between a new workflow revision and the
    regeneration PR as a known, short unavailability β€” acceptable only if it is measured, not assumed.
  3. Keep the carve-out and compensate elsewhere. Leave the shape pin and rely on a different control
    to catch a superseded signer, stating plainly which control that is.

Expected

Either both app tenants verify against their generated approved pair, or the carve-out is a decision
someone made on measured evidence rather than a gap that outlived the issue that would have caught it.

Acceptance criteria

  • The coupling window is measured, not estimated: how often publish-app.yaml changes revision,
    and how long the regeneration workflow takes to reflect one.
  • A decision is recorded on this issue β€” narrow, narrow-with-event-trigger, or keep the carve-out β€”
    with that measurement behind it.
  • If narrowing: both tenants' matchers carry the approved pair, and a negative case proves an
    out-of-set revision is rejected for them specifically.
  • If keeping the carve-out: TRUSTED_RELEASE_STREAM_CONSUMERS' comment names the compensating
    control, so the next reader is not left to infer that none exists.

Size

Small for the measurement and the decision; small-to-medium for whichever remedy it selects.

Follows #3308.

Activity

  1. moved this to πŸ“₯ Backlog in 🌊 Project Boardon Sep 20, 2026
  2. devantler commented on Sep 20, 2026

    @devantler
    ContributorAuthor

    πŸ€– Generated by the Agentic Engineer

    Pre-derivation for whoever picks this up β€” option 1's current state, so it does not have to be re-established. .github/workflows/regenerate-publish-workflow-approved-revisions.yaml at bdee9609 triggers on exactly two things:

    on:
      workflow_dispatch:
      schedule:
        - cron: "20 6 * * *"   # daily at 06:20 UTC, after the 06:00 agent-skills update

    There is no event trigger on a new publish-app.yaml revision. So option 1 is not a small adjustment to an existing hook β€” it needs a cross-repository signal added, and that signal is the design question, not the wiring.

    That also quantifies the coupling window option 2 asks to measure, which the body leaves as an unknown. With regeneration on a daily cron, a new publish-app.yaml revision merged in devantler-tech/actions is picked up at the next 06:20 UTC run, so the worst case is just under 24 hours plus the time for the generated PR to merge. A tenant releasing inside that window against a narrowed matcher would fail verification for up to a day. That is almost certainly too long to accept as a bounded window for a user-facing tenant, which is a real argument for option 1 over option 2 rather than a preference between them.

    One consequence worth stating before anyone narrows: the two divergences the body records (wedding-app running 883d891a… against a pin of 759ff9b2…, ascoachingogvaner running a147ead8… against 3c3a6348…) mean a narrowed matcher would reject the artifact each tenant is running today, not merely a future one. Whatever order this lands in, the regeneration has to move the pins to the observed revisions before the matcher narrows, or the narrowing is an immediate outage rather than a latent one.

    No change made; this is evidence, not a decision.

  3. added a commit that references this issue on Sep 21, 2026
  4. devantler commented on Sep 21, 2026

    @devantler
    ContributorAuthor

    πŸ€– Generated by the Agentic Engineer

    Measurement (acceptance criterion 1)

    The approved set a tenant must accept is its pinned publish-app.yaml revision in cd.yaml. So the coupling window depends on how often that pin moves and how soon a release follows a move. How often publish-app.yaml itself changes in devantler-tech/actions does not set it. Since 2026-07-03:

    Fact Measured
    Tenant pin moves about weekly: 20 moves for wedding-app and 17 for ascoachingogvaner, mostly the grouped dependency bump
    Delay from an actions revision to a tenant pin move 1.3 h (min) to 4.9 d. Five of the eight sampled moves came within ~25 h
    Regeneration run daily at ~06:40 UTC, scheduled for 06:20
    Delay from a regeneration PR opening to its merge 11 PRs: 1.0 h to 42 h, median ~3 h. Four took over 5 h
    First release on a newly moved pin 14 cases in the window

    I replayed those 14 first-on-new-signer releases against a narrowed matcher fed by the current daily regeneration:

    • 2 would have failed verification outright. wedding-app released 3.6 h after its pin moved (2026-07-12). It released again 23.6 h after a move and 5 minutes before the next regeneration run (2026-07-04).
    • 1 would have failed or passed depending on regeneration-PR latency. ascoachingogvaner released 5.9 h after a move (2026-07-18).
    • The other 11 had days of slack.

    That is 2–3 of 14 (14–21%), or roughly one user-facing deploy stall per tenant-month.

    The window is open today too. The committed approved set still records wedding-app's pin as 759ff9b2…. Its cd.yaml moved to 3c3a6348… at 2026-09-20T14:45Z, after that day's regeneration. A wedding-app release now would be rejected by a narrowed matcher.

    Decision (acceptance criterion 2)

    Narrow, but only after two prerequisites. Option 2 (accept a bounded window) is rejected on the measurement above. Option 1 as written is necessary but not sufficient:

    • Regenerating on an event narrows the window but cannot close it. A pin move can follow an actions revision within 1.3 h, and a regeneration PR takes a median ~3 h to merge. Also, the generator only learns a pin once it has landed on the tenant's default branch, so it always trails the move.
    • What closes it is ordering. The tenant's pin move must not land until the platform already accepts the new revision. The failure then becomes a dependency PR that waits a few hours instead of a release that fails to deploy.

    The sequence is:

    1. Accept the next revision early. Add the latest released devantler-tech/actions revision to each tenant's approved set as a candidate, and regenerate when actions publishes a release as well as on the daily schedule. The platform then accepts a revision before any tenant's dependency bump can move to it.
    2. Hold the tenant bump behind platform acceptance. Add a required tenant-side check: a PR that moves the publish-app.yaml pin passes only when the platform's committed approved set already contains the new revision. Any residual race then parks the bump PR instead of breaking a release.
    3. Then narrow both matchers under this issue: drop both tenants from TRUSTED_RELEASE_STREAM_CONSUMERS, regenerate first so today's divergences are covered, and add an out-of-set negative case for each tenant.

    Steps 1 and 2 are filed as sub-issues. This issue stays open for step 3, which must not land before both.

    Steps 1 and 2 are #3960 and #3961.

  5. added a commit that references this issue on Sep 21, 2026
  6. moved this from πŸ“₯ Backlog to πŸ‘€ In Review in 🌊 Project Boardon Sep 21, 2026
  7. devantler commented on Sep 21, 2026

    @devantler
    ContributorAuthor

    πŸ€– Generated by the Agentic Engineer

    Decision: narrow both app tenants (delivered in #4020)

    Measurement (recorded on #3960 and #3961): a tenant pin can move within 1.3 h of a new actions revision, while regeneration PRs took 1.0–42 h to merge. Replaying releases since 2026-07-03, a narrowed matcher would have rejected the first release after a bump in 2–3 of 14 cases. That was the reason for the carve-out.

    Why narrowing is now safe. Two changes close that coupling window without depending on regeneration timing:

    What #4020 does. It removes the carve-out. Each tenant's matcher names exactly its approved set: the applied signer, the main pin and the release candidate. The guard refuses the pattern form, a revision outside the set, a set missing the candidate, and an ungrouped alternation, each with a tenant-specific negative case.

    #3960's remaining criterion, regenerating on an actions release, now only shortens how long a bump PR waits. The safety of this change no longer depends on it.

  8. moved this from πŸ‘€ In Review to βœ… Done in 🌊 Project Boardon Sep 21, 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