fix: type-discriminate progress toast ids, and a CSPRNG fallback for newAttemptId

[cliffhall]

Closes #2216

Both findings from the v2.5.0 milestone-merge review (#2215), landed here rather than in that merge PR because its tree must stay byte-identical to origin/v2/main — the same handling #2000 → #2092 got after the v2.2.0 merge.

1. progressToastId collided across distinct progress streams

ProgressToken is string | number, so progress-${String(token ?? "default")} erased the type: the numeric token 7 and the string token "7" produced one id. Notifications keyed by the same id are replaced rather than stacked — that is the point of the id — so two concurrent progress streams overwrote each other's toast: one stream's ticks silently retitled the other's, and the first to finish took the survivor with it on auto-close. The absent case was worse still: it hardcoded the sentinel "default", which a server is free to send as a genuine string token.

The id now carries the token's type, and the no-token case gets a prefix of its own:

input	before	after
7	progress-7	progress-n:7
"7"	progress-7 ⚠️	progress-s:7
undefined	progress-default	progress-none
"default"	progress-default ⚠️	progress-s:default

progress-none is unreachable from any token value, since every present token's id carries a : after its n/s discriminator.

2. newAttemptId's fallback now prefers crypto.getRandomValues

randomUUID needs a secure context; getRandomValues does not, and exists in every browser that has crypto at all. So the exact situation the fallback existed for — a file:// page, a plain-HTTP non-loopback host — still had a CSPRNG on hand and we declined to use it. It now emits 16 hex-encoded random bytes there, keeping the same opaque-string id shape.

The call is written as a literal globalThis.crypto.getRandomValues(...) rather than through a bound alias, because CodeQL's js/insecure-randomness browser model recognizes a secure RNG by that method call shape.

Math.random survives as the last resort for a global with no crypto object at all, and the "never a security token" doc comment stays — it is why this is Medium rather than urgent. A counter-based last resort was considered and rejected: the id must be unique across page loads (it is written to sessionStorage and matched by a callback in a fresh document), so a per-page counter would collide by construction and undo #2165.

On CodeQL alert 72: this removes the reachable insecure path — the situation the alert was raised about (randomUUID unavailable on a plain-HTTP host) now takes a CSPRNG. Whether the query then closes the alert is not something this PR can assert, since Math.random remains a source in the same function. If it survives, it should be dismissed as a documented non-security use rather than argued down.

Not addressed, deliberately: CodeQL alert 73 (js/missing-rate-limiting on the test server's /oauth/revoke). Per the issue, the test servers are local single-user fixtures driven on loopback; rate-limiting them would slow and flake several smokes and defend nothing. That one gets dismissed on the alert.

Tests

• progressToasts.test.ts: the updated id expectations, plus a 7 vs "7" collision case, an absent-token case sweeping the values a server could send to impersonate the sentinel ("default", "none", "", "0", 0), and a distinctness sweep asserting seven distinct tokens yield seven distinct ids.
• oauthResume.test.ts: the single fallback test split into two arms — a getRandomValues arm asserting the CSPRNG is called, that Math.random is not, and that the id is 32 hex chars; and a no-crypto-at-all arm asserting the Math.random last resort. The crypto-member hiding is factored into a withoutCryptoMember helper (both members are inherited from Crypto.prototype, so the restore has to delete an own property rather than put a descriptor back). A vi.restoreAllMocks() was added to the file's afterEach — this project does not set restoreMocks globally, so the new spies would otherwise leak.

No UI change, so no screenshots. npm run local:gate green.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uw1s4LRBUzrFwLzPT4mAJn

[Copilot]

🟡 Changes recommended

The OAuth fallback implementation and no-crypto test have unresolved moderate issues.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Fixes progress-toast ID collisions and improves OAuth attempt-ID randomness.

Changes:

• Type-discriminates progress-token toast IDs.
• Adds a crypto.getRandomValues fallback.
• Expands regression coverage.

File summaries

File	Review
clients/web/src/utils/toasts/progressToasts.ts	Generates collision-resistant toast IDs. No issues found.
clients/web/src/utils/toasts/progressToasts.test.ts	Covers token-type and sentinel collisions. No issues found.
clients/web/src/lib/oauthResume.ts	Adds CSPRNG fallback. Moderate (1 vote): Remaining Math.random() may prevent retiring CodeQL alert 72. Moderate (1 vote): Calling a bound getRandomValues alias may not satisfy CodeQL; use a direct guarded call.
clients/web/src/lib/oauthResume.test.ts	Tests randomness fallbacks. Moderate (2 votes): The no-crypto test only hides methods; stub globalThis.crypto as undefined to test the actual absence path.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 3
• Review effort level: Balanced

[Copilot]

🟢 Approval recommended

All reviewed changes are covered by focused tests, with no unresolved issues.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟢 Approval recommended

All reviewed changes are covered by focused tests, with no unresolved issues.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced