ref:main

harden(lfs-chunking): streaming O(n^2) re-scan, client whole-file memory, PUT chunk-size cap #70

closed Opened by cole.christensen@gmail.com

Links

No links yet.

Real findings from the adversarial review (#68), deferred as structural/perf (the security finding — commit_manifest/get not binding content to the file OID — is FIXED on feat/lfs-chunking). Verified, not hypothetical:

  1. Elixir Chunker.chunk_stream re-scans the whole buffer on every incoming piece (chunker.ex drain/do_cut). Fed small pieces, this is O(pieces x chunk_size) ~ quadratic; for a 1-byte-piece stream it blows up. Fix: carry the rolling FastCDC fingerprint + scan offset across drain calls (Stream.transform accumulator) so only new bytes are scanned. Correctness is covered by the streaming==whole property; this is the algorithmic perf fix (do NOT cap/skip — fix the re-scan).

  2. Go tools/fastcdc Chunker.Next has the same re-scan (less severe — 64KB fills) but should also carry fp+index across fill().

  3. Go tq/chunked_upload.go chunkFile holds the ENTIRE file in memory (descs retains every chunk’s bytes), contradicting the streaming design for the cited 100s-of-GB use case. Fix: stream chunk -> query -> upload missing -> drop bytes; keep only {oid,size,offset} and re-read missing chunks from the file by offset.

  4. No max chunk size on PUT /chunks/ — a malicious client can store an arbitrarily large single ‘chunk’ (valid sha) and get/verify loads it whole. Add a chunk-size ceiling (small multiple of Chunker @default_max) enforced while streaming the body, mirroring the :too_large pattern.

Each should be reproduced under a hard constraint, profiled if needed, and fixed at the algorithm/structure level per the perf protocol. Follow-up to #68.

colechristensen cole.christensen@gmail.com commented 2026-06-18 02:46

Resolved. All items fixed and verified (boundaries byte-identical: golden vector + 23-case cross-impl corpus + streaming==whole unchanged):

  • Elixir chunk_stream O(n^2) re-scan → incremental scan carrying fp+offset (commit ca9d76c).
  • Go fastcdc Next re-scan → incremental scanWindow (commit 249163ab).
  • Go chunked upload whole-file memory → streams; keeps {oid,size,offset}, re-reads missing chunks by offset (249163ab).
  • PUT chunk-size cap (:chunk_too_large, 8 MiB) + put chunk-count cap (:too_many_chunks) (ca9d76c). Chunked interop/compat E2E 5/5 green with the rewritten chunker + streaming upload.