Skip to content

v2.10.0: 11-issue marathon (#104 sweep wrap-up + RFC 2047/2231 hardening + multi-account compose)

Choose a tag to compare

@kiki830621 kiki830621 released this 20 May 02:00

11-issue marathon — closes #126, #128, #129, #130, #131, #133, #134, #135, #137, #138, #139. Highlights: #131 multi-account from_address for compose/draft (new feature), #133 redirect_email boundary recipient validation (#41 consistency), #134 + #139 regression-lock for reply/forward + mailbox-CRUD actor wiring (closes #104 PR-A/B/C/D verify DA gaps), #135 RedirectEmailScriptBuilder escape coverage + byte-identity golden fixture, #137 resolveMailboxRef / mailboxRefByAccountId compose resolveAccountRef (#110 sibling DRY), #126 decodeRFC2047IfApplicable RFC 2047 §2 gate tightening + fail-fast regex, #125 / #122 / #115 / #124 RFC 2047 / 2231 attachment-filename hardening sweep across CJK + Yahoo / Outlook 16 / path-traversal vectors, #129 move_email/copy_email cross-account README limitation, #130 + #128 positional UUID / verb-anchor assertion hardening, #138 deferred (Codex layer migration — not recurrent). Test suite 475 → 482+; full 6-AI ensemble verify every PR.

Changed

  • MailController mutation-tool method signatures reordered for accountId-before-accountName consistency (#117 — from #114's Devil's Advocate review). The 5 single-message mutation methods (markRead, flagEmail, setFlagColor, setBackgroundColor, markAsJunk) declared parameters (id, mailbox, accountName, accountId, …) while the buildXxxScript builders and resolveMsgRef use (id, mailbox, accountId, accountName, …) — Swift label-based calls compiled fine, but the order mismatch was a footgun for anyone grepping signatures. Pure parameter reorder of the 5 signatures + 5 matching Server.swift call sites; zero behavior change.
  • Real PII removed from test fixtures and documentation (#119, #147). Hardcoded real email addresses (the maintainer's kiki830621@gmail.com, a third-party d06227105@ntu.edu.tw) and a real personal name embedded in an RFC 2047 Q-encoded header fixture were replaced with RFC 2606 synthetics (alice@example.com / bob@example.com / 測試) across 7 test files, the AccountMapper.swift doc-comment, a README.md save_attachment example, and the emlx-parser spec's Quoted-Printable decode scenario (#147's verify caught the spec/test drift the test-only #119 fix would otherwise have introduced). Test-only behavior; no production behavior change. Author-attribution credits and archived Spectra authorship metadata intentionally retain the maintainer's name/handle.
  • resolveMailboxRef / mailboxRefByAccountId now compose resolveAccountRef instead of duplicating its selector syntax inline (#137 — from #104 PR-D sister sweep). Pre-#137 three places knew how to build the account-only AppleScript selector: resolveAccountRef (the canonical), mailboxRefByAccountId (inlined UUID path), and resolveMailboxRef's fallback branch (inlined display_name path). If (account id "...") syntax ever needs hardening (e.g. escaping subtlety, Apple semantic change), pre-#137 all three would have to change in lock-step — classic drift risk. Refactor: both functions now call resolveAccountRef(accountId:accountName:) and concatenate the (first mailbox of … shell around it. mailboxRefByAccountId passes accountName: "" (sentinel — accountId is non-empty by contract, so resolveAccountRef always takes the UUID path; an empty-string accountName would surface in scripts as a visibly wrong fallback should a future caller bypass the contract). Output-preserving — all 17 existing AppleScriptRefBuilderTests (exact-string assertions for both UUID and display_name paths) stay green unchanged, and the full 475-test suite passes. Sibling to #110 (escape-function dedup) — both consolidate AppleScript-construction discipline at single chokepoints.
  • AppleScript escaping deduplicated — MailController.escapeForAppleScript removed in favor of the shared appleScriptEscape free function (#110). MailController carried a private func escapeForAppleScript(_:) whose body was character-for-character identical to the package-level appleScriptEscape free function used by every AppleScript builder (ComposeScriptBuilder, CreateRuleScriptBuilder, etc.) — two copies of the same escape order (backslash → double-quote → \r\n\n\r\t) drifting independently was a latent correctness hazard. The private method was deleted and all ~29 in-method call sites swapped to appleScriptEscape; appleScriptEscape is now the single AppleScript-escape chokepoint across the package. Pure refactor — appleScriptEscape is a synchronous file-level function (not actor-isolated), so no await was needed at any swapped call site; output is byte-identical for every input. grep -rn "escapeForAppleScript" Sources Tests now returns 0 hits.
  • README documents the move_email / copy_email cross-account limitation (#129 — from #127 verify DA-8). move_email and copy_email accept a single account_id that is threaded through both source msgRef and destination mailboxRef; cross-account transfer (source on account A, destination on account B) is not supported because Mail.app's AppleScript move msg to <mailboxRef> requires the destination to be expressed relative to a single account context. The Account Disambiguation section now explicitly states this limitation and notes that users wishing to recreate a message on a different account can use save_attachment + compose_email to manually rebuild the body and attachments (this is not a true move/copy of the original — message metadata, flags, and identity are not preserved). Docs-only; no behaviour change.

Added

  • compose_email / create_draft gain optional from_address parameter for multi-account sender selection (#131 — feature carved out of #104 PR-C scoping). Pre-#131 these two tools accepted a dead accountName parameter that was never read by the Server.swift handlers and never plumbed to the AppleScript builders — multi-account users couldn't choose which account to send/save FROM, and Mail.app fell back to its default account. Unlike the #104 sweep (which addresses account "<display_name>" selector collision when referencing existing mail), compose_email / create_draft make new outgoing message, so they have no selector defect — they had a missing-feature instead. Mail.app's sender property on an outgoing message is a STRING matching one of the user's configured email addresses (RFC 5322 addr-spec, optionally "Name <email@example.com>"), NOT an account id selector. buildComposeEmailScript and buildCreateDraftScript now accept fromAddress: String? = nil; when non-nil/non-empty they emit set sender to "<escaped>" inside the tell newMessage block (escaped via appleScriptEscape — header-injection-safe). MailController.composeEmail / createDraft validate fromAddress at the boundary via validateEmailAddresses(_, field: "from_address") (same #41 hardening applied to recipients). Server.swift handlers read arguments["from_address"]?.stringValue and plumb through. Backward compat preserved: omitting from_address produces byte-identical AppleScript to pre-#131 (no set sender to line) — Mail.app continues to use the default account. 8 new tests cover builder emit + escape + nil/empty fallback + boundary validation (control-char rejection / missing-@ rejection) for both compose and draft. Use list_accounts to discover available addresses.
  • create_draft now accepts optional cc / bcc recipient arrays (#107). create_draft's schema previously exposed only to — an agent that reasoned about who to CC (a referrer, a PI) could not carry that decision into the draft, forcing the user to re-add Cc/Bcc by hand in Mail.app. create_draft now mirrors compose_email: optional cc and bcc string arrays flow through MailController.createDraftbuildCreateDraftScript, emitting make new cc/bcc recipient AppleScript fragments (via the shared recipientFragment helper, same emission order as buildComposeEmailScript). Recipients are validated at the boundary via validateEmailAddresses (#41 consistency — to/cc/bcc all checked). Backward compat preserved: both fields are optional and absent from required; omitting them produces byte-identical AppleScript to the pre-#107 builder. The message-composition spec gains a normative cc/bcc requirement (covering both compose_email and create_draft). 3 new tests (buildCreateDraftScript cc/bcc inside the tell newMessage block + omitted-when-nil; create_draft schema advertises cc/bcc as non-required).
  • list_attachments / list_attachments_batch now stamp each attachment with a savable boolean (#105 — sister concern of #103). The crossValidateAttachments filter (#24) dropped SQLite rows whose name was absent from the .emlx body, but never checked whether the binary was actually retrievable — so a .partial.emlx attachment whose name is in the envelope yet whose binary is in neither the inline body nor Apple Mail's external Attachments/<rowId>/ cache was reported as available, and a subsequent save_attachment then failed opaquely with -10000 (#103). Each cross-validated entry now carries "savable": true|falsetrue when the binary is retrievable from the local store, false for the #103 precursor state — so callers can predict (and route around) the failure before attempting the save. On the .emlx-parse-failure fallback path the field is omitted (savability unknown); an absent savable must not be read as false. Implementation preserves the #24 no-decode perf property: a new MIMEParser.enumerateAttachmentInlinePresence extends the existing names-only tree walker to also report per-part inline-body presence (body has any non-whitespace byte — base64/QP of whitespace-only input decodes to empty, and .contains early-exits on the first content byte, so a 50-MB attachment is never decoded just to answer this); EnvelopeIndexReader-adjacent EmlxParser.attachmentSavability(rowId:mailboxURL:) combines that with the external-cache lookup. enumerateAttachmentNames is now derived from the new map's keys (single tree walk; behavior unchanged). When the same filename appears on multiple parts the first occurrence in document order wins (mirrors saveAttachment's parts.first selection — OR-combining would overstate savability). The sqlite-query-engine and batch-operations specs gain normative coverage of the savable field. 8 new tests (MIMEParser stripped-vs-present + enumerateAttachmentNames regression + duplicate-name first-wins; attachmentSavability external-present → true / external-missing → false / path-traversal name rejected; crossValidateAttachments savable stamping + omit-on-unknown).

Fixed

  • MailController.createMailbox / deleteMailbox wiring now has automated regression lock (#139 — from #136 verify DA-5, recurrence of #134's pattern). The 11 PR-D builder tests directly cover buildCreateMailboxScript / buildDeleteMailboxScript / resolveAccountRef, but the actor methods' call sites ("does createMailbox actually call the builder with the right params?") had no integration-free coverage. A future edit that inlines the AppleScript directly into the actor (let script = "tell application \"Mail\" …") would leave swift test green and silently kill #104 account_id disambiguation. Solution: 2 structural source-code introspection tests in MailboxCrudScriptBuilderTests read MailController.swift (via #filePath-derived path), extract the createMailbox / deleteMailbox function bodies (brace-balanced), and assert (a) buildCreateMailboxScript(name: / buildDeleteMailboxScript(name: call is present and (b) inline tell application "Mail" is absent. Brittle to deliberate refactors — when the wiring shape intentionally changes, the tests should be updated to reflect the new contract; that's the trade-off for directly catching the "actor stopped calling the builder" regression that semantic builder tests cannot detect without integration infrastructure. Mirrors #134's discipline (testable builder seam at the right layer) but adds a one-step-further structural pin for actor methods whose body is effectively a 2-line wrapper.
  • RedirectEmailScriptBuilder test coverage hardened — escape-discipline tests + byte-identity golden fixture (#135 — from #132 verify LOW bundle: SEC-6 + DA-4 residual). Pre-#135 RedirectEmailScriptBuilderTests.swift covered recipient-quote escape but not the symmetric escape positions (account_id, mailbox name, backslash inputs); PR #132 verified byte-identity of the nil-accountId path by 3 reviewers manually compiling-and-diffing, but no test pinned it. New tests close both gaps: 4 escape tests (account_id quote / mailbox quote / backslash-in-recipient / backslash-in-accountName) plus a byte-identity golden-string fixture for the accountId: nil fallback path that would catch any silent drift between appleScriptEscape and MailController.escapeForAppleScript-class helpers (already deduplicated by #110, but a golden fixture remains the cheapest insurance). Test-only; no production code change. The third LOW gap (forward_email display_name-fallback test) was already addressed by #134's new buildForwardEmailScript(id:mailbox:accountId:accountName:...) overload tests.
  • reply_email / forward_email resolveMsgRef wiring now has automated regression lock (#134 — from #132 verify DA-3). Pre-#134 MailController.replyEmail and forwardEmail computed ref = resolveMsgRef(...) inline and threaded ref into the (messageRef:) builders — the literal resolveMsgRef call that #104 PR-C added for account_id disambiguation had zero automated test (only MailAppIntegrationTests, which are XCTSkip-gated and don't pass accountId). Reverting resolveMsgRef → msgRef in either method would have left swift test green and silently reintroduced the #104 display_name-collision bug. New buildReplyEmailScript(id:mailbox:accountId:accountName:...) and buildForwardEmailScript(id:mailbox:accountId:accountName:...) overloads internalize the resolveMsgRef call so the wiring IS unit-testable; the existing (messageRef:) overloads stay intact for the 25 legacy tests that pre-build the ref. MailController.replyEmail / forwardEmail switched to the new overloads. 5 new tests (MailControllerComposeTests) cover UUID path + display_name fallback + empty-string-accountId fallback semantics for both reply and forward. Mirrors PR-B's #130 (positional UUID assertion) and PR-C's redirect builder pattern — the actor-method wiring sweep is now consistent across PR-A/B/C. Pre-fetch path (buildFetchOriginalContentScript) still computes ref inline; locking that wiring is a separate concern out of #134's scope.
  • redirect_email now validates recipient addresses at the boundary, matching compose_email / reply_email / forward_email (#133 — sister of #41, surfaced during #104 PR-C sister sweep). MailController.redirectEmail was the only message-relay tool that skipped validateEmailAddresses, so malformed addresses (control characters attempting header injection, missing @, multiple @) bypassed the clean MailError.invalidParameter boundary error and either reached Mail.app to surface as an opaque AppleScript error or silently produced an undeliverable redirect. Not a security defect (recipients are still appleScriptEscape'd so injection was never possible), but a consistency gap: callers relying on the #41 contract got divergent behavior depending on which relay tool they used. Fix is a one-line try validateEmailAddresses(to, field: "to") at the top of redirectEmail, mirroring forwardEmail (line 885). 2 new tests cover control-char and missing-@ rejection paths at the redirectEmail boundary.
  • RFC822Parser.decodeRFC2047 now strips CR/LF (in addition to space/tab) between consecutive encoded-words (#125 — sister of #99). Pre-fix the inter-encoded-word whitespace check at RFC822Parser.swift:129 only stripped and \t, so a CR/LF separator between two adjacent =?…?= encoded-words leaked through as a literal control character into the decoded result (e.g. =?utf-8?B?YWJj?=\n=?utf-8?B?ZGVm?="abc\ndef"). RFC 2047 §6.2 defines the inter-encoded-word whitespace as RFC 822 linear-white-space (LWS) — space + tab + CR + LF — so all four must be skipped. Subtle: Swift treats \r\n as a single extended grapheme cluster that does not equal any individual Character literal in a Character-level allSatisfy check; the fix iterates prefix.unicodeScalars so each LWS component matches scalar-wise. Header-level callers via parseHeaders are mostly unaffected because the upstream unfolding pass at RFC822Parser.swift:84-87 already normalises folded \r\n[ \t] sequences to a single space, but the per-parameter decodeRFC2047IfApplicable path (used by MIMEParser.resolveFilename after #115) does not unfold its input, so CR/LF between EWs could survive into the decoded filename pre-fix. 1 new test pins all five LWS separators (space, tab, LF, CR, CRLF) round-trip cleanly to abcdef. The originally-suggested fix in #125's body — tightening the gate's \s* to [ \t] — is moot: #115 round-2 already removed the multi-EW (\s*=?…?=)* repetition group from the gate (it is now a single-EW substring search), so the asymmetry lives only at the decoder layer, where this fix lands.
  • resolveFilename now assembles RFC 2231 continuation on the Content-Type name*N family symmetric to filename*N (#122 — sister of #99). MIMEParser.resolveFilename previously assembled RFC 2231 continuation segments only on the Content-Disposition filename*N family (step 2). A sender that emits only name*0*/name*1* on Content-Type (and no Content-Disposition filename at all — a rare but real Outlook 16 / non-conformant shape) hit the same #99 silent-drop class: params["name"] was nil, no continuation aggregator ran over contentTypeParams, the assembled (let alone decoded) name was never produced, enumerateAttachmentNames emitted no key, crossValidateAttachments dropped every SQLite row, list_attachments returned []. Fix: extracted the step-2 continuation-assembly block into a shared assembleRFC2231Continuation(params:paramName:) helper parameterised over the param name (DRY — two parallel aggregators was exactly what let this sister bug slip past #99), generalized the continuationIndex(_:paramName:) helper accordingly, then added two new resolution steps for the name family: 4a contentTypeParams["name*"] RFC 5987 single-segment defensive handler (mirrors step 1 for filename*), and 4b name*N continuation aggregator (mirrors step 2 for filename*N). Step 2 still produces byte-identical output (pinned by the existing #99 + #115 tests + round-2 regression tests, all green); resolution priority is unchanged — Content-Disposition filename still wins over Content-Type name family at every level. 2 new tests: testResolveFilename_contentTypeName_rfc2231WithNestedRfc2047 (mirrors the #99 Outlook 16 fixture but with name*0*/name*1* in contentTypeParams only, decodes to 中央研究院新進聘僱人員體格檢查表-1110331製表.pdf) and testResolveFilename_contentTypeName_rfc5987 (covers step 4a, decodes to 中文.pdf).
  • list_attachments / list_attachments_batch no longer silently return [] for emails whose attachment filename is an RFC 2047 encoded-word split across RFC 2231 continuation segments (#115). An older Yahoo Mail client (WebService/… YMailNorrin) encodes a CJK attachment filename as RFC 2047 encoded-words spread over RFC 2231 plain continuation parameters (filename*0 / filename*1, no trailing *), and the segment boundary cuts through the second encoded-word's =? opener (= ends filename*0, ?charset… starts filename*1). Root cause: RFC822Parser.parseHeaders applied decodeRFC2047 to every header value — including the structured Content-Disposition header — before MIMEParser split it into parameters. The header-level scan decoded the encoded-word fully contained inside filename*0 but left the straddling one untouched, producing a half-decoded value (測試 =?UTF-8?Q?…?=); resolveFilename then assembled a string no longer starting with =?, so its strict full-string-gated #99 second pass refused to decode it. crossValidateAttachments compared that garbage name against Mail.app's SQLite-cached human-readable name, found no intersection, and dropped every row. (Unlike #99, the defect is upstream of resolveFilename — the value is corrupted before parameter-splitting, so #99's per-return-point second pass could not catch it.) Fix: parseHeaders no longer RFC 2047-decodes Content-* headers at the raw-header level — per RFC 2047 §5 an encoded-word may not appear in a structured field body, and the only legitimate filename encoded-word lives inside a filename/name parameter, which MIMEParser.resolveFilename already decodes per-parameter. To keep that per-parameter layer compatible with everything the old raw-header scan used to handle, the decodeRFC2047IfApplicable gate was relaxed from a strict whole-string anchor to a substring search for one well-formed encoded-word, so a legitimate =?…?=.<ext> value (encoded basename + literal extension — a real non-conformant client pattern) is now decoded in place; the malformed report=?bogus.pdf case still survives untouched because decodeRFC2047 itself only rewrites well-formed encoded-words. The legacy Content-Type: name= add point in collectAttachmentNames was made symmetric with the same per-parameter decode so the emitted attachment-name set no longer carries a spurious raw =?…?= entry when raw-header decoding is gone. Emails using RFC 5987 filename*= (no encoded-word in the disposition header) were never affected. Also adds observability the diagnosis flagged as missing: list_attachments / list_attachments_batch now write a WARN: line to stderr when cross-validation drops all non-empty SQLite attachment rows (previously a completely silent []). 5 new tests: end-to-end enumerateAttachmentNames over a synthetic split-encoded-word RFC 2231 continuation fixture, end-to-end coverage of Content-Type: name="=?…?=" at the name-add path, parseHeaders pinning Content-Disposition as left-raw, a guard that Subject is still RFC 2047-decoded, and resolveFilename accepting =?…?=.pdf (encoded basename + literal extension).
  • save_attachment now returns an actionable error instead of the bare -10000 AppleEvent handler failed (#103). When the Tier-2 AppleScript fallback fails with -10000 (errAEEventFailed — Mail.app's own save attachment handler raised an internal exception, distinct from #101/#102's -1728/-1719 account-selector errors), the caller previously got only the generic "AppleScript error (-10000): Mail got an error: AppleEvent handler failed." with no recovery path. For save_attachment the dominant cause is an attachment whose binary is not in the local Mail store and which Mail.app could not re-fetch from the IMAP server. A new saveAttachmentAppleEventHint(code:accountName:rawMessage:localCopyConfirmedMissing:) pure helper re-words only the -10000 case (any other code rethrows unchanged, so the #101/#102 disambiguation diagnostics are never masked) into an actionable message with concrete recovery steps: re-fetch via the synchronize_account MCP tool (or Mail.app's Take All Accounts Online → Synchronize) → Rebuild the mailbox → manual Save-Attachment from the Mail.app UI, plus a save-path-writable check for the non-missing-binary case. The message is definitive, not hedged, when the MCP can prove the cause: the save_attachment handler threads localCopyConfirmedMissing — set when the Tier-1 SQLite + .emlx fast path threw attachmentNotFound, i.e. the MCP itself confirmed from local .emlx state that the binary is absent (no inline copy, no externalised copy) — into the helper, so a confirmed case states the cause outright while an unconfirmed -10000 stays hedged ("usually means"). A new MailError.operationFailed(String) case surfaces the hint verbatim (no misleading "AppleScript error (N):" prefix). Scope: this addresses the diagnosed Phase 3 (actionable error); the underlying -10000 save failure itself is a Mail.app-internal handler error and is not made to succeed — when the binary is genuinely absent locally and un-fetchable, no save is possible. Tier 1's .partial.emlx-no-external detection (attachmentNotFound) is already pinned by the existing testSaveAttachment_partialEmlxNoExternal_throwsAttachmentNotFound. 4 new tests cover the helper (definitive vs hedged -10000 message; other codes → nil; MailError.operationFailed verbatim description).
  • get_email / get_emails_batch no longer fall through to AppleScript silently when the SQLite Envelope Index lookup returns nil (#100). Both tools have run the SQLite + .emlx fast path since v2.0.0 (ff2f33a) — #100's "migrate to fast path" premise was stale. The real defect: EnvelopeIndexReader.mailboxURL(forMessageId:) returns nil (it does not throw) when a rowId is absent from the Envelope Index, and the fast-path block only wrapped a do/catch — so the nil-return case slipped out of the if let mailboxUrl straight to the AppleScript fallback with no stderr line. #69 closed the throw-path observability hole but left the nil-path one open, violating .claude/rules/r-must-direct-db.md's "logged fallback" rule; a bulk archive that missed the fast path for every message looked "mysteriously slow" with no diagnostic. Fix: a new fastPathFallthroughLog(tool:rowId:reason:perItem:) pure message builder + logFastPathFallthrough(...) writer route both fall-through modes through one chokepoint — the .error case is byte-identical to the pre-#100 inline catch-branch strings (pure refactor of the throw path), and the new .rowIdNotIndexed case is deliberately worded as a neutral "fast path miss" (NOT "failed"), since it fires legitimately on every call for EWS/Exchange accounts that have no local .emlx store (#9). No behavior change — the success path and the fallback decision are untouched; only the previously-missing stderr diagnostic is added. 5 new tests pin the message contract (neutral-miss wording, error detail, per-item suffix, byte-equivalence to the legacy inline strings, and the actual stderr write via the captureStderr fd-2 helper).
  • Non-string account_id arguments now warn to stderr instead of silently degrading — across all 14 account_id-accepting tools (#111 — verify P3 from #101). Every handler that reads account_id did arguments["account_id"]?.stringValue — for a JSON number / bool / array / object the ?.stringValue accessor returns nil, so a mistyped argument (e.g. account_id: 12345 instead of "12345") was indistinguishable from "no account_id supplied". The tool then silently fell back to the account_name (display_name) path — re-exposing exactly the #101 multi-account-same-display_name collision the account_id parameter exists to avoid, with no diagnostic. Post-fix: a new decodeAccountId(_:tool:) helper distinguishes the three cases — a JSON string returns the value verbatim (including empty string, preserving the downstream non-nil/non-empty resolver contract), a genuine JSON null or an absent key returns nil silently (legitimate "not supplied"), and any other type (.int / .double / .bool / .array / .object / .data) returns nil after writing a WARN: line to stderr naming the offending tool and the received type. All 14 call sites across Server.swift were swapped from the raw ?.stringValue read to decodeAccountId(arguments, tool: invokedTool); an invokedTool binding was captured before the switch because the create_mailbox / delete_mailbox / rule handler cases rebind name as a local. Behavior for correctly-typed callers is unchanged; the only observable difference is the new stderr diagnostic for the (always-erroneous) non-string case. 5 new XCTest methods pin the helper contract (string→verbatim, absent→nil, .null→nil, non-string types→nil, empty-string→verbatim).
  • 2 mailbox CRUD tools (create_mailbox, delete_mailbox) now accept account_id for multi-account-same-display_name disambiguation (#104 PR-D — the final sweep increment). Same -1728 / -1719 failure mode as PR-A/B/C — the account "<display_name>" AppleScript selector non-deterministically picks the wrong account when two accounts share a display_name. Pre-fix: creating a mailbox could attach it to the wrong account; deleting could remove the wrong account's mailbox. Post-fix: each tool accepts an optional account_id; when provided, AppleScript uses the (account id "<UUID>") selector. The two tools needed different resolvers: delete_mailbox references an existing mailbox, so it routes through the established resolveMailboxRef chokepoint; create_mailbox addresses an account directly (make new mailbox ... at <accountRef>), a shape no prior resolver covered — PR-D adds a new resolveAccountRef(accountId:accountName:) to AppleScriptRefBuilder.swift (purely additive — resolveMsgRef/resolveMailboxRef unchanged) that returns just the account selector. Both inline scripts were extracted into a new testable MailboxCrudScriptBuilder.swift (mirrors PR-B's MoveCopyDeleteScriptBuilder). Backward compat preserved: omitting account_id falls back to the legacy display_name path, byte-identical to the pre-PR-D inline scripts. 11 new tests (7 MailboxCrudScriptBuilder: UUID path + nil/empty-string fallback + quote escaping per builder; 4 resolveAccountRef: UUID path + nil/empty fallback + escaping). With PR-D the #104 sweep covers all 13 account-referencing AppleScript tools; PR-E (R-tool fallbacks) stays deferred — SQLite Tier 1 dominates, the fallback rarely fires.
  • 3 message-relay tools (reply_email, forward_email, redirect_email) now accept account_id for multi-account-same-display_name disambiguation (#104 PR-C). Same -1728 / -1719 failure mode as PR-A / PR-B — msgRef's account "<display_name>" AppleScript selector non-deterministically picks the wrong account when two accounts share a display_name. Pre-fix: replying to / forwarding / redirecting a message could resolve the wrong account's copy. Post-fix: each tool accepts an optional account_id; when provided, AppleScript uses the (account id "<UUID>") selector via Phase 0's AppleScriptRefBuilder.resolveMsgRef chokepoint. Implementation note: redirect_email's script was built inline in MailController — PR-C extracts it into a new testable RedirectEmailScriptBuilder.swift free builder (mirrors PR-B's MoveCopyDeleteScriptBuilder). reply_email / forward_email already delegate to buildReplyEmailScript / buildForwardEmailScript, which take an opaque pre-resolved messageRef: String; their signatures are unchanged (25 existing tests untouched) — the account_id is threaded through resolveMsgRef at the MailController call site (a msgRef → resolveMsgRef swap). Scope note: compose_email / create_draft are NOT part of this sweep — they make new outgoing message and never emit an account "<display_name>" selector, so they have no -1728/-1719 defect; their separate sender-account-selection gap (dead accountName parameter) is tracked at #131. Backward compat preserved: omitting account_id falls back to the legacy display_name path. 8 new tests (5 RedirectEmailScriptBuilder: UUID path + nil/empty-string fallback + multi-recipient + quote escaping; 3 reply/forward propagation tests verifying the builders embed the resolved UUID ref verbatim).
  • 3 movement/destruction tools (move_email, copy_email, delete_email) now accept account_id for multi-account-same-display_name disambiguation (#104 PR-B). Same -1728 / -1719 failure mode as PR-A's 5 mutation tools — msgRef's account "<display_name>" AppleScript selector non-deterministically picks the wrong account when two accounts share a display_name. Pre-fix: any of these 3 tools could fail or operate on the wrong account's message. Post-fix: each tool accepts an optional account_id; when provided, AppleScript uses (account id "<UUID>") selector. move_email and copy_email are particularly subtle because they emit TWO refs in one script (source msgRef + destination mailboxRef) — both refs flow through the same accountId since move/copy operations stay within a single account. Implementation: new MoveCopyDeleteScriptBuilder.swift with 3 free builder functions that delegate to Phase 0's AppleScriptRefBuilder.resolveMsgRef / resolveMailboxRef chokepoint. Backward compat preserved: omitting account_id falls back to the legacy display_name path. 6 new tests (2 per builder: UUID path verifies UUID in BOTH ref points for move/copy; display_name fallback verifies legacy form).

Security

  • externalAttachmentURL now rejects path-traversal attachment names, closing a pre-existing CWE-22 (Path Traversal) window surfaced + amplified by #105 (caught by the #105 6-AI verify Security reviewer). The attachment binary lookup joins the email-sender-controlled MIME filename onto a filesystem path via appendingPathComponent with no sanitisation — a crafted filename="../../../../etc/passwd" escaped Attachments/<rowId>/. Pre-#105 the probe fired once per deliberate save_attachment call; #105's new attachmentSavability runs externalAttachmentURL for every attachment name on every list_attachments / list_attachments_batch call, turning a passive listing into a filesystem existence-oracle (the savable boolean leaks "does file X exist on the victim's machine" to a remote sender). Fix: externalAttachmentURL now guards the name against path separators and ./.., returning nil for any path-bearing name — a name with a / is never a valid single-segment attachment filename. This also closes the strictly-worse arbitrary-read variant on the save_attachment path (which calls the same helper). 1 exploit-style regression test plants a decoy file exactly where the traversal would resolve and asserts the crafted name reports savable: false.
  • set_background_color now whitelists the color enum before AppleScript interpolation, closing a pre-existing CWE-94 (Code Injection) window (#116 — surfaced during #104 PR-A verify by Codex P2 / DA-F / Logic LOW#1). The color argument flowed from the MCP client through Server.swift's set_background_color handler into MutationToolScriptBuilder.buildSetBackgroundColorScript's set background color of <ref> to \(color) line without any validation — a malicious or buggy caller passing newline-bearing ("red\n do shell script \"…\""), quote-bearing, or semicolon-chained payloads could inject arbitrary AppleScript that ships into osascript (downstream CWE-78). Defense-in-depth fix: (1) new backgroundColorWhitelist: Set<String> constant co-located with the builder — single source of truth, lowercase-strict, exact-match against Apple Mail's 8-value enum; (2) Server.swift handler guards membership before delegating, throws MailError.invalidParameter with the valid set echoed back to the caller; (3) buildSetBackgroundColorScript adds a precondition (release-safe — Package.swift does not enable -Ounchecked) for programmer-error catch if any internal caller bypasses the handler gate. 4 new XCTest methods pin the constant against drift, exercise injection-payload rejection (newline / quote / semicolon), case-strictness ("Blue" / "BLUE" rejected), and whitespace-strictness ("blue " / "" rejected). Existing 2 happy-path tests (UUID path + display_name fallback for "blue" / "green") stay green. Backward compat preserved: callers using the 8 documented enum values (blue, gray, green, none, orange, purple, red, yellow) see identical behavior. Scope note: the diagnose-stage sister sweep covered the 6 AppleScript builder files and confirmed set_background_color.color is the only externally-supplied enum-shaped String slot raw-interpolated across those builder files; subsequent /idd-verify independently confirmed a separate raw-enum interpolation slot at MailController.swift:1111 (createRule's qualifier:\(qualifier)) that uses the same anti-pattern outside the builder boundary — fixed by #140 below.
  • create_rule now whitelists the qualifier enum before AppleScript interpolation, closing a pre-existing CWE-94 (Code Injection) window (#140 — sister fix to #116; independently surfaced by Security + Devil's Advocate + Codex during #116 /idd-verify). The qualifier argument flowed from MCP client through Server.swift's create_rule handler into MailController.createRule's inline make new rule condition with properties {... qualifier:\(qualifier), ...} line without any validation — a caller passing a newline-bearing payload ("does contain value, expression:\"\"}\n do shell script \"…\"\n tell newRule\n make new rule condition with properties { header:\"X\"") could inject arbitrary AppleScript that ships into osascript (downstream CWE-78). Sibling slots (header, expression, rule name, move_message mailbox) were already escapeForAppleScript-protected; only qualifier was raw. Defense-in-depth fix mirrors #116 + extends the #104 PR-B/C/D extraction pattern: (1) inline AppleScript extracted into new Sources/CheAppleMailMCP/AppleScript/CreateRuleScriptBuilder.swift free function (buildCreateRuleScript(name:, conditions:, actions:)) — output preserves the pre-extraction AppleScript shape for valid inputs (pinned by testBuildCreateRuleScript_byteEquivalenceWithInlineImplementation via ordered-substring containment — smoke regression, not full byte-equality); (2) new ruleQualifierWhitelist: Set<String> constant with the 8 empirically-verified Apple Mail RuleQualifier enum values from /System/Applications/Mail.app/Contents/Resources/Mail.sdef (begins with value, does contain value, does not contain value, ends with value, equal to value, less than value, greater than value, none); (3) Server.swift create_rule handler loops conditions and guard ruleQualifierWhitelist.contains(qualifier) before delegating, throwing MailError.invalidParameter with the 8 valid values echoed back; (4) builder-side precondition (release-safe — Package.swift does not enable -Ounchecked) for programmer-error catch if any internal caller bypasses the handler gate. 10 new XCTest methods cover the whitelist contract (drift-pin + injection-payload rejection + case-strictness + whitespace-strictness), happy-path emission (all 8 qualifiers + multi-condition + escape preservation + move/bool actions), and a regression snapshot. Backward compat preserved: callers using any of the 8 documented qualifier enum values see identical AppleScript output and identical behavior; non-whitelisted callers (previously silently producing invalid AppleScript that osascript would error on) now get a clean MCP-level invalidParameter error with the valid set echoed back. With #116 (above), the two AppleScript-enum injection slots surfaced by the #116 verify ensemble are both closed.
  • resolveMsgRef / msgRefByAccountId now reject non-numeric message id in release builds (#118 — from #114's Security advisory #3 / Logic check #3). The two AppleScriptRefBuilder message-reference builders interpolated id unquoted into whose id is <id>, guarded only by a debug-only assert that compiles out under -O — a release-build caller bypassing Server.requireMessageId could inject an AppleScript predicate (#50 / CWE-89-adjacent class). Both asserts replaced with a release-safe numeric check (Int(id) != nil ? id : "-1"): a non-numeric id collapses to an impossible id, so the malicious string is never interpolated and the script fails cleanly with -1728. Byte-identical output for valid numeric ids. 4 new injection / byte-equivalence tests.
  • MailController.msgRef now rejects non-numeric message id in release builds (#145 — sister of #118, surfaced by #118's verify ensemble). The legacy private message-reference builder (6 live read-path callers: get_email / get_email_headers / get_email_source / get_email_metadata / list_attachments / save_attachment AppleScript fallback) carried the identical release-unsafe debug-only assert. The same release-safe Int(id) guard was applied; private was widened to internal purely as the test seam. With #118, all three whose id is <id> interpolation sites across the codebase are now release-guarded — the #50-class predicate-injection surface is fully closed. 3 new tests exercised via MailController.shared.