fix(cli): keep attachment-download --all inside --output-dir

CHANGELOG.md

@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Security
+- `tb attachment-download --all` now strips directory parts and unsafe characters from attachment names before writing, so a crafted name such as `../../.bashrc` can no longer write outside `--output-dir`. Unnamed attachments are saved as `attachment-<partName>`. Idea from DreatNU-oss/thunderbird-cli `651d9f1` (reimplemented, not copied).
+
 ## [1.3.0] — 2026-09-28
 
 Needs add-on 2.4.0 for the calendar, task and Fast Actions commands. Until ATN finishes its manual review of 2.4.0 (required for Experiment APIs), this release attaches the signed 2.1.0 XPI plus an unsigned 2.4.0 build. The email, contacts and notes commands work with either.

cli/src/cli.js

@@ -9,7 +9,7 @@
 
 import { createRequire } from "module";
 import { Command } from "commander";
-import { api, output, outputError, getConfig, parseRelativeDate } from "./client.js";
+import { api, output, outputError, getConfig, parseRelativeDate, safeFileName } from "./client.js";
 import { basename, extname } from "path";
 import {
   listNotes,
@@ -1049,7 +1049,7 @@ program
       const results = [];
       for (const att of attachments) {
         const data = await api("POST", `/messages/${messageId}/attachment`, { partName: att.partName }, timeout);
-        let fileName = data.name || att.name || att.partName;
+        let fileName = safeFileName(data.name || att.name, `attachment-${att.partName}`);
         if (!extname(fileName)) {
           fileName += mimeToExt(data.contentType || att.contentType);
         }

cli/src/client.js

@@ -863,6 +863,18 @@ export function outputError(err, format = "json", opts = {}) {
   process.exit(1);
 }
 
+/**
+ * Turn a sender-controlled attachment name into a bare file name that stays inside the
+ * output directory: drops any directory part (either separator), control characters and
+ * characters Windows rejects, and falls back when nothing usable is left ("", ".", "..").
+ */
+export function safeFileName(name, fallback = "attachment") {
+  const base = String(name ?? "").split(/[\\/]/).pop();
+  const cleaned = base.replace(/[\x00-\x1f\x7f<>:"|?*]/g, "_").trim();
+  if (!cleaned || /^\.+$/.test(cleaned)) return fallback;
+  return cleaned;
+}
+
 /**
  * Parse relative date strings (7d, 2w, 3m, 1y, today, yesterday) to ISO dates
  */

docs/COMMANDS.md

@@ -229,6 +229,10 @@ tb attachment-download <messageId> <partName> --output <path>  # download one
 tb attachment-download <messageId> --all --output-dir <dir>    # download all
 ```
 
+With `--all`, each file is named after the attachment with any directory part and
+unsafe characters stripped, so a crafted name like `../../.bashrc` lands in `<dir>`
+(as `.bashrc`). Unnamed attachments are saved as `attachment-<partName>`.
+
 ## Fetch & Sync
 
 ```bash

test/quick-test.mjs

@@ -7,6 +7,7 @@
 import { createServer } from "http";
 import { WebSocketServer, WebSocket } from "ws";
 import { randomUUID } from "crypto";
+import { safeFileName } from "../cli/src/client.js";
 
 const PORT = 19700;
 const WS_PORT = 19701;
@@ -319,6 +320,16 @@ await new Promise(r => setTimeout(r, 200)); // ensure the waiter is registered b
 servers.mock.send(JSON.stringify({ type: "event", name: "test-event", data: { hello: "world" } }));
 test("GET /bridge/events?wait=test-event (live push)", await liveWait, r => r.event?.name === "test-event" && r.event?.data?.hello === "world");
 
+console.log("\n\x1b[1mAttachment file names\x1b[0m");
+test("safeFileName keeps a plain name", safeFileName("report.pdf"), r => r === "report.pdf");
+test("safeFileName strips ../ traversal", safeFileName("../../.bashrc"), r => r === ".bashrc");
+test("safeFileName strips absolute paths", safeFileName("/etc/cron.d/job"), r => r === "job");
+test("safeFileName strips Windows separators", safeFileName("..\\..\\evil.exe"), r => r === "evil.exe");
+test("safeFileName replaces control and reserved chars", safeFileName("a\x00b:c?.txt"), r => r === "a_b_c_.txt");
+test("safeFileName falls back on '..'", safeFileName("..", "attachment-1.2"), r => r === "attachment-1.2");
+test("safeFileName falls back on trailing separator", safeFileName("dir/", "attachment-1.2"), r => r === "attachment-1.2");
+test("safeFileName falls back on missing name", safeFileName(undefined, "attachment-1.2"), r => r === "attachment-1.2");
+
 console.log(`\n\x1b[1m${"─".repeat(40)}\x1b[0m`);
 console.log(`\x1b[1m${passed} passed, ${failed} failed, ${passed + failed} total\x1b[0m\n`);