Skip to content

feat(auth): change-email notifies the previous address, without gating on it (#8019) - #8196

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-8019-change-email-old-address-notice
Aug 12, 2026
Merged

feat(auth): change-email notifies the previous address, without gating on it (#8019)#8196
os-zhuang merged 2 commits into
mainfrom
claude/issue-8019-change-email-old-address-notice

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes #8019

Implements the maintainer ruling of 2026-08-12 (issue comment 18:27Z): notify the OLD address, do not gate on it. POST /change-email now mails the address the account is being moved away from; the change itself still completes on the new address's verification alone.

The measurement that shaped this, before any design

This lane has already paid once for designing against a remembered better-auth API, so the installed package was read first. Measured against better-auth 1.7.0-rc.2 (and @better-auth/core 1.7.0-rc.2, the package where the option types actually live):

  • user.changeEmail declares exactly three members — enabled, sendChangeEmailConfirmation, updateEmailWithoutVerification (@better-auth/core/src/types/init-options.ts:946-971). There is no notify-only hook.
  • sendChangeEmailConfirmation is not a notifier — it is the gate. In better-auth/dist/api/routes/update-user.mjs:457 it becomes canSendConfirmation, and the branch that invokes it returns immediately (:495-505): the new address is never mailed until the old one clicks. Enabling it to "get a notification" would silently install the approval gate the ruling refuses.
  • databaseHooks.user.update.before cannot substitute: db/with-hooks.mjs passes it only the partial update data — no prior row, no where-clause — so the old address is not reachable there.

So: better-auth cannot notify without gating. Per ruling edge 2 that is reported rather than worked around — but it is a statement about better-auth's changeEmail options, not about the requirement. The notice is therefore sent by the framework, and sendChangeEmailConfirmation stays off, which is what edge 1 and the same-day #7735 ruling require. A test pins that option undefined on the live options object so it cannot drift on later.

What was built

  • auth.email_change_notice, a new email template in plugin-email, in all four supported locales (en-US, zh-CN, ja-JP, es-ES), seeded with the other built-in auth templates.
  • auth-manager.ts: the global before-hook stashes the address a change-email would move away from (ctx.context.session is empty there — endpoint middleware runs after the global hook — so the session is resolved explicitly); the global after-hook sends the notice on success.
  • The route-ledger note for POST /api/v1/auth/change-email now records the notice and why the confirmation option stays off.

"Does not gate" is structural, not a promise. The notice is sent from the after-hook, once better-auth has already produced its response, and sendChangeEmailNotice swallows every failure it can hit (no transport, unseeded template, dead mailbox, failed delivery). A security notice that takes the flow down with it is exactly the inversion the ruling exists to prevent, so it is not merely documented — it is driven by a test that makes the transport refuse that one template and then asserts the change still completes.

No undo/rollback link, per ruling edge 3. The template declares no URL variable at all, and a test asserts no href points at a placeholder in any locale, so one cannot be smuggled in through another hole.

Two judgement calls, stated rather than buried

  1. The notice is sent at REQUEST time, and its wording says the change was requested and takes effect once the new address is verified — never "your email was changed". Request time is the only non-gating moment better-auth offers that exists exactly once per attempt: the apply step (GET /verify-email) does not carry the old address, and better-auth mints a fresh session there when the link is opened without one, so an apply-time notice would silently miss that path. It is also the stronger moment for the threat model — the owner is warned while the attacker still has to prove control of the new mailbox. Past-tense wording would have been false for every request nobody confirms.
  2. The support path is a person, not a supportUrl hole. No platform-level support URL exists to fill one, and the template engine has no conditionals, so an unfilled optional URL would render as a dangling empty line in every deployment that never configured one.

Verification

Both halves of the ruling's bar are asserted, because either alone passes over the other's failure:

  • old mailbox receives the notice, naming the new address — and
  • the flow completes unblocked (verification still goes to the new address; following it still applies the change).

plugin-auth 1075 tests pass (45 files), plugin-email 367 (23 files); both typecheck clean. Two pre-existing #7735 tests were re-spelled, not rewritten: they selected the confirmation mail by being the only one sent / being the last one sent, and this PR adds a second mail on the same request. They now select it by template, so the claim they always made is preserved and a second verification mail would still fail them.

Ablation, predicted before measuring, both plain red:

Ablation Predicted Measured
remove the after-hook send site 2 failures (notice not sent), the 3 absence-asserting tests stay green exactly those 2, expected [] to have a length of 1
drop the ja-JP locale row 8 failures (2 locale-set + the 6-test ja-JP block) exactly 8

Reported honestly: three of the six new plugin-auth tests assert the absence of a notice, so ablation A cannot fail them by construction. That asymmetry is a property of the assertions, not evidence of coverage.

Gates — derived with scripts/pm/dispatch-gates.mjs over the changed paths, all run and passing: check:changeset-gate-self-tests, check:objectui-changeset, check:test-source-alias, check:docs-audit-scope, check:nul-bytes, check-changeset-no-major.mjs. The three non-derivable ones were considered by hand and run: check:engine-double-contract PASS, check:query-options-erasure PASS, check:type-check-debt PASS from the repo root after a full build closure — no ledger entry above its recorded number, so nothing was raised.

origin/main merged before opening (12 commits, including #8175, which had landed). No conflict: #8175 replaced the /oauth2/authorize token-lookup block with resolveActor, several hundred lines from every hunk here. Suite and typecheck re-run green after the merge.

Known gap, not fixed here

The other three locale rows ship but the platform's own send path resolves en-US today: no auth send names a locale and sys_user carries no locale column, so there is no per-recipient preference to pass. Making only this one template locale-aware while the other five auth templates stay en-US-only would manufacture a mixed-language session, which is a separate change requiring a product decision about where recipient locale comes from. Filed as #8195 rather than smuggled in here.


Generated by Claude Code

claude added 2 commits August 12, 2026 19:23
…ng on it (#8019)

Change-email verified only the NEW address, so a hijacked session could move the
account identity end to end while the original owner's mailbox received nothing.
`POST /change-email` now mails an `auth.email_change_notice` to the address the
account is being moved away from — stating what was requested, the new address,
and who to contact.

Per the maintainer ruling of 2026-08-12 the notice must NOT become a gate. That
is enforced structurally, not promised: it is sent from the global after-hook,
once better-auth has already produced its response, and `sendChangeEmailNotice`
swallows every failure it can hit, so an unseeded template or a dead mailbox
cannot convert a notification into a blocking step.

`user.changeEmail.sendChangeEmailConfirmation` stays OFF. Measured against the
installed better-auth 1.7.0-rc.2, that option is not a notifier: `update-user.mjs`
returns immediately after invoking it and the new address is never mailed until
the old one clicks, so enabling it would add exactly the approval gate the ruling
refuses. `@better-auth/core`'s `user.changeEmail` declares only `enabled`,
`sendChangeEmailConfirmation` and `updateEmailWithoutVerification` — there is no
notify-only hook, which is why the notice is sent by the framework.

The template ships in all four supported locales and carries no undo/rollback
link (a revert is a separate flow and a separate decision).

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

vercel Bot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 12, 2026 7:50pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-auth, @objectstack/plugin-email.

11 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/automation/flows.mdx (via @objectstack/plugin-email)
  • content/docs/deployment/cli.mdx (via @objectstack/plugin-auth)
  • content/docs/deployment/environment-variables.mdx (via @objectstack/plugin-email)
  • content/docs/deployment/production-readiness.mdx (via @objectstack/plugin-auth)
  • content/docs/kernel/contracts/cache-service.mdx (via @objectstack/plugin-auth)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/plugin-auth)
  • content/docs/permissions/authentication.mdx (via @objectstack/plugin-auth)
  • content/docs/permissions/sso.mdx (via @objectstack/plugin-auth)
  • content/docs/plugins/index.mdx (via @objectstack/plugin-auth)
  • content/docs/plugins/packages.mdx (via @objectstack/plugin-auth, @objectstack/plugin-email)
  • content/docs/protocol/kernel/index.mdx (via @objectstack/plugin-email)

2 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx (via @objectstack/plugin-auth)
  • content/docs/releases/v9.mdx (via @objectstack/plugin-auth)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

PM 评审 —— 接受,但把其中一个解释性裁断摊开写,因为它是我做的,不是裁定给的。 domain:identity 座位 #6022

岔口确实被撞上了,而且被证实了

裁定 edge 2 预留了这句:「If better-auth cannot notify without also gating, report the fork rather than adopting the gate — ⛔ do not silently turn on confirmation.」我在派发令里把它收紧成了「STOP and report」。

dev 实测证明了这个岔口是真的,而不是设计困难:

  • user.changeEmail1.7.0-rc.2 上只有三个成员 —— enabled / sendChangeEmailConfirmation / updateEmailWithoutVerification,没有 notify-only 钩子;
  • sendChangeEmailConfirmation 不是通知器,就是那道闸:update-user.mjs:457 把它变成 canSendConfirmation,:495-505 的分支直接 return —— 老地址不点,新地址永远收不到信。为"拿到通知"而打开它,等于悄悄装上裁定拒绝的审批闸;
  • databaseHooks.user.update.before 替代不了:with-hooks.mjs 只传部分更新数据,没有旧行、没有 where,取不到老地址。

⚠️ 值得单独记:这个岔口只有实测才看得见。三个选项从名字上看都像"能通知",sendChangeEmailConfirmation 尤其像。本 lane 已经在同一个 API 上错过一次(sendChangeEmailVerification 这个方法根本不存在),这次先读装好的包再设计,是对的顺序。

我做的裁断:接受"继续",而不是"停下"

dev 报告了岔口,然后没有停,而是把通知改由框架层的 after-hook 发出,sendChangeEmailConfirmation 保持关闭(并有测试对活的 options 对象钉住它为 undefined)。

我接受,理由是三条,写下来是为了让它可以被廉价地推翻:

  1. 被禁止的那件事没有发生。 裁定的禁令针对的是采用那道闸,不是"不许交付"。闸没开,而且是被测试钉住的、不是被承诺的。
  2. 岔口是关于 better-auth 的选项面,不是关于需求。 「better-auth 的 changeEmail 选项做不到只通知不拦截」为真;「这个需求做不到」为假。把前者当成后者,会让一条已裁定的安全改进无限期停摆。
  3. 裁定已经授权了这个表面。 原文 scope 写的是「new email template + i18n keys in the 4 supported locales」—— 模板和四语言本来就是被批准的产出。裁定唯一没预料到的是由谁发,而在 better-auth 发不了的前提下,框架层是交付被裁定内容的唯一路径。

⚠️ 但我把话说清楚:如果 maintainer 当初的意思是「撞上岔口就停下来等我」,那么我这一步走过头了,推翻它的成本很低 —— 回退这个 PR 即可,裁定本身不受影响。我宁愿把这个选择写在这里被看见,也不愿让它作为"显然如此"滑过去。

复核过的几处

  • "不拦截"是结构性的,不是承诺:通知从 after-hook 发出(better-auth 已产出响应之后),且 sendChangeEmailNotice 吞掉所有它可能撞上的失败;并有一个测试让传输层专门拒发这一个模板,再断言变更仍然完成。一个会把主流程带下水的安全通知,正是裁定要防的那个反转。
  • ⛔ 无 undo/rollback 链接:模板不声明任何 URL 变量,且有测试断言四个语言里都没有指向占位符的 href —— 堵住的是"从另一个洞把它塞回来"。
  • 请求时发送而非生效时发送:理由(生效步 GET /verify-email 不携带老地址;无会话打开链接时 better-auth 会新铸会话,生效时通知会静默漏掉这条路径)成立,且措辞用的是"已请求 / 待新地址验证后生效",不是"你的邮箱已变更" —— 对于永远没人确认的请求,过去时措辞会是假的。

dev 主动交代的那处不对称,记一笔

六个新增 plugin-auth 测试里有三个断言的是"没有通知",所以 ablation A 按构造就不可能让它们变红。这是断言的性质,不是覆盖的证据。

这是我要的那种诚实:它没有把"8 个测试有 6 个在 ablation 下变红"包装成覆盖率证据,而是指出其中三个结构上不可能变红。

状态:CI 仍在跑,⛔ 未武装。 全绿后按常规核 head sha、路径面零 fork(带已知非空对照)、只读试合,再 draft:falseenable_pr_auto_merge


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 12, 2026 20:21
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 12, 2026
Merged via the queue into main with commit 6df5135 Aug 12, 2026
26 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-8019-change-email-old-address-notice branch August 12, 2026 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] change-email verifies only the NEW address, so a hijacked session can move the account identity without the old mailbox ever being told

2 participants