mirror of
https://github.com/ruvnet/RuView
synced 2026-08-10 20:31:42 +00:00
739d3219e6f9e9be6378b562309783d371cfd4b6
3 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Dragan Spiridonov
|
b09625ece7 |
fix(auth): close the four residual findings from the adversarial review
(a) Test fixture still invented an `iss` claim.
`bearer_auth.rs`'s `token_with_scope` added `"iss": ISSUER` to its tokens.
Harmless today because the verifier ignores `iss` — but it is the same
fixture-invents-reality pattern that hid the original `iss` bug for a day,
sitting in the second-largest auth test suite. Removed, with a pointer to
ruview-auth's regression test.
(b) Cross-process refresh race -> session revocation.
The single-flight guarantee was per-PROCESS. Every CLI invocation is a new
process with its own Session and mutex over one shared credential file, so
two commands run close together inside the 60s refresh window would each
present the same rotating refresh token — and the second is replay, which
identity answers by revoking the whole session family. The user gets logged
out for running two commands at once.
Now guarded by an advisory file lock, taken NON-BLOCKING. A busy lock means
another process is already refreshing, so we wait and re-read its result
rather than race it (20 x 150ms, then proceed anyway — the lock is advisory,
not a correctness barrier, and a dead holder must not wedge us). Blocking on
the lock would have parked the async executor, which is the exact mistake
just fixed in jwks.rs.
Unix only. On other platforms it is a documented no-op — a lock that does
nothing while claiming to protect is worse than none.
(c) One principal could exhaust the global ticket pool.
The 512 cap was global with no per-caller quota, so a single authenticated
`sensing:read` client looping on POST /api/v1/ws-ticket could hold every
slot for 30s and 503 everyone else — denial of service by the
lowest-privilege account the product issues. Added a 16-ticket
per-principal cap; a page needs a handful. Test asserts a noisy user hits
its own cap while a second user is still served and the global pool is
never exhausted.
(d) Debug impls printed live credentials.
`AuthState` derived Debug over the raw RUVIEW_API_TOKEN and
`StoredCredentials` over both OAuth tokens. Not leaking today — I checked
every call site — but this PR had already hand-written redacting Debug for
`OAuthState` and `TicketStore` for exactly this reason, and the two types
actually holding secrets were the ones that missed out. Both now redact.
Tests: 87 ruview-auth (2 new: lock exclusivity + non-blocking, Debug
redaction), 534 sensing-server (1 new: per-principal quota).
Co-Authored-By: Ruflo & AQE
|
||
|
Dragan Spiridonov
|
6300b1cbd2 |
feat(ws): gate WebSocket upgrades behind bearer-or-ticket (ADR-272)
Closes the hole measured in
|
||
|
Dragan Spiridonov
|
7d6d66941a |
feat(ws): single-use WebSocket ticket store (ADR-272 groundwork)
Audit finding this exists to close — verified empirically, not inferred. With RUVIEW_API_TOKEN set (operator believes auth is ON), a real WebSocket handshake carrying NO credential: /ws/sensing -> 101 Switching Protocols /ws/introspection -> 101 Switching Protocols /api/v1/stream/pose -> 101 Switching Protocols /api/v1/models -> 401 (control: REST is correctly gated) The REST control plane is locked while the DATA plane — live presence, pose and vitals — is open to anyone who can reach the port. Two of those paths sit outside PROTECTED_PREFIX entirely; the third is the documented EXEMPT_PATHS entry. The exemption was reasonable when added (a browser cannot set Authorization on an upgrade) but its blast radius is larger than it looks, and phase 3 sharpened the contrast by making REST genuinely strong. (To be precise about what was proven: the handshake is accepted. A payload frame was not captured in that window, so this is "the connection is established without a credential", not "data was read".) This commit adds only the store; wiring it into the middleware is a breaking change for browser clients and lands with the UI update. Design notes: - Single use. `consume` REMOVES the entry, so a replay of the same URL fails even inside the TTL. This is what makes a credential-in-a-query tolerable: by the time it reaches an access log or a Referer header, it is spent. - 30-second TTL. Long enough for a page to open a socket; too short to harvest. - It is not the credential. It authorizes one WebSocket. It cannot be replayed against /api/v1/*, cannot be refreshed, and carries no reusable identity. - The grant captures the ISSUING principal's scopes, so a WebSocket inherits exactly the authority of the credential that asked for it — a sensing:read session cannot mint a ticket that outranks itself. - Capped at 512 outstanding, self-healing as tickets expire, so an authenticated but misbehaving caller cannot grow the map without bound. - In-memory because that is correct, not merely convenient: a ticket surviving a restart would outlive the server that vouched for it. Native clients (Python, Rust CLI, TS MCP) are NOT browsers and will send a normal Authorization header on the upgrade instead — tickets would add a round-trip and a second credential path for no benefit. 12 tests: single-use enforced, replay refused, expiry refused AND pruned, unknown ticket refused, 256-bit unpredictability, grant carries issuer scopes, cap enforced and self-healing, and query parsing including `?myticket=x` not being read as `?ticket=x`. Co-Authored-By: Ruflo & AQE |