fix: harden web fetch HTML conversion [AI]#102033
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 8, 2026, 9:19 AM ET / 13:19 UTC. Summary PR surface: Source +546, Tests +260. Total +806 across 2 files. Reproducibility: Source-reproducible: current main uses broad regex replacements over untrusted HTML in the active raw-html fallback path, and the contributor transcript exercises the production fallback adapter after the fix. I did not establish a full current-main user-path timing failure or full live-network raw-html fallback repro. Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Land the bounded scanner after maintainer acceptance of the fallback-output and SDK-helper compatibility tradeoff, with final coordination against the adjacent title-only extraction PR. Do we have a high-confidence way to reproduce the issue? Source-reproducible: current main uses broad regex replacements over untrusted HTML in the active raw-html fallback path, and the contributor transcript exercises the production fallback adapter after the fix. I did not establish a full current-main user-path timing failure or full live-network raw-html fallback repro. Is this the best way to solve the issue? Yes with maintainer sign-off: the helper-boundary scanner is a maintainable hardening path without a new dependency and prior review edge cases are covered. The remaining issue is compatibility acceptance for changed fallback and SDK helper output, not a concrete code defect. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against e25fa79c5d19. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +546, Tests +260. Total +806 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (21 earlier review cycles; latest 8 shown)
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Current-head behavioral proof for PR #102033. Head: Protected source/sink: network-controlled HTML fetched by Proof entry point used: Protected sink/source result: the sensitive hidden strings Package script check before relying on this proof: inspected package scripts for Terminal proof command: node --import tsx - <<'EOF'
import { extractBasicHtmlContent } from './src/agents/tools/web-fetch-utils.ts';
const head = '87ebf94c37c153d686489dc01100b3fe3f0c501c';
const cases = [
{
name: 'ordinary',
html: `<!doctype html><html><head><title>Proof Ordinary</title></head><body><h1>Ordinary Heading</h1><p>Benign paragraph.</p><a href="/docs">Docs</a></body></html>`,
},
{
name: 'malformed',
html: `<!doctype html><html><head><title>Malformed Proof</title></head><body><p>Visible start</p><script>HIDDEN_SCRIPT</script a=">INJECTED PROMPT"><p>Visible after script</p><a href="/p"><h3>Card</a><p>Visible after link</p><h1>Head <a href=/x>link</h1><p>Final body</p></body></html>`,
},
];
console.log(`head=${head}`);
console.log('entrypoint=extractBasicHtmlContent, the production web_fetch raw-html fallback called by src/agents/tools/web-fetch.ts after fetch/readability fallback');
console.log('live_gap=full web_fetch network proof was not available: loopback server was blocked by SSRF and deterministic fetch transport is bypassed by runtime undici unless marked as test fetch; with that hook, Readability handled these pages before raw-html fallback');
for (const item of cases) {
const started = performance.now();
const result = await extractBasicHtmlContent({ html: item.html, extractMode: 'markdown' });
const tookMs = Math.round(performance.now() - started);
const text = result?.text ?? '';
const hiddenLeak = /HIDDEN_SCRIPT|INJECTED PROMPT/.test(text);
console.log(`case=${item.name} tookMs=${tookMs} title=${JSON.stringify(result?.title ?? '')} hiddenLeak=${hiddenLeak}`);
console.log(`text=${JSON.stringify(text.slice(0, 500))}`);
}
EOFTerminal proof output: Exploit/denied/fixed scenario: the malformed HTML includes browser-hidden raw text markers Positive control: the ordinary HTML page keeps intended title, heading, paragraph, and link rendering: Validation for this head: Known proof gap: this is a local runtime transcript for the production raw-html fallback adapter, not a live internet fetch of an attacker-controlled public page. The full |
|
@clawsweeper re-review |
|
Current-head verification for What changed:
Proof and gates:
Behavior proof:
|
* fix: harden web fetch html conversion * fix: consume malformed html tag tails * fix: consume invalid html tag spans * fix: preserve html scanner edge cases * fix: preserve title-only html fallback * fix: harden html scanner malformed tokens * fix: close html scanner review gaps * fix: keep malformed html raw text hidden * fix: keep html scanner comment content hidden * fix: skip unsupported html attribute values * fix: skip raw text from opener end * fix: drop malformed html tag tails * fix: preserve trailing slash href links * fix: close html scanner review gaps * fix: preserve literal less-than text * fix: bound html render context nesting * fix: drop bogus html closing tags * fix: handle abrupt html comments * fix: remove dead html scanner branches * fix: close quoted self-closing html tags * fix: track anchor text incrementally * fix: ignore quoted raw text markers * fix: close title contexts through literal markup * fix: close anchors through nested contexts * fix: close nested HTML contexts consistently
* fix: harden web fetch html conversion * fix: consume malformed html tag tails * fix: consume invalid html tag spans * fix: preserve html scanner edge cases * fix: preserve title-only html fallback * fix: harden html scanner malformed tokens * fix: close html scanner review gaps * fix: keep malformed html raw text hidden * fix: keep html scanner comment content hidden * fix: skip unsupported html attribute values * fix: skip raw text from opener end * fix: drop malformed html tag tails * fix: preserve trailing slash href links * fix: close html scanner review gaps * fix: preserve literal less-than text * fix: bound html render context nesting * fix: drop bogus html closing tags * fix: handle abrupt html comments * fix: remove dead html scanner branches * fix: close quoted self-closing html tags * fix: track anchor text incrementally * fix: ignore quoted raw text markers * fix: close title contexts through literal markup * fix: close anchors through nested contexts * fix: close nested HTML contexts consistently
What Problem This Solves
Fixes an issue where users fetching HTML content with
web_fetchcould experience excessive processing time when a page contains adversarial or malformed markup.AI-assisted: yes.
Why This Change Was Made
The HTML-to-Markdown fallback now uses a forward-only scanner for tag handling instead of broad wildcard replacements. It keeps the existing entity decoding contract, preserves basic titles, links, headings, and list rendering, and drops script/style/noscript raw-text blocks without adding a new dependency.
User Impact
Users can continue fetching ordinary HTML pages, while malformed raw HTML is handled more predictably and without the vulnerable replacement patterns. Provider, plugin, auth, config, storage, and approval behavior are unchanged.
Evidence
git diff --checknode scripts/run-vitest.mjs src/agents/tools/web-fetch-utils.test.ts src/agents/tools/web-fetch.test.ts(22 tests passed)