M69: web fetch on both backends, with M44b's network safety - #52
Open
RandyNorthrup wants to merge 14 commits into
Open
RandyNorthrup wants to merge 14 commits into
RandyNorthrup wants to merge 14 commits into
Conversation
PLAN.md D49 (folds in M44b), built to the plan review's six rules. - web_fetch on the Model API backend (a new `network` tool class): HTTPS only; every DNS answer checked against the non-public ranges (loopback, private, link-local, CGNAT, metadata, reserved; IPv4 inside IPv6 judged as IPv4); the connection pinned to a checked address through Node's https, which VS Code patches for its proxy and certificates (the proxy is asked to tunnel to the address; only a TLS answer is read). Same-host redirects checked and pinned again (5 at most), another host's handed back; 5 MiB after decompression, 30 s, text types only; HTML to Markdown in one linear pass (entities 8.1.0 decodes references). - Asks per host in Manual, Edit automatically and Auto; Bypass runs it; Plan and Restricted Mode refuse it. The page reaches the model between random markers as untrusted content. No billing. - Muse Code: webFetch on the ide server, listed only in a trusted workspace without sandboxNetwork restricted, annotated open-world and not read-only, with the extension's own modal before every call. - Row: the URL, the size and type, and what the model read; 23 strings in fifteen languages; harness scenario web-fetch. - Tests over a fake resolver and transport, a loopback transport test, an integration test inside VS Code 1.139.1 and 1.125.0 with a loopback proxy; 28 red drills; live: public pages (no model) and one Muse Code turn (4 model attempts, contributor model). - Docs: README (Web fetch, permission modes, privacy), PRIVACY, SECURITY, AGENTS layout, CONTRIBUTING (Networks), CHANGELOG, PLAN (M69 status, D3, M44b, §9), docs/certification/m69.md. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Every finding of the three class reviews of c3d7702, in one pass: - Muse Code's Stop reaches the ide webFetch call: IdeMcpServer aborts a call's signal when its request closes unanswered or notifications/cancelled names its id (both captured from Muse Code 1.4.0); the modal is raced against it, the offer is checked again after the answer, the fetch gets the signal, one modal per URL at a time. - Checked addresses are raced as RFC 8305 says (250 ms attempt delay, first TLS connection wins, the others stopped). - Failures in web fetch's own words (host, addresses tried), not M56's Meta advice; a proxy's tunnel refusal recognised by the transport's code; the detail names error codes only, never a certificate's names. - Server text outside the markers only as short tokens; the final URL, the title and a moved target inside them. - The HTML converter is bounded (100,000 characters, prefix depth 4, body rows unpadded, title source capped). - Every trailing dot stripped, empty labels refused. - RFC 7050 NAT64 prefix discovery; answers under it judged by their IPv4. - Damaged compression is the coding's failure; charset only from <meta>, an unknown label ignored. - Model text says "this tool"; Model API rows localized for a moved page and Restricted Mode; side chats are not offered web fetch. - sandboxNetwork described in fifteen manifest tables; docs narrowed where they overclaimed; PAC/noProxy seeing the pinned address documented. - The integration proxy tests share one setup and judge only the page's tunnels (VS Code's own requests may use the window's proxy). The full local gate did not finish under machine load; the record says which parts passed (all of quality:gates, secrets, SAST, a11y for the changed scenario) and leaves the full run to CI. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: b1ac17f0a8
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
PR #52 review (Codex): - NAT64 discovery asks the resolver the page's name used (getaddrinfo), and only its definite "no AAAA" (ENOTFOUND, or ENODATA) means no DNS64. A timeout, SERVFAIL or answers without an RFC 6052 prefix leave NAT64 unknown: no IPv6 answer is used, and a name or IPv6 literal with only IPv6 answers is refused as nat64Unknown (one new string, 14 tables). - A PermissionRequest hook's allow no longer replaces the per-host web fetch card; a hook may still deny or ask. Bypass and session-allowed hosts run without a card, as before. Muse Code's ide webFetch always asks in the extension's own modal and needed no change. - Swept the other failed-lookup and failed-check paths: none allows. Drills R32-R35 red and restored. Touched suites and the fast gates only (machine overloaded); the full gate is CI's. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec34f25fc6
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
PR #52 second review (Codex): on the Model API backend, trust revoked while the card or a PermissionRequest hook was pending did not stop the fetch. - After the card or hook resolves, the turn, trust and the mode are asked again before the fetch (a stopped turn cancels; Restricted Mode and a mode that now refuses refuse). - The fetch takes an isStillAllowed check, asked before each hop's lookup and connection, redirects included; a failed check ends it as "withdrawn" (one new string, 14 tables). - Once the page is in, it reaches the model only while fetch is still allowed. - Muse Code's ide webFetch passes its offer (trust, sandboxNetwork) as that check and asks it, with the stop, once the page is in. Swept every await on both paths. Drills R36-R43 red and restored. Touched suites and fast gates run; the full gate runs after this commit. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ad1815540
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
PR #52 third review (Codex): - getaddrinfo's ENOTFOUND can stand for other lookup failures, so it no longer proves "no DNS64". Discovery asks the system resolver and a DNS query of its own (c-ares resolve6) together: a prefix comes from either one's answers; with none, only the DNS query's NXDOMAIN or NODATA means no NAT64. Everything else leaves it unknown and IPv6 answers unused. - The converter now hides elements HTML closes without an end tag (<p>, <li>, <dt>, <dd>, cells, rows, ...) up to where a browser ends them: a closing start tag unless something open inside keeps them open, their own end tag, or the end tag of an element open around them. Counted open-element tracking keeps hostile pages linear. - Sweep: a hidden image's alt text, a self-closed hidden element, an unopened dialog, rp and datalist are left out too; no other error in the fetch path maps to an allowing verdict. Drills R44-R52 red and restored. Touched suites and fast gates run; the full gate runs after this commit. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 569dd869a0
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
PR #52 fourth review (Codex): a self-closed <template/> let its content through, the third converter finding in a row. This round audits the converter against the HTML parsing algorithm and follows it for everything that decides what is hidden or left out. - One stack of open elements with the algorithm's scopes, special and formatting elements, implied ends, a reduced adoption agency (a hidden formatting element reopens until its own end tag), </form>, foreign content (breakouts, integration points, CDATA), select, tables, headings and <body>/<html> attribute merging. Positions are kept per name and per tracked set, so every check is constant time and hostile pages stay linear. - Tokenizer: comments end at <!-->, <!--->, --> or --!>; </ before a non-letter is a bogus comment; CDATA outside foreign content is one; scripts follow the escape states; a slash counts only right before > and only on void and foreign elements; attribute names may begin with =. - parse5 8.0.1 (already in the tree) was measured and not adopted: it is quadratic on hostile nesting (40,000 nested lists in 73 s against 127 ms here). No dependency changes. Drills R53-R71 red and restored. Touched suites and fast gates run; the full gate runs after this commit. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ef2eabeda
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Conflicts kept both sides: the README settings table keeps web fetch's sandboxNetwork row and takes main's environmentVariables row; the certification index lists M69 and sign-in detection. en.ts and the manifest tables merged on their own. Typecheck, check:l10n (0 problems) and the unit suite (2,837 passed) ran on the merge. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The fifth review found three more gaps in the hand-written HTML path (<base href> ignored, the charset sniff reading <meta> in comments and other attributes, display:/**/none passing the hidden check): the fourth round, so it is redesigned rather than patched. - parse5 8.0.1 parses the page with the HTML standard's algorithm; the converter walks its tree with the existing Markdown writer, resolves links against the first <base href>, and leaves out what HTML hides (hidden, inert, aria-hidden, a dialog not opened, rp, template, noscript) and inline styles that hide, read with @csstools/css-tokenizer 4.0.1 (comments, escapes, case, !important, invalid later values). - html-encoding-sniffer 6.0.0 decodes a page as HTML does (BOM, header, the standard's <meta> prescan), UTF-8 by default. - parse5 is quadratic on hostile nesting, so the converter runs in its own bundle, dist/pageWorker.js (212 KiB, budget 300), on a worker thread started at the first page and stopped at 10 s or 512 MiB; such a page, or a worker that cannot start, is refused with the reason (three strings, 14 tables). The bundle-split gate keeps it off dist/extension.js (473 KiB, down from 490). - Dependencies pinned exactly, already in the lockfile, MIT, audit clean; entities is no longer direct. Node 20: the production worker ran on a portable Node 20.18.3. README and SECURITY say exactly what is left out and that text a stylesheet hides still reaches the model, marked untrusted. Drills P1-P16 red and restored. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ntent) Two class reviewers and Grok Build over a49d373: - Encodings: the sniffer's crash on a malformed <meta> content is read past; XHTML is sniffed as XML; a later <meta> changes a tentative encoding (the standard's reparse); x-user-defined and replacement are decoded by the Encoding standard's own definitions (Node 20.18 has no decoder for the first, no Node for the second); any other encoding a runtime cannot decode refuses the page ("undecodable"), never read as UTF-8; a text page's byte order mark decides first; the header's charset is a MIME parameter. - Workers: at most two conversions at once, the wait honouring the fetch's signal; a deadline passing during conversion is named as the conversion's; failure details are short codes; crashes are logged by name, code and frames, never the message; the tests prove the worker thread ends. - Hidden content: popover, closed <details> (all but its summary) and declarative shadow roots (children in their slots) read as they render; the title only from <head>; inline display by its real grammar, a bracket stack, and var() on a hiding property counted as hiding. - Shipping: the .vsix check requires dist/pageWorker.js; an integration test converts through it in VS Code. - Docs: a worker per page, the developer rows and layout. Two strings (one new, one reworded) in 14 tables. Drills Q1-Q19, R1-R6. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The Mac mini rig failed two htmlToMarkdown tests on 02e8ca1: their 5 s limits, written for the hand-written converter, took 13.6 and 17.5 s on parse5 there. On parse5 that time is the page worker's to bound (tested in pageConverter.test.ts); the pure converter's tests now check the output bound only, on inputs still past it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19f843f33b
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
PR #52 review of 19f843f (Codex): - XHTML went through parse5's HTML parser, which misreads its XML syntax (<script src="x"/> swallowed the text after it). No XML parser is bundled, so application/xhtml+xml is refused with its own reason (one string, 14 tables), dropped from the Accept header, and the XML sniffing path is removed. - visibility is inherited: a hidden ancestor no longer drops its subtree. The walk carries the inherited value; a descendant with visibility: visible shows, an invisible element keeps its tags but writes no text or void element. display:none and content-visibility:hidden still take everything; swept: visibility was the only inherited hiding. Drills V1-V6 red and restored. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83eedf262e
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex found a <col style="visibility:collapse"> whose cells the converter still wrote: the third round on the converter's hiding, so by the owner's rule the area is redesigned rather than patched. Hiding cannot be worked out completely without being a browser, and the attempt protects nothing (the same words fit in visible small print); the untrusted markers around everything a page returns are the defence. The converter now leaves out only what its structure never makes page text (the head, scripts, styles, template content, noscript, frames and media, form controls, SVG, MathML) and reads no CSS and no hiding attribute, hidden included, for one consistent rule. A declarative shadow root's content is written where its template stands. inlineStyle.ts and @csstools/css-tokenizer are removed (pageWorker.js 216.8 -> 201.2 KiB). The tool description and the model's notice say the result is the page's text as served, which can include text a browser would not show, all of it untrusted; README, SECURITY, PRIVACY, PLAN, CHANGELOG and the certification say the same, with red drills W1 to W9. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Owner
Author
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This branch has not been deployed
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
1 participant
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.
What
M69 (PLAN D49, folds in M44b): the model can read a web page on both backends.
web_fetchtool.mcp__ide__webFetchon the extension'sideserver (Muse Code's ownweb_fetchis switched off).Network safety
CONNECT <address>:443, TLS SNI = host). Candidates race per RFC 8305. PAC/noProxy rules see the IP; that is documented as a deliberate trade-off (sending the name would let the proxy resolve it again).Permissions
A new
networktool class: Bypass runs it, Plan refuses it, Manual / Edit automatically / Auto ask per host. Restricted Mode refuses. On Muse Code the tool is listed only in a trusted workspace wheresandboxNetworkis notrestricted, declaresreadOnlyHint: false, openWorldHint: true, and the extension asks in its own dialog before every call; a Muse Code Stop cancels the call (captured live).Review
Built, then reviewed in three parallel classes (concurrency/lifecycle; wire/security; failure paths/docs); every finding fixed in b1ac17f or answered with evidence in
docs/certification/m69.md.Gate
quality:gatesexited 0 locally (format, lint, typecheck, l10n, knip, dpdm, jscpd 0 clones, 2,629 unit tests, build and budgets, audit). The full locala11yrun did not complete (headless Chrome crashed under machine load); the affected and new scenarios passed separately, as did gitleaks and semgrep. CI's three-OS run is the full gate. 59 red drills recorded.Live: Muse Code 1.4.0 on the contributor model, 8 model attempts in all (listing and calling, then the cancel captures).
Owner questions left open (in the cert record): an enterprise off switch; whether Muse Code's MCP "always allow" should silence the extension's dialog; allowing fetches in Plan mode.
🤖 Generated with Claude Code