fix(outbound-attachment): propagate trusted-html marker write failure

wanglu241 · wangwllu · commit a79ecb27ec7f · 2026-07-12T18:19:22.000+08:00
Addresses ClawSweeper P1: swallowed marker write failure. On marker write
failure, best-effort delete the staged file then rethrow so the caller's
normal media-failure fallback handles it instead of a later silent reload
drop. Adds a test.
diff --git a/src/media/outbound-attachment.test.ts b/src/media/outbound-attachment.test.ts
@@ -1,9 +1,14 @@
 // Outbound attachment tests cover media loading rules for outgoing messages.
-import { describe, expect, it, vi } from "vitest";
+import { afterEach, describe, expect, it, vi } from "vitest";
 
 const loadWebMedia = vi.hoisted(() => vi.fn());
 const markTrustedGeneratedHtmlPath = vi.hoisted(() => vi.fn());
 const saveMediaBuffer = vi.hoisted(() => vi.fn());
+const rm = vi.hoisted(() => vi.fn(async () => {}));
+
+vi.mock("node:fs/promises", () => ({
+  rm,
+}));
 
 vi.mock("./web-media.js", () => ({
   loadWebMedia,
@@ -17,6 +22,10 @@ vi.mock("./store.js", () => ({
 const { resolveOutboundAttachmentFromBuffer, resolveOutboundAttachmentFromUrl } =
   await import("./outbound-attachment.js");
 
+afterEach(() => {
+  vi.clearAllMocks();
+});
+
 describe("resolveOutboundAttachmentFromUrl", () => {
   it("preserves the loaded file name when staging outbound media", async () => {
     const buffer = Buffer.from("pdf");
@@ -59,9 +68,38 @@ describe("resolveOutboundAttachmentFromUrl", () => {
 
     expect(markTrustedGeneratedHtmlPath).toHaveBeenCalledWith(
       "/tmp/media/outbound/report---uuid.html",
+      buffer,
     );
   });
 
+  it("propagates a marker write failure and best-effort unlinks the staged file", async () => {
+    const buffer = Buffer.from("<!doctype html><title>x</title>");
+    loadWebMedia.mockResolvedValueOnce({
+      buffer,
+      contentType: "text/html",
+      fileName: "report.html",
+      trustedGeneratedHtmlSource: true,
+    });
+    saveMediaBuffer.mockResolvedValueOnce({
+      path: "/tmp/media/outbound/report---uuid.html",
+      contentType: "text/html",
+    });
+
+    markTrustedGeneratedHtmlPath.mockReset();
+    rm.mockReset();
+    rm.mockResolvedValueOnce(undefined);
+    const markerError = new Error("marker write failed");
+    markTrustedGeneratedHtmlPath.mockRejectedValueOnce(markerError);
+
+    await expect(
+      resolveOutboundAttachmentFromUrl("/tmp/openclaw/report.html", 1024),
+    ).rejects.toThrow(markerError);
+
+    expect(rm).toHaveBeenCalledWith("/tmp/media/outbound/report---uuid.html", {
+      force: true,
+    });
+  });
+
   it("does not mark untrusted outbound HTML staging", async () => {
     const buffer = Buffer.from("<!doctype html><title>x</title>");
     loadWebMedia.mockResolvedValueOnce({
diff --git a/src/media/outbound-attachment.ts b/src/media/outbound-attachment.ts
@@ -1,6 +1,5 @@
 // Outbound attachment helpers prepare media attachments for channel delivery.
-import { logVerbose } from "../globals.js";
-import { formatErrorMessage } from "../infra/errors.js";
+import { rm } from "node:fs/promises";
 import { buildOutboundMediaLoadOptions, type OutboundMediaAccess } from "./load-options.js";
 import { saveMediaBuffer } from "./store.js";
 import { loadWebMedia, markTrustedGeneratedHtmlPath } from "./web-media.js";
@@ -38,12 +37,10 @@ export async function resolveOutboundAttachmentFromUrl(
   // the staged file is treated as an arbitrary outbound HTML and rejected.
   if (media.trustedGeneratedHtmlSource) {
     try {
-      await markTrustedGeneratedHtmlPath(saved.path);
+      await markTrustedGeneratedHtmlPath(saved.path, media.buffer);
     } catch (err) {
-      // best-effort: marker write is non-fatal — if the staged file vanished we'd reject at the gate anyway
-      logVerbose(
-        `outbound-attachment: failed to mark trusted-generated HTML at ${saved.path}: ${formatErrorMessage(err)}`,
-      );
+      await rm(saved.path, { force: true }).catch(() => {});
+      throw err;
     }
   }
   return { path: saved.path, contentType: saved.contentType };
