[Burntgogi]

Scope

Two network-input boundaries, rebased onto upstream f956b65ff0aac6b8eebfb69bdabb5185e84beafc:

• Authenticate parsed HTTP heads before 100 Continue, body ingestion or body-buffer allocation. Public health/OPTIONS requests do not ingest unused bodies and close when a body is declared.
• Check decoded JPEG/PNG sizes before caller output allocation and account retained RGB against one 64 MiB request budget across chat, batched embeddings and Clef image batches, before model preparation. Trusted offline decoders and the encoded-video pipeline retain their existing policies.

This is not a total process-memory limit: compressed data, decoder internals, conversions and model preparation have additional costs. Please review the network decoded-image policy/limit before landing.

Concurrent upstream work preserved

The existing arithmetic-overflow fixes and HTTP body limit are retained. #573 landed during review; this branch retains its control-token guard/BPE coverage and tokenizer dev dependency. Concurrent V4.1 request-preparation changes were assessed separately and are not replaced by this input-boundary patch. The fork's main and upstream main were not changed by this work.

Review follow-up (2026-10-21)

• API and decision socket tests now share one bounded fixed-length/interim response reader in HTTP.
• The original raw-spawn correction introduced a Linux-only test dependency, which made serve leave the Mac-pure set and failed two recipe units in the next T0 run. The bounded decision socket fixture now writes its small request, invokes the handler and reads the response on the same thread. Denied requests still withhold their declared body and require a final response without interim 100 Continue. Remove the helper dependency instead of changing architecture allowlists, the purity derivation or expected crate lists.
• Capacity refusal is tested with identical valid inputs at required-length minus-one/exact/plus-one, typed VisionError::Limits below projector capacity, and exact tokens, offsets, keys and prepared outputs at accepted boundaries. Diagnostic English is not pinned.
• Dated removal rationale: the old prepare-first-versus-malformed-second-frame clause contradicted request-wide decoding before preparation. all_inline_images_decode_before_any_model_prepare and bulk_images_share_one_rgb_budget_and_preserve_error_indices preserve the actual safety invariant; the capacity gates were not replaced by generic invalid-input cases.

Observed verification

Pinned nightly-2026-08-28, x86_64 Linux, RUSTFLAGS='-C target-cpu=native':

• CLI build passed; serve/vision host suites on the rebased tree: 519 passed, 197 ignored, 19 suites.
• check-arch and pinned formatting passed. The real purity derivation again includes bloomery-serve, without modifying its rule or copying new expected lists.
• Repository-pinned just 1.58.0 (checksum verified), with the pinned Rust toolchain: recipes.py --self-test passed all 208 gate recipes with zero failures after removing the dependency.
• Published follow-up d3c938f59996d8e21ce685bd20d078392d76acc6: upstream T0 CI passed; the latest automated review is APPROVED / CLEAN on that exact head. These are host/automated-review results, not GPU landing evidence.
• Actual loopback HTTP: wrong key receives 401 without sending a body or receiving interim 100; authorized Expect/body completion succeeds; health/OPTIONS ignore declared bodies and close; valid small PNG chat succeeds, over-budget headers fail with 400 before the large output allocation.
• Actual Clef host helper accepts ordered small image spans and reports the second over-budget input as images[1] before preparation of the batch.

Missing landing evidence

No CUDA build or GPU gate was run. The contributor's RTX 3070 Ti has an existing resident workload and the required CUDA build toolchain is absent; no workload was stopped. The Clef adapter remains needs-gpu: these are host-tier results, not card landing evidence. Maintainer/card gates and normal review are required. No inference-quality or performance claim is made.

[github-actions]

Verdict: MUST_FIX: The tests duplicate response parsing and remove refusal clauses without a dated reason.

Must fix

1. Keep one test response parser (kind B, R14)

crates/serve/src/api.rs:5678 and crates/serve/src/decide.rs:1511 independently parse the same response headers, enforce the same buffer limit and read the same fixed-length body.
Share this now so future framing changes have one owner. Moving the parser into the existing HTTP module removes one function and repeated parsing code.

Delete both local parsers. Put this definition under #[cfg(test)] in crates/serve/src/http.rs, with a doc sentence describing the test response reader:

pub(crate) fn response(r: &mut impl BufRead) -> (u16, String, Vec<u8>) {
    let mut head = String::new();
    loop {
        let mut line = String::new();
        assert_ne!(r.read_line(&mut line).expect("response head"), 0, "{head}");
        head.push_str(&line);
        if line == "\r\n" {
            break;
        }
    }
    let status = head.split_whitespace().nth(1).unwrap().parse().unwrap();
    let len = if status == 100 {
        0
    } else {
        head.lines()
            .find_map(|line| line.strip_prefix("Content-Length: "))
            .expect("a fixed-length response")
            .parse::<usize>()
            .expect("the response length")
    };
    assert!(len < 64 * 1024, "a bounded test response: {len}");
    let mut body = vec![0; len];
    r.read_exact(&mut body).expect("response body");
    (status, head, body)
}

In the API test, import crate::http::response and remove BufRead from its local imports. Replace the decision test's let response = written(&client); through its final response expression with:

let (status, head, body) =
    http::response(&mut std::io::BufReader::new(&client));
assert!(head.contains("Connection: close"), "{head}");
handled.join().unwrap();
(status, String::from_utf8(body).expect("a UTF-8 response"))

2. Preserve named refusals and record the removed ordering clause (kind B)

crates/serve/src/arch/gemma4v.rs:90 and crates/serve/src/arch/gemma4v.rs:105 replace named context/projector refusals with any 400. The malformed-second-frame case is also removed without a dated reason. A mutant returning an unrelated 400 now passes these assertions.
Restore the clauses now so later regressions cannot hide behind the status code. Keep the existing status assertions and add, respectively:

assert_eq!(error.message, "image input exceeds the embedding context");

assert!(error.message.contains("loaded projector token capacity"));
// PIN(2026-10-21): All images decode before preparation; the old prepare-first case contradicts this policy.

Checked

Duplicates: both cited response readers share header parsing, the buffer limit and fixed-length body reading. Dryer reported zero pairs and excluded gpu-gates.
Complexity: keep_alive rose 9 → 16 through authorization and unread-body branches.
Silent failure: inline-image decoding errors propagate through invalid(...) as 400 responses.
Tests, static mutation audit: replacing the context/projector error with an unrelated 400 leaves the changed Gemma assertions green. Both named refusal paths still exist.
Gates: refusal clauses and the ordering case removed without a dated reason; no numeric pins moved.
Escape catalog: no C1–C4 patterns matched.

Notes

Tests were not run during this static validation. The PR supplies no FAIL-first transcript or GPU gate evidence.

_{Card check: needs-gpu. This pull request changes crates/gpu-gates/src/bin/shared/clef_seat.rs and carries no gate run on a card (CONTRIBUTING's evidence list); a maintainer adds gpu-verified after one.}

_{🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)}

[github-actions]

Keep the named context refusal; a status-only assertion accepts unrelated 400 errors.

Suggested change

	assert_eq!(error.code, 400);
	assert_eq!(error.code, 400);
	assert_eq!(error.message, "image input exceeds the embedding context");

[Burntgogi]

Coordination note: a separate Burntgogi session is handling the #590 review/CI follow-ups. This session will not edit or push fix/network-input-budgets. Its planned order is #587 CI performance, then #591, then the two remaining #524 host/tool items. #591 may touch crates/serve/src/api.rs and serving documentation, so it will stay on a separate branch and reuse the current shared request-preparation path; integration will be checked against #590 and #595 rather than copying their changes. The currently published #590 head observed here is 2e9adf4; no assumption is made about the other session's unpublished work.

[github-actions]

Since the last review:

• Response parsing: fixed. Both socket tests use http.rs:339.
• Refusal coverage: fixed. Identical valid inputs exercise rejected and accepted boundaries, projector refusals require VisionError::Limits, and the PR body gives a dated reason for removing the old ordering clause.

Verdict: CLEAN: Both earlier findings are resolved, and no must-fix finding remains in the new commit.

Checked

Duplicates: 2 new named items checked; the shared response reader replaces both local parsers. Dryer reported 0 pairs; it excluded gpu-gates, whose patch is unchanged in this re-review.
Complexity: keep_alive 9 → 16 adds authorization and unread-body branches already reviewed. The Gemma input test 17 → 23 adds boundary cases and output comparisons; no new production decision.
Silent failure: 1 new status-parse .ok() site followed; its expect fails the test on invalid input. Missing lengths and truncated responses also fail explicitly.
Tests, static mutation audit: changing the context comparison at request.rs:173 from > to >= fails exact-context acceptance. Dropping the projector refusal at media.rs:253 fails typed refusal assertions at capacities 71 and 120. Moving body reading ahead of authorization at api.rs:1165 fails the socket status assertions.
Gates: no numeric pins moved; the removed ordering clause has a dated rationale. Accepted boundaries compare tokens, offsets, image keys and prepared outputs.
Escape catalog: no C1–C4 patterns matched.

Notes

Tests were not run during this static review. The PR reports host results but supplies no FAIL-first transcript or GPU gate evidence.

_{Card check: needs-gpu. This pull request changes crates/gpu-gates/src/bin/shared/clef_seat.rs and carries no gate run on a card (CONTRIBUTING's evidence list); a maintainer adds gpu-verified after one.}

_{🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)}

[github-actions]

Since the last review:

• Response parsing remains fixed; the shared response reader is unchanged.
• Refusal coverage remains fixed; the boundary assertions are unchanged.

Verdict: CLEAN: The incremental change removes the helper dependency and preserves the socket test's authorization checks.

Checked

Duplicates: 0 new named items; no remaining threads:: references in serve. Dryer reported 0 pairs. Its gpu-gates exclusion concerns code unchanged in this update.
Complexity: crapper's keep_alive 9 → 16 adds authorization and unread-body branches; the Gemma input test 17 → 23 adds boundary assertions. Both changes were already reviewed. This update removes a helper closure.
Silent failure: 0 new sites. Socket writes and response parsing still fail explicitly on errors.
Tests, static mutation audit: moving body reading before authorization at api.rs:1165 sends an interim 100 Continue; after the server read timeout, decide.rs:1514 rejects that response. Removing authorization also fails the 401 assertions. Valid requests supply their complete body and request closure, so synchronous handling needs no client progress.
Gates: no thresholds, assertions or cases removed or relaxed in this update.
Escape catalog: no C1–C4 patterns matched. No production hot path changes.

Notes

Tests were not rerun during this static review. The PR reports host verification but supplies no FAIL-first transcript or GPU gate evidence.

_{Card check: needs-gpu. This pull request changes crates/gpu-gates/src/bin/shared/clef_seat.rs and carries no gate run on a card (CONTRIBUTING's evidence list); a maintainer adds gpu-verified after one.}

_{🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)}

[Burntgogi]

Read-only coordination/integration check complete at the other session's published head d3c938f: T0 is SUCCESS, review APPROVED, mergeable. I did not edit or push this branch.

In a new detached temporary worktree, #603 (ebcb327, based on main including #595) merged with #590 without conflicts in api.rs or docs/serving.md. The real-BPE guarded-ID regression passed on the combined tree. An actual combined MockEngine HTTP listener with a smoke-only API key returned 401 to an unauthorized request head without waiting for its declared body, then served authorized opt-in prompt IDs with a detokenization roundtrip and orderly shutdown. The temporary merge/worktree was removed after verification. No GPU inference or merger of either PR is claimed. This account has READ permission; landing remains with a maintainer/the owning session.

[Burntgogi]

Fresh shared-upstream compatibility check; PR head remains d3c938f59996d8e21ce685bd20d078392d76acc6 (no rebase, commit or push).

• Reviewed the 22 commits from the reviewed base f956b65ff0aac6b8eebfb69bdabb5185e84beafc through live main 5b626c3d6cf86ada4bc75dfb3f2442383c971358. Overlap is the API's family request preparation/guarded prompt ids and serving documentation, not a duplicate of the auth-before-body or decoded-image budget change.
• The observed GitHub merge ref a9549592a1e679cc2ff462c943a00f7968bc38d1 still used base 938167b2997b48b60593c27956efd36b352a8fad. For fresh evidence I locally merged the unchanged PR head with 5b626c3d6cf86ada4bc75dfb3f2442383c971358 without committing; it merged cleanly, tree dc5cf1b74a331c2a692d85a3bb60e173d28768dd. No shared upstream code/ref was written.
• Built the actual host bloomery-serve binary, then ran the serve/vision host verification: 525 passed, 19 suites, 197 repository-ignored cases. The first attempt used a custom target directory without the bare CLI at the fixture's required target/debug/bloomery-serve; I fixed that prerequisite by building the real binary in the expected target, not by editing/skipping a test or accepting the error.
• Independently launched that built server on loopback: missing-key headers received 401 without sending the announced body or receiving 100; correct bearer headers received 100 then an actual 200 completion; public health returned 200 without an unused announced body/100; authenticated malformed JSON returned 400. Captured replies/startup/logs contained no probe key, and the owned server exited.

This is host/network integration evidence, not CUDA/Clef GPU execution, model accuracy or performance proof. needs-gpu remains; upstream merge/deployment and required device evidence stay with the maintainers. The existing exact-head T0 result is not relabelled as a new live-base remote run.

[juliankang4]

@Burntgogi Thanks for the contribution. The review is done, so I'm merging this.