Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions docs/BACKLOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ Ordered by value descending, then difficulty ascending (cheapest first at equal
| 48 | **#1020** | The first-run bootstrap Administrator is created with no email address, and the PHI notification gate cannot see it | 5 | 3 | _fill-in_ | P2 | `_ensure_bootstrap_admin` calls `create_user` with no `email=` (`auth/service.py:527-533`), so the account holding `frozenset(Permission)` has a NULL email and `SecurityEventNotifier.notify` returns before enqueueing (`pipeline/security_notify.py:130`), making all ten notice types no-op for it — including LOGIN_AFTER_FAILURES, the compromise signal. The part with teeth is gate blindness: the PHI startup gate refuses to serve without a notification channel but computes readiness from `notify_security_events` plus `email_smtp_host` plus `email_from` alone (`__main__.py:2260`), so it would report a healthy channel while no notice about the all-permission account could be delivered. The allocated title's second half is REFUTED and the body says so: lockout is time-bounded (15 minutes by default, `settings.py:1769`), an admin reset clears it (`store/store.py:7693`), the documented break-glass is a sealed file (`api/app.py:5104`), and no email-driven recovery flow exists anywhere in the code. Value 5 because every event is still an audit row surfaced by `GET /me/security-events`. Difficulty 3 with no schema change; the only judgment is which of three candidate fixes the owner wants. |
| 49 | **#1019** | install-selfheal.ps1 has no installed-vs-source payload-parity instrument, and it wires the most privileged hook in the estate | 5 | 3 | _fill-in_ | P3 | The installer copies `worktree-selfheal.ps1` to `~/.claude-hooks/` and wires it as a user-scope SessionStart hook with no divergence detection: no `-Status` (the surface is `-ConfigDir` plus `-HookPath`), no version stamp, a bare `Copy-Item -Force` (`:57`), and no test that reads the installed copy — every selfheal test binds `ROOT` under a synthetic home, and the repo's only installed-vs-source parity test names the gate copy alone (`tests/test_gate_installed_parity.py:44`). The title's "no parity instrument at all" is narrowed in the body: source-level guard parity DOES exist (`test_both_installers_carry_the_same_refusal`), as do the CLAUDECODE refusal, the backup-validate-rollback, and an unconditional payload refresh — what is absent is payload parity. The claimed comparator is also wrong, and PR #191 has since sharpened this: it landed payload parity on `install-git-hooks.ps1` (SHA256, IN SYNC/STALE, plus a pytest-side assertion), so `install-selfheal.ps1` is now the ONLY installer in the estate without one, with two worked examples to copy. Privilege is substantiated: user scope in every config dir, and a hook that runs `git checkout` on the shared primary unattended (`worktree-selfheal.ps1:106`). Measured 2026-08-04, installed and source agree (`c41c70ecf885`), so detection is absent rather than divergence present. Difficulty 3 with four constraints, chiefly folding CRLF exactly as the existing instruments do. |
| 50 | **#236** | Test-this-step and test-up-to-step with pinned upstream values | 5 | 4 | _fill-in_ | P2 | Real debug breadth — whole-handler traced values already fold onto rows (`mergeLiveValues`, ide/src/stepsModel.ts:544) so partial runs are a convenience, but pinning an expensive `db_lookup`/`fhir_lookup` has no equivalent at all; largely a stop condition plus state dump on ADR 0072's shipped trace, with the lookup mock and keeping `buildLensTraceArgs` (:674) incapable of emitting `--show-phi` the real work. |
| 51 | **#1022** | disable_mfa has no last-factor guard where delete_webauthn_credential does, so the two removal paths can be ordered to reach zero factors | 5 | 4 | _fill-in_ | P2 | `delete_webauthn_credential` computes `last_second_factor` and refuses when MFA is required (`auth/service.py:2426-2432`); `disable_mfa` does nothing between `get_user` and `disable_totp` (`:2086-2087`), so a user holding TOTP plus one passkey can delete the passkey (permitted while `totp_enabled` is True) and then disable TOTP, arriving at zero enrolled factors — the state ADR 0068 AC-10 says the system shall refuse and `docs/SECURITY.md:752` asserts unconditionally. The consequence is overstated in the obvious reading and the body corrects it: login enforcement is NOT missing, since `mfa_verified=not mfa_required` (`:718`) plus the ASVS 6.3.3 access gate in `require()` (`api/security.py:224-234`) make the outcome a forced re-enrollment rather than single-factor access. Value 5: genuine, demonstrable, defeats a numbered acceptance criterion by ordering and makes a shipped doc guarantee untrue, but no bypass and no PHI consequence. Difficulty 4 because the raise needs mapping at two call sites that would otherwise 500, one existing test breaks by construction (`tests/test_mfa.py:189`), and two docs move with it. Filed because ADR 0068 line 140 promised this parity follow-up and no item carries it. |
| 51 | **#1022** | disable_mfa has no last-factor guard where delete_webauthn_credential does, so the two removal paths can be ordered to reach zero factors | 5 | 4 | _fill-in_ | P2 | `delete_webauthn_credential` computes `last_second_factor` and refuses when MFA is required (`auth/service.py:2426-2432`); `disable_mfa` does nothing between `get_user` and `disable_totp` (`:2086-2087`), so a user holding TOTP plus one passkey can delete the passkey (permitted while `totp_enabled` is True) and then disable TOTP, arriving at zero enrolled factors — the state ADR 0068 AC-10 says the system shall refuse. (Corrected: the false doc claim was the `DELETE /me/mfa` **route-table row** at `docs/SECURITY.md:329`, since fixed to state the absence; `:752` is passkey-scoped and defensible, so the remaining doc obligation is ADR 0068 line 140.) The consequence is overstated in the obvious reading and the body corrects it: login enforcement is NOT missing, since `mfa_verified=not mfa_required` (`:718`) plus the ASVS 6.3.3 access gate in `require()` (`api/security.py:224-234`) make the outcome a forced re-enrollment rather than single-factor access. Value 5: genuine, demonstrable, defeats a numbered acceptance criterion by ordering and makes a shipped doc guarantee untrue, but no bypass and no PHI consequence. Difficulty 4 because the raise needs mapping at two call sites that would otherwise 500, one existing test breaks by construction (`tests/test_mfa.py:189`), and two docs move with it. Filed because ADR 0068 line 140 promised this parity follow-up and no item carries it. |
| 52 | **#165** | DB schema browser + ad-hoc query runner | 5 | 5 | _fill-in_ | DEMAND-GATE | Corepoint-parity authoring aid whose external-SQL-client workaround is fully clean — the only DB reach today is the `SELECT 1` reachability probe (`messagefoundry/transports/database.py:484-501`) and dry-run refuses `db_lookup` (`messagefoundry/pipeline/dryrun.py:570`); the build is a net-new API surface plus per-dialect introspection, read-only statement gating, a permission, audit and a console pane. |
| 53 | **#232** | Steps view for routers | 5 | 5 | _fill-in_ | P2 | Real Steps-view breadth gap exactly where destination selection is decided, with a workaround — read a five-line guard-and-return — clean enough to hold it off the top; a `route` row kind widens the ADR 0076 §3 grammar, so an amendment lands first, then `return []` disambiguation in a lens that skips routers outright today (messagefoundry/lens.py:306, :344-347), a router palette, and byte-stable rewrite parity. |
| 54 | **#78** | Custom message-definition data model + conformance validator; NCPDP codec | 5 | 6 | _money pit_ | DEMAND-GATE | Corepoint-parity persisted-definition model plus a report-only validator and an additive NCPDP codec, all cleanly worked around today by a code-first Handler, so useful breadth rather than a blocker; the whole scope is still remainder — NCPDP appears nowhere in `messagefoundry/` and `profile` is merely "reserved for a conformance-profile" (`messagefoundry/parsing/validate.py:56`) — spanning a new stored model the code reads, a validator, and a new codec class. |
Expand Down Expand Up @@ -4841,7 +4841,9 @@ Retiring the tree costs the engine nothing operationally: **`tests/test_ech_egre

⚠️ **Lead with the correction: MFA enforcement at login is NOT missing and must not be touched.** The obvious reading of the title sends a fixer at enforcement that already exists. Disabling TOTP under `require_mfa` would not leave the account reachable with a single factor: `login` resolves `mfa_required` and issues the session with `mfa_verified=not mfa_required` (`auth/service.py:715-718`), and `require()` applies the second factor as an ASVS 6.3.3 **access** gate, 403ing every request whose (method, path) is not one of the six exempt pairs (`api/security.py:189-234`, `:79-88`). So the post-disable state is a **forced re-enrollment** — recoverable, because the enroll routes ride `require_reauth_only*` with `mfa_gate=False` (`api/security.py:586-589`) — not a factor downgrade and not a lockout.

**The defect that survives.** The invariant is defeated by ordering, and it makes a shipped guarantee untrue: `docs/SECURITY.md:752` states the refusal as an unconditional property of the system, which the code does not deliver on this path. `disable_mfa` (`auth/service.py:2083-2099`) does nothing between `get_user` at `:2086` and `disable_totp` at `:2087` — it consults neither `has_webauthn_credentials` nor `_mfa_required_for`. It is the single enforcement point: both `api/auth_routes.py:438` and `messagefoundry_webconsole/routes/account.py:281` reach it unguarded.
**The defect that survives.** The invariant is defeated by ordering. `disable_mfa` (`auth/service.py:2083-2099`) does nothing between `get_user` at `:2086` and `disable_totp` at `:2087` — it consults neither `has_webauthn_credentials` nor `_mfa_required_for`. It is the single enforcement point: both `api/auth_routes.py:438` and `messagefoundry_webconsole/routes/account.py:281` reach it unguarded.

⚠️ **CORRECTED 2026-08-04, and the correction narrows the doc half of this item.** This body originally cited `docs/SECURITY.md:752` as stating the refusal "as an unconditional property of the system". **That citation was wrong.** Line 752 sits inside a WebAuthn paragraph, describes *passkey* removal, and quotes the passkey guard's own error string ("enroll another factor first") — the path where the guard genuinely **does** exist. It is defensible as written. The false claim was one line in a **route table**: `docs/SECURITY.md:329`, the row for `DELETE /me/mfa`, which asserted "refused when it would remove the last factor while MFA is required". That route resolves to `disable_my_mfa` (`api/auth_routes.py:429`) whose own docstring says "turn off the caller's **TOTP** MFA", and there is **no WebAuthn credential DELETE route in `api/` at all** — passkey removal is console-only. `:329` has been corrected to state the absence and point here; the remaining doc obligation for this item is **ADR 0068 line 140**, not a `SECURITY.md` line.

**This is a promised follow-up that was never filed, which is the main reason to file it rather than close it.** ADR 0068 line 140 records "TOTP-disable keeps its existing behavior this lane (parity follow-up recorded)" — and no backlog item carries that follow-up. Searches for `disable_mfa`, "TOTP-disable", "parity follow-up" and "last factor" across `docs/BACKLOG.md` and the closed archive return zero hits. So the asymmetry is a recorded decision, not an oversight.

Expand Down
2 changes: 1 addition & 1 deletion docs/SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ tuple: they act only on the caller's own account.
| `GET` | `/me/mfa` | `require` | |
| `POST` | `/me/mfa/enroll` | `require_reauth_only_action` (action `mfa_enroll`) | password-only step-up — the MFA gate is skipped so a required-but-unenrolled user cannot deadlock |
| `POST` | `/me/mfa/confirm` | `require_reauth_only_action` (action `mfa_confirm`) | per-actor ceremony limiter; password-only step-up |
| `DELETE` | `/me/mfa` | `require_step_up_action` (action `mfa_disable`) | refused when it would remove the last factor while MFA is required |
| `DELETE` | `/me/mfa` | `require_step_up_action` (action `mfa_disable`) | step-up bound to the disable action (current factor + a fresh password). ⚠️ **No last-factor guard** — this is the TOTP path (`disable_mfa`), and it does **not** refuse when it would leave the account with zero enrolled factors. The passkey removal path does refuse; see BACKLOG #1022 for the asymmetry |
| `GET` | `/me/sessions` | `require` | |
| `GET` | `/me/security-events` | `require` | |
| `DELETE` | `/me/sessions/{session_id}` | `require_reauth_only` | password-only step-up |
Expand Down
Loading