feat(fe): describe the browser a session was created from - #4270
Merged
Conversation
sea-snake
marked this pull request as ready for review
August 22, 2026 18:16
|
✅ No security or compliance issues detected. Reviewed everything up to c83b3e0. Security Overview
Detected Code Changes
|
sea-snake
force-pushed
the
fe/describe-browser
branch
from
August 22, 2026 18:45
f06c233 to
67d0542
Compare
sea-snake
force-pushed
the
fe/describe-browser
branch
2 times, most recently
from
August 22, 2026 19:25
b1a37d6 to
4d9c5d4
Compare
The settings list shows the user which browsers hold a session, so each one needs a label they will recognise. Derived from the user agent, most-specific token first, because every later token appears inside the earlier ones' agents. It names the device where a device word exists — `Safari on iPhone`, `Chrome on Mac`, `Chrome on Chromebook` — and takes the platform's own device model where there is one, so an Android entry reads `Chrome on Pixel 5`. The label is coarse by construction: several distinct browsers report the same string, so `Chrome on Mac` twice is normal. That is why the settings list identifies an entry by its id rather than by this name, and why the canister treats it as a label rather than as evidence. Nothing imports it yet; the sign-in that sends it lands three PRs up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sea-snake
force-pushed
the
fe/describe-browser
branch
from
August 22, 2026 19:49
4d9c5d4 to
2179a85
Compare
MRmarioruci
approved these changes
Sep 2, 2026
MRmarioruci
reviewed
Sep 2, 2026
sea-snake
removed this pull request from stack #4277
September 9, 2026 20:32
sea-snake
added this pull request to stack #4324
September 9, 2026 20:34
sea-snake
removed this pull request from stack #4324
September 9, 2026 20:35
sea-snake
added this pull request to stack #4323
September 9, 2026 20:38
sea-snake
removed this pull request from stack #4323
September 9, 2026 20:42
sea-snake
added this pull request to stack #4326
September 9, 2026 20:43
MRmarioruci
reviewed
Sep 10, 2026
MRmarioruci
reviewed
Sep 10, 2026
MRmarioruci
reviewed
Sep 10, 2026
…lives `describeBrowser` parses a user agent and returns a `BrowserDescription`. Its neighbours in `channelHandlers/` all import `Channel` and handle a JSON-RPC method; it does neither, and one of its two callers is the devices page. It belongs in `lib/utils`. `Vivaldi` and `DuckDuckGo` had hardcoded rows because both carry `Chrome/` and would otherwise be labelled Chrome — two arbitrary picks from a long list. Worse, an unrecognised browser fell back to the whole user agent as its *name*, capped at 64 bytes. Both go: a browser is named by the product token it appends, which covers Vivaldi, Yandex and anything else that self-identifies under one rule. Brave and Arc ship Chrome's agent unchanged and still read as Chrome, which is what they are asking for. Versions are dropped from the name. The canister fixes a description at registration, so one captured here would sit frozen at whichever build first signed in. The operating system had the same fallback, which is how a whole agent could end up in the field. It takes the platform out of the parenthesised block instead. The tests are rebuilt around what is under test — brand, system, form factor, client hints, limits — instead of one six-wide positional tuple, and now cover a fork that names itself, one that hides, and a platform none of the seven names. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
sea-snake
removed this pull request from stack #4326
September 10, 2026 12:55
sea-snake
added this pull request to stack #4328
September 10, 2026 12:55
MRmarioruci
approved these changes
Sep 10, 2026
A device stating Watch, XR, Automotive or EInk is none of desktop, mobile or tablet. Falling through to `mobile` called a watch a phone and an e-reader a desktop, so those four resolve to Unknown instead, after the Tablet check so a device stating both keeps the variant that exists. The singular `formFactor` fallback goes with it: only `formFactors` is ever requested, so the singular could never arrive. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Brave and singular form-factor hints are unsupported, and Unicode truncation can corrupt tokens.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds browser metadata resolution for future session/device management.
Changes:
- Resolves browser, OS, form factor, and hardware model.
- Caps stored tokens and adds UA/client-hint tests.
File summaries
| File | Description |
|---|---|
describeBrowser.ts |
Implements browser description resolution. |
describeBrowser.test.ts |
Tests user agents, hints, and limits. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVi99RYo2jyi2kCurgovNJ
sea-snake
added a commit
that referenced
this pull request
Sep 10, 2026
Design: #4224. Overview: #4230. Nothing imports this until the sign-in in #4273. The canister identifies a browser by a key it proves possession of (#4264), so the frontend has to hold one, and rotate it, so a key captured from one sign-in cannot claim that browser at the next. A non-extractable P-256 keypair in IndexedDB, per identity, plus a successor generated alongside it. A sign-in signs with the current key and announces the successor; the successor is promoted only once the canister has confirmed the sign-in. **Promoting after confirmation rather than before is what survives a lost response.** If the reply never arrives, the browser still holds the key the canister has, so its next attempt presents the same one rather than a successor the canister never saw — which the canister would treat as an unknown browser and enrol as a second entry for the same machine. **Sign-ins are serialised with a web lock**, because two at once would leave whichever wrote last holding a key the canister never accepted. Where the Web Locks API is missing the calls run unserialised: not blocking sign-in on it is the deliberate trade, and the window is one concurrent sign-in on the same device. **A browser that no longer matches what it registered as signs in as a new one.** The description it reported (#4270) is stored beside the key pair, and compared before anything is sent. A registered entry keeps the description it was created with — the canister ignores what a sign-in reports once an entry is being advanced — so a browser whose brand, system, form factor or model has changed would otherwise keep rotating an entry describing something it no longer is. Where it differs, this presents a fresh key pair, which no entry holds and which therefore registers under its own. Deciding here rather than being told keeps that a purely local operation: nothing has been sent when the comparison happens, so the entry left behind is untouched and a retry hits the same state and takes the same branch. The description is written with the key pair and never on its own, so the comparison is always against what the canister was actually sent. The private key never leaves IndexedDB and never appears in a delegation chain. Tests: `browser-key.store.test.ts` (21), including the successor promoted only after the call succeeds, a lost response leaving the current key in place, concurrent sign-ins serialised, the store working with the lock API absent, and a changed description signing in on a fresh key pair while an unchanged one keeps rotating. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
sea-snake
added a commit
that referenced
this pull request
Sep 10, 2026
…4273) Design: #4224. Overview: #4230. Depends on #4272 for the record store, #4271 for the browser key, and #4270 for the browser description — this is what gives all three their production caller. The canister side is complete after #4265, but nothing hands an app a session yet. This is the JSON-RPC method that does. **`ii_session_delegation`** is what an app calls instead of `icrc34_delegation` when it wants a session rather than a long-lived delegation. It answers with the session chain, extended to the app's own key, and the app mints five-minute delegations from it with no further browser involvement. The `ii_` prefix is deliberate: an RPC method travels over a transport shared with other providers and has to say which one it belongs to, which is why every II-specific RPC method carries it. **`targets` on the chain is restricted to the II canister.** That is a developer guardrail, not a defence against a thief — a thief holding the chain can refresh with it either way. Revocability is the protection that matters, and saying otherwise in a security table would be dishonest. **The consent duration is honoured**, and an SSO organization's cap still binds it: `valid_for` carries the lifetime the user chose, and an SSO identity sends a duration even when the user picked none, matching the ICRC-34 path. **The request carries only a session public key and an optional derivation origin.** An app cannot ask for an access level or a lifetime, because both are the user's to decide at consent. `icrc25_permissions` now reports `ii_session_delegation` as a granted scope so an app can discover the method. `icrc34_delegation` itself is unchanged. Tests: `sessionDelegation.test.ts` (5) covers a request for another method being ignored and params carrying no session key being rejected. The success path has no unit test here, which is worth knowing rather than glossing: the harness does not stub the authentication store, so `createSession` never completes in it, and neither this path's SSO clamp nor the ICRC-34 one is covered. #4248 stubs enough to drive the handler to an answer and exercises the `targets` restriction and the held-session path. Review `createSession` by reading it. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Design: #4224. Overview: #4230. Nothing imports this until the sign-in in #4273.
Settings shows which browsers hold a session so the user can end one they do not recognise, and each entry needs to be something they will recognise. The client is the only party that can see what it is running on, so it is the one that has to say.
describeBrowserresolves the four things the record holds (#4242) — a brand, an operating system, a form factor, and the hardware model where one is reported — rather than composing a label out of them.Resolving rather than naming is the point. A composed label fixes the wording at the moment of sign-in: a browser that registered before "Chrome OS" became "ChromeOS" would read the old name for as long as its record existed, and the agent it was parsed from is not kept, so nothing could re-derive it. With tokens the wording lives where the list is read (#4249), so renaming a product — or teaching this table to recognise a browser it used to miss — reaches every stored record instead of only the browsers that sign in afterwards.
Two sources, because neither covers the field alone. The user agent carries the brand and the system on every engine, and its table stays ordered most specific first, because every later token appears inside the earlier ones' agents: matching
Chromefirst would claim every Chromium-based browser, and every Chromium agent carriesSafari/as well. Client hints are Chromium-only — Safari and Firefox expose nothing at all — so they supply only what no agent can: the hardware model, which a reduced Android agent reports as the literal stringK, and a form factor the browser states rather than one inferred. Only the pluralformFactorsis asked for; the singular shape Chromium shipped first is not, because only current browsers are a concern. A statedWatch,XR,AutomotiveorEInkresolves toUnknown: the canister names desktops, mobiles and tablets, and calling a watch a phone is worse than naming it as none of them.A browser that hides reads as the one it imitates. Brave ships a plain Chrome agent on purpose and strips the hints that would give it away, so it reads as Chrome — which is what the agent says, and the honest answer rather than a gap. Asking
navigator.brave.isBrave()would name it, at the cost of a check per browser this frontend has heard of, which is the hardcoded list this table exists to avoid. A browser that does name itself in its agent, Vivaldi or DuckDuckGo, travels as the token it gave rather than resolving to Chrome, without needing a row of its own.An unrecognised browser or system is carried as the token that arrived, capped to what the canister accepts, because that row is the one this list exists to surface.
maxTouchPointsis consumed here rather than stored: an iPad in desktop mode sends a Mac agent and exposes no hints, so it is the only thing that tells it from a Mac.Tests:
describeBrowser.test.ts(37) — the 20 real user-agent strings, one per rung of both ladders, including the iPad and Mac pair separated only bymaxTouchPoints; the model used when present and absent when empty or refused; a stated form factor, the four that resolve toUnknown, and a tablet stated beside one of them; a browser that hides itself reading as the one it imitates; and an unrecognised token capped to 64 bytes.