Skip to content

control-plane: eight pre-deploy defects, headlined by two tenants sharing one Neon database (and one customer's teardown deleting another's) #9143

Description

@JSONbored

Status first

The control-plane is not deployed and currently cannot be. There is no deploy workflow in
.github/workflows/ (only ci.yml, which runs its tests); cf:deploy is a manual wrangler deploy;
and control-plane/wrangler.jsonc:43,54 still carry "REPLACE_WITH_REAL_NEON_PROJECT_ID" and
"REPLACE_WITH_REAL_KV_NAMESPACE_ID", so a deploy fails at the KV binding.

Every defect below is therefore latent — none can hurt a contributor today. They are filed because
this is the hosted-ORB substrate and a first deploy would ship all of them intact. Also stated plainly
for the record: no secrets are committed; vars hold only NEON_PROJECT_ID,
LOOPOVER_ENABLE_PAGERDUTY, MAIN_APP_BASE_URL, and every sensitive value is a documented
wrangler secret put. There is no fail-open auth path in this Worker — verified by running the real
app.use("/v1/tenants/*") matcher: an unset/blank ADMIN_TOKEN yields 503 service_not_configured,
never open.

1. Two tenant names differing only in case or punctuation share one Neon branch, database, role and password

branchNameFor (control-plane/src/neon-database-driver.ts:67-74) lowercases and collapses:

const raw = `tenant-${request.product}-${request.tenant.name}`.toLowerCase();
const sanitized = raw.replaceAll(/[^a-z0-9_-]+/g, "-").replaceAll(/-{2,}/g, "-")

roleNameFor / databaseNameFor are aliases of it. The registry keys on the raw name
(tenant-registry.ts:70), and the create route validates only typeof name === "string" && !!name.trim()
(http-app.ts:161). So "Acme", "acme", "acme corp", "acme-corp", and "acme.corp" are distinct
registry records that all derive one branch name.

provisionNeonDatabase then takes its idempotent path — findBranchByName hits the first tenant's
branch and reveal_password hands tenant #2 tenant #1's live credentials. #8026 added a hash suffix,
but only in the sanitized.length > 63 branch; short colliding names never reach it.

dropNeonDatabase is name-derived too, so DELETE /v1/tenants/Acme?product=orb deletes the branch
acme is still running on — one customer's teardown destroys another's database.

Fix: always append the hash suffix rather than only when truncating, or validate name at
http-app.ts against the exact charset the branch name preserves (^[a-z0-9][a-z0-9-]{0,40}$) so
registry key and branch name are 1:1.

2. One tenant's update permanently deletes another live tenant's webhook-routing index entry

upsert (tenant-registry.ts:137-145) maintains the installation:<id> secondary index by comparing
only against its own previous record, never checking whether the key still points at this tenant:

if (previous?.orbInstallationId !== undefined && previous.orbInstallationId !== record.orbInstallationId) {
  await kv.delete(installationIndexKeyFor(previous.orbInstallationId));
}

Meanwhile http-app.ts:184-192 deliberately lets a failed/torn down tenant's installation claim be
taken over (isRecreatableState). Sequence: tenant A created with orbInstallationId: 100, provision
fails, record persists as failed with that id → tenant B created with 100, index now points at B, B
goes active → anyone re-creates or deletes A, whose teardown upsert writes A without
orbInstallationIdkv.delete("installation:100"), B's pointer.

B stays active with orbInstallationId: 100, but getByOrbInstallationId(100) returns undefined, so
orb-webhook-router.ts:106 answers 404 unknown_installation for every delivery, forever.
orbInstallationId is settable only at create (http-app.ts:166-170) and B already exists, so
POST /v1/tenants 409s — no autonomous or manual exit. B silently stops reviewing PRs.

Fix: make the index delete conditional on the key equalling this tenant's own primary key; add a route
to re-link orbInstallationId on an existing tenant.

3. POST /v1/tenants/rollout is completely inert

Four independent breaks in one chain (http-app.ts:233-262, container-driver.ts:98-111): the route
only upserts the registry and restarts nothing; the sole reader createTenantContainer early-returns on
isProvisioned(); provisionTenant({ name }, …) (http-app.ts:210) constructs a fresh Tenant
carrying only name, dropping pinnedVersion; and LOOPOVER_PINNED_VERSION has zero consumers in
the repo — no Dockerfile, entrypoint, or src/ code reads it. container-driver.ts:70-73's comment
("whose entrypoint resolves the versioned artifact itself") describes code that does not exist.

The endpoint returns 200 echoing the pin. Rollback is the control you least want to discover is
fake.
Fix: implement the chain, or delete the endpoint until the image side exists.

4. Injected env vars are lost on every container restart

createTenantContainer passes envVars only on the one-time provisioning start(). The vendored SDK
resolves const envVars = options?.envVars ?? this.envVars (@cloudflare/containers container.js:1327)
where this.envVars = {} is a plain class field, never persisted and never set by
OrbTenantContainer/AmsTenantContainer (worker.ts:39-52). Both later wake paths supply none: ORB via
containerFetchstartAndWaitForPorts(port, {abort}), AMS via stub.start({ entrypoint }) only.

An ORB tenant idling past sleepAfter = "10m" restarts with LOOPOVER_TENANT_SECRET_TOKEN absent — the
#8202 credential delivery works exactly once per tenant lifetime. For AMS (sleepAfter = "1m") every
cron wake after provision runs with no env at all. Fix: persist the env map in the DO's ctx.storage
beside PROVISIONED_STORAGE_KEY and reload into this.envVars on construction/onStart.

5. A hosted ORB tenant can never verify a routed webhook

The router's header says the container "runs the SAME self-host webhook-handling code unmodified and
re-verifies independently". That handler fails closed on a missing secret
(src/orb/webhook.ts:38-43). But createTenantContainer injects exactly three env vars —
LOOPOVER_PINNED_VERSION, LOOPOVER_TENANT_SECRET_TOKEN, LOOPOVER_CENTRAL_POSTHOG_KEY — and
ORB_GITHUB_WEBHOOK_SECRET (plus the tenant's DB URL, Redis URL and App credentials) is never set on any
tenant container. Every correctly-signed delivery gets a 401 from the container, and GitHub eventually
disables the hook.

Compounding: fetchBrokeredStoredSecret (src/orb/broker-client.ts:104), the ORB-side consumer of the
bootstrap secret, has no call site in src/ — only tests. The ORB half of #8202 is asserted in tests
and never wired.

6. The one unauthenticated route buffers an unbounded body twice before verifying

orb-webhook-router.ts:85-91 clones the request, reads the full body, and computes an HMAC-SHA256 over
it — all before authentication, with no content-length pre-check and no streaming limit. Its declared
twin handleOrbWebhook has both (src/orb/webhook.ts:28-37, governed by
GITHUB_WEBHOOK_MAX_BODY_BYTES). Same shape as #8888, which was filed and fixed in src/ and did not
touch this separate workspace.

7. A 10-minute per-tenant poll under a 5-minute cron guarantees overlapping ticks

DEFAULT_POLL_TIMEOUT_MS = 10 * 60 * 1000 for one tenant, a deliberately sequential loop, a */5 * * * *
cron (wrangler.jsonc:35), nextDueAt advanced only after the poll resolves (ams-wake.ts:106-115),
and no lock or in-flight marker. One hung tenant → tick 2 fires while tick 1 blocks, sees the same
nextDueAt, and calls stub.start() again; the SDK fast-paths if (this.container.running) return 0
and silently discards the entrypoint. Both ticks race to write nextDueAt/lastExitCode, so #7182's
"0=success/2=failure" contract can report the wrong tenant's outcome. With N due tenants a tick's worst
case is N × 10 minutes, far past any waitUntil budget. Fix: write nextDueAt forward before
starting, cap the timeout well under the cron interval, bound tenants per tick.

8. KV's eventual consistency makes every uniqueness and claim check advisory

The name-conflict check, the installation-already-claimed check, and the rollout precondition scan are
all kv.get followed later by an unconditional kv.put. Workers KV has no CAS and no cross-PoP
read-your-writes. Two concurrent creates both observe "no conflict"; the second overwrites the first, so
the first tenant's secretRef is lost and its broker enrolment can never be revoked. Simpler variant: a
DELETE during an in-flight create runs deprovisionTenant with existing.secretRef still undefined →
revokeTenantSecrets early-returns (secret-driver.ts:101) → the create's final upsert resurrects the
record as active over destroyed infrastructure. Fix: move the tenant index into a Durable Object or
D1 so check-and-claim is one serialized transaction; at minimum refuse DELETE on provisioning.

Requirements

Treat 1, 2 and 5 as deploy blockers — they are cross-tenant data isolation, a permanent silent
webhook outage, and a feature that cannot work at all. 3 and 4 should be fixed or removed before anyone
relies on rollback or on delivered credentials. 6, 7, 8 are hardening.

Add the missing deploy workflow and replace the two REPLACE_WITH_REAL_* placeholders as part of
whichever change first makes this deployable, so the gap between "tests pass" and "deployable" is closed
deliberately.

Test Coverage Requirements

99%+ patch coverage, branch-counted, per fix. Isolation tests for 1 (colliding names must not share a
branch) and 2 (an unrelated tenant's index survives) are the load-bearing ones.

Links & Resources

maintainer-only — hosted tenancy and data isolation.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions