Skip to content

A non-numeric stored smtp_port is silently dropped and mail goes out on 587 instead — the setting is ignored without a word #13190

Description

@os-elon

Filed unassigned and ungraded by the #12993 dev seat while repairing the SMTP port drift surface. Different defect class from that card (that one is duplication; this is a silent degradation), so it was deliberately not folded in.

Measured

smtpOptionsFromMailSettings coerces smtp_port with Number(...) and then OMITS the key when the result is not finite, rather than refusing:

...(port!=null&&Number.isFinite(port) ? { port } : {}),

Measured on origin/main@dc729f358 through the built dist/index.js:

smtpOptionsFromMailSettings({ smtp_host: 'smtp.x', smtp_port: 'abc' })
-> { "host": "smtp.x" } // the port is GONE, not refused
new SmtpTransport(that).describe().port
-> 587 // the built-in default, silently

So a configured port of abc becomes a working-looking connection to 587. Nothing throws, nothing warns, and describe() — the diagnostic surface — reports 587 as though it had been chosen.

Why it is worth a card

SmtpTransport already refuses a bad port loudly, and would have: passing NaN through instead of dropping the key produces SmtpTransport: invalid port 'abc' (expected 1-65535). The omission is what converts a loud refusal into a silent default, one layer above the guard that exists to be loud.

AGENTS.md asks a degradation to be deliberate and visible; this one is neither. It is also the declared-but-not-delivered shape #5087 / #5094 closed for this same package's provider list: the admin's saved configuration and what the server actually does are allowed to differ with no signal.

Reachability — the form is not the only door

The mail settings form declares min: 1, max: 65535 on smtp_port and service-settings really does enforce it (declaredBounds / validatePatch), so the FORM refuses abc. That is not the whole population:

  • OS_* environment resolution feeds settings values without passing the form's numeric window (ADR-0007 precedence: OS_* env is above tenant and user).
  • Rows stored before the manifest carried its bounds are read back on every boot and never re-validated.

Both routes reach smtpOptionsFromMailSettings with a value the form would have rejected. Not established here: how many deployments actually carry one — the reachability argument above is read off the resolver's precedence, not measured against a real tenant.

Suggested shape, not established

Pass the coerced value through rather than dropping it, so the existing guard speaks: it already produces a refusal naming both the offending value and the range. The distinction that has to survive is absent (legitimately fall back to 587) versus present but unreadable (refuse) — the current code collapses the two, and smtp_port: '' is deliberately in the first bucket today.

Re-check

node -e "const m=require('./packages/plugins/plugin-email/dist/index.js'); console.log(JSON.stringify(m.smtpOptionsFromMailSettings({smtp_host:'smtp.x',smtp_port:'abc'})))"

Refs

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions