Fix: Handle libsodium decryption errors as poison messages (#177)

Co-authored-by: Baudbot <hornet@agentmail.to>

Author: baudbot-agent

slack-bridge/broker-bridge.mjs

@@ -786,11 +786,18 @@ function verifyBrokerEnvelope(message) {
 }
 
 function decryptEnvelope(message) {
-  const plaintext = sodium.crypto_box_seal_open(
-    fromBase64(message.encrypted),
-    cryptoState.serverBoxPublicKey,
-    cryptoState.serverBoxSecretKey,
-  );
+  let plaintext;
+  try {
+    plaintext = sodium.crypto_box_seal_open(
+      fromBase64(message.encrypted),
+      cryptoState.serverBoxPublicKey,
+      cryptoState.serverBoxSecretKey,
+    );
+  } catch (err) {
+    // Wrap libsodium errors (e.g., "incorrect key pair for the given ciphertext")
+    // into a format that isPoisonMessageError() can detect
+    throw new Error(`failed to decrypt broker envelope (message_id: ${message.message_id || "unknown"})`);
+  }
   if (!plaintext) {
     throw new Error(`failed to decrypt broker envelope (message_id: ${message.message_id || "unknown"})`);
   }

test/broker-bridge.integration.test.mjs

@@ -308,6 +308,139 @@ describe("broker pull bridge semi-integration", () => {
     expect(valid).toBe(true);
   });
 
+  it("acks poison messages with decryption failures (wrong keys)", async () => {
+    await sodium.ready;
+
+    let pullCount = 0;
+    let ackPayload = null;
+
+    // Generate valid encryption with mismatched keys to trigger "incorrect key pair"
+    const wrongBoxKeypair = sodium.crypto_box_keypair();
+    const serverBoxKeypair = sodium.crypto_box_seed_keypair(new Uint8Array(Buffer.alloc(32, 11)));
+    const brokerSignKeypair = sodium.crypto_sign_seed_keypair(new Uint8Array(Buffer.alloc(32, 15)));
+
+    const payload = JSON.stringify({ source: "slack", type: "message", payload: { text: "test" }, broker_timestamp: 123 });
+    // Encrypt with WRONG key (wrongBoxKeypair) but bridge will try to decrypt with serverBoxKeypair
+    const encrypted = sodium.crypto_box_seal(new TextEncoder().encode(payload), wrongBoxKeypair.publicKey);
+    const encryptedB64 = toBase64(encrypted);
+
+    const broker = createServer(async (req, res) => {
+      if (req.method === "POST" && req.url === "/api/inbox/pull") {
+        pullCount += 1;
+        const brokerTimestamp = Math.floor(Date.now() / 1000);
+        
+        const canonical = canonicalizeEnvelope("T123BROKER", brokerTimestamp, encryptedB64);
+        const signature = sodium.crypto_sign_detached(canonical, brokerSignKeypair.privateKey);
+
+        const messages = pullCount === 1
+          ? [{
+              message_id: "m-decrypt-fail-1",
+              workspace_id: "T123BROKER",
+              encrypted: encryptedB64,
+              broker_timestamp: brokerTimestamp,
+              broker_signature: toBase64(signature),
+            }]
+          : [];
+
+        res.writeHead(200, { "Content-Type": "application/json" });
+        res.end(JSON.stringify({ ok: true, messages }));
+        return;
+      }
+
+      if (req.method === "POST" && req.url === "/api/inbox/ack") {
+        let raw = "";
+        for await (const chunk of req) raw += chunk;
+        ackPayload = JSON.parse(raw);
+
+        res.writeHead(200, { "Content-Type": "application/json" });
+        res.end(JSON.stringify({ ok: true, acked: ackPayload.message_ids?.length ?? 0 }));
+        return;
+      }
+
+      if (req.method === "POST" && req.url === "/api/send") {
+        res.writeHead(200, { "Content-Type": "application/json" });
+        res.end(JSON.stringify({ ok: true, ts: "1234.5678" }));
+        return;
+      }
+
+      res.writeHead(404, { "Content-Type": "application/json" });
+      res.end(JSON.stringify({ ok: false, error: "not found" }));
+    });
+
+    await new Promise((resolve) => broker.listen(0, "127.0.0.1", resolve));
+    servers.push(broker);
+
+    const address = broker.address();
+    if (!address || typeof address === "string") {
+      throw new Error("failed to get broker test server address");
+    }
+    const brokerUrl = `http://127.0.0.1:${address.port}`;
+
+    const testFileDir = path.dirname(fileURLToPath(import.meta.url));
+    const repoRoot = path.dirname(testFileDir);
+    const bridgePath = path.join(repoRoot, "slack-bridge", "broker-bridge.mjs");
+    const bridgeCwd = path.join(repoRoot, "slack-bridge");
+
+    let bridgeStdout = "";
+    let bridgeStderr = "";
+    let bridgeExit = null;
+
+    const bridge = spawn("node", [bridgePath], {
+      cwd: bridgeCwd,
+      env: {
+        ...cleanEnv(),
+        SLACK_BROKER_URL: brokerUrl,
+        SLACK_BROKER_WORKSPACE_ID: "T123BROKER",
+        SLACK_BROKER_SERVER_PRIVATE_KEY: toBase64(serverBoxKeypair.privateKey),
+        SLACK_BROKER_SERVER_PUBLIC_KEY: toBase64(serverBoxKeypair.publicKey),
+        SLACK_BROKER_SERVER_SIGNING_PRIVATE_KEY: b64(32, 13),
+        SLACK_BROKER_PUBLIC_KEY: b64(32, 14),
+        SLACK_BROKER_SIGNING_PUBLIC_KEY: toBase64(brokerSignKeypair.publicKey),
+        SLACK_BROKER_ACCESS_TOKEN: "test-broker-token",
+        SLACK_ALLOWED_USERS: "U_ALLOWED",
+        SLACK_BROKER_POLL_INTERVAL_MS: "50",
+        BRIDGE_API_PORT: "0",
+      },
+      stdio: ["ignore", "pipe", "pipe"],
+    });
+
+    bridge.stdout.on("data", (chunk) => {
+      bridgeStdout += chunk.toString();
+    });
+    bridge.stderr.on("data", (chunk) => {
+      bridgeStderr += chunk.toString();
+    });
+    const bridgeExited = new Promise((_, reject) => {
+      bridge.on("error", (err) => {
+        if (ackPayload !== null) return;
+        reject(new Error(`bridge spawn error: ${err.message}; stdout=${bridgeStdout}; stderr=${bridgeStderr}`));
+      });
+      bridge.on("exit", (code, signal) => {
+        bridgeExit = { code, signal };
+        if (ackPayload !== null) return;
+        reject(new Error(`bridge exited early: code=${code} signal=${signal}; stdout=${bridgeStdout}; stderr=${bridgeStderr}`));
+      });
+    });
+
+    children.push(bridge);
+
+    const ackWait = waitFor(
+      () => ackPayload !== null,
+      10_000,
+      50,
+      `timeout waiting for ack; pullCount=${pullCount}; exit=${JSON.stringify(bridgeExit)}; stdout=${bridgeStdout}; stderr=${bridgeStderr}`,
+    );
+
+    await Promise.race([ackWait, bridgeExited]);
+
+    expect(ackPayload.workspace_id).toBe("T123BROKER");
+    expect(ackPayload.protocol_version).toBe("2026-02-1");
+    expect(ackPayload.message_ids).toContain("m-decrypt-fail-1");
+    
+    // Verify that the bridge logged a decryption error before acking
+    expect(bridgeStderr).toContain("failed to decrypt");
+  });
+
   it("forwards user messages to agent in fire-and-forget mode without get_message/turn_end RPCs", async () => {
     await sodium.ready;