fix(webconsole): attribute the bulk connection control's denial rows to the browser (BACKLOG #1742)

[wshallwshall]

What this changes

ui_bulk_control in messagefoundry_webconsole/routes/connection_writes.py called
dual_role_control without client, so a per-channel RBAC denial raised from a browser-initiated
bulk control would write an auth.channel_denied row with a NULL client.

One line changes:

result = await core.dual_role_control(
    engine, identity, name, action, role=role, client=client_ip(request)
)

client_ip was already imported in this module for the purge-confirm route, so the fix adds no
import. _dual_role_control (messagefoundry/api/app.py) declares client: str | None = None and
threads it into _control_guard and _audit_channel_denied.

ADR 0150 gives NULL a meaning: no client was in scope. A browser-initiated control has one, so NULL
there would not be missing data, it would be a false statement about the row's provenance, on the
row an investigator most wants a host for.

Severity is Low and conditional. MessageFoundry has zero deployments, so nothing has recorded a
wrong row; a deploying site would.

BACKLOG #1742 status text

Do not edit docs/BACKLOG.md here (policy stub; the ledger is maintainer-internal). Suggested
status for the item:

Built. ui_bulk_control now passes client=client_ip(request) into dual_role_control, so a
bulk-initiated per-channel denial records the browser's address instead of NULL. Covered by
packaging/messagefoundry-webconsole/tests/test_ui_bulk_control_client.py, which fails on the
pre-fix code. The row's unverified half was checked and holds: the console PER-NAME control
already recorded the address correctly, via _ui_control forwarding the browser's Request into
the JSON handlers.

Measured before and after

Both arms on ONE tree, only the client= kwarg differing, with the worktree source forced ahead of
site-packages (see the import-path note below).

Test	Without client=	With client=
test_the_per_name_console_control_records_the_browser_address (positive control)	PASS	PASS
test_the_bulk_console_control_records_the_browser_address	FAIL, client=None	PASS
test_every_denied_target_in_one_batch_is_attributed	FAIL, [None, None, None]	PASS
test_a_permitted_bulk_target_is_not_denied (discriminator)	PASS	PASS

The pre-fix row, read from the audit table:

{'actor': 'op', 'action': 'auth.channel_denied', 'channel_id': 'IB_DENIED',
 'detail': '{"channel": "IB_DENIED"}', 'client': None}

The positive control matters: a bare "client is not NULL" test could fail for reasons unrelated to
the bulk route (a column nothing writes, a denial path taking no client, a transport reporting no
peer). Driving the SAME denial through the sibling per-name route on the same engine and transport,
and seeing 127.0.0.1, isolates the bulk route as the difference. The discriminator guards the
mirror failure: an in-scope name must reach the 404 and write no denial row, or the other three
tests would be vacuous against a guard that denied everything.

HOW THE TESTS WERE RUN, and why that qualifier is here

In this worktree, pytest imports the web console from .venv/Lib/site-packages/, NOT from the
worktree source. The engine is installed editable (_editable_impl_messagefoundry.pth); the console
is a plain non-editable copy plus messagefoundry_webconsole-0.2.15.dist-info. So an unqualified
local console run tests a snapshot and would go green having executed none of this change.

Every result above was produced with the source forced ahead of site-packages:

$env:PYTHONPATH = "<worktree root>"; .\.venv\Scripts\pytest.exe packaging/messagefoundry-webconsole/tests -q

CI is unaffected: .github/workflows/ci.yml installs -e packaging/messagefoundry-webconsole, so
the webconsole job runs against the source tree.

A note for anyone else checking this, because the obvious instrument gives the WRONG answer. Run
from the worktree root, python -c "import messagefoundry_webconsole as m; print(m.__file__)" prints
the WORKTREE path and reports all clear, because -c puts the working directory first on
sys.path. Pytest does not. The question is what PYTEST imports, so measure it from inside the
suite:

import messagefoundry_webconsole as m
def test_where() -> None:
    raise AssertionError(m.__file__)

Measured here: the -c form printed the worktree path; the in-suite form printed the site-packages
path. Same worktree, same interpreter, opposite answers.

Checks run

All from the worktree venv, with PYTHONPATH set for the pytest legs.

• ruff check . - passed
• ruff format --check . - passed, 1306 files
• mypy messagefoundry messagefoundry_webconsole --exclude 'messagefoundry/tray/' (the CI form) - passed, 293 files
• pytest packaging/messagefoundry-webconsole/tests - 505 passed, 3 skipped
• pre-commit hooks at commit time - all passed, including the ledger gate, SPDX, leak guard and bandit

NOT run, and why: the engine suite (tests/), which this change cannot reach - it touches one
console route and adds one console test file. Legs that only ever run on a hosted runner, including
windows-service-smoke, the SQL Server and Postgres store legs, and the load leg, must be read on
the PR after this process exits.

Findings raised in passing, none acted on

None of these are fixed here. Each is reported so the measurement is not lost.

1. docs/adr/0150-client-address-on-audit-entries.md (decision item 4, deliberate-NULL bullets)
   says _audit_channel_denied gets no request "when handed to the console seam as a bare
   callback". The shipped code contradicts that: connection_writes.py passes
   client_ip(request) into core.audit_channel_denied in the purge-confirm route. This is the
   SDS-3.7 shape, a stated rationale resting on a premise that stopped holding. An ADR edit, not a
   code edit.
2. docs/adr/0155-*.md and scripts/security/dast_target.py both state that an in-process ASGI
   transport leaves request.client as None. That is false for httpx.ASGITransport, which
   defaults the peer to ("127.0.0.1", 123). The new tests depend on the true behavior, so a
   reviewer reading ADR 0155 would wrongly conclude they cannot pass.
3. messagefoundry/api/app.py inlines request.client.host if request.client else None in two
   places rather than calling client_ip. That is the second, divergent notion of the client
   address the client_ip docstring exists to prevent. Predates this change.
4. messagefoundry_webconsole/routes/core.py renders a missing client as the string "<unknown>"
   where client_ip returns None. Predates this change; NOT touched, that file belongs to
   another Builder right now.
5. dr_activate / dr_release write NULL clients on BOTH the JSON and console planes, so this is
   symmetric rather than a console-parity defect, and it is already on ADR 0150's own "not done"
   list. Reported, not touched.
6. The worktree bootstrap gap in the import-path section above is worth its own item: a console
   source change is untestable in a new.ps1 worktree without PYTHONPATH, and the failure is
   silent and green. scripts/worktree/new.ps1 passes -e packaging/messagefoundry-webconsole,
   yet this venv ended up with a non-editable console, so either the flag does not take or
   something later replaced it.

Question the brief left open

The four tests all land on the fall-through _control_guard denial, because the target names no
registered connection. The shared-outbound denial inside _dual_role_control is a second site that
the same client now feeds, and it is not covered here. Adding that would need a live
RegistryRunner with a registered outbound, which is a larger fixture than this item's scope. Say
whether you want it and I will take it as a follow-up.

Not done, by rule

Not enqueued, auto-merge not armed, not merged. The Lander owns all three. The item claim is still
held (claim.ps1 -Release 1742 once this lands).

[wshallwshall]

LANDER INSPECTION -- labelled self-review, not a peer review

Posted under the korus LANDER.md line 364 obligation: with required_approving_review_count: 0
I am the last reader. I did not author this change.

The defect is an audit-attribution gap, and I verified it by enumeration

_dual_role_control in messagefoundry/api/app.py:2153 takes client: str | None = None, and that
value flows to both _control_guard(engine, identity, name, client) and
_audit_channel_denied(engine, identity, name, client). Omitting it does not fail; it writes a
denial row with no operator attribution.

Every call site on origin/main:

Site	passes client=
app.py:2227 -- JSON start	yes, client_ip(request)
app.py:2236 -- JSON stop	yes
app.py:2245	yes
routes/connection_writes.py:93 -- console bulk control	no

So one path out of four loses the operator's host on exactly the rows that record a refusal. The
same file already passes client_ip(request) to core.audit_channel_denied at line 154, so this
PR makes the function internally consistent as well as consistent with its siblings.

Why the fix is the right shape

Six lines, one keyword argument, no new plumbing -- client_ip and request are both already in
scope at the call site. The comment cites ADR 0150 for why request is the browser's, which is the
load-bearing premise: the console mounts in-process, so without that the argument would name the
engine rather than the operator.

One note for the reader, not a blocker

The failure mode here is the quiet kind. An audit row written with client=None is still a row --
it appears in the log, it looks complete, and nothing anywhere reports that the field is empty.
That is why this survived: no gate asks whether an audit record identifies anybody. The fix is
right, and the absence of a check that would have caught it is worth someone's attention
separately.

Verdict

Verdict: merge.