feat(signal): support outbound attachments via signal-cli attachments

The native Signal adapter previously logged-and-dropped any
OutboundFile entries with a TODO. This wires through to signal-cli's
already-supported `send` JSON-RPC `attachments` parameter:

  - New `sendAttachments(platformId, files)` helper writes each
    OutboundFile.data Buffer to a temp file in os.tmpdir(), passes
    the paths to `tcp.rpc('send', { attachments: [...] })`, then
    cleans up the temp files in finally{}.
  - `deliver()` no longer drops files — sends accompanying text
    first via the existing sendText (preserving chunking + textStyle),
    then attachments as a single send call.
  - Filename sanitization replaces `/`, `\`, and `\0` with `_` so
    operator-supplied filenames can't escape tmpdir (CWE-22).
  - Tests cover: single attachment, text+attachment ordering,
    multiple attachments in one send, group destinations, and a
    structural invariant proving sanitized paths stay inside tmpdir().

Total signal tests: 33 → 37 (one stale "drops files" test replaced
by the positive-behavior tests).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

src/channels/signal.test.ts

@@ -548,6 +548,141 @@ describe('SignalAdapter', () => {
     });
   });
 
+  // --- Outbound attachments ---
+
+  describe('deliver — attachments', () => {
+    // Real fs writes happen in tmpdir(); confirm the bytes round-trip and
+    // are cleaned up after deliver returns.
+    it('sends a single attachment via attachments[] param', async () => {
+      const fs = await import('node:fs');
+      const adapter = createAdapter();
+      await adapter.setup(createMockSetup());
+      tcpRef.fakeSocket.write.mockClear();
+
+      await adapter.deliver('+15555550123', null, {
+        kind: 'file',
+        content: {},
+        files: [{ filename: 'report.md', data: Buffer.from('# Report\n\nbody') }],
+      });
+
+      const sendCalls = getRpcCallsForMethod('send');
+      expect(sendCalls.length).toBe(1);
+      const params = sendCalls[0].params as Record<string, unknown>;
+      expect(params.recipient).toEqual(['+15555550123']);
+      expect(params.account).toBe('+15551234567');
+      expect(params.message).toBeUndefined();
+      const paths = params.attachments as string[];
+      expect(paths).toHaveLength(1);
+      expect(paths[0]).toMatch(/signal-out-\d+-[a-z0-9]+-report\.md$/);
+      // Temp file should no longer exist — finally{} cleanup ran
+      expect(fs.existsSync(paths[0])).toBe(false);
+
+      await adapter.teardown();
+    });
+
+    it('sends text first, then attachment, when both are present', async () => {
+      const adapter = createAdapter();
+      await adapter.setup(createMockSetup());
+      tcpRef.fakeSocket.write.mockClear();
+
+      await adapter.deliver('+15555550123', null, {
+        kind: 'file',
+        content: { text: 'Here is the digest' },
+        files: [{ filename: 'digest.md', data: Buffer.from('content') }],
+      });
+
+      const sendCalls = getRpcCallsForMethod('send');
+      expect(sendCalls).toHaveLength(2);
+      // First call: text message
+      expect(sendCalls[0].params).toEqual(
+        expect.objectContaining({ message: 'Here is the digest', recipient: ['+15555550123'] }),
+      );
+      expect((sendCalls[0].params as Record<string, unknown>).attachments).toBeUndefined();
+      // Second call: attachment, no message
+      expect(sendCalls[1].params).toEqual(
+        expect.objectContaining({ recipient: ['+15555550123'] }),
+      );
+      const attachments = (sendCalls[1].params as Record<string, unknown>).attachments as string[];
+      expect(attachments).toHaveLength(1);
+
+      await adapter.teardown();
+    });
+
+    it('sends multiple attachments in a single send call', async () => {
+      const adapter = createAdapter();
+      await adapter.setup(createMockSetup());
+      tcpRef.fakeSocket.write.mockClear();
+
+      await adapter.deliver('+15555550123', null, {
+        kind: 'file',
+        content: {},
+        files: [
+          { filename: 'a.txt', data: Buffer.from('a') },
+          { filename: 'b.png', data: Buffer.from([0x89, 0x50, 0x4e, 0x47]) },
+        ],
+      });
+
+      const sendCalls = getRpcCallsForMethod('send');
+      expect(sendCalls).toHaveLength(1);
+      const attachments = (sendCalls[0].params as Record<string, unknown>).attachments as string[];
+      expect(attachments).toHaveLength(2);
+      expect(attachments[0]).toMatch(/-a\.txt$/);
+      expect(attachments[1]).toMatch(/-b\.png$/);
+
+      await adapter.teardown();
+    });
+
+    it('uses groupId for group destinations', async () => {
+      const adapter = createAdapter();
+      await adapter.setup(createMockSetup());
+      tcpRef.fakeSocket.write.mockClear();
+
+      await adapter.deliver('group:abc123', null, {
+        kind: 'file',
+        content: {},
+        files: [{ filename: 'pic.jpg', data: Buffer.from('jpg') }],
+      });
+
+      const sendCalls = getRpcCallsForMethod('send');
+      expect(sendCalls).toHaveLength(1);
+      const params = sendCalls[0].params as Record<string, unknown>;
+      expect(params.groupId).toBe('abc123');
+      expect(params.recipient).toBeUndefined();
+
+      await adapter.teardown();
+    });
+
+    /**
+     * Defensive test: `OutboundFile.filename` is operator-supplied data, so
+     * the implementation must not let a filename containing path separators
+     * escape the temp directory. We feed an attempt-to-traverse filename and
+     * assert the resolved path stays strictly inside `tmpdir()`.
+     */
+    it('keeps temp paths inside tmpdir even when filename contains path separators', async () => {
+      const path = await import('node:path');
+      const os = await import('node:os');
+      const adapter = createAdapter();
+      await adapter.setup(createMockSetup());
+      tcpRef.fakeSocket.write.mockClear();
+
+      await adapter.deliver('+15555550123', null, {
+        kind: 'file',
+        content: {},
+        files: [{ filename: '../sneaky.txt', data: Buffer.from('x') }],
+      });
+
+      const sendCalls = getRpcCallsForMethod('send');
+      const paths = (sendCalls[0].params as Record<string, unknown>).attachments as string[];
+      const resolvedTmp = path.resolve(os.tmpdir());
+      const resolvedResult = path.resolve(paths[0]);
+      // path.resolve normalizes away any "../"; if sanitization failed, the
+      // result would resolve to tmpdir's parent.
+      expect(resolvedResult.startsWith(resolvedTmp + path.sep)).toBe(true);
+
+      await adapter.teardown();
+    });
+  });
+
   // --- Text styles ---
 
   describe('text styles', () => {
@@ -784,34 +919,6 @@ describe('SignalAdapter', () => {
     });
   });
 
-  // --- Outbound files ---
-
-  describe('outbound files', () => {
-    it('logs a warning and drops unsupported file attachments', async () => {
-      const { log } = await import('../log.js');
-      const warnMock = log.warn as unknown as ReturnType<typeof vi.fn>;
-
-      const adapter = createAdapter();
-      await adapter.setup(createMockSetup());
-      warnMock.mockClear();
-
-      await adapter.deliver('+15555550123', null, {
-        kind: 'text',
-        content: { text: 'with an attachment' },
-        files: [{ filename: 'hi.txt', data: Buffer.from('hi') }],
-      });
-
-      const sendCalls = getRpcCallsForMethod('send');
-      expect(sendCalls.length).toBeGreaterThan(0);
-      expect(warnMock).toHaveBeenCalledWith(
-        'Signal: outbound files not supported, dropping',
-        expect.objectContaining({ platformId: '+15555550123', count: 1 }),
-      );
-
-      await adapter.teardown();
-    });
-  });
-
   // --- setTyping ---
 
   describe('setTyping', () => {

src/channels/signal.ts

@@ -8,9 +8,9 @@
  * Ported from v1 — see v1 source for commit history.
  */
 import { execFileSync, execSync, spawn } from 'node:child_process';
-import { existsSync, readFileSync, unlinkSync } from 'node:fs';
+import { existsSync, readFileSync, unlinkSync, writeFileSync } from 'node:fs';
 import { createConnection, type Socket } from 'node:net';
-import { homedir } from 'node:os';
+import { homedir, tmpdir } from 'node:os';
 import { join } from 'node:path';
 
 import type { ChannelAdapter, ChannelSetup, InboundMessage, OutboundMessage } from './adapter.js';
@@ -744,6 +744,51 @@ export function createSignalAdapter(config: {
     log.info('Signal message sent', { platformId, length: text.length });
   }
 
+  /**
+   * Send one or more file attachments via signal-cli's `send` JSON-RPC, which
+   * accepts an `attachments` array of host filesystem paths. The OutboundFile
+   * Buffer is materialized to an OS temp file so signal-cli can read it, then
+   * removed in the finally block.
+   *
+   * Caption text, if any, is sent first via `sendText` (which handles chunking
+   * + textStyles) — keeps this function single-purpose and avoids a long
+   * caption colliding with signal-cli's per-message size limits.
+   */
+  async function sendAttachments(platformId: string, files: { filename: string; data: Buffer }[]): Promise<void> {
+    if (!connected || !tcp) return;
+    if (files.length === 0) return;
+
+    const tempPaths: string[] = [];
+    for (const file of files) {
+      const safeName = file.filename.replace(/[/\\\0]/g, '_');
+      const tempPath = join(tmpdir(), `signal-out-${Date.now()}-${Math.random().toString(36).slice(2, 8)}-${safeName}`);
+      writeFileSync(tempPath, file.data);
+      tempPaths.push(tempPath);
+    }
+
+    try {
+      const params: Record<string, unknown> = { attachments: tempPaths };
+      if (config.account) params.account = config.account;
+      if (platformId.startsWith('group:')) {
+        params.groupId = platformId.slice('group:'.length);
+      } else {
+        params.recipient = [platformId];
+      }
+      await tcp.rpc('send', params);
+      log.info('Signal attachments sent', { platformId, count: files.length, filenames: files.map((f) => f.filename) });
+    } catch (err) {
+      log.error('Signal: attachment send failed', { platformId, count: files.length, err });
+    } finally {
+      for (const p of tempPaths) {
+        try {
+          unlinkSync(p);
+        } catch {
+          /* best-effort cleanup */
+        }
+      }
+    }
+  }
+
   async function waitForDaemon(): Promise<boolean> {
     const maxWait = 30_000;
     const pollInterval = 1000;
@@ -847,17 +892,6 @@ export function createSignalAdapter(config: {
     },
 
     async deliver(platformId: string, _threadId: string | null, message: OutboundMessage): Promise<string | undefined> {
-      if (message.files && message.files.length > 0) {
-        // Native adapter doesn't yet forward file uploads to signal-cli's
-        // `send --attachment`. Don't silently swallow — operators need to see
-        // that an attachment was requested but not sent.
-        log.warn('Signal: outbound files not supported, dropping', {
-          platformId,
-          count: message.files.length,
-          filenames: message.files.map((f) => f.filename),
-        });
-      }
-
       const content = message.content as Record<string, unknown> | string | undefined;
       let text: string | null = null;
       if (typeof content === 'string') {
@@ -865,9 +899,14 @@ export function createSignalAdapter(config: {
       } else if (content && typeof content === 'object' && typeof content.text === 'string') {
         text = content.text;
       }
-      if (!text) return undefined;
 
-      await sendText(platformId, text);
+      const files = message.files ?? [];
+
+      // Send accompanying text first so it lands above the attachment(s) in
+      // the recipient's chat. Both branches no-op cleanly if their input is
+      // empty, so any combination of (text, files) works.
+      if (text) await sendText(platformId, text);
+      if (files.length > 0) await sendAttachments(platformId, files);
       return undefined;
     },