fix: memory-ingest fetch/read boundary hardening (iss-30 subset) - #38
Merged
Conversation
Addresses the fetch/read-boundary subset of iss-30 (parser-parity and partial-failure-reporting instances remain a tracked follow-up; see DECISIONS.md): - C12: defaultFetch read the response body without checking the HTTP status, so a 404/500 error page was stored as source content. A non-2xx response is now an ingest error (logic extracted to readFetchedResponse and unit-tested). - P11: blockedFetchIP missed NAT64 (64:ff9b::/96) and 6to4 (2002::/16) IPv6 addresses embedding an IPv4 metadata/loopback/private destination. It now extracts and re-checks the embedded v4 (scoped to the well-known prefixes). - C13: materialFromLocal read a local file with no size cap. It now rejects an over-cap file before reading it whole into RAM (test asserts the size-cap message so it fails on revert, not the downstream NUL check). - tilde: a ~user path was mangled into home+"user"; only ~ or ~/ expands now. Reviewed: security PASS, ruthless FIX-FIRST (a false-pin size test) fixed + doc nit (embeddedIPv4 well-known-prefix scope) applied. Assisted-by: Claude:claude-opus-4-8
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.
Addresses the fetch/read-boundary subset of iss-30 (the memory-ingest input-boundary cluster). iss-30 stays open for its remaining instances — see below.
Fixes (
internal/core/memory)defaultFetchread the response body without checking the status, so a 404/500 error page was stored as source content. A non-2xx response is now an ingest error (logic extracted to a unit-testedreadFetchedResponse).blockedFetchIPmissed NAT64 (64:ff9b::/96) and 6to4 (2002::/16) IPv6 addresses embedding an IPv4 metadata/loopback/private destination; it now extracts and re-checks the embedded v4 (scoped to the well-known prefixes; both the DNS-resolution and dial-time guard paths hit it).materialFromLocalread a local file with no cap; it now rejects an over-cap file before reading it whole into RAM.~userpath was mangled intohome+user; only~/~/…expands now.Tests (TDD, each fails on revert — verified)
Non-2xx status matrix (synthetic response), NAT64/6to4 SSRF cases added to the existing guard test, over-cap local file (asserts the size-cap message, not the downstream NUL check), and the tilde
~/vs~userbehaviour.make preflight+record-lintgreen.Reviewed
security-reviewer PASS (P11 closes the well-known transition vectors, verified by execution; C12/C13/tilde correct; no guard regressed). ruthless-reviewer FIX-FIRST — caught that the size test was a false pin (a sparse file trips the NUL check regardless); fixed to assert the cap message. Both NICE-TO-HAVEs applied.
iss-30 remaining (tracked, DECISIONS.md)
The larger "ingest test-suite" instances stay open: the
--keep-originalpartial-failure reporting, CRLF parser-parity (parseFrontmattervssplitFileFrontmatter), and broader URL-ingest/content-type/PDF path coverage.Assisted-by: Claude:claude-opus-4-8