fix(ocap-kernel): request only the URL the fetch caveat approved by sirtimid · Pull Request #1026 · MetaMask/ocap-kernel

sirtimid · 2026-08-19T09:24:21Z

Fixes MetaMask/MetaMask-planning#7557.

Problem

A vat granted fetch could reach any host regardless of its network.allowedHosts. Three routes, one shape: the URL the caveat approves is not the URL that gets requested.

TOCTOU (CWE-367 — the reported PoC). Both enforcement points resolved the vat's input to a URL for validation and then handed the input itself to fetch, which resolved it a second time. An input answering differently on each read was validated as an allowed host and requested as a forbidden one. harden() does not help: it freezes the wrapper, not the vat's argument.
Redirects. redirect defaults to follow and the caveat only ever saw the pre-flight URL, so an allowed host answering 302 Location: http://169.254.169.254/latest/meta-data/ sent the vat's request outside the allowlist and handed the vat the response body. Not merely a risk from a compromised third party: an allowlist may perfectly well name a host the vat itself controls, which makes this a general escape.
Transport replacement. undici honours a dispatcher in init, and a dispatcher decides where the bytes go whatever URL the caveat approved. Found while fixing (2); not in the report.

Fix

New in @metamask/kernel-utils:

resolveFetchInput resolves an input exactly once and returns the resolved URL plus a stand-in to forward in its place.
makeGuardedFetch wraps a fetch so a guard runs before every request it makes, following redirects itself, one hop at a time.

Both enforcement points route through them — makeCaveatedFetch/makeHostCaveat (ocap-kernel) and makeHostRestrictedFetch (kernel-language-model-service) — and are now one-line delegations.

The caveat takes a URL and nothing else. The deleted FetchCaveat handed every policy the raw fetch args and made it re-derive the URL itself, via the now-deleted resolveUrl — the exact CWE-367 read. A guard author now has no raw input to mis-resolve, and no partially-accurate init to mis-read: the first request's init is assembled from the caller's arguments while a hop's comes from the redirect rewrite, so an init-reading policy would have been sound only after the first hop — the hop every request makes. requestOnce likewise takes the resolved { url, input } pair rather than a destination and the URL it is claimed to be, so passing the caller's own input is a type error rather than a comment violation.

Closed

Vector	Handling
Stringifier answering differently on successive reads (the PoC)	Rejected, rather than serviced at whichever URL it showed first
`Request` subclass overriding its `url` getter	URL read through the genuine accessor
`Request` state in a mutable own property (undici on Node 22)	`Request` copied, then rebuilt around the resolved URL as a string
Redirect out of the allowlist	Refused; the forbidden host is never contacted
`redirect: 'follow'` in `init` or on a `Request`	Overridden, not merged
`dispatcher` in `init`	Refused — from a snapshot of `init`, so an accessor cannot answer the check with `undefined` and the transport with a dispatcher
`dispatcher` planted as an own property of a `Request`	Shed by the copy that precedes the rebuild
`baseFetch` that walks a chain itself despite `redirect: 'manual'`	Refused, rather than returned as the approved resource

Semantics

Follow per hop, don't reject outright. A redirect that stays inside allowedHosts is followed as fetch would have, so http→https and /x→/x/ keep working. response.url names where the chain ended and response.redirected reports the chain. Getting redirected right needs a proxy: the vat fetch endowment hardens its response, so the flag cannot be written onto it. The proxy is not the response — Response.prototype.text.call(view) throws where view.text() works — which the JSDoc records.
Except in a browser, where no redirect can be followed at all. Checking a hop requires redirect: 'manual', and a browser answers that with an opaque-redirect response — status 0, no headers — which hides the target instead of exposing it. VatSupervisor runs in an iframe, so this is the extension's behaviour, not a corner case: a redirected request fails there rather than being followed. Failing closed is the only option; the alternative is approving a URL and requesting whatever the Location said. Non-redirected requests are unaffected.
Only follow is overridden. It is the one mode that could walk to an unapproved host. manual and error ask for less, so they are obeyed rather than silently upgraded. The mode is answered on the status alone, before the Location is read, as the spec orders it — so error fails a redirect that names nowhere to go rather than passing it off as an ordinary response, which is also what undici does.
Three things fail closed rather than quietly:
- a hop that keeps the body when that body cannot be sent again. Stricter than the spec, which replays a stream whose source it kept — a Request's body is a stream however it was built, so fetch(new Request(url, {method:'POST', body:'x'})) across a 307 now errors where fetch would have replayed it. Buffering every body up front to avoid that would make an unbounded upload an unbounded allocation; the error names the hop and the remedy.
- a hop to a scheme fetch will not follow (data:, file:, blob:), refused by name rather than incidentally as a nameless host.
- a baseFetch that followed a redirect below the guard. Every request here asks for manual, so a response reporting redirected means a chain was walked that the guard never saw — silently degrading back to the pre-flight-only checking this PR replaces.
Spec rewrite otherwise: 303 (and 301/302 from a POST) → bodyless GET, credentials dropped when a hop leaves the origin, 20-hop limit.

Verification

Against live http.createServer instances on Node 22.20 and 24.18 (both in the CI matrix; engines: >=22), exercising the built dist under lockdown — 49 checks pass on both, and 13 of them fail against the pre-fix build, with the forbidden server recording GET /secrets.

Covered: single cross-host redirect; chain ending on an allowed host; excursion to another allowed host and back; redirect loop; vat-supplied follow/manual/error and a Request carrying its own mode; 307 with string and stream bodies; 303 POST→GET; cross-origin credential stripping; non-fetchable scheme; empty Location; dispatcher in init, planted on a Request, and hidden behind an accessor. Plus the non-regressions — GET, POST string/stream bodies, Request reuse, Request + init override, AbortController propagation across a hop — all with bodies intact.

All 33 mutations of guarded-fetch.ts are caught by its tests, including each arm of the origin comparison, each entry of both header lists, the 303 HEAD exemption, and the body: null and method-case edge cases. Deleting resolveFetchInput's defensive new Request(input) copy is caught too — that copy is what sheds a planted dispatcher, and asserting it needs a live server, since the rebuilt Request keeps a dispatcher out of reach whether or not it carried one. Coverage is layered: real-server tests in kernel-utils, unit tests at both enforcement points, and an end-to-end test in kernel-test driving every attack from a real vat through the real supervisor and the hardened Snaps ResponseWrapper.

Not addressed

DNS rebinding. The caveat matches the hostname a vat names, so an allowlisted name resolving to a loopback or link-local address reaches it. Which address a name resolves to at connect time is undici's business and not reachable from here.
No audit trail. A refused hop throws; the vat sees the error and may swallow it, leaving no kernel-side record that an allowlist violation was attempted. The signal is currently inverted — successful egress reaches logger.debug, a denial reaches nothing — and the success path carries no URL either, so there is no egress-destination trail in either direction. Worth a follow-up: an onRefusal on makeGuardedFetch would also catch the non-guard refusals (bad Location, non-fetchable scheme, hop cap, unreplayable body).
cache is not carried across a hop. This package compiles against undici's RequestInit, which has no cache field because undici ignores cache mode, so a hop in a browser realm reverts to the default.
Stream request bodies in a browser. The Request rebuild passes the original through as a RequestInit, which needs duplex when the body is a stream. undici exposes Request.prototype.duplex; browsers do not, so a browser vat's stream-bodied Request fails before any request is made. Fail-closed, and narrow — streaming uploads need HTTPS and HTTP/2 anyway.

🤖 Generated with Claude Code

Note

High Risk
Changes security-critical vat network confinement and redirect/integrity semantics; incorrect behavior could allow exfiltration or break legitimate fetches.

Overview
Security fix for vat fetch escaping network.allowedHosts (CWE-367): the allowlist was checked on one URL resolution while the underlying fetch could read the input again or follow redirects to forbidden hosts.

Adds resolveFetchInput and makeGuardedFetch in @metamask/kernel-utils so the guard runs on the URL that will actually be requested, with a safe stand-in input forwarded to baseFetch. Redirects are handled manually (redirect: 'manual' on every hop) so each Location is host-checked; caller redirect: 'follow' is overridden, manual/error honored. Also rejects dispatcher in init, blocks non-replayable bodies on body-preserving redirects, and verifies integrity against the final response body (not intermediate 3xx bodies).

makeCaveatedFetch / makeHostCaveat (ocap-kernel) and makeHostRestrictedFetch (Ollama) now delegate to makeGuardedFetch; the host caveat takes a URL only (removed resolveUrl / raw-args caveat). Regression coverage spans unit tests, live HTTP servers, and kernel vat endowment tests.

^{Reviewed by Cursor Bugbot for commit 282fbaf. Bugbot is set up for automated code reviews on this repo. Configure here.}

A vat granted fetch could reach any host regardless of network.allowedHosts. The allowlist and the file: prohibition were checked against one resolution of the vat's input, and then the input itself was handed to fetch, which resolved it a second time. An input answering differently on each read was validated as an allowed host and requested as a forbidden one (CWE-367). Reported as a SEV-1 vat sandbox escape in MetaMask/MetaMask-planning#7557. Add resolveFetchInput to @metamask/kernel-utils, which resolves an input exactly once and returns a stand-in to forward in its place, and route both enforcement points through it: makeCaveatedFetch in ocap-kernel and makeHostRestrictedFetch in kernel-language-model-service. The caveat now receives a URL rather than raw input, so the old shape is no longer expressible. Two further vectors, neither in the report, are closed with it. A Request subclass can override its url getter, so the URL is read through the genuine accessor. And undici on Node 22 keeps a Request's state in a configurable own property, which lets a vat make it answer differently on each read and lets it leave a URL object of its own in there to mutate while the caveat is still awaiting; a Request is therefore copied and then rebuilt around the resolved URL as a string. The copy has to come first, because the rebuild reads its argument as a RequestInit by string name and would otherwise pick up a planted dispatcher. Verified against live HTTP servers on Node 22 and 24: every variant is refused and the forbidden host is never contacted, while GET, POST bodies, Request reuse, stream bodies and abort propagation are unaffected. Each regression test was mutation-checked against the code it guards. Not addressed, and unchanged by this: the caveat sees only the pre-flight URL, so a redirect from an allowed host to a disallowed one is still followed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The fetch caveat saw only the URL a vat asked for. `redirect` defaults to 'follow', so an allowed host answering `302 Location: http://169.254.169.254/latest/meta-data/` sent the vat's request to a host outside `network.allowedHosts` and handed it the response body. An allowlist may perfectly well name a host the vat itself controls, so this is a general escape rather than a risk from a compromised third party. Left open by the previous commit, which fixed the TOCTOU escape in the same functions and said so. Add makeGuardedFetch to @metamask/kernel-utils, which follows redirects itself rather than leaving them to fetch, re-running the guard on each hop's URL, and route both enforcement points through it: makeCaveatedFetch in ocap-kernel and makeHostRestrictedFetch in kernel-language-model-service. A hop out of the allowlist fails the call and the forbidden host is never contacted; a hop that stays inside is followed as fetch would have, so the allowlist keeps meaning what it says, and response.url and response.redirected still describe the chain. The redirected flag needs a proxy: the vat fetch endowment hardens the response it returns, so the flag cannot be written onto it. A vat cannot ask to opt out. Only `follow` is overridden — on init and on a Request alike, both of which the vat controls — since it is the one mode that could walk to an unapproved host; `manual` and `error` ask for less and are obeyed. Hops follow the fetch spec's rewrite: a 303, and a 301 or 302 from a POST, becomes a bodyless GET; credentials are dropped when a hop leaves the origin; the chain gives up after 20. Three things fail closed rather than quietly. A hop that keeps the body, when that body cannot be sent a second time, errors instead of hanging or sending a truncated request — stricter than the spec, which replays a stream whose source it kept, and a Request's body is a stream however it was built. A hop to a scheme fetch will not follow is refused by name. And an opaque redirect — what a browser answers a manual redirect with, hiding the target instead of exposing it — fails rather than handing the vat a status-0 husk it would read as the resource it asked for. Closed with it, and not in the report: undici honours a `dispatcher` in `init`, which stands in for the transport and so decides where the bytes go whatever URL the caveat approved. It is refused rather than dropped, since dropping it would fall back to the global transport and egress anyway. Verified against live HTTP servers on Node 22 and 24: 49 checks pass, and 13 of them fail against the pre-fix build with the forbidden server recording the request. All 33 mutations of the new module are caught by its tests. Not addressed: the caveat matches the hostname a vat names, so an allowlisted name resolving to a loopback or link-local address still reaches it; and a refused hop is not logged, so a vat that swallows the error leaves no trace. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The `dispatcher` rejection read `rawInit.dispatcher` for the check and then re-read it via `{ ...rawInit }`, so an accessor answering `undefined` once and a dispatcher thereafter passed the check and still reached the transport — the same substitution this wrapper exists to stop. Node's global fetch honours a plain object with a `dispatch` method, and the Snaps endowment spreads `init` verbatim, so it was reachable from a vat. The check now reads the snapshot, so the value checked is the value forwarded. Also from review: - `redirect: 'error'` returned the 302 rather than throwing when the hop carried no `Location`. The mode is now answered on the status alone, before the `Location` is read, as the spec orders it and undici does. - A `baseFetch` that walked a chain despite `redirect: 'manual'` had its response returned as the approved resource, silently degrading to the pre-flight-only checking this branch replaces. Refused now. - `asRedirected` threw a proxy-invariant `TypeError` for a response carrying `redirected` as a frozen own value, which is this repo's house style for a test double. It reports the target's value instead of crashing. - Two throw paths abandoned a response body without cancelling it, holding its connection open. Every early exit now goes through `discardBody`. - The guard is consulted before the replayability check, so a hop out of the allowlist is reported as that rather than as a body problem. - The `dispatcher` check is reached through `in`, so it compiles where the ambient `RequestInit` is the DOM one. It previously failed to typecheck in the seven DOM-lib packages and for anyone compiling from source. `FetchGuard` takes the URL alone. It was also handed an `init` describing only the first hop's caller-supplied fields, so a method- or body-restricting policy would have been enforced for `{ method: 'POST' }` and skipped for the same request wrapped in a `Request` — sound only after the first hop, which is the hop every request makes. Normalizing could not fix it, since `init.redirect` is unconditionally `manual` by the time a guard sees it. `requestOnce` likewise takes the resolved `{ url, input }` pair, so passing the caller's own input is a type error rather than a comment violation. Four regression tests, each verified to fail against the unfixed source. One covers the defensive `new Request(input)` copy: deleting it is a live escape that the previous structural assertion could not see, because the rebuilt `Request` keeps a dispatcher out of reach whether or not it carried one, so the property is only observable on the wire. Test setup deduplicated by about 200 lines with no assertion changes, and comments trimmed to the non-obvious why. The changelogs now name the browser-realm limit: checking a hop needs `redirect: 'manual'`, which a browser answers with an opaque-redirect response, so a browser-hosted vat cannot follow a redirect at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 461793e. Configure here.}

`integrity` was copied onto every hop's init, and every request this wrapper makes asks for `redirect: 'manual'`, so `fetch` held a digest of the resource against each 3xx body it was handed and turned it into a network error. Any chain longer than a single request therefore failed, however right the digest was; verified against undici, which answers a manual redirect with the real response and checks the digest against it. The digest is now withheld from `baseFetch` — as `''`, since only an empty string means "no digest" to `fetch` and an absent init member leaves a `Request`'s own integrity standing — and checked against the body actually handed back, which is where the spec spends it too, after the chain has been walked. Checking it means reading it, so it is read from a `clone`, whose tee leaves the caller's bytes intact. The new `subresource-integrity.ts` reads the metadata and digests the bytes. It is stricter than `fetch` in one respect: metadata naming no algorithm SRI is defined over is refused rather than ignored, because a caller that asked for a digest and had none checked is worse off than one that asked for nothing. Covered against live servers in `kernel-utils` — including the digest of a hop, which must be refused where a digest of the resource is accepted — and end to end in `kernel-test` through the real supervisor and the hardened Snaps `ResponseWrapper`, whose own `clone()` is what the check goes through there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

github-actions · 2026-08-19T14:02:18Z

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	72.22% ⬆️ +0.17%	9510 / 13167
🔵	Statements	72.08% ⬆️ +0.19%	9672 / 13418
🔵	Functions	72.91% ⬆️ +0.04%	2248 / 3083
🔵	Branches	66.31% ⬆️ +0.51%	3888 / 5863

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
packages/kernel-language-model-service/src/ollama/fetch.ts	100% 🟰 ±0%	100% 🟰 ±0%	100% 🟰 ±0%	100% 🟰 ±0%
packages/kernel-test/src/vats/endowment-fetch.ts	0% 🟰 ±0%	0% 🟰 ±0%	0% 🟰 ±0%	0% 🟰 ±0%	16-105
packages/kernel-utils/src/fetch-input.ts	95.65%	85.71%	100%	95.65%	36
packages/kernel-utils/src/guarded-fetch.ts	100%	100%	100%	100%
packages/kernel-utils/src/index.ts	100% 🟰 ±0%	100% 🟰 ±0%	100% 🟰 ±0%	100% 🟰 ±0%
packages/kernel-utils/src/subresource-integrity.ts	100%	100%	100%	100%
packages/nodejs-test-workers/src/workers/mock-fetch.ts	0% 🟰 ±0%	0% 🟰 ±0%	0% 🟰 ±0%	0% 🟰 ±0%	6-66
packages/ocap-kernel/src/vats/network-caveat.ts	100% 🟰 ±0%	100% 🟰 ±0%	100% 🟰 ±0%	100% 🟰 ±0%
packages/repo-tools/src/test-utils/fetch-input.ts	100%	100%	100%	100%
packages/repo-tools/src/test-utils/index.ts	100% 🟰 ±0%	100% 🟰 ±0%	100% 🟰 ±0%	100% 🟰 ±0%

Generated in workflow #4663 for commit 53aef6c by the Vitest Coverage Report Action

Keep a Changelog has a Security category; a **SECURITY:** prefix under Fixed was standing in for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

FUDCo

That's quite a bit heavier than I would have predicted, but it's also much more comprehensive. LGTM.

sirtimid and others added 4 commits August 19, 2026 11:23

docs: trim the fetch changelog entries to the consumer-facing change

615e223

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

sirtimid marked this pull request as ready for review August 19, 2026 12:30

sirtimid requested a review from a team as a code owner August 19, 2026 12:30

cursor Bot reviewed Aug 19, 2026

View reviewed changes

Comment thread packages/kernel-utils/src/guarded-fetch.ts

sirtimid and others added 2 commits August 19, 2026 15:07

chore: rerun CI

282fbaf

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

rekmarks reviewed Aug 19, 2026

View reviewed changes

Comment thread packages/ocap-kernel/CHANGELOG.md Outdated

rekmarks reviewed Aug 19, 2026

View reviewed changes

Comment thread packages/kernel-language-model-service/CHANGELOG.md Outdated

docs: file the fetch confinement fix under Security

53aef6c

Keep a Changelog has a Security category; a **SECURITY:** prefix under Fixed was standing in for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

FUDCo approved these changes Aug 19, 2026

View reviewed changes

sirtimid added this pull request to the merge queue Aug 19, 2026

Merged via the queue into main with commit 37db357 Aug 19, 2026
37 checks passed

sirtimid deleted the sirtimid/fix-fetch-input-toctou branch August 19, 2026 21:34

Navigation Menu

Uh oh!

fix(ocap-kernel): request only the URL the fetch caveat approved - #1026

fix(ocap-kernel): request only the URL the fetch caveat approved#1026
sirtimid merged 7 commits into
mainfrom
sirtimid/fix-fetch-input-toctou

sirtimid commented Aug 19, 2026 •

edited by cursor Bot

Loading

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

github-actions Bot commented Aug 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

FUDCo left a comment •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

sirtimid commented Aug 19, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Closed

Semantics

Verification

Not addressed

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report

Uh oh!

Uh oh!

Uh oh!

FUDCo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sirtimid commented Aug 19, 2026 •

edited by cursor Bot

Loading

github-actions Bot commented Aug 19, 2026 •

edited

Loading

FUDCo left a comment •

edited

Loading