Sign-out audit rows record the client address, and the no-local-Administrator WARNING keeps every name (BACKLOG #3339, #3282) by wshallwshall · Pull Request #2435 · MEFORORG/MessageFoundry

wshallwshall · 2026-10-19T20:28:59Z

Batch 294 (auth), items #3339 and #3282. This PR gets its own because #3339 changes a security control: what the auth.logout audit row records, through a now-required client keyword on AuthService.logout. #3282 rides along because its commits are interleaved with #3339's on the Builder's branch, and splitting them out would mean rewriting the Builder's history. It is a WARNING rewording in the same file, with no control change.

Merge danger -- #3339

Reversible: yes -- a revert on main undoes all of it.

Blast radius: AuthService.logout in messagefoundry/auth/service.py (new required client keyword); POST /auth/logout in messagefoundry/api/auth_routes.py and /ui/logout in messagefoundry_webconsole/routes/core.py (both pass client_ip(request)); the web console seam digest moves 5d9c162790077dfc -> 1f2d208818510f18 (messagefoundry/api/_ui_seam.py, messagefoundry_webconsole/__init__.py, tests/golden/webconsole_seam.snapshot), so any other seam-moving branch clashes on that value by design (BACKLOG #1220). Routes answer the same as before.

Evidence -- #3339

Before: at main 6d6986fdd, AuthService.logout has no client parameter and neither route passes one, so every auth.logout row has a NULL client.
After: a test for each route reads the auth.logout row back and asserts actor, client and outcome, with sign-in and sign-out from different pinned addresses. The /ui/logout control signs out with a cookie naming no session and expects (None, <address>, nothing_to_revoke). The JSON route's control is the existing rotation-race test, which now also checks actor and client. A service-level test checks client reaches the row on both outcomes.

Merge danger -- #3282

Reversible: yes.

Blast radius: the text of one start WARNING (AuthService.report_administrators_needing_an_outside_service). logging_setup.py and secretscrub.py are untouched.

Evidence -- #3282

Before: sending the old wording through the shipped filter chain (tests/_phi_log_capture.filtered_sink, the _install_phi_filters chain) loses dir-a (the control).
After: the new wording lists the names after "Accounts that need one:", ahead of the word "password"; through the same chain all three names survive. With main's wording swapped back in, the test fails.

Source branch

b294-3339-3282, head b7b1d8417fbb0ad6c178cbd79359a827dcc391a8, on main 6d6986fdd.

Item	Commits
#3339	`80b836029` build, `69d2c6088` review round 1, `8662c5573` round 2 (reverts round 1's owner pre-read), `3411694c0` branch-review follow-ups
#3282	`6d3f81646` build, `b7b1d8417` changelog limits

git merge-tree against PR 2387's head and against batch 294's other branch (b294-3350) both exit 0.

Builder report (summarised by the Manager; the full text is in the commit messages)

#3339 -- BUILT, one part still open.

AuthService.logout takes a REQUIRED client keyword (as revoke_own_session does) and writes it on the auth.logout row. Five existing test call sites pass client=None.
Closing step 2: no change. Engine PR 2384's detail.outcome nothing_to_revoke already marks a sign-out that ended nothing. Code review argued the row asks for "its own event", not a detail field on the same action. The Lander or the owner decides whether step 2 is met.
Not fixed, still open: a console sign-out that loses a rotation race keeps a NULL actor (PR 2384 finding 3). Round 1 tried reading the owner before the revoke. Round 2 showed that read revokes and reports a row whose seal fails, and adds a failure path before the revoke. A read with no side effects would need a second get_session reader, which tests/test_session_seal.py forbids. The pre-read was reverted, and the docstring and changelog say why.
Closing step 4, the ASVS 16.2.1 re-read, is vault record work for the Manager and is not in this PR.
Console CHANGELOG.md entry added (changelog.d/README.md requires one for a console change). Fragment changelog.d/3339.security.md.
Claim on #3339 is still held, because of the open race-loser part.

#3282 -- BUILT. Fragment changelog.d/3282.fixed.md, which notes that a username shaped like a credential label or a date can still be rewritten by the filters, and that the audit row holds the full list. Claim released.

Checks, on the final tree unless noted:

ruff check "All checks passed!"; ruff format --check "2100 files already formatted".
mypy: linux "no issues found in 361 source files"; win32 "342"; tests "1387"; scripts/asvs "6". The win32, tests and asvs legs were last run before the final docstring, changelog and test-docstring commits, which change no types.
pytest, scoped: 397 passed (seam, api_auth, ui_logout_affordance, session_supersession, asvs_phase0, auth_before_body, oidc_step_up, session_seal, session_rotation_wiring, changelog_fragments); 108 passed on a rerun after the final edits; 72 passed (notice and provision-admin tests); 170 passed (test_release_pipeline); 17 passed (seam snapshot after #3282).
Not run: the full suite, and the PostgreSQL and SQL Server contract legs. Read the full test legs and the server-DB legs on this PR.

Shipped open (code review, for rows)

Pre-existing: if auth.logout raises after the revoke, /ui/logout returns a 500 and never clears the cookie or sends Clear-Site-Data.
At least three other log lines lose text to the same scrub: ldap.py:2308 ("...SPNEGO token: %s"), uploads.py:2148 and :2165 ("...next pass: %s"). The review suggests one test that checks every log format string against the filter chain.
The sibling sign-out audits record no client: revoke_other_sessions and revoke_sessions_for_user (auth.session_revoked).
docs/PHI.md section 6 still says audit_log.actor is "always populated", which PR 2384's NULL-actor rows contradict.
ADR 0150's count of "25 auth audit sites" that stay NULL no longer counts auth.logout correctly.
There are now three auth.logout row readers in tests (#3749 item 9).
A username shaped like a credential label or a date can still be lost from the WARNING through the filters.
The race-loser NULL actor above (PR 2384 finding 3), which keeps #3339 open.

Proposed vault banners (the Lander's to flip)

#3339: BUILT, pending merge. POST /auth/logout and /ui/logout pass client_ip, and AuthService.logout records it on the auth.logout row through a required client keyword. Tests read the row back for each route (actor, client, outcome), signing in and out from different addresses; a control signs out with a cookie naming no session. Closing step 2 was already met by engine PR 2384: a sign-out that revokes nothing records detail.outcome nothing_to_revoke on the same action. Open: a console sign-out that loses a rotation race keeps a NULL actor (PR 2384 finding 3); closing step 4, the ASVS 16.2.1 re-read, is vault record work. Web console seam digest moved to 1f2d208818510f18.
#3282: BUILT, pending merge. The start WARNING lists the accounts after "Accounts that need one:", ahead of the word "password", so the shipped CredentialScrubFilter no longer masks the first name. A test sends the real WARNING through the shipped handler filter chain and finds every name; a control shows the old wording loses one. logging_setup.py and secretscrub.py are unchanged.

AuthService.logout takes an optional client keyword and records it on the auth.logout audit row. POST /auth/logout and /ui/logout both pass client_ip(request), the extractor the other audited auth routes use. Closing step 2 was already met by PR 2384 (BACKLOG #3029): a sign-out that revokes nothing writes its own recorded detail.outcome, nothing_to_revoke. Unchanged here. Not fixed: a console sign-out that loses a rotation race keeps a NULL actor. The rotated row no longer answers to the presented hash, so a lookup by it finds nothing; naming the owner needs the store to keep a re-keyed session's prior hash, on three backends. Tests read the row back for each route (actor, client, outcome), with a pinned peer. Control: /ui/logout with a cookie naming no session records nothing_to_revoke, no actor, and the address. The web console seam digest moved (5d9c162790077dfc to 499c326be7a5a50b) because logout's signature changed.

…iew round 1 (BACKLOG #3339) - With no actor, logout reads the session through _load_session before supersede_session and names that row's owner. A console sign-out that loses a rotation race now names the owner, as POST /auth/logout's own dependency read does (engine PR 2384 open finding 3). The owner comes from a row whose seal verified, not from the blanked row the revoke returns. A cookie re-keyed before the request still names no actor. - client is a required keyword, as on revoke_own_session, so a new sign-out caller cannot omit it silently. Test call sites pass it. - The route tests sign in and sign out from different addresses, so the row cannot pass by recording the session's login address. - New service tests: client reaches the row on both outcomes, and the race test, red on the prior logout (its injection never fires) and red when the owner is taken only from an ended row. - Changelog drops its completeness claim; the seam digest is re-derived (now 1f2d208818510f18).

The round 1 pre-read went through _load_session, the one sanctioned session reader. It ends and reports a row whose seal fails, so the revoke then found nothing and the row said nothing_to_revoke with no actor for a session this call ended. It also put a failure point ahead of the revoke. A side-effect-free read would need a second get_session reader, which tests/test_session_seal.py forbids. logout attributes the owner of the row the revoke ended again, as PR 2384 left it. A console sign-out that loses a rotation race keeps a NULL actor; the docstring and changelog say so, and why. The POST /auth/logout race test now signs in from another address, so its client assertion is the sign-out's own address.

The start WARNING from report_administrators_needing_an_outside_service put the usernames right after "no usable password:". The shipped CredentialScrubFilter reads "password: <value>" as a credential and masked the first name. The list now follows "Accounts that need one:", ahead of the word "password". logging_setup.py and secretscrub.py are unchanged. Test: every name survives the filter chain every engine handler carries (filtered_sink, the real _install_phi_filters), not caplog. A control sends the old wording through the same chain and loses the first name. The test is red on the prior wording.

…OG #3339) - packaging/messagefoundry-webconsole/CHANGELOG.md records the console change and the seam move, as changelog.d/README.md requires. - The logout docstring no longer says a NULL client means no request: client_ip can find no address on a real request. The race residual cites engine PR 2384 open finding 3 rather than this item. - The /ui/logout control test says it pins still-open behaviour (PR 2384 open finding 6, held by vault BACKLOG #3749).

…(BACKLOG #3282) The WARNING lost its first name from the day it was added, so this is not a regression undone. A name that looks like a credential label or a date can still be rewritten by the log filters; the audit row holds the complete list. Proposed PR title: Sign-out audit rows record the client address, and the no-local-Administrator WARNING keeps every name (BACKLOG #3339, #3282) Proposed banner #3339: BUILT, pending merge. POST /auth/logout and /ui/logout pass client_ip, and AuthService.logout records it on the auth.logout row through a required client keyword. Tests read the row back for each route (actor, client, outcome), signing in and out from different addresses; a control signs out with a cookie naming no session. Closing step 2 was already met by engine PR 2384: a sign-out that revokes nothing records detail.outcome nothing_to_revoke on the same action. Open: a console sign-out that loses a rotation race keeps a NULL actor (PR 2384 finding 3); closing step 4, the ASVS 16.2.1 re-read, is vault record work. Web console seam digest moved to 1f2d208818510f18. Proposed banner #3282: BUILT, pending merge. The start WARNING lists the accounts after "Accounts that need one:", ahead of the word "password", so the shipped CredentialScrubFilter no longer masks the first name. A test sends the real WARNING through the shipped handler filter chain and finds every name; a control shows the old wording loses one. logging_setup.py and secretscrub.py are unchanged.

wshallwshall · 2026-10-19T20:29:18Z

QA line, BUILDER.md step 11, posted by the Manager from the Builder's report as given (BACKLOG #3339, #3282, head b7b1d8417fbb0ad6c178cbd79359a827dcc391a8):

I ran the code-review skill at xhigh three times, with your exact args. It printed no level tag, only a JSON findings list (10, then 10, then 9 findings). The two repair rounds were for #3339; the third run covered the whole branch after #3282.

Applied: client made required; sign-in and sign-out from different addresses in the tests; the pre-read added, then reverted; changelog wording (no completeness claim, no false "again"); the logout docstring no longer says a NULL client means no request; the control test says it pins behaviour that is still open (#3749); the console CHANGELOG entry; filtered_sink reused instead of a copied helper.

Left open: the seven items listed under "Shipped open" in the PR body.

wshallwshall · 2026-10-19T21:07:03Z

Manager triage of the red test (windows-2025, py3.14) leg, run 39319681012, job 118290365286, head b7b1d8417fbb0ad6c178cbd79359a827dcc391a8.

Result: 1 failed, 42585 passed. The failure is tests/test_lifespan_teardown.py::test_a_cancel_during_the_first_drain_still_runs_every_later_step: TimeoutError at line 331, where asyncio.wait_for(task, _WAIT_S) (10 s) waits for the cancelled lifespan task to finish its teardown.
This branch changes no lifespan, pipeline or api/app.py code (git diff --stat 6d6986fdd b7b1d8417 over those paths is empty). It changes AuthService.logout, the two sign-out routes, and one WARNING string.
That test does not appear in the failed logs of the 8 most recent failed ci.yml runs (39313307034, 39183210721, 39177122501, 39175790071, 39173562168, 39170936041, 39131597083, 39124776717). Control: the same grep finds other FAILED tests/ lines in 2 of them.
Reading: a one-off timing failure on the Windows runner, not caused by this diff. Recommended action: one rerun of the failed job once the run completes. A second red on the same test would be a finding, not a flake.

wshallwshall · 2026-10-19T22:04:23Z

Lander: enqueued at b7b1d84, pinned.

Review: the QA line names this head (code-review at xhigh, three runs).
The 8 shipped-open findings are filed as vault #3884, now on vault main (pass 8, squash 20092876). Graded non-blocking: items 1 and 8 are pre-existing, and the rest are log-text loss, doc drift, test duplication and missing clients on sibling rows. None changes behaviour against main or weakens a control. That grade is a Lander decision under the owner's Driver delegation, not an owner ruling. #3339 stays open on item 8.
The windows-2025 red (test_lifespan_teardown) passed on its one rerun.
All 9 live required contexts are SUCCESS on this head, and so are the PostgreSQL and SQL Server contract legs.
This PR moves the web console seam digest. A seam-moving PR that lands after it re-syncs.

wshallwshall · 2026-10-19T22:32:49Z

Lander: re-enqueued at b7b1d84, the same pinned head. The eviction at 22:23:24Z (merge-group run 39332349173) was test_ui_dr_outlive::test_a_console_release_cut_off_by_the_deadline_still_completes on web console windows-2022, 1 failed / 1096 passed. That is the deadline race filed as vault #3871, its third hit today across three PRs (2387 ubuntu, 2436 windows-2025, this one windows-2022). This PR touches no DR code.

github-actions · 2026-10-19T22:45:59Z

CI failed in test (ubuntu-latest, py3.14) / Tests (pytest) (and 1 more failing job) while this pull request was in the merge queue, so the queue ejected it.

Its own head can still be green: the queue revalidates the merge, and the path gates that skip on a pull request run there. Read the run before retrying.

https://github.com/MEFORORG/MessageFoundry/actions/runs/39332349173

wshallwshall · 2026-10-19T22:53:13Z

Manager triage of the merge-queue ejection, run 39332349173 (gh-readonly-queue/main/pr-2435-d18836b0), PR head b7b1d8417fbb0ad6c178cbd79359a827dcc391a8. Two failing legs:

web console tests (windows-2022, py3.14): 1 failed, 1096 passed. The failure is test_ui_dr_outlive.py::test_a_console_release_cut_off_by_the_deadline_still_completes, the same test that ejected PR 2436 in run 39328582619 (an unrelated diff).
- Across the last 40 ci.yml runs, the web console Windows legs failed this test in those two queue groups only.
- The same two PRs' later queue groups (39334524198, 39334593749, 39335750057) passed both Windows web console legs, and so did both PRs' own pull-request runs.
- Reading: an intermittent test, not this diff. Neither PR touches DR, request-timeout or web console DR code.
test (ubuntu-latest, py3.14): 1 failed, 42477 passed. The failure is tests/test_durability_hook_push_bound.py::test_the_TEARDOWN_ends_a_SUSPENDED_process_frozen_before_it_ran: "could not arm: the teardown census did not see the frozen process". The test failed while setting up its own suspended process, before checking anything. This PR touches nothing in that area.

No code change pushed. The DR-outlive test has now failed twice in one evening and needs its own ledger row, for the Lander to file.

github-actions · 2026-10-19T23:14:16Z

CI failed in sql server (store + connector) 2025 / Run the throughput-lever backend invariants on real SQL Server (B1 inline, B2 batch-claim, ...) while this pull request was in the merge queue, so the queue ejected it.

Its own head can still be green: the queue revalidates the merge, and the path gates that skip on a pull request run there. Read the run before retrying.

https://github.com/MEFORORG/MessageFoundry/actions/runs/39335750057

wshallwshall · 2026-10-19T23:15:37Z

Lander: re-enqueued at b7b1d84, the same pinned head. This eviction (23:05:30Z, merge-group run 39335750057) was a third, unrelated flake: tests/test_stage_dispatcher.py::test_adr0070_8_correlated_outage_single_reload_rearms_all[sqlserver] on sql server (store + connector) 2025. This PR touches no dispatcher or store code. Each of its three evictions was a different test, and none is in its diff.

wshallwshall · 2026-10-19T23:17:22Z

Manager triage of the third merge-queue ejection, run 39335750057 (gh-readonly-queue/main/pr-2435-cc24a84cf), PR head b7b1d8417fbb0ad6c178cbd79359a827dcc391a8.

sql server (store + connector) 2025: 1 failed, 255 passed. The failure is tests/test_stage_dispatcher.py::test_adr0070_8_correlated_outage_single_reload_rearms_all[sqlserver], assert False.
This PR changes no pipeline, store or stage-dispatcher code (git diff --stat 6d6986fdd b7b1d8417 -- messagefoundry/pipeline messagefoundry/store tests/test_stage_dispatcher.py is empty).
Over the last 40 ci.yml runs, the SQL Server 2025 leg succeeded 26 times and failed once: this run.
Known intermittent test: vault BACKLOG fix(release): install and inspect the wheels the smokes only named, and pin the harness engine (BACKLOG #1701, #1585) #1270 (note 2026-10-01, "failed again on the 2025 leg and passed on a re-run with no change") and #3456 (notes 2026-10-13, passes m5 and m15) already record it.
Reading: not this diff. No code change pushed.

github-actions · 2026-10-19T23:18:33Z

Security failed in dependency-and-secret-scan (pip-audit, npm-audit, gitleaks) / Verify every declared dependency is a real, established distribution while this pull request was in the merge queue, so the queue ejected it.

Its own head can still be green: the queue revalidates the merge, and the path gates that skip on a pull request run there. Read the run before retrying.

https://github.com/MEFORORG/MessageFoundry/actions/runs/39340479880

wshallwshall · 2026-10-19T23:20:43Z

Manager triage of the fourth merge-queue ejection, Security run 39340479880 (gh-readonly-queue/main/pr-2435-cc24a84cf), PR head b7b1d8417fbb0ad6c178cbd79359a827dcc391a8.

dependency-and-secret-scan, step "Verify every declared dependency is a real, established distribution": dependency vetting could not reach PyPI (URLError(TimeoutError('_ssl.c:1064: The handshake operation timed out'))). The gate fails closed by design when it cannot reach PyPI.
This PR changes no dependency (pyproject.toml, uv.lock and requirements.lock are untouched).
Reading: a network failure on the runner, not this diff. No code change pushed.

wshallwshall · 2026-10-19T23:21:18Z

Lander: re-enqueued at b7b1d84, the same pinned head. Fourth eviction (23:18:41Z, Security merge-group run 39340479880): dependency-and-secret-scan failed closed because it could not reach PyPI ("The handshake operation timed out"). That is a transient network failure, and the gate refusing while blind is correct. This PR changes none of pyproject.toml, uv.lock or requirements.lock. Each of its four evictions had a different cause, none in its diff.

wshallwshall added 6 commits October 19, 2026 14:38

wshallwshall added the qa Builder QA record posted; not a merge gate label Oct 19, 2026

github-actions Bot added the ci-red A required check went red. Attribute it before retrying. label Oct 19, 2026

wshallwshall added this pull request to the merge queue Oct 19, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 19, 2026

wshallwshall added this pull request to the merge queue Oct 19, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 19, 2026

wshallwshall added this pull request to the merge queue Oct 19, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 19, 2026

wshallwshall added this pull request to the merge queue Oct 19, 2026

Merged via the queue into main with commit d61284d Oct 19, 2026
73 of 75 checks passed

wshallwshall deleted the b294-3339-3282 branch October 19, 2026 23:57

Conversation

wshallwshall commented Oct 19, 2026

Merge danger -- #3339

Evidence -- #3339

Merge danger -- #3282

Evidence -- #3282

Source branch

Builder report (summarised by the Manager; the full text is in the commit messages)

Shipped open (code review, for rows)

Proposed vault banners (the Lander's to flip)

Uh oh!

wshallwshall commented Oct 19, 2026

Uh oh!

wshallwshall commented Oct 19, 2026

Uh oh!

wshallwshall commented Oct 19, 2026

Uh oh!

Uh oh!

wshallwshall commented Oct 19, 2026

Uh oh!

github-actions Bot commented Oct 19, 2026

Uh oh!

wshallwshall commented Oct 19, 2026

Uh oh!

Uh oh!

github-actions Bot commented Oct 19, 2026

Uh oh!

wshallwshall commented Oct 19, 2026

Uh oh!

wshallwshall commented Oct 19, 2026

Uh oh!

github-actions Bot commented Oct 19, 2026

Uh oh!

Uh oh!

wshallwshall commented Oct 19, 2026

Uh oh!

wshallwshall commented Oct 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant