fix(sheets): bound recalc/read/write against untrusted-input DoS + fix exports map - #1
Merged
Merged
Conversation
…x exports map Every write and load routes through recalc(), whose dependency graph, range materialization, formula parsing, and string output were all unbounded. Raw client workbooks (saveSheetWorkbookAction) and agent tool calls (sheets_set_cells) could hang or exhaust memory: V1 range amplification (getRangeValues / SUM over an oversized declared range) V2 O(N^2) dependency graph (N cells each SUM a wide range) V3 bulk CSV import of many formula cells V4 REPT/CONCAT output bomb (tiny input, huge output) V5 oversized formula parse cost (single giant body and many max-length bodies) New src/lib/limits.ts adds SheetsLimits + DEFAULT_LIMITS, a typed dash-free SheetsLimitError (code/limit/actual, instanceof-safe), and a RecalcBudget (op counter + wall-clock deadline) threaded through recalc(). Source caps reject oversized input before the expensive work runs (the only defense that can bound a single synchronous parser.parse call, which a counter cannot interrupt): formula length 8192, cell content 32767, sheet dims 1048576 x 16384, populated cells 1e6, range cells 65536, batch 1e5. The string-amplifier functions (REPT/CONCAT/CONCATENATE/TEXTJOIN) are overridden to refuse output larger than a cell can hold, with a post-eval coerceResult guard for the raw & operator. maxRangeCells is 65536, below Excel's row ceiling: fast-formula-parser evaluates a single range super-linearly (a 200k-cell SUM costs ~66s in one uninterruptible call, with a sharp cliff past ~65k), so the per-range cap must sit under that cliff. Limit breaches thrown inside onCell/onRange (which the parser rewraps as FormulaError) are stashed and re-thrown after parse(). All limits are optional overrides over DEFAULT_LIMITS, so existing callers are unchanged; a realistic ~10k-cell workbook recalcs in well under the budgets. Also fixes the package.json exports map: add a default/require condition to "." and "./react" and add "./package.json", which unblocks webpack/next build in consuming apps (previously worked around with a next.config alias). Adds src/lib/recalc.dos.test.ts reproducing V1..V5 with the real payloads plus a false-positive guard. Bumps 0.1.0 -> 0.2.0.
…ors trip in ~1.5-2s The round-4 op-budget + Excel-spec caps bound every DoS vector, but two residual within-cap payloads only terminated at the 10s wall-clock: an O(N^2) dep graph (thousands of =SUM(A1:A20000) cells) and a parse-aggregate flood (thousands of ~8000-char =1+1+... bodies). A 10s single-thread block is still an availability hit. Lower recalcWallClockMs 10000->1800 and scale recalcOpBudget 500M->90M to the same time envelope. Both vectors now trip in ~1.5-1.8s (V2 on the op budget, V5-aggregate on the wall clock) instead of ~10s. A realistic ~10k-cell workbook recalcs in ~70ms, so 1800ms keeps a ~25x false-positive margin. No architecture change; only the default constants (and the test pinning them) are tuned.
The V5 aggregate DoS test tightened only totalFormulaCharsBudget and left recalcWallClockMs at the 1800ms default, so on a slow CI runner the ~250 expensive 8000-char parses crossed the wall-clock deadline first and the test flaked with recalc_time_budget_exceeded instead of the asserted formula_parse_budget_exceeded. Lower the char budget so it trips after a few dozen parses and lift the wall clock, making the parse-char budget the deterministic binding constraint. Production code is unchanged.
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
Every write and load in
@hasna/sheetsroutes throughrecalc(), whose dependency graph, range materialization, formula parsing, and string output were all unbounded. Raw client workbooks (saveSheetWorkbookAction) and agent tool calls (sheets_set_cells) could hang the process or exhaust memory. This PR bounds all of them and fixes the broken exports map.Vectors neutralized (real PoC payloads, measured after the fix)
getRangeValues("A1:A2000000")on a 100-row grid=SUM(A1:A200000)/ 200k rowsrange_too_large=SUM(A1:A20000)recalc_op_budget_exceeded(bounded)batch_too_large=REPT("z",300000000)#VALUE!cellformula_too_long=1+1+...recalc_time_budget_exceeded(bounded)What changed
src/lib/limits.ts:SheetsLimits+DEFAULT_LIMITS, a typed dash-freeSheetsLimitError(code/limit/actual,instanceof-safe), and aRecalcBudget(integer op counter + wall-clock deadline).parser.parse, which a counter cannot interrupt): formula length 8192, cell content 32767, sheet dims 1048576×16384, populated cells 1e6, range cells 65536, batch 1e5.onCell/onRange(which the parser rewraps asFormulaError) are stashed and re-thrown afterparse().REPT/CONCAT/CONCATENATE/TEXTJOINoverridden to refuse output larger than a cell can hold, plus a post-evalcoerceResultguard for the raw&operator.default/requirecondition added to.and./react, plus./package.json— unblocks webpack/next buildin consuming apps (previously worked around with anext.configalias).SheetsLimitError,SheetsLimits,SheetsLimitCode,LimitOptions,DEFAULT_LIMITSexported from the package root.0.1.0→0.2.0.Design note (deviation)
maxRangeCellsis 65536, below Excel's 1,048,576 row ceiling.fast-formula-parserevaluates a single range super-linearly — a 200k-cellSUMcosts ~66s in one uninterruptible synchronous call, with a sharp cost cliff past ~65k cells — so the per-range cap has to sit under that cliff to be an effective single-call CPU bound. This is 100x+ above any realistic in-app range.Backward compatibility
All limits live in
DEFAULT_LIMITSand every new parameter is optional, so existing callers are unchanged; a realistic ~10k-cell workbook recalcs in well under every budget (verified by a false-positive test).Tests
src/lib/recalc.dos.test.tsreproduces V1..V5 with the real payloads, verifies theSheetsLimitErrorshape, pinsDEFAULT_LIMITSto the shipped values, and guards against false positives. Full suite: 68 pass / 0 fail.tsc --noEmitclean.bun run buildclean.