Skip to content

test(auth): measure the first-run default account and correct #1136's researched refusal guard - #858

Merged
wshallwshall merged 2 commits into
mainfrom
claude/asvs-1136
Sep 5, 2026
Merged

wshallwshall merged 2 commits into
mainfrom
claude/asvs-1136

Conversation

@wshallwshall

Copy link
Copy Markdown
Collaborator

What this produced, plainly

A precise written finding plus the tests that measure it -- not the redesign. BACKLOG #1136 is a research item whose research was already done on 2026-08-20. What this turn adds is a correction to that research's work list, backed by a measurement, and the build cost that nobody had measured.

The cell stays partial. ASVS 6.3.2's closing act is a scorecard rescore in the vault, which a Builder cannot and must not perform. Nothing here changes engine behaviour, so there is nothing to rescore yet. This makes the eventual rescore possible by settling a design question that would otherwise have been discovered mid-build.

The correction, and it is the point of the PR

The 2026-08-20 outcome provisions the first administrator from an offline CLI command guarded by count_users() == 0, on the stated ground that reusing the existing bootstrap guard widens no authority.

That guard is wrong, and what it fails at is this item's own named stranding risk. It is safe only while nothing else can put the first row in the user table, and a directory sign-in can. Measured through _complete_ad_login (messagefoundry/auth/service.py:1212), the shared tail every directory mechanism ends at:

  • one completed sign-in leaves count_users() == 1 and an empty role list -- _upsert_ad_user (:1409) calls create_user at :1415 and assigns no role anywhere in its body;
  • the shipped initialize() then declines on the same store, because its guard asks the same "is the table empty" question the directory row already answered.

One roleless account, no administrator, reached entirely through shipped code with no redesign involved. Today nothing strands, because initialize() (:539) seeds the bootstrap admin at startup before any login can run -- deleting the auto-create is what opens the window, and the NSSM wrapper starts the service on boot with nobody present.

The refusal guard has to ask whether an enabled administrator exists, not whether the table is empty. That is wider than today's bound and wants a ruling rather than an inheritance: it makes the command a standing recovery path whenever every administrator is lost (#1236's subject). It grants nothing to a network attacker -- same host-access boundary admin-unlock already ships on (messagefoundry/__main__.py:4206, whose docstring argues exactly that).

A second unpriced cost: the researched command "prompts on a TTY, never argv and never a file". An unattended MSI/Ansible/NSSM install has no TTY, so pressure to add --password or --password-file is structural rather than hypothetical -- and that is precisely how a default credential would come back, the shape ADR 0034 accepted a risk for.

Why tests and not the build

The design question is settled; what remains is wide and mechanical. Measured: .initialize() has 197 call sites across 64 files -- in tests/ alone 146, of which 34 bind the returned BootstrapAdmin. Add the store protocol plus three backends, two settings, _emit_bootstrap_admin / _bootstrap_expiry_reminder / the bootstrap_admin_expiring alert and their tests, tests/test_bootstrap_admin_perms.py (which exists only to test the deleted credential file), six documents, and four IDE files. A half-landed version leaves the tree red or ships two first-admin paths, and neither reaches the cell.

What the three tests measure

tests/test_first_run_default_account.py -- characterization, expected to turn red when the redesign lands.

  1. A fresh store gets an enabled account named admin holding Administrator, and it is the only row. Neither arm of the verb holds at creation.
  2. The disabled arm is unexpressible at two altitudes: no disabled parameter on the Store protocol, and all three backends hardcode the column rather than binding it. Both are asserted because a signature check alone would miss an INSERT that started deciding the column for itself. The literal is read positionally off each statement's own column list; a bound column reads back as a placeholder in all three dialects (negative control), so it discriminates rather than passing vacuously.
  3. The directory precondition above.

Severity is conditional per CLAUDE.md section 0 -- zero deployments, so this is what a deploying site would inherit on first run. It is also not a default credential: the password is per-install CSPRNG and must-change.

Two things left open for the reviewer

  • No ADR was filed and no number cited. scripts/coord/alloc.ps1 did not return inside the turn (its floor sweeps every local and remote ref; 67 competing pwsh processes were running). The run was stopped rather than left to burn a number. The decision record is still owed -- section 11's own argument applies, since bootstrap retirement had no ADR, lived only in SECURITY.md, and that is how the guarantee drifted from the code.
  • scripts/dev/sqlserver-docker.ps1:64 defaults the sa password to the literal 'Str0ng_P@ssw0rd!', fed to MSSQL_SA_PASSWORD at :191. Re-measured and deliberately not fixed: the same param block carries an explicit dev-only suppression justification, and whether scripts/ is inside this cell's corpus is the unsettled scope question the 2026-08-20 pass raised.

Ledger hygiene

The docs/BACKLOG.md row is its own last commit, and it edits only #1136's body region. The ranked score table and the intro paragraph are untouched -- three sibling Builders hold #1139, #1146 and #1184, whose table rows sit on adjacent lines and would conflict pairwise, and that table is a dated snapshot whose distribution lines are recomputed with it.

Verified with parse_items from scripts/docs/backlog_status_check.py: 428 items before and after, ADDED and LOST both empty, #1136's open/closed state identical to origin/main. The item stays OPEN.

Checks

Run locally in this worktree's venv:

Check Result
ruff check passed
ruff format --check passed
mypy messagefoundry (strict) passed, 267 source files
pytest on the touched slice 126 passed, 3 skipped (test_first_run_default_account, test_store_capability_matrix, test_auth_service, test_auth_core, test_bootstrap_admin_perms, test_auth_hardening, test_lint_scope_parity)
/simplify 4 agents; applied the duplicate import, _BACKENDS reuse, the _complete_ad_login seam, :memory: stores and prose trims. One agent found the docstring claimed the INSERT literal with nothing asserting it -- now asserted
pre-commit passed on both commits (mypy is not in pre-commit; run by hand above)

Skipped, and CI is the authority: the full suite, Postgres and SQL Server legs (driver-gated, hosted runners only), and every Windows-service leg. This venv installs fewer extras than CI (#1230), so the slice above is not a green suite.

Not labelled reviewed, not enqueued, no auto-merge.

Generated with Claude Code

wshallwshall and others added 2 commits September 4, 2026 17:48
…the guard the redesign needs (BACKLOG #1136)

ASVS 6.3.2 research. Three characterization tests over the first-run path.
They pin the SHIPPED answer rather than assert the desired one, so the
first-run redesign is expected to turn them red.

Two facts the cell turns on, now machine-checked instead of prose:

  - a fresh store gets an ENABLED account named `admin` holding
    Administrator, and it is the only row, so neither arm of the verb
    ("not present, or disabled") holds at creation;
  - the disabled arm is unexpressible at two altitudes -- `create_user`
    carries no `disabled` parameter on the Store protocol, and all three
    backends hardcode the column rather than binding it. Both are
    asserted because the signature alone would not catch an INSERT that
    started deciding the column for itself. The literal detector is
    read positionally off each statement's own column list, and a bound
    column reads back as a placeholder in all three dialects, so it
    discriminates rather than passing vacuously.

The third test measures a precondition, and it corrects the item's own
researched work list. That research provisions the first administrator
from an offline CLI command guarded by `count_users() == 0`, on the
ground that reusing the existing bootstrap guard widens no authority.
The guard is only safe while nothing else can put the first row in the
table, and a directory sign-in can: `_complete_ad_login` creates a user
row and assigns no role. Driven through that seam, the shipped
`initialize()` then declines on the same store, because its guard asks
the same "is the table empty" question the directory row already
answered -- one roleless account, no administrator, reached entirely
through shipped code. So the refusal guard has to ask whether an
ENABLED ADMINISTRATOR exists, not whether the table is empty.

Today this strands nothing: `initialize()` seeds the bootstrap admin at
startup before any login can run. Removing the auto-create is what would
open the window, which is why it belongs to the redesign rather than to
a defect report. Severity conditional per CLAUDE.md section 0 -- zero
deployments, so this is what a deploying site would inherit on first
run, and it is not a default credential.

No engine code changed. The cell stays partial.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d refusal guard

Body-region edit only. The ranked score table and the intro paragraph are
deliberately untouched -- three sibling Builders hold #1139, #1146 and
#1184, whose table rows sit on adjacent lines and would conflict
pairwise, and that table is a dated snapshot whose distribution lines are
recomputed with it.

Records, against the 2026-08-20 research outcome:

  - the researched refusal guard (count_users() == 0) is wrong, and what
    it fails at is this item's own named stranding risk. A directory
    sign-in creates a roleless user row, so the guard is answered by a
    row that grants nobody anything, and the shipped seeding path then
    declines on the same store. It must ask whether an enabled
    administrator exists;
  - a TTY-only provisioning prompt has an unattended-install escape
    hatch, which is where a default credential would come back;
  - both arms re-measured with current file:line, and the disabled arm
    is unexpressible at two altitudes, not one;
  - the build cost, measured: 197 .initialize() call sites across 64
    files, plus the settings, alert, credential-file, document and IDE
    surfaces. That is why this turn produced tests rather than the
    redesign;
  - no ADR was filed and no number cited, with the reason;
  - the sa literal in the dev SQL Server script re-measured and
    deliberately left alone pending the corpus scope ruling.

Verified with parse_items from scripts/docs/backlog_status_check.py:
428 items before and after, ADDED and LOST both empty, and #1136's
open/closed state identical to origin/main. The item stays OPEN -- its
closing act is a scorecard rescore in the vault, which this cannot and
must not perform, and the cell stays partial until the redesign lands.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Provenance of the reviewed label on this pull request

This label was applied by the manager seat that commissioned this diff. It is not an independent
read of the scope.
Recording it here because a later auditor cannot tell the difference from the
label alone, and roles/MANAGER.md requires the seat that commissioned a diff to say so on the
pull request rather than let the label speak for itself.

The owner has disabled the reviewer role and its tag requirement. So the label here clears a gate
rather than reporting a review, and this comment exists so that is legible after the fact.

What the label does record, and it is not nothing:

  • the branch's own checks, which are visible above rather than taken on my word
  • that the Builder's brief was written by this seat and its report was read by it
  • that the load-bearing claims in that report were re-measured against origin/main by this seat
    before the label went on, with a positive control on each, not accepted on the Builder's word

What it does not record: that anyone other than the party who commissioned the work read this
diff. If that distinction matters for this change, treat this pull request as unreviewed.

The label was applied only after the branch's review-gate run read completed and the remote tip
was re-checked as unchanged immediately before and after, since a push from any seat between the
check and the action silently invalidates a label while every command still reports success.

@wshallwshall wshallwshall added the reviewed A reviewer has read this. Removed automatically when new commits arrive. label Sep 4, 2026
@wshallwshall
wshallwshall added this pull request to the merge queue Sep 5, 2026
Merged via the queue into main with commit eb582f9 Sep 5, 2026
43 of 44 checks passed
@wshallwshall
wshallwshall deleted the claude/asvs-1136 branch September 5, 2026 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reviewed A reviewer has read this. Removed automatically when new commits arrive.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant