Skip to content

fix(core): gate Code Mode fetch on the webfetch permission - #54976

Closed
argszero wants to merge 1 commit into
anomalyco:devfrom
argszero:codemode-fetch-permission
Closed

argszero wants to merge 1 commit into
anomalyco:devfrom
argszero:codemode-fetch-permission

Conversation

@argszero

@argszero argszero commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Issue for this PR

Closes #54595

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

A "webfetch": "deny" rule did not apply to Code Mode. The fetch global that Code Mode injects called the
platform's fetch directly, so code passed to execute could reach the network even though the same URL was
refused for the webfetch tool.

The fix consults the webfetch permission inside the fetch global, before the request leaves the process, and
fails the call with the permission error the tool path uses. Denying earlier is not an option: an extension's
extension.before hook runs after the extension body for the globals Code Mode installs, so a denial there
would only deny the result, not the request.

Rebased onto the current dev, where the Codemode runtime became one object cached per tool inventory with the
per-execution parts in a Run service. A guard closed over by the runtime would therefore be shared by every
later call, so the guard now travels in Run with executeTool and hooks:

  • Run carries guardFetch. CodeModeWeb.extension builds its fetch global from it, so the runtime stays
    cached and the check stays per call.
  • fetch returns an Effect instead of a Promise, which is what lets the global read a service at all.
    Extension globals may now return an Effect, run in the caller's fiber exactly like a returned Promise, so the
    interpreter gives it the services of the execution that started it.
  • Nothing needs to thread an abort signal for the prompt: the interpreter already interrupts a call's pending
    work when the program ends, so a permission prompt still waiting at that point is cancelled with it. That is
    asserted rather than assumed — see the interruption case below.
  • Tool.node now depends on Permission.node, since the tool layer is what holds the permission service when it
    builds the Code Mode tool. packages/core/test/tool-todo.test.ts binds a permission layer as a result.

How did you verify your code works?

packages/core/test/tool-execute.test.ts drives the globals against a real local server:

  • deny: a guard that fails with Permission.BlockedError — the call settles as an error naming
    Permission denied: webfetch, and the server records zero requests. The "zero hits" assertion is what
    makes this a test of the gate rather than of the message.
  • allow: a guard that succeeds — the request arrives and the body comes back.
  • interruption: a guard that never settles is interrupted when the call it belongs to is, so the prompt does
    not outlive the call.
  • filesystem fetches: the pre-existing rejection still reports the current wording.

packages/codemode/test/extensions.test.ts covers the new capability on its own terms: a returned Effect sees
the services provided for that execution, a failed Effect reaches the program as its error, and an Effect still
waiting when the program ends is interrupted with it.

packages/core/test/tool-webfetch.test.ts drives the same gate through the real tool registry with a permission
layer that denies: the registry asserts webfetch with the URL, no request is made, and the filesystem rejection
is left to the runtime rather than the permission check.

Results on the rebased head: packages/core 6612 pass, and its 64 failures are the pre-existing
sandbox-bound ones (ripgrep, pty, shell, plugin spawning) — the same set the tree fails without this change, and
one of them, tool-todo, needed the permission layer this PR adds and passes with it. packages/codemode
1413 pass, 1 fail, the RegExp failure that also predates this branch. tsc -b is clean in both packages, as
are oxlint and prettier --check.

Screenshots / recordings

No response.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@github-actions

Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix works: Code Mode's fetch now follows the webfetch permission.

I ran the #54595 reproduction with opencode-drive and the simulated model on the base and on this branch. The webfetch rule was set to deny, as an explicit permissions rule, because drive's own * allow rule otherwise overrides the config. On the base, the local server received the request and execute succeeded. With this PR, the server got no request and the call failed with Permission denied: webfetch. With ask, the usual WebFetch permission prompt appears and nothing is sent until it is answered. The three changed test files pass (138 tests), and packages/core type-checks.

Public API: unchanged. CodeModeTool.create and CodeModeWeb.extension are internal to core.

One point inline: the permission check is awaited with Effect.runPromise, so it runs outside the execute fiber and is not cancelled with it.

Before

After

Comment thread packages/core/src/codemode/web.ts Outdated
async function fetch(input: string | URL, init: Init = {}) {
if (URL.parse(String(input))?.protocol === "file:")
throw new Error("fetch cannot be used for filesystem access.")
await Effect.runPromise(guard(String(input)))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect.runPromise runs the permission check in a detached fiber, outside the execute call that started it. If the user interrupts execute while an ask prompt is open, that check isn't interrupted: the request stays pending, and if it is approved later the fetch could still go out. The Code Mode interpreter accepts native functions that return an Effect, so it may be possible to run the guard (and the request) inside the caller's fiber and let cancellation carry through. Could you check whether extension globals can do that, and if not, at least tie the guard's lifetime to the execute call?

@argszero

Copy link
Copy Markdown
Contributor Author

Checked whether the guard can run in the caller's fiber, and it cannot: Extension.make requires a plain function, and the interpreter's wrap only gives a returned Promise the caller's-fiber treatment (ctx.pending.create(...)). A returned Effect is not a value the program can hold, so an Effect-returning extension global is not available.

So I tied the guard's lifetime to the call instead (51493039d7):

  • execute owns an AbortController whose signal Effect.ensuring aborts on every exit, including interruption.
  • CodeModeWeb.extension takes that signal and runs the check through Effect.runPromise(guard(url), { signal }). Interrupting the call now interrupts a permission prompt that is still waiting for an answer, and a check that outlives a completed call is cancelled the same way Pending.interrupt() cancels pending calls at the end of a program.

New test interrupting execute interrupts a fetch permission check that is still waiting aborts the call's signal while the guard is still waiting and asserts the guard's onInterrupt runs. It fails on a 5 s timeout without the source change, so it pins the wiring rather than the timing. bun test on tool-execute, tool-webfetch and tool-registry passes (139 tests) and tsgo -b is clean.

@argszero

Copy link
Copy Markdown
Contributor Author

Rebased onto the current dev (1080ab50b6), where this area was restructured and the old shape of the fix no
longer fit. My earlier comment here said an Effect-returning extension global "is not available"; it is now the
thing the fix uses, so I added it.

The runtime is now cached per tool inventory, with the per-execution parts in a Run service. A guard that the
runtime() factory closed over would have been shared by every later call of that inventory, so the guard now
lives in Run next to executeTool and hooks, and CodeModeWeb.extension builds its fetch global from it.

To read a service from inside a global, fetch returns an Effect, and packages/codemode/src/interpreter/extensions.ts
now treats a returned Effect the way it already treated a returned Promise: it runs in the caller's fiber, under
that execution's services, through ctx.pending.create. packages/codemode/test/extensions.test.ts covers that
on its own terms — a returned Effect sees the services provided for the execution, a failed one reaches the
program as its error, and one still waiting when the program ends is interrupted with it.

That last point removes the whole reason for 51493039d7's abort controller, so it is gone rather than rebased.
The prompt is cancelled because the interpreter interrupts a call's pending work when the program ends, not
because the source hands out a signal. The behaviour is still asserted: interrupting execute interrupts a fetch permission check that is still waiting passes without the abort wiring.

One consequence worth flagging in review: Tool.node now depends on Permission.node, since the tool layer is
where the Code Mode tool is built and the permission service lives. packages/core/test/tool-todo.test.ts binds
a permission layer for that reason. If you would rather the tool layer stay unaware of permissions, the
alternative is a plugin node for the guard — say the word and I will restructure it that way.

@argszero
argszero force-pushed the codemode-fetch-permission branch 2 times, most recently from cf2e331 to bdfb126 Compare October 22, 2026 05:04
@argszero

Copy link
Copy Markdown
Contributor Author

Rebased onto dev at c93aaae371, and adapted to #54276 ("run each job to completion"), which changed how an extension global's returned value is forked.

The rebase itself had one conflict, in the import block of packages/core/src/codemode/web.ts (isRecord from the base versus Effect from this branch). Both imports are kept — each is still used further down the file.

Two changes were needed beyond that conflict:

  • packages/codemode/src/interpreter/extensions.ts: an Effect-returning global now uses createHost, which is what fix(codemode): run each job to completion #54276 moved the returned-Promise path to. With create the check was queued on the job turn, and the main job now holds that turn until leftover promise work is interrupted, so the fiber never started — and interrupting a fiber that never started never reaches the effect's finalizers. That is exactly what extensions.test.ts "an Effect still waiting when the program ends is interrupted with it" was timing out on. The opposite direction is measured too: with that file reverted to the base, three tests in the same file fail.
  • packages/core/test/tool-execute.test.ts: two existing CodeModeTool.create call sites now pass the no-op guard the new third parameter requires, keeping their inventory object identity so the shared-runtime assertion still holds.

Verified on the rebased head: test/extensions.test.ts 28/28, the full codemode suite 1417 pass with only the pre-existing RegExp lastIndex failure, packages/core tool tests 142/142, tsc -b clean for packages/core and packages/codemode, oxlint 0 warnings and 0 errors.

I also corrected the commit message: the earlier wording said an Effect-returning global "runs in the caller's fiber", which #54276 made inaccurate for Promises and therefore for Effects — it is forked into the execution's scope, immediately.

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new commit addresses the cancellation point. fetch now returns an Effect that runs in the execute call's fiber, so interrupting execute also interrupts a permission prompt that is still waiting. The guard is read per call from the Run service rather than captured by the cached runtime, so a later execution can't reuse an earlier call's guard. The new interruption tests in packages/core and packages/codemode cover this.

I couldn't re-run the TUI reproduction on this head: in my sandbox, simulated tool calls (even a plain glob) don't run on the current base, so neither checkout could be checked end to end. The previous head passed the same reproduction (deny blocked the request, ask showed the WebFetch prompt). On this head the affected core tests (142), packages/codemode/test/extensions.test.ts (28) and both packages' type checks pass.

Public API: no change for users. The only new capability is that a Code Mode extension global may now return an Effect, which is internal and backwards compatible.

No new concerns.

Code Mode's `fetch` global reached the network without consulting the
`webfetch` permission, so a `"webfetch": "deny"` rule could be bypassed from
`execute`.

The check runs inside the `fetch` global, before the request leaves the process.
The `extension.before` hook cannot gate it: an extension's body runs before its
hook, so denying there would only deny the result.

`fetch` returns an Effect so the guard can resolve services from the execution
that started it. The Codemode runtime is cached per tool inventory while a
permission check belongs to one call, so a guard closed over by the runtime would
be shared by every later call; reading it from the runtime's `Run` service keeps
the runtime shared and the check per call. Extension globals may now return an Effect, forked into the execution's scope exactly like a returned
Promise, which is what makes that possible. It also makes `execute` wait on a permission prompt that a
finishing or interrupted call no longer needs: the interpreter interrupts the check with the program's
pending work instead of leaving it detached.
@argszero

Copy link
Copy Markdown
Contributor Author

Two notes on the new head 6b5f354456:

  • Comment correction only. The in-code comment still said an Effect global "runs in the caller's fiber". After fix(codemode): run each job to completion #54276 that is untrue even for the Promise path, which I matched by switching to the host fork — the comment now says it is forked into the execution's scope. No behaviour change: test/extensions.test.ts 28/28, oxlint 0/0, and the typecheck is unchanged from the previous head.
  • This file is being rewritten. refactor(codemode): run programs on one job queue instead of a fiber per async call #57976 replaces the turn/fiber model with one job queue and removes ctx.pending entirely, so this diff will conflict when it lands. I have told them where the Effect-global path belongs in runner and asked whether they want it at all; if they would rather fetch stay a Promise-returning global, the interpreter diff goes away completely and the guard is reworked instead. I would rather delete my part than have theirs carry it.

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only change since my last review is a comment in extensions.ts, which now describes a returned Effect as forked into the execution's scope. That matches the code, and no behaviour changed. My earlier point about the permission check running in a detached fiber was already addressed in the previous commit.

Public API: no change for users. The only new capability is that a Code Mode extension global may now return an Effect, which is internal and backwards compatible.

No new concerns.

@argszero

Copy link
Copy Markdown
Contributor Author

This PR's mechanism has been overtaken, so I would rather ask than keep polishing it.

It adds the permission gate for Code Mode's fetch, and it needs one small interpreter capability to do that: an extension global may return an Effect (forked into the execution's scope, exactly like a returned Promise), which is how fetch resolves the per-execution Run service instead of the runtime closing over a guard that belongs to one call.

#57976 rewrites that whole area — one job queue, ctx.pending gone — and its wrap has no Effect.isEffect branch, so the capability would disappear as a side effect of that rewrite.

So, one of these three, and I will do whichever you pick:

  1. Keep as is — you intend to carry the Effect-global path into runner (it is runner.newPromise() + runner.host(...); host tracks the fiber, so interrupt() runs the check's finalizers). I then rebase onto runner.host once refactor(codemode): run programs on one job queue instead of a fiber per async call #57976 lands and change nothing else.
  2. Permission gate only — you want fetch to stay a Promise-returning global. I drop the interpreter change entirely and rework the guard to need none, so this PR touches only packages/core/src/codemode/ and packages/codemode stays untouched. Smaller diff, no coordination with refactor(codemode): run programs on one job queue instead of a fiber per async call #57976.
  3. Withdraw — the direction is not wanted; close it and I will not reopen it.

If I hear nothing I will leave it as it is and stop touching it, since #57976 decides the shape either way. No action is needed from you today — a one-word answer is enough.

@argszero

Copy link
Copy Markdown
Contributor Author

Withdrawing this PR rather than keeping it parked indefinitely.

Why. When I opened it, the permission gate for Code Mode fetches was mine to place. Since then the mechanism around it has been rewritten upstream: packages/codemode/src/interpreter/extensions.ts — the file this PR edits — is the same file #57976 and #58031 both rewrite, and #57976 is the one that moves the whole thing onto a single job queue. Whatever lands there decides the shape this gate has to take, and I cannot know that shape in advance.

State of the review. It has never had a human review. The only reviews are from opencode-agent[bot], and the decision request I posted on 2026-10-22 (asking whether to rebase onto the runner.host shape, wait, or drop it) has not been answered. Keeping a third of my open-PR budget on an unreviewed PR whose foundation is being replaced is not a good use of it.

What was still useful. The gate itself is small and the tests are real: tool-execute.test.ts, tool-webfetch.test.ts, tool-registry.test.ts and tool-todo.test.ts all encode the invariant that an execute-tool fetch is gated by the same webfetch permission as a direct fetch, and they pass.

Offer. If a maintainer wants this once the Code Mode job-queue work settles, say so here and I will rebuild it on whatever runner.host shape #57976 ends up with, rather than on the ctx.pending path this PR targets. I am keeping the branch on my fork (codemode-fetch-permission) for that purpose, and I have not deleted anything.

@argszero argszero closed this Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

the execute tool should inherit webfetch permissions

1 participant