[security] Non-admin lease creation can choose provider key names later treated as Crabbox-owned cleanup targets

[coygeek]

[security] Non-admin lease creation can choose provider key names later treated as Crabbox-owned cleanup targets

Summary

The coordinator's POST /v1/leases route accepts providerKey from any
authenticated caller and stores it as the lease's provider key without an
admin gate, per-owner namespace, or validation against the lease id. The
normal Go CLI derives this value from the lease id before sending the request
(internal/cli/provider_coordinator.go:41-59,
internal/cli/lease.go:155-160), and the documentation describes brokered
provider keys as stable per-lease names (docs/features/ssh-keys.md:51-62).
The HTTP API itself does not enforce that invariant.

That makes providerKey a non-admin-controlled provider-resource identifier.
When it matches Crabbox's cleanup pattern (crabbox-cbx-[a-f0-9]{12}), the
AWS and Hetzner release paths later treat the name as a Crabbox-managed key
and delete it under the broker's provider credentials. This crosses the
maintainer-documented boundary for destructive provider actions against
resources Crabbox cannot strongly identify as its own: a low-privilege
coordinator user can cause provider key-pair or SSH-key deletion using a name
that was not derived from their lease.

The original draft overstated two details that are not supported by current
source. First, cleanup is not triggered for arbitrary names such as the legacy
default crabbox-steipete; both AWS and Hetzner cleanup require the
crabbox-cbx-<12 hex> pattern. Second, deleting a provider key object does not
by itself prove loss of SSH access to an already-running machine, because cloud
providers usually inject the public key at instance creation time. The
source-backed impact is unauthorized mutation/deletion of provider-side
Crabbox key records and identity confusion in broker-managed provider
metadata.

Affected Components

• Checked commit: 6dba4afd1c5a4be0a780cf035d1b397e8982d478
• Component: coordinator lease creation, provider SSH key identity, provider
  cleanup.
• Affected files and lines:
  • worker/src/types.ts:124-194 includes providerKey in the public
    LeaseRequest body.
  • worker/src/fleet.ts:867-872 exposes POST /v1/leases as a non-admin
    route.
  • worker/src/config.ts:112-120 and worker/src/config.ts:255-327 build a
    LeaseConfig directly from the request body; providerKey is assigned
    from input.providerKey ?? "crabbox-steipete" at config.ts:316.
  • worker/src/fleet.ts:1645-1671 reserves only the workspace provider-key
    prefix and admin-gates native image sources and host pinning. It does not
    reject or derive caller-supplied ordinary providerKey values.
  • worker/src/fleet.ts:1737-1758 stores the chosen config.providerKey in
    the lease record.
  • worker/src/fleet.ts:12771-12773 defines the cleanup-trusted key pattern
    as crabbox-cbx-[a-f0-9]{12}.
  • worker/src/aws.ts:1018-1035 accepts an existing EC2 key pair by name
    without verifying that its public key matches the lease's submitted public
    key, and worker/src/aws.ts:1067-1072 uses that same name as the EC2
    KeyName.
  • worker/src/fleet.ts:15607-15608 deletes an AWS EC2 key pair on release
    whenever the stored lease providerKey matches the Crabbox cleanup
    pattern.
  • worker/src/hetzner.ts:97-151 uses providerKey as the Hetzner SSH-key
    name for lookup/create/delete. Hetzner does compare public-key identity
    during creation (hetzner.ts:103-108), which reduces the collision impact,
    but cleanup still deletes by raw matching name when the stored key passes
    the coordinator cleanup predicate (worker/src/fleet.ts:14949-14953).
  • worker/src/provider-labels.ts:4-23 stores the caller-selected provider
    key in provider labels, so provider metadata no longer reliably reflects a
    broker-derived per-lease identity.

Attack Path

Attacker role: authenticated non-admin coordinator user with a signed user
token or shared operator token.

Prerequisites:

• The deployment exposes the coordinator lease API to authenticated users.
• The broker is configured for AWS or Hetzner.
• The attacker can send a crafted POST /v1/leases request rather than using
  the stock Go CLI, which normally derives ProviderKey from the new lease id.
• For the strongest AWS path, the attacker chooses a providerKey matching the
  Crabbox cleanup pattern and colliding with a provider key pair that already
  exists in the broker's AWS account.

Steps:

1. Send POST /v1/leases as a non-admin caller with a body containing a
   chosen providerKey, for example a crabbox-cbx-<12 hex> value associated
   with another lease or a guessed Crabbox-managed provider key.
2. leaseConfig copies the value into config.providerKey
   (worker/src/config.ts:316), and createLease stores it in the lease record
   (worker/src/fleet.ts:1737-1758). The ordinary non-admin guard at
   worker/src/fleet.ts:1645-1671 does not reject the value unless it uses the
   reserved workspace prefix.
3. On AWS, ensureSSHKey calls DescribeKeyPairs and returns successfully if
   the chosen key name already exists (worker/src/aws.ts:1018-1021), without
   comparing the submitted public key to the existing EC2 key pair. The
   instance launch then uses the attacker-selected KeyName
   (worker/src/aws.ts:1067-1072).
4. When the attacker's lease is released and has a cloud instance to clean up,
   AWS cleanup calls DeleteKeyPair for the stored lease.providerKey if it
   matches crabbox-cbx-[a-f0-9]{12} (worker/src/fleet.ts:15607-15608,
   worker/src/aws.ts:1003-1009).
5. On Hetzner, a mismatched existing key blocks server creation
   (worker/src/hetzner.ts:103-108), so the same collision is harder to turn
   into a successful active lease. However, the API still accepts the
   non-admin-selected provider key as the provider SSH-key name, and successful
   leases with cleanup-pattern names are later deleted by raw name
   (worker/src/fleet.ts:14949-14953, worker/src/hetzner.ts:142-151).

Control/data flow:

authenticated non-admin request body
  -> LeaseRequest.providerKey
  -> leaseConfig(...).providerKey
  -> stored LeaseRecord.providerKey
  -> AWS KeyName / Hetzner SSH key name and provider labels
  -> release cleanup deletes matching crabbox-cbx-* provider key

Impact

A non-admin coordinator caller can make the broker perform provider key
operations against an identifier that is not derived from the caller's lease.
For AWS, the source proves that an existing EC2 key pair name is accepted
without public-key verification, used for instance launch, and then deleted on
release if it matches Crabbox's cleanup pattern. For Hetzner, public-key
identity comparison prevents the direct mismatched-key creation path, but the
same unvalidated identifier still reaches the provider key namespace and
cleanup code.

The practical impact is provider-resource integrity and availability damage:
Crabbox can delete or mislabel provider-side SSH key resources that it cannot
strongly bind to the releasing lease. This matches the in-scope policy example
for destructive provider actions against resources Crabbox cannot strongly
identify as its own. The issue does not expose private SSH keys, and current
source does not prove that deleting the provider key object removes SSH access
from already-running instances, so the severity should stay below the original
draft's high-impact availability claim.

Severity Assessment

CVSS Assessment

Metric	v3.1	v4.0
Score	5.4 / 10.0	5.3 / 10.0
Severity	Medium	Medium
Vector	CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:N/I:L/A:L	CVSS:4.0/AV:N/AC:L/AT:N/PR:L/UI:N/VC:N/VI:L/VA:L/SC:N/SI:N/SA:N
Calculator	CVSS v3.1 Calculator	CVSS v4.0 Calculator

The score reflects a network-reachable authenticated non-admin caller, no user
interaction, no confidentiality impact, and limited integrity/availability
impact to provider-side key-pair identity and lifecycle consistency. The
score is not higher because current source does not prove private-key exposure,
admin-token compromise, cross-owner coordinator data access, or guaranteed SSH
loss on already-running machines.

Recommended Remediation

• Do not trust providerKey from ordinary POST /v1/leases requests. For
  non-admin requests, derive the provider key server-side from the final lease
  id, matching the documented providerKeyForLease format
  (crabbox-<lease id> with underscores rewritten to dashes).
• Preserve a narrowly reviewed admin-only or internal path only if operators
  still need to supply a provider key manually. That path should reject
  crabbox-cbx-* names that do not correspond to the lease being created, or
  otherwise record ownership metadata that cleanup can verify before deletion.
• For AWS, verify that an existing EC2 key pair name matches the submitted
  public key before treating it as reusable. If AWS cannot return enough key
  material to prove a match, fail closed or require an admin-only override.
• For Hetzner and AWS cleanup, delete provider key resources only when the key
  name is derived from the same lease id or when stored ownership metadata shows
  Crabbox created that exact key for the releasing lease.
• Add regression tests under worker/test/fleet.test.ts and provider-specific
  tests for crafted non-admin providerKey values, AWS existing-key handling,
  and cleanup refusing to delete a key that is not bound to the lease.

Validation

Validation method: static source review of the verification target checkout at
6dba4afd1c5a4be0a780cf035d1b397e8982d478.

Evidence:

• Maintainer policy: SECURITY.md defines destructive provider actions against
  resources Crabbox cannot strongly identify as its own as in scope, while
  excluding only trusted-operator/project-configuration behavior without a
  separate Crabbox boundary bypass.
• Operations guidance: docs/security.md states that the coordinator owns
  authentication, authorization, lease state, provider credentials, cost
  guardrails, and cleanup.
• Public request shape: worker/src/types.ts:124-194 includes providerKey in
  LeaseRequest.
• Non-admin reachability: worker/src/fleet.ts:867-872 routes POST /v1/leases to createLease; worker/src/fleet.ts:1645-1671 has no
  non-admin rejection for ordinary providerKey values.
• Source/control/sink: worker/src/config.ts:316 copies the request value,
  worker/src/fleet.ts:1757 stores it, worker/src/aws.ts:1018-1035 and
  worker/src/aws.ts:1067-1072 use it as an EC2 key-pair identity, and
  worker/src/fleet.ts:15607-15608 deletes matching AWS key-pair names during
  release cleanup.
• Hetzner counterevidence and residual risk: worker/src/hetzner.ts:103-108
  rejects mismatched existing public keys, which blocks the most direct
  Hetzner collision path. The same file still uses providerKey as the
  provider SSH-key name (worker/src/hetzner.ts:97-151,
  worker/src/hetzner.ts:262-269), and the coordinator cleanup path deletes
  matching Crabbox-pattern key names by raw lease value
  (worker/src/fleet.ts:14949-14953).
• Normal-client counterevidence: the stock Go coordinator backend sets
  cfg.ProviderKey = providerKeyForLease(leaseID) before sending the lease
  request (internal/cli/provider_coordinator.go:41-59), so exploitation
  requires a crafted request or modified client rather than the default CLI
  flow.

No dynamic proof was run in this child verifier. The verdict is based on exact
source/control/sink evidence and policy-boundary review.

[clawsweeper]

Codex review: keeping this open for maintainer follow-up; there is still a little grit to resolve. Reviewed June 25, 2026, 9:57 PM ET / 01:57 UTC.

Summary
The issue is still necessary: current main still lets a non-admin lease request carry providerKey, stores that value on the lease, and later uses the stored name for AWS/Hetzner provider-key cleanup when it matches the Crabbox key pattern. The source path is direct enough to keep open for security review, but remediation needs maintainer judgment because it changes broker authorization and provider cleanup semantics.

Reproducibility: yes. A crafted non-admin POST /v1/leases can carry providerKey, and source inspection shows that value reaches lease storage, AWS/Hetzner key identity, provider labels, and cleanup predicates without server-side enforcement of the CLI-derived per-lease invariant.

Next step
This is security-sensitive broker authorization and provider cleanup work, so it should stay with maintainer security review rather than an autonomous fix lane.

Security
Needs attention: Source inspection supports a security-sensitive non-admin provider-resource mutation concern in the coordinator/provider cleanup path.

Review details

Best possible solution:

Derive non-admin broker provider keys server-side from the final lease identity, preserve any manual override only behind an explicit admin/internal path, and bind provider key cleanup to lease ownership rather than a caller-controlled name pattern.

Do we have a high-confidence way to reproduce the issue?

Yes. A crafted non-admin POST /v1/leases can carry providerKey, and source inspection shows that value reaches lease storage, AWS/Hetzner key identity, provider labels, and cleanup predicates without server-side enforcement of the CLI-derived per-lease invariant.

Is this the best way to solve the issue?

Mostly yes, but maintainer policy is needed for overrides. The narrow technical direction is to derive non-admin provider keys from the lease server-side and require admin-owned ownership metadata or a deliberate override path for any manual provider key.

AGENTS.md: found and applied where relevant.

Remaining risk / open question:

• The remediation changes a public coordinator request field that may be used by custom integrations, so maintainers need to decide whether any admin/internal provider-key override remains supported.
• No live AWS or Hetzner provider operation was run during review; the issue is source-reproducible, but regression tests should cover the cleanup and collision paths before accepting a fix.

Codex review notes: model internal, reasoning high; reviewed against 6dba4afd1c5a.

Label changes

Label changes:

• add P1: An authenticated non-admin caller can cross a broker/provider authorization boundary, but the proven impact is limited to provider key identity and cleanup rather than full credential disclosure.
• add impact:security: The issue concerns non-admin control over provider resource identifiers and destructive cleanup under coordinator/provider credentials.
• add issue-rating: 🦞 diamond lobster: Current issue advisory state selects this label.
• add clawsweeper:source-repro: Current issue advisory state selects this label.
• add clawsweeper:no-new-fix-pr: Current issue advisory state selects this label.
• add clawsweeper:needs-product-decision: Current issue advisory state selects this label.
• add clawsweeper:needs-security-review: Current issue advisory state selects this label.

Label justifications:

• P1: An authenticated non-admin caller can cross a broker/provider authorization boundary, but the proven impact is limited to provider key identity and cleanup rather than full credential disclosure.
• impact:security: The issue concerns non-admin control over provider resource identifiers and destructive cleanup under coordinator/provider credentials.

Evidence reviewed

Security concerns:

• [medium] Non-admin providerKey reaches cleanup selectors — worker/src/fleet.ts:1645
  The lease-create path accepts ordinary caller-supplied provider key names for non-admin requests, persists them, and release cleanup later deletes matching Crabbox-pattern provider keys by that stored value.
  Confidence: 0.9
• [medium] AWS accepts existing key pairs by name only — worker/src/aws.ts:1018
  AWS key provisioning returns after DescribeKeyPairs finds a key name and does not prove the existing key belongs to the lease's submitted public key before launch and later cleanup use that name.
  Confidence: 0.86

What I checked:

• Repository policy read: AGENTS.md was present and read fully; its provider-neutral architecture and security/configuration guidance applies because this issue crosses coordinator/provider boundaries. (AGENTS.md:1, 6dba4afd1c5a)
• Public request field: LeaseRequest declares providerKey?: string, so direct coordinator callers can supply a provider key name in the public lease-create body. (worker/src/types.ts:186, 6dba4afd1c5a)
• Request value is trusted into config: leaseConfig assigns providerKey: input.providerKey ?? "crabbox-steipete", preserving caller input before provider preparation. (worker/src/config.ts:316, 6dba4afd1c5a)
• Non-admin guard omits ordinary providerKey: createLease blocks workspace-reserved provider keys, native lease sources, and host pinning, but there is no non-admin rejection for ordinary caller-supplied providerKey names. (worker/src/fleet.ts:1645, 6dba4afd1c5a)
• Stored value reaches lease record: The lease record stores providerKey: config.providerKey, so the caller-selected value becomes persistent lease state. (worker/src/fleet.ts:1757, 6dba4afd1c5a)
• Cleanup trusts the stored name pattern: validCrabboxProviderKey accepts crabbox-cbx-[a-f0-9]{12}, and AWS/Hetzner release paths delete provider SSH keys by the stored lease providerKey when it matches that predicate. (worker/src/fleet.ts:12771, 6dba4afd1c5a)

Likely related people:

• steipete: GitHub path history shows early Cloudflare coordinator and broker AWS lease work plus recent AWS region validation, workspace AWS image, and workspace SSH ingress changes in the affected files. (role: original and recent coordinator/AWS area contributor; confidence: high; commits: b4cd35644393, 52d30da07cc0, edc893117b73; files: worker/src/fleet.ts, worker/src/aws.ts, worker/src/config.ts)
• vincentkoc: GitHub path history shows recent changes in AWS cleanup, worker quality/security cleanup, ARM lease selection, and config/provider hardening near this provider lifecycle surface. (role: recent AWS and Worker hardening contributor; confidence: medium; commits: b9163e9d971f, b13d4afa1a4e, 583f473aa01b; files: worker/src/aws.ts, worker/src/config.ts, worker/src/fleet.ts)
• coygeek: Local blame on the central current-main lines points to ae54e7a, and they authored this issue plus related source-backed provider-selector reports, making them a useful context source. (role: current-line author and security reporter; confidence: medium; commits: ae54e7ae9577, e7be0273d063; files: worker/src/fleet.ts, worker/src/config.ts, worker/src/aws.ts)

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.