fix(security): make the SMTP password write-only, never sent to the browser (BUNYIP-432) #432

Merged

longjacksonle merged 1 commit from fix/BUNYIP-432-smtp-password-write-only into main

2026-08-01 22:26:20 +02:00

longjacksonle commented

2026-08-01 22:21:57 +02:00

Owner

What and why

The admin email settings response carried smtp_password_masked, built with the shared mask_secret helper, which reveals the last four characters of the real password (sk...-style prefix too). The server decrypted the stored SMTP password and sent a truncated form of it to the client on every page load, so the secret left the server and was partly recoverable from the page payload or a proxy. This is the PMS-342 defect class for a different credential.

Fix

The field is now write-only:

The response carries only the boolean has_smtp_password. The password field (smtp_password_masked) is removed from both the domain EmailConfigResponse and the web mirror, so no plaintext and no masked/last-4 form is serialized - it is structurally impossible to leak from this endpoint now.
The settings page renders a fixed-length mask (••••••••, a constant independent of the real length) driven solely by has_smtp_password, so nothing about the secret is inferable from the display. When no password is set it shows a distinct non-secret placeholder.
Saving a new password still works; leaving the field blank still leaves the stored password unchanged (the web omits the field when empty, the API only re-encrypts a non-empty value - both pre-existing, unchanged here).

Storage review (acceptance criterion 4)

The SMTP password is already encrypted at rest with AES-256-GCM via the shared EncryptionKeySet (smtp_password ciphertext + smtp_password_nonce + key_version; EmailConfig::from_db_row decrypts, the update handler encrypts). The LC-164 approach is already in place - it is not recoverable in plaintext from the database or config, so no storage migration is needed. Only the read path was leaking it.

Other secrets on the surface (acceptance criterion 5)

The email settings page has exactly one secret, the SMTP password, now write-only. No other secrets are on that surface.

Cross-surface finding (out of scope, flagged for follow-up): the same mask_secret (last-4) helper is used by the Stripe config response (secret_key_masked / webhook_secret_masked) on the /admin/stripe surface, so those keys are still returned in the last-4 masked form. That surface is owned by PMS-342 (marked Done with the last-4 mask). Under this ticket's stricter write-only posture the same treatment would apply; recommend a follow-up rather than expanding this PR into the Stripe UI.

Acceptance criteria

The settings page payload contains no representation of the SMTP password.
The field renders a fixed-length mask independent of the real password length.
Saving a new password works; saving with the field untouched leaves it unchanged.
Storage reviewed: already AES-256-GCM encrypted at rest, not recoverable in plaintext.
Other secrets on the surface checked and recorded (only the SMTP password here; Stripe noted for follow-up).
Test asserting the password is absent from the settings response body.

Verification

just check-container green (173 web tests, api handler tests, 312 domain; clippy -D warnings, fmt). New tests: the API email-config response body carries no password (the only password token is has_smtp_password; smtp_password_masked absent), and the web password field renders the fixed-length mask when set and a non-secret placeholder when not. The guarantee is structural - the field no longer exists on either response type, so no endpoint can serialize it. I also confirmed no other endpoint returns the password: the test-connection response (BUNYIP-433) and the email-update audit (password_changed boolean) never carry it.

🤖 Generated with Claude Code

https://claude.ai/code/session_018TXaT3P192nDsZzbzHETb9

## What and why The admin email settings response carried `smtp_password_masked`, built with the shared `mask_secret` helper, which reveals the last four characters of the real password (`sk...`-style prefix too). The server decrypted the stored SMTP password and sent a truncated form of it to the client on every page load, so the secret left the server and was partly recoverable from the page payload or a proxy. This is the PMS-342 defect class for a different credential. ## Fix The field is now write-only: - The response carries only the boolean `has_smtp_password`. The password field (`smtp_password_masked`) is removed from both the domain `EmailConfigResponse` and the web mirror, so no plaintext and no masked/last-4 form is serialized - it is structurally impossible to leak from this endpoint now. - The settings page renders a fixed-length mask (`••••••••`, a constant independent of the real length) driven solely by `has_smtp_password`, so nothing about the secret is inferable from the display. When no password is set it shows a distinct non-secret placeholder. - Saving a new password still works; leaving the field blank still leaves the stored password unchanged (the web omits the field when empty, the API only re-encrypts a non-empty value - both pre-existing, unchanged here). ## Storage review (acceptance criterion 4) The SMTP password is already encrypted at rest with AES-256-GCM via the shared `EncryptionKeySet` (`smtp_password` ciphertext + `smtp_password_nonce` + `key_version`; `EmailConfig::from_db_row` decrypts, the update handler encrypts). The LC-164 approach is already in place - it is not recoverable in plaintext from the database or config, so no storage migration is needed. Only the read path was leaking it. ## Other secrets on the surface (acceptance criterion 5) The email settings page has exactly one secret, the SMTP password, now write-only. No other secrets are on that surface. Cross-surface finding (out of scope, flagged for follow-up): the same `mask_secret` (last-4) helper is used by the Stripe config response (`secret_key_masked` / `webhook_secret_masked`) on the /admin/stripe surface, so those keys are still returned in the last-4 masked form. That surface is owned by PMS-342 (marked Done with the last-4 mask). Under this ticket's stricter write-only posture the same treatment would apply; recommend a follow-up rather than expanding this PR into the Stripe UI. ## Acceptance criteria - [x] The settings page payload contains no representation of the SMTP password. - [x] The field renders a fixed-length mask independent of the real password length. - [x] Saving a new password works; saving with the field untouched leaves it unchanged. - [x] Storage reviewed: already AES-256-GCM encrypted at rest, not recoverable in plaintext. - [x] Other secrets on the surface checked and recorded (only the SMTP password here; Stripe noted for follow-up). - [x] Test asserting the password is absent from the settings response body. ## Verification `just check-container` green (173 web tests, api handler tests, 312 domain; clippy `-D warnings`, fmt). New tests: the API email-config response body carries no password (the only `password` token is `has_smtp_password`; `smtp_password_masked` absent), and the web password field renders the fixed-length mask when set and a non-secret placeholder when not. The guarantee is structural - the field no longer exists on either response type, so no endpoint can serialize it. I also confirmed no other endpoint returns the password: the test-connection response (BUNYIP-433) and the email-update audit (`password_changed` boolean) never carry it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_018TXaT3P192nDsZzbzHETb9

longjacksonle added 1 commit

2026-08-01 22:21:57 +02:00

fix(security): make the SMTP password write-only, never sent to the browser (BUNYIP-432)

E2E PR gate / Install + reachability (no deployment secrets) (pull_request) Successful in 15s

Details

Check / fmt + clippy + build + tests (pull_request) Successful in 4m20s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

f67acee851

The admin email settings response carried `smtp_password_masked`, built with `mask_secret`, which reveals the last four characters of the real password. The server was decrypting the stored password and sending a truncated form of it to the client on every page load, so the secret left the server and was recoverable in part from the page payload or a proxy.

The field is now write-only. The response carries only the boolean `has_smtp_password`; no plaintext and no masked/last-4 form of the password is serialized. The settings page renders a fixed-length mask (a constant string, independent of the real password length) driven solely by that flag, so nothing about the secret is inferable from the display. Saving a new password still works, and leaving the field blank still leaves the stored password unchanged (the web omits the field when empty and the API only re-encrypts a non-empty value - unchanged by this PR).

Storage review (acceptance criterion 4): the SMTP password is already encrypted at rest with AES-256-GCM via the shared `EncryptionKeySet` (nonce + key_version columns; `EmailConfig::from_db_row` decrypts, the update handler encrypts), i.e. the LC-164 approach is already in place. It is not recoverable in plaintext from the database or config, so no storage migration is needed - only the read path was leaking it.

Other secrets on the surface (acceptance criterion 5): the email settings page has exactly one secret, the SMTP password, now write-only. Recorded separately on the issue: the shared `mask_secret` helper is also used by the Stripe config response (`secret_key_masked` / `webhook_secret_masked`) on the /admin/stripe surface, so those keys are still returned in the same last-4 masked form. That is a different surface owned by PMS-342 (marked Done with the last-4 mask); flagging it for a follow-up under the stricter write-only posture rather than expanding this PR.

Tests: an API test asserts the email-config response body contains no password representation (the only `password` token is `has_smtp_password`, and `smtp_password_masked` is absent), and a web test asserts the password field renders the fixed-length mask when a password is set (and a distinct non-secret placeholder when not), with no last-4 or old placeholder leaking into the markup.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018TXaT3P192nDsZzbzHETb9

longjacksonle scheduled this pull request to auto merge when all checks succeed

2026-08-01 22:22:36 +02:00