Skip to content

docs(backlog): file #1441, the WARNING+ PHI-filter bypass in non-serve CLI subcommands - #850

Closed
wshallwshall wants to merge 3 commits into
mainfrom
claude/priceless-easley-1cb72a
Closed

wshallwshall wants to merge 3 commits into
mainfrom
claude/priceless-easley-1cb72a

Conversation

@wshallwshall

Copy link
Copy Markdown
Collaborator

Files BACKLOG #1441. Ledger row only -- no engine code changes, no test files.

The defect

logging.lastResort carries filters=[] and formatter=None. Two of the CLI's 32 subcommands install a root logging handler (serve, supervise); the other 30 run with an empty root handler list, so the standard library services their records through lastResort. That bypasses RedactionFilter, CredentialQueryScrubFilter and ControlCharScrubFilter -- the three filters logging_setup._install_phi_filters puts on every configured handler.

Stated in the conditional per CLAUDE.md section 0: there are zero deployments, so nothing is leaking. What is wrong is the code -- a PHI-safety control the serve path has does not exist in 30 sibling processes.

Measured, with controls

Every reading has a paired control that fires, so an absence is distinguishable from a clean result.

arm root.handlers WARNING output
as shipped, real messagefoundry backup path [] synthetic PID segment and full traceback verbatim
control, configure_logging first [StreamHandler] + 3 filters PID|[redacted] in message and traceback

A real messagefoundry backup run holds root.handlers == [] throughout and loads 18 WARNING+ traceback sites. Negative control: 19 DEBUG/INFO exc_info sites are dropped, not leaked, because lastResort.level is WARNING.

Synthetic HL7 only, per section 9.

Re-verified before landing

Measured at 46ea10a78, re-verified at a2eef0f37 because main moved 58 commits in between and both measured files changed (__main__.py +264, logging_setup.py +113). All counts unchanged.

One claim did go stale in that window and was corrected rather than left standing: #1199's fix landed as 99887f5a4 (#820) and an earlier draft said it was not on main. Re-checked -- ensure_logger_sink is per-logger, never appears in __main__.py, and does not close this gap. The row records that as a method note.

Scope

Files the item only. No fix, by instruction. The row weighs both candidate remedies and records honestly that remedy (1) -- configure logging in main() -- empties caplog for the 6 test files that use it, of 40 asserting on captured CLI output. Measured as a paired test.

Checks run

ledger_check (with a firing negative control on an unallocated number), backlog_status_check, link_check, and the full pre-commit set on the changed file. All pass. No test file added, so tests/tooling_manifest.txt does not apply.

Number allocated atomically with scripts/coord/alloc.ps1 from this worktree.

Related: #1054, #1055 (same class, both shipped), #1199 (same premise, opposite consequence).

🤖 Generated with Claude Code

wshallwshall and others added 2 commits September 4, 2026 17:34
…e CLI subcommands

logging.lastResort carries filters=[] and formatter=None. Two of the CLI's 32
subcommands install a root handler (serve, supervise); the other 30 run with an
empty root handler list, so their WARNING+ records reach stderr through
lastResort and bypass the three filters _install_phi_filters puts on every
configured handler.

Measured with paired controls: a real `messagefoundry backup` run holds
root.handlers == [] throughout and loads 18 WARNING+ traceback sites; the same
record with configure_logging first comes out PID|[redacted].

Measured at 46ea10a and re-verified at a2eef0f, because main moved 58
commits in between and both measured files changed. The counts are unchanged.
#1199's fix landed as 99887f5 (PR 820) in that window; it is per-logger, never
appears in __main__.py, and does not close this gap.

Files the item only. No fix: the item weighs both candidate remedies and records
that remedy (1) empties caplog for the 6 test files that use it, of 40 asserting
on captured CLI output.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the unread Green and mergeable, but not marked read. Read it, then add: reviewed label Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

@wshallwshall

This pull request is green and nobody has marked it read.

Every check that has reported is passing and the reviewed label is absent, so
a reviewer has read this is the only thing between it and a merge. Nothing else reports that, which is
why this comment exists (BACKLOG #1413).

To clear it, IN THIS ORDER:

  1. If the branch is behind main, update it FIRST: gh pr update-branch --repo MEFORORG/MessageFoundry 850.
    That push fires synchronize, and synchronize REMOVES the reviewed label. Labelling
    before updating throws the label away and costs a round trip.
  2. Read the diff.
  3. gh pr edit 850 --repo MEFORORG/MessageFoundry --add-label reviewed

DO NOT TRIAGE THIS FROM mergeStateStatus. It returns one value with precedence, so BEHIND,
DIRTY and UNSTABLE each mask the missing check -- the requirement is invisible until you act on
something else. This check never reads that field; see scripts/ci/check_unread_prs.py.

A seat can find every pull request in this state in one call: gh pr list --label unread.
The label is withdrawn automatically once this pull request leaves the state.

The reviewed label is a PROCESS gate. It records that a step happened. It does not
establish that an independent party looked, and nothing here should be read as saying it does.

Resolved by the Lander. The ledger tail serialising: this branch appends
its item, main appended another. Different items, so both sides stand,
with main's block in the position it holds on main.

Set-difference control over item numbers: nothing lost, nothing invented,
no duplicate headings. #1069 is absent because main archived it at
7f97202, not because this merge dropped it.

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

Copy link
Copy Markdown
Collaborator Author

Superseded by #940, which merged at 11:52 CT as 89b2574c. This pull request's row (BACKLOG #1441) is on main now, byte-identical to what was filed here.

Closed by the Lander, not rejected. Eight ledger-only pull requests were consolidated into one so that nine filings cost one merge cycle rather than eight. Under strict branch protection each of these would have re-dirtied the other seven.

The consolidation merged each branch rather than cherry-picking, so every number arrived carried by a real merge parent, and it re-applied only this branch's own sections onto main's current file. That mattered: taking any of these branches wholesale would have silently reverted #1426, #1440, #1454, #1455 and #1456. Final state verified at 443 rows against a target computed before building, 678 additions and zero deletions, no duplicate numbers.

Nothing here needs re-filing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

unread Green and mergeable, but not marked read. Read it, then add: reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant