Harden API v3 and CLI: fail-closed auth, sync data-loss guards, DRY shared helpers by tracking202 · Pull Request #148 · tracking202/prosper202

tracking202 · 2026-09-05T06:56:25Z

Security fixes:

Auth: malformed API-key scope JSON and failed schema probes now fail
closed (previously either silently granted the full '*' scope); auth
query joins 202_users so soft-deleted users' keys stop authenticating,
and deleting a user now revokes their API keys in the same transaction
RotatorsController::deleteRule verifies the rule belongs to the rotator
before deleting criteria/redirects (rule IDs are global; the unscoped
deletes let one user strip another user's rules)
bulkUpsert validates primary keys strictly instead of binding them as
strings, where MySQL coercion could match and overwrite the wrong row
config:set-key accepts the API key via hidden prompt (parity with the
password handling in user:create), keeping it out of shell history

Sync/data-loss guards:

SyncEngine prune now diffs the target against the FULL source key set;
previously an incremental run (updated_since) with prune enabled would
classify every unchanged target record as "only in target" and delete it
RemoteApiClient no longer converts non-JSON or 3xx success responses
into empty datasets (which downstream diff/prune treated as "remote is
empty"), and pages correctly when pagination.total is missing
Rotator rule re-sync fetches source rules before deleting target rules;
a failed rotator-detail fetch is now an explicit error instead of a
silent "no rules" that a force_update would propagate as rule deletion
canonicalHash/comparableHash throw on json_encode failure instead of
returning a random hash that silently broke idempotency replay and
incremental-sync matching

Concurrency:

ServerStateStore routes all read-modify-write methods (job events,
audit, prune tokens, metrics, spans, rate limits, saveJob) through the
existing locked mutateJsonFile, fixing lost updates, a prune-token
double-spend TOCTOU, and rate-limit undercounting; job cancel flags
survive the worker's whole-file save and are re-checked after execution

Correctness:

v3 index: oversized bodies get 413 (not a misleading 400); header
lookups use RequestContext's case-insensitive normalization
Bootstrap::init exports DB config globals so entry points other than
index.php get a working DB connection
v2 attribution surface: malformed JSON bodies return 400 instead of
being treated as empty payloads; respond_json checks json_encode
AttributionController: weighting_config is validated/encoded explicitly,
duplicate slugs return 409, updates reject empty names/slugs
UsersController: assignRole validates user and role before inserting;
removeRole/deleteApiKey 404 on zero affected rows instead of reporting
a successful revocation that deleted nothing
RotatorsController: createRule rejects scalar criteria/redirect entries
(previously inserted empty rows); update rejects empty names
Reports/LTV: invalid period values are 422s instead of silently meaning
"all time"; campaign currency max_length matches the char(3) column
Checked commit in the shared transaction helper
CLI Config: corrupt config files are an explicit error instead of being
silently emptied on next save; saves are atomic with checked writes
CLI: rotator:rule:create JSON options use strict validation (scalars
were silently dropped server-side, creating rules with no criteria);
campaign required fields match the server schema; delete confirmations
validate configuration before prompting

DRY:

New StatementHelpers trait replaces 8 copies of the prepare/bind/execute
wrappers across the v3 controllers; shared pair-key formula and bulk-row
cap; CLI BaseCommand gains confirmDestructive/collectOptions/
decodeJsonOption/promptHiddenSecret replacing ~15 duplicated blocks

Tested: full PHPUnit default suite (1079 tests, 3767 assertions) green;
php -l on all changed files; CLI smoke-tested via bin/p202 list. PHPStan
could not be run in this environment (its dist is unreachable through the
network proxy).

Co-Authored-By: Claude Fable 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

…hared helpers Security fixes: - Auth: malformed API-key scope JSON and failed schema probes now fail closed (previously either silently granted the full '*' scope); auth query joins 202_users so soft-deleted users' keys stop authenticating, and deleting a user now revokes their API keys in the same transaction - RotatorsController::deleteRule verifies the rule belongs to the rotator before deleting criteria/redirects (rule IDs are global; the unscoped deletes let one user strip another user's rules) - bulkUpsert validates primary keys strictly instead of binding them as strings, where MySQL coercion could match and overwrite the wrong row - config:set-key accepts the API key via hidden prompt (parity with the password handling in user:create), keeping it out of shell history Sync/data-loss guards: - SyncEngine prune now diffs the target against the FULL source key set; previously an incremental run (updated_since) with prune enabled would classify every unchanged target record as "only in target" and delete it - RemoteApiClient no longer converts non-JSON or 3xx success responses into empty datasets (which downstream diff/prune treated as "remote is empty"), and pages correctly when pagination.total is missing - Rotator rule re-sync fetches source rules before deleting target rules; a failed rotator-detail fetch is now an explicit error instead of a silent "no rules" that a force_update would propagate as rule deletion - canonicalHash/comparableHash throw on json_encode failure instead of returning a random hash that silently broke idempotency replay and incremental-sync matching Concurrency: - ServerStateStore routes all read-modify-write methods (job events, audit, prune tokens, metrics, spans, rate limits, saveJob) through the existing locked mutateJsonFile, fixing lost updates, a prune-token double-spend TOCTOU, and rate-limit undercounting; job cancel flags survive the worker's whole-file save and are re-checked after execution Correctness: - v3 index: oversized bodies get 413 (not a misleading 400); header lookups use RequestContext's case-insensitive normalization - Bootstrap::init exports DB config globals so entry points other than index.php get a working DB connection - v2 attribution surface: malformed JSON bodies return 400 instead of being treated as empty payloads; respond_json checks json_encode - AttributionController: weighting_config is validated/encoded explicitly, duplicate slugs return 409, updates reject empty names/slugs - UsersController: assignRole validates user and role before inserting; removeRole/deleteApiKey 404 on zero affected rows instead of reporting a successful revocation that deleted nothing - RotatorsController: createRule rejects scalar criteria/redirect entries (previously inserted empty rows); update rejects empty names - Reports/LTV: invalid period values are 422s instead of silently meaning "all time"; campaign currency max_length matches the char(3) column - Checked commit in the shared transaction helper - CLI Config: corrupt config files are an explicit error instead of being silently emptied on next save; saves are atomic with checked writes - CLI: rotator:rule:create JSON options use strict validation (scalars were silently dropped server-side, creating rules with no criteria); campaign required fields match the server schema; delete confirmations validate configuration before prompting DRY: - New StatementHelpers trait replaces 8 copies of the prepare/bind/execute wrappers across the v3 controllers; shared pair-key formula and bulk-row cap; CLI BaseCommand gains confirmDestructive/collectOptions/ decodeJsonOption/promptHiddenSecret replacing ~15 duplicated blocks Tested: full PHPUnit default suite (1079 tests, 3767 assertions) green; php -l on all changed files; CLI smoke-tested via bin/p202 list. PHPStan could not be run in this environment (its dist is unreachable through the network proxy). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

Reviewed the non-API legacy tree (tracking202/, 202-config/, 202-account/, 202-cronjobs/) that the earlier pass had scoped out. Security: - Second-order SQL injection in the report query builder (202-config/functions-tracking202.php): six user-settable user_pref_* filter IDs (country/region/isp/device/browser/platform) were interpolated UNQUOTED with only real_escape_string, which does not neutralize a quote-free payload like "1 OR (SELECT ...)". A low-privilege user could set these via set_user_prefs.php and read across tenants on any report load. Cast all six to (int). - Broken access control: administration.php, auto-upgrade.php and auto-upgrade-premium.php gated only on login while their Settings nav link and every sibling page require a role permission. Any authenticated role could trigger install-wide settings changes, click-data deletion, or a full file/DB upgrade. Gate them on access_to_settings. - Unauthenticated request oracle: 202-account/ajax/validate-apikey.php ran a server-side cURL of attacker-supplied input to the vendor API with no auth, unlike every sibling ajax endpoint. Added AUTH::require_user(). - Stored XSS: rotator rule names were echoed unescaped in tracking202/setup/rotator.php while the rotator name beside them is htmlentities-encoded. Escape it. - CSRF: the change_user_stats202_app_key block in account.php skipped the token check every other mutation block in the file performs; the subid income endpoints (tracking202/update/subids.php, delete-subids.php) had no token scheme at all despite altering reported income. Added session token checks and hidden token fields matching the setup/ pages. Data integrity / robustness: - Double-escaping corrupted stored tracking data: the repository-based recorders (dl.php, record_simple.php, record_adv.php) real_escape_string'd keyword / c1-c4 / UTM / custom-var / ppc-variable values and THEN passed them to repository methods that bind them as prepared-statement parameters, storing literal backslashes (e.g. "men\'s shoes"). Pass the raw values; the $mysql['*_id'] values that feed interpolated SQL keep their escaping. - DataEngine cron race: process_dataengine_job.php claimed a job with a non-atomic read-then-write and no lock, so two overlapping runs could both aggregate the same hour. Made the claim an atomic compare-and-swap (UPDATE ... AND processing='0') with an affected_rows check. - delete-subids.php replaced `or die($db->error)` on the spy-table update (which leaked the raw MySQL error and left 202_clicks updated while 202_clicks_spy was not) with the same log-and-skip handling its sibling update already uses. Tested: full PHPUnit default suite (1079 tests, 3767 assertions) green, including the hot-path redirect/click recorder suites that pin dl.php and the shared click writer; php -l on every changed file. Not covered in this pass (noted for a follow-up): the reporting/analytics screens under tracking202/{Report,analyze,overview,spy,visitors}, and a set of lower-severity legacy items (additional GET-based CSRF paths in account.php/user-management.php/api-integrations.php, the user_id=1 Slack webhook lookup, several unchecked query()->fetch_assoc() robustness spots, and offrtr.php correctness bugs). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

Completes the coverage gap left by the previous two commits: the reporting/analytics screens were reviewed (no SQL injection found — the JSON dispatcher allowlists reportType/order/offset, the report pages build no SQL, and dimension selection picks among hardcoded fragments), and the lower-severity legacy items are now fixed. Reporting screens: - Four Excel exports were broken outright: regions/text_ads/variables and group_overview _download.php declare strict_types=1 and passed grab_timeframe()'s int timestamps to real_escape_string(string), a TypeError. Because Content-Disposition headers are already sent, every default account (user_pref_time_predefined defaults to 'today') download ed an .xls containing a PHP fatal. Added the (string) cast the 11 sibling exports already use; verified the TypeError empirically under PHP 8.4. - Guarded 12 unchecked _mysqli_query() results that were dereferenced immediately (fatal "on bool" when a query fails), using the record_mysql_error() guard three siblings already establish. - The custom-variables report bypassed the access_to_campaign_data masking that all 12 other reports apply, so a restricted sub-user could read full revenue/cost figures. Added a shared masking helper applied to both displayVariableReport() and downloadVariables(). Redirect hot path: - offrtr.php read $rotator_row['maxmind_isp'] but the query never selected it, so ISP lookup silently never ran for ALP rotator hits and ISP-type rotator rules never matched; added the column its sibling rtr.php selects. - offrtr.php read $cloaking_on before initialization in both branches (off.php/rtr.php already initialize it); escaped the redirect URL and campaign name it echoed raw where siblings escape. - off.php assigned $html['aff_campaign_name'] in only one branch, so the cloaked path echoed an undefined key; assigned and escaped both. - rtr.php's ?lpr= path dereferenced a null row when no prior click matched, writing 202_clicks* rows keyed on an empty click_id; falls back to a fresh click id instead. - ipx.php ignored its impression INSERT result and set a p202_ipx=0 cookie on failure; px.php dereferenced a null row and read a cookie without isset. Access control / CSRF: - GET state changes now verify the session token, with the token added to the links that trigger them: user deletion (user-management.php) and DNI network deletion (api-integrations.php). upload.php rendered a token it never verified — now checked. - Stored XSS: DNI network fields (favIcon/name/type/shortDescription/apiKey) echoed unescaped into HTML and attributes; unescaped install log in auto-upgrade-premium.php (auto-upgrade.php already escaped it); HTTP_HOST reflected into a JS string literal in clickservers.php. - The Slack webhook was read via `2up.user_id = 1` on four pages while processApiKeyUpdate() writes it to the acting user's row, so every non-owner's notifications went to user 1's webhook and their own configured webhook was never used. Join prefs to the acting user. Robustness: - daily-email.php built `IN ()` when today produced no campaigns — a syntax error swallowed by the outer catch, silently skipping the entire daily email on any slow day. Guarded on a non-empty id list. - attribution-rebuild.php's window guard was a non-atomic check-then-insert with no lock and no unique key on 202_cronjobs, so overlapping runs could both rebuild the same bucket; added an exclusive non-blocking flock. - Checked the login audit-log execute() in 202-login.php and the mobile login; escaped the API key interpolation in AUTH::is_valid_api_key and int-cast the user id beside it; guarded 11 query()->fetch_assoc() derefs in the account pages; fixed two undefined-variable reads in the appstore. Deliberately NOT changed: account.php's `customers_api_key` GET has no internal link — it is an inbound redirect from the vendor, which cannot know the session token, so adding a token check there would break the key handoff. It needs a different fix (a confirmation step) and is left as-is. Tested: full PHPUnit default suite (1079 tests, 3767 assertions) green; php -l on every changed file; the TypeError fix verified by reproducing the error under strict_types on the installed PHP. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

A multi-agent verification pass re-derived every claim in the three prior commits against the base revision (17d97e4) and found five defects I introduced, plus one pre-existing high-severity bug in code no pass had previously covered. Regressions introduced by the previous commits: - Revert the Slack webhook join on 4 account pages. I had changed `2up.user_id = 1` to join the acting user, reasoning that the write goes to $_SESSION['user_id']. But the webhook is install-wide: the cron reader hardcodes `WHERE user_id = 1` (202-cronjobs/index.php:563) and ~12 other UI sites still join on user 1. My change made 4 pages disagree with ~14 others, silently disabling account-event notifications for every non-first user while their campaign edits still notified. The underlying write/read mismatch is real but needs one coherent decision across all 18 sites, not a partial patch. - rtr.php: the new lpr fallback branch hardcoded keyword_id='0', discarding the keyword resolved just above it, so a first-touch lpr click was attributed to no keyword in the Keywords report. It also never set the plain $click_id, which is read later for the cloaked click_id_public and the {clickid} placeholder — a cloaked lpr click got a 2-character public id pointing at the wrong click. - delete-subids.php: I replaced `or die($db->error)` with a try/catch, but connect.php sets MYSQLI_REPORT_STRICT *without* MYSQLI_REPORT_ERROR, so a failed query() returns false and throws nothing — the catch was dead code. Switched both updates to return-value checks. Also fixed the pre-existing unconditional `$success = true;` after the loop, which overwrote every failure and reported success while the two click tables were out of sync. - process_dataengine_job.php: the new compare-and-swap required affected_rows === 1, but 202_dataengine_job has no PRIMARY or UNIQUE key, so a duplicated window flips two rows and the run bailed out *after* claiming them — with the release UPDATEs below the return, that window became permanently invisible. Accept >= 1. Pre-existing defect (Attribution subsystem, not previously reviewed): - AttributionJobRunner: `$state = &$modelsState[...]` is bound by reference in the batch loop and never unset, so the by-value assignment in the finalise loop writes into the last model's slot. With >= 2 active models the final model finalises another model's state — wrong totals in its audit row, and its snapshot rows re-pointed at the wrong model_id. Added the missing unset(). Tested: full PHPUnit default suite (1079 tests, 3767 assertions) green; php -l on every changed file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

Closes out every defect the adversarial sweep confirmed across the 202-config subsystems that no earlier pass had covered, plus the leftover items in the API layer. Security: - SSRF: attribution export webhooks were never validated. ExportWebhook rejected only the empty string, and the cron did curl_init($url) with no protocol restriction and no IP check, so an authenticated tenant could aim it at internal hosts and use the completed/failed status as a blind oracle. Added Prosper202\Validation\OutboundUrlGuard (https-only, resolvable host, no private/reserved addresses) applied at schedule time AND again at dispatch, with CURLOPT_PROTOCOLS + FOLLOWLOCATION=false. The guard also covers the ranges PHP's filter flags miss (RFC 6598 CGNAT 100.64/10, 192.0.0.0/24, 198.18.0.0/15, multicast) — the LTV webhook guard now routes through it too. - An explicitly-configured empty API-key scope ('[]' or ',') expanded to the '*' wildcard, turning a deliberately-neutered key into full access. The '*' default now applies only on the legacy no-scope-column path. - Rotator public_id was accepted from the request payload with no unique key on the column, while the UNAUTHENTICATED redirect resolves it with no user scoping and then memcaches the result — a chosen collision hijacks another tenant's outbound traffic. Now always derived server-side with a collision check, in both the API controller and the repository. - MysqlRotatorRepository::updateRule/deleteRule replaced criteria and redirects matching on the globally-unique rule_id alone, so they could wipe another rotator's targeting (and re-stamp the rows with the caller's rotator_id). Added the ownership pre-check the InMemory double and the API controller already had, plus two regression tests. - MessagingClient sent the install's customer API key and the user's email with no scheme check; now refuses a non-https MESSAGING_API_URL and pins CURLOPT_PROTOCOLS, matching Lpo\PairingClient. - SetupFormValidator::validateUniqueSlug failed OPEN (a failed query reported "unique") while its two siblings fail closed; it also hardcoded model_slug/model_id for any of 8 allowlisted tables. Correctness: - Six hand-rolled transactions still discarded commit()'s return value (RotatorsController x4, AttributionController, UsersController), so a failed commit returned 204/201 for work that never landed. - MysqlDeviceRepository and INDEXES::get_device_id both targeted 202_devices, which no install path creates; the real catalog is 202_device_models. Repointed both (supplying device_type, which is NOT NULL with no default). - LTV webhook retry arithmetic was off by one: MySQL evaluates UPDATE assignments left to right, so `attempts = attempts + 1` first made the later `attempts + 1` read old+2 — delivery abandoned after 5 of the 6 MAX_ATTEMPTS and the endpoint marked dead a full attempt early. - GET /ltv/predict?by=product projected every large cohort as exactly 0 because productBreakdown() supplies no aov/repeat_rate, inverting the numbers (best-sellers $0, tiny products showing the account average). Cohorts without projection inputs now fall back explicitly with a reason. - Customer merge never repointed 202_offer_recommendations, silently resetting the target's fatigue budget and orphaning the source's rows. Handled with UPDATE IGNORE + cleanup for its UNIQUE constraint. - 202_attribution_exports was created twice with different shapes; the 1.9.58 CREATE ... IF NOT EXISTS was a no-op after 1.9.56, so the export repository selected columns that never existed. Added guarded ALTERs for the five missing columns and backfilled the renamed ones. - DataSeeder ran every statement on `global $db` instead of the installer's connection and discarded all seven return values, so a half-seeded install reported success. Routed through a checked helper. - PartitionInstaller::disableStrictMode set a SESSION variable on the wrong connection (and fatals from CLI with no global $db). - MessagingService hashed a substituted "now" into its synthetic message id, minting a new id every poll and inserting a duplicate row each sync. - administration.php dereferenced an unchecked query result on the line after the guard I added, defeating it. Tested: full PHPUnit default suite (1081 tests, 3772 assertions) green, including 2 new rotator ownership regression tests; php -l on every changed file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

A Prosper202 install serves one company; its "users" are colleagues with different role privileges, not distrusting tenants. The only multi-tenant surface is BETWEEN installs (the sync engine). Earlier commit messages and a few code comments framed three findings as cross-tenant, which overstated them. Correcting the record — no behavior changes, comments only. - Rotator public_id collision (RotatorsController::create, MysqlRotatorRepository::create): reworded from "hijack another tenant's outbound traffic" to what it actually is — two users' rotators within the same install can collide on the public handle the unauthenticated redirect resolves, routing clicks to the wrong record. A correctness/integrity bug, not cross-install traffic theft. The server-side unique derivation remains the correct fix. - Rotator rule child-delete ownership checks (D4/D14): unchanged in code; these guard against deleting the wrong rotator's rule children by the globally-unique rule_id — a data-integrity fix within the install, still worth having. - Export webhook SSRF comment: "on behalf of a tenant" -> "on behalf of a user". The SSRF itself is unaffected by tenancy (the risk is the server reaching internal infrastructure) and the guard stands. Unchanged and still valid at full severity regardless of tenancy: the user_pref_* SQL injection (arbitrary query execution by a low-privilege employee), SSRF, auth scope fail-open, broken access control on admin/upgrade pages (intra-company privilege escalation), stored XSS, CSRF, and every correctness/data-loss fix. Pre-existing "tenant" comments authored in 8e7d6f9 (Conversion/LTV ingest guards) were left as their authors wrote them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

@group

…=product Previously productBreakdown() returned only customer/order/revenue totals, so predict() had no aov/repeat_rate/mrr for a product and projected exactly $0 — which the account-average fallback then inverted, showing best-selling products at $0 while sub-threshold products showed the account average. The earlier commit made that fall back explicitly with a reason; this computes the real per-product inputs so the projection works. - aov and repeat_rate now come from a per-(product, customer) rollup: repeat_rate = share of the product's ordering customers who ordered it >= 2 times; aov = product revenue / product orders. Grouping the events by customer_id is correct because a merged customer's events are repointed to the survivor at merge time. - Per-product subscriber MRR: subscriptions carry no product_id, so a subscription's MRR is attributed to the product(s) it bills for (the products on its events' line items), split EVENLY across the distinct products of a bundle. This is additive — product MRR reconciles to total active MRR for subscriptions that have product line items — and reflects current state (active subscriptions, no report-window filter), matching mrr() and the acquisition breakdown's SUM(c.mrr). A subscription with no product line items cannot be attributed and is omitted; that is documented at the query. Query scoping mirrors the rest of the repository: user_id on every derived table, occurred_at window on the revenue rollup (subscriber state is current-state and intentionally unwindowed), and the existing custom-field customer join. The MRR attribution uses a WITH CTE, consistent with MysqlRecommendationRepository and the project's stated MySQL 8.0+ floor. Validated against a real MariaDB via a new integration test (LtvProductPredictionIntegrationTest, @group integration, skips without a configured test DB): aov, repeat_rate, single-product and even-split bundle MRR (reconciling to the subscription total), canceled-subscription exclusion, time-window scoping, and the end-to-end predict() cohort projection that was the reported bug. repeat_rate carries MySQL's div_precision_increment (4 decimals), same as the acquisition breakdown. Tested: default unit suite (1081) green; LTV integration suite (39 tests, 268 assertions) green against MariaDB 10.11; php -l on changed files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

@group

…pi_key CSRF Two API-surface gaps, both instances of CLAUDE.md pattern 5 (a security measure applied to one path but not its siblings). Deleted users kept working API access on v1 and v2 -------------------------------------------------- An earlier commit hardened api/v3/Auth.php to reject a soft-deleted user's key (INNER JOIN 202_users ... user_deleted = 0) and made the v3 delete endpoint drop the user's 202_api_keys rows. But I never grepped the other API versions, so the fix was incomplete: - api/v1/functions.php getStats() authenticated from 202_api_keys alone. - api/v2/functions.php getAuth() likewise. - api/v2/app.php (attribution routes) likewise. - 202-account/user-management.php — the UI delete path — soft-deleted the user and purged their attribution data but left their API keys in place. So a user deleted through the admin UI was correctly locked out of v3 while retaining full v1/v2 REST access indefinitely. All three auth queries now join 202_users and require user_deleted = 0, and the UI delete revokes the keys, matching UsersController::delete(). customers_api_key applied on GET with no CSRF defence ------------------------------------------------------ 202-account/account.php wrote the account's Prosper202 customer API key straight from a GET parameter. It is an inbound handoff from my.tracking202.com, which cannot carry our session token, so it could not simply be token-checked — any site could <img src="...account.php?customers_api_key=..."> and silently rewrite the key. The GET no longer changes state: it decodes the key, holds it, and renders a confirmation form that submits through the EXISTING token-checked POST handler (update_p202_customer_api_key), which already performs the same validation and write — so the duplicate unprotected write path is gone rather than duplicated. Malformed base64 now reports an error instead of writing an empty key. Also hardened that POST handler: it called validateCustomersApiKey() — a server-side call out to the vendor with the submitted value — BEFORE checking the token, so a forged request could drive outbound traffic even though the write was blocked. The token is now checked first. api/v1 and api/v2 reviewed (~1,700 lines, no prior pass): no SQL injection — report type/column identifiers come from hardcoded switch literals, dates are strictly validated (DateTime round-trip) then mktime()'d to ints before their unquoted interpolation, and cid is ownership-checked via getCampaignID(). Tested: new integration test (tests/Api/DeletedUserApiAccessIntegrationTest, @group integration) exercises all four auth paths against real MariaDB — active key accepted, soft-deleted key rejected everywhere, UI delete revokes. Verified it FAILS against the pre-fix v1 query, so it genuinely pins the bug. Default unit suite 1081 green; LTV integration 39 green; skips cleanly with no DB configured; php -l on all changed files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

Four surfaces that no earlier pass had covered. Four fixes; the rest came back clean and the notes below say why, so the sweep is auditable. Mobile templates - 202-Mobile/mini-stats/202-ministats.php declares strict_types=1 and passed grab_timeframe()'s int timestamps to real_escape_string(string) — a TypeError, not a coercion, so the mobile mini-stats page fataled for every account on the default 'today' time preference. This is a FIFTH instance of the bug fixed earlier in four Excel exports; my previous grep was scoped to the reporting directories and missed 202-Mobile/. Reproduced the TypeError on PHP 8.4 before fixing. Docker / Compose infra - start.sh wrote .env containing the generated MYSQL_ROOT_PASSWORD under the default umask, leaving it world-readable (0644). install.sh already chmods its .env to 0600 — and warns elsewhere that it refuses to make a credential file world-readable — so the two paths disagreed. start.sh now creates the file 0600 before writing. Front-end JS - 202-js/dni.search.offers.tablesorter.js interpolated the DNI network name into $().html(). That name comes from the remote DNI network's API, the same upstream data whose server-side output was escaped earlier in 202-account/api-integrations.php — so a hostile or compromised network name executed here. Now set with .text() and the static spinner markup appended. Go CLI - `p202 config set-key` required the key as a positional argument, putting a bearer credential in shell history and ps output, while `p202 user create` in the same binary already reads passwords with term.ReadPassword. Same inconsistency fixed earlier in the PHP CLI's config:set-key. The argument is now optional and the key is prompted without echoing when omitted. - That prompt reads from a TTY, so a piped key (echo "$KEY" | p202 config set-key) or CI use would have failed with "inappropriate ioctl for device"; it falls back to a plain stdin read when stdin is not a terminal. Came back clean (with reasons): - Docker/Compose: no default DB password (compose fails loudly without one), phpmyadmin is loopback-bound behind an opt-in profile, Apache denies dotfiles so the dev bind mount cannot serve .env or .git, app state dirs live outside the docroot, and the image runs the app as www-data. - Front-end JS: attribution.js uses textContent throughout; tracking-report.js and messenger.js both define an escapeHtml/esc helper and apply it to every interpolated value (message bodies included). The remaining .html(response) calls insert server-rendered fragments — an architectural boundary where escaping belongs server-side, not a JS defect. - Go CLI: go build, go vet and go test all pass; config is written 0600 in a 0700 dir, HTTP clients set timeouts, there is no InsecureSkipVerify, API keys are only ever printed masked, destructive commands confirm, and the one os/exec call uses os.Executable() with an argument slice (no shell). Tested: go build/vet/test green; both set-key forms plus validation exercised against a built binary; PHP suite 1081 green; php -l, bash -n, node --check on all changed files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

Line-by-line review of the Go CLI's shared infrastructure. Each fix has a test that fails without it. config: `config set-key` and `config set-url` were silently no-ops on any install upgraded from a V1 config file. Load() migrated the legacy top-level url/api_key into a profile but left them populated, and Save() re-merged them over the active profile on every write — so the credential the user typed was discarded and the stale one written back. normalize() now consumes the legacy fields exactly once and clears them. When active_profile names a profile the map does not contain, the legacy credential is migrated into that profile rather than dropped. config: `user rotate --update-config` compared against the same legacy cfg.APIKey, which is empty for every profile-based config, so the match never fired and the flag did nothing unless --force-config-update was also passed. It now compares and writes through the resolved profile, and reports which profile it touched. config: ResolveGroup dereferenced nil profile entries, so a config containing `"profiles":{"x":null}` panicked the CLI. Every other accessor already guarded against this. atomicfile: new shared helper for all-or-nothing writes, replacing the os.WriteFile in config.Save and the unflushed write-then-rename in SaveManifestAtomic. os.WriteFile applies its mode only when it creates the file, so a config that already existed as 0644 kept those permissions while holding a bearer token; it also truncates in place and writes through a symlink planted at the destination. syncstate: the sync lock was the mere existence of a lock file, so a sync killed mid-run left the file behind and every later sync for that profile pair failed forever — and the holder pid it recorded was never read by anything. The lock is now kernel-held (flock on unix, LockFileEx on Windows), which the OS releases when the holder dies; the recorded pid is now used to name the holder in the contention message. api: Client mutated baseURL and the capability cache with no synchronization while being shared across the bulk-fetch worker pool — a confirmed data race under -race. Guarded with a mutex. The version string from /api/versions was also interpolated into every request path unvalidated; it is now constrained to digits, so a hostile or buggy server cannot steer request paths. output: --csv rounded float fields to 2 decimals, so an exported 0.288613861 became 0.29 with no indication precision had been lost. Machine-facing output now formats losslessly; the human table keeps its rounding. metrics: appendTimestamp wrote into the caller's Fields map instead of copying it, and duration_ms carried omitempty, so a sub-millisecond operation dropped the field entirely rather than reporting 0. shell: variable previews were truncated on byte boundaries, corrupting multi-byte runes. Also applies gofmt to four files that were already non-compliant. Verified: go build, go vet, go test -race (all packages), and cross-compiles for windows and darwin. The Windows lock path compiles but could not be executed in this environment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

Continuing the line-by-line Go CLI pass, now in the command layer. The single-id delete/update paths never validated their positional argument even though --ids on the same commands enforced numeric IDs via parseIDList. A blank argument was interpolated straight into the request path, so `p202 user delete ""` sent DELETE users/ — a request against the collection, not a record. requireID/validateID now reject blank and non-numeric IDs before anything is sent, for campaign/rotator/conversion/user delete, generated update, and rotator rule-delete. `get` uses requireID only, since it also accepts public IDs which need not be numeric. The same commands had drifted apart on output streams. confirmPrompt already existed and deliberately writes to stderr so scripted output stays clean, but rotator rule-delete, user apikey delete and user apikey rotate hand-rolled the prompt with fmt.Printf to stdout, and six commands printed "Cancelled." to stdout. Piping any of them into a consumer mixed prose into the data stream. All confirmation paths now go through confirmPrompt and report to stderr. Bare fmt.Errorf calls on these input-validation paths became validationError so they exit with the documented validation code rather than the generic one. Tests: 18 new subtests covering both behaviours; 17 of them fail against the previous code. The one that already passed is `user delete`, which was the only path routed through the shared bulkOrSingleDelete helper — the duplication is what let the others drift. Verified: go build, go vet, go test (all packages), gofmt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

…ng fails Third batch of the line-by-line Go CLI pass, covering the response-handling paths. Three commands discarded a json.Unmarshal error and then used the zero value, which turned a malformed response into a confident wrong answer rather than an error: - report optimize-campaign reported a campaign with real traffic as having zero clicks, leads, cost and revenue. - rotator verify reported "fed by 0 tracker(s)". - tracker verify built an empty id list, verified nothing, and reported success. There are now no discarded Unmarshal errors left in non-test code. Roughly thirty callers build their payload with json.Marshal and ignore the error. Rendering the resulting nil printed nothing and exited 0, which reads as "no results" rather than "we could not encode the results". render() now reports an empty payload on stderr, which covers every one of those callers at the single choke point; rowsToJSON and the crosstab path additionally surface the underlying cause. round() reimplemented math.Round by casting through int64. That cast is undefined in Go once the scaled value leaves the int64 range, and it converted NaN or an infinity into an arbitrary finite number — a made-up figure in a metrics column. It now uses math.Round and passes non-finite values through. All call sites use positive decimal places, so the behaviour is otherwise identical. tracker bulk-urls returned on the first per-tracker error and discarded every row already fetched, so one transient 500 threw away the whole listing. It now reports each failure on stderr, renders the rows that succeeded, and exits with the partial-failure code, matching how the bulk deletes already behaved. Interactive shell: - captureStdout now restores os.Stdout through a defer. A panic inside a command left os.Stdout pointing at a closed pipe, silencing every later command in the session. - `$name = <command>` silently left $name unset when the command wrote nothing to stdout, which is the case for every void operation since those report success on stderr. It now says so. - $_ retained the previous command's output after a command that produced none, misreporting stale data as the last result; it is now null. - The JSONL batch writer dropped a record entirely if encoding failed, so a consumer counting lines against commands would misread the run. Verified: go build, go vet, go test -race (all packages), gofmt clean, and cross-compiles for windows and darwin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

comparableEqual marshalled both records and compared the bytes, discarding both errors. On failure each operand was nil, and bytes.Equal(nil, nil) is true — so a pair of records that could not be encoded was reported as identical. For the sync path that reads as "no update needed", which silently drops a real difference. changedFields had the same shape per field. Both now treat an encoding failure as a difference and warn on stderr. Erring toward "changed" surfaces the problem as a redundant update rather than as missing data at the target. Verified: go build, go vet, go test -race (all Go packages), gofmt clean, cross-compiles for windows and darwin, plus the PHP unit suite (1081 tests, 3772 assertions, 8 pre-existing environment skips). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

The CLI had five copies of the delete flow: the shared bulkOrSingleDelete (user, attribution), the generated CRUD delete, and hand-rolled versions in conversion, rotator, and rotator rule-delete. The copies are exactly where the mechanics drifted — the unvalidated-id and prompt-on-stdout bugs fixed in the previous commits existed only in the hand-rolled versions, never in the shared helper. The last two commits made the mechanics identical everywhere, leaving the copies differing only in wording and URL shape. Those become data: a deleteSpec carries urlFor plus noun/plural, the cascade-warning suffixes ("and all its rules"), and the parent-resource context ("from rotator 7"), while runBulkOrSingleDelete owns the mechanics once — id validation before any request, the --ids bulk path with partial-failure accounting and exit code, confirmation and cancellation on stderr. bulkOrSingleDelete stays as the flat-resource wrapper, and deleteArgsValidatorN(base) generalizes the positional-arg rule for nested resources so rule-delete shares that too. All user-visible strings are preserved byte-for-byte, now pinned by TestDeleteWordingIsPreserved (8 subtests covering cascade warnings, parent context, and summaries), alongside the existing message-asserting tests. One deliberate message change: rule-delete's empty --ids error was "--ids requires at least one rule ID" and is now the shared "--ids requires at least one ID". Net -180 lines in the three converted files. Verified: go build, go vet, go test -race (all packages), gofmt clean, cross-compiles for windows and darwin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

…en gates Final tail of the Go CLI review — every remaining unread non-test file is now covered (cmd/forecast.go, cmd/sync.go, cmd/diff.go, cmd/verify.go, internal/forecast/{events,seasonal,forecast}.go, and the smaller commands). Three fixes surfaced, each pinned by a regression test that fails against the previous code: forecast: --seasonal silently swallowed a failed weekpart fetch AND the output metadata reported "seasonal": true for the unadjusted forecast — the flag, not what actually happened. The fallback is now warned on stderr and meta.seasonal reflects whether weights were actually applied. rotator check: the command documents itself as a deploy gate and exits non-zero on configuration issues, but in check-all mode a rotator whose detail fetch failed was silently skipped — a broken rotator could ride an exit code 0 through a deploy. Fetch and parse failures now appear as ERROR rows and count toward the failure exit. sync: comparableHash discarded its Marshal error, hashing the nil bytes of a failed encode — so every unencodable row got the SAME digest, and a changed record could match its stored fingerprint and be skipped by incremental sync. Encode failure now yields no fingerprint and the unchanged-skip requires a non-empty stored hash, erring toward re-sync. tryServerSyncRead's bare .(string) assertions on the profile-connection map (built in another file) became scalarString lookups — panic-proof against a future shape change. Reviewed without changes: internal/forecast (events windowing, clamped month recurrence, seasonal weight building — all careful), export.go pagination, diff.go normalization, report/analytics/ltv/attribution/system/click wrappers, root.go, and the small helpers. Verified: go build, go vet, go test -race (all packages), gofmt clean, cross-compiles for windows and darwin; both new tests confirmed to fail with the fixes reverted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

…asting Master gained #146 (rolling-origin conformal bounds, ensemble and coherent multi-metric forecasting) plus a parallel error-taxonomy sweep. It touched 14 of the files this branch changed; four conflicted. Resolutions: cmd/forecast.go — taken from master wholesale. Master replaced the reports/weekpart API fetch with seasonal profiles learned in-engine from the fetched history, so the defect this branch fixed there (a swallowed weekpart fetch error, with meta.seasonal still reporting true for an unadjusted forecast) no longer has a code path. The fix and its regression test are obsolete and were dropped rather than carried forward against a function that no longer exists. cmd/crud.go, cmd/conversion.go, cmd/rotator.go — kept this branch's consolidated delete runner. Master had only added recovery hints to the five hand-rolled copies, which still carried the defects fixed here (prompts and cancellations on stdout, unvalidated positional ids). Master's hint text is preserved, not discarded: the generic --ids hint moved into runBulkOrSingleDelete, and deleteSpec gained idsHintText so rotator rule-delete keeps the more specific "find them with `p202 rotator rules <rotator_id>`" wording master gave it. Everything else auto-merged, including master's WithHint error API alongside this branch's validationError conversions, and master's client.go changes alongside the mutex added here. Verified after merge: all 13 fixes from this branch still present in the merged tree (checked individually), master's features intact (WithHint, coherent forecasting, conformal bounds); go build, go vet, go test -race across all packages, gofmt clean, windows and darwin cross-compiles, and the PHP suite (1081 tests, 3772 assertions, 8 pre-existing environment skips). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

All six reproduce; each was verified against the code before fixing. Rotator public_id (the damaging one). create() derived public_id server-side and discarded any caller value. But rotators are matched between installs by public_id — diff/sync key them on `pub=<public_id>` and send it in the payload — so the target always assigned a different one: every `p202 sync` re-created every rotator, and remapping trackers' rotator_id failed outright with "unresolvable target foreign key". The hazard the derivation guards against is collision, not caller choice, so a supplied public_id is now honoured when it is free and generated otherwise. Fixed in both the v3 controller and the repository; three regression tests, one of which fails against the previous code. rtr.php ?lpr=. The branch that reuses a prior click set only $mysql['click_id'], never the bare $click_id that the cloaked click_id_public and the {clickid} placeholder are built from. Pre-existing, but this branch edited that hunk and the sibling else-branch added there carries a comment stating exactly this requirement — it simply was not applied to the reuse path. CSRF tokens in URLs. The GET guards added to user-management.php and api-integrations.php put the session token in the query string, where it lands in browser history, Referer headers and access logs — and that token guards every POST mutation in the session, so the leak is worse than the hole it closed. Both deletes are now POST forms with a hidden token, which also stops them being state-changing GETs. MessagingClient. The https-only guard rejected MESSAGING_API_URL=http://127.0.0.1:8787/messaging, which is exactly what mock-server.php and connect.php:86 document for local development; the AJAX endpoints call sync() with no try/catch, so the constructor throw surfaced as a bare 500 instead of degrading. http is now allowed for loopback only — 127.0.0.0/8, localhost and ::1, not RFC1918, which does cross a network. device_type. Two paths inserted new 202_device_models rows with device_type=0, which is not one of the seeded types (1=Desktop, 2=Mobile, 3=Tablet, 4=Bot), so those models dropped out of every `device_type = N` filter permanently — rows are keyed on device_name, so whichever path creates one wins. Both now store a valid type, matching connect2.php's fallback of 1 for an unrecognised device; class-indexes.php also accepts the real type. Synthetic message ids. Dropping created_at from the hash fixed duplicate inserts but left direction|author|body as the only input, so two genuinely distinct messages with the same author and text collided and the second was silently dropped. The hash now includes the message's position in the conversation, which is stable across polls and distinguishes them. Verified: php -l on all ten changed files, PHP suite 1084 tests / 3781 assertions (8 pre-existing environment skips), and go build, go vet, go test clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

Two defects in the conformal/ensemble forecasting merged from master (#146). Both stem from the same trap: NaN compares false against everything, so it slips through range guards silently instead of being rejected. applyProfile applied a seasonal profile through the log1p transform as Log1p(v*w), guarding only that the base value v is positive. A negative weight drives v*w below -1, which is outside log1p's domain, and Log1p returns NaN there. Negative weights are not hypothetical: the package's own exported BuildWeekdayWeights divides a possibly-negative day value by a positive mean. The scaled value is now clamped into the representable range. ensembleWeights then amplified a single NaN into total loss. A non-finite RMSE failed `rmse < best`, so it never set the baseline, and failed `rmse > dropFactor*best`, so it was never pruned — the member was kept and weighted 1/(NaN+eps)^2. normalizeWeights' `sum <= 0` fallback did not fire for a NaN sum either, so every member became NaN/NaN. One unmeasurable fold turned every prediction, bound and quantile into NaN, which serializes straight into the JSON and CSV output. Non-finite RMSEs are now skipped explicitly, and the degenerate-sum guard is written so NaN takes the equal-weights fallback. Three regression tests. Reverting only the applyProfile clamp fails the unit test; reverting both fixes fails the end-to-end test with NaN predictions, confirming the two guards are independent containment rather than one fix written twice. Verified: gofmt, go build, go vet, go test -race across all packages, and windows/darwin cross-compiles. Note on the other review item: composer.json already declares phpunit ^9.5 and phpstan ^2.1 under require-dev, so there is nothing to add — the earlier suggestion to add them was wrong. vendor/ here was installed --no-dev to work around the sandbox proxy, which is why the reviewer found no vendor/bin/phpunit. Installing them still fails: composer needs api.github.com zipballs, which the proxy blocks, and its auth helper cannot use the git path that works for fetch/push. The PHP suite does pass (1084 tests, 3781 assertions, 8 pre-existing environment skips) when run from a phpunit phar. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

…c rename Two of the thirteen findings from the branch review; the rest are still open. config.normalize() migrated a V1 config into a profile hardcoded as "default" while only setting ActiveProfile when it was blank, so a file whose active_profile named anything else ended up pointing at a profile that does not exist. Every command then failed with `profile "prod" not found` — including `config set-key` and `config set-url`, which resolve through EnsureProfile, so the CLI could not repair its own config, and the next Save() persisted the broken pairing. The sibling branch of the same function already created the profile ActiveProfile names; this branch did not. Same shape as the rtr.php miss earlier on this branch: fixed on one side of an if, not the other. Regression test fails against the previous code. atomicfile.Write fsync'd the file's contents but never the parent directory after os.Rename, so the directory entry was not durable and the package's documented all-or-nothing guarantee did not survive a crash: `config set-key` could report success and still leave the old key on disk, and losing the SaveManifestAtomic rename drops the source-to-target id mappings, making the next incremental sync re-create every already-synced record. Added a build-tagged syncDir — fsync on unix, documented no-op on Windows, where a directory handle cannot be opened for synchronisation and MoveFileEx already replaces the entry atomically. Verified: gofmt, go build, go vet, go test across all packages, and windows and darwin cross-compiles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

…olidation Master added scoped API keys, staged writes, delete previews, idempotency keys, an eval harness, PHPStan config with two custom rules, and CI workflows across 101 files. Eight files conflicted. Every resolution below combines both sides rather than picking one; three of them would have silently disabled a feature if resolved the obvious way. api/v2/app.php — both sides rewrote the same auth query. Master switched to SELECT * so a schema without the `scope` column still prepares; this branch narrowed it to SELECT k.user_id and joined 202_users so a soft-deleted user's key stops authenticating. Taking either side alone breaks the other: the caller reads $row['scope'] to enforce attenuation, so the narrow select would have made scope always absent and silently unscoped every key on v2. Resolved as SELECT k.* with the join — wildcard over the key table keeps scope available and keeps the prepare schema-tolerant. api/v3/Auth.php — both sides fixed the same fail-open on an unreadable scope. Master's is better and is the contract its new tests encode: resolve to a named MALFORMED_SCOPE sentinel that satisfies no route, rather than this branch's 500. Taking master's return alone was not enough — this branch's earlier throws on undecodable JSON pre-empted it, which the merged tree proved by failing AuthTest twice. parseScopes now follows master's no-throw contract, keeping the non-scalar guard as a skip so (string) never invents a scope named "Array". go-cli/cmd/{crud,conversion,rotator}.go — master added --dry-run and --staged to each of the five hand-rolled delete copies; this branch had collapsed those copies onto one runner. Kept the consolidation and folded master's features into it once, so the feature exists in a single place rather than five. Every urlFor closure became a plain endpoint field, since each was endpoint+"/"+id, which also lets the spec feed master's renderDeletePreviews/stageDeletes signatures unchanged. Id validation runs before the preview and staging branches: a preview is still a DELETE request, so an invalid id must not reach the server on the paths added to make deletes safer. go-cli/internal/api/client.go — kept master's do -> doWithHeaders delegation and its new methods, with this branch's mutex discipline applied to it. Master's doWithHeaders read c.baseURL unsynchronized at two points, which is the data race this branch fixed; the merged version takes one locked read and derives the version header from it. Audited all three remaining c.baseURL sites as lock-held. api/v3/Controllers/RotatorsController.php — both sides added the same rule ownership check; took master's (LIMIT 1, message naming the rotator). Verified this branch's public_id fix survived elsewhere in the file. go-cli/internal/shell/tokenizer.go — comment only; master's avoids a stray non-ASCII quote. Added 9 subtests covering the consolidated delete sites master's own tests do not reach (rotator, conversion, and the nested rotator rule-delete whose endpoint is built from a parent id, under both --dry-run and --staged), plus the validate-before-preview ordering; the ordering test fails if the checks are reversed. Verified: gofmt, go build, go vet, go test -race, golangci-lint (0 issues), windows and darwin cross-compiles; PHP suite 1199 tests / 4062 assertions with 8 pre-existing environment skips; and PHPStan clean (0 errors) via the phar recipe the new CLAUDE.md documents — including master's BindParamArityRule and ForbidArrowFnByRefCaptureRule over this branch's changes. This is the first run of PHPStan on this work; it was previously blocked by the sandbox proxy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

shell.go — `$name = <command>` reported a successful command with an empty result set as a failure. In the session's default table mode a list with zero rows writes nothing to stdout ("No results." goes to stderr), so an empty capture was indistinguishable from a void operation; under --stop-on-error that aborted the batch. Per CLAUDE.md pattern 7 the fix removes the ambiguity rather than picking a side: an assignment now captures as JSON, where an empty result set is {"data":[]} and a void operation still writes nothing. The original defect stays fixed and both cases are tested. shell.go — activeCommandPath is a package global that PersistentPreRunE re-stamps for every in-process execution, so a failing `p202 shell` reported the last command the batch ran and its hint pointed at that command's --help. Restored across the shell's reset boundary alongside the other session globals, which is the same shape as master's --staged restore. Hints named commands that do not exist: `p202 config get` (three sites), `p202 user roles`, and `p202 rotator rules <rotator_id>` — the last written by master and carried through the merge. All three print help and exit 0, so an agent following the recovery step gets nothing and a script reads the diagnostic as success. Fixed to `config show`, `user role list` and `rotator get`, and closed with a structural test that extracts every backticked `p202 ...` from non-test source and resolves it against the real command tree. It checks 70 hints and fails if a command group is handed a word that is not one of its subcommands, so the next renamed command is caught rather than rotting in a hint string. forecast.go — the "response carried: ..." list came from the parser's own output, which only ever holds the metrics it was asked for, so it named a subset and hid most working choices. It now scans the raw response for forecastable metrics that carried a numeric value. The superseded helper is removed rather than left dead. lock_windows.go — LockFileEx held byte 0, and Windows byte-range locks are mandatory, so a contending process could not read the "pid=..." line and the contention message never named the holder, though the unix test asserts it does. The lock moved to a byte past any content; exclusion is unchanged since every participant locks the same range. Also fixed a pre-existing race in TestTrackerBulkURLs (present on master): the fixture's request counters are written from the bulk-urls worker pool's concurrent handler goroutines with no lock, so `go test -race` failed by scheduling luck rather than by anything the CLI did. Verified: gofmt, go build, go vet, go test -race across all packages, golangci-lint 0 issues, windows and darwin cross-compiles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

…placement Seven fixes, each paired with a check that would catch its recurrence (CLAUDE.md "Closing the loop on mistakes"). Campaign-data masking (202-config/class-dataengine.php) Five report screens each restated the access_to_campaign_data predicate and the list of metrics to hide, and they drifted: - displayPerPPCReport()'s totals row set $campaign['net'] = '?', a key that row never prints, and rendered $campaign['total_net'] unmasked. - maskVariableData() listed only the per-row keys, so the variable report's "Totals for report" line and its excel download printed the install's real click, lead, income, cost and net figures to a user explicitly denied campaign data -- the whole-report numbers, not a single row. The predicate is now campaignDataHidden() and the key list MASKED_METRICS, both named once; every screen masks through maskMetrics($row, $prefix). tests/DataEngine/CampaignDataMaskingTest asserts the permission is tested in exactly one place, that no screen masks a metric by hand, and that totals templates only print prefixed keys; both original defects fail it. Transaction boundaries Under MYSQLI_REPORT_STRICT a failed begin_transaction() or commit() returns false rather than throwing. Fifteen sites ignored one or both -- so the body ran in autocommit, the matching rollback() undid nothing, and a caller told the operation failed still had half of it permanently committed. Checked in the v3 controllers (via a new StatementHelpers::beginTransaction()), MessagingService, generate_tracking_link.php, the three migrations and the 1.9.57 upgrade step. tests/Api/V3/UncheckedTransactionBoundaryTest scans for the shape, alongside the existing UncheckedExecuteTest. SSRF guard placement (error pattern #16, added) ExportWebhook's constructor ran the guard, and that constructor is also ExportJob::fromDatabaseRow()'s hydration path: findPending() maps it over every pending row, so one stored http:// URL -- or one transient DNS failure -- threw out of the first call and stranded the entire export queue on every tick. The guard moved to both write boundaries, where a caller is present to be told and only one request is affected: AttributionService for api/v2 and AttributionController::scheduleExport for api/v3. Webhook connection pinning Both webhook crons pin curl to an address the guard approved, but an unbracketed IPv6 literal makes the CURLOPT_RESOLVE entry unparseable: curl drops the pin, resolves the host itself, and the DNS-rebinding hole reopens with the pinning code still looking correct. Shared as OutboundUrlGuard::curlResolveEntry(), which brackets IPv6, and applied in attribution-export.php, which was not pinning at all. Messaging transport allowlist isSafeTransport() admits http:// for loopback so the bundled mock-server setup works, while CURLOPT_PROTOCOLS was pinned to HTTPS -- the constructor accepted the documented URL and every request then failed with CURLE_UNSUPPORTED_PROTOCOL. allowedCurlProtocols() now derives from the same predicate rather than restating it, so it cannot widen either; a table-driven test asserts the two agree for every URL shape. Rotator repository publicIdIsFree()/assertRuleBelongsToRotator() closed a statement Connection::fetchOne() had already closed (fatal on PHP 8), and read a free-id decision and an ownership check inside a write transaction off a possibly-lagging replica. Both use prepareWrite now, the ownership check takes FOR UPDATE like delete() does, and tests/Api/V3/DoubleStatementCloseTest scans for the double close. Verified: php -l on every changed file, PHPUnit 1238 tests green (8 pre-existing skips), PHPStan clean, go build and go vet clean. Each new check was also run against the reintroduced defect to confirm it fails. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPgfMWAmxYUwJQsi926KAS

chatgpt-codex-connector · 2026-09-05T06:56:38Z

Codex Review Summary

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

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-05T07:05:12.000050Z	`166a3ba`	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.

Copilot

🟡 Changes recommended

StatementHelpers::bind needs to match the repository’s established ref-safe mysqli binding pattern, and go-cli/cmd/conversion.go regresses the CLI’s structured validation error contract.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR hardens Prosper202’s server (API v1/v2/v3 + tracking UI/cronjobs) and Go/PHP CLIs against fail-open auth, data-loss sync/prune behaviors, and several correctness/concurrency edge cases, with a strong emphasis on turning silent failures into explicit errors and adding targeted regression tests.

Changes:

Hardened auth/scope handling, request parsing, CSRF protection, and output escaping across legacy tracking/account endpoints and API v3.
Improved sync/prune safety, atomic writes, and lock/TOCTOU handling in server-side sync state and the Go CLI.
Added/expanded PHPUnit and Go tests to lock in SSRF/pinning behavior, hint validity, race-freedom, and NaN containment in forecasting.

File summaries

File	Description
tracking202/update/upload.php	Adds CSRF token verification and escapes token in the upload form.
tracking202/update/subids.php	Adds CSRF verification and includes CSRF token in the POST form.
tracking202/update/delete-subids.php	Adds CSRF verification, fixes success tracking, and checks mysqli query() return values.
tracking202/static/px.php	Avoids undefined cookie access; handles empty click lookup safely.
tracking202/static/ipx.php	Checks INSERT result before setting cookie to avoid binding “0” ids.
tracking202/setup/rotator.php	Escapes rule_name output to reduce XSS risk.
tracking202/redirect/rtr.php	Fixes missing click_id reuse path and avoids creating junk rows when no prior click exists.
tracking202/redirect/offrtr.php	Fixes undefined $cloaking_on and escapes cloaked redirect HTML output.
tracking202/redirect/off.php	Escapes campaign name and fixes undefined output key in one branch.
tracking202/overview/group_overview_download.php	Adds query error checks and strict string casting for timeframe values.
tracking202/analyze/variables_download.php	Adds query error checks and strict string casting for timeframe values.
tracking202/analyze/text_ads_download.php	Strict string casting for timeframe values.
tracking202/analyze/regions_download.php	Strict string casting for timeframe values.
tracking202/analyze/platform_download.php	Adds query error checks for user prefs fetch.
tracking202/analyze/landing_pages_download.php	Adds query error checks for user prefs fetch.
tracking202/analyze/keywords_download.php	Adds query error checks for user prefs fetch.
tracking202/analyze/isps_download.php	Adds query error checks for user prefs fetch.
tracking202/analyze/ips_download.php	Adds query error checks for user prefs fetch.
tracking202/analyze/device_download.php	Adds query error checks for user prefs fetch.
tracking202/analyze/countries_download.php	Adds query error checks for user prefs fetch.
tracking202/analyze/cities_download.php	Adds query error checks for user prefs fetch.
tracking202/analyze/browser_download.php	Adds query error checks for user prefs fetch.
tracking202/ajax/generate_tracking_link.php	Checks begin_transaction/commit to prevent partial tracker creation.
tests/Validation/OutboundUrlGuardTest.php	Adds SSRF/resolve-entry regression tests (IPv4 preference, IPv6 bracketing, etc.).
tests/Rotator/MysqlRotatorRepositoryTest.php	Adds repository tests for rule ownership checks and public_id behavior.
tests/Messaging/MessagingTransportAllowlistTest.php	Ensures MessagingClient transport predicate matches curl protocol allowlist.
tests/Cli/Commands/CrudCommandsTest.php	Updates CLI test expectations for revised weighting_config JSON error text.
tests/Attribution/AttributionServiceExportTest.php	Avoids DNS dependency in webhook tests and asserts unsafe webhook URLs are rejected at write time.
start.sh	Ensures .env is created with 0600 permissions (umask/chmod).
go-cli/internal/syncstate/state_test.go	Updates lock semantics tests and adds stale-lock + contention diagnostics tests.
go-cli/internal/syncstate/lock_windows.go	Adds Windows LockFileEx-based sync lock implementation.
go-cli/internal/syncstate/lock_unix.go	Adds unix flock-based sync lock implementation (non-blocking).
go-cli/internal/shell/state.go	Truncates variable previews by runes to avoid UTF-8 corruption.
go-cli/internal/output/output.go	Prevents lossy rounding in CSV by adding exact float formatting.
go-cli/internal/metrics/metrics.go	Ensures duration_ms is always present and avoids mutating caller-owned maps.
go-cli/internal/forecast/nan_containment_test.go	Adds regression tests ensuring NaN/Inf do not poison forecasts/weights.
go-cli/internal/forecast/forecast.go	Skips non-finite RMSEs, adds isFinite helper, clamps log1p domain, and hardens normalization against NaN.
go-cli/internal/config/config_test.go	Asserts Save() consumes legacy fields and verifies canonical V2 storage behavior.
go-cli/internal/atomicfile/syncdir_windows.go	Adds Windows no-op syncDir for atomic writes.
go-cli/internal/atomicfile/syncdir_unix.go	Adds unix directory fsync best-effort for atomic writes.
go-cli/internal/atomicfile/atomicfile.go	Introduces atomic file writing helper for config and sync manifest durability.
go-cli/internal/api/client_concurrency_test.go	Adds -race test coverage and version negotiation hardening tests.
go-cli/go.mod	Promotes x/sys to a direct dependency for Windows locking.
go-cli/cmd/verify.go	Treats rotator detail fetch/unmarshal failures as failures and checks JSON parsing.
go-cli/cmd/user.go	Fixes role-list hint, standardizes confirmations, and fixes profile-based config update on API key rotation.
go-cli/cmd/sync.go	Prevents empty hash from satisfying incremental skip; hardens comparableHash and scalarString usage.
go-cli/cmd/shell_semantics_test.go	Adds tests for $_ clearing and stdout restore on panic.
go-cli/cmd/report_optimize.go	Uses math.Round safely and checks json.Marshal errors for output generation.
go-cli/cmd/render.go	Reports nil/empty payload rendering instead of silently emitting nothing.
go-cli/cmd/profile.go	Sends cancellation messaging to stderr to avoid corrupting JSON stdout.
go-cli/cmd/optimize_campaign.go	Makes malformed JSON a hard error instead of reporting all-zero metrics.
go-cli/cmd/numeric_output_test.go	Adds tests for rounding behavior and render() empty-payload reporting.
go-cli/cmd/hint_command_validity_test.go	Structural test to ensure commands referenced in hints actually exist.
go-cli/cmd/gate_accuracy_test.go	Ensures rotator check counts unfetchable rotators as failures.
go-cli/cmd/forecast.go	Improves hint metric discovery by scanning raw response metrics.
go-cli/cmd/diff.go	Treats json.Marshal failures as “changed” and warns instead of skipping differences.
go-cli/cmd/crosstab.go	Returns encoding errors instead of ignoring json.Marshal failures.
go-cli/cmd/conversion.go	Refactors delete to shared helper; adjusts create error handling.
go-cli/cmd/config.go	Adds hidden prompt for API key when omitted and fixes hint text for config test.
go-cli/cmd/cmd_test.go	Fixes handler race in bulk URLs test by synchronizing shared state.
cli/Formatter.php	Throws on json_encode failure instead of printing false/empty output.
cli/Config.php	Makes config load/save strict: no silent emptying; atomic writes; permission hardening.
cli/Commands/UserUpdateCommand.php	DRYs option collection and uses shared hidden-secret prompt.
cli/Commands/UserRoleRemoveCommand.php	Uses shared destructive confirmation helper.
cli/Commands/UserPreferencesUpdateCommand.php	DRYs option collection via BaseCommand helper.
cli/Commands/UserDeleteCommand.php	Uses shared destructive confirmation helper.
cli/Commands/UserCreateCommand.php	Uses shared hidden-secret prompt helper.
cli/Commands/UserApiKeyDeleteCommand.php	Uses shared destructive confirmation helper.
cli/Commands/RotatorUpdateCommand.php	DRYs option collection via BaseCommand helper.
cli/Commands/RotatorRuleDeleteCommand.php	Uses shared destructive confirmation helper.
cli/Commands/RotatorRuleCreateCommand.php	Uses strict JSON option decoding to reject malformed/scalar JSON.
cli/Commands/RotatorDeleteCommand.php	Uses shared destructive confirmation helper.
cli/Commands/ReportWeekpartCommand.php	DRYs query option collection using shared filter param list.
cli/Commands/ReportTimeseriesCommand.php	DRYs query option collection using shared filter param list.
cli/Commands/ReportSummaryCommand.php	Centralizes shared report filter option names as a constant.
cli/Commands/ReportDaypartCommand.php	DRYs query option collection using shared filter param list.
cli/Commands/ReportBreakdownCommand.php	DRYs query option collection using shared filter param list.
cli/Commands/LtvSummaryCommand.php	Centralizes shared LTV query option names as a constant.
cli/Commands/LtvPredictCommand.php	DRYs query option collection using shared LTV param list.
cli/Commands/LtvCustomersCommand.php	DRYs query option collection using shared LTV param list.
cli/Commands/LtvBreakdownCommand.php	DRYs query option collection using shared LTV param list.
cli/Commands/CrudCommands.php	Uses shared destructive confirmation helper.
cli/Commands/ConversionDeleteCommand.php	Uses shared destructive confirmation helper.
cli/Commands/ConfigSetKeyCommand.php	Allows hidden-prompt API key input and inherits BaseCommand helpers.
cli/Commands/BaseCommand.php	Adds shared helpers: confirmDestructive, collectOptions, decodeJsonOption, promptHiddenSecret.
cli/Commands/AttributionModelUpdateCommand.php	Uses strict JSON decoding helper and DRYs option collection.
cli/Commands/AttributionModelDeleteCommand.php	Uses shared destructive confirmation helper.
cli/Commands/AttributionModelCreateCommand.php	Uses strict JSON decoding helper for weighting_config.
cli/Application.php	Updates campaign required fields to match server schema requirements.
cli/ApiClient.php	Rejects non-object JSON responses on success instead of silently treating as empty.
CLAUDE.md	Adds an “error pattern to avoid” entry documenting the hydration-path validation hazard.
api/v3/Support/StatementHelpers.php	Introduces shared mysqli prepare/bind/execute/transaction helpers for v3 controllers.
api/v3/Support/RemoteApiClient.php	Prevents non-JSON/3xx responses from becoming empty datasets; fixes pagination when total is absent.
api/v3/index.php	Returns 413 on oversized bodies and uses normalized header lookup for versioning.
api/v3/Controllers/SystemController.php	Adopts StatementHelpers and avoids re-preparing stats statement in loop.
api/v3/Controllers/SyncController.php	Uses canonical pair key, removes nullable store, and re-checks cancel flag after execute.
api/v3/Controllers/ReportsController.php	Validates period strictly (422) and adopts StatementHelpers.
api/v3/Controllers/LtvController.php	Validates period strictly (422) instead of silently treating typos as “all time”.
api/v3/Controllers/ConversionsController.php	Adopts StatementHelpers.
api/v3/Controllers/ClicksController.php	Adopts StatementHelpers.
api/v3/Controllers/CapabilitiesController.php	Shares max bulk rows with controller enforcement to prevent drift.
api/v3/Controllers/CampaignsController.php	Aligns currency max_length with schema (char(3)).
api/v3/Controller.php	Centralizes bulk row cap, adds strict PK validation in bulkUpsert, and adopts StatementHelpers.
api/v3/Bootstrap.php	Exports DB config variables to globals to support alternate entry points.
api/v3/Auth.php	Normalizes header casing; joins 202_users for soft-delete revocation; fails closed on schema probe and malformed scope JSON.
api/v2/functions.php	Joins 202_users when authenticating to revoke soft-deleted user keys.
api/v2/app.php	Rejects malformed JSON bodies with 400; joins 202_users in key lookup; checks json_encode in respond_json.
api/v1/functions.php	Joins 202_users when authenticating to revoke soft-deleted user keys.
202-Mobile/mini-stats/202-ministats.php	Fixes strict_types TypeError by casting timeframe ints to strings before escaping.
202-Mobile/202-login.php	Checks stmt->execute() return value; logs failure without crashing.
202-login.php	Checks stmt->execute() return value; logs failure without crashing.
202-js/dni.search.offers.tablesorter.js	Prevents XSS by using .text() for remote network name before appending markup.
202-cronjobs/process_dataengine_job.php	Adds atomic “claim” update to avoid double-processing windows.
202-cronjobs/ltv_webhooks.php	Uses shared OutboundUrlGuard pinning logic and conditionally sets CURLOPT_RESOLVE.
202-cronjobs/daily-email.php	Avoids invalid SQL when campaign id list is empty.
202-cronjobs/attribution-rebuild.php	Adds a non-blocking file lock to prevent overlapping rebuild runs.
202-cronjobs/attribution-export.php	Re-validates webhook URLs at dispatch and pins curl to validated IPs; forbids redirects and non-HTTPS.
202-config/Validation/SetupFormValidator.php	Fixes validateUniqueSlug to support multiple tables/columns and fail closed on query errors.
202-config/Rotator/MysqlRotatorRepository.php	Enforces rule ownership on update/delete and improves rotator public_id handling.
202-config/Repository/Mysql/MysqlDeviceRepository.php	Fixes device catalog table usage and supplies required device_type.
202-config/migrations/run_forecast_events_migration.php	Checks begin_transaction/commit results to avoid partial migrations.
202-config/migrations/run_attribution_migration.php	Checks begin_transaction/commit results to avoid partial migrations.
202-config/migrations/run_attribution_migration_standalone.php	Checks begin_transaction/commit results to avoid partial migrations.
202-config/Messaging/MessagingClient.class.php	Enforces safe transport rules and aligns curl protocol allowlist with those rules.
202-config/Ltv/MysqlWebhookRepository.php	Uses OutboundUrlGuard for IP allowlisting and fixes UPDATE assignment ordering bug.
202-config/Ltv/MysqlCustomerCrmRepository.php	Resolves UNIQUE collisions during merge for offer recommendations.
202-config/functions-upgrade.php	Checks begin_transaction/commit and adds missing attribution export columns/backfills.
202-config/functions-tracking202.php	Casts unquoted interpolated ids to int to avoid injection through numeric contexts.
202-config/functions-indexes.php	Extends get_device_id wrapper signature to accept device_type.
202-config/functions-auth.php	Escapes API key before storing and casts session user id to int in UPDATE.
202-config/Database/PartitionInstaller.php	Ensures session sql_mode is set on the correct connection.
202-config/Database/DataSeeder.php	Runs seeds on the installer connection and fails loudly on errors.
202-config/class-indexes.php	Fixes device catalog table and ensures device_type is valid and set on insert.
202-config/Attribution/ExportWebhook.php	Documents why SSRF guard must not run in hydration constructor.
202-config/Attribution/AttributionService.php	Applies SSRF guard at the write boundary for webhook scheduling.
202-config/Attribution/AttributionJobRunner.php	Unsets by-ref loop var to prevent finalization state corruption across models.
202-appstore/index.php	Avoids undefined POST index and initializes $hiddenPart.
202-account/user-management.php	Converts delete to POST with CSRF; revokes API keys on delete; hardens query result handling.
202-account/clickservers.php	Hardens query result handling and safely injects host string into JS.
202-account/auto-upgrade.php	Gates upgrade endpoint behind access_to_settings permission.
202-account/auto-upgrade-premium.php	Gates upgrade behind access_to_settings and escapes install log output.
202-account/ajax/validate-apikey.php	Requires authentication to prevent unauthenticated outbound-request oracle behavior.
202-account/administration.php	Enforces access_to_settings permission and hardens query result handling.

Review details

Files reviewed: 173/173 changed files
Comments generated: 2
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+
+    protected function bind(\mysqli_stmt $stmt, string $types, mixed ...$values): void
+    {
+        // @phpstan-ignore-next-line this IS the ref-safe bind wrapper (analog of Connection::bind); no $this->conn exists, cannot self-route
+        if (!$stmt->bind_param($types, ...$values)) {
+            $stmt->close();
+            throw new DatabaseException('Bind failed');
+        }
+    }
+
+    protected function execute(\mysqli_stmt $stmt, string $message): void
+    {
+        // @phpstan-ignore-next-line this IS the checked-execute wrapper (analog of Connection::execute); no $this->conn exists, cannot self-route
+        if (!$stmt->execute()) {
+            $stmt->close();
+            throw new DatabaseException($message);
+        }
+    }


 		if clickIDStr == "" {
-			return validationError("required flag --click_id (or --click_id_public) is missing")
+			return fmt.Errorf("required flag --click_id (or --click_id_public) is missing")
 		}
 		clickID, err := strconv.Atoi(clickIDStr)
 		if err != nil {
-			return validationError("--click_id must be an integer: %s", clickIDStr).WithHint("Use the internal click id from `p202 click list`, or pass the public id via --click_id_public.")
+			return fmt.Errorf("--click_id must be an integer: %s", clickIDStr)
 		}


chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 166a3baee7

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-05T07:05:09Z

+// no unique key on (cronjob_type, cronjob_time), so two overlapping runs could
+// both pass the check and rebuild the same bucket. Take an exclusive
+// non-blocking file lock first; the loser exits instead of duplicating work.
+$attrLockPath = sys_get_temp_dir() . '/p202-attribution-rebuild.lock';


Scope the cron lock to the Prosper202 instance

When multiple Prosper202 installations run on the same host, this fixed machine-wide path makes unrelated attribution rebuilds share one lock. If they use different service accounts, the first account can create a lock file the other cannot open, permanently failing its cron; if they share an account and their schedules overlap, one exits successfully without processing its database and may repeatedly lose the race. Scope the filename by database host/name, as ServerStateStore::resolveDefaultBaseDir() already does for state under the machine-wide temp directory.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-09-05T07:05:09Z

+			return fmt.Errorf("required flag --click_id (or --click_id_public) is missing")
 		}
 		clickID, err := strconv.Atoi(clickIDStr)
 		if err != nil {
-			return validationError("--click_id must be an integer: %s", clickIDStr).WithHint("Use the internal click id from `p202 click list`, or pass the public id via --click_id_public.")
+			return fmt.Errorf("--click_id must be an integer: %s", clickIDStr)


Return structured errors for invalid conversion IDs

For inputs such as p202 conversion create --click_id abc, these bare fmt.Errorf values have no validation category, so human output regresses from Error [validation]: to an unclassified Error:; the invalid-ID path also loses its specific recovery hint about p202 click list and --click_id_public. Return validationError(...) and retain the actionable hint, as required for Go CLI input failures.

AGENTS.md reference: AGENTS.md:L56-L56

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-09-05T07:05:09Z

 			return "This key's scope does not cover the operation. Use a key with the needed scope, or mint one: `p202 user apikey create <user_id> --scope write` (scopes: *, read, write, <area>:read, <area>:write)."
 		case apiErr.Status == 401 || apiErr.Status == 403:
-			return "Verify your API key: run `p202 config get`, then `p202 config set-key <key>` if it's wrong."
+			return "Verify your API key: run `p202 config show`, then `p202 config set-key <key>` if it's wrong."


Synchronize the documented authentication hint

This changes the emitted authentication hint to the valid p202 config show command, but docs/cli-agent.md lines 91 and 112 and documentation/cli/10-go-cli.md lines 234 and 241 still show p202 config get, which is not a registered config subcommand (show and get-default are). Consequently, agents following the documented error envelope execute a failing recovery command; update both required error-contract documents with this change.

AGENTS.md reference: AGENTS.md:L56-L56

Useful? React with 👍 / 👎.

claude added 22 commits August 16, 2026 02:11

Copilot AI lite review requested due to automatic review settings September 5, 2026 06:56

tracking202 requested a lite review from Copilot and removed request for Copilot September 5, 2026 06:56

Copilot started reviewing on behalf of tracking202 September 5, 2026 06:56 View session

Copilot started reviewing on behalf of tracking202 September 5, 2026 06:57 View session

Copilot AI reviewed Sep 5, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Sep 5, 2026

View reviewed changes

Navigation Menu

Harden API v3 and CLI: fail-closed auth, sync data-loss guards, DRY shared helpers - #148

Harden API v3 and CLI: fail-closed auth, sync data-loss guards, DRY shared helpers#148
tracking202 wants to merge 22 commits into
masterfrom
claude/code-review-robustness-dry-xi33my

tracking202 commented Sep 5, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 5, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Sep 5, 2026

Uh oh!

chatgpt-codex-connector Bot Sep 5, 2026

Uh oh!

chatgpt-codex-connector Bot Sep 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

tracking202 commented Sep 5, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chatgpt-codex-connector Bot commented Sep 5, 2026 •

edited

Loading