url-userinfo-credential: two residual shapes still pass — secret containing "/" and the scheme-less form #143
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: mathias/hyperguild#143
Reference in New Issue
Block a user
Follow-up to #142 (fixed in v0.18.4, live on the cluster as
ingestion:3c70b6f). Two shapes the new rule does not cover, surfaced during the infra#466 review. Filing rather than reopening #142, since that rule works for what it was written against.The shipped rule:
Verified by replicating the pattern exactly, with invented values only:
https://mathias:<40hex>@git.d-ma.behttps://mathias:abcd/efgh/ijkl…@git.d-ma.beoauth2:<40hex>@git.d-ma.be1. A secret containing
/— the one that matters[^@\s/]{8,}excludes/from the secret half. Standard base64 uses/in its alphabet (base64url does not), so any base64-encoded credential with a/in it is not scrubbed. That is not a corner case: it depends only on the bytes the encoder happened to produce, so the same credential is caught or missed depending on luck.The
/exclusion is there to stophttps://host:443/pathbeing read as userinfo. That job can be done by anchoring on the@instead — a userinfo secret is followed by@before any/:Removing
/from the secret class while keeping@as the terminator preserves the must-not-scrub cases, becausehttps://git.d-ma.be:443/mathias/infra.githas no@at all. This needs the full must-not-scrub table re-run, since it widens the secret class — in particular a URL with an@later in a path or query (https://host:8080/a@b) is the case to check.2. The scheme-less form
A credential store line or a log fragment can appear without its scheme:
Worth a companion rule, with a tight enough user half and a long enough secret floor to avoid matching ordinary
word:word@hosttext. Email addresses are the obvious false-positive risk (name:tag@example.com), so this one needs the negative cases more than the positive ones.Considered and rejected
https://u:abcd efgh@host): not matched, and it should stay that way. Real secrets do not contain spaces, and matching across whitespace would drop large amounts of legitimate prose.https://org/mathias:…@host): not matched, but that is not valid URL userinfo — a/ends the authority — so there is nothing to catch.Why this is worth doing rather than noting
The scrubber is the control that decides whether a credential-printing mistake is survivable. #142 established that the git credential-store shape had been walking through it across multiple sessions. These two residuals are the same shape with an encoding difference and a missing prefix, and the mistake that produces them is identical.
That said, this is narrower than #142: the leak that prompted it was caught by the shipped rule, verified by execution against the live
Scrub().Acceptance
/is dropped, rule named in the warn loguser:<secret>@hostform is droppedhttps://host:8080/a@b,https://host/path?to=a@b, and a bare email address in prosescrubber_test.go, red before greenCreated via git-mcp on behalf of @mathias
Correcting my own issue: the proposed fix for (1) is worse than the gap
I tested the widening I suggested before implementing it, and it is the wrong change. Recording this rather than quietly editing the body, because the reasoning is the useful part.
The proposed pattern creates false positives on image references with a digest. Dropping
/from the secret class, tested with invented values:image: localhost:5000/monitoring-tools@sha256:…https://registry:5000/mathias/ingestion@sha256:…https://host:8080/aaaaaaaaaaaa@versionhttps://mathias:abcd%2Fefgh…@git.d-ma.behttps://mathias:abcd/efgh…@git.d-ma.beA scheme-bearing image ref with a digest would be dropped as a credential. This estate logs those constantly, and
Scrubdrops the whole turn — so the "fix" would silently delete legitimate ingestion signal to close a narrow gap. Exactly the trade the scrubber's header warns about in the other direction.And the
/exclusion is not arbitrary — it encodes a real property. In a URL, userinfo precedes the authority, so the@comes before any/. "No slash in the secret half" is a correct expression of "this is userinfo, not a path". Removing it discards the only thing separating the two shapes.So how reachable is the literal-
/case at all? Narrower than I implied:/should be written%2F— which contains no literal slash and is matched by the shipped rule. (Stated as the mechanism, not verified — verifying it against the real store is not something to do casually, and it does not change the conclusion.)printf 'https://oauth2:%s@%s\n' "$TOKEN" "$HOST". A raw token containing/lands literally.DMABE_GITEA_API_TOKENis 40 bytes; if hex it can never contain/. Only a base64-alphabet secret can.So: real but narrow, and not fixable by widening the character class.
Revised scope for this issue. (1) needs design, not a patch — options worth weighing are requiring the post-
@text to look like a host, or a separate high-specificity rule for the raw-printf shape, or percent-encoding at the producer so the stored form always matches. (2), the scheme-less form, is unaffected by any of this and remains a straightforward addition guarded by email false-positive cases.Downgrading my own framing: the original body said the same credential is "caught or missed depending on luck". That is true only for a base64-alphabet secret written without encoding, which is a narrower claim than it read as. The leak that prompted #142 was caught by the shipped rule, verified by execution against the live
Scrub().Not starting an implementation on (1) until the shape is decided. Leaving this open with the analysis rather than shipping a change that trades a narrow gap for a broad false positive.
Created via git-mcp on behalf of @mathias