Skip to content

fix(config): make the SMTP port match the implicit-TLS transport - #328

Merged
argszero merged 1 commit into
mainfrom
fix/smtp-port-implicit-tls
Sep 30, 2026
Merged

argszero merged 1 commit into
mainfrom
fix/smtp-port-implicit-tls

Conversation

@argszero

Copy link
Copy Markdown
Owner

Summary

63fc76fd (2026-08-20) switched the sender from STARTTLS to implicit TLS (SmtpTransport::relay), because a TLS handshake against the STARTTLS port (587) makes the server answer in clear text and rustls fails with InvalidContentType. Its message says "port changed to 465", but the diff only touched prose: both values that actually carry the port stayed at 587 — the sample draft in config/config.example.toml and config.rs::default_smtp_port().

So both ways a user can pick up that number still give 587:

  • ensure_config (src/main.rs) copies config/config.example.toml to <ATP_DATA_DIR>/config.toml on first start, and DEFAULT_CONFIG embeds the same file for standalone binaries — uncommenting the [mail] block therefore reproduces exactly the failure 63fc76fd set out to fix;
  • #[serde(default = "default_smtp_port")] supplies 587 when smtp_port is omitted.

Either way the verification mail is never delivered (registration answers 502), and nothing tells the operator that the port is the problem.

Related Issue

No issue — found while auditing value carriers; nothing tracks it.

Changes

  • config/config.example.toml — sample smtp_port = 587 → 465, matching the paragraph five lines above it, which has said "implicit TLS → port 465" since 63fc76fd.
  • src/config.rs — default_smtp_port() returns 465, and its doc comment now names the mechanism (mail.rs picks the transport mode; relay = implicit TLS = 465) so the next protocol change updates the value instead of only the prose.
  • src/smtp_port_gate.rs (new, test-only, zero new dependencies) — keeps both carriers equal to the port the transport mode requires:
    • the expectation is derived, not a snapshot: the mode comes from mail.rs's code (SmtpTransport::relay( → implicit TLS, builder_dangerous( → STARTTLS) and the port from lettre's own constants (SUBMISSIONS_PORT / SUBMISSION_PORT), so switching to STARTTLS makes the gate demand 587;
    • the two extractors must be handled in opposite ways — the .toml sample is a comment line (comments must be read), while mail.rs must have its comments masked first (its header prose mentions builder_dangerous, which would otherwise be read as code);
    • one axis test + a positive-control test (every extractor must return something; the masker must drop the header prose) + a synthetic teeth test.
  • src/main.rs — #[cfg(test)] mod smtp_port_gate;.

Tests

  • cargo test — 429 passed / 0 failed (baseline 426; +3 new)
  • cargo fmt --check — clean
  • cargo clippy --all-targets -- -D warnings — clean
  • New/updated unit tests added

A/B, reverting only the two carriers to their pre-fix values while keeping the gate:

tree axis test report
pre-fix values FAILED names both carriers — config/config.example.toml:33 ... 587, should be 465 and src/config.rs:71 ... 587, should be 465
pre-fix, only the .toml reverted FAILED names only the .toml carrier ⇒ each rule has its own teeth
this PR ok (3/3) —

The two non-axis tests stay green in every leg, as they should (they judge the scanner, not the values).

Checklist

  • Branch name follows the convention (fix/…)
  • Commit message uses Conventional Commits
  • Single responsibility, minimal change

63fc76f switched the sender from STARTTLS to implicit TLS
(`SmtpTransport::relay`) and its message says "port changed to 465", but its
diff only touched prose: both port-carrying values stayed 587 —
`config/config.example.toml`'s sample draft and `config.rs::default_smtp_port()`.

Copying the sample (`ensure_config` writes it to `<ATP_DATA_DIR>/config.toml`
on first start, and `DEFAULT_CONFIG` embeds the same file) or omitting the key
(serde default) therefore pairs 587 with an implicit-TLS transport: the client
starts a TLS handshake on the STARTTLS port, Gmail answers in clear text and
rustls reports InvalidContentType, so the verification mail is never delivered
(registration returns 502).

Both carriers now say 465, and `config.rs` names the mechanism so the next
protocol change updates the value instead of only the prose.

New test-only gate `src/smtp_port_gate.rs` derives the expected port from the
transport mode in `mail.rs` (`SmtpTransport::relay` -> lettre's
`SUBMISSIONS_PORT`, `builder_dangerous` -> `SUBMISSION_PORT`) rather than
hardcoding it, so switching protocols demands both carriers follow.
@argszero

Copy link
Copy Markdown
Owner Author

Self-review (committer; GitHub does not let me approve my own PR).

Scope: two values that carry the SMTP port, plus a gate that keeps them equal to the port the transport mode requires. No production behaviour changes beyond the port default.

Checked

  • The fix targets the value carriers only. The protocol choice itself is untouched: mail.rs already rejects STARTTLS in prose ("to use 587 STARTTLS you must switch to SmtpTransport::builder_dangerous(...).tls(Tls::Opportunistic(...))"), so changing the implementation would be a design change, not a drift fix.
  • The gate derives its expectation instead of snapshotting it: mode from mail.rs's code, port from lettre's own constants. If someone switches to STARTTLS the gate will demand 587 in both carriers.
  • The two extractors are deliberately asymmetric and the doc comment says why: the .toml sample is a comment line, mail.rs must be masked first (its header prose names builder_dangerous, which would otherwise read as code).
  • A/B: with the pre-fix values the axis test fails and names both carriers with line numbers; with only one carrier reverted it names only that one. The other two tests stay green throughout, since they judge the scanner rather than the values.
  • Full suite, fmt --check and clippy --all-targets -- -D warnings are all clean (429/0, baseline 426).

Not in scope / explicitly not claimed

  • The gate proves the two carriers agree with the mode; it does not prove SMTP is reachable, and it does not police the prose in mail.rs's header (that is a documentation claim — fixed as data, not gated).
  • Mail's derived Default yields smtp_port = 0, but it has no consumers anywhere in the repo, so it is unreachable and left alone.

Merging since CI is green and the change is self-contained.

@argszero
argszero merged commit 2ce3a88 into main Sep 30, 2026
2 checks passed
@argszero
argszero deleted the fix/smtp-port-implicit-tls branch September 30, 2026 00:20
@argszero argszero mentioned this pull request Sep 30, 2026
12 tasks done
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.

1 participant