[Security] The remote-media byte cap and redirect refusal have no real test #618

Closed
opened 2026-07-22 16:21:49 +00:00 by brenno · 0 comments
Owner

Found in the pre-publication test review.

Evidence: test/media_fetch_test.dart:53 is the only assertion in the test named for the byte cap:

expect(kMaxRemoteMediaBytes, greaterThan(0));

In the coverage report, lib/utils/media_fetch_io.dart:54 (request.followRedirects = false;), :61 (the Content-Length cap) and :65-70 (the counting cap) have zero hits. test/network_sink_guard_test.dart:128 only scans for RegExp(r"HttpClient\(") per file plus a count — the prose line at :132 about "no redirects plus byte cap" is a comment, not an assertion. Delete line 54 and nothing turns red.

SECURITY.md:311 makes this a public promise: "redirects (a 3xx must not walk around the host check), downloads are capped".

Why this matters now: the code is correct today, but on publication SECURITY.md becomes a promise to outsiders, and the first person to run grep followRedirects test/ sees that nothing stands behind it. The project already knows how to do this: for the CVE download the seam was carved out into GithubBulkTransport.streamCapped specifically "so the limit is reachable here" (test/cve_download_cap_test.dart:16-18).

Proposal: carve the same seam in media_fetch_io.dart — a readCapped(Stream<List<int>>, {maxBytes}) and an assertNoRedirect(HttpClientResponse) — and test those two directly. No server, no socket, three assertions.

Found in the pre-publication test review. **Evidence:** `test/media_fetch_test.dart:53` is the only assertion in the test named for the byte cap: expect(kMaxRemoteMediaBytes, greaterThan(0)); In the coverage report, `lib/utils/media_fetch_io.dart:54` (`request.followRedirects = false;`), `:61` (the Content-Length cap) and `:65-70` (the counting cap) have **zero hits**. `test/network_sink_guard_test.dart:128` only scans for `RegExp(r"HttpClient\(")` per file plus a count — the prose line at `:132` about "no redirects plus byte cap" is a comment, not an assertion. Delete line 54 and nothing turns red. `SECURITY.md:311` makes this a public promise: "redirects (a 3xx must not walk around the host check), downloads are capped". **Why this matters now:** the code is correct today, but on publication SECURITY.md becomes a promise to outsiders, and the first person to run `grep followRedirects test/` sees that nothing stands behind it. The project already knows how to do this: for the CVE download the seam was carved out into `GithubBulkTransport.streamCapped` specifically "so the limit is reachable here" (`test/cve_download_cap_test.dart:16-18`). **Proposal:** carve the same seam in `media_fetch_io.dart` — a `readCapped(Stream<List<int>>, {maxBytes})` and an `assertNoRedirect(HttpClientResponse)` — and test those two directly. No server, no socket, three assertions.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
LibreKAT/Ocideck#618
No description provided.