Преглед изворни кода

fix(code-runtime-python): cap the raw frame length before JSON.parse and bound the log sink

Addresses the review's remaining two items:
- FRAME_PARSE_CAP_BYTES (64 MiB) drops an fd-3 frame whose raw length exceeds
  it BEFORE toString/JSON.parse: the 256 MiB wire ceiling bounds the bytes, not
  the decoded structure, and a compact wide frame near that ceiling could decode
  to far more host memory. A regression test writes a 65 MiB log frame plus a
  normal one and asserts the oversized frame is dropped while the trailing frame
  still lands in logs (fail-before: without the cap the oversized text is parsed
  and admitted, truncating the ledger so the trailing frame is dropped). The
  forged-oversized lower-bound test's frame is reduced to stay under the cap
  while still exercising the truncation path.
- The log sink writes through the def-time bound encode+write primitives (not
  send_sync, whose body resolves _encode_json_plain and self.write_encoded at
  call time), so a rebind cannot break a log frame.
Chinesezjc пре 1 месец
родитељ
комит
ab40136b02

+ 7 - 2
packages/code-runtime/code-runtime-python/py/bootstrap.py

@@ -1032,8 +1032,13 @@ async def _run(channel: ProtocolChannel) -> None:
 
     logs = LogBuffer(
         int(boot["maxLogBytes"]),
-        sink=lambda text, truncated=False: _send_sync_cls(
-            {"type": "log", "text": text, **({"truncated": True} if truncated else {})}
+        # The sink writes through the def-time bound encode+write primitives
+        # (not _send_sync_cls, whose body still resolves _encode_json_plain and
+        # self.write_encoded at call time) so a rebind cannot break a log frame.
+        sink=lambda text, truncated=False: _write_encoded_cls(
+            _encode_plain_cls(
+                {"type": "log", "text": text, **({"truncated": True} if truncated else {})}
+            )
         ),
     )
 

+ 17 - 0
packages/code-runtime/code-runtime-python/src/index.ts

@@ -205,6 +205,18 @@ function materializePyScripts(): string {
  */
 const FRAME_CEILING_BYTES = 256 * 1024 * 1024
 
+/**
+ * A frame's RAW length is capped before JSON.parse: the 256 MiB wire ceiling
+ * bounds the bytes on fd 3, not the decoded structure, and a compact wide
+ * frame near that ceiling (e.g. a huge array of tiny elements) could decode to
+ * far more host memory than the wire admitted — an OOM inside the receive
+ * path. 64 MiB raw admits every legal config (the widest in-tree completion
+ * and binding frames are ~12 MB) while bounding decode amplification to a
+ * roughly constant factor of the wire bytes. A hostile-peer invariant, not a
+ * deployment choice.
+ */
+const FRAME_PARSE_CAP_BYTES = 64 * 1024 * 1024
+
 /**
  * Fragments the unframed fd-3 buffer may hold before they are coalesced into
  * one Buffer, bounding retained per-chunk overhead that {@link
@@ -1351,6 +1363,11 @@ export class PythonCodeRuntime extends CodeRuntime {
             buffered = buffered.subarray(newline + 1)
             /* v8 ignore next -- an empty line comes only from a forged `\n\n` write. */
             if (line.length === 0) continue
+            // Drop an oversized frame BEFORE toString/JSON.parse: the 256 MiB
+            // wire ceiling bounds the raw bytes, not the decoded structure (see
+            // FRAME_PARSE_CAP_BYTES), so a near-ceiling compact wide frame must
+            // not be parsed whole.
+            if (line.length > FRAME_PARSE_CAP_BYTES) continue
             const text = line.toString('utf8')
             // JSON.parse would silently ROUND an integer token outside the
             // safe range before validation could see it, so a forged frame

+ 37 - 8
packages/code-runtime/code-runtime-python/tests/runtime.spec.ts

@@ -1731,6 +1731,34 @@ describe('PythonCodeRuntime — programs and bindings', () => {
     expect(result.error?.kind).not.toBe('worker-exit')
   }, 15_000)
 
+  it('drops an fd-3 frame whose raw length exceeds the parse cap before decoding it', async () => {
+    // The 256 MiB wire ceiling bounds the RAW frame bytes, not the decoded
+    // structure; a compact wide frame near that ceiling could decode to far
+    // more host memory. The receive path caps raw frames at
+    // FRAME_PARSE_CAP_BYTES before toString/JSON.parse and drops the oversized
+    // one like any junk frame, so the following normal frame is still
+    // processed. Fail-before: without the cap the oversized log text would be
+    // parsed and admitted (truncating the ledger), and the trailing frame
+    // would be dropped as post-truncation instead of appearing in logs.
+    const { runtime } = await setup({ maxWallMs: 60_000 })
+    const result = await runtime.run({
+      program: [
+        'import os',
+        // One frame just past the 64 MiB parse cap.
+        'os.write(3, b"{\\"type\\":\\"log\\",\\"text\\":\\"" + b"a" * (65 * 1024 * 1024) + b"\\"}\\n")',
+        'os.write(3, b"{\\"type\\":\\"log\\",\\"text\\":\\"after-cap\\"}\\n")',
+        'return "done"',
+      ].join('\n'),
+      bindings: [],
+    })
+    expect(result.error).toBeUndefined()
+    expect(result.value).toBe('done')
+    // The oversized frame was dropped before parse; the trailing frame was
+    // processed normally (its text survives in logs).
+    expect(result.logs).toContain('after-cap')
+    expect(result.logs.some(line => line.length > 1024 * 1024)).toBe(false)
+  }, 90_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
@@ -4574,20 +4602,21 @@ describe('PythonCodeRuntime — hostile peer', () => {
   }, 8000)
 
   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 `\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
-    // the child's address space generous enough to BUILD the frame.
+    // A forged `log` frame carrying a control-heavy string: NULs escape
+    // several-fold (one NUL -> six bytes `\u0000`). The raw frame stays under
+    // the host's 64 MiB parse cap (4 MiB of `\u0000` text = 24 MiB raw) while
+    // the escaped form would be ~24 MiB. Charging it required building that
+    // escaped copy first, so a 32-byte maxLogBytes could still force a large
+    // host allocation. The cheap `length + 3` lower bound truncates it instead.
+    // The host's own heap is what is under test, so keep the child's address
+    // space generous enough to BUILD the frame.
     const { runtime } = await setup({ maxLogBytes: 128, addressSpaceMb: 1024, maxWallMs: 60_000 })
     const before = process.memoryUsage().heapUsed
     const result = await runtime.run({
       program: [
         'import os',
         // Written as a raw frame so the child's own ledger never sees it.
-        'os.write(3, b\'{"type":"log","text":"\' + b"\\\\u0000" * (24 * 1024 * 1024) + b\'"}\\n\')',
+        'os.write(3, b\'{"type":"log","text":"\' + b"\\\\u0000" * (4 * 1024 * 1024) + b\'"}\\n\')',
         'return "settled"',
       ].join('\n'),
       bindings: [],