From 739ac73e2961e7a1246e73242253b8e7413c03df Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 5 Nov 2026 03:37:46 +0000 Subject: [PATCH] fix(python): charge used heap, not a per-call reservation; stop echoing the final value Each embedded python3 call reserved max_memory/64 work units up front (1M at the 64 MiB default) out of the 100M budget, so the 96th call in a script failed even for `pass`. Charge Monty heap growth past the run's high-water mark instead; repeated large runs still share one budget. Monty converts the module's final value to a host object outside its memory and time limits, so a shared list graph as the last expression (x = [x, x] thirty times) expanded to 2^30 leaves and hung past the timeout. End every program with a None statement and drop the REPL-style echo, matching CPython for -c, files and stdin. --- crates/bashkit/docs/python.md | 9 +- crates/bashkit/src/builtins/python.rs | 93 +++++++++++++++---- .../integration/execution_budget_tests.rs | 12 ++- .../integration/python_integration_tests.rs | 5 +- .../integration/python_security_tests.rs | 30 ++++++ .../tests/spec_cases/python/python.test.sh | 12 +-- knowledge/log.md | 1 + knowledge/runtimes/python-builtin.md | 23 +++-- 8 files changed, 143 insertions(+), 42 deletions(-) diff --git a/crates/bashkit/docs/python.md b/crates/bashkit/docs/python.md index dc2958c78..bc288bf04 100644 --- a/crates/bashkit/docs/python.md +++ b/crates/bashkit/docs/python.md @@ -71,13 +71,14 @@ python3 -c "print(2 ** 10)" # Output: 1024 ``` -### Expression Evaluation +### Expression Values Are Not Echoed -When no `print()` is called, the last expression is displayed (REPL behavior): +Like CPython, `python3 -c`, script files and stdin do not print the value of +the last expression. Use `print()`: ```bash -python3 -c "2 + 2" -# Output: 4 +python3 -c "2 + 2" # prints nothing +python3 -c "print(2 + 2)" # Output: 4 ``` ### Script Files (from VFS) diff --git a/crates/bashkit/src/builtins/python.rs b/crates/bashkit/src/builtins/python.rs index cc41eeacd..0ea5c9f68 100644 --- a/crates/bashkit/src/builtins/python.rs +++ b/crates/bashkit/src/builtins/python.rs @@ -47,6 +47,8 @@ const DEFAULT_MAX_RECURSION: usize = 200; // Security hard-stop: catastrophic regex backtracking can bypass cooperative // interpreter budget checks, so disable regex stdlib module in untrusted code. const DISABLED_STDLIB_MODULES: &[&str] = &["re"]; +/// Monty heap bytes per shared work unit (see `BudgetTracker::charge_memory_peak`). +const PYTHON_MEMORY_BYTES_PER_WORK_UNIT: usize = 64; // Security decision: virtual Python datetime uses a fixed UTC instant so // sandboxed code cannot fingerprint host clock/timezone state. const VIRTUAL_NOW_UNIX_SECS: i64 = 1_704_067_200; // 2024-01-01T00:00:00Z @@ -58,6 +60,8 @@ struct BudgetTracker { runtime: LimitedTracker, execution: Option, vm_checkpoints: Cell, + /// Highest Monty heap size already charged to `execution`, in bytes. + charged_memory: Cell, } impl BudgetTracker { @@ -76,6 +80,31 @@ impl BudgetTracker { Ok(()) } + /// Charge heap growth past the previous high-water mark, one work unit per + /// [`PYTHON_MEMORY_BYTES_PER_WORK_UNIT`] bytes. + /// + /// Important security decision: a run pays for the peak memory it really + /// uses, so repeated invocations still share one budget (they cannot each + /// take a fresh `max_memory`), while small runs stay cheap. Reserving + /// `max_memory / 64` per call up front capped a default script at ~95 + /// `python3` calls even for `pass`. + fn charge_memory_peak(&self) -> std::result::Result<(), ResourceError> { + let Some(budget) = &self.execution else { + return Ok(()); + }; + let current = self.runtime.current_memory(); + let charged = self.charged_memory.get(); + if current <= charged { + return Ok(()); + } + let units = current.div_ceil(PYTHON_MEMORY_BYTES_PER_WORK_UNIT) + - charged.div_ceil(PYTHON_MEMORY_BYTES_PER_WORK_UNIT); + self.charged_memory.set(current); + budget + .consume_work(u64::try_from(units).unwrap_or(u64::MAX)) + .map_err(Self::budget_error) + } + fn check_vm(&self) -> std::result::Result<(), ResourceError> { if let Some(budget) = &self.execution { budget.check().map_err(Self::budget_error)?; @@ -115,7 +144,8 @@ impl ResourceTracker for BudgetTracker { additional_bytes: impl FnOnce() -> usize, ) -> std::result::Result<(), ResourceError> { self.consume_work()?; - self.runtime.on_grow(additional_bytes) + self.runtime.on_grow(additional_bytes)?; + self.charge_memory_peak() } fn gc_interval(&self) -> Option { @@ -276,7 +306,7 @@ impl std::fmt::Debug for PythonExternalFns { /// python3 -c "print('hello')" /// python3 script.py /// echo "print('hello')" | python3 -/// python3 -c "2 + 2" # expression result printed +/// python3 -c "print(2 + 2)" # prints 4 (last expression is not echoed) /// python3 --version /// python3 -c "print(open('/tmp/f.txt').read())" /// ``` @@ -540,12 +570,8 @@ impl Builtin for Python { { limits.common.max_duration = limits.common.max_duration.min(remaining); } - // Monty exposes an independent VM ceiling. Reserve a conservative - // share so repeated runtime invocations cannot each claim a fresh full - // memory/allocation allowance from one host request. - ctx.consume_budget_work( - u64::try_from(limits.common.max_memory.div_ceil(64)).unwrap_or(u64::MAX), - )?; + // Monty's own memory ceiling is per run; heap growth is charged to the + // shared work budget as it happens (`BudgetTracker::charge_memory_peak`). let code = if let Some(prelude) = self .external_fns @@ -609,7 +635,19 @@ async fn run_python( code }; - let runner = match MontyRun::new(code.to_owned(), filename, vec![], CompileOptions::default()) { + // Important security decision: end every program with a `None` statement so + // Monty's result is always `None`. Monty converts the module's final value + // to a host `MontyObject` outside its memory/time limits; a shared list + // graph (`x = [x, x]` thirty times) expands to 2^30 leaves there. CPython + // never echoes the final expression for `-c`, files or stdin, so nothing + // user-visible is lost. A trailing backslash would join `None` onto the last + // line, so leave that (already invalid) code alone. + let code = if code.trim_end().ends_with('\\') { + code.to_owned() + } else { + format!("{code}\nNone\n") + }; + let runner = match MontyRun::new(code, filename, vec![], CompileOptions::default()) { Ok(r) => r, Err(e) => return Ok(format_exception(e)), }; @@ -623,6 +661,7 @@ async fn run_python( runtime: LimitedTracker::new(limits), execution: execution_budget.clone(), vm_checkpoints: Cell::new(0), + charged_memory: Cell::new(0), }; // Important security decision: cap collected print output at the same // memory budget as the VM heap. Monty 0.0.19 added a byte cap on @@ -736,13 +775,7 @@ async fn run_python( ); return Ok(format_exception_with_output(err, &buf)); } - RunProgress::Complete(result) => { - // If the result is not None and there was no print output, - // display the result (like Python REPL behavior for expressions) - if !matches!(result, MontyObject::None) && buf.is_empty() { - buf = format!("{}\n", result.py_repr()); - } - + RunProgress::Complete(_) => { return Ok(ExecResult::ok(buf)); } } @@ -1315,10 +1348,32 @@ mod tests { } #[tokio::test] - async fn test_inline_expression() { + async fn test_inline_expression_is_not_echoed() { + // Matches CPython: `python3 -c "2 + 3"` prints nothing. Echoing made + // Monty export the final value to the host, unbounded by its limits. let r = run(&["-c", "2 + 3"], None).await; assert_eq!(r.exit_code, 0); - assert_eq!(r.stdout, "5\n"); + assert_eq!(r.stdout, ""); + } + + #[tokio::test] + async fn test_large_shared_final_value_finishes() { + // Regression: a 2^30-leaf shared list as the last expression expanded + // to ~500 MiB on the host and ignored the timeout. + let r = run( + &["-c", "x = [1]\nfor _ in range(30):\n x = [x, x]\nx"], + None, + ) + .await; + assert_eq!(r.exit_code, 0, "{}", r.stderr); + assert_eq!(r.stdout, ""); + } + + #[tokio::test] + async fn test_trailing_backslash_stays_a_syntax_error() { + let r = run(&["-c", "x = 1 + \\"], None).await; + assert_ne!(r.exit_code, 0); + assert!(r.stderr.contains("Error"), "{}", r.stderr); } #[tokio::test] @@ -1407,7 +1462,7 @@ mod tests { #[tokio::test] async fn test_list_comprehension() { - let r = run(&["-c", "[x*2 for x in range(3)]"], None).await; + let r = run(&["-c", "print([x*2 for x in range(3)])"], None).await; assert_eq!(r.exit_code, 0); assert_eq!(r.stdout, "[0, 2, 4]\n"); } diff --git a/crates/bashkit/tests/integration/execution_budget_tests.rs b/crates/bashkit/tests/integration/execution_budget_tests.rs index a04a12d17..16fbfb380 100644 --- a/crates/bashkit/tests/integration/execution_budget_tests.rs +++ b/crates/bashkit/tests/integration/execution_budget_tests.rs @@ -373,10 +373,12 @@ async fn jq_stream_empty_consumes_shared_work_budget() { #[cfg(feature = "python")] #[tokio::test] -/// TM-DOS-096: separate Python entries cannot each claim a fresh VM allowance. +/// TM-DOS-096: separate Python entries cannot each claim a fresh VM allowance: +/// the heap each run actually uses is charged to the shared work budget. async fn repeated_python_entries_share_runtime_admission_budget() { let limits = ExecutionLimits::new() - .max_work_units(1_500_000) + // ~19 MiB of Python heap at 64 bytes per unit. + .max_work_units(300_000) .max_aggregate_input_bytes(100_000); let mut bash = Bash::builder() .limits(limits) @@ -385,8 +387,10 @@ async fn repeated_python_entries_share_runtime_admission_budget() { .build(); assert_budget_exhausted( - bash.exec("python -c 'print(1)'; python -c 'print(2)'") - .await, + bash.exec( + "python -c 'x = \"a\" * (12 * 1024 * 1024)'; python -c 'x = \"a\" * (12 * 1024 * 1024)'", + ) + .await, ); } diff --git a/crates/bashkit/tests/integration/python_integration_tests.rs b/crates/bashkit/tests/integration/python_integration_tests.rs index 16fab9936..4a3edd279 100644 --- a/crates/bashkit/tests/integration/python_integration_tests.rs +++ b/crates/bashkit/tests/integration/python_integration_tests.rs @@ -152,7 +152,8 @@ mod basic_execution { let mut bash = bash_python(); let r = bash.exec("python3 -c \"2 + 3\"").await.unwrap(); assert_eq!(r.exit_code, 0); - assert_eq!(r.stdout, "5\n"); + // Like CPython, the last expression is not echoed. + assert_eq!(r.stdout, ""); } #[tokio::test] @@ -216,7 +217,7 @@ mod basic_execution { let mut bash = bash_python(); let r = bash.exec("python3 -c \"'hello'\"").await.unwrap(); assert_eq!(r.exit_code, 0); - assert_eq!(r.stdout, "'hello'\n"); + assert_eq!(r.stdout, ""); } } diff --git a/crates/bashkit/tests/integration/python_security_tests.rs b/crates/bashkit/tests/integration/python_security_tests.rs index b9ca2789c..0081ad4b5 100644 --- a/crates/bashkit/tests/integration/python_security_tests.rs +++ b/crates/bashkit/tests/integration/python_security_tests.rs @@ -288,6 +288,36 @@ mod blackbox_class_escape { mod whitebox_resource_limits { use super::*; + /// Regression: every invocation used to reserve `max_memory / 64` work + /// units up front (1M at the 64 MiB default) out of the 100M budget, so + /// the 96th `python3` call in one script failed even for `pass`. + #[tokio::test] + async fn many_small_invocations_fit_in_one_script() { + let mut bash = bash_python_limits(PythonLimits::default()); + let r = bash + .exec("n=0; for i in $(seq 1 300); do python3 -c 'pass' && n=$((n+1)); done; echo $n") + .await + .unwrap(); + assert_eq!(r.stdout.trim(), "300", "stderr: {}", r.stderr); + } + + /// Memory a Python run actually uses is still charged to the shared work + /// budget, so repeated large runs cannot each take a fresh allowance. + #[tokio::test] + async fn memory_used_is_charged_to_the_work_budget() { + let mut bash = Bash::builder() + .python_with_limits(PythonLimits::default()) + .env("BASHKIT_ALLOW_INPROCESS_PYTHON", "1") + // ~64 MiB of Python heap at 64 bytes per unit. + .limits(ExecutionLimits::new().max_work_units(1_000_000)) + .build(); + let r = bash + .exec("for i in 1 2 3 4 5 6 7 8; do python3 -c 'x = \"a\" * (16 * 1024 * 1024)' || break; done") + .await; + let err = r.expect_err("8 x 16 MiB must exhaust a 1M-unit work budget"); + assert!(err.to_string().contains("work units"), "{err}"); + } + #[tokio::test] async fn tight_memory_blocks_list_bomb() { let mut bash = bash_python_tight(); diff --git a/crates/bashkit/tests/spec_cases/python/python.test.sh b/crates/bashkit/tests/spec_cases/python/python.test.sh index ccb6f943e..e0a187a9c 100644 --- a/crates/bashkit/tests/spec_cases/python/python.test.sh +++ b/crates/bashkit/tests/spec_cases/python/python.test.sh @@ -13,17 +13,17 @@ hello ### end ### python3_expression_arithmetic -# Expression result displayed (REPL behavior) -python3 -c "2 + 3" +# Like CPython, -c does not echo the last expression +python3 -c "2 + 3"; echo "rc=$?" ### expect -5 +rc=0 ### end ### python3_expression_string -# String expression result -python3 -c "'hello'" +# Like CPython, -c does not echo the last expression +python3 -c "'hello'"; echo "rc=$?" ### expect -'hello' +rc=0 ### end ### python3_multiline diff --git a/knowledge/log.md b/knowledge/log.md index 25097a030..995eb5245 100644 --- a/knowledge/log.md +++ b/knowledge/log.md @@ -3,6 +3,7 @@ ## 2026-11-05 * **Python `re` gate parses code**: the embedded Monty `python3` blocked `import re` by matching lines that start with `import`/`from`, so `pass; import re` or `if True: import re` loaded the module anyway (found by a downstream integration review). The gate now walks the parsed statement tree. See [Python Builtin](runtimes/python-builtin.md). +* **Python call cap and final-value hang**: each embedded `python3` call reserved `max_memory / 64` work units up front, so a default script failed on its 96th call; heap growth is now charged as it happens. The last expression is no longer echoed (CPython never does for `-c`/files/stdin): exporting it ran outside Monty's limits, and `x = [x, x]` thirty times hung past the timeout. Found reviewing a downstream integration. See [Python Builtin](runtimes/python-builtin.md). ## 2026-11-03 diff --git a/knowledge/runtimes/python-builtin.md b/knowledge/runtimes/python-builtin.md index ecb7455a3..4c2d22cd4 100644 --- a/knowledge/runtimes/python-builtin.md +++ b/knowledge/runtimes/python-builtin.md @@ -53,10 +53,17 @@ For security, execution is runtime-gated on `BASHKIT_ALLOW_INPROCESS_PYTHON=1` ### Supported Usage -`python3 -c "code"` (REPL-like: last expression printed), `python3 script.py` +`python3 -c "code"`, `python3 script.py` (from VFS), stdin (`echo "code" | python3`, `python3 - <<< ...`), `--version`/`-V`. Shebang lines are stripped automatically. +The value of the last expression is not echoed, matching CPython. Bashkit +appends a `None` statement to every program so Monty's final value is always +`None`: Monty converts the final value to a host object outside its memory and +time limits, and a shared list graph (`x = [x, x]` thirty times) expanded to +2^30 leaves there (~500 MiB, past the timeout). Until 2026-11-05 bashkit echoed +the value REPL-style. + ### Resource Limits Monty enforces its own limits independent of Bashkit's shell limits, @@ -74,12 +81,14 @@ budget even though it allocates nothing on the VM heap. There is no allocation-count knob, Monty removed `max_allocations` in 0.0.19. Each Python entry also consumes the request-scoped `ExecutionBudget`: source -bytes are charged as aggregate input, configured memory contributes a -conservative non-refundable admission reservation, and Monty's time/allocation -checkpoints consume shared work units. VFS pauses and external host calls keep -the same budget clone. Re-entering Python from a later command, substitution, -or pipeline stage therefore cannot obtain fresh aggregate fuel; Monty's limits -remain independently enforced. +bytes are charged as aggregate input, Monty heap growth past the run's +high-water mark is charged as work (one unit per 64 bytes), and Monty's +time/allocation checkpoints consume shared work units. VFS pauses and external +host calls keep the same budget clone. Re-entering Python from a later command, +substitution, or pipeline stage therefore cannot obtain fresh aggregate fuel; +Monty's limits remain independently enforced. Until 2026-11-05 each entry +reserved `max_memory / 64` units up front (1M at the 64 MiB default), which +capped a default script at ~95 `python3` calls even for `pass`. Since Monty 0.0.4 the parser also enforces a nesting-depth limit (200 release / 35 debug) against stack overflow from deeply nested expressions.