From f67a880a1aa784f5097aa1f7b1514d630323e918 Mon Sep 17 00:00:00 2001 From: Dragan Spiridonov Date: Thu, 23 Jul 2026 09:20:31 +0200 Subject: [PATCH] docs: correct three doc-vs-code contradictions found by adversarial review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All three are my own drift — the exact failure this session criticised in other repos' ADRs and then reproduced. 1. ADR-271 §2 still listed "`iss` matches the configured issuer verbatim" in the accept-rule. That rule was removed from the code in 12635a85 because Cognitum issues no `iss` and it rejected every real token. The ADR's own "Facts about the tokens" section 30 lines above already said tokens carry no `iss` — the document contradicted itself for a day. Also removed a claim that tests cover "issuer mismatch including a trailing-slash-only difference"; that test was deleted with the rule and no longer exists. 2. `ruview-auth/src/lib.rs:59` said "**No login flow.** ... this crate only verifies" — 15 lines above `pub mod login;`. Now states the accurate thing: the login flow is behind the non-default `login` feature, so a verifying server never compiles it. 3. ADR-271's scope table still described the old prefix-denylist admin set. Replaced with the fail-closed rule that actually ships, including why: `/api/v1/adaptive/train` was reachable with `sensing:read`. Also records a KNOWN INCOMPLETE that the ADRs previously implied was done: the browser cannot obtain an OAuth token at all. `wifi-densepose login` writes ~/.ruview/credentials.json, which a browser cannot read; the UI reads localStorage['ruview-api-token'], populated only by the manual-paste QuickSettings panel. `grep -ril "oauth|cognitum|pkce" ui/` returns nothing. So the WebSocket ticket mechanism ADR-272 introduces "for browsers" is today only exercisable with the legacy static shared secret OAuth was meant to replace. Server-side gating is correct and complete; the browser half of the story these ADRs tell is not built. Recorded rather than left implied. Co-Authored-By: Ruflo & AQE --- .../ADR-271-cognitum-oauth-resource-server.md | 41 +++++++++++++++++-- v2/crates/ruview-auth/src/lib.rs | 6 ++- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/docs/adr/ADR-271-cognitum-oauth-resource-server.md b/docs/adr/ADR-271-cognitum-oauth-resource-server.md index a243fdf8..a89ec047 100644 --- a/docs/adr/ADR-271-cognitum-oauth-resource-server.md +++ b/docs/adr/ADR-271-cognitum-oauth-resource-server.md @@ -81,10 +81,16 @@ accept explicitly: typ == "access" AND NOT setup AND NOT workload AND account_id is a non-empty string AND exp is in the future -AND iss matches the configured issuer verbatim AND the scope required by the route is held ``` +**Note there is no `iss` check.** An earlier revision of this section listed +"`iss` matches the configured issuer verbatim" — that rule was implemented, +shipped, and rejected EVERY real token, because Cognitum access tokens carry no +`iss` claim (see §"Facts about the tokens" above, which contradicted this +paragraph for a day). Removed in the code; removed here. The JWKS is the issuer +binding. + Divergence from `oauthBearer.ts` would be a bug rather than a preference: a token meta-llm rejects must not be one RuView accepts. The algorithm is **fixed to ES256 by our code** — the header's `alg` is only ever compared against that @@ -114,7 +120,17 @@ RuView registers two scopes (dashboard ADR-060, identity migration `0016`): | Scope | Grants | |---|---| | `sensing:read` | sensing/pose streams, one-shot inference, reading model and recording metadata | -| `sensing:admin` | `POST /api/v1/train/*`, `DELETE /api/v1/models/{id}`, `DELETE /api/v1/recording/{id}` | +| `sensing:admin` | every mutating route not explicitly allowlisted as read-safe — training (`/api/v1/train/*` AND `/api/v1/adaptive/train`), model and recording deletion, config writes | + +**The gate is fail-closed for writes, and that polarity is load-bearing.** An +earlier revision enumerated admin routes by prefix and let everything else fall +through to `sensing:read`. `POST /api/v1/adaptive/train` — which trains a +classifier, overwrites the on-disk model and swaps the live one — does not match +`/api/v1/train/`, so it was reachable with `sensing:read`, the scope +`wifi-densepose login` requests by default. Found by adversarial review. Now: +reads are open, writes require admin unless the exact path is on a short +allowlist of non-destructive mutations. A route added tomorrow is admin-gated +until someone classifies it. **No hierarchy**: `sensing:admin` does not imply `sensing:read`. Consent means exactly what it said, and a token needing both must have consented to both. @@ -154,6 +170,24 @@ take no HTTP dependency at all. - A new dependency, `jsonwebtoken` — the same crate, same major version, that identity itself uses to sign these tokens. +## Known incomplete: the browser cannot obtain an OAuth token + +`wifi-densepose login` writes to `~/.ruview/credentials.json` — a file a browser +cannot read. The UI's `ws-ticket.js` reads a bearer from +`localStorage['ruview-api-token']`, which is populated **only** by the +QuickSettings manual-paste panel. There is no "Sign in with Cognitum" control, +no redirect flow, and `grep -ril "oauth|cognitum|pkce" ui/` returns nothing. + +So a user who signs in via the CLI gets **no benefit in the browser UI**, and +the WebSocket ticket mechanism this ADR's sibling (ADR-272) introduces "for +browsers" is today only exercisable with the legacy static shared secret that +OAuth was meant to replace. The server-side gating is correct and complete; the +browser half of the story these ADRs tell is not built. + +Deliberately recorded rather than left implied, because the ADRs read as though +the browser path exists. Closing it needs a UI sign-in flow that puts an OAuth +access token where the page can reach it — a separate piece of work. + ## Alternatives considered **Keep `RUVIEW_API_TOKEN` only.** Zero work, and adequate for a single-user @@ -192,8 +226,7 @@ caller and its scopes). canonical gate) and default features. The matrix signs real ES256 tokens with a runtime-generated key — no key material is committed — and covers `alg:none`, forged signatures, spliced payloads, unknown `kid`, expiry on both sides of the -leeway, issuer mismatch including a trailing-slash-only difference, `typ` -confusion, `setup`/`workload` smuggled onto a `typ=access` token, missing and +leeway, `typ` confusion, `setup`/`workload` smuggled onto a `typ=access` token, missing and empty `account_id`, and scope escalation. The load-bearing case is diff --git a/v2/crates/ruview-auth/src/lib.rs b/v2/crates/ruview-auth/src/lib.rs index 0bbdbb0c..d31a09f3 100644 --- a/v2/crates/ruview-auth/src/lib.rs +++ b/v2/crates/ruview-auth/src/lib.rs @@ -56,8 +56,10 @@ //! //! ## What this crate deliberately does not do //! -//! - **No login flow.** Obtaining a token (PKCE, loopback, OOB paste) is the -//! client's job and lives elsewhere; this crate only verifies. +//! - **No login flow by default.** Obtaining a token (PKCE, loopback, OOB +//! paste) lives behind the non-default `login` feature, so a server that only +//! verifies never compiles it. See [`login`] and ADR-271's 2026-07-22 +//! amendment for why it lives here rather than in a second crate. //! - **No revocation check.** There is no introspection endpoint. The 15-minute //! token lifetime *is* the revocation window, which is precisely why //! long-lived setup/workload credentials are refused outright.