fix(image-write): add a per-session cumulative volume cap - #291
Merged
Conversation
Per-message caps (MAX_IMAGE_DATA_LENGTH + the 10 MB decode limit) bound a single image write but nothing stops a compromised webview from looping image-write with distinct PNG-magic-prefixed payloads — each content- addresses to a NEW <docFolder>/assets/ file, filling the disk silently while every per-message cap is individually satisfied. Add the count/volume bound every other webview->host channel already has: a generous 512 MiB cumulative byte budget scoped to the editor session (one per panel). Once crossed it rejects further writes and warns the user once; reopening the document starts a fresh budget. Real paste flows never approach it.
Review-cycle fixes for the session volume cap: - Make the budget a required dep (one SessionVolumeBudget object) so the pre-cap 'unbounded' state is no longer a silent default of ImageWriteDeps. - Add release(): refund a reservation when the write fails, so a run of transient FS failures can't exhaust the session cap and lock out pastes. - Guard reserve()/release() against NaN/negative input, which would otherwise poison or rewind the running total (disabling the cap). - Correct the module header (this is the first session-cumulative bound, not MAX_LINT_DIAGNOSTICS precedent) and the reserve/rollback comments. - Add tests: per-panel budget isolation, default-budget normal-paste flow, release-on-failure, early-exit paths skipping the charge, domain guards. - Bump version 0.1.59 -> 0.1.60 + CHANGELOG entry for the user-visible cap.
…cope Cycle-2 review fixes: - Remove release()/refund from SessionVolumeBudget. workspace.fs.writeFile is not atomic, so a failed write can still leave a partial content-addressed file; refunding the reservation would let an attacker loop failing writes and grow the disk without bound while every charge is returned. Never refunding keeps total disk growth bounded by the budget — the fail-safe choice for a disk-fill cap. Cost (a session with many genuine FS failures may hit the cap early; reopen to reset) is negligible against 512 MiB. - Narrow handleImageWrite's try to the write alone and post the success result outside it, so a throwing postResult can no longer be misread as a write failure (wrong toast + double post). - Test: over-cap-data-string path now asserts the budget is not charged, like its sibling early-exit tests; add an end-to-end no-refund exhaustion test.
Record why the budget is reserved before the whole write (createDirectory + writeFile): a pre-write setup failure is charged despite writing no bytes, but that is deliberate — reserving here keeps budget rejection cleanly separate from I/O failures, and the over-charge is immaterial (createDirectory only fails on a broken FS, which blocks pastes regardless of the cap). Comment-only.
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.
Summary
Add a per-session cumulative volume cap to the image-write path. The existing per-message caps bound a single write but do nothing against a compromised webview that loops
image-writewith distinct PNG-magic-prefixed payloads — each content-addresses to a new<docFolder>/assets/file, filling the disk silently while every per-message cap is individually satisfied.Changes
image-write-budget.ts: a pure session-scoped cumulative byte counter (createSessionVolumeBudget) that rejects once a generous 512 MiB ceiling is crossed and fires a one-time warning (a per-write toast would itself be a notification-flood vector).handleImageWritegains an optionalreserveBudgetdep, checked after validation (only bytes that would actually hit disk are counted) and before the write; on reject it postsok:falsewithout a toast — the budget owns its one-time warning.createImageWriteWiringconstructs one budget per panel (= per editor session) and wiresreserveBudget+ the warning. Reopening the document starts a fresh budget.Related
Test Plan
pnpm compilegreenpnpm test:unitgreen (newimage-write-budget.test.tspins the trip + warn-once; service test pins the budget-reject path; wiring test pins the per-session cap end-to-end)pnpm lintclean on changed files