Skip to content

feat(plugin-auth): a forced password change requires a password different from the current one - #27440

Merged
objectstack-fleet[bot] merged 9 commits into
mainfrom
claude/harden-forced-password-change
Oct 29, 2026
Merged

objectstack-fleet[bot] merged 9 commits into
mainfrom
claude/harden-forced-password-change

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk

Clause-②: no (narrowing)
Metadata-protocol: none
Skills-docs: none — no skills/** file teaches the forced password change, passwordHistoryCount or the change-password door (git grep -i over skills/ for must_change_password, mustChangePassword, passwordHistoryCount, change-password, reset-password, PASSWORD_REUSE, PASSWORD_RESET_REQUIRED: 0 hits each; control defineStack: 22 files); the hand-written authentication guide says the user is held "until they complete a password change", which stays true.

维护者速读

  • 改了什么:管理员要求"下次登录必须改密码"的用户,完成这次强制改密时新密码必须与当前(管理员发的临时)密码不同;/change-password 与 /reset-password 两个能清除该标记的入口都拒绝相同密码(400 PASSWORD_REUSE),控制台强制改密页用用户语言提示同一句话。另外 /change-password 上任何"密码重复"的拒绝(强制改密的和 passwordHistoryCount 历史环的)都只在当前密码验证通过之后才给出,当前密码错误一律回 INVALID_PASSWORD。
  • 为什么改:强制改密的目的就是让用户离开管理员知道的密码;现在由服务端直接保证,不再取决于 passwordHistoryCount 的设置。这是一次接受集收窄(BREAKING,按 patch 发布)。
  • 风险与回滚:只影响强制改密标记仍在的用户;用临时密码原样"改密"的自动化脚本会被拒。passwordHistoryCount 大于 0 时,当前密码错误且新密码命中历史的请求,答复从 PASSWORD_REUSE 变为 INVALID_PASSWORD。读标记失败时放行并写一条降级日志。回滚即 revert 本 PR。
  • 先看:packages/plugins/plugin-auth/src/auth-manager-password-session-policy.ts(assertPasswordNotReused 里"先验当前密码再判新密码"那段与 isForcedRotationPending 的降级日志)、auth-manager-before-hook.ts(传入 currentPassword 的那几行)、.changeset/forced-password-change-new-password.md(声明行与 FROM/TO)。
  • 两分钟验证:pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2 src/forced-password-change-new-password.test.ts,预期 8/8 通过。
  • 需要你做的:无,席位落地。

What changes

  • BREAKING (an accept-set narrowing), shipped as patch. The metadata protocol does not move (Metadata-protocol: none), so all three packages stay patch and the changeset carries the **BREAKING** banner, a FROM / TO paragraph and its ADR-0087 disposition, not-required (no-migration-prescription): no metadata key, spelling, export or stored shape moves; what narrows is a credential door's accept set.
  • plugin-auth, the forced change. While an administrator's forced password change (sys_user.must_change_password) is outstanding, assertPasswordNotReused refuses a new password that verifies against the CURRENT hash with 400 PASSWORD_REUSE and the message "Choose a new password that is different from your temporary password.", whatever passwordHistoryCount is (the default 0 included). The flag is cleared only by stampPasswordChangedAt, which the after-hook runs on a successful /change-password or /reset-password; both doors pass through this check in the before-hook first, so every user-facing door that clears the flag is covered. Sign-up (a new user) and the administrator's own Set Password with the requirement switched off are the other writers of the flag and are not user completion doors.
  • A reuse refusal is given only after the current password is confirmed, for both the forced change and the passwordHistoryCount ring. On /change-password better-auth verifies currentPassword only after the before-hook, so assertPasswordNotReused first verifies the request's currentPassword against the current hash, and judges the new password (forced refusal, then the history ring) only when that holds. A wrong current password is left to the endpoint's own INVALID_PASSWORD, whatever the new password is. The changed answer: with passwordHistoryCount above 0, a wrong current password with a recycled new one answered PASSWORD_REUSE and now answers INVALID_PASSWORD; a confirmed request reusing the current or a recent password still answers PASSWORD_REUSE. /reset-password carries no such claim (its token is the authority) and is judged directly.
  • FROM / TO. FROM: with the flag set and the default history depth, a change or reset to the same password completed and cleared the flag; a wrong current password with a recycled new one answered PASSWORD_REUSE. TO: refused 400 PASSWORD_REUSE until a different password is chosen; a wrong current password answers INVALID_PASSWORD whatever the new one is.
  • The flag read is reported when it fails. isForcedRotationPending still fails open (treated as not set) on a sys_user lookup error, as every read in assertPasswordNotReused does, and now reports that the forced-change check did not run through logDurabilityDegradation, the channel stampPasswordChangedAt reports its write through. The assertPasswordNotReused docblock in auth-manager.ts and the before-hook comment say the claim gate sits above the history ring as well as the forced refusal.
  • Unchanged: with no forced change outstanding, which passwords count as reuse is decided by passwordHistoryCount exactly as before (depth 0 still accepts the same password). The code is the existing PASSWORD_REUSE (the condition is reuse of the current password), so the error-code ledger and packages/spec do not move. Cost: one sys_user flag read per change or reset, and on /change-password one extra password verification.
  • Console. The forced-reset screen (PASSWORD_RESET_REQUIRED) shows auth.remediation.password.sameAsTemporary in the user's language when the new password equals the one typed as current, and sends no request. The key is added to all ten locale packs (the key-parity test requires it). An expiry (PASSWORD_EXPIRED) screen is unchanged; the server judges reuse there.
  • .changeset/forced-password-change-new-password.md: @objectstack/plugin-auth, @object-ui/app-shell, @object-ui/i18n patch, with the two declaration lines, the banner, FROM / TO and the ADR-0087 marker.

Tests

  • New packages/plugins/plugin-auth/src/forced-password-change-new-password.test.ts, end to end through AuthManager.handleRequest over a real ObjectQL engine (better-sqlite3, the plugin's own manifest, so the internal: true strip of sys_account.password is live). Forced change, history off: refused on /change-password with the flag set (status, code, message; flag still set; current password still signs in); a different password completes and clears the flag; a wrong current-password claim gets INVALID_PASSWORD, not PASSWORD_REUSE; /reset-password refused then completed; control: no flag and depth 0 still accepts the same password.
  • Same file, history on (no forced change), the three ordering cases: a confirmed current password with a password from the history ring answers PASSWORD_REUSE; a confirmed current password reused as the new one answers PASSWORD_REUSE; a wrong current password answers the endpoint's own INVALID_PASSWORD, whatever the new password is.
  • auth-manager.test.ts: the depth-0 no-op case pins that only the sys_user flag is read and sys_account is not; a new case pins that a failed sys_user flag read fails open (depth 0 accepts) and reports exactly once through the logger with { object: 'sys_user', fields: ['must_change_password'] }.
  • New ui/packages/app-shell/src/console/__tests__/RemediationOverlay.forcedResetNewPassword.test.tsx: en and zh text shown and no request sent; a different password is sent; an expiry screen sends the same password on.

Acceptance notes

  • The flag read fails open (treated as not set) on a lookup error, as every read in assertPasswordNotReused already does; the failure is now logged at the durability channel's level (warn, the one level AuthManagerOptions.logger guarantees), once per affected request.
  • The Console check compares the two typed strings; it never replaces the server's refusal, which stays the authority for every client.

Verification

Every reading below is on head 44d99fe72 (this PR's head; origin/main not merged again: GitHub reports the branch mergeable, merge base c5d463417), taken in the rework run of session_017xRBcVy4VXmkQjn2MGeaWk. The readings an earlier body cited on f1c03b281 are superseded by these.

Package tests and typecheck
  • pnpm --filter @objectstack/plugin-auth test: Test Files 203 passed (203), Tests 3852 passed, 10 skipped (3862).
  • pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2 src/auth-manager.test.ts src/forced-password-change-new-password.test.ts: Test Files 2 passed (2), Tests 313 passed (313).
  • pnpm --filter @objectstack/plugin-auth build && pnpm --filter @objectstack/plugin-auth typecheck: exit 0; check:test-typecheck: OK (10 files / 88 errors / 23 pinned signatures held, unchanged).
  • From ui/: pnpm exec vitest run --maxWorkers=2 packages/app-shell/src/console/__tests__/RemediationOverlay: Test Files 3 passed (3), Tests 12 passed (12); the forced-reset file is one of the three.
  • Dogfood battery, closure built with pnpm --filter '@objectstack/dogfood^...' build, then under the verify lock pnpm --filter @objectstack/dogfood test (no shard, no path): VERDICT command-exit 0, Test Files 348 passed, 3 skipped (351), Tests 2806 passed, 20 skipped (2826).
Ablation of the new log call (landed by `scripts/ablation-replace.mjs`, restored to the HEAD blob, `git diff HEAD` empty)
  • host.logDurabilityDegradation( replaced with a no-op call (anchor 1 to 0, blob 93a97d4a2ae2 to 2127a2c857e4): auth-manager.test.ts 1 failed, 304 passed, the one red being the new failed-flag-read case. Restored: blob after restore equals the HEAD blob 93a97d4a2ae2.
  • The earlier ablations of the forced refusal, the claim gate and the Console check were taken on f1c03b281 and are not re-run here; those lines are unchanged on this head.
Declaration gates
  • pnpm check:adr-0087-registration: exit 0; "1 declared-breaking changeset(s), each carrying an ADR-0087 disposition", .changeset/forced-password-change-new-password.md [BREAKING+clause-②-narrowing] not-required (no-migration-prescription).
  • node scripts/check-changeset-no-major.mjs: exit 0; "This diff introduces no major bump"; the level axis reads NOT APPLICABLE locally (no payload).
  • node scripts/check-changeset-no-major.mjs --event EVENT_JSON, with a pull_request payload whose body is this body (GITHUB_EVENT_NAME=pull_request, --base origin/main): exit 0; "LEVEL AXIS: this PR declares clause-② no (narrowing) and metadata-protocol none, and it introduces no changeset entry graded minor or above and moves no metadata-protocol baseline".
Derived gate families

node scripts/pm/dispatch-gates.mjs --commands at 44d99fe72: 67 commands, each run locally with its exit code captured before any pipe. 64 exit 0 on the first pass; three first read as build-state prerequisites (exit 3 PREREQUISITE NOT MET for check:dual-build-cjs-loads and check:type-check-debt, and pnpm --dir ui exec vitest run scripts/ red only on an unbuilt @objectstack/client entry), and each exits 0 once re-run after the full package build: check:type-check-debt under the verify lock (re-measure OK, 0 raw errors), check:dual-build-cjs-loads (112 entry points across 69 packages load), ui/scripts (Test Files 169 passed, 2 skipped; Tests 4901 passed, 2 skipped). --ran reconciliation with exit codes recorded: 67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN. The derivation warned that the tree is 53 commits behind origin/main and four family-defining files changed across that range, so its list is the branch tree's own; repo-wide scans (pnpm lint, the full check:* battery) are left to CI.


Generated by Claude Code

@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 29, 2026
@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 11 documentable anchor(s).

3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/kernel/contracts/auth-service.mdx (via AuthManager (symbol, a top-level class))
  • content/docs/kernel/services-checklist.mdx (via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx (via AuthManager (symbol, a top-level class))

⛔ 2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx (via must_change_password (literal, a string literal in isForcedRotationPending))
  • content/docs/releases/v18/18-2.mdx (via AuthManager (symbol, a top-level class))

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.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: sys_user (literal, 48 pages)
  • the SDK route bridge reached 58 of 210 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json a552e8699255db754446256aabac02c81ebd73a8 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 0daffe1d5cc46cfea449e1684fc7111be1f91259 — the merge of head 44d99fe72416bcac825e75e860e6453deb0a2eff into base a552e8699255db754446256aabac02c81ebd73a8, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 0daffe1d5cc46cfea449e1684fc7111be1f91259 && git checkout 0daffe1d5cc46cfea449e1684fc7111be1f91259
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin a552e8699255db754446256aabac02c81ebd73a8 44d99fe72416bcac825e75e860e6453deb0a2eff && git checkout -B drift-repro a552e8699255db754446256aabac02c81ebd73a8 && git merge --no-ff 44d99fe72416bcac825e75e860e6453deb0a2eff

node scripts/docs-audit/affected-docs.mjs --json a552e8699255db754446256aabac02c81ebd73a8

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs a552e8699255db754446256aabac02c81ebd73a8 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 2a467e5fc0fbb35db9152cb49d3469cd2b39d8e5
Local-runs: none
Scope: packages/plugins/plugin-auth/src/auth-manager-before-hook.ts, packages/plugins/plugin-auth/src/auth-manager-password-session-policy.ts, packages/plugins/plugin-auth/src/auth-manager.ts, packages/plugins/plugin-auth/src/auth-manager.test.ts, packages/plugins/plugin-auth/src/forced-password-change-new-password.test.ts, ui/packages/app-shell/src/console/RemediationOverlay.tsx, ui/packages/app-shell/src/console/tests/RemediationOverlay.forcedResetNewPassword.test.tsx, ui/packages/i18n/src/locales/{ar,de,en,es,fr,ja,ko,pt,ru,zh}.ts + packages/plugins/plugin-auth/src/auth-manager-after-hook.ts (the change / reset stamp and the sign-up stamp), packages/plugins/plugin-auth/src/auth-manager-password-session-policy.ts (stampPasswordChangedAt, resolvePasswordChangeUserId, recordPasswordHistory, the remediation gate that raises PASSWORD_RESET_REQUIRED), packages/plugins/plugin-auth/src/admin-user-endpoints.ts and admin-import-users.ts (the flag's writers), packages/plugins/plugin-auth/src/internal-field-readback.ts (recoverInternalFieldsForSystemRead), packages/plugins/plugin-auth/package.json (exports), packages/platform-objects/src/identity/sys-user.object.ts (must_change_password), packages/spec/src/api/error-code-ledger.zod.ts (PASSWORD_REUSE, PASSWORD_RESET_REQUIRED), ui/packages/auth/src/createAuthClient.ts (the Console's change-password door), packages/objectql/CHANGELOG.md (the #22646 entry), scripts/check-changeset-no-major.mjs (the declaration prompt), review of record 6495005521 on #27477.

Face: narrowing. The diff touches no packages/spec/src/** non-test path and no governed path; the PR body declares Clause-②: no and the maintainer dispatched this review on the disclosure ruling. On this face the scope floor is every path of the diff outside the prose faces; the changeset is the lane seat's at ACCEPT and is read here only for ②.

Security-hardening ruling: this record names positions and refusals; it carries no reproduction.

① Derived judgments

The accept set and the served surface the diff changes, read off the code on this head:

  1. Forced-change refusal, both completion doors. While sys_user.must_change_password is set, /change-password and /reset-password refuse a newPassword that verifies against the CURRENT credential hash with 400 PASSWORD_REUSE and the forced-rotation message, whatever passwordHistoryCount is (the default 0 included). Coverage of the doors is complete: the only user-facing clearer of the flag is stampPasswordChangedAt, run by the after-hook on a successful /change-password or /reset-password (and on /sign-up/email for a user that did not exist, which is not a completion), and both doors pass the before-hook's assertPasswordNotReused first on the same candidate. The flag's writers are the administrator's doors (create-user, set-user-password, import-users) under system context. Correct.
  2. Order of refusals on /change-password. The hook runs before better-auth's own verification of currentPassword, so the diff verifies the request's claim against the current hash first and judges the candidate only when the claim holds; an unconfirmed claim returns undefined and the endpoint answers its own INVALID_PASSWORD. Read off the code and the dev's ablation (claimHolds = true turned the wrong-claim case into PASSWORD_REUSE), consistent with better-auth's hook architecture. Correct, and it is wider than the body says: the gate sits ABOVE the history ring too, so with passwordHistoryCount greater than 0 a request with a wrong current password and a recycled new one, which answered PASSWORD_REUSE on main, now answers INVALID_PASSWORD. That closes a confirmation oracle over the history ring for a caller who holds a session but not the password, and it is a changed answer on an input that was already refused, not a narrowing. The changeset states it; the PR body does not (must-fix 2).
  3. /reset-password. currentPassword is undefined, the claim holds by construction, and the token (resolved by resolvePasswordChangeUserId through findVerificationValue) is the authority. The forced refusal applies directly; a different password completes and the after-hook clears the flag. Correct.
  4. Return value and the ring. With the flag set and depth 0 the function returns undefined after the forced check, so no history stash is made and the after-hook only stamps; with depth greater than 0 it falls through to the ring and returns currentHash exactly as on main. The after-hook's success test (isAPIError) is unchanged, so a refused change stamps nothing and the flag stays set, pinned at the door. Correct.
  5. Cost. One sys_user read per /change-password and /reset-password always, even with depth 0 and no flag (pinned by the amended unit case: findOne sees sys_user and never sys_account); on /change-password the current password is now verified twice (the hook, then the endpoint), one extra scrypt per change. Acceptable for a credential door; noted.
  6. Fail-open. isForcedRotationPending answers false on a lookup error, so a sys_user read failure lets the forced change complete with the same password, silently: no log line marks that the control did not run. The file's other reads fail open too, but the stamp beside them (stampPasswordChangedAt) logs a durability degradation when its write fails. Should-fix 1.
  7. must_change_password read. The column is readonly: true, group: 'Admin', not internal, so the system-context projection carries it; === 1 is read for SQLite drivers. Correct. The credential columns are recovered through recoverInternalFieldsForSystemRead, unchanged.
  8. Public surface. assertPasswordNotReused is a private method on AuthManager; FORCED_ROTATION_REUSE_MESSAGE is exported from auth-manager-password-session-policy.ts, which package.json exports (., ./client-address, ./rate-limit-storage) does not address and the entry does not re-export, so it is shipped bytes and not a published accept set. PASSWORD_REUSE and PASSWORD_RESET_REQUIRED are already ledger entries; the forced case is a second message under the existing code, whose condition is reuse of the current password, so the ledger does not move. @object-ui/i18n gains auth.remediation.password.sameAsTemporary in all ten packs (the key-parity test holds). Additive. Correct.
  9. Console. ChangePasswordForm refuses locally only when forcedReset (gate PASSWORD_RESET_REQUIRED) and the typed new password equals the typed current one, before any request; PASSWORD_EXPIRED sends the same password on and the server judges it. The Console's changePassword is better-auth's /change-password under the bound client, so the server refusal in item 1 stays the authority for every client. Correct.
  10. Unchanged. With no forced change outstanding, reuse is judged by passwordHistoryCount exactly as before, depth 0 accepting the same password (the control case). The administrator's set-user-password with the requirement switched off clears the flag by its own write and is not a user completion door. Correct.

② Semver level

Declared: Clause-②: no, Metadata-protocol: none; @objectstack/plugin-auth, @object-ui/app-shell, @object-ui/i18n at patch.

The arm is missing. A request every holder of a forced-change flag could send on main and have accepted, POST /api/v1/auth/change-password or /reset-password with the new password equal to the current one on a deployment at the default history depth, is now refused 400. That is an accept-set narrowing of a served runtime surface, the class check-changeset-no-major.mjs describes at its own prompt ("add (narrowing) if it narrows one, a BREAKING change, whatever its level") and the class the review of record 6495005521 on #27477 FAILed a bare no for today, with the dev adopting Clause-②: no (narrowing) on the next head (5eb62f096) and the PASS of record following; #22646 (packages/objectql/CHANGELOG.md, 18.0.0) is the template entry. The changeset's feat prefix and the body's own sentence ("the change now has to move the user to a new password") describe a tightening, and a client that automates a forced change with the issued password (an onboarding script, a test harness) breaks at this door. Measurable now: node scripts/pm/check-owed-reviews.mjs lists neither this PR nor #27431 among the PRs owing a contract review, because the face reader sees no arm and no path hit; the review the maintainer ordered is invisible to the sweep until the declaration says what the diff does.

What is right and stays: Metadata-protocol: none (no authorable key, value or type changes) and the patch level on all three packages (ruling A: a non-protocol narrowing ships patch and keeps its BREAKING carriers). The changed answer in ① item 2 is a refusal-code change on an already-refused input; it belongs in the FROM / TO, not in the level. The arm owes the PR body a Skills-docs: line: no skills/** file teaches the forced password change, passwordHistoryCount or the change-password door, so none — REASON is the admissible value, read at ACCEPT.

③ Boundary flags

  • (a) "The flag read fails open (treated as not set) on a lookup error, as every read in assertPasswordNotReused already does." Accepted as the file's posture; the silence is the finding (should-fix 1), not the direction.
  • (b) "The Console check compares the two typed strings; it never replaces the server's refusal." Accepted; ① item 9.
  • (c) Body drift. The PR body on this head is the pre-2663005b6 account: "What changes" bullet 2 describes the claim check as the forced branch's alone, while the head gates the history ring too and changes the answer for a wrong-current, recycled-new request from PASSWORD_REUSE to INVALID_PASSWORD; the Tests section omits the three history-on cases the test file now carries; and "Verification" names its readings on f1c03b281, which two source-changing commits follow, so the body's evidence names a head that is not this head. The changeset (2a467e5fc) states the reorder correctly. Must-fix 2.
  • (d) open_questions: the body lists none (「需要你做的:无」). Author is objectstack-fleet[bot]; no login note owed.

④ Design judgment

On the narrowing face, against the published spec on main: the change takes no declared capability away at the consumer and makes the spec's own requirement real. sys_user.must_change_password is declared as "the user is blocked (403 PASSWORD_RESET_REQUIRED) until they change their password"; on main at the default history depth the change could be a no-op, so the flag cleared while the user kept the password the administrator knows, which defeats the only reason the flag exists. Judging reuse against the CURRENT hash under the flag, independent of passwordHistoryCount, is the narrowest rule that restores the declaration; keeping the code PASSWORD_REUSE keeps the ledger whole; reading both completion doors through the one hook that already owns reuse keeps a single rule. Placing every reuse refusal after the current-password claim is the right order for a credential door: a refusal that answers before the claim is confirmed is an oracle over the ring for a session holder without the password, and /reset-password is rightly exempt because its token is the claim. Judged per hits file:

  • auth-manager-password-session-policy.ts: the forced read, the claim gate and the refusal are placed so that no path reaches a reuse answer before the claim holds, the depth-0 early return is preserved after the forced check, and the ring's return value is unchanged; holds.
  • auth-manager-before-hook.ts: the claim is read only on /change-password and passed as '' when absent, so an absent claim is refused by the endpoint's own schema rather than judged; holds.
  • auth-manager.ts: the private method's docblock states the contract the module implements; holds.
  • forced-password-change-new-password.test.ts: end to end through handleRequest over a real engine with the plugin's own manifest, so the internal: true strip and the readback are live; pins refusal, completion, the wrong-claim answer, both doors, the control and the three history-on orderings; holds. auth-manager.test.ts: the depth-0 case now pins the one sys_user read; holds.
  • RemediationOverlay.tsx and its test: the local check is gated on the forced code only, and the test pins en and zh text, no request on a match, a different password sent, and the expiry screen unchanged; holds.
  • The ten locale files: one key each; holds.

Ablation evidence in the body (forced refusal off, claim skipped, Console check off; each restored to the HEAD blob) is consistent with the pins read here and was taken on f1c03b281; the head's own evidence is CI's green Test Core and Test UI on 2a467e5fc. Not re-run.

Must-fix

  1. Declaration: in the changeset body and the PR body, Clause-②: no (narrowing); keep Metadata-protocol: none and the three patch grades; add the **BREAKING** (an accept-set narrowing) banner with a FROM / TO paragraph (FROM: with the flag set and the default history depth, a change or reset to the same password completed and cleared the flag; a wrong current password with a recycled new one answered PASSWORD_REUSE. TO: refused 400 PASSWORD_REUSE until a different password is chosen; a wrong current password answers INVALID_PASSWORD whatever the new one is); exactly one ADR-0087 HTML-comment marker in the changeset body, category not-required (no-migration-prescription), stating that no metadata key, spelling, export or stored shape moves and that what narrows is a credential door's accept set; and a Skills-docs: line in the PR body (② names the admissible value). Then re-run pnpm check:adr-0087-registration and check-changeset-no-major locally.
  2. Body drift, in the same edit: "What changes" states that the claim gate sits above every reuse refusal, the forced one and the history ring's, and names the changed answer (INVALID_PASSWORD for a wrong current password whatever the new one is); the Tests section names the three history-on cases; "Verification" names the head it was read on, and states which readings are from f1c03b281 and which (CI) are on 2a467e5fc, or is re-read on the head.

Should-fix

  1. When the sys_user flag lookup in isForcedRotationPending throws, log through the host's durability-degradation channel (as stampPasswordChangedAt does for its write) that the forced-change control did not run for this request, so a read outage of a security control is visible in the log rather than silent (① item 6).
  2. auth-manager.ts docblock and the before-hook comment: say in one sentence that the gate is now above the history ring as well, so the next reader of assertPasswordNotReused does not take the forced branch for the only consumer of the claim.

Implemented-by: claude/harden-forced-password-change
Reviewed-by: session_017xRBcVy4VXmkQjn2MGeaWk

VERDICT: FAIL

The refusal covers both completion doors, the claim-first order closes an oracle, the ledger and the published surface do not move, and the Console keeps the server as the authority; the FAIL is the declaration and the body that no longer describes the head (must-fixes 1 and 2). A head that carries the corrected declaration and body and nothing else owes a delta review by this reviewer, not a full round.

…he changeset declares the narrowing

The sys_user must_change_password read in isForcedRotationPending still
fails open, but now reports through logDurabilityDegradation, the channel
stampPasswordChangedAt reports its write through. The docblock and the
before-hook comment say the current-password claim gate sits above the
history ring as well as the forced refusal. The changeset declares
Clause-② no (narrowing) with the BREAKING banner, FROM / TO and its
ADR-0087 disposition.

Claude-Session: https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l and removed size/m labels Oct 29, 2026
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk
os-dev-report

{
  "issue": null,
  "status": "done",
  "branch": "claude/harden-forced-password-change",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/27440",
  "session": "session_017xRBcVy4VXmkQjn2MGeaWk (subagent of the PM session; parent's id)",
  "premise_still_valid": true,
  "summary": "Rework of #27440 against contract review 6501172187, one commit 44d99fe72 on top of 2a467e5fc, pushed to the same branch with no force and no merge of origin/main (GitHub reports it mergeable, merge base c5d463417). Must-fix 1: the changeset now declares `Clause-②: no (narrowing)` and keeps `Metadata-protocol: none` and the three `patch` grades. It carries the `**BREAKING** (an accept-set narrowing)` banner with FROM / TO, which names the changed answer: a wrong current password with a recycled new one at history depth > 0 now gets INVALID_PASSWORD, not PASSWORD_REUSE. It has exactly one ADR-0087 marker, `not-required (no-migration-prescription)`. Must-fix 2 and the PR-body half of must-fix 1 are in the staged replacement body /tmp/claude-0/-home-user-objectstack/bdda939c-7424-5594-af38-0e6fe5d7eeed/scratchpad/pr-27440/pr-body.md: the three declaration lines including `Skills-docs: none — REASON` (git grep over skills/ for seven forced-change terms found 0 hits each; control defineStack found 22 files), What changes, the three history-on test cases, and Verification read on 44d99fe72. The PR body was NOT patched; the seat writes it. Should-fix 1: `isForcedRotationPending` still fails open and now reports through `logDurabilityDegradation`, the channel `stampPasswordChangedAt` uses. A new unit case pins this. Should-fix 2: the `assertPasswordNotReused` docblock in auth-manager.ts and the before-hook comment now say the claim gate sits above the history ring as well. PR stays draft. Deviations: the body keeps the create-time session-URL footer, with this run's session named in Verification prose. The failed-flag-read log fires once per affected request, the same as the stamp beside it, not once per boot; recorded in Acceptance notes.",
  "tests": "All on 44d99fe72. `pnpm --filter @objectstack/plugin-auth test`: Test Files 203 passed (203), Tests 3852 passed | 10 skipped (3862). Targeted `pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2 src/auth-manager.test.ts src/forced-password-change-new-password.test.ts`: 2 files, 313/313. `pnpm --filter @objectstack/plugin-auth build && ... typecheck`: exit 0, check:test-typecheck OK (10 files / 88 errors / 23 pins held). The first typecheck attempt exited 2 only because the examples program reads the package's own unbuilt dist. From ui/: `pnpm exec vitest run --maxWorkers=2 packages/app-shell/src/console/__tests__/RemediationOverlay`: Test Files 3 passed (3), Tests 12 passed (12). Ablation of the new log call via scripts/ablation-replace.mjs: anchor `host.logDurabilityDegradation(` went 1 to 0, blob 93a97d4a2ae2 to 2127a2c857e4. auth-manager.test.ts went 1 failed | 304 passed; the red case was the new failed-flag-read case. Restored blob equals HEAD blob 93a97d4a2ae2 and `git diff HEAD` is empty. The suite imports src directly, so no dist leg was needed.",
  "gates": "Local, exit codes captured before any pipe. `pnpm check:adr-0087-registration` exit 0: 1 declared-breaking changeset, [BREAKING+clause-②-narrowing] not-required (no-migration-prescription). `node scripts/check-changeset-no-major.mjs` exit 0 (no major; level axis NOT APPLICABLE locally). With `--base origin/main --event` and a pull_request payload carrying the staged body (GITHUB_EVENT_NAME=pull_request): exit 0, 'LEVEL AXIS: this PR declares clause-② `no (narrowing)` and metadata-protocol `none` ... a non-protocol change at `patch`'. `check:nul-bytes` exit 0. `dispatch-gates.mjs --commands` derived 67 families, all run. 64 read exit 0 on the first pass. Three first hit build-state prerequisites: check:dual-build-cjs-loads exit 3, check:type-check-debt exit 3 (lock queue), and `pnpm --dir ui exec vitest run scripts/` exit 1 on an unbuilt @objectstack/client entry. All three exit 0 after the full package build: debt re-measure under the lock OK; dual-build 112 entry points across 69 packages load; ui/scripts 169 files passed, 2 skipped. `--ran` with exit codes recorded: 67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN (derived zero). Caveat: the derivation warned the tree is 53 commits behind origin/main and 4 family-defining files changed, so the list is the branch tree's own. CI on 44d99fe72: all 63 check-runs completed, 55 success and 8 skipped (opt-in or path-scheduled), 0 failures. The successes include Lint & Repo Gates, TypeScript Type Check, Test Core (8/8 shards), Build Core, Governed Surface Queue Guard, Lint UI, Type Check UI, Test UI (16/16), Build & E2E UI, Live E2E UI, Check Changeset and Dogfood Regression Gate. Check Changeset ran against the current, unpatched body; it re-reads on the seat's body edit.",
  "dogfood_battery": "44d99fe72 · closure built (`pnpm --filter '@objectstack/dogfood^...' build`, BUILD lock VERDICT command-exit 0) · `pnpm --filter @objectstack/dogfood test` under the lock: VERDICT command-exit 0 · Test Files 348 passed | 3 skipped (351) · Tests 2806 passed | 20 skipped (2826) · no failures",
  "mcp_calls": "0",
  "api_writes": "1 — POST issues/27440/comments (this os-dev-report, via scripts/pm/post-stamped.mjs); plus 1 git push (not REST). No PR body PATCH, no label write, no ready flip.",
  "open_questions": [],
  "out_of_scope_findings": []
}

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 44d99fe72416bcac825e75e860e6453deb0a2eff
Local-runs: none
Scope: packages/plugins/plugin-auth/src/auth-manager-before-hook.ts, packages/plugins/plugin-auth/src/auth-manager-password-session-policy.ts, packages/plugins/plugin-auth/src/auth-manager.ts, packages/plugins/plugin-auth/src/auth-manager.test.ts, packages/plugins/plugin-auth/src/forced-password-change-new-password.test.ts, ui/packages/app-shell/src/console/RemediationOverlay.tsx, ui/packages/app-shell/src/console/tests/RemediationOverlay.forcedResetNewPassword.test.tsx, ui/packages/i18n/src/locales/{ar,de,en,es,fr,ja,ko,pt,ru,zh}.ts + packages/plugins/plugin-auth/src/auth-manager.ts (logDurabilityDegradation, the host picks), the PasswordSessionPolicyHost type and stampPasswordChangedAt in the policy module, the cited code named in record 6501172187 (unchanged on this head), .changeset/forced-password-change-new-password.md (read for ②), the PR body as rewritten 2026-10-29T17:34Z, os-dev report 6503847427, scripts/check-changeset-no-major.mjs and scripts/check-adr-0087-registration.mjs (the readers of the declaration), packages/objectql/CHANGELOG.md (the #22646 entry).

Face: narrowing, now declared. Delta review of record 6501172187 (FAIL on the declaration and the body, head 2a467e5fc). The net delta is one commit, 44d99fe72, on that head, no merge of origin/main (merge base unchanged, c5d463417); git diff 2a467e5fc..44d99fe72 touches the changeset, two comments, the forced-flag read's error path and its host wiring, and one unit case. Every ① and ④ judgment of the earlier record on the unchanged lines stands by reference; this record judges the delta and the body.

Security-hardening ruling: this record names positions and refusals; it carries no reproduction.

① Derived judgments

  1. No accept-set change in the delta. The forced refusal, the claim gate, the history ring, the /reset-password path, the return value and the Console check are byte-identical to 2a467e5fc; the three test files that pin them are unchanged except for the one new unit case. Items 1–5 and 7–10 of 6501172187 ① hold on this head.
  2. The flag read now reports its failure (should-fix 1). isForcedRotationPending takes the host's logDurabilityDegradation and, on a thrown sys_user lookup, calls it with a message naming the control that did not run, the error, and { object: 'sys_user', fields: ['must_change_password'] }, then returns false as before. logDurabilityDegradation is config.logger?.warn(message, { ...meta, error }), the same channel and level stampPasswordChangedAt reports its failed write through, so the two degradations beside each other read alike in a log. The direction (fail open) is unchanged and was accepted; the silence is gone. The host pick in auth-manager.ts adds logDurabilityDegradation, and PasswordSessionPolicyHost already declares it, so no type widens. The new unit case pins both halves: depth 0 still accepts when the read throws, and logger.warn is called exactly once with the object, the field and the error text. The dev's ablation of the log call (that case alone red, blob restored to HEAD) is consistent with the pin; not re-run. Correct.
  3. Comments (should-fix 2). The assertPasswordNotReused docblock in auth-manager.ts and the before-hook comment now state that the claim gate sits above both reuse refusals, the forced change's and the history ring's, and that a wrong current password answers INVALID_PASSWORD whatever the new password is; the docblock also names the fail-open-with-report behaviour. Both match the code read in the earlier record. Correct.
  4. Cost. Unchanged: one sys_user flag read per change or reset and one extra verification of the claim on /change-password; the log call runs only on the error path.

② Semver level

Declared on the changeset and the PR body, both line-initial: Clause-②: no (narrowing), Metadata-protocol: none; @objectstack/plugin-auth, @object-ui/app-shell, @object-ui/i18n at patch; PR body Skills-docs: none — REASON with a measured reading (seven forced-change terms, zero hits each under skills/**, a defineStack control that hits; the hand-written authentication guide's sentence stays true). The changeset carries the **BREAKING** (an accept-set narrowing) banner; a FROM / TO that names the narrowed request (same password back while the flag is set, at the default depth, on either door), the changed answer (wrong current password with a recycled new one at a depth above 0: INVALID_PASSWORD, which answered PASSWORD_REUSE before), the /reset-password exemption, a "For clients" sentence for the automation that breaks, and the fail-open-with-report behaviour; and exactly one ADR-0087 HTML-comment marker in not-required (no-migration-prescription) whose text states that no key, spelling, export or stored shape moves and that what narrows is a credential door's accept set. This is must-fix 1 of the earlier record, in the #22646 shape; the level stays patch under ruling A and Metadata-protocol: none stays right. Both readers agree on the head: Lint & Repo Gates (which carries check:adr-0087-registration) concluded success and Check Changeset concluded success on 44d99fe72; the dev's local readings (adr-0087-registration exit 0 naming this changeset as [BREAKING+clause-②-narrowing], the no-major gate's level axis reading no (narrowing) / none at patch from the staged body) are consistent with those conclusions.

③ Boundary flags

  • (a) Fail-open on the flag read: accepted direction, now reported once per affected request at the durability channel's level (warn), the one level AuthManagerOptions.logger guarantees; the body's acceptance note says exactly that. Closed.
  • (b) The Console check compares the typed strings and never replaces the server's refusal: unchanged, accepted.
  • (c) Body (must-fix 2). The body on this head describes the tree: the three declaration lines are the first lines after the signature line; the 维护者速读 states the narrowing, the ordering change for the history ring and the changed answer in its own words, and names the two-minute check at 8/8; "What changes" states that the claim gate sits above both reuse refusals, carries the FROM / TO and the changed answer, and names the fail-open report; Tests names the three history-on cases and the new failed-flag-read case; Verification names 44d99fe72 as the head of every reading and says which earlier ablations (forced refusal, claim gate, Console check) stand on unchanged lines from f1c03b281. No drift.
  • (d) Checks on 44d99fe72 at review time: every concluded non-skipped run is success, the ten required contexts among them (Lint & Repo Gates, TypeScript Type Check, Test Core, Build Core, Governed Surface Queue Guard, Lint UI, Type Check UI, Test UI, Build & E2E UI, Live E2E UI); second runs of Check Changeset and the PR-body guards, the re-reads triggered by the body edit, were in_progress. The landing pre-check (全绿) is the seat's read at landing, not this record's.
  • (e) The (narrowing) checklist line's battery: the dev's full local dogfood run on 44d99fe72 under the verify lock (348 files passed / 3 skipped, 2806 tests passed / 20 skipped, VERDICT exit 0) and CI's Dogfood Regression Gate success on the head; read, not re-run.
  • (f) open_questions: none (the report's array is empty); out_of_scope_findings: none. Author is objectstack-fleet[bot]; no login note owed.

④ Design judgment

Unchanged from 6501172187 on the narrowing itself: the change makes the must_change_password declaration real at both completion doors through the one hook that already owns reuse, keeps the ledger whole, and orders every reuse refusal after the current-password claim, closing a history-ring oracle for a session holder without the password. On the delta: a security control that fails open now says so where the stamp beside it already does, at the same level and with the same shape, so an operator reading the log sees the two degradations as one family; the single new unit case is the right layer for it (the mocked engine throws on sys_user alone, and the assertion names the exact meta the channel emits). Nothing in the delta can regress a package consumer. No must-fix; no should-fix outstanding.

Implemented-by: claude/harden-forced-password-change
Reviewed-by: session_017xRBcVy4VXmkQjn2MGeaWk

VERDICT: PASS

Delta review complete: the declaration is corrected in the #22646 shape, the body describes the head, the fail-open read is reported through the existing durability channel and pinned, and the two comments state the gate's real extent. The seat reads the second Check Changeset run's conclusion before landing.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk

ACCEPT → head 44d99fe72416bcac825e75e860e6453deb0a2eff · 2026-10-29T17:37Z

维护者速读

  • 改了什么:用户被管理员要求“下次登录必须改密码”时,完成这次强制改密,新密码必须和当前密码不同。/change-password 和 /reset-password 两个入口都会拒绝相同的密码,返回 400 PASSWORD_REUSE。另外,/change-password 上所有“密码重复”的拒绝,都要先验证当前密码通过后才会给出。
  • 为什么改:QA 第三轮发现的安全加固(细节按披露裁决不公开)。
  • 风险与回滚:这是 BREAKING 收窄,按 patch 发布。用临时密码原样“改密”的自动化脚本会被拒。passwordHistoryCount 大于 0 时,如果当前密码错误、新密码又命中历史记录,返回从 PASSWORD_REUSE 变为 INVALID_PASSWORD。回滚就是 revert 本 PR。
  • 先看:packages/plugins/plugin-auth/src/auth-manager-password-session-policy.ts、.changeset/forced-password-change-new-password.md。
  • 两分钟验证:pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2 src/forced-password-change-new-password.test.ts,预期 8 个用例全过。
  • 需要你做的:无。

席位意见:收。增量契约复核在本 head 上 PASS(6503923468,判的是 6501172187 之后的增量);dogfood 全套 348 个文件、0 失败。

席位核对

  1. 形态:draft、目标 main;按披露裁决不建卡,正文无关闭关键词。正文已由席位按 dev 暂存稿写入,回读一致。
  2. 前提实测:契约复核 6501172187 和 6503923468 都读过代码。ablation 结果和新增用例一致。
  3. 门禁:本 head 共 57 个 success、10 个 skipped、0 失败。Lint & Repo Gates、TypeScript Type Check 都是 success。Check Changeset 两次运行(含改正文后的重读)都是 success。
  4. 接受的偏差:读取强制改密标记失败时仍然放行,但现在会通过降级日志通道报告。复核已接受这个方向。
  5. 范围外去向:无。Skills-docs: none — REASON 有实测的 grep 结果支撑。
  6. 落地:不属于受管面,527 行,低于 3000 行上限 → pr_ready + automerge_enable。

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 29, 2026 17:38
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 29, 2026 17:38
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 29, 2026
Merged via the queue into main with commit 6bc7643 Oct 29, 2026
68 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/harden-forced-password-change branch October 29, 2026 18: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.

1 participant