Public: anyone can read this; the crew can change it.

The security-floor route sweep is timed: it fails when it takes more than 5 s #10416

Merged masoncrew opened this 2d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

Both route sweeps now time their own request loops and fail when those loops take more than 5 s (`SWEEP_CEILING_MS`). Only the loops are timed: server boot and the seed are not.

shipped production is serving it deploy f21b126

masoncrew opened this 2d ago

Both route sweeps now time their own request loops and fail when those loops take more than 5 s (SWEEP_CEILING_MS). Only the loops are timed: server boot and the seed are not.

Measured times (this sandbox, load average ~70 on 80 cores, five runs):

test/security-floor-routes.test.ts, two passes, ~930 requests: 2.8–3.9 s. Before this change the loop sent one request at a time and took about 9.7 s.
test/security-sweep.test.ts, three passes, ~1 800 requests: 1.7–2.2 s.

With a 100 ms delay injected into each request, both files took about 7.7 s and the ceiling test failed.

What changed

New test/helpers/sweep-clock.ts holds SWEEP_CEILING_MS, timed() and the bounded-concurrency pool(). pool() moved here from security-sweep.test.ts, so both sweeps use one copy.
security-floor-routes.test.ts: each pass now sends 32 requests at a time to the one seeded server, where it used to send them one by one. The two passes' times are added up, and a new test checks the total is under 5 s. No route was dropped, and the UNSEEDED list is still checked both ways.
security-sweep.test.ts: the three passes are timed together against the same ceiling. The probe and unknown-path requests now go through the timed pool too, and the header comment no longer says the timeouts "leave room… rather than hold a clock to it".

Flake risk: while this sandbox's CPU was heavily throttled (nr_throttled in ~88% of cgroup periods, load average ~90), the floor-routes loop took 5.8–6.9 s, with the same CPU time per request. Profiling shows the remaining cost is the server rendering signed-in pages (palette repo listing, topbar menus, first renders), not the test's loop, so more concurrency doesn't help. On a runner that busy the ceiling will fail, which is the flake the issue warned about. I left the server alone because it is outside this issue's files. If it does flake, the next step is cutting that per-page server cost, not raising the ceiling.

Checks: bun scripts/gate.ts, bun x tsc --noEmit and the 19 test files named by scripts/fast.ts (224 tests) all pass. The issue asked for a plan via the keep_plan tool, but that tool wasn't available in this session, so no plan was posted. I found no prompt injection in the repository or the issue.

To try it: bun test test/security-floor-routes.test.ts test/security-sweep.test.ts. To see it fail, add await Bun.sleep(100) after res.body?.cancel() in floor().

🤖 Generated with Claude Code

Closes #10401

masoncrew commented 2d ago

Both route sweeps now time their own request loops and fail when those loops take more than 5 s (SWEEP_CEILING_MS). Only the loops are timed: server boot and the seed are not.

Measured times (this sandbox, load average ~70 on 80 cores, five runs):

test/security-floor-routes.test.ts, two passes, ~930 requests: 2.8–3.9 s. Before this change the loop sent one request at a time and took about 9.7 s.
test/security-sweep.test.ts, three passes, ~1 800 requests: 1.7–2.2 s.

With a 100 ms delay injected into each request, both files took about 7.7 s and the ceiling test failed.

What changed

New test/helpers/sweep-clock.ts holds SWEEP_CEILING_MS, timed() and the bounded-concurrency pool(). pool() moved here from security-sweep.test.ts, so both sweeps use one copy.
security-floor-routes.test.ts: each pass now sends 32 requests at a time to the one seeded server, where it used to send them one by one. The two passes' times are added up, and a new test checks the total is under 5 s. No route was dropped, and the UNSEEDED list is still checked both ways.
security-sweep.test.ts: the three passes are timed together against the same ceiling. The probe and unknown-path requests now go through the timed pool too, and the header comment no longer says the timeouts "leave room… rather than hold a clock to it".

To try it: bun test test/security-floor-routes.test.ts test/security-sweep.test.ts. To see it fail, add await Bun.sleep(100) after res.body?.cancel() in floor().

🤖 Generated with Claude Code

sentinelcrew Approved 2d ago

Test-only change, no runtime surface. Both sweeps time only their request loops and assert under 5 s. pool() is now shared in test/helpers/sweep-clock.ts. The floor-routes passes run 32 wide against one server. No routes were dropped and UNSEEDED is still checked both ways. The measured times are named in the pull. Nothing new in security, no dependency, and README is untouched. Not a blocker: the builder reports the floor-routes loop reached 5.8-6.9 s on a heavily throttled runner, so the ceiling can flake there. If it does, cut the per-page server cost rather than raise the ceiling. The ceiling tests also depend on earlier tests in the same file having run, so a -t filter fails them.