feat(diagnostics): add the report data model and the redactor that fills it - #5568
Open
aodhanroche wants to merge 3 commits into
Open
aodhanroche wants to merge 3 commits into
aodhanroche wants to merge 3 commits into
Conversation
This was referenced Sep 16, 2026
aodhanroche
added this pull request to stack #5574
September 16, 2026 17:06
alex-rumsey-foundry
requested changes
Sep 22, 2026
alex-rumsey-foundry
left a comment
alex-rumsey-foundry
left a comment
Contributor
There was a problem hiding this comment.
One legitimate false-positive concern (errorcode matching the signed-URL param heuristic) and one real leak (hyphen not treated as a home-directory boundary terminator, so /Users/sam-2/... is left unredacted). See inline comments.
aodhanroche
force-pushed
the
Aodhan/260901/diagnostics-3-redaction-report
branch
from
September 22, 2026 15:10
ceb7d2b to
da89b3f
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…lls it The first two pieces of the diagnostics work: the shape a troubleshooting snapshot takes, and the thing that makes one safe to send. `report.py` is a schema-versioned pydantic model covering what is worth knowing when an engine has gone wrong -- version, host, resolved paths, which config layer each setting came from, which secrets are set and from where, which libraries loaded and which failed, which projects are registered, and where the logs are. Versioned so a support tool can still read a report written by an older engine, and so an unknown field is not a parse failure. `redaction.py` is what every value passes through on the way in, because a report exists to be handed to somebody else. It removes three kinds of thing: - Values whose config key says they hold a credential (`api_key`, `env`, `headers`, ...). Keys are kept; only values go, so a reader can still see that a setting is configured. - Values the engine knows are secrets because it read them from a `.env` file. Scrubbing these from free text is the only way to catch a credential that some library logged into an error message. - Things that look like credentials wherever they came from, plus the home directory and username, which identify a person rather than a machine. Redaction is counted per reason and the counts travel with the report, because "0 values hidden" and "3 values hidden" are the difference between a setting that is empty and a setting that was withheld from the reader. A value is masked whole rather than by substring match: a password containing `@` was otherwise partly written into the output, leaving most of a live credential readable. Nothing consumes either module yet. The health checks, the bundle writer, and the manager that assembles a report are the PRs above this one in the stack. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…as a path boundary Two ways the rules were wrong in opposite directions. `code`, `sig`, and `auth` were looked for anywhere inside a query-parameter name, so `?errorcode=500` was hidden along with `?assignee=` and `?author=` -- values holding nothing secret and carrying the answer somebody opened the report to find. Each abbreviation now matches only as the whole parameter name, which is the spelling the real credentials use (Azure's `sig`, OAuth's `code`). `signature` and `authorization` are added as full words so the vendor-prefixed names they appear in are still caught. The home-directory boundary allowed `_` and `.` as terminators but not `-`, so `/Users/sam-2/...` was the one spelling left in a report verbatim. In a file whose whole purpose is to err toward hiding too much, over-redacting a sibling directory is the safe way to be wrong and leaving the home path readable is not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
aodhanroche
force-pushed
the
Aodhan/260901/diagnostics-3-redaction-report
branch
from
September 22, 2026 18:41
da89b3f to
0c71888
Compare
Two of the report's defaults assert something rather than leave it blank: `identity_normalized` says the home directory was taken out, and `worker_ready` says a worker is up. Both have to default to the non-claiming value, or a report that checked neither tells its reader otherwise. `report.py` had no tests, so nothing held them there. Also covers the two redaction branches nothing reached: a credential-named list, and a settings key that is not text. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
The diagnostics work needs two foundations: the shape a troubleshooting snapshot takes, and the thing that makes one safe to hand to someone else. Redaction has to live in the engine — it's the only process that knows its own API keys, so it's the only one that can catch a key some library logged verbatim into an error message.
Fix
report.py— a schema-versioned pydantic model: version, host, resolved paths, which config layer each setting came from, which secrets are set and from where, which libraries loaded or failed, projects, log locations.redaction.py— everything passes through aRedactoron the way in. It strips values whose key says "credential" (keys kept, so you can still see a setting is configured), values it knows are secrets from.env, and anything credential-shaped plus home dir and username. Every removal is counted per reason and travels with the report, because "0 hidden" vs "3 hidden" is the difference between empty and withheld.Worth a look: values are masked whole, not by substring — a password with an
@in it used to get partly written out. 55 tests, each asking whether the secret appears anywhere in the output rather than whether the field says<redacted>.No consumers yet; those are the PRs above.
🤖 Generated with Claude Code