Repository navigation
fix(core): apply webfetch permissions to Code Mode fetch - #59952
Closed
rekram1-node wants to merge 1 commit into
Closed
rekram1-node wants to merge 1 commit into
rekram1-node wants to merge 1 commit into
Conversation
The execute (Code Mode) tool exposes a `fetch` global that went straight to the network. The webfetch tool asks the `webfetch` permission for every URL, but Code Mode did not, so a `webfetch: deny` rule or a Console "Can't fetch the web" policy did nothing against a model that used `execute` to send data to any server. - core: the tool registry now asserts the `webfetch` permission (resource = the URL, same shape as the webfetch tool) in the extension.before hook of every fetch a Code Mode program makes. A denied URL throws inside the program and its progress row settles as an error. Tool.node now depends on Permission.node. - codemode: extension globals were called before their `extension.before` hook ran, so a failing hook only hid the result while the request had already been sent. The hook now runs first and a denied call never reaches the host function. A denied async global still rejects its promise; any other denied global throws. Verified: new cases in codemode/test/codemode.test.ts (denied host function is never called) and core/test/tool-execute.test.ts (webfetch deny rule fails the script's fetch, allow rule lets it through, each URL is asserted) both go red without the fix. bun typecheck in core and codemode, codemode suite (1415 pass), core suite (only pre-existing local failures: ripgrep download and macOS bash 3.2 shell tests, also failing on dev), bun run check.
Contributor
Author
|
Opened against the wrong branch by mistake, closing. |
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.
Stacked on #59948. Merge that first.
Problem
fetchinside Code Mode never checked permissions. A URL someone denied forwebfetchcould still be fetched from a Code Mode program, and an "ask" rule never asked.Change
fetchis now an Effect global (from the PR this is stacked on). Before sending, it asks the samewebfetchpermission as the tool, for its URL, on behalf of theexecutecall it runs under. Code Mode itself still knows nothing about permissions. Core owns the check, as it does for tools.CodeModeExecutionis a small new service holding theexecutecall's context, so future globals (like anfsmodule) can use the same check as their tools. The Tool layer now depends on Permission so it can hand it to the execute tool.A denied URL is never requested. The program catches the permission error like any other rejected fetch.
Visual
Out of scope
fetchstill follows redirects without checking each hop, while thewebfetchtool checks every hop. I'll do that in a follow-up PR.fsextension.Verification
bun test test/tool-execute.test.tsinpackages/core: new tests check that fetch askswebfetchfor its URL with the execute call's session and call id, and that a denied URL never reaches a local server.bun run testinpackages/core: 6900 pass. The 7 failures are shell tests that also fail ondevlocally.bun run checkfrom the root: clean.