fix(traffic): give the one-box canary its secrets #477

Merged
gmackie merged 4 commits from fix/traffic-lifecycle-recovery into main 2026-08-27 21:07:38 +00:00
Owner

Root cause of a live control-plane incident: forgegraf.com served from forgegraph-onebox, a Worker with zero secrets, so every bearer request 401d and all 7 node agents went offline for ~30 minutes.

The enable path comments that the one-box target "shares secrets" with the stage. True for a node service (env file from stage secrets each deploy); false for a Worker, where Cloudflare scopes secrets per worker name. The canary is created with none, deploying code adds none, and they cannot be copied off the primary because Worker secrets are write-only. They must be pushed from our encrypted store. putWorkerSecret already existed with exactly one caller, none in the one-box path.

Why it is worse than a degraded canary: verifyBearerTokenWithDB returns invalid when FG_API_TOKEN is unset, before the DB is consulted. A canary owning the production hostname denies all machine auth platform-wide, and the deploy meant to replace it needs that same auth to finish, so it cannot self-recover.

enable now pushes stage secrets to the canary and refuses to enable if any fail, naming them. Half-configured is the dangerous state.

Root cause of a live control-plane incident: forgegraf.com served from forgegraph-onebox, a Worker with **zero secrets**, so every bearer request 401d and all 7 node agents went offline for ~30 minutes. The enable path comments that the one-box target "shares secrets" with the stage. True for a node service (env file from stage secrets each deploy); false for a Worker, where Cloudflare scopes secrets per worker *name*. The canary is created with none, deploying code adds none, and they cannot be copied off the primary because Worker secrets are write-only. They must be pushed from our encrypted store. `putWorkerSecret` already existed with exactly one caller, none in the one-box path. Why it is worse than a degraded canary: `verifyBearerTokenWithDB` returns invalid when FG_API_TOKEN is unset, *before* the DB is consulted. A canary owning the production hostname denies all machine auth platform-wide, and the deploy meant to replace it needs that same auth to finish, so it cannot self-recover. enable now pushes stage secrets to the canary and refuses to enable if any fail, naming them. Half-configured is the dangerous state.
fix(traffic): give the one-box canary its secrets
Some checks failed
CI / gitleaks (pull_request) Has been cancelled
CI / storybook (pull_request) Has been cancelled
CI / ci (pull_request) Has been cancelled
f27bd9f180
Root cause of a live control-plane incident: forgegraf.com was serving
from forgegraph-onebox, a Worker with zero secrets, so every bearer
request 401'd and all 7 node agents went offline for ~30 minutes.

The enable path's own comment says the one-box target "shares
secrets/DB/resources" with the stage. That is true for a node service,
which gets an env file built from stage secrets on every deploy. It is
false for a Worker: Cloudflare scopes secrets per worker *name*, so the
canary is created with none, and deploying code to it does not add any.
They also cannot be copied off the primary, because Worker secrets are
write-only. They have to be pushed from our encrypted store, which is the
only place the plaintext exists. putWorkerSecret already existed and had
exactly one caller, none of them the one-box path.

Why this is worse than a degraded canary: verifyBearerTokenWithDB returns
invalid when FG_API_TOKEN is unset, before the database is consulted. A
canary owning the production hostname therefore denies all machine auth
platform-wide -- and the deploy meant to replace it needs that same auth
to finish, so it cannot self-recover.

enable now pushes the stage's secrets to the canary and refuses to enable
if any fail, naming them. Half-configured is the dangerous state: the
canary looks healthy and fails on the first authenticated request.

A decryption failure never falls back to pushing ciphertext -- that would
leave the canary looking configured while holding a plausible bad value.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Author
Owner

Preview environment is live: https://pr-477-forgegraph.forgegraf.com

Deployed f27bd9f1 with the beta stage's environment. It redeploys on every push and is destroyed when this PR closes.

Preview environment is live: https://pr-477-forgegraph.forgegraf.com Deployed `f27bd9f1` with the beta stage's environment. It redeploys on every push and is destroyed when this PR closes.
fix(traffic): refuse to enable a canary missing the primary's secrets
Some checks failed
CI / gitleaks (pull_request) Has been cancelled
CI / storybook (pull_request) Has been cancelled
CI / ci (pull_request) Has been cancelled
66180854c2
Pushing the stage's secrets is not proof the canary is complete, and my
own previous commit made exactly that mistake: it verified the pushes
succeeded, not that the required secrets exist. A canary can receive every
stage secret, report zero failures, and still be fatally incomplete.

On this install 26 of the primary Worker's 41 secrets are not in
ForgeGraph's store at all -- FG_API_TOKEN, FG_SESSION_KEY,
FG_ENCRYPTION_KEY, BETTER_AUTH_SECRET, every OAuth client secret. They
were set directly with
wrangler secret put <key>

Create or update a secret for a Worker

POSITIONALS
  key  The variable name to be accessible in the Worker  [string] [required]

GLOBAL FLAGS
  -c, --config    Path to Wrangler configuration file  [string]
      --cwd       Run as if Wrangler was started in the specified directory instead of the current working directory  [string]
  -e, --env       Environment to use for operations, and for selecting .env and .dev.vars files  [string]
      --env-file  Path to an .env file to load - can be specified multiple times - values from earlier files are overridden by values in later files  [array]
  -h, --help      Show help  [boolean]
  -v, --version   Show version number  [boolean]

OPTIONS
      --name  Name of the Worker. If this is not specified, it will default to the name specified in your Wrangler config file.  [string], and Worker secrets are
write-only, so nothing in the system can read them to copy them. The
sync would have pushed 15 of 41 and declared success.

Cloudflare withholds secret values but not secret *names*, so parity is
checkable without ever holding plaintext -- which is the only option here,
since for those 26 no plaintext exists anywhere the server can reach.
enable now compares the two Workers' secret names and refuses with
PRECONDITION_FAILED, naming what is missing and how to add it, rather than
handing the production hostname to a Worker that cannot authenticate.

That is the check that would have prevented today's incident.

331 tests pass across packages/api.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Author
Owner

Preview environment is live: https://pr-477-forgegraph.forgegraf.com

Deployed 66180854 with the beta stage's environment. It redeploys on every push and is destroyed when this PR closes.

Preview environment is live: https://pr-477-forgegraph.forgegraf.com Deployed `66180854` with the beta stage's environment. It redeploys on every push and is destroyed when this PR closes.
fix(traffic): block the shift itself when the canary lacks the primary's secrets
Some checks failed
CI / gitleaks (pull_request) Successful in 6s
CI / storybook (pull_request) Successful in 1m23s
forgegraph/ci CI failed
CI / ci (pull_request) Failing after 9m46s
64ceead063
The enable-time check does not cover the case that caused the incident.
A deploy runs canary_deploying -> shifting without ever calling enable, so
a split enabled before that check existed still hands the production
hostname to a canary that cannot authenticate. ForgeGraph's own split is
in exactly that state right now.

shiftAndBake is the one place every path to serving traffic passes
through, so the guard belongs there: compare the two Workers' secret names
and throw before the weight moves. It composes with the earlier fixes --
startBake records the reason, the sweeper retries once then fails the
deploy -- so the outcome is a failed deploy naming the missing secrets
instead of a live 401 outage.

Only applies to Workers lanes; node-platform targets carry no workerName
and build their own env files from stage secrets.

The canary API is typed structurally rather than as
Pick<CloudflareClient, ...> so the lifecycle does not pull the client
module into its graph for two method signatures.

334 tests pass across packages/api.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Author
Owner

Reviewed. The diagnosis is right and this is not superseded — git grep listWorkerSecretNames|syncSecretsToCanary|assertCanaryCanServeTraffic against main returns nothing, and the three traffic commits that landed since (82d29c2d #475, d2af6f91 #479, 849263f3 #482) all address hostname stealing, a different facet of the same incident.

The red CI is not this PR's fault. Task 23991 failed in Vet and test Go agent on a transient module-proxy error:

github.com/klauspost/cpuid/v2@v2.2.11: read "https://proxy.golang.org/…zip":
stream error: stream ID 99; INTERNAL_ERROR; received from peer

The same log reports test | passed | 1151 passed / 0 failed / 10 skipped. A re-run should go green.


The blocker: this would stop deploys fleet-wide, including forgegraf.com

assertCanaryCanServeTraffic fires in shiftAndBake, which every one-box deploy passes through. But the only thing that ever pushes secrets to a canary is syncSecretsToCanary, called from exactly one place — the enable mutation in packages/api/src/routers/traffic.ts:303. Nothing re-syncs on deploy.

I checked the production control-plane DB:

select enabled, count(*) from traffic_splits group by 1;
 f | 1
 t | 22

All 22 enabled splits — calzone, controlsfoundry, creator, crucible, daily-dose, driftport, fabforge, festigram, forgegraph, fryos, habit-app, insure, jobs-pulse, latchflow, linear-clone, netcontrol, omnidat-app, playtrek, streamconductor, test-dojo, trip, veritas — were enabled before this code existed, so none of their canaries has ever received a secret push. On merge, each one's next production deploy throws at the shift step, and re-running enable is the only remediation.

It is worse for ForgeGraph itself: the PR body notes 26 of the primary's 41 secrets were set out-of-band with wrangler secret put and are not in the store, so even a disable/re-enable would fail the strict parity check. That is a self-blocking condition on the platform's own deploy path.

Suggested shape: land the enable-time sync now, but either put the parity guard behind a flag / warn-only mode, or add a shift-time sync before the shift-time assert, so an already-enabled split self-heals instead of dead-ending. The 26 out-of-band secrets need backfilling into the store before a strict check goes live either way.

Two smaller issues

  1. listWorkerSecretNames fails in opposite directions. It returns [] whenever data.success is false and never checks resp.ok. A CF API error on the primary lookup therefore makes the guard silently pass (fails open, defeating it); the same error on the canary lookup reports every primary secret as missing and blocks the deploy (fails closed, spuriously).
  2. The deploymentTargets select in shiftAndBake runs before the workerName check, so every shift pays a DB roundtrip even on node-platform lanes. Cosmetic.

Not merging — the guard's rollout needs a decision, not a patch.

Reviewed. **The diagnosis is right and this is not superseded** — `git grep listWorkerSecretNames|syncSecretsToCanary|assertCanaryCanServeTraffic` against main returns nothing, and the three traffic commits that landed since (`82d29c2d` #475, `d2af6f91` #479, `849263f3` #482) all address hostname stealing, a different facet of the same incident. **The red CI is not this PR's fault.** Task `23991` failed in `Vet and test Go agent` on a transient module-proxy error: ``` github.com/klauspost/cpuid/v2@v2.2.11: read "https://proxy.golang.org/…zip": stream error: stream ID 99; INTERNAL_ERROR; received from peer ``` The same log reports `test | passed | 1151 passed / 0 failed / 10 skipped`. A re-run should go green. --- ## The blocker: this would stop deploys fleet-wide, including forgegraf.com `assertCanaryCanServeTraffic` fires in `shiftAndBake`, which every one-box deploy passes through. But the only thing that ever *pushes* secrets to a canary is `syncSecretsToCanary`, called from exactly one place — the `enable` mutation in `packages/api/src/routers/traffic.ts:303`. **Nothing re-syncs on deploy.** I checked the production control-plane DB: ```sql select enabled, count(*) from traffic_splits group by 1; f | 1 t | 22 ``` All 22 enabled splits — `calzone`, `controlsfoundry`, `creator`, `crucible`, `daily-dose`, `driftport`, `fabforge`, `festigram`, **`forgegraph`**, `fryos`, `habit-app`, `insure`, `jobs-pulse`, `latchflow`, `linear-clone`, `netcontrol`, `omnidat-app`, `playtrek`, `streamconductor`, `test-dojo`, `trip`, `veritas` — were enabled **before this code existed**, so none of their canaries has ever received a secret push. On merge, each one's next production deploy throws at the shift step, and re-running `enable` is the only remediation. It is worse for ForgeGraph itself: the PR body notes 26 of the primary's 41 secrets were set out-of-band with `wrangler secret put` and are not in the store, so even a disable/re-enable would fail the strict parity check. That is a self-blocking condition on the platform's own deploy path. **Suggested shape:** land the enable-time sync now, but either put the parity guard behind a flag / warn-only mode, or add a shift-time *sync* before the shift-time *assert*, so an already-enabled split self-heals instead of dead-ending. The 26 out-of-band secrets need backfilling into the store before a strict check goes live either way. ## Two smaller issues 1. **`listWorkerSecretNames` fails in opposite directions.** It returns `[]` whenever `data.success` is false and never checks `resp.ok`. A CF API error on the **primary** lookup therefore makes the guard silently pass (fails open, defeating it); the same error on the **canary** lookup reports every primary secret as missing and blocks the deploy (fails closed, spuriously). 2. The `deploymentTargets` select in `shiftAndBake` runs before the `workerName` check, so every shift pays a DB roundtrip even on node-platform lanes. Cosmetic. Not merging — the guard's rollout needs a decision, not a patch.
gmackie force-pushed fix/traffic-lifecycle-recovery from 64ceead063
Some checks failed
CI / gitleaks (pull_request) Successful in 6s
CI / storybook (pull_request) Successful in 1m23s
forgegraph/ci CI failed
CI / ci (pull_request) Failing after 9m46s
to d975761077
All checks were successful
CI / gitleaks (pull_request) Successful in 7s
CI / storybook (pull_request) Successful in 1m47s
forgegraph/ci CI passed
CI / ci (pull_request) Successful in 11m47s
2026-08-27 20:42:51 +00:00
Compare
Author
Owner

Preview environment is live: https://pr-477-forgegraph.forgegraf.com

Deployed d9757610 with the beta stage's environment. It redeploys on every push and is destroyed when this PR closes.

Preview environment is live: https://pr-477-forgegraph.forgegraf.com Deployed `d9757610` with the beta stage's environment. It redeploys on every push and is destroyed when this PR closes.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
gmackie/ForgeGraph!477
No description provided.