Do not fail the resources index when an audit row cannot be written

[66Ton99]

Why

ResourcesIndexController::_logSecretAccesses() turned any failure of the secret_accesses insert into an InternalErrorException(500), taking the whole index response down with it.

That was tolerable while contain[secret] was a rare, deliberate request. It no longer is — both existing clients ask for it on this endpoint:

• the browser extension, in findAllByIdsForShare and findAllForDecrypt;
• the Android client, now on every full refresh, so it can keep a usable local replica offline (see passly_android#5).

So this endpoint serves whole pages of secrets on a routine background path, and a single failing audit insert would deny listing and synchronisation to every client — repeatedly, on every retry, until someone fixed the underlying cause by hand. An auxiliary audit table should not be able to take down the primary read path.

What

Count the failures and report them once per request via Cake\Log\Log::error(), with the count, the user id and the first error message.

Logging per secret would only trade a 500 for a drowned error log: with a broken audit table, a page of a few thousand resources would write one line per secret, on every synchronisation.

Deliberately not changed: SecretsViewController and ResourcesViewController still throw. Those are single-secret reads a human explicitly asked for, where "log or deny" is the right audit posture. The comment in the code says so, to stop this being made uniform later without thinking.

Compatibility

No client-visible change on the success path — the response is byte-identical. The request contract (contain/filter whitelist), serialization and response DTO are untouched; the diff is confined to the catch branch. The only behavioural difference is that a failed audit write now yields 200 with the normal body instead of 500 with none, which no client consumes as a feature.

Trade-off worth recording

A secret can now be delivered without a matching secret_accesses row. That is a deliberate choice of availability over "log or deny" on the bulk path, and it belongs in the administrator documentation alongside the offline-replica caveats.

Testing

• vendor/bin/phpcs --standard=phpcs.xml src/Controller/Resources/ResourcesIndexController.php — clean.
• vendor/bin/phpstan analyse (project config, level 6) on the changed file — no errors.
• PHPUnit could not be run locally. tests/TestCase/Controller/Resources/ResourcesIndexControllerTest.php errors out in setUp with PROCEDURE test.TruncateDirtyTables does not exist, inside cakephp-test-suite-light's table sniffer, before any application code runs. I confirmed this reproduces identically on a clean checkout of main, so it is an environment issue and not a regression from this change. Cause: the sniffer creates that procedure with a two-statement execute(), which MySQL 8.4 does not run; CI uses MySQL 8.0, and nixpkgs has dropped 8.0 as EOL. CI on this PR runs the real suite against MySQL 8.0, MariaDB 11.5 and PostgreSQL 13.

No test is added: there is no existing coverage asserting the 500, and provoking an audit-insert failure would mean mocking the association purely to observe a log line.

🤖 Generated with Claude Code