fix(security): stop the rate-limit floor panicking actix's router #420

Merged

David merged 3 commits from fix/BUNYIP-426-audit-rollup-2026-07-30 into main

2026-08-01 00:49:11 +02:00

Claude-Run commented

2026-07-31 17:23:57 +02:00

Member

Runtime verification against a local stack found the F7 middleware cloning the inner HttpRequest and holding it across service.call. Actix's router then calls HttpRequest::match_info_mut, which unwraps Rc::get_mut and asserts it holds the only reference, so every non-exempt request panicked its worker and the client saw a dropped connection instead of a response. Borrow req.request() for the duration of the subject resolution instead, and capture the path as a String for the throttled-request log line.

A unit test greps the module for the clone so the shape cannot come back.

#BUNYIP-426

Runtime verification against a local stack found the F7 middleware cloning the inner `HttpRequest` and holding it across `service.call`. Actix's router then calls `HttpRequest::match_info_mut`, which unwraps `Rc::get_mut` and asserts it holds the only reference, so every non-exempt request panicked its worker and the client saw a dropped connection instead of a response. Borrow `req.request()` for the duration of the subject resolution instead, and capture the path as a `String` for the throttled-request log line. A unit test greps the module for the clone so the shape cannot come back. #BUNYIP-426

Claude-Run added 2 commits

2026-07-31 17:23:57 +02:00

fix(security): close the 2026-07-30 audit sweep findings fbfb0b837e

Implements the seven deferred findings from the BUN-SEC-2026-07-30 sweep (F4 through F10). Each is independent; they land together because the roll-up tracks them as one checklist.

F4 - the Secure cookie attribute now derives from the transport, not from ENVIRONMENT. `Config::cookies_secure(&req)` returns true in production, on a TLS listener, or when a trusted-proxy peer sends `X-Forwarded-Proto: https`, reusing the same trusted-proxy gate as `extract_client_ip` so the header cannot be forged by a direct client. All 18 set-cookie sites use it. The dev-sso stack is publicly reachable over real TLS with `ENVIRONMENT=development` and `COOKIE_DOMAIN=.a8n.run`, so every session cookie there previously shipped without Secure and was sent in cleartext to any sibling `http://*.a8n.run` name. Plain-HTTP `just dev` on localhost still gets `secure(false)`.

F5 - `provision_app_role` no longer interpolates the statement text into its error. Two of the eight statements carry `BUNYIP_APP_PASSWORD` as an SQL literal, and the error is logged at `error!` on any provisioning failure, so an ordinary operational hiccup wrote the `bunyip_app` credential to the container log. The statements now carry a stable label and the error names the label only.

F6 - all four dunite crates are pinned by `rev` instead of `branch = "main"`, and both release recipes name the workspace members explicitly instead of running `cargo update --workspace`. dunite supplies bunyip's security kernel (Argon2id, AES-256-GCM, JWT config, the OIDC key set), and every `just create-release` used to roll it forward to whatever `main` pointed at, with the only trace a changed hash in Cargo.lock.

F7 - `RateLimitFloor` applies the previously dead `API_UNAUTH` and `API_AUTH` presets underneath every bunyip-api route, so an endpoint added without its own `check_rate_limit` is capped by default rather than uncapped by default. `POST /v1/auth/refresh` and `GET /v1/auth/password-reset/verify` were the concrete gaps. Health probes, the version endpoints, and the HMAC-authenticated Stripe webhook are exempt; the check fails open so a rate-limit table outage cannot take the API down. The registration cap also applies in every environment now, with a looser non-production budget so the e2e suite's serial runs from one CI egress IP do not trip it.

F8 - `POST /v1/billing/setup-intent` no longer varies its response by whether the email is registered, and now requires the signup challenge token the register form already carries. The old `409`-versus-`200` split was a clean user-enumeration oracle, and every negative probe created a real Customer and SetupIntent on the production Stripe account.

F9 - the magic-link, email-verification, and password-reset consumes are guarded claims whose `rows_affected` decides the race, matching `claim_login_approval_code`. Each call site claims the token before any side-effecting work, so two concurrent magic-link verifies of a first-time email can no longer both reach `UserRepository::create` and collide on the unique email index.

F10 - every external `FROM` in both production Dockerfiles carries an immutable digest alongside its tag, and the api runtime moves from alpine 3.21 to the alpine 3.23 that compose.yml already pins for postgres. The compose hardening trio the report mentions in passing (`no-new-privileges`, `cap_drop`, `read_only`) is deliberately not included: it is already tracked in BUNYIP-205, and `cap_drop: [ALL]` on the postgres service needs runtime validation this change cannot provide.

Removals that no compiler would catch are gated by `scripts/check-security-invariants.sh`, wired into `just check` and the CI check workflow: the ENVIRONMENT-derived cookie flag, the dunite branch pin, the setup-intent email lookup, and an undigested base image each fail the build if reintroduced. F5 and F9 are gated by unit tests instead, since both shapes are expressible in Rust.

#BUNYIP-426

fix(security): stop the rate-limit floor panicking actix's router

E2E / Playwright against deployment (pull_request) Successful in 1m38s

Details

Check / fmt + clippy + build + tests (pull_request) Successful in 31m9s

Details

fa40423cee

Runtime verification against a local stack found the F7 middleware cloning the inner `HttpRequest` and holding it across `service.call`. Actix's router then calls `HttpRequest::match_info_mut`, which unwraps `Rc::get_mut` and asserts it holds the only reference, so every non-exempt request panicked its worker and the client saw a dropped connection instead of a response. Borrow `req.request()` for the duration of the subject resolution instead, and capture the path as a `String` for the throttled-request log line.

A unit test greps the module for the clone so the shape cannot come back.

#BUNYIP-426

David added 1 commit

2026-08-01 00:35:45 +02:00

Merge main into fix/BUNYIP-426-audit-rollup-2026-07-30

E2E PR gate / Install + reachability (no deployment secrets) (pull_request) Successful in 25s

Details

Check / fmt + clippy + build + tests (pull_request) Successful in 13m25s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

1b4728a00a

Three conflicts, all additive: the CI job and the `just check` chain now run both `check-workflow-secrets.sh` (BUNYIP-425, from main) and `check-security-invariants.sh` (BUNYIP-426), and `middleware/mod.rs` re-exports both `resolve_rate_limit_subject` (the floor's subject resolver) and main's `super_admin_allowed` / `SuperAdminUser`.

No semantic conflict between the floor and main's admin rate-limit management: `RateLimitRepository::check_and_increment` / `get_retry_after` resolve the effective config through `RateLimitConfigRepository::effective` internally, so the floor picks up a persisted override for `api_auth` / `api_unauth` without a call-site change, and both presets were already in `RateLimitConfig::ALL` for the admin read path.

#BUNYIP-426

David scheduled this pull request to auto merge when all checks succeed

2026-08-01 00:37:25 +02:00