From 8093d22164b75ea45b015d32c4678fb5ce2935b9 Mon Sep 17 00:00:00 2001 From: Chinesezjc Date: Sun, 2 Aug 2026 23:01:36 +0800 Subject: [PATCH] fix(code-runtime-python): bound stray capture by serialized cost, chunk-scan, and flush on destroy The line-aggregating stray capture from the previous round regressed three ways the review caught. Rewrite it on the fd-3 reader's raw-Buffer-chunk shape: accumulate chunks with a byte counter and split on the raw 0x0a byte, so a large newline-free write no longer re-copies the residual and re-scans from index 0 per chunk (both O(N^2)). Meter each admitted entry by serialized cost through a new jsonStringCostUpTo that walks to the cap and stops, so a near-budget control-char-dense line never allocates the sixfold-inflated JSON.stringify result the old ledger did (the critical: ~1.6 GiB transient under a large maxLogBytes). Flush the residual explicitly in the closeDeadline handler before it destroys the streams, so a setsid escapee's path (which fires no end) does not drop a leader's final newline-free diagnostic. Harden the sync-spawn leak assertion to a set difference against a pre-run snapshot, immune to a parallel worker's concurrent tmpdir create/delete. Decline the round-2 request to enforce the fd-3 ceiling per-frame: the counter check must precede Buffer.concat to prevent ~2x memory doubling (two regression tests assert this), and the batch-edge false reject it would fix is reachable only at a maxLogBytes/maxValueBytes configured within one pipe read of the 256 MiB ceiling, far past the defaults. Documented at the check and in the note Alternatives. Add flood, NUL-flood, short-escape, and closeDeadline-flush regression tests (restoring per-file 100% coverage); update the Agent Note and zh pair. --- ...-runtime-python-settlement-fixes.i18n.yaml | 4 +- ...31-code-runtime-python-settlement-fixes.md | 10 +- ...code-runtime-python-settlement-fixes.zh.md | 10 +- .../code-runtime-python/src/index.ts | 169 ++++++++++++------ .../tests/boot-write-failure.spec.ts | 12 +- .../code-runtime-python/tests/runtime.spec.ts | 56 ++++++ 6 files changed, 195 insertions(+), 66 deletions(-) diff --git a/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.i18n.yaml index 45ccc1425f..c2bcbbeee6 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.i18n.yaml +++ b/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.md -2026-07-31-code-runtime-python-settlement-fixes.md: 06df6f2c882f47c03ea45a4ec5085ef6c66a7013 -2026-07-31-code-runtime-python-settlement-fixes.zh.md: 8aa0e61f8818af3fd47fa589e32bf40336825268 +2026-07-31-code-runtime-python-settlement-fixes.md: 7bb01f08f62994029680cdab3572a10c8971474b +2026-07-31-code-runtime-python-settlement-fixes.zh.md: 169cc34d3edc82ea52c89c37da4db8e9e63dd661 diff --git a/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.md b/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.md index 06df6f2c88..7bb01f08f6 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.md +++ b/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.md @@ -54,13 +54,13 @@ Also in `src/index.ts`, `spawn` is called before the settlement Promise executor ### Stray pipe output is aggregated by line, not by transport chunk -Also in `src/index.ts`, native stdout/stderr bytes (C-extension writes, `os.write` past the pipe buffer) were pushed to `logs` one entry per Node `data` chunk. `logs` entries are joined with `\n` downstream (Code Mode), so a single newline-free write larger than one pipe read — arriving as several `data` chunks — read back with model-visible newlines inserted at arbitrary transport boundaries. Capture now holds a per-stream residual, admits an entry only on a real `\n`, and flushes the trailing partial once on the pipe's `end`, matching the child's own line-granular `log` frames. The residual stays bounded by the ledger: when it would cross the budget with no newline in sight it is admitted (and truncated) immediately, and once the ledger has truncated, buffering stops so a newline-free flood cannot retain host memory for output that can never be admitted. +Also in `src/index.ts`, native stdout/stderr bytes (C-extension writes, `os.write` past the pipe buffer) were pushed to `logs` one entry per Node `data` chunk. `logs` entries are joined with `\n` downstream (Code Mode), so a single newline-free write larger than one pipe read — arriving as several `data` chunks — read back with model-visible newlines inserted at arbitrary transport boundaries. Capture now accumulates raw `Buffer` chunks with a running byte counter (the same shape as the fd-3 reader, and for the same reason: a string `+=` accumulator re-copies the whole residual per chunk and scanning it from index 0 each chunk is a second quadratic — both O(N²) on a large newline-free write), splits on the raw `0x0a` byte, and admits one entry per complete line. A newline never appears inside a UTF-8 multibyte sequence, so decoding each split line is safe without a streaming decoder. The residual stays bounded by the ledger: when its raw byte count would cross the budget with no newline in sight it is flushed (admitted and truncated) immediately, and once the ledger has truncated, buffering stops so a newline-free flood cannot retain host memory for output that can never be admitted. The per-entry charge itself is metered by SERIALIZED cost through `jsonStringCostUpTo`, which walks the string to the cap and stops — the previous `Buffer.byteLength(JSON.stringify(text))` allocated the whole escaped form first, so a near-budget control-char-dense line under a large `maxLogBytes` could momentarily allocate over a gigabyte just to measure it. The residual is flushed on the pipe's `end` and also explicitly in the `closeDeadline` handler before it destroys the streams: a `setsid` escapee holding the pipes open forces settlement through that path without an `end`, so a final newline-free `os.write(1, …)` the leader emitted before exiting would otherwise be dropped from `logs`. ## Testing -- `tests/boot-write-failure.spec.ts` mocks `spawn` so the fd-3 pipe throws on the boot write — the one path a real subprocess cannot be coerced into — and asserts `run()` resolves a `worker-exit` rather than rejecting. A sibling case makes the mocked `spawn` throw SYNCHRONOUSLY and asserts `run()` still resolves a `worker-exit` and leaves no `dsh-code-runtime-python-*` directory behind in `tmpdir` (before/after diff). Both are isolated in this spec so the real-subprocess suite is untouched. +- `tests/boot-write-failure.spec.ts` mocks `spawn` so the fd-3 pipe throws on the boot write — the one path a real subprocess cannot be coerced into — and asserts `run()` resolves a `worker-exit` rather than rejecting. A sibling case makes the mocked `spawn` throw SYNCHRONOUSLY and asserts `run()` still resolves a `worker-exit` and leaves no NEW `dsh-code-runtime-python-*` directory in `tmpdir` (a set difference against a pre-run snapshot, so a sibling worker's concurrent create or delete cannot flake the assertion). Both are isolated in this spec so the real-subprocess suite is untouched. - `tests/residual-detach.spec.ts` unit-tests `detachResidual`: the carried copy equals the residual, owns a backing store sized to its own length (fixture kept above Node's Buffer pool threshold), and does not share the source frame's `ArrayBuffer`. -- `tests/runtime.spec.ts` — the output-cap case asserts the `ceiling - envelope` bound (268435392) and its message. A daemon-thread case drives four threads emitting unterminated writes through settlement's flush. A native-write case writes 200 KiB with no newline via `os.write` under a raised `maxLogBytes` and asserts it reads back as EXACTLY one log entry (proving stray output is aggregated by line, not split at pipe-chunk boundaries); a companion writes `b"one\ntwo\nthree"` and asserts three entries (proving real newlines still delimit). The same-group reap case spawns a SIGTERM-ignoring same-group descendant that releases the pipes and bumps a heartbeat file; the test asserts the heartbeat STOPS after the grace-window SIGKILL — an assertion robust whether the killed descendant is reaped or lingers as a zombie, so it holds where PID 1 does not wait() orphans. A dispose-after-resolve case asserts `dispose()` of a completed run with a same-group survivor returns only after the survivor stops executing (proving the run stays in `live` until its group is reaped), with an `expect(afterDispose).toBeGreaterThan(0)` guard so the frozen-heartbeat assertion cannot pass vacuously when the file was never written. A deadline case busy-blocks the event loop past both timers and asserts the survivor's heartbeat freezes (proving the poll's deadline arm sends SIGKILL itself rather than cancelling the unfired escalation). The cross-loop case runs a binding from a worker thread's own `asyncio.run` loop while the main coroutine yields with `await asyncio.sleep`, asserting the reply round-trips instead of timing out; a companion case abandons a thread's call so its loop closes, then answers it before a later binding — asserting the pump survives the closed-loop `call_soon_threadsafe` (host-gated ordering makes it deterministic, fail-before hangs the later binding to the wall clock). The inherited-soft-limit case runs the interpreter through a `ulimit -S -t` wrapper that sets a CPU soft limit below `cpuSeconds` and asserts the applied `RLIMIT_CPU` soft is the inherited value, not the configured one (CPU rather than address space, since macOS ignores `ulimit -v`); a companion inherits a 1 s CPU soft, has the program trap SIGXCPU and busy-loop past it, and asserts the settlement recheck reports a timeout — proving the recheck uses the effective soft, not the configured `cpuSeconds`. A control-heavy-diagnostic case raises a NUL-flood exception under a small `maxValueBytes` and asserts the serialized frame fits (proving the diagnostic is metered by serialized cost). A non-integer-budget case asserts a fractional `maxLogBytes`/`maxValueBytes` rejects at load. +- `tests/runtime.spec.ts` — the output-cap case asserts the `ceiling - envelope` bound (268435392) and its message. A daemon-thread case drives four threads emitting unterminated writes through settlement's flush. A native-write case writes 200 KiB with no newline via `os.write` under a raised `maxLogBytes` and asserts it reads back as EXACTLY one log entry (proving stray output is aggregated by line, not split at pipe-chunk boundaries); a companion writes `b"one\ntwo\nthree"` and asserts three entries (proving real newlines still delimit). A newline-free-flood case writes 2 MiB under a 4 KiB `maxLogBytes` and asserts the capture ends at the truncation marker and stays under budget (proving the residual is bounded by the ledger, not buffered whole); a NUL-flood companion writes 4000 newline-free NULs under the same budget and asserts truncation (proving the residual is charged by SERIALIZED cost, ~6× raw, measured without allocating the escaped copy); a short-escape case writes a line mixing a tab, quote, backslash, a `\uXXXX` control, a multibyte character, and ASCII, asserting it round-trips verbatim (exercising every branch of `jsonStringCostUpTo`). A closeDeadline-flush case has the leader write a newline-free diagnostic then spawn a `setsid` orphan holding the pipes open, and asserts the diagnostic survives in `logs` (proving the residual is flushed before the deadline destroys the streams). The same-group reap case spawns a SIGTERM-ignoring same-group descendant that releases the pipes and bumps a heartbeat file; the test asserts the heartbeat STOPS after the grace-window SIGKILL — an assertion robust whether the killed descendant is reaped or lingers as a zombie, so it holds where PID 1 does not wait() orphans. A dispose-after-resolve case asserts `dispose()` of a completed run with a same-group survivor returns only after the survivor stops executing (proving the run stays in `live` until its group is reaped), with an `expect(afterDispose).toBeGreaterThan(0)` guard so the frozen-heartbeat assertion cannot pass vacuously when the file was never written. A deadline case busy-blocks the event loop past both timers and asserts the survivor's heartbeat freezes (proving the poll's deadline arm sends SIGKILL itself rather than cancelling the unfired escalation). The cross-loop case runs a binding from a worker thread's own `asyncio.run` loop while the main coroutine yields with `await asyncio.sleep`, asserting the reply round-trips instead of timing out; a companion case abandons a thread's call so its loop closes, then answers it before a later binding — asserting the pump survives the closed-loop `call_soon_threadsafe` (host-gated ordering makes it deterministic, fail-before hangs the later binding to the wall clock). The inherited-soft-limit case runs the interpreter through a `ulimit -S -t` wrapper that sets a CPU soft limit below `cpuSeconds` and asserts the applied `RLIMIT_CPU` soft is the inherited value, not the configured one (CPU rather than address space, since macOS ignores `ulimit -v`); a companion inherits a 1 s CPU soft, has the program trap SIGXCPU and busy-loop past it, and asserts the settlement recheck reports a timeout — proving the recheck uses the effective soft, not the configured `cpuSeconds`. A control-heavy-diagnostic case raises a NUL-flood exception under a small `maxValueBytes` and asserts the serialized frame fits (proving the diagnostic is metered by serialized cost). A non-integer-budget case asserts a fractional `maxLogBytes`/`maxValueBytes` rejects at load. ## Alternatives considered @@ -84,7 +84,9 @@ Also in `src/index.ts`, native stdout/stderr bytes (C-extension writes, `os.writ **Bill the host-side `capMessage` backstop by serialized cost, matching the child's `_cap_message`.** Rejected: the two caps guard different things. `_cap_message`'s output re-crosses fd 3 as a JSON string, so its escaped width is what the frame ceiling bounds — serialized billing is required there. `capMessage`'s output goes straight into `CodeRunResult.error.message` and never re-crosses a frame-bounded channel, so the honest measure of what it retains is the raw byte length of the model-visible string. An honest child has already capped by serialized cost and raw length ≤ serialized cost, so a well-formed message passes unchanged; a forged control-heavy message could serialize to ~6× its raw length, but since it travels no capped channel, billing it by that inflated wire width would truncate a legitimately-sized diagnostic for no containment gain. Each side's JSDoc documents the split and points at the other. -**Push stray pipe output one entry per `data` chunk.** Rejected: `logs` entries are joined with `\n` downstream, so a transport chunk boundary would become a model-visible newline — a single native write split across pipe reads would read back with spurious line breaks. Aggregating by real newline (residual + flush on `end`) matches the child's line-granular `log` frames; the ledger still bounds a newline-free flood by admitting-and-truncating the residual when it would cross the budget. +**Push stray pipe output one entry per `data` chunk.** Rejected: `logs` entries are joined with `\n` downstream, so a transport chunk boundary would become a model-visible newline — a single native write split across pipe reads would read back with spurious line breaks. Aggregating by real newline (raw-chunk buffer + split on `0x0a`) matches the child's line-granular `log` frames; the ledger still bounds a newline-free flood by admitting-and-truncating the residual when it would cross the budget. + +**Enforce the fd-3 frame ceiling per-frame (split before the counter check) to avoid a batch-edge false reject.** Rejected: the ceiling check reads the byte counter BEFORE any `Buffer.concat`, precisely so a hostile program cannot force ~2× the 256 MiB ceiling of host memory (the counter and the join are a second copy of everything held). Splitting first to bill a single frame would `Buffer.concat` an over-ceiling frame before rejecting it, reintroducing that doubling — two regression tests assert the pre-concat order for exactly this reason. The batch-edge false reject the per-frame order would fix (a legitimate near-cap frame whose newline-bearing chunk also carries the next frame's leading bytes nudging the counter over the ceiling for one pipe read) is reachable only when `maxLogBytes`/`maxValueBytes` is configured within one pipe read of the 256 MiB ceiling — orders of magnitude past the 32/64 KiB defaults. The memory-safety bound against hostile input at any config takes precedence over a false reject reachable only at a pathological near-ceiling config; the counter's over-count and this trade-off are documented at the check. ## Consequences diff --git a/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.zh.md b/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.zh.md index 8aa0e61f88..169cc34d3e 100644 --- a/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.zh.md +++ b/.agents/notes/implemented/bug-fix/2026-07-31-code-runtime-python-settlement-fixes.zh.md @@ -54,13 +54,13 @@ Status: implemented ### Stray pipe output is aggregated by line, not by transport chunk -同样在 `src/index.ts` 中，原生 stdout／stderr 字节（C 扩展写入、越过管道缓冲区的 `os.write`）过去每来一个 Node `data` 分片就被推入 `logs` 一条条目。`logs` 条目在下游（Code Mode）会用 `\n` 拼接，因此一次大于单次管道读取、且不含换行符的写入——它以若干个 `data` 分片到达——回读时会在任意传输边界处被插入模型可见的换行符。现在捕获会为每个流持有一份残余数据，仅在遇到真正的 `\n` 时才准入一条条目，并在管道 `end` 时一次性冲刷尾部的不完整部分，与子进程自己的按行粒度的 `log` 帧相符。该残余数据仍受账本约束：当它在看不到换行符的情况下将要越过预算时，会被立即准入（并截断）；而一旦账本已经截断，缓冲便停止，从而一场不含换行符的洪泛无法为永远无法被准入的输出保留宿主内存。 +同样在 `src/index.ts` 中，原生 stdout／stderr 字节（C 扩展写入、越过管道缓冲区的 `os.write`）过去每来一个 Node `data` 分片就被推入 `logs` 一条条目。`logs` 条目在下游（Code Mode）会用 `\n` 拼接，因此一次大于单次管道读取、且不含换行符的写入——它以若干个 `data` 分片到达——回读时会在任意传输边界处被插入模型可见的换行符。现在捕获会以一个持续推进的字节计数器累积原始 `Buffer` 分片（与 fd-3 读取器同一形态，出于同一原因：一个字符串 `+=` 累加器会为每个分片重新复制整份残余数据，而每个分片都从索引 0 扫描它则是第二重平方——在一次大的不含换行符的写入上二者都是 O(N²)），在原始的 `0x0a` 字节处切分，并为每个完整行准入一条条目。换行符绝不会出现在一个 UTF-8 多字节序列内部，因此对每个切出的行做解码无需流式解码器即可安全进行。该残余数据仍受账本约束：当它的原始字节计数在看不到换行符的情况下将要越过预算时，会被立即冲刷（准入并截断）；而一旦账本已经截断，缓冲便停止，从而一场不含换行符的洪泛无法为永远无法被准入的输出保留宿主内存。每条条目的计费本身通过 `jsonStringCostUpTo` 按序列化开销计量，它把字符串走到上限即停止——先前的 `Buffer.byteLength(JSON.stringify(text))` 会先分配出整份转义后的形式，因此在一个较大的 `maxLogBytes` 之下，一行接近预算、控制字符密集的内容，仅仅为了度量它就可能瞬时分配超过一 GB。该残余数据会在管道 `end` 时冲刷，也会在 `closeDeadline` 处理器销毁流之前被显式冲刷：一个持有管道不放的 `setsid` 逃逸者会迫使结算在没有 `end` 的情况下走那条路径，因此 leader 在退出前发出的最后一次不含换行符的 `os.write(1, …)` 否则会从 `logs` 中被丢弃。 ## Testing -- `tests/boot-write-failure.spec.ts` 对 `spawn` 做 mock，使 fd-3 管道在引导写入时抛出异常（这是真实子进程无法被迫进入的唯一路径），并断言 `run()` resolve 出一个 `worker-exit` 而非 reject。一个同级用例让被 mock 的 `spawn` 同步抛出，并断言 `run()` 仍然 resolve 出一个 `worker-exit`，且不会在 `tmpdir` 中留下任何 `dsh-code-runtime-python-*` 目录（前后差分）。两者都被隔离在这个 spec 中，因此真实子进程测试套件不受影响。 +- `tests/boot-write-failure.spec.ts` 对 `spawn` 做 mock，使 fd-3 管道在引导写入时抛出异常（这是真实子进程无法被迫进入的唯一路径），并断言 `run()` resolve 出一个 `worker-exit` 而非 reject。一个同级用例让被 mock 的 `spawn` 同步抛出，并断言 `run()` 仍然 resolve 出一个 `worker-exit`，且不会在 `tmpdir` 中留下任何新的 `dsh-code-runtime-python-*` 目录（相对一份运行前快照做集合差分，因此一个同级 worker 的并发创建或删除不会让该断言变得不稳定）。两者都被隔离在这个 spec 中，因此真实子进程测试套件不受影响。 - `tests/residual-detach.spec.ts` 对 `detachResidual` 做单元测试：向前传递的副本与残余数据相等、拥有一个大小与其自身长度一致的底层存储（fixture 保持在 Node 的 Buffer 池阈值之上），并且不与源帧的 `ArrayBuffer` 共享。 -- `tests/runtime.spec.ts`：output-cap 用例断言 `ceiling - envelope` 上界（268435392）及其消息。一个 daemon 线程用例驱动四个线程穿过结算的 flush 发出未结束的写入。一个 native-write 用例在抬高后的 `maxLogBytes` 之下，通过 `os.write` 写入 200 KiB 且不含换行符，断言它回读时恰好是一条日志条目（证明散逸输出是按行聚合的，而不是在管道分片边界处被切开）；一个配套用例写入 `b"one\ntwo\nthree"`，断言得到三条条目（证明真正的换行符仍然起分隔作用）。same-group 回收用例 spawn 一个忽略 SIGTERM 的同进程组后代，它释放管道并递增一个心跳文件；该测试断言在宽限窗口的 SIGKILL 之后心跳停止：无论被杀死的后代是被回收还是作为僵尸进程滞留，这个断言都成立，因此它在 PID 1 不 wait() 孤儿进程的环境下同样成立。一个 dispose-after-resolve 用例断言，对一个已完成、且存在同进程组存活者的运行调用 `dispose()`，只有在该存活者停止执行之后才返回（证明该运行会一直留在 `live` 中，直到它的进程组被回收），并带有一个 `expect(afterDispose).toBeGreaterThan(0)` 守卫，使得当心跳文件从未被写入时，冻结心跳的断言不会被空洞地通过。一个 deadline 用例忙阻塞事件循环越过两个定时器，断言该存活者的心跳冻结（证明轮询的截止时间分支自身发送 SIGKILL，而不是取消尚未触发的升级）。cross-loop 用例在主协程通过 `await asyncio.sleep` 让出时，从一个工作线程自己的 `asyncio.run` 事件循环运行一个绑定，断言该回复完成往返而不是超时；一个配套用例放弃某个线程的调用，使其事件循环关闭，随后在一个后续绑定之前回答它——断言 pump 在关闭事件循环上的 `call_soon_threadsafe` 之后仍然存活（由宿主门控的顺序使其具有确定性，未修复时会把后续绑定拖到墙钟上挂起）。inherited-soft-limit 用例通过一个 `ulimit -S -t` 包装脚本运行解释器，将 CPU 软限制设为低于 `cpuSeconds`，并断言实际应用的 `RLIMIT_CPU` 软限制是继承来的值，而不是配置的值（用 CPU 而非地址空间，因为 macOS 忽略 `ulimit -v`）。一个配套用例继承 1 秒的 CPU 软限制，让程序捕获 SIGXCPU 并忙循环越过它，断言结算复查报告 timeout——证明复查用的是实际生效的软限制，而不是配置的 `cpuSeconds`。一个 control-heavy-diagnostic 用例在一个较小的 `maxValueBytes` 之下抛出一个 NUL 洪泛异常，断言序列化后的帧能放得下（证明该诊断是按序列化开销计量的）。一个 non-integer-budget 用例断言一个小数的 `maxLogBytes`／`maxValueBytes` 在加载期被拒绝。 +- `tests/runtime.spec.ts`：output-cap 用例断言 `ceiling - envelope` 上界（268435392）及其消息。一个 daemon 线程用例驱动四个线程穿过结算的 flush 发出未结束的写入。一个 native-write 用例在抬高后的 `maxLogBytes` 之下，通过 `os.write` 写入 200 KiB 且不含换行符，断言它回读时恰好是一条日志条目（证明散逸输出是按行聚合的，而不是在管道分片边界处被切开）；一个配套用例写入 `b"one\ntwo\nthree"`，断言得到三条条目（证明真正的换行符仍然起分隔作用）。一个 newline-free-flood 用例在一个 4 KiB 的 `maxLogBytes` 之下写入 2 MiB，断言捕获终止于截断标记且保持在预算之内（证明残余数据受账本约束，而不是被整体缓冲）；一个 NUL-flood 配套用例在同一预算之下写入 4000 个不含换行符的 NUL，断言发生截断（证明残余数据是按序列化开销计费的，约为原始的 6 倍，且在度量时不分配转义后的副本）；一个 short-escape 用例写入一行混合了制表符、引号、反斜杠、一个 `\uXXXX` 控制字符、一个多字节字符和 ASCII 的内容，断言它原样完成往返（覆盖 `jsonStringCostUpTo` 的每一条分支）。一个 closeDeadline-flush 用例让 leader 写入一段不含换行符的诊断，随后 spawn 一个持有管道不放的 `setsid` 孤儿进程，断言该诊断在 `logs` 中存留下来（证明残余数据在截止时间销毁流之前被冲刷）。same-group 回收用例 spawn 一个忽略 SIGTERM 的同进程组后代，它释放管道并递增一个心跳文件；该测试断言在宽限窗口的 SIGKILL 之后心跳停止：无论被杀死的后代是被回收还是作为僵尸进程滞留，这个断言都成立，因此它在 PID 1 不 wait() 孤儿进程的环境下同样成立。一个 dispose-after-resolve 用例断言，对一个已完成、且存在同进程组存活者的运行调用 `dispose()`，只有在该存活者停止执行之后才返回（证明该运行会一直留在 `live` 中，直到它的进程组被回收），并带有一个 `expect(afterDispose).toBeGreaterThan(0)` 守卫，使得当心跳文件从未被写入时，冻结心跳的断言不会被空洞地通过。一个 deadline 用例忙阻塞事件循环越过两个定时器，断言该存活者的心跳冻结（证明轮询的截止时间分支自身发送 SIGKILL，而不是取消尚未触发的升级）。cross-loop 用例在主协程通过 `await asyncio.sleep` 让出时，从一个工作线程自己的 `asyncio.run` 事件循环运行一个绑定，断言该回复完成往返而不是超时；一个配套用例放弃某个线程的调用，使其事件循环关闭，随后在一个后续绑定之前回答它——断言 pump 在关闭事件循环上的 `call_soon_threadsafe` 之后仍然存活（由宿主门控的顺序使其具有确定性，未修复时会把后续绑定拖到墙钟上挂起）。inherited-soft-limit 用例通过一个 `ulimit -S -t` 包装脚本运行解释器，将 CPU 软限制设为低于 `cpuSeconds`，并断言实际应用的 `RLIMIT_CPU` 软限制是继承来的值，而不是配置的值（用 CPU 而非地址空间，因为 macOS 忽略 `ulimit -v`）。一个配套用例继承 1 秒的 CPU 软限制，让程序捕获 SIGXCPU 并忙循环越过它，断言结算复查报告 timeout——证明复查用的是实际生效的软限制，而不是配置的 `cpuSeconds`。一个 control-heavy-diagnostic 用例在一个较小的 `maxValueBytes` 之下抛出一个 NUL 洪泛异常，断言序列化后的帧能放得下（证明该诊断是按序列化开销计量的）。一个 non-integer-budget 用例断言一个小数的 `maxLogBytes`／`maxValueBytes` 在加载期被拒绝。 ## Alternatives considered @@ -84,7 +84,9 @@ Status: implemented **按序列化开销对宿主侧的 `capMessage` 兜底做计费，与子进程的 `_cap_message` 相符。** 已否决：这两处上限守护的是不同的东西。`_cap_message` 的输出会作为一个 JSON 字符串再次穿过 fd 3，因此帧上限约束的是它转义后的宽度——那里必须按序列化计费。`capMessage` 的输出直接进入 `CodeRunResult.error.message`，绝不会再次穿过一个受帧上限约束的通道，因此对它所保留内容的诚实度量是模型可见字符串的原始字节长度。一个诚实的子进程已经按序列化开销设过上限，而原始长度 ≤ 序列化开销，因此一条格式良好的消息会原样通过；一条伪造的、控制字符密集的消息可能序列化到其原始长度约 6 倍，但由于它不经过任何受上限约束的通道，按那个被抬高的传输宽度对它计费只会截断一条尺寸合法的诊断，而换不来任何收束上的收益。每一侧的 JSDoc 都记录了这一区分，并指向另一侧。 -**每来一个 `data` 分片就把散逸的管道输出推入一条条目。** 已否决：`logs` 条目在下游会用 `\n` 拼接，因此一个传输分片边界会变成一个模型可见的换行符——一次被拆散在多次管道读取中的原生写入会带着无端的换行回读。按真正的换行符聚合（残余数据 + 在 `end` 时冲刷）与子进程的按行粒度的 `log` 帧相符；账本仍然通过在残余数据将要越过预算时把它准入并截断，来约束一场不含换行符的洪泛。 +**每来一个 `data` 分片就把散逸的管道输出推入一条条目。** 已否决：`logs` 条目在下游会用 `\n` 拼接，因此一个传输分片边界会变成一个模型可见的换行符——一次被拆散在多次管道读取中的原生写入会带着无端的换行回读。按真正的换行符聚合（原始分片缓冲 + 在 `0x0a` 处切分）与子进程的按行粒度的 `log` 帧相符；账本仍然通过在残余数据将要越过预算时把它准入并截断，来约束一场不含换行符的洪泛。 + +**逐帧强制 fd-3 帧上限（在计数器检查之前先切分）以避免一次批次边缘的误拒。** 已否决：帧上限检查在任何 `Buffer.concat` 之前读取字节计数器，正是为了让一个敌意程序无法迫使宿主内存达到 256 MiB 帧上限的约 2 倍（计数器与那次拼接是所持全部内容的第二份副本）。先切分以对单个帧计费，会在拒绝一个超上限的帧之前就 `Buffer.concat` 它，从而重新引入那种翻倍——正是出于这个原因，有两个回归测试断言了先计数后拼接的顺序。逐帧顺序本会修复的那次批次边缘误拒（一个合法的接近上限的帧，其携带换行符的分片同时也带上了下一帧的起始字节，在一次管道读取中把计数器推过上限）只有当 `maxLogBytes`／`maxValueBytes` 被配置到距 256 MiB 帧上限一次管道读取以内时才可达——比 32／64 KiB 的默认值高出好几个数量级。在任何配置下都抵御敌意输入的内存安全边界，优先于一个仅在病态的接近上限配置下才可达的误拒；计数器的超额计数与这一权衡都记录在该检查处。 ## Consequences diff --git a/packages/code-runtime/code-runtime-python/src/index.ts b/packages/code-runtime/code-runtime-python/src/index.ts index ebee63e9c4..b6b9bd8b59 100644 --- a/packages/code-runtime/code-runtime-python/src/index.ts +++ b/packages/code-runtime/code-runtime-python/src/index.ts @@ -13,7 +13,6 @@ */ import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process' -import { StringDecoder } from 'node:string_decoder' import { accessSync, copyFileSync, constants as fsConstants, mkdtempSync, rmSync } from 'node:fs' import { tmpdir } from 'node:os' import { delimiter, dirname, isAbsolute, join } from 'node:path' @@ -313,6 +312,37 @@ const TRUNCATION_MARKER = '… [truncated]' */ const TRUNCATION_MARKER_BYTES = Buffer.byteLength(TRUNCATION_MARKER, 'utf8') +/** + * Serialized JSON-string cost of `text` (the two quotes plus each character's + * escaped byte width), measured WITHOUT materializing the escaped copy, and + * abandoned the instant it exceeds `maxBytes`. `JSON.stringify(text)` would + * allocate the whole escaped form first — up to sixfold a control-char-dense + * string — so a near-budget line under a large `maxLogBytes` could momentarily + * allocate over a gigabyte just to measure it. This walks code point by code + * point and stops at the cap, so the measurement allocates nothing. + * @param text - the candidate string. + * @param maxBytes - the largest serialized size the caller can admit. + * @returns the exact serialized byte cost, or `undefined` once it exceeds `maxBytes`. + */ +function jsonStringCostUpTo(text: string, maxBytes: number): number | undefined { + let bytes = 2 // the enclosing quotes + for (const character of text) { + const code = character.codePointAt(0) as number + // Control characters below 0x20 escape to `\uXXXX` (6) except the five with + // short forms `\b \t \n \f \r` (2); `"` and `\` escape to 2; everything else + // rides at its raw UTF-8 width. + if (code < 0x20) { + bytes += code === 0x08 || code === 0x09 || code === 0x0a || code === 0x0c || code === 0x0d ? 2 : 6 + } else if (code === 0x22 || code === 0x5c) { + bytes += 2 + } else { + bytes += Buffer.byteLength(character, 'utf8') + } + if (bytes > maxBytes) return undefined + } + return bytes +} + /** * Cap a done-frame `error.message` to `maxValueBytes` host-side: a forged done * frame can carry an arbitrarily long message, so truncate by RAW UTF-8 byte @@ -729,26 +759,22 @@ export class PythonCodeRuntime extends CodeRuntime { logs.push(logTruncationMarker(this.config.maxLogBytes)) return } - // Past the lower bound the escape expands the string at most sixfold, - // so this copy is bounded by ~6x the remaining budget. - const cost = Buffer.byteLength(JSON.stringify(text), 'utf8') + 1 - if (cost > logBudget) { + // Past the lower bound, measure the exact serialized cost without + // allocating the escaped copy: `jsonStringCostUpTo` walks to the cap and + // stops, so even a near-budget control-char-dense line never materializes + // a sixfold-inflated `JSON.stringify` result. `+ 1` for the separator. + const measured = jsonStringCostUpTo(text, logBudget - 1) + if (measured === undefined) { logsTruncated = true logs.push(logTruncationMarker(this.config.maxLogBytes)) return } - logBudget -= cost + logBudget -= measured + 1 logs.push(text) } // Stray-byte capture: anything the child writes to its stdout/stderr // (native prints, C-extension writes) still counts against the ledger. - // One STREAMING decoder per pipe: a multibyte UTF-8 sequence can span - // two chunks (native writes, os.write past the pipe buffer), and - // decoding each chunk independently would corrupt both halves into - // replacement characters. StringDecoder holds the partial sequence - // until its continuation bytes arrive; the pipes are separate byte - // streams, so they cannot share one decoder. // // Output is admitted per LINE, not per transport chunk. `logs` entries // are joined with `\n` downstream (Code Mode), so each entry must be one @@ -756,47 +782,67 @@ export class PythonCodeRuntime extends CodeRuntime { // boundary into a model-visible newline, so a single 200 KiB native write // split across pipe reads would read back with spurious line breaks. The // child's own `log` frames are already line-granular; stray capture - // matches them by holding a per-stream residual and admitting only on a - // real `\n`. A run of bytes carrying no newline accumulates in the - // residual; the ledger bounds it — `admit` charges each completed line, so - // a newline-free flood is capped when the pending residual would cross the - // budget, and the trailing partial is flushed once on `end`. - const strayOut = { decoder: new StringDecoder('utf8'), residual: '' } - const strayErr = { decoder: new StringDecoder('utf8'), residual: '' } - const captureStray = (stray: { decoder: StringDecoder; residual: string }, chunk: Buffer): void => { + // matches them by splitting on `\n`. + // + // Buffered as raw `Buffer` chunks with a running byte counter, exactly + // like the fd-3 reader below and for the same reasons: a string `+=` + // accumulator re-copies the whole residual on every pipe chunk (quadratic + // on a large newline-free write), and scanning it from index 0 each chunk + // is a second quadratic. Appending a chunk is O(1); the split happens only + // when a `\n` actually arrived. A newline never appears inside a UTF-8 + // multibyte sequence (continuation bytes are 0x80–0xBF), so splitting on + // the raw 0x0a byte and decoding each complete line is safe without a + // streaming decoder — a line's bytes are whole by construction. + interface StrayBuffer { chunks: Buffer[]; bytes: number } + const strayOut: StrayBuffer = { chunks: [], bytes: 0 } + const strayErr: StrayBuffer = { chunks: [], bytes: 0 } + const captureStray = (stray: StrayBuffer, chunk: Buffer): void => { // Once the ledger has truncated, stop buffering: admit() is a no-op past - // that point, so continuing to grow the residual would retain host - // memory for output that can never be admitted. + // that point, so continuing to accumulate would retain host memory for + // output that can never be admitted. if (logsTruncated) return - stray.residual += stray.decoder.write(chunk) - let newline = stray.residual.indexOf('\n') - while (newline >= 0) { - admit(stray.residual.slice(0, newline)) - stray.residual = stray.residual.slice(newline + 1) - newline = stray.residual.indexOf('\n') + stray.chunks.push(chunk) + stray.bytes += chunk.length + if (chunk.includes(0x0a)) { + let buffered = Buffer.concat(stray.chunks) + let newline: number + while ((newline = buffered.indexOf(0x0a)) >= 0) { + admit(buffered.subarray(0, newline).toString('utf8')) + buffered = buffered.subarray(newline + 1) + } + // Carry the residual as a fresh right-sized copy, not the subarray view + // (which would pin the whole concat allocation). See detachResidual. + stray.chunks = detachResidual(buffered) + stray.bytes = buffered.length } // Newline-free residual is bounded by the ledger, not left to grow with // the stream: an `os.write(1, b"A"*N)` flood carrying no newline would // otherwise accumulate N bytes in host memory before `end`. When the - // pending residual would cross the budget, admit it now — admit() - // truncates and marks the ledger, and the truncation short-circuit above - // stops further buffering on the next chunk. - if (stray.residual.length + 3 > logBudget) { - admit(stray.residual) - stray.residual = '' + // pending residual would cross the budget, admit it now — admit() charges + // its serialized cost, truncates, and marks the ledger, and the + // truncation short-circuit above stops buffering on the next chunk. The + // raw byte count is a safe lower bound on the serialized cost, so this + // fires no later than the budget is genuinely at risk. + if (stray.bytes + 3 > logBudget) { + flushStray(stray) } } + // Flush a pipe's residual into `logs`. Called on the budget threshold + // above, on the pipe's `end` (normal drain), and — for the setsid-escapee + // path where destroy() forces settlement without an `end` — explicitly in + // the closeDeadline handler. Idempotent: it clears what it admits, so a + // later flush is a no-op. The `chunks.length` guard is the only emptiness + // check needed — `data` never emits a zero-length Buffer, so a non-empty + // chunk list always decodes to a non-empty tail. + function flushStray(stray: StrayBuffer): void { + if (stray.chunks.length === 0) return + const tail = Buffer.concat(stray.chunks).toString('utf8') + stray.chunks = [] + stray.bytes = 0 + admit(tail) + } child.stdout.on('data', (chunk: Buffer) => { captureStray(strayOut, chunk) }) child.stderr.on('data', (chunk: Buffer) => { captureStray(strayErr, chunk) }) - // Flush each pipe's residual and decoder when it ends: a final line with - // no trailing newline, plus output that STOPS mid-sequence (native code - // killed between bytes) leaves a partial character in the decoder, which - // end() renders as U+FFFD rather than dropping the evidence. `end` fires - // before `close` settles the run, so the flush is admitted into `logs`. - const flushStray = (stray: { decoder: StringDecoder; residual: string }): void => { - const tail = stray.residual + stray.decoder.end() - if (tail.length > 0) admit(tail) - } child.stdout.on('end', () => { flushStray(strayOut) }) child.stderr.on('end', () => { flushStray(strayErr) }) @@ -829,11 +875,23 @@ export class PythonCodeRuntime extends CodeRuntime { // ceiling this check exists to enforce. The counter is exact and free, // and the retained chunks are released here so the rejected payload is // not still held while the run settles. - // Reading the counter rather than the line length also charges the - // whole unframed buffer, which over-counts by at most the newline- - // bearing chunk's own length (one pipe read): the residual carried in - // is always a partial line, so nothing but the current line can be - // larger than that. + // + // The counter charges the whole unframed buffer, which over-counts by at + // most the newline-bearing chunk's own length (one pipe read): the + // residual carried in is always a partial line, so nothing but the + // current line can be larger than that. That over-count is deliberate and + // load-bounded on the OTHER side: the config cap is `ceiling - envelope`, + // and a legitimate near-cap frame plus a following chunk's leading bytes + // could in principle nudge the counter over the ceiling for one read + // window — but only when maxLogBytes/maxValueBytes is configured within + // one pipe read of the 256 MiB ceiling, orders of magnitude past the + // 32/64 KiB defaults. Enforcing the ceiling per-frame instead (splitting + // before the check) would require `Buffer.concat`-ing an over-ceiling + // single frame before rejecting it, reintroducing the peak-memory + // doubling this pre-concat check and its regression tests exist to + // prevent; the memory-safety bound against hostile input at any config + // takes precedence over a false-reject reachable only at a pathological + // near-ceiling config. if (pendingBytes > FRAME_CEILING_BYTES) { pendingChunks = [] sealedBlocks = [] @@ -1233,12 +1291,17 @@ export class PythonCodeRuntime extends CodeRuntime { // `close` awaits every stdio stream draining, which a setsid-escaped // orphan holding our inherited pipes can prevent forever. Bound that // wait: after SIGKILL has had the grace window plus a margin to reap the - // child itself, force settlement on the decided result. Detaching the - // stream handles lets `close` land as a no-op if it ever arrives, and - // stops the orphan's stray output from being accounted against a run - // that already finished. `unref` so the deadline never keeps the host - // process alive on its own. + // child itself, force settlement on the decided result. Flush any + // newline-free stray residual FIRST — a leader that wrote a diagnostic + // with `os.write(1, ...)` and exited leaves it buffered, and destroying + // the stream below drops it before an `end`/`close` flush could run, so + // the diagnostic would be lost from `logs`. Detaching the stream handles + // then lets `close` land as a no-op if it ever arrives, and stops the + // orphan's stray output from being accounted against a run that already + // finished. `unref` so the deadline never keeps the host process alive. closeDeadline = setTimeout(() => { + flushStray(strayOut) + flushStray(strayErr) proto.destroy() child.stdout.destroy() child.stderr.destroy() diff --git a/packages/code-runtime/code-runtime-python/tests/boot-write-failure.spec.ts b/packages/code-runtime/code-runtime-python/tests/boot-write-failure.spec.ts index c488ea2f37..37b3c9c8c4 100644 --- a/packages/code-runtime/code-runtime-python/tests/boot-write-failure.spec.ts +++ b/packages/code-runtime/code-runtime-python/tests/boot-write-failure.spec.ts @@ -74,7 +74,13 @@ describe('PythonCodeRuntime — boot-write failure', () => { // misuse) and stranded the staging directory materializePyScripts had just // written, which only settle() removes. The fix catches it, unlinks the // directory, and resolves the same `worker-exit` class as an async ENOENT. - const before = readdirSync(tmpdir()).filter(name => name.startsWith('dsh-code-runtime-python-')) + // Snapshot as a SET, then assert no dir NEW relative to it survives. Strict + // array equality would flake: vitest's forks pool runs runtime.spec.ts in a + // sibling worker that concurrently creates and removes + // `dsh-code-runtime-python-*` dirs, so a concurrent create OR delete in the + // window would fail `toEqual`. The set difference is immune to both — it + // only asserts THIS run left nothing behind. + const before = new Set(readdirSync(tmpdir()).filter(name => name.startsWith('dsh-code-runtime-python-'))) spawnMock.mockImplementation(() => { throw Object.assign(new Error('EMFILE: too many open files'), { code: 'EMFILE' }) }) const ctx = new Context() const fiber = await ctx.plugin(PythonCodeRuntime) @@ -84,8 +90,8 @@ describe('PythonCodeRuntime — boot-write failure', () => { expect(result.error?.kind).toBe('worker-exit') expect(result.error?.message).toContain('python spawn error') - const after = readdirSync(tmpdir()).filter(name => name.startsWith('dsh-code-runtime-python-')) - expect(after).toEqual(before) + const leaked = readdirSync(tmpdir()).filter(name => name.startsWith('dsh-code-runtime-python-') && !before.has(name)) + expect(leaked).toEqual([]) await fiber.dispose() }) }) diff --git a/packages/code-runtime/code-runtime-python/tests/runtime.spec.ts b/packages/code-runtime/code-runtime-python/tests/runtime.spec.ts index c884699707..14326e363f 100644 --- a/packages/code-runtime/code-runtime-python/tests/runtime.spec.ts +++ b/packages/code-runtime/code-runtime-python/tests/runtime.spec.ts @@ -729,6 +729,40 @@ describe('PythonCodeRuntime — programs and bindings', () => { expect(result.logs.join('').length).toBeLessThan(4096) }) + it('bounds a control-char-dense native residual by serialized cost, not raw length', async () => { + // A newline-free NUL flood passes the cheap `length + 3` lower bound at a + // raw length well under the budget, but each NUL serializes to `` (6 + // bytes), so the true JSON cost is ~6x. The ledger must charge that + // serialized cost — and `jsonStringCostUpTo` must measure it WITHOUT + // allocating the escaped copy, so a near-budget line under a large + // maxLogBytes cannot momentarily allocate a multi-gigabyte `JSON.stringify` + // result. Under a small budget the residual is truncated once the serialized + // cost crosses it. + const { runtime } = await setup({ maxLogBytes: 4096 }) + const result = await runtime.run({ + program: ['import os', 'os.write(1, b"\\x00" * 4000)', 'return None'].join('\n'), + bindings: [], + }) + expect(result.error).toBeUndefined() + expect(result.logs.at(-1)).toBe(logTruncationMarker(4096)) + }) + + it('charges the exact serialized cost of short-escape and quote/backslash characters', async () => { + // Exercises every branch of jsonStringCostUpTo's per-character cost: a tab + // and other C0 controls with short JSON forms (\t etc., 2 bytes), a quote + // and backslash (2 bytes each), a `\uXXXX` control (6 bytes), a multibyte + // BMP character (raw UTF-8 width), and plain ASCII. Under a budget large + // enough to admit it, the line survives verbatim — proving the cost walker + // does not over- or under-charge and the string round-trips unescaped. + const { runtime } = await setup({ maxLogBytes: 4096 }) + const result = await runtime.run({ + program: ['import os', String.raw`os.write(1, "\ta\"b\\c\x01é\n".encode("utf-8"))`, 'return None'].join('\n'), + bindings: [], + }) + expect(result.error).toBeUndefined() + expect(result.logs).toEqual(['\ta"b\\c\x01é']) + }) + it('fails a completion dict with a non-string key as invalid-output (no key coercion)', async () => { // json.dumps would coerce {1: "a", "1": "b"} to a single "1" key, silently // dropping data. The shape validator rejects it before encoding. @@ -2043,6 +2077,28 @@ describe('PythonCodeRuntime — budgets, termination, disposal', () => { expect(elapsed).toBeLessThan(4_000) }, 8000) + it('flushes a newline-free diagnostic when the closeDeadline forces settlement', async () => { + // A leader that writes an unterminated diagnostic via `os.write(1, ...)` and + // then exits, leaving a setsid orphan holding the pipes open, settles through + // the closeDeadline destroy() path — which fires no `end`. The residual must + // be flushed before destroy() drops it, or the diagnostic is lost from + // `logs`. The value is decided by the done frame; the diagnostic must survive. + const { runtime } = await setup({ graceMs: 100 }) + const result = await runtime.run({ + program: [ + 'import os, subprocess, sys', + 'os.write(1, b"leader-diagnostic-no-newline")', + 'subprocess.Popen([sys.executable, "-c", "import time; time.sleep(5)"],', + ' start_new_session=True)', + 'return "escaped"', + ].join('\n'), + bindings: [], + }) + expect(result.error).toBeUndefined() + expect(result.value).toBe('escaped') + expect(result.logs).toContain('leader-diagnostic-no-newline') + }, 8000) + it('reaps a same-group child that ignores SIGTERM and releases the pipes before close', async () => { // The same-group counterpart to the setsid-orphan case above. A descendant // left in the child's OWN process group (no setsid, so `kill(-pid)` reaches