Harden inbound envelope handling and iOS attachment outbox

ios/ClawChat/Sources/ClawChat/Conversation.swift

@@ -31,6 +31,15 @@ public struct ChatAttachment: Equatable, Sendable {
         self.filename = filename
         self.mime = mime
     }
+
+    /// The wire-level kind this attachment travels as.
+    public var wireKind: Wire.MediaKind {
+        switch kind {
+        case .image: return .image
+        case .audio: return .audio
+        case .file:  return .file
+        }
+    }
 }
 
 public enum ChatDeliveryState: Equatable, Sendable {
@@ -237,6 +246,27 @@ public final class Conversation: ObservableObject {
                 messages[idx] = msg
                 saveToStore()
             }
+            // A message carrying an attachment has to go back out over the
+            // media path — the text path would transmit only its caption.
+            if let attachment = m.attachments.first {
+                let caption = m.text.isEmpty ? nil : m.text
+                Task { [weak self] in
+                    do {
+                        _ = try await conn.send(
+                            media: attachment.wireKind,
+                            bytes: attachment.bytes,
+                            filename: attachment.filename,
+                            mime: attachment.mime,
+                            caption: caption,
+                            messageId: m.id
+                        )
+                        self?.updateDelivery(id: m.id, to: .sent)
+                    } catch {
+                        self?.updateDelivery(id: m.id, to: deliveryStateForSendError(error))
+                    }
+                }
+                continue
+            }
             Task { [weak self] in
                 do {
                     _ = try await conn.send(text: m.text, messageId: m.id)
@@ -299,24 +329,25 @@ public final class Conversation: ObservableObject {
         filename: String? = nil,
         mime: String? = nil
     ) {
-        guard let conn = connection, conn.isReady else { return }
         let caption = draft.trimmingCharacters(in: .whitespacesAndNewlines)
         draft = ""
         let messageId = Wire.newId()
+        let attachment = ChatAttachment(kind: kind, bytes: bytes, filename: filename, mime: mime)
         let chat = ChatMessage(
             id: messageId,
             sender: .me,
             text: caption,
-            attachments: [ChatAttachment(kind: kind, bytes: bytes, filename: filename, mime: mime)],
+            attachments: [attachment],
             delivery: .sending
         )
+        // Append and persist before touching the connection, mirroring send().
+        // The message survives an app restart while still in .sending state.
         messages.append(chat)
-        let wireKind: Wire.MediaKind
-        switch kind {
-        case .image: wireKind = .image
-        case .audio: wireKind = .audio
-        case .file:  wireKind = .file
-        }
+        saveToStore()
+        // If we're not connected, leave it as .sending — drainOutbox() will
+        // pick it up the moment the connection becomes ready.
+        guard let conn = connection, conn.isReady else { return }
+        let wireKind = attachment.wireKind
         Task { [weak self] in
             do {
                 _ = try await conn.send(

ios/ClawChat/Tests/ClawChatTests/ConversationOutboxTests.swift

@@ -98,4 +98,87 @@ final class ConversationOutboxTests: XCTestCase {
         XCTAssertEqual(c.messages.count, 1)
         XCTAssertEqual(c.messages.first?.delivery, .sending)
     }
+
+    // MARK: - Attachments queued while offline
+
+    func testOfflineAttachmentIsQueuedAsSending() {
+        // sendAttachment with no connection must behave like send(): append
+        // the message in .sending rather than discarding it.
+        let c = Conversation()
+        c.messageStore = MessageStore(profileDir: workDir)
+        c.draft = "look at this"
+        c.sendAttachment(
+            kind: .image,
+            bytes: Data([0xFF, 0xD8, 0xFF, 0xE0]),
+            filename: "cat.jpg",
+            mime: "image/jpeg"
+        )
+        XCTAssertEqual(c.messages.count, 1)
+        let m = try? XCTUnwrap(c.messages.first)
+        XCTAssertEqual(m?.delivery, .sending)
+        XCTAssertEqual(m?.sender, .me)
+        XCTAssertEqual(m?.text, "look at this", "caption should ride along with the attachment")
+        XCTAssertEqual(m?.attachments.count, 1)
+        XCTAssertEqual(m?.attachments.first?.filename, "cat.jpg")
+        XCTAssertEqual(c.draft, "", "draft is consumed as the caption")
+    }
+
+    func testOfflineAttachmentSurvivesAppRestartWithBytesIntact() {
+        let bytes = Data([0x01, 0x02, 0x03, 0x04, 0x05])
+        do {
+            let c = Conversation()
+            c.messageStore = MessageStore(profileDir: workDir)
+            c.sendAttachment(kind: .file, bytes: bytes, filename: "doc.bin", mime: "application/octet-stream")
+            XCTAssertEqual(c.messages.count, 1)
+        }
+
+        // Fresh Conversation + store on the same dir, as after a cold launch.
+        let c2 = Conversation()
+        c2.messageStore = MessageStore(profileDir: workDir)
+        c2.loadFromStoreIfAvailable()
+        XCTAssertEqual(c2.messages.count, 1, "the queued attachment must be persisted, not dropped")
+        let m = c2.messages.first
+        XCTAssertEqual(m?.delivery, .sending)
+        XCTAssertEqual(m?.attachments.count, 1, "attachment must survive the round-trip")
+        XCTAssertEqual(m?.attachments.first?.bytes, bytes, "attachment bytes must be byte-identical")
+        XCTAssertEqual(m?.attachments.first?.kind, .file)
+        XCTAssertEqual(m?.attachments.first?.mime, "application/octet-stream")
+    }
+
+    func testRehydratedAttachmentIsSelectedByTheDrainQueue() {
+        // drainOutbox re-sends every from-me message left in .sending/.failed.
+        // A rehydrated media message must qualify, and must still carry the
+        // attachment that the media send path needs.
+        let c = Conversation()
+        c.messageStore = MessageStore(profileDir: workDir)
+        c.draft = "caption"
+        c.sendAttachment(kind: .audio, bytes: Data([0x11, 0x22]), filename: "vm.m4a", mime: "audio/mp4")
+        c.draft = "text only"
+        c.send()
+
+        let c2 = Conversation()
+        c2.messageStore = MessageStore(profileDir: workDir)
+        c2.loadFromStoreIfAvailable()
+
+        let pending = c2.messages.filter { $0.sender == .me && $0.delivery == .sending }
+        XCTAssertEqual(pending.count, 2)
+        let withMedia = pending.filter { !$0.attachments.isEmpty }
+        XCTAssertEqual(withMedia.count, 1, "the media message must be part of the drain set")
+        XCTAssertEqual(withMedia.first?.attachments.first?.kind, .audio)
+        // The drain branches on this to pick the media send path over text.
+        XCTAssertEqual(withMedia.first?.attachments.first?.wireKind, .audio)
+        XCTAssertEqual(withMedia.first?.text, "caption")
+
+        // And the text-only message must still route via the text path.
+        let textOnly = pending.filter { $0.attachments.isEmpty }
+        XCTAssertEqual(textOnly.count, 1)
+        XCTAssertEqual(textOnly.first?.text, "text only")
+    }
+
+    func testAttachmentWireKindMapping() {
+        // The mapping drainOutbox uses to re-send a persisted attachment.
+        XCTAssertEqual(ChatAttachment(kind: .image, bytes: Data()).wireKind, .image)
+        XCTAssertEqual(ChatAttachment(kind: .audio, bytes: Data()).wireKind, .audio)
+        XCTAssertEqual(ChatAttachment(kind: .file, bytes: Data()).wireKind, .file)
+    }
 }

ios/ClawChat/Tests/ClawChatTests/ConversationTests.swift

@@ -42,10 +42,15 @@ final class ConversationTests: XCTestCase {
         XCTAssertEqual(msg.attachments.first?.bytes.count, 3)
     }
 
-    func testSendAttachmentWhenNotReadyIsIgnored() {
+    func testSendAttachmentWhenNotReadyQueuesAsSending() {
+        // Same contract as send(): with no ready connection the message is
+        // queued in .sending for drainOutbox to pick up, not discarded.
         let c = Conversation()
         c.sendAttachment(kind: .image, bytes: Data([1, 2, 3]), filename: "x.png", mime: "image/png")
-        XCTAssertTrue(c.messages.isEmpty)
+        XCTAssertEqual(c.messages.count, 1)
+        XCTAssertEqual(c.messages.first?.delivery, .sending)
+        XCTAssertEqual(c.messages.first?.sender, .me)
+        XCTAssertEqual(c.messages.first?.attachments.first?.filename, "x.png")
     }
 
     func testIncomingAttachmentKindMapping() {

plugin/src/allowlist.ts

@@ -2,7 +2,7 @@
 // and the agent. The Pilot trust handshake already proved the sender is who
 // they claim (X25519 + Ed25519); we just enforce *who is allowed at all*.
 
-import { pilotAddrBase } from "./config.js";
+import { canonicalPilotAddr, pilotAddrBase } from "./config.js";
 
 export type AllowlistDecision =
   | { allowed: true; peer: string }
@@ -15,9 +15,19 @@ export function decideAllowlist(
   if (!rawSrcAddr || typeof rawSrcAddr !== "string") {
     return { allowed: false, reason: "malformed-src" };
   }
-  const peer = pilotAddrBase(rawSrcAddr);
+  // The address grammar accepts hex in either case, so compare on the
+  // canonical form. `resolveAccount` already canonicalizes the configured
+  // set; the fallback scan covers sets assembled by other callers.
+  const peer = canonicalPilotAddr(pilotAddrBase(rawSrcAddr));
   if (!allowlist.has(peer)) {
-    return { allowed: false, reason: "not-in-allowlist" };
+    let found = false;
+    for (const entry of allowlist) {
+      if (canonicalPilotAddr(entry) === peer) {
+        found = true;
+        break;
+      }
+    }
+    if (!found) return { allowed: false, reason: "not-in-allowlist" };
   }
   return { allowed: true, peer };
 }

plugin/src/config.ts

@@ -65,7 +65,7 @@ export function resolveAccount(
     accountId,
     enabled: raw.enabled !== false,
     socketPath: raw.socketPath ?? DEFAULT_SOCKET_PATH,
-    allowlist: new Set(raw.allowlist ?? []),
+    allowlist: new Set((raw.allowlist ?? []).map(canonicalPilotAddr)),
     appPort: raw.appPort ?? DEFAULT_APP_PORT,
     handshakeTrustAutoApprove: raw.handshakeTrustAutoApprove !== false,
     sharedSecret: hasSecret ? raw.sharedSecret : undefined,
@@ -78,6 +78,17 @@ export function isValidPilotAddress(addr: string): boolean {
   return typeof addr === "string" && PILOT_ADDR_RE.test(addr);
 }
 
+/**
+ * Put a pilot address in the single case the rest of the plugin uses. The
+ * network id is decimal and the node id is hex, so upper-casing the whole
+ * string only touches the hex groups. Matches the form `nodeIdToAddress`
+ * emits in peer-address.ts, so addresses derived from either path compare
+ * equal.
+ */
+export function canonicalPilotAddr(addr: string): string {
+  return addr.toUpperCase();
+}
+
 /** Strip an optional `:PORT` suffix from a pilot address-with-port. */
 export function pilotAddrBase(addrWithMaybePort: string): string {
   const lastColon = addrWithMaybePort.lastIndexOf(":");

plugin/src/inbound.ts

@@ -11,7 +11,7 @@ import { mkdirSync, writeFileSync } from "node:fs";
 import { join } from "node:path";
 import { tmpdir } from "node:os";
 
-import type { ResolvedPilotAccount } from "./config.js";
+import { canonicalPilotAddr, pilotAddrBase, type ResolvedPilotAccount } from "./config.js";
 import { createAegisScan, type AegisScan } from "./aegis-scan.js";
 import { decideAllowlist } from "./allowlist.js";
 import type { PeerAddressCache } from "./peer-address.js";
@@ -83,6 +83,12 @@ export type InboundDeps = {
    * envelopes are dropped with a warning. Default 25 MiB.
    */
   maxMediaBytes?: number;
+  /**
+   * How far an envelope's `ts` may sit from local time, in either direction,
+   * for the HMAC path to accept it. Only applies when authorization came
+   * from the shared secret rather than the allowlist. Default 2 minutes.
+   */
+  hmacMaxSkewMs?: number;
   /**
    * If set, the pipeline sends an `ack` envelope back to the peer once a
    * message has been fully reassembled (text or media). Lets senders
@@ -122,13 +128,15 @@ export class InboundPipeline {
   private recentOrder: Array<{ id: string; ts: number }> = [];
   private readonly mediaDir: string;
   private readonly maxMediaBytes: number;
+  private readonly hmacMaxSkewMs: number;
   private readonly aegisScan: AegisScan;
 
   constructor(deps: InboundDeps) {
     this.deps = deps;
     this.recent = deps.recentIds ?? new Set();
     this.mediaDir = deps.mediaDir ?? join(tmpdir(), "claw-pilot-inbound");
     this.maxMediaBytes = deps.maxMediaBytes ?? 25 * 1024 * 1024;
+    this.hmacMaxSkewMs = deps.hmacMaxSkewMs ?? 120_000;
     this.aegisScan = deps.aegisScan ?? createAegisScan();
     try {
       mkdirSync(this.mediaDir, { recursive: true });
@@ -199,7 +207,22 @@ export class InboundPipeline {
     const hmacOK = this.deps.account.sharedSecret
       ? await verifyEnvelope(env, this.deps.account.sharedSecret)
       : false;
-    if (!hmacOK) {
+    // The bypass additionally requires the envelope's `ts` to be inside the
+    // accepted window. `ts` is covered by the HMAC, so the window bounds how
+    // long a given signed envelope stays usable. Outside it, authorization
+    // falls back to the allowlist instead of being granted by the secret.
+    const skewMs = Math.abs(Date.now() - env.ts);
+    const hmacFresh = hmacOK && Number.isFinite(env.ts) && skewMs <= this.hmacMaxSkewMs;
+    if (hmacOK && !hmacFresh) {
+      this.deps.logger.warn("pilot inbound: HMAC envelope timestamp outside window", {
+        srcAddr: dg.srcAddr,
+        id: env.id,
+        ts: env.ts,
+        skewMs: Number.isFinite(skewMs) ? skewMs : null,
+        maxSkewMs: this.hmacMaxSkewMs,
+      });
+    }
+    if (!hmacFresh) {
       const decision = decideAllowlist(dg.srcAddr, this.deps.account.allowlist);
       if (!decision.allowed) {
         this.deps.logger.warn("pilot inbound: dropped — not allowed", {
@@ -211,11 +234,7 @@ export class InboundPipeline {
       }
       peer = decision.peer;
     } else {
-      // Strip any port suffix for consistent logging / dispatch.
-      const colonIdx = peer.lastIndexOf(":");
-      if (colonIdx > 0 && /^\d+$/.test(peer.slice(colonIdx + 1))) {
-        peer = peer.slice(0, colonIdx);
-      }
+      peer = canonicalPilotAddr(pilotAddrBase(peer));
       this.deps.logger.debug?.("pilot inbound: HMAC verified — bypassing allowlist", {
         srcAddr: peer,
         id: env.id,