fix(traffic): close the three ways a Worker secret sync misreports itself #542

Merged
gmackie merged 1 commit from fix/harden-worker-secret-sync into main 2026-08-30 23:57:35 +00:00
Owner

Three fixes from the 2026-08-30 control-plane incident. Each one made a correct-looking system misreport its own state, and each cost real time tonight.

1. A plain [vars] name wedged enable permanently

Cloudflare answers 10053 binding name already in use to secret put on a name already bound as an env var. The Worker has the value — it just isn't a secret. But syncSecretsToCanary recorded it as failed, and traffic.enable throws on any failed entry with nothing gating that throw. So the split could never be enabled, and no retry could clear it.

Hit live on OTEL_EXPORTER_OTLP_ENDPOINT, which agent/cmd/agent/deploy_env.go:57 injects as a var at deploy time while the stage store also carried it:

env.OTEL_EXPORTER_OTLP_ENDPOINT ("https://otlp.forgegraf.com")   Environment Variable

Now classified skipped. A genuine push error is still failed — there's a test for that, because collapsing the two would hide real breakage.

2. The CLI sync could re-introduce what #541 removed

forge secret sync --target cloudflare had no denylist, so it could push DATABASE_URL back onto a Worker through the other door. It targets forgegraf, which is why it hasn't bitten yet — but the apex now routes through the one-box router, so that Worker serves production.

Mirrors WORKER_FORBIDDEN_SECRET_KEYS, with a test on each side so the two lists can't drift apart silently.

3. The recovery workflow retried instead of diagnosing

It probed forgegraf.com 60 times over two minutes while printing the answer on the line immediately above the loop:

direct_worker_node_http=200 direct_worker_hub_http=200
curl: (22) The requested URL returned error: 503     x60

Direct worker healthy + domain broken is a routing fault, not a secrets fault. Retrying cannot fix a hostname pointed at a different script. Five consecutive red runs reported only 503.

It now fails immediately and says so, naming the query that identifies the hostname's owner. The other branch is explicit too: if the direct probe was also unhealthy, it says the fault is the Worker, not the binding.

Tests

  • 54 vitest across the three affected src/lib files; 3 new covering the 10053 split, including that a real CF error is still failed
  • Go test pinning both denylists against drift
  • go build ./... clean, YAML parses

Note: packages/api vitest can't run through its normal entrypoint locally (PGlite global setup fails, pre-existing), so these were run with a scoped config. That gap is exactly what let a broken test through on #541, so this time the whole blast radius was run rather than one file.

🤖 Generated with Claude Code

Three fixes from the 2026-08-30 control-plane incident. Each one made a correct-looking system misreport its own state, and each cost real time tonight. ## 1. A plain `[vars]` name wedged `enable` permanently Cloudflare answers **`10053 binding name already in use`** to `secret put` on a name already bound as an env var. The Worker *has* the value — it just isn't a secret. But `syncSecretsToCanary` recorded it as `failed`, and `traffic.enable` throws on any `failed` entry with **nothing gating that throw**. So the split could never be enabled, and no retry could clear it. Hit live on `OTEL_EXPORTER_OTLP_ENDPOINT`, which `agent/cmd/agent/deploy_env.go:57` injects as a var at deploy time while the stage store also carried it: ``` env.OTEL_EXPORTER_OTLP_ENDPOINT ("https://otlp.forgegraf.com") Environment Variable ``` Now classified `skipped`. A genuine push error is still `failed` — there's a test for that, because collapsing the two would hide real breakage. ## 2. The CLI sync could re-introduce what #541 removed `forge secret sync --target cloudflare` had no denylist, so it could push `DATABASE_URL` back onto a Worker through the other door. It targets `forgegraf`, which is why it hasn't bitten yet — but the apex now routes through the one-box router, so that Worker serves production. Mirrors `WORKER_FORBIDDEN_SECRET_KEYS`, with a test on each side so the two lists can't drift apart silently. ## 3. The recovery workflow retried instead of diagnosing It probed `forgegraf.com` 60 times over two minutes while printing the answer on the line immediately above the loop: ``` direct_worker_node_http=200 direct_worker_hub_http=200 curl: (22) The requested URL returned error: 503 x60 ``` Direct worker healthy + domain broken is a **routing** fault, not a secrets fault. Retrying cannot fix a hostname pointed at a different script. Five consecutive red runs reported only `503`. It now fails immediately and says so, naming the query that identifies the hostname's owner. The other branch is explicit too: if the direct probe was *also* unhealthy, it says the fault is the Worker, not the binding. ## Tests - 54 vitest across the three affected `src/lib` files; 3 new covering the 10053 split, including that a real CF error is still `failed` - Go test pinning both denylists against drift - `go build ./...` clean, YAML parses Note: `packages/api` vitest can't run through its normal entrypoint locally (PGlite global setup fails, pre-existing), so these were run with a scoped config. That gap is exactly what let a broken test through on #541, so this time the whole blast radius was run rather than one file. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
fix(traffic): close the three ways a Worker secret sync misreports itself
All checks were successful
CI / gitleaks (pull_request) Successful in 6s
CI / storybook (pull_request) Successful in 2m17s
forgegraph/ci CI passed
CI / ci (pull_request) Successful in 10m46s
d5ccfc8a00
All three surfaced during the 2026-08-30 control-plane incident, and each one
made a correct-looking system lie about its own state.

1. A name already bound as a plain [vars] entry wedged `enable` permanently.
   Cloudflare answers 10053 to `secret put` on such a name. The Worker HAS the
   value, just not as a secret — but the sync recorded it as `failed`, and
   `traffic.enable` throws on any failure with nothing gating the throw. So the
   split could never be enabled and no retry could ever clear it. Hit on
   OTEL_EXPORTER_OTLP_ENDPOINT, which deploy_env.go injects as a var while the
   stage store also carried it. Now classified `skipped`; a genuine push error
   is still `failed`.

2. `forge secret sync --target cloudflare` had no denylist, so the CLI path
   could re-introduce exactly what #541 removed from the canary path. Mirrors
   WORKER_FORBIDDEN_SECRET_KEYS, with a test on both sides so they cannot
   drift apart unnoticed.

3. control-plane-recovery.yml retried a domain probe 60 times while printing
   the answer on the line above it. When the direct workers.dev probe is 200
   and forgegraf.com is not, the secrets are fine and the HOSTNAME points at a
   different script — retrying proves nothing. It now fails immediately, says
   it is a routing fault, and names the query that identifies the owner. Five
   consecutive red runs said only "503".

Tests: 54 vitest (3 new for the 10053 split, including that a real error is
still failed), plus a Go test pinning both denylists.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Author
Owner

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

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

Preview environment is live: https://pr-542-forgegraph.forgegraf.com Deployed `d5ccfc8a` with the beta stage's environment. It redeploys on every push and is destroyed when this PR closes.
gmackie deleted branch fix/harden-worker-secret-sync 2026-08-30 23:57:35 +00:00
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!542
No description provided.