fix(extension): take the origin from the browser, not from the message body - #82
Merged
Merged
Conversation
…e body Every origin decision in the background — `isOriginTrusted`, `trustOrigin`, `checkOriginPin`, `pinOrigin`, and what the consent prompt names — read `req.origin`, a field the content script placed in the *message body*. The `sender.id === chrome.runtime.id` check above it proves the message came from this extension. It says nothing about which page it came from: every content script (the manifest injects across `<all_urls>`), the popup, the options page, the consent window and the offscreen document all pass it. So the origin was a claim by an untrusted party about itself — and `isOriginTrusted` honoured claims. Asserting `origin: "https://bank.example"` was enough to be handed the silent bypass the user had granted that site by ticking "remember". Today that is defence-in-depth rather than an exploit: `content_scripts` omits `all_frames`, and `content.ts` rejects `event.source !== window`. But a compromised renderer is precisely what a content script is meant to survive, and under a generic task relay this value decides whether a DID gets deactivated. Take `sender.origin`, which the browser attributes and page content cannot forge, and overwrite whatever the body claimed. Done once at the message boundary, so every consumer downstream becomes trustworthy without touching any of them. It is not the one-liner it looks like. `sender.origin` alone is insufficient: an extension-internal sender reports `chrome-extension://<id>`, a perfectly real origin that would pass any check asking merely "is an origin present?". So a page-facing message must also have a `tab` behind it, and one that does not is refused — there is no legitimate sender for that. `attestedOrigin` lives in `@openvtc/pnm-core` rather than the extension: it is pure, the extension package has no test harness, and a mobile agent's IPC bridge must hold itself to the same rule. Two more in the same area: - `signTrustTask` — the most powerful method on the page-facing surface, signing an *arbitrary* envelope with the holder key — prompted for nothing and discarded the origin it was handed. The ergonomic argument ("per-signature prompts would be crippling") is sound, but "don't prompt a site the user has connected" is not "don't prompt anyone": as written, any page could obtain a signature over anything, which under a generic relay is a straight bypass of the VTA's policy engine. Now gated on the attested origin, naming the task type it is being asked to sign. A connected site still short-circuits and sees no prompt. - The content script's method ternary ended in `: RUNTIME_LOGIN`, so any method name it did not recognise silently became a login request. Now an explicit table that fails closed. Also repairs the lockfile: `npm ci` has been failing since `@openvtc/vti-tsp-js` was added without it (#80). The repo has no CI, so nothing caught it. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
stormer78
added a commit
that referenced
this pull request
Jul 13, 2026
#84) Adding a capability used to mean touching five places: a method on the provider, an arm in the content script's dispatch chain, a background handler, an offscreen forward, and a client in core. Each arm was a fresh opportunity to get the authorization wrong, and the surface grew one bespoke hole at a time. One method replaces them. The page proposes `(typeUri, payload)` and nothing else; the VTA decides everything that matters. **The device mints the envelope.** The obvious shape is for the page to hand over a complete Trust Task and for the wallet to sign it — that is what `vault/sign-trust-task` does, and it is exactly what must not happen here. Counter-signing an RP-authored envelope means attesting to a document the wallet did not write: the RP chooses the issuer, the recipient, the expiry, the id — every field the VTA subsequently trusts *because the wallet signed it*. The wallet becomes a notary for claims it never checked. So the page's input is reduced to the only two things it is entitled to propose, and everything carrying authority is written by the device. **The origin is stamped by the device, inside the payload.** It comes from `sender.origin` (browser-attested, #82), never from anything the page said about itself, and it rides in `payload.ext` — so it is inside the digest an approver signs. The origin shown to the human is bound to the payload that executes and cannot be swapped afterwards. A page that sets the key itself is overwritten. **It prompts every time, and offers no "remember".** `gatedConsent` would be the obvious choice and is the wrong one: it short-circuits on origin trust, and origin trust is not capability trust. Under a per-method surface, "remember this site" meant "this site may call `vaultList()` and `proxyLogin()`" — because those were the only things it could call. Under a *generic* relay the same tick would mean "this site may ask my agent to do anything at all", and one made on a `vaultList()` prompt would silently authorize a DID deactivation a year later. The set of things the grant covers grew without the user ever being asked about the new members. The VTA's policy engine is the real authority, and its `requireConsent` gives the approver a properly informed prompt. But `config.policy.enforcement` is opt-in and **defaults to off** — so on a default deployment this prompt is the only thing between an arbitrary page and an arbitrary task. It does not get to be skippable. The long-term answer is scoped grants — `(origin, subject, typeGlob, expiry)`, rememberable only for tasks the VTA classifies `sideEffects: none`. Until those exist, ask. **A rejection is a result, not an error.** A `requireConsent` reject carries the VTA-signed consent requests an approver must render and the digest the page must display for the cross-device match. Collapsing it into a thrown error would throw the informed-consent flow away at the last hop — the one place nobody would look for it. 8 new tests: the device mints the envelope; envelope fields a page tries to author stay in the payload where the VTA's closed schema rejects them; the origin is stamped by the device and a page cannot forge it; other `ext` members survive; no origin means no stamp rather than an invented one; the caller's payload is not mutated; a consent-required rejection is returned intact. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
stormer78
added a commit
that referenced
this pull request
Jul 14, 2026
…ould self-approve (#86) Two security regressions, both from earlier PRs in this series, both found by adversarially re-reading the code against the design rather than trusting it. **The relay read its origin from the message body.** #82 added an attested-origin gate that overwrites the body's claim with `sender.origin` for page-facing message types. #84 then added `requestTask` and never put it in that set. So the one method whose origin is stamped into `payload.ext` — and therefore rides *inside the digest an approver signs* — was reading it straight from the content script's `window.location.origin`, which any extension-internal context can assert. A hostile page cannot forge it; a hostile extension context, which is exactly the threat the gate exists for, can. `RUNTIME_REQUEST_TASK` is now in the set. **`signTrustTask` short-circuited on a remembered origin.** #82 routed it through `gatedConsent` for its ergonomics, without noticing that `gatedConsent` returns `true` with no prompt for a trusted origin. But this method signs an *arbitrary* envelope with the holder key, so a remember-grant that silenced it means "let this site sign anything, forever" — a tick made once on a login prompt authorizing a DID deactivation a year later. It now uses `requestConsent`, which always asks. Origin trust is not capability trust; that was the whole point, and I had inverted it on the most powerful method on the surface. To stop the first bug's *class* from recurring: the page-facing set now derives from `PAGE_FACING_RUNTIME_TYPES`, a single source in `bridge-protocol.ts`, and the content script's dispatch table is typed `Record<BridgeMethod, string>`. Adding a page-facing method without registering it is now a compile error naming the method — which is how `requestTask` should have been caught the first time. (The content script bundles as a classic script and cannot import values, so the two files cannot literally share the list; the type binding is what keeps them in step.) Also: the per-action prompts (`requestTask`, `signTrustTask`) no longer render a "Remember this site" checkbox. There is nothing to remember for a one-shot authorization, and a checkbox that silently discards its own tick lies to the user. A `noRemember` flag suppresses it. `npm run build`, `npm run lint`, `npm test` (101) clean. Verified the compile guard fails when a method is removed from the table. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
stormer78
added a commit
that referenced
this pull request
Jul 16, 2026
…4, D8-F6) (#88) F4 (offscreen.ts) — the inbound mediator listener re-armed with a single 2s setTimeout from onClose only, and startInbound swallowed errors without rescheduling. A mediator outage longer than one retry left the listener dead, silently missing every consent request until the MV3 worker happened to reboot. Replace with a per-VTA exponential-backoff scheduler (scheduleInboundReconnect): 2s doubling to 60s, re-arming on FAILED attempts too, resetting on a successful (re)connect. startInbound now returns success; reconcileInbound schedules a retry for any VTA that fails to come up and clears backoff for one that succeeds. onClose fires only on an unexpected drop, and the forgotten-VTA close path cancels any pending retry, so a deliberately-closed listener isn't resurrected (a cancellation guard also covers the in-flight-attempt race). (R1.5) F6 (background.ts) — the page-facing authenticated fetch proxy (handleApiGet/handleApiPost) called fetch() with no AbortSignal, so a blackholed VTA hung the page's await for minutes and stacked requests pinned the MV3 worker. Route both through a shared proxyFetch that applies AbortSignal.timeout (20s); a stall now surfaces as a clean { ok: false, error } via the dispatcher's existing catch. (R1.2, R4.1 one shared helper rather than two copies) Type-checks clean (tsc -b packages/extension). No version bump: the extension is an unpublished artifact and recent fix(extension) PRs (#82/#86) followed the same convention. Signed-off-by: Glenn Gore <glenn.g@affinidi.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.
Prerequisite for putting a consent surface in this extension. A surface that renders "do you approve this, for this site" is only as trustworthy as its idea of which site is asking — and right now that idea comes from the site.
The defect
Every origin decision in the background —
isOriginTrusted,trustOrigin,checkOriginPin,pinOrigin, and what the consent prompt names — readsreq.origin, a field the content script placed in the message body.The
sender.id === chrome.runtime.idcheck above it proves the message came from this extension. It says nothing about which page it came from. Every content script (the manifest injects across<all_urls>), the popup, the options page, the consent window and the offscreen document all pass it.So the origin was a claim by an untrusted party about itself — and
isOriginTrustedhonoured claims. Assertingorigin: "https://bank.example"was enough to be handed the silent bypass the user had granted that site by ticking "remember".Today this is defence-in-depth rather than a live exploit:
content_scriptsomitsall_frames, andcontent.tsrejectsevent.source !== window. But a compromised renderer is precisely what a content script is meant to survive, and under a generic task relay this value decides whether a DID gets deactivated.The fix
Take
sender.origin— browser-attributed, not forgeable by page content — and overwrite whatever the body claimed. Done once at the message boundary, so every consumer downstream becomes trustworthy without touching any of them.It is not the one-liner it looks like.
sender.originalone is insufficient: an extension-internal sender reportschrome-extension://<id>, which is a perfectly real origin and would sail through any check that merely asked "is an origin present?". So a page-facing message must also have atabbehind it, and one that doesn't is refused — there is no legitimate sender for that.attestedOriginlives in@openvtc/pnm-core, not the extension: it's pure, the extension package has no test harness at all, and a mobile agent's IPC bridge will need to hold itself to the same rule.Two more in the same area
signTrustTaskprompted for nothing and discarded the origin it was handed. It is the most powerful method on the page-facing surface — it signs an arbitrary envelope with the holder key.The ergonomic argument in the comment ("per-signature prompts would be crippling for normal RP usage") is sound. But "don't prompt a site the user has connected" is not "don't prompt anyone". As written, any page could obtain a signature over anything — which under a generic task relay is a straight bypass of the VTA's policy engine: why go through a gate that can require consent, when you can ask the wallet to sign whatever you like?
Now gated on the attested origin, naming the task type it is being asked to sign. A site the user has already connected short-circuits inside
gatedConsentand still sees no prompt, so the ergonomics survive.The content script's method ternary ended in
: RUNTIME_LOGIN— so any method name it didn't recognise (a typo, a future method this build predates, anything a page cared to invent) silently became a login request. Now an explicit table that fails closed.Also
Repairs the lockfile —
npm cihas been failing since@openvtc/vti-tsp-jswas added without it (#80). This repo has no CI, so nothing caught it, and nothing is checking any of the above either. Worth fixing separately; I'd suggest at minimumnpm ci && npm run build && npm run lint && npm teston PRs.Tests
Five new cases in core covering
attestedOrigin, including the one that matters: a sender with no tab has no attested origin whatever origin it reports — thechrome-extension://case that a naive presence check would wave through.npm ci,npm run build,npm run lint,npm test(75 passing) all clean.