Repository navigation
id(directory_mail): OutboxSink writes To/Subject into the .eml header block with none of SmtpSink's validation — a newline in either forges arbitrary headers #2404
Description
Problem
pkgs/id/src/directory_mail.rs defines three MailSinks for the same Mail { to, subject, body }, but only one of them validates the header fields:
SmtpSink::send(directory_mail.rs:148ff) refuses a non-ASCII or non-plain recipient (plain_address, :148-160), refuses a sender that is not a plain address at construction (:130-143), and refuses a subject containing any control character: "the subject has a line break or control character" (:161-164). The module's own test at :453 sends a smuggleda@b.co>\r\nRCPT TO:<x@y.zrecipient and expects refusal.OutboxSink::send(directory_mail.rs:58-76) performs no validation at all and interpolates both fields straight into the header block of the.emlfile it writes:
write!(file, "To: {}\nSubject: {}\n\n{}", mail.to, mail.subject, mail.body)A subject (or to) containing \n therefore forges arbitrary extra headers in the stored message — Bcc:, a second Content-Type:, or a whole second message after a blank line — in a file whose entire purpose is to be read/picked up as email. Mail's fields are pub (directory_mail.rs:23-31) and the sinks are the crate's public mail API, so the invariant the SMTP path enforces is silently absent for any present or future caller of the file sink (today's only in-tree caller, code_mail in directory_view.rs:1466, happens to pass a normalized address and a fixed subject — the sink itself guarantees nothing).
Evidence (verified this run, main 5a34fa70)
Laptop probe test added to the verbatim directory_mail.rs tests in a laptop worktree (file md5 58718d786ae90d6ea7cb65688cbc20e2, identical on sandbox main and the laptop; worktree removed after the run):
let sink = OutboxSink::new(dir.clone());
let mail = Mail {
to: "victim@example.com".into(),
subject: "Your code\nBcc: attacker@evil.example".into(),
body: "hello".into(),
};
sink.send(&mail).unwrap();
// read back the single .eml file written
assert!(found.contains("\nBcc: attacker@evil.example"));Result: test directory_mail::tests::probe_outbox_writes_injected_header ... ok — the .eml on disk contains a real forged Bcc: header line. The same subject is refused by SmtpSink::send before any network I/O (control-character check), so the two sinks disagree on what a legal Mail is.
Existing tests cover the outbox happy path only (text.starts_with("To: a@b.co\nSubject: Your code\n\n"), :290); no test feeds a control character to OutboxSink.
Why it matters
The outbox directory is a trust boundary: files dropped there are mail. A header-forgery primitive in one sink but not its sibling is exactly the kind of inconsistency that gets exploited when a new caller (a world mail action with a user-chosen subject, a different confirmation flow) reuses the public sink trusting the module's established invariant. The fix is small and the invariant already exists in the same file — it should live in one place (on Mail construction or shared at the top of every send) instead of per-sink.
Acceptance criteria
-
OutboxSink::sendrefuses aMailwhosetois not a plain ASCII address or whosesubjectcontains a control character, with the same error wording/classes asSmtpSink— or the validation moves to a single shared place (e.g. a checkedMailconstructor /Mail::validate) that all three sinks use; either way, no sink writes unvalidated header fields. - A regression test mirrors the SMTP smuggling test (:453) for the outbox sink: a subject of
"Your code\nBcc: x@y.z"and a recipient containing\r\nare both refused and no.emlfile is created. - The legitimate path is unchanged: the existing outbox test (:290) and the directory confirmation flows (
code_mail, directory_view.rs:1466 — normalized address, fixed subjects) pass unmodified. - Out of scope:
CommandSinktimeout behaviour (id(directory_mail): CommandSink has no timeout — a mail program that never exits hangs the directory request forever #1788) and the SMTP reply-reader bounds (id(directory_mail): SmtpSink's reply reader has no total deadline and no line bound — a trickling or never-terminating relay reply hangs the directory request forever #1899) — already tracked separately.
Activity
developing-today commentedon Oct 29, 2026
CODE-1226
dezren39 commentedon Oct 29, 2026
🤖 Agent note · filed
OutboxSink::send(pkgs/id/src/directory_mail.rs:58-76) — it interpolatesto/subjectinto the .eml header block unchecked whileSmtpSinkin the same file refuses control characters and non-plain addresses; pin whether validation is per-sink or shared onMail, then route a small_fix packet.Bcc:header line from a newline-bearing subject, probe test passed, worktree removed.11 remaining items
dezren39 commentedon Nov 1, 2026
🤖 Agent note · done
dezren39 commentedon Nov 3, 2026
agent-note:v1
state: triaged (coverage pass) — fix already written, shepherd enqueued
Agent: queue worker (repo-worker-code), OpenChamber scheduled lane 02cafe76
Found via: queue claim (task 7916, triage)
Verdict: still live and already fixed in an open PR; the gap is that nobody is shepherding it. Follow-up enqueued as task 8166.
fix(id): validate outbox mail headers, head bb7e92d,pkgs/id/src/directory_mail.rs+44/-13) is the fix from queue task 5279 (2026-11-01): shared header validation now runs beforeOutboxSinkcreates the.emland beforeSmtpSinkopens the network, with the injection regression test.gh pr checks: onlyCodeRabbit skipping), so it needs main merged in and the policy check set run before merging. Task 8166 shepherds that; it must not re-implement.developing-today commentedon Nov 3, 2026
agent-note:v1
state: triaged (coverage pass)
Coverage pass 2026-11-03: reviewed in the fleet-wide per-issue pass (drift / overlap / already-handled / stale / blockers / ordering). Verdict: still live; queued in repo-health-code-pilot as triage at priority 30 (priority/low). If this is already handled at current main, the triage closes it with evidence; if it needs splitting, the triage files the split.
Found via: coverage pass 2026-11-03
Merge pull request #3423 from developing-today/queue/5279-2404-outbox…
dezren39 commentedon Nov 3, 2026
🤖 Agent note · done
3e65e3f3ba6e9939af954f2d5d5657116201a704.OutboxSinknow runs the sharedMail::validate_header_fieldsbefore creating the .eml file, so a newline in To/Subject can no longer forge headers; the two injection regressions are in the test suite (directory_mail: 20 passed / 0 failed on the laptop after the shepherd's conflict resolution against current main).