#650

fix(jq): enforce file input and live-byte budgets

Openasafyishwants to merge intofrom

jq empty file... currently bypasses maxInputBytes and retains a whole file batch without reserving maxLiveBytes. The same gap affects --rawfile and --slurpfile and reproduces on unchanged main.

Let jq opt into the shared reader's file accounting using caller-owned leases. Reserve file bytes before reading, charge actual input bytes, wait for all pending reads on failure, and release the reservations when jq finishes. Other commands keep their existing accounting. Most of the jq diff is indentation to include file reads in its existing try/finally lifetime.

Related to #642 and #634. This is an independent fix against main; it contains no streaming pipeline changes.

Validation: three focused tests cover aggregate input limits, retained batch memory, and cleanup after errors/success. The two limit regressions fail before the fix. All 826 jq/query-engine/shared-reader consumer tests pass, as do typecheck, formatting, and banned-pattern lint.

Activity

7
vercel[bot]11d ago

@asafyish is attempting to deploy a commit to the Vercel Labs Team on Vercel.

A member of the Team first needs to authorize it.

auto-maintain[bot]11d ago

🤖 auto-maintain review

Automated, advisory triage for @asafyish's PR. Facts below are read from the GitHub API.

CheckResult
Author's merged PRs (this repo)1
Account established✅ (age 5392d · 13 followers · 7 public repos)
Commits signed/verified⚠️ 0/2
Changeset included✅ (.changeset/jq-file-input-budgets.md)

Review panel: 🔴 high highest severity

just-bash maintainer code review: 🟢 low

No actionable defects found in the complete base-to-head diff.

General code review: 🔴 high

The live-byte enforcement remains bypassable during reads whose stat size is stale or advisory.

  • packages/just-bash/src/utils/file-reader.ts:141 — When stat underreports or a file grows, readBytesFrom allocates the entire file before the actual-byte reservation at lines 145–152. A large advisory-size file can therefore exceed maxLiveBytes and exhaust memory before the limit is raised; the read must be bounded/chunked against the remaining budget.

Adversarial security: 🟡 medium

The new accounting still permits a pre-enforcement memory-budget bypass when stat size is advisory or stale.

  • packages/just-bash/src/utils/file-reader.ts:141 — File content is fully materialized before an underestimated stat size is corrected. A mutable or custom filesystem can report a tiny size and return an arbitrarily large file, exceeding maxLiveBytes and potentially exhausting memory before the post-read reservation rejects it. Use a bounded/streaming read or enforce the limit during reading.

Adversarial security (second opinion): 🟢 low

Diff is limited to jq/file-reader byte accounting plus a test and changeset; the new lease path reserves before reading, charges actual bytes, settles all in-flight reads before propagating fatal errors, and releases leases in jq's finally on every exit path. Removing the per-file check in accountFileInput is safe because the scope's consumeInput enforces a strictly tighter aggregate bound against the same limits object, and the added rethrowFatalExecutionError calls stop limit/abort/security errors from being swallowed as 'No such file or directory' or 'parse error'. No backdoors, network, process, dependency, or CI changes; no actionable security findings.

Standard Bash and host portability: 🟡 medium

File reservations introduce a host-dependent false live-byte failure for unreadable non-regular files.

  • packages/just-bash/src/utils/file-reader.ts:133 — Release the pre-read reservation when reading fails. Ordinary host directories have nonzero stat sizes, so `jq empty directory` can incorrectly hit maxLiveBytes and exit 126 before readFile reports EISDIR; the result varies with filesystem directory metadata.

Posted by auto-maintain. This automated code review is advisory; a human maintainer makes the call.

asafyish10d ago

Verified both findings against the previous head and fixed them in 2073392c.

  • jq empty /a; jq empty /a; echo continued with a 100-byte file and maxInputBytes: 50 previously printed continued. File input now goes through the existing execution scope first, so exceeding the budget terminates the execution.
  • A filesystem reporting stat().size === 0 for readable content previously failed with “file grew while being read”. The reader now charges actual bytes and reserves any growth through the existing lease mechanism. The regression checks both a successful read and enforcement of the live-byte limit.

Two focused regressions fail before the fix and pass afterward. All 373 jq tests, 35 shared-reader tests, typecheck, and changed-file formatting pass.

asafyish9d ago

Reproduced the directory-size finding and fixed it in 9af012e7. A directory reporting size 4096 under maxLiveBytes: 300 previously caused fatal exit 126 before its read error. Pre-read content reservations now use regular-file sizes only; the same example retains the existing jq file-error diagnostic and exit 2. One regression was added. All 374 jq tests, typecheck, scoped formatting and knip pass.

The advisory-size observation describes a separate limitation of IFileSystem's whole-file read contract: readFileBytes/readFileBuffer return an already-materialized value. This patch reserves known sizes before reads and checks/charges actual bytes afterward; it cannot bound allocations inside an arbitrary provider before that provider returns. Enforcing that requires a bounded-read filesystem API and adapter changes, beyond this retained-input accounting fix. I am not adding that API in this PR.