Skip to content

security: bootstrap token path exposes mutex-stall DoS β€” no rate limit, no lockout, no alert#76322

Closed
fede-kamel wants to merge 4 commits into
openclaw:mainfrom
fede-kamel:security/bootstrap-dos-poc
Closed

security: bootstrap token path exposes mutex-stall DoS β€” no rate limit, no lockout, no alert#76322
fede-kamel wants to merge 4 commits into
openclaw:mainfrom
fede-kamel:security/bootstrap-dos-poc

Conversation

@fede-kamel

@fede-kamel fede-kamel commented May 3, 2026

Copy link
Copy Markdown
Contributor

Closes #77980

Summary

  • Bootstrap token auth path had no rate limiting, unlike the device-token path which has full rateLimiter.check() + rateLimiter.recordFailure() protection
  • An attacker opening N WebSocket connections with bogus bootstrap tokens serialises all through the shared verifyDeviceBootstrapToken mutex, stalling legitimate device pairing with no IP lockout or alert
  • Fix mirrors the existing device-token pattern: check before verify, record failure on reject, reset on success
  • Deferred failure accounting: bootstrap recordFailure is only flushed if the entire handshake fails. A device with a stale bootstrap token + valid device token is a legitimate connection β€” the pending failure is discarded when device-token succeeds

Changes

src/gateway/auth-rate-limit.ts

  • Added AUTH_RATE_LIMIT_SCOPE_BOOTSTRAP_TOKEN = "bootstrap-token" constant

src/gateway/server/ws-connection/auth-context.ts

  • Bootstrap path: check rate limit β†’ skip verify if limited β†’ verify β†’ reset on success
  • Failure accounting is deferred: pendingBootstrapFailure flag set on rejection; recordFailure called only after all fallback paths (device-token) complete and auth is still false
  • If device-token succeeds, the pending bootstrap failure is silently discarded

Test plan

17/17 passing (pnpm test src/gateway/server/ws-connection/auth-context.test.ts):

Unit (mock limiter)

  • Bootstrap path blocked before verifyBootstrapToken when rate-limited
  • recordFailure called on rejected bootstrap token (no device-token fallback)
  • reset called on successful bootstrap token
  • Scope isolation: check called with "bootstrap-token", never "device-token"
  • Stale bootstrap + valid device token β†’ auth succeeds, bootstrap recordFailure NOT called ← covers the deferred-failure invariant

Integration (real AuthRateLimiter)

  • 5 failed attempts β†’ 6th blocked before verifyBootstrapToken runs (mutex never acquired)
  • Locked attacker IP does not affect a clean IP
  • Legitimate device with correct token from a clean IP succeeds after attacker is locked out

Real behavior proof

  • Behavior addressed: bootstrap-token auth on verifyDeviceBootstrapToken had no rate limit. An attacker hammering bogus bootstrap tokens from one IP serialised through the shared withLock mutex (disk read + crypto compare + disk write per attempt) and starved legitimate pairing handshakes. With this PR, rateLimiter.check() runs before verifyDeviceBootstrapToken, so attempts past the threshold short-circuit without acquiring the mutex.
  • Real environment tested: local Node v22.22.2 runtime against the real patched src/gateway/server/ws-connection/auth-context.ts and src/gateway/auth-rate-limit.ts from this branch (security/bootstrap-dos-poc HEAD f3b4859b1b). No vitest, no mocks of the patched modules. The reproducer drives resolveConnectAuthState + resolveConnectAuthDecision directly with a real createAuthRateLimiter instance and a synthetic verifyBootstrapToken whose body holds an async mutex for 5 ms to model the real withLock-serialised disk I/O.
  • Exact steps or command run after this patch: a node runtime tsx reproducer at https://gist.github.com/fede-kamel/95a5086497fc730cd4841befb126742c imports the patched modules and runs three phases β€” sequential attacker burst, sustained attacker + concurrent victim, and a clean-IP victim. Run as node --import tsx <local-copy>.mts from a worktree on this branch.
  • Evidence after fix: live terminal output captured from the node runtime against the patched modules:
Bootstrap-token DoS rate-limit proof (PR 76322)
Imports real patched resolveConnectAuthDecision + createAuthRateLimiter
from security/bootstrap-dos-poc worktree.

maxAttempts=5  windowMs=60s  lockoutMs=60s  exemptLoopback=false
simulated mutex hold per verify call: 5ms

=== Phase 1: 20 sequential attacker attempts from a single IP ===
attempt | reason                  | rateLimited | verifier?
--------|-------------------------|-------------|--------------
    1   | bootstrap_token_invalid | false       | ENTERED
    2   | bootstrap_token_invalid | false       | ENTERED
    3   | bootstrap_token_invalid | false       | ENTERED
    4   | bootstrap_token_invalid | false       | ENTERED
    5   | bootstrap_token_invalid | false       | ENTERED
    6   | rate_limited            | true        | skipped
    7   | rate_limited            | true        | skipped
    8   | rate_limited            | true        | skipped
    9   | rate_limited            | true        | skipped
   10   | rate_limited            | true        | skipped
   11   | rate_limited            | true        | skipped
   12   | rate_limited            | true        | skipped
   13   | rate_limited            | true        | skipped
   14   | rate_limited            | true        | skipped
   15   | rate_limited            | true        | skipped
   16   | rate_limited            | true        | skipped
   17   | rate_limited            | true        | skipped
   18   | rate_limited            | true        | skipped
   19   | rate_limited            | true        | skipped
   20   | rate_limited            | true        | skipped

attacker verifier-call total: 5 of 20 attempts

=== Phase 2: legitimate victim under sustained attack ===

victim attempt 1: 5.28ms  authOk=true  method=bootstrap-token
victim attempt 2: 5.32ms  authOk=true  method=bootstrap-token
victim attempt 3: 5.24ms  authOk=true  method=bootstrap-token
victim attempt 4: 5.19ms  authOk=true  method=bootstrap-token
victim attempt 5: 5.20ms  authOk=true  method=bootstrap-token

victim average latency under attack: 5.24ms
attacker verifier entries during 500ms attack: 5
attacker call rate during attack: ~12 verifier-acquisitions/sec

=== Phase 3: IP isolation β€” victim from clean IP ===

clean victim IP: 5.26ms  authOk=true  method=bootstrap-token
  • Observed result after fix: Phase 1 β€” exactly 5 of 20 attacker attempts entered the verifier; the remaining 15 short-circuited at the rate-limit check and never acquired the verifyDeviceBootstrapToken mutex. Phase 2 β€” while the attacker IP was hammering in a tight loop for 500 ms, only 5 verifier acquisitions occurred (post-lockout the IP cannot acquire the mutex at all); the victim from a separate IP completed bootstrap auth at avg 5.24 ms per attempt (one mutex hold), not NΓ—mutex_hold as the pre-patch code would have produced. Phase 3 β€” a third clean IP authenticated normally in 5.26 ms, confirming the lockout is per-IP and does not leak across IPs.
  • What was not tested: real WebSocket transport layer (ws server upgrade and IP extraction). The handshake-decision rate-limit gate is the unit changed by this PR; transport handling is upstream of resolveConnectAuthDecision and unmodified.

Notes for reviewer

  • The withLock mutex in src/infra/device-bootstrap.ts is unchanged; the fix is purely a pre-flight rate-limit check that prevents the mutex from being acquired by IPs that have exhausted their attempt budget.
  • exemptLoopback: false was used in the reproducer because the policy default exempts 127.0.0.1/::1 so localhost CLI sessions are never locked out; the attack vector targets non-loopback callers reaching the gateway WS port.
  • This PR is the foundational PoC. The follow-up rate-limit fix lives in fix(gateway): rate-limit pre-auth bootstrap-token verify to prevent mutex DoSΒ #77527, which extends the same pattern to the bootstrap-token verify path's failure-counter side effects.

@fede-kamel fede-kamel requested a review from a team as a code owner May 3, 2026 00:03
@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime scripts Repository scripts size: L labels May 3, 2026
@clawsweeper

clawsweeper Bot commented May 3, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed May 31, 2026, 8:22 AM ET / 12:22 UTC.

Summary
The PR adds a bootstrap-token auth rate-limit scope, gates bootstrap-token verification before the mutex-backed verifier, defers failed-bootstrap accounting across device-token fallback, and expands auth-context tests.

PR surface: Source +35, Tests +255. Total +290 across 3 files.

Reproducibility: yes. for the review finding from source: an invalid bootstrap token sets pendingBootstrapFailure, a locked device-token bucket sets deviceTokenRateLimited, and the only bootstrap recordFailure calls are skipped on that path. I did not run tests because this was a read-only review.

Review metrics: 1 noteworthy metric.

  • Auth limiter scopes: 1 added. Adding a new per-IP auth bucket changes bootstrap-token lockout and fallback accounting semantics that need maintainer attention before merge.

Merge readiness
Overall: πŸ§‚ unranked krab
Proof: 🦞 diamond lobster
Patch quality: πŸ§‚ unranked krab
Result: blocked by patch quality or review findings.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Fix the rate-limited device-token fallback accounting gap and add a focused regression test.
  • Have maintainers choose this deferred-accounting branch or the companion bootstrap-token branch before landing.

Risk before merge

  • [P2] The current branch can still leave invalid bootstrap-token verifier calls uncharged when the device-token fallback bucket is already rate-limited, which weakens the intended mutex-stall DoS mitigation.
  • [P1] There is an open companion branch at fix(gateway): rate-limit pre-auth bootstrap-token verify to prevent mutex DoSΒ #77527 for the same bootstrap-token surface with different failure-accounting semantics, so maintainers should avoid landing both or drifting the policy.

Maintainer options:

  1. Fix deferred fallback accounting (recommended)
    Record the pending bootstrap failure when the device-token fallback is rate-limited, and add a regression test for that exact mixed-bucket path before merge.
  2. Choose the companion branch instead
    If maintainers prefer the immediate-accounting and WS integration shape in fix(gateway): rate-limit pre-auth bootstrap-token verify to prevent mutex DoSΒ #77527, pause or close this branch only after that PR is fixed and ready to land as the canonical implementation.
  3. Accept deferred semantics explicitly
    Maintainers can keep deferred accounting, but should do so only after the rate-limited fallback case is handled and documented in tests.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Record pending bootstrap-token failures when the device-token fallback is rate-limited, add a regression test covering invalid/stale bootstrap plus already-rate-limited device-token fallback, and preserve the existing valid device-token fallback no-charge invariant.

Next step before merge

  • The remaining blocker is a narrow auth-context accounting defect with a clear regression test, assuming maintainers keep this branch as the landing candidate.

Security
Needs attention: The diff improves bootstrap-token auth throttling but leaves a concrete mixed-bucket accounting gap in the new security boundary.

Review findings

  • [P1] Record bootstrap failures when fallback is rate-limited β€” src/gateway/server/ws-connection/auth-context.ts:289-291
Review details

Best possible solution:

Land one canonical bootstrap-token rate-limit fix that gates before verify, records invalid bootstrap attempts even when later fallback is rate-limited, preserves the no-charge invariant for a valid device-token fallback, and carries focused regression proof.

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

Yes for the review finding from source: an invalid bootstrap token sets pendingBootstrapFailure, a locked device-token bucket sets deviceTokenRateLimited, and the only bootstrap recordFailure calls are skipped on that path. I did not run tests because this was a read-only review.

Is this the best way to solve the issue?

No as submitted. The pre-verify rate-limit gate is the right shape, but the deferred failure accounting must also charge the bootstrap bucket when fallback is rate-limited, or maintainers should choose the companion branch as the canonical fix.

Full review comments:

  • [P1] Record bootstrap failures when fallback is rate-limited β€” src/gateway/server/ws-connection/auth-context.ts:289-291
    When a request has an invalid bootstrap token and a device-token candidate whose bucket is already locked, pendingBootstrapFailure is set but the !deviceTokenRateLimited branch is skipped, so the function falls through without charging the bootstrap-token bucket. An attacker can pre-lock the device-token bucket and then keep hitting the mutex-backed bootstrap verifier without consuming the new bootstrap bucket; record the pending failure on the rate-limited fallback path and add that regression test.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 2e254005a016.

Label changes

Label changes:

  • add merge-risk: 🚨 security-boundary: The diff changes pre-auth rate-limit enforcement, and the current accounting gap weakens that auth throttling boundary.
  • add merge-risk: 🚨 availability: If merged as-is, the branch can leave a path where attackers keep reaching the mutex-backed bootstrap verifier after pre-locking the device-token bucket.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal output from a Node runtime reproducer exercising the patched auth-decision and rate-limiter modules, showing verifier calls short-circuit after the attempt threshold; this does not resolve the code finding.

Label justifications:

  • P1: This is a security/availability fix for a gateway pre-auth DoS path, and the submitted branch still has a merge-blocking accounting gap.
  • merge-risk: 🚨 availability: If merged as-is, the branch can leave a path where attackers keep reaching the mutex-backed bootstrap verifier after pre-locking the device-token bucket.
  • merge-risk: 🚨 security-boundary: The diff changes pre-auth rate-limit enforcement, and the current accounting gap weakens that auth throttling boundary.
  • rating: πŸ§‚ unranked krab: Overall readiness is πŸ§‚ unranked krab; proof is 🦞 diamond lobster and patch quality is πŸ§‚ unranked krab.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The PR body includes terminal output from a Node runtime reproducer exercising the patched auth-decision and rate-limiter modules, showing verifier calls short-circuit after the attempt threshold; this does not resolve the code finding.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal output from a Node runtime reproducer exercising the patched auth-decision and rate-limiter modules, showing verifier calls short-circuit after the attempt threshold; this does not resolve the code finding.
Evidence reviewed

PR surface:

Source +35, Tests +255. Total +290 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 2 52 17 +35
Tests 1 256 1 +255
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 308 18 +290

Security concerns:

  • [high] Bootstrap limiter can be bypassed after device-token lockout β€” src/gateway/server/ws-connection/auth-context.ts:289
    Because pending bootstrap failures are only flushed inside the non-rate-limited device-token branch, a locked device-token bucket can prevent the new bootstrap bucket from recording failed bootstrap verifies.
    Confidence: 0.88

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs src/gateway/server/ws-connection/auth-context.test.ts.
  • [P1] git diff --check.

What I checked:

  • Repository policy read: Root AGENTS.md and scoped src/gateway/AGENTS.md were read fully; security/core hardening, fallback behavior, and gateway test guidance apply to this review. (AGENTS.md:1, 2e254005a016)
  • Current main has no bootstrap-token rate-limit gate: Current main calls verifyBootstrapToken directly when a bootstrap token is present, while the sibling device-token path checks AUTH_RATE_LIMIT_SCOPE_DEVICE_TOKEN first. (src/gateway/server/ws-connection/auth-context.ts:188, 2e254005a016)
  • Mutex-backed verifier is the availability-sensitive path: verifyDeviceBootstrapToken runs under the module-level withLock and reads/writes bootstrap state while checking the supplied pairing token. (src/infra/device-bootstrap.ts:418, 2e254005a016)
  • PR head still drops pending bootstrap failures on rate-limited device-token fallback: At PR head, pendingBootstrapFailure is only recorded in the no-device-token early return or inside the non-rate-limited device-token failure branch; if deviceTokenRateLimited is true, the final return is reached without charging the bootstrap bucket. (src/gateway/server/ws-connection/auth-context.ts:260, 0b3083393f1b)
  • PR proof exists for the main rate-limit behavior: The PR body includes terminal output from a Node tsx reproducer using the patched auth decision and real AuthRateLimiter, showing only the first five attacker attempts enter the verifier and later attempts short-circuit as rate_limited. (0b3083393f1b)
  • Related same-surface PR remains open: The companion bootstrap-token PR is still open and mergeable but not landed, so it is not a safe superseding close target for this branch. (69637a53b7b8)

Likely related people:

  • steipete: Recent commits touched gateway auth rate-limiter durations, auth-context, device-bootstrap, and message-handler paths central to this PR. (role: recent area contributor; confidence: high; commits: 54a27f4e5757, a1d7a7536aff, f5eca3f84cbf; files: src/gateway/auth-rate-limit.ts, src/gateway/server/ws-connection/auth-context.ts, src/infra/device-bootstrap.ts)
  • buerbaumer: The gateway auth rate-limiting feature and scoped limiter behavior appear in the history at commit 30b6ecc. (role: original rate-limiter feature contributor; confidence: high; commits: 30b6eccae5af; files: src/gateway/auth-rate-limit.ts)
  • pgondhi987: Recent merged work changed auth-context and bootstrap/device-pairing behavior around shared-auth rotation and setup-code pairing. (role: recent auth and bootstrap-path contributor; confidence: medium; commits: c923b077841c, 386d321634b3, b17e77a22bf4; files: src/gateway/server/ws-connection/auth-context.ts, src/infra/device-bootstrap.ts)
  • ngutman: History shows QR/bootstrap auth preference and bounded device-token handoff work on the same bootstrap-auth path. (role: bootstrap auth behavior contributor; confidence: medium; commits: 017bc5261c9b, 226ca1f324a4, a9140abea6d4; files: src/gateway/server/ws-connection/auth-context.ts, src/infra/device-bootstrap.ts)
  • vincentkoc: Commit af0c086 adjusted mixed-handshake shared-auth rate-limit behavior adjacent to this fallback-accounting change. (role: adjacent shared-auth rate-limit contributor; confidence: medium; commits: af0c0862f22c; files: src/gateway/server/ws-connection/auth-context.ts)
What the crustacean ranks mean
  • πŸ¦€ challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • πŸ¦ͺ silver shellfish: thin signal; proof, validation, or implementation needs work.
  • πŸ§‚ unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

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.

@fede-kamel fede-kamel force-pushed the security/bootstrap-dos-poc branch from f0e1f7d to e7377e4 Compare May 3, 2026 00:08
@fede-kamel fede-kamel changed the title fix(gateway): rate-limit bootstrap token path to prevent mutex-stall DoS security: bootstrap token path exposes mutex-stall DoS β€” no rate limit, no lockout, no alert May 3, 2026
@fede-kamel

Copy link
Copy Markdown
Contributor Author

This is ready for merge review.

All maintainer feedback addressed:

  • Deferred failure accounting β€” bootstrap recordFailure is no longer charged on a path that later succeeds via device-token. A device with a stale bootstrap token + valid device token passes cleanly with zero false positives on the rate-limit counter.
  • PoC scripts removed from scripts/ β€” the proof lives in the test suite, not in runnable attack tooling.
  • 17/17 tests passing β€” unit coverage of every scope/path combination including the cross-path invariant, plus three integration tests against a real AuthRateLimiter instance proving the lockout fires and IP isolation holds.

The DoS vector is closed: attacker connections are blocked at rateLimiter.check() before verifyDeviceBootstrapToken() is ever called, so the mutex is never acquired and legitimate pairing is unaffected regardless of flood volume.

@fede-kamel

Copy link
Copy Markdown
Contributor Author

@clawsweeper additional feedback?

@fede-kamel fede-kamel reopened this May 3, 2026
@fede-kamel

Copy link
Copy Markdown
Contributor Author

@clawsweeper additional feedback?

@fede-kamel fede-kamel force-pushed the security/bootstrap-dos-poc branch 2 times, most recently from 7498236 to fc20092 Compare May 3, 2026 04:59
@fede-kamel

Copy link
Copy Markdown
Contributor Author

/clawsweeper re-review

@fede-kamel

Copy link
Copy Markdown
Contributor Author

Friendly bump for re-review.

  • Merged latest main (HEAD now adf0cdcd3d) β€” pulls in f696be950bdd "fix(gateway): keep runtime metadata on session rows", which clears the unrelated checks-node-agentic-control-plane-agent-chat failure that was red on the prior SHA.
  • Touched-surface tests green locally: pnpm test src/gateway/server.sessions.store-rpc.test.ts src/gateway/server/ws-connection/auth-context.test.ts β†’ 18/18.
  • Scope is now strictly the bootstrap auth path (rate-limit + deferred failure accounting). The npm-trust finding that was on this branch earlier was split out and landed separately as fix(plugins): require provenance for official npm trustΒ #76501.

cc @vincentkoc β€” happy to reshape, split further, or rebase if you'd prefer a different surface; same footing as the trust-pinning rework.

@fede-kamel

Copy link
Copy Markdown
Contributor Author

@steipete when you have a moment β€” small security fix (bootstrap mutex-stall DoS PoC), CI green and currently mergeable. Happy to address feedback. Thanks!

@fede-kamel

Copy link
Copy Markdown
Contributor Author

cc @openclaw/openclaw-secops for visibility β€” this is the PoC that motivates the rate-limit fix in #77492. Mergeable. No rush. Thanks!

@fede-kamel fede-kamel force-pushed the security/bootstrap-dos-poc branch from b742e55 to f3b4859 Compare May 9, 2026 17:58
@openclaw-barnacle openclaw-barnacle Bot added size: M and removed scripts Repository scripts size: L proof: sufficient ClawSweeper judged the real behavior proof convincing. labels May 9, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 9, 2026
@fede-kamel fede-kamel force-pushed the security/bootstrap-dos-poc branch from 79f7a9f to 70178b5 Compare May 9, 2026 18:12
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 9, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 9, 2026
@jesse-merhi jesse-merhi added the security Security documentation label May 25, 2026
@clawsweeper clawsweeper Bot added rating: πŸ§‚ unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels May 25, 2026
@clawsweeper

clawsweeper Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@jesse-merhi jesse-merhi removed the security Security documentation label May 25, 2026
@clawsweeper clawsweeper Bot added the P1 High-priority user-facing bug, regression, or broken workflow. label May 25, 2026
@clawsweeper

clawsweeper Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

πŸ”₯ Warming up: real-behavior proof passed; findings, security review, or rank-up moves are still in progress.

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

  • Merged PRs are hatchable.
  • Open PRs are hatchable when they are status: πŸ‘€ ready for maintainer look, status: πŸš€ automerge armed, or labeled clawsweeper:automerge.
  • Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.
What is this egg doing here?
  • Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
  • The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
  • Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
  • The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
  • Rarity is just collectible sparkle: πŸ₯š common, 🌱 uncommon, πŸ’Ž rare, ✨ glimmer, and 🌈 legendary.

Finding 1: verifyDeviceBootstrapToken() has no rate limiting unlike the
device-token path, and serializes all calls through a module-level mutex.
An attacker with N concurrent connections can stall legitimate device
pairing proportionally with no IP lockout or alert triggered.

poc-bootstrap-dos.mjs      β€” in-process PoC (confirmed x74 stall)
poc-bootstrap-dos-real.mjs β€” full WS PoC with Ed25519-signed device
                             identities, real OpenClaw-protocol server
poc-polyfill-node18.mjs    β€” Node 18 polyfill helper for toSorted
Record bootstrap rate-limit failure only after all fallback auth paths
(device-token) have run and auth is still false. A device presenting a
stale bootstrap token alongside a valid device token is a legitimate
connection and must not accumulate bootstrap-scope failures.

Adds a test covering the cross-path case: stale bootstrap + valid
device token β†’ authOk true, bootstrap recordFailure never called.

Removes runnable PoC scripts (poc-bootstrap-dos*.mjs, .ts) ahead of
maintainer review; the rate-limiting proof lives in the test suite.
@fede-kamel fede-kamel force-pushed the security/bootstrap-dos-poc branch from 70178b5 to 0b30833 Compare May 31, 2026 12:15
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 31, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels May 31, 2026
@fede-kamel

Copy link
Copy Markdown
Contributor Author

Closing as superseded by main.

The bootstrap-token rate limiting this PR adds already landed on main in commit ecbd97e968 ("fix(gateway): rate-limit bootstrap-token verification"), with the same design β€” including the deferred-failure-accounting invariant this PR was built around:

  • src/gateway/server/ws-connection/auth-context.ts imports and uses AUTH_RATE_LIMIT_SCOPE_BOOTSTRAP_TOKEN: it checks the per-IP bucket before verifyDeviceBootstrapToken, records a failure on reject, and resets on success β€” mirroring the device-token protection.
  • The pendingBootstrapFailure flag (auth-context.ts:214/217/270) defers the bootstrap recordFailure until the whole handshake fails, so a stale bootstrap token + valid device token is not penalized β€” the exact invariant in this PR's test plan.
  • auth-context.test.ts on main carries the matching coverage.

Rebasing this branch onto current main produces an effectively empty diff, so there is nothing left to merge.

Linked issue #77980 is fixed by the same commit and is being closed alongside.

What would reopen this: a concrete bootstrap path on main that still reaches verifyDeviceBootstrapToken without a rate-limit gate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gateway Gateway runtime merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: πŸ§‚ unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: M status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: bootstrap token path lacks rate limiting, lockout, and alerting on failed verifies

2 participants