fix(code-runtime-python): reserve the log array envelope byte, unblock SIGXCPU before re-raise

Addresses the review's two remaining code warnings and the three suggestions: - Log ledgers (host and child) start one byte below the budget, reserving the serialized outer-array envelope (two brackets and n-1 commas over n entries' separators); the exact-zero test moves to maxLogBytes 104 and a new exact-limit case pins that maxLogBytes 5 admits ['a'] (5 bytes) while 4 truncates to the marker alone. - die_if_cpu_exhausted unblocks SIGXCPU (pthread_sigmask SIG_UNBLOCK, captured at import, None-guarded for Windows) before re-delivering it, so a program that masks SIGXCPU, burns past the soft limit, and returns is still classified as a timeout; a regression test pins the masked path. - ast.parse passes filename="<model>" so parse-time syntax diagnostics carry the same source label as compile and runtime tracebacks; the syntax-error test asserts the label. - The NUL-escape test comments use the true six-byte JSON escape \u0000 instead of the caret notation; the README Known Limitations (en + zh) records that PID-reuse protection is inert on macOS; a combined-rebind regression test pins BaseException plus the traceback reporter rebinding together.
2026-09-11 04:00:38 +00:00 · 2026-08-31 14:41:08 +08:00
parent 96597c5ed8
commit 4e0d77c1d6
6 changed files with 141 additions and 12 deletions
@@ -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 packages/code-runtime/code-runtime-python/README.md
-README.md: 3522c7726460b10f0366c56566336465a76491c5
-README.zh.md: 95984cf1ae3677aff4a5c4565bed87a57701d086
+README.md: 9184573748a2dd32c97fe92b6ae91ab89279516a
+README.zh.md: ff4698f06004a2da8e77cb8772a2b23d462d2361
@@ -36,6 +36,7 @@ No direct invalidation; the named consumer owns any request-prefix changes.

 - **The cross-language guard covers executed values and frame field sets, not field types** — `tests/protocol-mirror.e2e.ts` compares `PROTOCOL_FD`, the log truncation marker, and each `TypedDict`'s required and optional fields against a real `python3`. Comparing field types across TypeScript and Python has no mechanical equivalent here, so review plus the backend's real-subprocess suite owns type-level drift.
 - **`RLIMIT_AS` is not enforced on macOS** — the dyld shared cache mapped into every process at exec exceeds any practical address-space cap, and the kernel rejects the `setrlimit` call, so `addressSpaceMb` is skipped there. `cpuSeconds` and `maxWallMs` still bound every run.
+- **PID-reuse protection is inert on macOS** — `readProcessStart` reads `/proc/<pid>/stat`, which Darwin does not provide, so the identity re-check that guards `killGroup` against signalling a recycled pgid always passes there; the guard degrades to the pre-existing behavior rather than paying a `ps` fork on a teardown path. The process-group teardown and the `closeDeadline` bound still contain the run.
 - **A descendant that calls `setsid()` / `start_new_session=True` escapes teardown.** Termination signals the child's process group with `kill(-pid)`; a descendant that moves itself into a fresh session is no longer in that group and no signal reaches it. If it also releases the inherited stdout/stderr/fd-3 pipes, the leader's `close` still settles the run, and after the `closeDeadline` bound the fiber goes quiescent while that orphan keeps running. This is the containment boundary, not a security one — model code has bash-equivalent trust, and a bash tool can `setsid` away just the same. Reaching such an orphan would require tracking every descendant pid (as the bash-local backend's process-inspector does) and is deferred; the process-group teardown reaps everything that stays in the group.
 - **A combined log-and-value peak is not modelled by the load gate.** Each budget is checked against `addressSpaceMb` on its own. A model daemon thread that keeps writing while the completion value is metered and framed can refill the log pending toward `maxLogBytes` during that window, so the two peaks add in a way no gate admits or rejects. A gate over `(maxLogBytes + maxValueBytes)` was considered and deferred: its discriminating case cannot be scheduled deterministically under `RLIMIT_AS`, so the gate would only prove its own arithmetic. When the combined peak is reached the run dies as `worker-exit` -- containment holds and only the failure classification is degraded.
 - **A 1-second dual-limit `ulimit -t 1` CPU overrun is reported as `worker-exit`, not a timeout.** When the host starts under a hard CPU limit equal to the soft (`ulimit -t N` sets both) and that limit is 1, `_clamped` cannot lower the soft to 0, so the kernel SIGKILLs the busy loop in the same tick and SIGXCPU is never delivered. The host classifies a CPU overrun only on `signal === 'SIGXCPU'`, so the overrun is reported as `worker-exit`. For a dual limit of 2 or more the soft is lowered by one unit, SIGXCPU fires, and the run is a timeout. Containment holds in both cases; only the classification is degraded.
@@ -36,6 +36,7 @@ host 与 CPython 子进程在子进程的 fd 3 上交换一个无版本号的 JS

 - **跨语言 guard 覆盖执行值与帧字段集，但不覆盖字段类型** —— `tests/protocol-mirror.e2e.ts` 使用真实 `python3` 比较 `PROTOCOL_FD`、日志截断标记，以及每个 `TypedDict` 的必填和可选字段。跨 TypeScript 与 Python 比较字段类型在此没有机械等价物，因此类型级漂移由 review 加后端真子进程套件负责。
 - **`RLIMIT_AS` 在 macOS 上不施加** —— 在 exec 时映射进每个进程的 dyld 共享缓存超过任何实际的地址空间上限，内核会拒绝该 `setrlimit` 调用，故 `addressSpaceMb` 在那里被跳过。`cpuSeconds` 与 `maxWallMs` 仍约束每一次运行。
+- **PID 复用防护在 macOS 上失效** —— `readProcessStart` 读取 `/proc/<pid>/stat`，Darwin 不提供它，因此防止 `killGroup` 对已回收的 pgid 发信号的同一性复检在那里恒通过；该防护退化为既有行为，而非在拆卸路径上付出一次 `ps` fork。进程组拆卸与 `closeDeadline` 上界仍约束该次运行。
 - **调用 `setsid()` ／ `start_new_session=True` 的后代会逃出 teardown。** 终止是用 `kill(-pid)` 向子进程的进程组发信号；一个把自己移入新会话的后代已不在该进程组内，任何信号都到不了它。若它同时释放了继承而来的 stdout／stderr／fd-3 管道，leader 的 `close` 仍会结算该次运行，在 `closeDeadline` 到界之后 fiber 变为完全停稳，而那个孤儿仍在运行。这是 containment 边界，而非安全边界——模型代码具有等同 bash 的信任级别，一个 bash 工具同样能 `setsid` 逃逸。要够到这样的孤儿需要追踪每一个后代 pid（如 bash-local 后端的 process-inspector 所做），此项已推迟；进程组 teardown 会回收所有留在组内的进程。
 - **日志与完成值的叠加峰值未被加载门建模。** 每项预算都是各自对照 `addressSpaceMb` 检查的。模型的 daemon 线程可以在完成值被计量并分帧的窗口内持续写入、把日志 pending 重填到接近 `maxLogBytes`，于是两个峰值以任何门都不曾放行也不曾拒绝的方式相加。对 `(maxLogBytes + maxValueBytes)` 设门的方案经评估后推迟：它的判别用例无法在 `RLIMIT_AS` 之下确定性地构造出来，因此该门只能证明自己的算术。叠加峰值被触及时该次运行死为 `worker-exit`——containment 仍然成立，只是失败分类失真。
 - **1 秒双限 `ulimit -t 1` 下的 CPU 超限会被报告为 `worker-exit`，而非超时。** 当宿主在一个硬 CPU 限制等于软限制（`ulimit -t N` 同时设置两者）且该限制为 1 的环境下启动时，`_clamped` 无法把软限制降到 0，因此内核在同一 tick 直接 SIGKILL 忙循环，SIGXCPU 永不送达。宿主只在 `signal === 'SIGXCPU'` 时把 CPU 超限分类为超时，因此该超限被报告为 `worker-exit`。当双限为 2 或更大时，软限制会被降低一个单位，SIGXCPU 触发，该次运行成为超时。两种情况 containment 都成立；只是分类被降级。
@@ -98,7 +98,15 @@ class LogBuffer:

    def __init__(self, max_bytes: int, sink) -> None:
        self._max_bytes = max_bytes
-        self._remaining = max_bytes
+        # The ledger starts one byte below max_bytes: each entry is charged its
+        # JSON-string cost plus one separator byte, and the serialized outer
+        # logs array adds one more byte of envelope (two brackets and n-1 commas
+        # over n entries' separators), so a result that exactly exhausts the
+        # ledger would serialize to max_bytes + 1. Reserving that byte keeps an
+        # admitted result within the configured cap; the truncation-marker entry
+        # is envelope, not payload, and rides uncharged (``_max_bytes`` stays the
+        # configured value for the marker's message text).
+        self._remaining = max_bytes - 1
        self._truncated = False
        # Re-entrant so a caller may hold it across a compound read-modify-write
        # (``_LogStream.write`` reads ``remaining`` several times and then calls
@@ -1044,7 +1052,12 @@ async def _run(channel: ProtocolChannel) -> None:
    max_value_bytes = int(boot["maxValueBytes"])
    done: dict[str, Any] | str
    try:
-        module = ast.parse(program)
+        # filename="<model>" keeps the source label consistent with the later
+        # compile(wrapped, "<model>", ...) and the runtime traceback filtering
+        # (safe_model_traceback drops frames whose filename is not "<model>");
+        # the default "<unknown>" would leak a different label into model-visible
+        # syntax diagnostics.
+        module = ast.parse(program, filename="<model>")
        wrapper = ast.AsyncFunctionDef(
            name="__dsh_main__",
            args=ast.arguments(
@@ -1941,6 +1954,12 @@ def _make_cpu_enforcer() -> Any:
    sigxcpu = signal.SIGXCPU
    kill = os.kill
    getpid = os.getpid
+    # SIGXCPU unmasking primitives for the re-raise below: a program can mask
+    # the signal and return past the soft limit, so the re-delivered signal
+    # must be unblocked first. Captured here (import time) so a rebind cannot
+    # defeat them; ``None`` on platforms without ``pthread_sigmask`` (Windows).
+    pthread_sigmask = getattr(signal, "pthread_sigmask", None)
+    sig_unblock = getattr(signal, "SIG_UNBLOCK", None)

    def die_if_cpu_exhausted(cpu_seconds: int) -> None:
        """Die by re-delivered SIGXCPU when the CPU budget is already spent.
@@ -1992,6 +2011,16 @@ def _make_cpu_enforcer() -> Any:
        kids = getrusage(rusage_children)
        spent = own.ru_utime + own.ru_stime + kids.ru_utime + kids.ru_stime
        if spent >= cpu_seconds:
+            # A program can mask SIGXCPU (``pthread_sigmask(SIG_BLOCK, ...)``),
+            # burn past the soft limit, and return during the soft-to-hard gap;
+            # the re-delivered SIGXCPU below would then stay PENDING and the
+            # child would exit normally with a success result. Unblock it on the
+            # current thread before re-raising, so the signal is delivered and
+            # the host sees the kernel-authoritative timeout classification. On
+            # platforms without ``pthread_sigmask`` (Windows) the signal is not
+            # maskable this way, so the call is guarded.
+            if pthread_sigmask is not None:
+                pthread_sigmask(sig_unblock, (sigxcpu,))
            set_signal(sigxcpu, sig_dfl)
            kill(getpid(), sigxcpu)

@@ -969,7 +969,14 @@ export class PythonCodeRuntime extends CodeRuntime {
      const logs: string[] = []

      // One host-side ledger covers normal frames, forged frames, and stray stdout bytes.
-      let logBudget = this.config.maxLogBytes
+      // The ledger starts one byte below maxLogBytes: each entry is charged its
+      // JSON-string cost plus one separator byte, and the serialized outer logs
+      // array adds one more byte of envelope (two brackets and n-1 commas over n
+      // entries' separators), so a result that exactly exhausts the ledger would
+      // serialize to maxLogBytes + 1. Reserving that byte keeps an admitted
+      // result within the configured cap; the truncation-marker entry is
+      // envelope, not payload, and rides uncharged.
+      let logBudget = this.config.maxLogBytes - 1
      let logsTruncated = false
      const admit = (text: string): void => {
        // Post-truncation admits are no-ops: once the ledger has truncated, the
@@ -539,6 +539,33 @@ describe('PythonCodeRuntime — inherited resource limits', () => {
    expect(result.error?.message).toContain('CPU time exhausted')
  }, 15_000)

+  it('reports a timeout when a program masks SIGXCPU and returns past the soft limit', async () => {
+    // A program can mask SIGXCPU (pthread_sigmask SIG_BLOCK), burn past the
+    // soft CPU limit, and return during the soft-to-hard gap. The settlement
+    // recheck (`die_if_cpu_exhausted`) must UNBLOCK the signal before
+    // re-delivering it, or the SIGXCPU stays pending and the child exits
+    // normally with a success result. With the unblock, the re-delivered
+    // SIGXCPU (default disposition) terminates the child and the host
+    // classifies the run as a timeout. Fail-before: without the unblock the
+    // run reports `value: "escaped"` and no error. The masking is guarded by
+    // hasattr so the case is a no-op on platforms without pthread_sigmask.
+    const { runtime } = await setup({ cpuSeconds: 1, maxWallMs: 12_000 })
+    const result = await runtime.run({
+      program: [
+        'import signal, time',
+        'if hasattr(signal, "pthread_sigmask"):',
+        '    signal.pthread_sigmask(signal.SIG_BLOCK, {signal.SIGXCPU})',
+        'end = time.perf_counter() + 1.05',
+        'while time.perf_counter() < end:',
+        '    pass',
+        'return "escaped"',
+      ].join('\n'),
+      bindings: [],
+    })
+    expect(result.error?.kind).toBe('timeout')
+    expect(result.value).toBeUndefined()
+  }, 20_000)
+
  it('rechecks CPU at settlement against the effective inherited soft limit', async () => {
    // The settlement-time CPU recheck must compare against the EFFECTIVE soft
    // limit (`_clamped` may have lowered it to a stricter inherited value), not
@@ -848,7 +875,7 @@ describe('PythonCodeRuntime — programs and bindings', () => {

  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
+    // raw length well under the budget, but each NUL serializes to `\u0000` (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
@@ -1294,6 +1321,10 @@ describe('PythonCodeRuntime — programs and bindings', () => {
    })
    expect(result.error?.kind).toBe('exception')
    expect(result.error?.message).toContain('SyntaxError')
+    // The parse-time diagnostic must carry the same source label as compile and
+    // runtime tracebacks (ast.parse passes filename="<model>"); a stale
+    // "<unknown>" label would leak an inconsistent origin to the model.
+    expect(result.error?.message).toContain('File \"<model>\"')
    expect(result.value).toBeUndefined()
  })

@@ -1536,6 +1567,35 @@ describe('PythonCodeRuntime — programs and bindings', () => {
    expect(result.error?.kind).not.toBe('worker-exit')
  }, 15_000)

+  it('still reports the exception when BaseException and the traceback reporter are rebound together', async () => {
+    // The two rebind families compose: `__main__.BaseException = ValueError`
+    // must not change which class the `_run` catch resolves (it is a pre-program
+    // local), and a rebound reporter (`_SAFE_MODEL_TRACEBACK`/`_cap_message`/
+    // `_model_traceback`/`_UNRENDERABLE_DIAGNOSTIC`) must not break the done
+    // frame — `safe_model_traceback` holds its primitives as import-time closure
+    // cells. A `KeyError` (not a `ValueError` subclass) escapes a catch that
+    // resolves to the rebound class, so without the local binding the run would
+    // misreport as `worker-exit`; with it, the run reports the exception and the
+    // fallback reporter still produces the fixed literal.
+    const { runtime } = await setup({ maxWallMs: 10_000 })
+    const result = await runtime.run({
+      program: [
+        'import __main__',
+        'def boom(*a, **k):',
+        '    raise RuntimeError("hijacked")',
+        '__main__.BaseException = ValueError',
+        '__main__._SAFE_MODEL_TRACEBACK = boom',
+        '__main__._cap_message = boom',
+        '__main__._model_traceback = boom',
+        '__main__._UNRENDERABLE_DIAGNOSTIC = boom',
+        'raise KeyError("real failure")',
+      ].join('\n'),
+      bindings: [],
+    })
+    expect(result.error?.kind).toBe('exception')
+    expect(result.error?.kind).not.toBe('worker-exit')
+  }, 15_000)
+
  it('bounds an over-cap exception-group nesting on the copy', async () => {
    // Exception groups link through `exceptions`, not the cause/context
    // dunders, so the cap has to count that edge too — otherwise a deeply
@@ -3360,7 +3420,7 @@ describe('PythonCodeRuntime — hostile peer', () => {
  }, 20_000)

  it('rejects a control-heavy oversized completion on its length, not its escaped copy', async () => {
-    // Every "\x00" escapes to the six bytes "\^@", so the escaped form of a
+    // Every "\x00" escapes to the six bytes "\u0000", so the escaped form of a
    // 40 MB string is ~240 MB. The walk must refuse on the cheap
    // `len(current) + 2` lower bound; the 384 MiB address space holds the raw
    // string but not its escaped expansion, so a pre-escape check dies on
@@ -3614,7 +3674,7 @@ describe('PythonCodeRuntime — hostile peer', () => {

  it('caps a control-heavy exception diagnostic by its serialized cost, not raw bytes', async () => {
    // The diagnostic crosses fd 3 inside a JSON frame where a control character
-    // escapes sixfold (a NUL is one raw byte, six as `\^@`). Capping by raw
+    // escapes sixfold (a NUL is one raw byte, six as `\u0000`). Capping by raw
    // UTF-8 length would let a NUL-heavy message near maxValueBytes serialize to
    // ~6x that and breach the frame ceiling — the silent worker-exit inversion
    // the load-time cap check exists to prevent. The child meters the diagnostic
@@ -3801,7 +3861,8 @@ describe('PythonCodeRuntime — hostile peer', () => {

  it('marks a dropped tail when the ledger lands on exactly zero remaining', async () => {
    // One 100-character line costs 103 serialized bytes (quotes + separator),
-    // consuming a 103-byte budget EXACTLY. Landing on zero never trips
+    // consuming a 104-byte budget minus the 1-byte array-envelope reservation
+    // (104 - 1 = 103) EXACTLY. Landing on zero never trips
    // LogBuffer's "cost > remaining" branch, so `_truncated` stays unset and the
    // stream's own `remaining > 0` guard silently discarded the unscanned tail —
    // the run reported a complete log while dropping text. The tail must be
@@ -3810,8 +3871,10 @@ describe('PythonCodeRuntime — hostile peer', () => {
    // buffered-empty path used to force the marker out incidentally.) A single
    // wide line is used rather than many narrow ones so the CHILD ledger is the
    // one that lands on zero: the host's identical ledger truncates first when
-    // many small entries precede the long marker text.
-    const { runtime } = await setup({ maxLogBytes: 103, maxWallMs: 10_000 })
+    // many small entries precede the long marker text. `["y"*100]` serializes to
+    // exactly 104 bytes (103 payload + 1 envelope), so 104 is the smallest
+    // budget that admits the entry.
+    const { runtime } = await setup({ maxLogBytes: 104, maxWallMs: 10_000 })
    const result = await runtime.run({
      program: ['print("y" * 100 + "\\n" + "z" * 10, end="")', 'return "done"'].join('\n'),
      bindings: [],
@@ -3824,6 +3887,34 @@ describe('PythonCodeRuntime — hostile peer', () => {
    expect(result.logs.some(line => line.includes('z'))).toBe(false)
  }, 15_000)

+  it('keeps an admitted log within the serialized array envelope at the exact limit', async () => {
+    // Each entry is charged its JSON-string cost plus one separator byte, and
+    // the serialized outer logs array adds one more byte of envelope (two
+    // brackets and n-1 commas). The ledgers reserve that byte, so a result that
+    // exactly exhausts the ledger still serializes within the configured cap:
+    // `["a"]` is 5 bytes, and `maxLogBytes: 5` admits it (ledger 4 = the 3-byte
+    // quoted entry + 1 separator), while `maxLogBytes: 4` (ledger 3) truncates
+    // to the marker alone.
+    const { runtime } = await setup({ maxLogBytes: 5, maxWallMs: 10_000 })
+    const result = await runtime.run({
+      program: ['print("a")', 'return "done"'].join('\n'),
+      bindings: [],
+    })
+    expect(result.error).toBeUndefined()
+    expect(result.value).toBe('done')
+    expect(result.logs).toContain('a')
+    expect(result.logs.some(line => line.includes('log capture truncated'))).toBe(false)
+
+    const tight = await setup({ maxLogBytes: 4, maxWallMs: 10_000 })
+    const result2 = await tight.runtime.run({
+      program: ['print("a")', 'return "done"'].join('\n'),
+      bindings: [],
+    })
+    expect(result2.error).toBeUndefined()
+    expect(result2.logs).not.toContain('a')
+    expect(result2.logs.some(line => line.includes('log capture truncated'))).toBe(true)
+  }, 15_000)
+
  it('charges the JSON-escaped cost of control characters against the log ledger', async () => {
    // A NUL renders as \u0000 (6 bytes) in the serialized outer logs; the
    // ledger must charge that expansion, or a control-character flood admits
@@ -4248,7 +4339,7 @@ describe('PythonCodeRuntime — hostile peer', () => {
  it('drops a forged oversized log frame on its code-unit lower bound, before escaping it', async () => {
    // A forged `log` frame carrying a control-heavy string sits below the
    // 256 MiB fd-3 frame ceiling but escapes several-fold: 24 MiB of NULs
-    // becomes ~144 MiB of `\^@`. Charging it required building that escaped
+    // becomes ~144 MiB of `\u0000`. Charging it required building that escaped
    // copy first, so a 32-byte maxLogBytes could still force a
    // hundreds-of-megabytes host allocation. The cheap `length + 3` lower bound
    // truncates it instead. The host's own heap is what is under test, so keep