expr: bound the memory a webhook CHECK may allocate (SQL-431) - #38162
Open
mtabebe wants to merge 1 commit into
Open
expr: bound the memory a webhook CHECK may allocate (SQL-431)#38162mtabebe wants to merge 1 commit into
mtabebe wants to merge 1 commit into
Conversation
mtabebe
force-pushed
the
fix/webhook-check-memory-cap
branch
from
August 11, 2026 16:44
81ae96c to
de610ce
Compare
Problem: A webhook CHECK is a user-authored expression that environmentd evaluates once per in-flight request, and nothing bounded what it could allocate. The webhook path caps request count (500) and body size (5 MiB), but the evaluation's footprint is per-request and was bounded only by MAX_STRING_FUNC_RESULT_BYTES, a 100 MiB per-call ceiling sized for a cluster. That ceiling multiplies by request concurrency, so a CHECK doing length(repeat(body, 20)) >= 0 turned 150 concurrent 5 MB posts into 8.7 GiB of RSS, growing with the number of clients. Every request returned 200. Nothing refused the work, so the memory was just held. Solution: - Give RowArena an optional budget and build the validation arena with one, defaulting to 20 MiB via a new dyncfg, webhook_validation_memory_budget_bytes. - Enforce in two places, because amplifiers allocate in two shapes. Functions that can predict their result size consult max_string_func_result_bytes, which narrows the constant to the arena's remaining budget, so an over-budget result is never allocated at all. This now covers the string amplifiers and array_fill, which sizes its result from a parameter rather than its input. Everything else is caught by a post-call check in the evaluator, which covers functions with no ceiling of their own. string_to_array, for one, builds its array straight into the arena and consults none. - Add EvalError::TempStorageBudgetExceeded for the over-budget case rather than reusing LengthTooLarge, whose "requested length too large" text misdescribes an over-budget array or arena-built result. - Report the budget rather than enforce it in the arena itself. Its pushes are infallible, and refusing one would hand back a truncated value. - Stop copying an owned result into the arena. String and Vec<u8> outputs went through push_string/push_bytes, which copied and dropped the original, so an amplifying call materialized its bytes twice at the peak. push_owned_bytes adopts the allocation as a region when the bytes would not fit the active region anyway, and copies when they would, so a small value still shares a region rather than getting one of its own. - Register webhook_validation_memory_budget_bytes with parallel-workload's FlipFlagsAction and mzcompose's UNINTERESTING_SYSTEM_PARAMETERS, which bin/lint-test-flags requires for every new dyncfg. An unbudgeted arena, which is every arena in a dataflow, is unaffected beyond one branch on a None. Testing: - New integration test webhook_validation_memory_budget in server.rs: a 2 MiB body that amplifies past the 20 MiB default is refused with 400, a smaller body through the same source succeeds, and raising then lowering the dyncfg flips the result both ways. - New expr unit tests test_repeat_respects_arena_budget, test_array_fill_respects_arena_budget, and test_arena_built_result_respects_budget cover the pre-check paths (a parameter-sized string and array) and the post-call path. miri_test_arena_budget and miri_test_arena_adopts_owned_bytes_and_keeps_references in row.rs cover the arena bookkeeping and buffer adoption. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mtabebe
force-pushed
the
fix/webhook-check-memory-cap
branch
from
August 11, 2026 16:57
de610ce to
4a5f741
Compare
mtabebe
marked this pull request as ready for review
August 11, 2026 19:07
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
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.
Problem:
A webhook CHECK is a user defined expression that environmentd evaluates once per in-flight request, and nothing bounds what it could allocate.
The webhook path caps request count (500) and body size (5 MiB), but the evaluation's footprint is per-request and was bounded only by MAX_STRING_FUNC_RESULT_BYTES, a 100 MiB per-call ceiling sized for a cluster.
That ceiling multiplies by request concurrency, so a CHECK doing length(repeat(body, 20)) >= 0 turned 150 concurrent 5 MB posts into 8.7 GiB of RSS, growing with the number of clients.
Solution:
An unbudgeted arena, which is every arena in a dataflow, is unaffected
Testing: