Skip to content

fix(#2871): Files.load size cap + FileBlobStore unique tmp + BlobStor…#92

Merged
Skobeltsyn merged 1 commit into
mainfrom
fix/2871-files-blobstore-hardening
May 30, 2026
Merged

fix(#2871): Files.load size cap + FileBlobStore unique tmp + BlobStor…#92
Skobeltsyn merged 1 commit into
mainfrom
fix/2871-files-blobstore-hardening

Conversation

@Skobeltsyn
Copy link
Copy Markdown
Contributor

…e.verify

ChatGPT audit (#2865) P1 — blob/file operational hardening. Three small, independent surfaces, fixed together.

Files.load size cap

  • New maxBytes: Long = DEFAULT_MAX_BYTES parameter on Files.load, loadOrNull, loadAll, loadAllOrSkip. Default 20 MiB.
  • Size check via Files.size(path) BEFORE bytes are read into memory — a 4 GiB upload fails-fast without OOMing the JVM, instead of being slurped into a byte array first.
  • New OversizedFileException(path, sizeBytes, maxBytes) names all three values in the message so a misconfigured callsite is debuggable.
  • loadAllOrSkip deliberately does NOT swallow OversizedFileException — it's a fail-fast safety check, not a soft predicate. Document via the KDoc + suggest pre-filtering or a smaller maxBytes.

FileBlobStore unique tmp filename

  • Pre-#2871 the put path used dir.resolve("$hash.tmp"). Two threads putting the SAME hash (rare but valid — same bytes computed independently) would both write to the same tmp filename, race on truncate/rename. The final atomic rename works for the WINNING thread, but a loser could rename its partial file over the winner's good copy.
  • Now: dir.resolve("$hash.${UUID.randomUUID()}.tmp"). Per-attempt unique tmp filename, no collision possible. Atomic rename still works — target is keyed on hash, so the second rename is a same-bytes overwrite.

BlobStore.verify

  • New default-method BlobStore.verify(ref): Boolean. Re-reads via get(ref) and rehashes; returns true when stored bytes still match the recorded hash, false on absence / corruption / truncation.
  • Default impl on the interface so existing InMemoryBlobStore and FileBlobStore both work without changes. Backends with cheaper checksum strategies can override.
  • Not on the hot path of get — opt-in by the caller (audit-time integrity scan, snapshot resume sanity check).

Regression coverage in FilesBlobStoreHardeningTest (5 cases):

  • Files.load oversize → OversizedFileException with all three values named in the diagnostic.
  • Files.load under-cap success path (256-byte PNG).
  • Files.DEFAULT_MAX_BYTES pinned at 20 MiB (changing it requires test update).
  • BlobStore.verify true / false / missing-blob contract.
  • FileBlobStore.put 16 concurrent threads writing the same hash — all succeed, no .tmp leftovers in the directory, all refs share the deterministic hash, final blob verifies.

docs/multimodal.md Files reference updated with the maxBytes default and the new OversizedFileException / verify semantics.

Full ./gradlew test + detekt green. detekt baseline regenerated for the new test file's package-naming + max-line-length entries (same shape as the other agents_engine.* test files).

…e.verify

ChatGPT audit (#2865) P1 — blob/file operational hardening. Three small,
independent surfaces, fixed together.

Files.load size cap
- New `maxBytes: Long = DEFAULT_MAX_BYTES` parameter on `Files.load`,
  `loadOrNull`, `loadAll`, `loadAllOrSkip`. Default 20 MiB.
- Size check via `Files.size(path)` BEFORE bytes are read into memory —
  a 4 GiB upload fails-fast without OOMing the JVM, instead of being
  slurped into a byte array first.
- New `OversizedFileException(path, sizeBytes, maxBytes)` names all
  three values in the message so a misconfigured callsite is
  debuggable.
- `loadAllOrSkip` deliberately does NOT swallow `OversizedFileException`
  — it's a fail-fast safety check, not a soft predicate. Document via
  the KDoc + suggest pre-filtering or a smaller `maxBytes`.

FileBlobStore unique tmp filename
- Pre-#2871 the put path used `dir.resolve("$hash.tmp")`. Two threads
  putting the SAME hash (rare but valid — same bytes computed
  independently) would both write to the same tmp filename, race on
  truncate/rename. The final atomic rename works for the WINNING
  thread, but a loser could rename its partial file over the winner's
  good copy.
- Now: `dir.resolve("$hash.${UUID.randomUUID()}.tmp")`. Per-attempt
  unique tmp filename, no collision possible. Atomic rename still
  works — target is keyed on `hash`, so the second rename is a
  same-bytes overwrite.

BlobStore.verify
- New default-method `BlobStore.verify(ref): Boolean`. Re-reads via
  `get(ref)` and rehashes; returns `true` when stored bytes still
  match the recorded hash, `false` on absence / corruption /
  truncation.
- Default impl on the interface so existing `InMemoryBlobStore` and
  `FileBlobStore` both work without changes. Backends with cheaper
  checksum strategies can override.
- Not on the hot path of `get` — opt-in by the caller (audit-time
  integrity scan, snapshot resume sanity check).

Regression coverage in `FilesBlobStoreHardeningTest` (5 cases):
- `Files.load` oversize → `OversizedFileException` with all three
  values named in the diagnostic.
- `Files.load` under-cap success path (256-byte PNG).
- `Files.DEFAULT_MAX_BYTES` pinned at 20 MiB (changing it requires
  test update).
- `BlobStore.verify` true / false / missing-blob contract.
- `FileBlobStore.put` 16 concurrent threads writing the same hash —
  all succeed, no `.tmp` leftovers in the directory, all refs share
  the deterministic hash, final blob verifies.

docs/multimodal.md `Files` reference updated with the `maxBytes`
default and the new `OversizedFileException` / `verify` semantics.

Full ./gradlew test + detekt green. detekt baseline regenerated for
the new test file's package-naming + max-line-length entries (same
shape as the other agents_engine.* test files).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Skobeltsyn Skobeltsyn merged commit 6b07f72 into main May 30, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant