spec: browser extension workstream - spike record + production scoping - #2243
spec: browser extension workstream - spike record + production scoping#2243remicolin wants to merge 13 commits into
Conversation
…g (PRODUCTION.md, UX.md) Docs-only. Spike record (SPEC.md, CE-01 protocol plan) plus production scoping: Enterprise-only RP integration via the hosted verification page, Chrome Web Store CI publishing, custody hardening, versioning/cross-compat matrix, Euclid gap analysis. Mirrored in Linear project 'Browser Extension v1 (Production)' (SELF-3690..3701); repo spec is canonical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesChrome Extension Workstream
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…deliberately per CEP ticket Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ebaadefd-9c7b-48fb-8ed6-0518397b755f
📒 Files selected for processing (5)
specs/projects/sdk/INDEX.mdspecs/projects/sdk/workstreams/chrome-extension/PRODUCTION.mdspecs/projects/sdk/workstreams/chrome-extension/SPEC.mdspecs/projects/sdk/workstreams/chrome-extension/UX.mdspecs/projects/sdk/workstreams/chrome-extension/plans/CE-01-transfer-protocol.md
| 1. Trigger: push to `main` touching `packages/chrome-extension/**`, `packages/webview-app/**`, `packages/webview-bridge/**`, or `packages/mobile-sdk-alpha/**` (same paths-filter pattern as self-infra `staging.yml`). | ||
| 2. Build chain: common -> sdk -> bridge -> webview-app -> extension (the exact chain the harnesses use today). |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Trigger releases for every artifact-changing dependency.
The build chain includes common -> sdk, but the path filter only watches the extension, webview-app, bridge, and mobile SDK paths. Changes in common or sdk can alter the published artifact without running this pipeline, leaving production fixes unpublished.
| | Relayer event vocabulary | Additive-only. Never repurpose an existing event or change payload shapes; old extensions must keep working against a newer relayer (PR #166 follows this: new emissions only) | | ||
| | TEE proving protocol | Owned by the SDK; the extension embeds `mobile-sdk-alpha` at build time, so it tracks whatever the SDK supports. Server-side proving changes must tolerate one store-review window of stale clients | | ||
| | Hosted page <-> extension shim | postMessage contract carries a protocol version; the hosted page feature-detects and falls back to QR for absent/stale extensions - fallback IS the compatibility story | | ||
| | Bridge (webview-app <-> bridge-host) | Not an external interface: both sides are built from the same commit into one artifact; no cross-version support needed | | ||
| | Store artifact | Manifest version `YYYY.MM.DD.N`, monotonic, never reused; last-N zips retained for rollback | | ||
|
|
||
| Compatibility gate in CI (CEP-02): the harness suite runs the NEW extension against the CURRENT staging relayer and TEE - i.e., every merge proves the laggard-compatible direction before publishing. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Test the compatibility direction that can fail in production.
The spec requires old extensions to work against a newer relayer, but the stated gate only runs the new extension against the current staging relayer. Add the last published extension artifact against the candidate relayer; otherwise store-review lag can cause transfer failures after a relayer rollout.
| 1. Extension generates an ephemeral P-256 keypair and a `transferSessionId` (uuid), joins the relayer room, and renders a QR: `self-ext-link://{transferSessionId, extensionPublicKey}` (also shown as copyable text for emulator dev). | ||
| 2. Mobile dev-gated screen scans the QR with the existing scanner, shows a confirm sheet listing what will be sent, then loads `{mnemonic}` from keychain service `secret` and all documents via `loadDocumentCatalogDirectlyFromKeychain` / `loadDocumentByIdDirectlyFromKeychain`. | ||
| 3. Mobile derives a shared key (ephemeral mobile keypair x extension public key), encrypts the payload AES-256-GCM, and emits it through the relayer room. Relayer never sees plaintext; the extension key never leaves the extension. | ||
| 4. Extension decrypts, validates shape (`DocumentCatalog`, `IDDocument` guards from `@selfxyz/common`), prompts for a custody password, encrypts at rest, and writes through the same `secureStorage` keys webview-app expects. | ||
| 5. Both ends render a short authentication string - 4 emojis from `sha256('self-ext-link-sas-v1' || sharedSecret)` mapped over a 64-emoji table (`@selfxyz/mobile-sdk-alpha/utils/sas`, shared by the app screen and the extension) - so the user can visually confirm the keys agree. The check happens BEFORE any secret moves: on scan the phone emits a hello (ephemeral public key only) into a second relayer room (`helloSessionId` from the QR, because the relayer forwards one `self_app` per session), both sides display the emojis, and the phone's Send button ships the envelope only after the user compares. Cross-implementation match (node/elliptic vs WebCrypto) is asserted by the import harness and a known-vector unit test. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Make helloSessionId part of the QR contract.
Lines 41-45 say the QR contains only transferSessionId and extensionPublicKey, but the pre-secret hello is sent to helloSessionId “from the QR.” The handshake cannot be routed deterministically, so the SAS gate may stall before transfer. Add the field or consistently use transferSessionId on both clients.
| - Passkey vault: single "Unlock with Touch ID" button, auto-triggered on popup open (with the button as the retry affordance), password UI absent. | ||
| - Password vault: password field (secure input with reveal toggle), inline error message on wrong password + field shake, attempt counter after 3 failures, and "Unlock & enable Touch ID" upsell button when PRF is available. | ||
| - "Forgot password? Reset" stays a two-step destructive pattern: link -> explanation ("this browser only; your phone keeps everything") -> red confirm. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Retain a non-destructive passkey recovery path.
SPEC.md says the password remains a fallback, but this flow removes password UI for passkey vaults. If the PRF credential is unavailable, canceled, or lost, users have no non-destructive way to decrypt the vault and may be forced into data loss via reset. Keep the password fallback or specify another recoverable wrapped-key path.
| | Passkey ceremony | OS-controlled | button `verifying` state | | ||
| | Vault decrypt on unlock | <1s | button `verifying` state | | ||
| | Proof generation | 10-30s | staged progress card (existing pattern) | | ||
| | Backend confirmation (RP page) | 1-10s | page-side; SDK exposes status stream | |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Use the bridge result contract for embed status delivery.
The UX assumes the RP page receives an SDK status stream, but SPEC.md states that embed mode has no SDK socket and that the bridge translates setResult/dismiss. If the page waits on the unavailable stream, backend confirmation can remain stuck indefinitely. Specify the bridge-delivered typed result/failure events as the source of truth.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41d52f4510
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| { | ||
| "transferSessionId": "<uuid v4>", | ||
| "receiverPublicKey": "<P-256 uncompressed public key (04||X||Y), hex>", | ||
| "relay": "<relayer base URL, e.g. wss://websocket.staging.self.xyz>" | ||
| } |
There was a problem hiding this comment.
The conclusion above requires a second relayer room for the pre-send SAS hello because only the first self_app is forwarded per session, but this QR schema gives the phone only transferSessionId. An implementation following this schema either sends the hello in the transfer room and has the encrypted envelope dropped, or skips the SAS gate entirely, so include helloSessionId or a deterministic derivation in the encoded QR before this plan is used.
Useful? React with 👍 / 👎.
| 1. Trigger: push to `main` touching `packages/chrome-extension/**`, `packages/webview-app/**`, `packages/webview-bridge/**`, or `packages/mobile-sdk-alpha/**` (same paths-filter pattern as self-infra `staging.yml`). | ||
| 2. Build chain: common -> sdk -> bridge -> webview-app -> extension (the exact chain the harnesses use today). | ||
| 3. Gates: unit tests + `boot-check`, `import-check`, `disclose-check` puppeteer harnesses against Chrome for Testing (already headless-capable), gitleaks, bundle-size budget. | ||
| 4. Version: manifest `version` derived from a monotonic counter (date-based `YYYY.MM.DD.N`, mirroring self-infra image tags); the manifest `key` field is stripped for store builds (store assigns the real ID; the pinned key remains for local/dev loads). |
There was a problem hiding this comment.
Chrome Web Store uses the manifest version field, and Chrome's docs require dot-separated integer parts where non-zero parts cannot start with 0 (docs). The proposed YYYY.MM.DD.N value produces invalid manifests for zero-padded months or days such as 2026.07.29.1, so the automated publish job will fail unless the date parts are unpadded or the display date is moved to version_name.
Useful? React with 👍 / 👎.
| Goal: merging to the release branch publishes a new extension version with no human in the loop except the PR review itself. | ||
|
|
||
| - **Pipeline** (GitHub Actions in self/self): | ||
| 1. Trigger: push to `main` touching `packages/chrome-extension/**`, `packages/webview-app/**`, `packages/webview-bridge/**`, or `packages/mobile-sdk-alpha/**` (same paths-filter pattern as self-infra `staging.yml`). |
There was a problem hiding this comment.
This trigger set omits common/** even though the next step builds common first and the extension SDK imports @selfxyz/common for proving/document logic. A commit that changes shared circuit inputs, proving helpers, or document validation would not run the extension gates or publish a refreshed artifact until some listed package path also changes, leaving the store build stale or untested for that dependency update.
Useful? React with 👍 / 👎.
| 4. Version: manifest `version` derived from a monotonic counter (date-based `YYYY.MM.DD.N`, mirroring self-infra image tags); the manifest `key` field is stripped for store builds (store assigns the real ID; the pinned key remains for local/dev loads). | ||
| 5. Upload: `chrome-webstore-upload-cli` with an OAuth refresh token stored in repo secrets (scoped Google Cloud project owned by the org, not an individual); publish target starts as **trusted testers**, flips to public per release train. | ||
| 6. Post-publish: job posts the version + store review status to the team channel; failed store review pages the owner. | ||
| - **Store review latency** is the one step we cannot automate (hours to days). Mitigation: publish cadence decoupled from merge cadence via the trusted-tester channel (instant) + public promotion when review clears. Proving-protocol changes must stay backward compatible for at least one store-review window since old extension versions linger. |
There was a problem hiding this comment.
This mitigation assumes the trusted-tester channel is instant, but Chrome Web Store review still gates submitted extension updates; Chrome's review docs state review times vary and can take days or weeks (docs), and Chromium Extensions maintainers have clarified tester submissions also go through review (thread). In this release path, trusted testers cannot validate a just-merged artifact until review completes, so the spec needs an unpacked/internal distribution path or should stop relying on tester publication as the immediate lane.
Useful? React with 👍 / 👎.
| 6. Post-publish: job posts the version + store review status to the team channel; failed store review pages the owner. | ||
| - **Store review latency** is the one step we cannot automate (hours to days). Mitigation: publish cadence decoupled from merge cadence via the trusted-tester channel (instant) + public promotion when review clears. Proving-protocol changes must stay backward compatible for at least one store-review window since old extension versions linger. | ||
| - **Rollback**: store rollback is re-publishing the previous artifact; CI keeps the last N built zips as workflow artifacts, and the version counter never reuses a number. | ||
| - **Secrets**: refresh token + extension ID in GitHub environments with required reviewers; token rotation documented in the runbook. |
There was a problem hiding this comment.
Storing the upload token in a GitHub environment with required reviewers means the upload job cannot access those secrets until a reviewer approves the deployment; GitHub documents that jobs referencing such environments wait for approval before proceeding (docs). That contradicts the no-human-in-the-loop release goal above, so use branch-restricted environment secrets without required reviewers for the automated path or call out the manual gate explicitly.
Useful? React with 👍 / 👎.
| 5. Upload: `chrome-webstore-upload-cli` with an OAuth refresh token stored in repo secrets (scoped Google Cloud project owned by the org, not an individual); publish target starts as **trusted testers**, flips to public per release train. | ||
| 6. Post-publish: job posts the version + store review status to the team channel; failed store review pages the owner. | ||
| - **Store review latency** is the one step we cannot automate (hours to days). Mitigation: publish cadence decoupled from merge cadence via the trusted-tester channel (instant) + public promotion when review clears. Proving-protocol changes must stay backward compatible for at least one store-review window since old extension versions linger. | ||
| - **Rollback**: store rollback is re-publishing the previous artifact; CI keeps the last N built zips as workflow artifacts, and the version counter never reuses a number. |
There was a problem hiding this comment.
Chrome Web Store updates require every uploaded package to have a larger manifest version than the previous package (docs). Re-publishing the previous artifact as-is will carry the older manifest version and be rejected after a bad release has already advanced the store version, so rollback needs to rebuild the previous code with a fresh monotonically higher version instead of reusing the saved zip verbatim.
Useful? React with 👍 / 👎.
…aunch gates) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-page (CEP-09 target) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nd popup scope (mini home) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
specs/projects/sdk/workstreams/chrome-extension/PRODUCTION.md (2)
46-46: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse a Chrome-compatible manifest version format.
YYYY.MM.DD.Nwill produce invalid leading-zero version components for months/days like07/29; Chrome manifest versions must be unpadded integers without leading zeroes. Define the generator to emit unpadded integers and ensure generated versions are valid before upload.Source: MCP tools
50-50: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRepackage rollback contents with a fresh version number.
Re-publishing the previous ZIP unchanged will not satisfy Chrome Web Store’s update versioning, because rollbacks still require a higher version number for the rollback package (or an in-dashboard rollback workflow). Update this spec to describe re-zipping the previous contents with a new version, not just the previous artifact.
Source: MCP tools
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 964eb823-8ac6-48ec-ade5-dc234ba1fed0
📒 Files selected for processing (2)
specs/projects/sdk/workstreams/chrome-extension/PRD.mdspecs/projects/sdk/workstreams/chrome-extension/PRODUCTION.md
🚧 Files skipped from review as they are similar to previous changes (1)
- specs/projects/sdk/workstreams/chrome-extension/PRD.md
…specs skill) SPEC.html (spike record), SPEC-PRD.html, SPEC-PRODUCTION.html, SPEC-UX.html replace the markdown versions: locked template chrome, mermaid diagrams (enterprise flow, system topology, publish pipeline, link-flow SAS), static answered decision cards, cross-doc topbar links. INDEX updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ions, privacy; blocks CEP-03) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…NEY.html) Journeys A/B/C with every screen, states, and Euclid components; five open design questions (DQ-1..5) rendered as review cards: emoji set vetting, biometric affordance, QR expiry treatment, success motion, popup frame. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…auto lock, fail-closed mid-session) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…pole sequencing, per-RP flag Findings from a product review of the full scope: - new PRD F6/F7 requirements + post-link lifecycle section (account drift, rotation fails closed via key epoch, phone-lost, uninstall, multi-browser, no-usable-document guard); linked-browser surface is a log, not a revoke control, stated plainly - metrics table rewritten with an instrument per metric, activation metric added, headline comparisons moved server-side so consent cannot make them unmeasurable, time-to-proof raised to 60s because the session policy makes unlock the common path - trusted-tester gate becomes an explicit accepted risk (test documents only until security sign-off) - CEP-08/CEP-09 marked as external long poles; demo-blocking vs public-listing split called out - journey: unlock is a first-class step, empty-document state guarded upstream Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d vs backlogged) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ptographically The QR is a trusted OOB channel we were under-using: v2 left sender authentication to a human emoji comparison. v3 adds a 32-byte linkSecret, never sent to the relayer, as the HKDF salt, so only a device that scanned the QR can produce an authenticating envelope. The emoji step is demoted to covering the observed-QR case rather than being the trust anchor. Includes a correction to the review's SAS-grindability claim. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- plans/CE-01-transfer-protocol.md rewritten as the versioned v3 contract: full QR schema (helloSessionId and linkSecret were both missing, so an implementer could not route the handshake), trust model, key schedule with rationale, message flow, receiver fail-closed rules, typed failure/nack table with retry semantics, size cap, relayer facts, version history - PRODUCTION: the CI path filter must cover every artifact-changing dependency including common, or a fix there silently ships nothing - PRODUCTION: compatibility gates now cover BOTH directions - the spec requires old extensions to work against a newer relayer, which is the direction store-review lag exposes, and the old gate never tested it - SPEC (spike record): callout marking its transfer section superseded as a protocol reference, pointing at the versioned doc Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Review feedback addressed (CodeRabbit):
|
1 participant
Docs-only: brings the chrome-extension workstream specs to dev.
SPEC.md+plans/CE-01-transfer-protocol.md: spike record (CE-01..CE-06 done, validated on real device 2026-07-27; risks with outcomes).PRODUCTION.md: production scoping - extension as a thin shell over webview-app/SDK/bridge/Euclid (maintenance-burden model), Chrome Web Store CI auto-publishing, Enterprise-only RP integration via the hosted verification page (no@selfxyz/qrcode/coresurface - legacy, no partner use), versioning/cross-compat matrix, security gates, backlog CEP-01..CEP-12.UX.md: journey maps (SAS emoji check gates the send button on the phone), loading-state inventory, Euclid v1.4.6 gap analysis with exact component citations.INDEX.md: two workstream rows.The spike branch (
feat/chrome-extension-spike) is a prototype and will not be merged - it's reference material and a working demo. Production is implemented fresh on dev through the CEP backlog; surviving pieces (envelope crypto, vault, SAS module, harnesses) are ported deliberately per ticket with normal review.Mirrored in Linear: project "Browser Extension v1 (Production)" (SELF-3690..3701) under the Chrome Extension Disclosure Spike initiative. Repo spec is canonical. Related: self-infra PR #166 (relayer session-relay robustness).
🤖 Generated with Claude Code
Summary by CodeRabbit