[tracking202]

Backlog: 9KK7

What was wrong

I confirmed this by reading the code and reproduced it live on an instance built from origin/1.9.79. GET /users/{id}/preferences answered SELECT * FROM 202_users_pref. Any key that could read preferences therefore got every integration secret as stored:

• the ClickBank, JVZoo, Zaxaa and IPQS keys;
• the Slack webhook;
• the RevContent id and secret;
• the Landing Page Optimizer's lpo_site_key and lpo_bridge_config (its ctx_key is a signing key).

Every one of those values showed up in the response bytes. The same applied to the CLIs (p202 user prefs get, user:prefs:get) and to the answer the update commands print. Meanwhile documentation/api/15-system.md said the secrets are never served.

A second finding: a body that GET answered could not be sent back. The brief says it "must still round-trip", but on origin/1.9.79 PUT refused it with 422 for user_id and every other column it does not write. It now round-trips.

What changed

Prosper202\User\ServedPreferences (new) decides what leaves the server:

• SERVED is an allow-list of the 55 columns that hold no credential. These are served as stored.
• Every other column is answered as {"stored": true|false}. That includes the 9 on SECRET, and any column on neither list, so it fails closed.
• "Stored" means non-null and not blank. "0" counts as stored.

I picked the object shape over a sibling *_stored key. The field keeps its name, and a reader that expects a string fails loudly instead of quietly reading "" (CLAUDE.md #28). The shape is documented as StoredSecret in docs/openapi.yaml.

UsersController

• The raw row stays private (preferencesRow()).
• getPreferences() and the PUT answer go through serve().
• accountCurrency() reads the raw row.

PUT accepts a GET body (CLAUDE.md #4). ServedPreferences::withoutEchoes() sorts the keys first:

• A key PUT does not write is read-only. It is accepted with the value GET gave it and refused with any other.
• A secret's {"stored": …} sent back as GET gave it keeps the secret. Any other object is refused, naming the field. So {"stored": false} does not clear a secret ("" does), and {"stored": true} is never stored as a value.
• A secret is still set from a string. The cb_verified reset still happens only when cb_key changes.
• A body of nothing but unchanged keys gets a 422 (No valid fields to update), the same answer as the CRUD base.

Staged-change redactor (second line of defence). StagedChangesController::SECRET_KEY_SUBSTRINGS gains revcontent_user. revcontent_user_id had been classified there as not a credential, while this item treats it as a secret. PreferenceSecretCoverageTest now takes its allow-list from ServedPreferences::SERVED, so the tree has one classification of the table.

CLIs

• Go p202 user prefs get/update and PHP user:prefs:get/update print cb_key: (stored) or (not stored).
• --json, --ndjson and --quiet print the API's answer unchanged.
• I updated the help text and made no other CLI change.
• The Go help text is deliberately short. A longer Long text moved TestSearchReadsEditVerbsAsUpdate's "set the api url" off config set-url, because the search index reads Long.

Docs: documentation/api/14-users.md, documentation/api/15-system.md, docs/openapi.yaml, docs/cli.md, docs/cli-agent.md, the route comment in api/v3/index.php, and a "Fixed: Security." line in both changelogs (no exploit steps).

Sweep: reads of 202_users_pref that leave the server

Reader	What it serves
GET /users/{id}/preferences, and the answer of PUT (also the result a staged apply of that PUT returns)	Fixed here
GET /system/integrations	Already only secret_stored booleans
Other v3 reads (AdministrationController: keyword, retention, MaxMind, deletion settings; TrackingBaseLookup: domain)	No secret column
Legacy api/v1, api/v2, api/legacy	Read no preferences
MCP	The go-cli catalogue has no user tool; the hosted connector's keys have no users area (AiConnect)
p202 user prefs update	Builds its body from flags only, never from a GET
sync / export / import (Go portableEntities, server SyncEngine)	Carry no users or preferences, so they cannot drop or blank a secret
MysqlUserRepository::getPreferences() (SELECT *)	No caller outside tests
Pages	Out of scope as briefed: Account › API integrations' form values (another item), and many pages reading account 1's Slack webhook to post to Slack

Tests

• tests/User/ServedPreferencesTest (unit and structural):
  • every column of UserTables::usersPref() is on exactly one list, and the lists name only real columns;
  • the known credentials are pinned off SERVED;
  • every column PUT writes is listed;
  • the served shape, through serve() and through UsersController::getPreferences() (mocked mysqli);
  • the stored-state rule;
  • echo and refusal cases.
• tests/User/PreferenceSecretsAreNotServedIntegrationTest (@group integration, scratch DB; also run alone on a freshly created DB):
  • GET serves none of the 9 secrets' bytes;
  • PUT with the JSON-decoded GET body keeps every secret and cb_verified, and answers the body sent;
  • a new cb_key is stored and resets verification, with the others kept;
  • a changed stored state or a changed read-only key writes nothing;
  • only-echoes is a 422.
• tests/Cli/Commands/UserPreferencesCommandsTest and Go TestUserPrefsShowAStoredSecretAsItsState / TestUserPrefsJSONKeepsTheStoredStateTheAPIGave / TestWithStoredStatesLeavesEverythingElseAlone.

Planted removals. Each was backed up to a fresh mktemp file and checked with cmp. Each counts as caught only when the named test fails with FAILURES! naming it, or --- FAIL: for Go. All 12 were caught:

Plant	Caught by
P1: getPreferences serves the raw row	ServedPreferencesTest::testGetPreferencesServesTheRowThroughIt, PreferenceSecretsAreNotServedIntegrationTest::testGetServesNoSecretAndSaysWhichAreStored
P2: serve() serves everything	testASecretIsServedOnlyAsWhetherOneIsStored
P3: cb_key moved to SERVED	testEveryColumnOfTheTableIsServedOrSecret
P4: a SERVED column dropped from the lists	testEveryColumnOfTheTableIsServedOrSecret
P4b: a new column added to usersPref() and classified nowhere	testEveryColumnOfTheTableIsServedOrSecret
P5: echoes dropped unchecked	testWhatComesBackChangedIsRefusedByName, integration testAStoredStateThatIsNotTheRecordsWritesNothing
P6: a stored state handed to the writer	testAGetBodySentBackChangesNothingItHoldsUnchanged, integration testPutWithTheBodyGetAnsweredKeepsEverySecret
P7: updatePreferences skips withoutEchoes	integration testPutWithTheBodyGetAnsweredKeepsEverySecret
P8: the revcontent_user needle removed	PreferenceSecretCoverageTest::testEveryCredentialColumnOfUsersPrefIsRedactedOnApply
P9: Go renders the raw state	TestUserPrefsShowAStoredSecretAsItsState
P10: PHP CLI renders the raw state	UserPreferencesCommandsTest::testAStoredSecretIsShownAsItsState
P11: "" reads as stored	testStoredMeansAValueIsThere, integration testGetServesNoSecretAndSaysWhichAreStored

Verification

.claude/skills/p202-verify/scripts/verify.sh --since origin/1.9.79 --changed (after the commit, with a scratch database for the schema tier):

Scope report
------------
  go         PASS  ran under CI's go1.25.14 via GOTOOLCHAIN (local go is 1.24)
  golangci   PASS  ran under CI's go1.25.14 via GOTOOLCHAIN (local go is 1.24)
  openapi    PASS
  patterns   PASS
  phpcs      PASS
  phpstan    PASS
  schema     PASS
  syntax     PASS
  unit       PASS

Not covered by this script:
  live      stand up an instance, drive the path a user would take
  agent     add and run a case under tests/fixtures/agent-eval/cases/
  ci        a workflow still in_progress has not passed; read the run

The unit tier ran 6095 tests, with 5 skipped: the documented DB-singleton skips, plus the cron test that skips when the checkout has a 202-config.php.

An earlier --changed run on the working tree had one unit failure: VacuousAssertionRuleTest, where PHPStan hit No space left on device writing /tmp/phpstan because the shared disk was nearly full. That is environmental. The test passed when run alone, and the run above is clean.

Integration suites, against scratch DBs p202_9kk7_*: PreferenceSecretsAreNotServedIntegrationTest (5/5) and PreferencesWriteIntegrationTest (5/5).

Live pass: an instance from this branch (php 8.3 -S, MariaDB 10.11), driven by a script:

• Stored the 5 secrets PUT writes through PUT /users/1/preferences. The other 4 (RevContent id and secret, lpo_site_key, lpo_bridge_config) were stored by SQL, because nothing in the tree writes them: the RevContent columns are dead, and LPO pairing goes through the hosted service.
• GET /users/1/preferences, p202 user prefs get 1 (table and --json), p202 user prefs update and bin/p202 user:prefs:get 1 (table and --json): none of the 9 secret values appears in any of the bytes (grepped one by one). Each secret shows as {"stored":true} or (stored).
• Sent the GET body back to PUT: 200, the answer equals the body, and all 9 secrets and cb_verified are unchanged in the DB.
• A body with {"cb_key":{"stored":false}}, a string lpo_site_key and user_id: 2: 422 naming those three, nothing written.
• PUT of a new cb_key: stored in the DB, cb_verified reset, the other secrets kept, and the answer does not contain the new key.
• GET /system/integrations: no secret bytes.

Not run

• No agent-eval case was added or run. No eval case reads preferences, and the hosted connector cannot reach the route.
• The @group instance suites were not run. UserManagementRulesInstanceTest only asserts a refusal on this route.
• CI: not yet observed.

For the reviewer

• PayKickstart, 1.9.79: PayKickstart's product IPN, verified with each campaign's key #268 (1YEB), in flight. It adds paykickstart_ipn_secret_keys and, in getPreferences(), PayKickstartKeys::withoutKeys($row), which serves a count as paykickstart_ipn_keys_stored. 1.9.79 does not have that column yet. Whichever PR merges second must:
  • list the column in ServedPreferences::SECRET; ServedPreferencesTest fails until it is classified;
  • reconcile the two shapes in getPreferences(), which will also conflict textually;
  • make sure a GET body still round-trips: paykickstart_ipn_keys_stored would be a served key PUT refuses as unknown unless it is listed as read-only.
• revcontent_user_id is now a secret, as the brief names it. The staged-apply redactor's earlier comment called it a non-credential account id.
• Unknown-key message. PUT's refusal of an unknown key now lists every accepted name, the read-only ones included, because the accepted set now includes every column GET answers.
• Staging a GET body. A staged PUT of a whole GET body is still refused for staging, because its secret-named keys trip the staging guard. That is unchanged, and it fails in the safe direction.

Found outside this item

• cli/Commands/UserPreferencesUpdateCommand.php describes --user-daily-email as "on/off". The server takes "" or 00-23, so on is refused. Left alone per "no other CLI change".
• 202_users_pref.revcontent_user_id and revcontent_user_secret have no reader or writer in the tree (dead columns since RevContent was removed).

---

Generated by Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-23T21:40:14.351485Z We couldn’t complete this review. Comment @codex review to try again.	354e07c	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

[github-actions]

Code review: CLEAN

For 354e07c: 0 issue(s). The review is encrypted for the maintainers in the review-findings artifact of this run.

[github-actions]

Adversarial review: NON-BLOCKING

For dbf68dd: 3 finding(s). The report is encrypted for the maintainers in the review-findings artifact of this run.

Rewritten on every push; the check fails on BLOCKING.

[tracking202]

@copilot resolve the merge conflicts on this branch.

[Copilot]

@copilot resolve the merge conflicts on this branch.

Merged 1.9.79 into this branch. The only conflict was 202-config/changelogs.json: I kept the base's updated "After an upgrade…" line and this PR's Security line, and the file still parses as JSON. I did not run tests after the merge.