Skip to content

A crash mid-swap or a canary restore can put a revoked preview live after disable_previews #3805

Open

Description

@dawsontoth

The fix for #3613 makes disable_previews a barrier every node acknowledges: each node records the revoked identity generation under the per-app admission lock, and every preview activation (origin and peer) checks it inside that lock before its swap. These paths do not run that check, or fence a different generation, so a release of the revoked identity can still go live on a node after disable reported success:

  • Crash recovery. A worker that dies mid-swap leaves a complete candidate and an activation journal. settleInterruptedActivation (components/Application.ts, called from recoverInterruptedActivations) rolls it forward at the next preparation or restart without consulting the fence.

  • Canary restore. When certification rejects a release, restoreRejectedRelease (components/canaryRollout.ts) puts the predecessor back through prepareApplication without aroundActivation. If the predecessor was a fenced preview, it goes live again.

  • A failed deploy's rollback. A super user's deploy over <app>-pr-<n> carries no generation, so it takes the unfenced path. If the census sees that build and the activation then rolls back, the fenced predecessor comes back live and disable misses it.

  • Two enables at once. Two enable_previews of one app on two nodes, before the role replicates, each mint a generation and one is lost. Previews deployed through the other node carry a generation disable never fences.

The first three need a crash, a rejection or a failed deploy to land during a disable, and all of these stay bounded by the preview's expiresAt (at most the grant's maxTtlDays, 7 by default), after which the expiry sweep drops it. The revoked identity cannot deploy anything new.

A fix would refuse a fenced publication in recoverInterruptedActivations' failures map, keeping the candidate, aside and journal intact, merge fenced live trees into the componentsFailedClosed() refusals the loaders already honor, and check the restoration source's generation under the publication lock. Planning review for #3613 weighed this and chose to keep it out of that fix, since it changes recovery for every deploy.

Review-Deferral: 6dcb02d3f24337d7 severity=minor category=security
Review-Deferral: b3b729fd60768722 severity=minor category=concurrency

Part of #3042.

🤖 Generated with Claude Code

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

    Type

    Fields

    Priority

    P3

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions