fix(agents): guard delivery-evidence attachment recursion against cycles

Pick-cat · Pick-cat · commit 3c6b46cb56f9 · 2026-06-26T22:13:02.000+08:00
diff --git a/src/agents/embedded-agent-runner/delivery-evidence.test.ts b/src/agents/embedded-agent-runner/delivery-evidence.test.ts
@@ -0,0 +1,46 @@
+import { describe, expect, it } from "vitest";
+import { collectDeliveredMediaUrls } from "./delivery-evidence.js";
+
+describe("collectDeliveredMediaUrls attachment recursion", () => {
+  it("collects media URLs across nested attachments", () => {
+    const urls = collectDeliveredMediaUrls({
+      payloads: [
+        {
+          url: "https://example.com/root.png",
+          attachments: [
+            { mediaUrl: "https://example.com/child.png" },
+            { attachments: [{ filePath: "/tmp/grandchild.jpg" }] },
+          ],
+        },
+      ],
+    });
+    expect(urls.toSorted()).toEqual([
+      "/tmp/grandchild.jpg",
+      "https://example.com/child.png",
+      "https://example.com/root.png",
+    ]);
+  });
+
+  it("does not overflow the stack on a self-referential attachments cycle", () => {
+    // Payloads arrive as in-process `unknown` objects; a malformed self-referential
+    // attachments chain previously recursed until the stack overflowed.
+    const cyclic: Record<string, unknown> = { url: "https://example.com/loop.png" };
+    cyclic.attachments = [cyclic];
+
+    let urls: string[] = [];
+    expect(() => {
+      urls = collectDeliveredMediaUrls({ payloads: [cyclic] });
+    }).not.toThrow();
+    expect(urls).toEqual(["https://example.com/loop.png"]);
+  });
+
+  it("does not overflow on a mutual attachments cycle", () => {
+    const a: Record<string, unknown> = { mediaUrl: "https://example.com/a.png" };
+    const b: Record<string, unknown> = { mediaUrl: "https://example.com/b.png" };
+    a.attachments = [b];
+    b.attachments = [a];
+
+    const urls = collectDeliveredMediaUrls({ payloads: [a] });
+    expect(urls.toSorted()).toEqual(["https://example.com/a.png", "https://example.com/b.png"]);
+  });
+});
diff --git a/src/agents/embedded-agent-runner/delivery-evidence.ts b/src/agents/embedded-agent-runner/delivery-evidence.ts
@@ -80,7 +80,19 @@ function collectStringValues(value: unknown, output: Set<string>) {
   }
 }
 
-function collectMediaUrlsFromRecord(record: Record<string, unknown>, output: Set<string>) {
+function collectMediaUrlsFromRecord(
+  record: Record<string, unknown>,
+  output: Set<string>,
+  // Payloads arrive as in-process `unknown` objects, so a malformed
+  // self-referential `attachments` chain would recurse until the stack
+  // overflows. Track visited records to bound the descent, matching
+  // redactStringsDeep in embedded-agent-subscribe.tools.ts.
+  seen = new WeakSet<object>(),
+) {
+  if (seen.has(record)) {
+    return;
+  }
+  seen.add(record);
   collectStringValues(record.mediaUrl, output);
   collectStringValues(record.mediaUrls, output);
   collectStringValues(record.path, output);
@@ -90,7 +102,7 @@ function collectMediaUrlsFromRecord(record: Record<string, unknown>, output: Set
   if (Array.isArray(attachments)) {
     for (const attachment of attachments) {
       if (attachment && typeof attachment === "object" && !Array.isArray(attachment)) {
-        collectMediaUrlsFromRecord(attachment as Record<string, unknown>, output);
+        collectMediaUrlsFromRecord(attachment as Record<string, unknown>, output, seen);
       }
     }
   }

Original file line number	Diff line number	Diff line change
`@@ -80,7 +80,19 @@ function collectStringValues(value: unknown, output: Set<string>) {`
`80`	`80`	`}`
`81`	`81`	`}`
`82`	`82`
`83`		`-function collectMediaUrlsFromRecord(record: Record<string, unknown>, output: Set<string>) {`
	`83`	`+function collectMediaUrlsFromRecord(`
	`84`	`+ record: Record<string, unknown>,`
	`85`	`+ output: Set<string>,`
	`86`	+ // Payloads arrive as in-process `unknown` objects, so a malformed
	`87`	+ // self-referential `attachments` chain would recurse until the stack
	`88`	`+ // overflows. Track visited records to bound the descent, matching`
	`89`	`+ // redactStringsDeep in embedded-agent-subscribe.tools.ts.`
	`90`	`+ seen = new WeakSet<object>(),`
	`91`	`+) {`
	`92`	`+ if (seen.has(record)) {`
	`93`	`+ return;`
	`94`	`+ }`
	`95`	`+ seen.add(record);`
`84`	`96`	`collectStringValues(record.mediaUrl, output);`
`85`	`97`	`collectStringValues(record.mediaUrls, output);`
`86`	`98`	`collectStringValues(record.path, output);`
`@@ -90,7 +102,7 @@ function collectMediaUrlsFromRecord(record: Record<string, unknown>, output: Set`
`90`	`102`	`if (Array.isArray(attachments)) {`
`91`	`103`	`for (const attachment of attachments) {`
`92`	`104`	`if (attachment && typeof attachment === "object" && !Array.isArray(attachment)) {`
`93`		`- collectMediaUrlsFromRecord(attachment as Record<string, unknown>, output);`
	`105`	`+ collectMediaUrlsFromRecord(attachment as Record<string, unknown>, output, seen);`
`94`	`106`	`}`
`95`	`107`	`}`
`96`	`108`	`}`