From 113dc37091b3b8bf24a478665422a147fe878dc9 Mon Sep 17 00:00:00 2001 From: JJ Fullmer <7743340+darksidemilk@users.noreply.github.com> Date: Fri, 21 Aug 2026 12:56:41 -0500 Subject: [PATCH] Declare which plugin columns are credentials, and which only look like 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 --- capone/hooks/addcaponeapi.hook.php | 25 ++++++++++++++++ pushbullet/hooks/addpushbulletapi.hook.php | 25 ++++++++++++++++ slack/hooks/addslackapi.hook.php | 25 ++++++++++++++++ windowskey/hooks/addwindowskeyapi.hook.php | 33 ++++++++++++++++++++++ 4 files changed, 108 insertions(+) diff --git a/capone/hooks/addcaponeapi.hook.php b/capone/hooks/addcaponeapi.hook.php index aacb38f1..2c0edbb1 100644 --- a/capone/hooks/addcaponeapi.hook.php +++ b/capone/hooks/addcaponeapi.hook.php @@ -55,9 +55,34 @@ public function __construct() parent::__construct(); $this->registerInstalled([ ['API_VALID_CLASSES', 'injectAPIElements'], + ['API_SENSITIVE_FIELDS', 'declareSensitiveFields'], ['CUSTOMIZE_DT_COLUMNS', 'customizeDT'], ]); } + /** + * Declares capone.key as NOT a credential. + * + * It matches Redaction::CREDENTIAL_PATTERN on the bare word "key" and is + * nothing of the sort: the edit form calls it "Key to match", and it is + * the DMI string capone compares against to pick an image. The + * unauthenticated capone endpoint posts it in the clear on every lookup, + * so treating it as secret would protect a value the protocol publishes + * anyway -- while blanking the one column that says which rule an audit + * row changed. + * + * The 'exempt' bucket exists so a plugin can make this call about its own + * model. Core must not: the bundled plugins are a fetched artifact (ADR + * 0009), and a core entry naming a plugin class fails on any tree that + * has not fetched them, which includes a fresh clone and CI. + * + * @param mixed $arguments The tier maps to modify. + * + * @return void + */ + public function declareSensitiveFields($arguments) + { + $arguments['exempt'][$this->node][] = 'key'; + } /** * Customize our new columns. * diff --git a/pushbullet/hooks/addpushbulletapi.hook.php b/pushbullet/hooks/addpushbulletapi.hook.php index 4fe8d302..3fbcc3ce 100644 --- a/pushbullet/hooks/addpushbulletapi.hook.php +++ b/pushbullet/hooks/addpushbulletapi.hook.php @@ -55,8 +55,33 @@ public function __construct() parent::__construct(); $this->registerInstalled([ ['API_VALID_CLASSES', 'injectAPIElements'], + ['API_SENSITIVE_FIELDS', 'declareSensitiveFields'], ]); } + /** + * Declares the access token as a secret the API must never emit. + * + * injectAPIElements() below puts this class in $validClasses, so until + * now every pushbullet row the API returned carried the token in clear + * to any caller holding pushbullet.view. The token is the whole + * credential: it posts as that Pushbullet account. + * + * The 'always' tier rather than the ordinary one, for the same reason + * ldap.bindPwd is there: nothing reads it back. Only the web tier sends + * it, to Pushbullet's API, and it does so through the model. + * + * The audit trail reads this registry too (ADR 0021 Decision 6), so this + * is also what keeps the old value out of an auditChange row when + * somebody rotates the token. + * + * @param mixed $arguments The tier maps to modify. + * + * @return void + */ + public function declareSensitiveFields($arguments) + { + $arguments['always'][$this->node][] = 'token'; + } /** * This function injects pushbullet elements for * api access. diff --git a/slack/hooks/addslackapi.hook.php b/slack/hooks/addslackapi.hook.php index b585f6d0..fd0f740a 100644 --- a/slack/hooks/addslackapi.hook.php +++ b/slack/hooks/addslackapi.hook.php @@ -55,9 +55,34 @@ public function __construct() parent::__construct(); $this->registerInstalled([ ['API_VALID_CLASSES', 'injectAPIElements'], + ['API_SENSITIVE_FIELDS', 'declareSensitiveFields'], ['CUSTOMIZE_DT_COLUMNS', 'customizeDT'], ]); } + /** + * Declares the webhook token as a secret the API must never emit. + * + * injectAPIElements() below puts this class in $validClasses, so until + * now every slack row the API returned carried the token in clear to any + * caller holding slack.view. Holding it is being able to post into that + * workspace as FOG. + * + * The 'always' tier rather than the ordinary one, for the same reason + * ldap.bindPwd is there: nothing reads it back. Only the web tier sends + * it, to Slack, and it does so through the model. + * + * The audit trail reads this registry too (ADR 0021 Decision 6), so this + * is also what keeps the old value out of an auditChange row when + * somebody rotates the token. + * + * @param mixed $arguments The tier maps to modify. + * + * @return void + */ + public function declareSensitiveFields($arguments) + { + $arguments['always'][$this->node][] = 'token'; + } /** * Customize our new columns. * diff --git a/windowskey/hooks/addwindowskeyapi.hook.php b/windowskey/hooks/addwindowskeyapi.hook.php index 20ce26f5..7aba2607 100644 --- a/windowskey/hooks/addwindowskeyapi.hook.php +++ b/windowskey/hooks/addwindowskeyapi.hook.php @@ -55,8 +55,41 @@ public function __construct() parent::__construct(); $this->registerInstalled([ ['API_VALID_CLASSES', 'injectAPIElements'], + ['API_SENSITIVE_FIELDS', 'declareSensitiveFields'], ]); } + /** + * Classifies this plugin's two pattern-matching columns. + * + * windowskey.key is a Windows product key, which is the same kind of + * thing as core's host.productKey -- so it goes in the same tier core + * puts that one in, the ORDINARY tier, not 'always'. That tier is + * stripped from API list payloads and kept on a direct single-entity + * GET, which is the shape that stops a bulk dump of every key an install + * holds without breaking a caller that asks for one key it is entitled + * to. The plugin's own pages are unaffected either way: they read the + * model directly, and the list grid is served by the web tier at + * ?node=windowskey&sub=list, not by the API emitter that strips. + * + * windowskeyassociation.windowskeyID is the opposite case. It matches + * Redaction::CREDENTIAL_PATTERN only because the word "key" is in the + * plugin's name -- it is a foreign key to the windowskey row, an integer + * id, and redacting it would blank the association's only meaningful + * column in the audit trail while protecting nothing. Hence the 'exempt' + * bucket, which exists so a plugin can say this about its own model: + * core must not name a plugin's class, because the bundled plugins are a + * fetched artifact and a core entry for one breaks on any tree that has + * not fetched them. + * + * @param mixed $arguments The tier maps to modify. + * + * @return void + */ + public function declareSensitiveFields($arguments) + { + $arguments['fields'][$this->node][] = 'key'; + $arguments['exempt']['windowskeyassociation'][] = 'windowskeyID'; + } /** * This function injects site elements for * api access.