Declare which plugin columns are credentials, and which only look like it - #22
Merged
Conversation
…e it Four models carry a friendly key matching Redaction::CREDENTIAL_PATTERN and none of them said what it was, so fogproject's coverage gate had nothing to check them against. Two are real credentials that the API was handing out: pushbullet.token and slack.token. Both classes are in $validClasses -- their own api hooks put them there -- so every list the API returned carried the token in clear to any caller holding pushbullet.view or slack.view. Holding either is being able to post as that account. Both go in the 'always' tier, for the same reason ldap.bindPwd is there: nothing reads them back, only the web tier sends them, and it does so through the model. windowskey.key is a Windows product key, the same kind of thing as core's host.productKey, so it goes in the tier core puts that one in -- the ordinary tier, stripped from API lists and kept on a direct single-entity GET. That stops a bulk dump of every key an install holds without breaking a caller entitled to one. The plugin's pages are unaffected: they read the model, and the list grid is served by the web tier at ?node=windowskey&sub=list, not by the API emitter that strips. The other two only look like credentials, and say so through the 'exempt' bucket the hook now carries: capone.key is the DMI string capone matches an image on -- the edit form calls it "Key to match" -- and the unauthenticated capone endpoint posts it in the clear on every lookup. Redacting it would protect a value the protocol publishes anyway while blanking the column that says which rule an audit row changed. windowskeyassociation.windowskeyID is an integer foreign key that matches only because the word "key" is in the plugin's name. Core cannot hold either answer on a plugin's behalf: the bundled plugins are a fetched artifact (ADR 0009), so a core entry naming a plugin class fails on any tree that has not fetched them, including a fresh clone and CI. The 'exempt' bucket exists so the plugin can answer for its own model. Needs fogproject's side of the seam: without it the exempt bucket is not passed to the event and these two declarations are ignored -- which leaves them redacted, the safe direction. Co-Authored-By: Claude <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.
Four bundled plugins carry a friendly key matching
Redaction::CREDENTIAL_PATTERNand none of them said what it was.Two are real credentials the API was emitting.
pushbulletandslackboth put their class in$validClassesvia their own api hooks, so every list the API returned carried the token in clear to any caller holdingpushbullet.view/slack.view. Holding either token is being able to post as that account. Both go in thealwaystier — nothing reads them back, the same reasoning asldap.bindPwd.One is a product key.
windowskey.keyis the same kind of value as core'shost.productKey, so it goes in the tier core puts that one in: the ordinary tier, stripped from API lists and kept on a direct single-entity GET. The plugin's own pages are unaffected — they read the model, and the list grid is served by the web tier at?node=windowskey&sub=list, not by the API emitter that strips.Two only look like credentials, declared through the new
exemptbucket:capone.keywindowskeyassociation.windowskeyIDCore cannot answer either on a plugin's behalf: the bundled plugins are a fetched artifact (ADR 0009), so a core entry naming a plugin class fails on any tree that has not fetched them — a fresh clone, or CI. That is why the
exemptbucket exists.Depends on
fogproject's side of the seam (
API_SENSITIVE_FIELDSgaining theexemptbucket). Without it the bucket is not passed to the event and the two exemptions are ignored — which leaves those fields redacted, the safe direction. The credential declarations work either way.Verified
tests/run-all.shhere: 7 passed, 0 failed.tests/audit-redaction-coverage.test.phpwith the seam in place: 66 checks, 0 failures, and four mutations confirmed the gate catches each declaration being removed or pointed at a renamed column.🤖 Generated with Claude Code