Skip to content

id(directory_mail): plain_address accepts '@', 'a@', '@b' and 'a@@b.c' — SmtpSink's only address-shape gate never counts the @ or checks either side is non-empty #2637

Closed

Description

@dezren39

Problem

plain_address in pkgs/id/src/directory_mail.rs:199-205 — the only shape check behind SmtpSink — is just:

fn plain_address(text: &str) -> bool {
    text.contains('@')
        && text
            .chars()
            .all(|c| c.is_ascii_graphic() && !matches!(c, '<' | '>' | ',' | ';' | '"' | '\\'))
}

It never checks that there is exactly one @, or that either side of it is non-empty. So strings with no local part, no domain, or several @s all pass as a "plain address".

Evidence (verified this run, main dd2c88ff)

Laptop probe over a verbatim copy of plain_address (probe: ~/git/fleet/probe-2026-10-30-laneA-plainaddr/, rustc, main dd2c88ff):

"a@b.co" => true            "first.last+tag@example.test" => true
"@" => true                 "a@" => true
"@b" => true                "a@@b.c" => true
"a@b@c.d" => true           "a@b@c" => true
"no-at-sign" => false       "a b@c.d" => false

Call sites that trust it:

  • SmtpSink::new (directory_mail.rs:129-144) documents "Fails if … from is not a plain address" and gates the operator-supplied sender (--world-mail-from, commands/serve.rs:478-485) on is_ascii + plain_address only. SmtpSink::new(relay, "@"), "a@", "@b", "a@@b.c" all return Ok.
  • SmtpSink::send (directory_mail.rs:150-166) gates the recipient on the same function, then formats both addresses straight into the wire: MAIL FROM:<{from}>, RCPT TO:<{to}>, and the From:/To: headers. A bogus sender therefore surfaces only at send time as an opaque relay reply error from expect() — or, against a lenient relay, goes out as a null / double-@ reverse-path, so confirmation mail (the directory's email codes, directory_view.rs:1466 code_mail) fails or bounces nowhere instead of the configuration being refused at startup.

Contrast with the directory's own email shape: normalize_email (directory.rs:1800-1828) requires exactly one @ in effect (split_once + a domain charset that excludes @), a non-empty local part (≤64, no leading/trailing/doubled dots), and a domain with at least one dot and valid labels. Recipients that arrive through account confirmation are held to that; the SMTP sink's own gate — the layer whose job is not to trust its caller — is held to almost nothing. The module's tests (directory_mail.rs:405-412) pin the no-@ and non-ASCII cases but no empty-side or multi-@ case.

Related, distinct: #2404 (OutboxSink header injection — missing control checks), #1899 (reply-reader deadline), #1788 (CommandSink timeout). None covers this shape gap.

Why it matters

SmtpSink is the fail-closed layer for confirmation mail: its constructor and send checks exist so a bad address is refused before a connection is opened (a property its tests assert for the internationalised case). A shape check that accepts @ alone defeats that for the one address no other layer validates — the configured sender — turning a startup-time config error into a per-message relay failure, or into mail sent with an unusable reverse-path.

Acceptance criteria

  • plain_address (or its replacement) refuses an address with zero or more than one @, an empty local part, or an empty domain — at minimum "@", "a@", "@b", "a@@b.c", "a@b@c.d" all return false / are refused.
  • SmtpSink::new refuses those senders at construction with the existing "not a plain address" error, and SmtpSink::send refuses those recipients before connecting (pinned by a no-connection test like the SMTPUTF8 one).
  • Legitimate addresses keep passing: a@b.co, first.last+tag@example.test, and the existing test set at directory_mail.rs:405-412.
  • Triage pins the intended strictness: whether the sink should reuse crate::directory::normalize_email's shape (one @, dotted domain, label rules) or a deliberately weaker exactly-one-@ + non-empty-sides rule — either way the choice is recorded and tested, and any divergence from normalize_email is documented in the module.
  • Existing directory_mail tests pass.

Activity

  1. added
    kind/bugSomething isn't working (prefixed form of the classic bug label)
    priority/lowNice to have; do when convenient
    queue/queuedHas a queued task in the Supabase agent queue
    source/creatorFiled by the new-issue-creator job
    task/triageQueue packet kind: verify, reproduce, and route this item
    app/idPackage/app: id (pkgs/ or apps/ in this repo)
    effort/xsTrivial: expected under 30 minutes of work
    on Oct 30, 2026
  2. dezren39 commented on Oct 30, 2026

    @dezren39
    MemberAuthor

    🤖 Agent note · filed

    • Agent: new-issue-creator job (lane A, new-issue-creator-code-a)
    • Session / ID: cron new-issue-creator-code-a run 2026-10-30 03:35 CDT
    • Found via: filed this run
    • Task: Filed after verifying plain_address verbatim on the laptop: it accepts "@", "a@", "@b", "a@@b.c" — SmtpSink's sender/recipient gate never counts the @ or checks either side.
    • Expectation: effort/xs triage — pin whether the sink reuses normalize_email's shape or an exactly-one-@ + non-empty-sides rule, then route the fix.
    • Links: probe laptop ~/git/fleet/probe-2026-10-30-laneA-plainaddr/
  3. added
    work/assignedQueue worker lifecycle state
    queue/claimedQueue worker lifecycle state
    and removed
    queue/queuedHas a queued task in the Supabase agent queue
    on Nov 1, 2026
  4. 28 remaining items

  5. added
    queue/claimedQueue worker lifecycle state
    status/parkedDeliberately paused with a recorded resume condition
    queue/parkedQueue task is parked (waiting on CI, time, or an event)
    and removed
    queue/claimedQueue worker lifecycle state
    status/parkedDeliberately paused with a recorded resume condition
    queue/parkedQueue task is parked (waiting on CI, time, or an event)
    on Nov 2, 2026
  6. added a commit that references this issue on Nov 3, 2026
  7. dezren39 commented on Nov 3, 2026

    @dezren39
    MemberAuthor

    task 5421 (repo-health-code-pilot, lane code-b): done. PR #4653 merged (merge commit 5082108) — plain_address now requires exactly one @ with non-empty local/domain sides; 18/18 directory_mail tests passed; Codex review on head 849790c reported no findings. Basic guard check was still QUEUED fleet-wide at merge; merged under standing merge authority. Issue closed by the merge.

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

Metadata

Metadata

Assignees

Labels

app/idPackage/app: id (pkgs/ or apps/ in this repo)effort/xsTrivial: expected under 30 minutes of workkind/bugSomething isn't working (prefixed form of the classic bug label)priority/lowNice to have; do when convenientsource/creatorFiled by the new-issue-creator jobtask/small-fixQueue packet kind: bounded fix with reproduction and targeted tests

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions