[knittingcode]

Summary

Adds tests and negative controls to main for protections that had none. No production code changes.

Motivation

Read-only gap analyses of the security tests found regressions that no test on main would catch:

1. A weakened route permission. Changing a tenant route's #[TenantPermission] to a weaker permission passed every test.
2. Missing or non-atomic audit writes for operator commands. For example, removing the audit write from flowcrm:user:enable, or enabling outside the audit transaction, passed every test.
3. Log redaction and pseudonyms (fix(logging): remove query strings from logged URLs #16, fix(security): share the login limits across every spelling of an account #23). A URL key that kept its fragment when the URL had no query, and a failed-login pseudonym computed with a different case folding than the account lookup, both passed every test. Several other protections were tested but had no negative control: the referer key, the values of sensitive keys, database driver messages, and the member list order and total.

Changes

• RouteSecurityTest::testEveryTenantRouteDeclaresItsPinnedPermission: pins the exact permission of every tenant route on main:

  • api_v1_organization → organization.view;
  • api_v1_organization_member and api_v1_organization_members → membership.list.

  Adding a tenant route or changing its permission fails the test, so the map must be updated on purpose.

• AuditTrailTest::testOperatorUserChangesAreAuditedWithTheirAction: disable, enable and reset-password each write user.disabled, user.enabled and user.password_reset. The test checks:

  • actor platform_operator;
  • no actor user, role, organization or IP;
  • empty changes;
  • no password, e-mail or hash in the event.

• AuditTrailTest::testOperatorEnableAndProvisioningWhoseAuditEventCannotBeWrittenChangeNothing: while the audit write fails, these commands store nothing:

  • flowcrm:user:enable;
  • flowcrm:user:create;
  • flowcrm:organization:create;
  • flowcrm:membership:add.

• The existing helper whileAuditFailsFor() now delegates to a new whileAuditFails($when), with no change in behaviour.

• SecurityLoggingTest::testFailedLoginsOfOneAccountShareOnePseudonymWhateverTheSpelling: two failed logins, one with the address as stored and one upper-cased with U+0130, must log the same account_ref. The lookup folds both to the same account, so the log must too.

• JsonLineFormatterTest::testEveryUrlKeyLosesTheFragmentOfAUrlWithoutAQuery: /accounts#search=… under every URL key must lose the fragment.

• Ten new negative controls in controls.php, each next to the related entries:

  • a tenant route weakened to another permission;
  • enabling a user without an audit event;
  • a user enabled outside the audit transaction;
  • URL keys keep their fragment;
  • referer not treated as a URL key;
  • values of sensitive keys logged;
  • database driver messages logged;
  • failed-login pseudonym folds case unlike the user lookup;
  • member list ordered by id only;
  • member list total counts removed members.

• Docs: the BE-18 control count is 48 (main's 38 plus these 10) in authentication.md, ADR-019 and dev-environment.md, which still said 28. The authentication guide's test table names the pinned permissions, the operator commands checked with a failing audit write, and the pseudonym folding. The observability test table names the fragment case.

Testing

• Local, in a disposable Compose project with real PostgreSQL and PHP-FPM:
  • RouteSecurity, AuditTrail, TenantAuthorization and Authentication tests: 41 tests OK.
  • With the permission control applied, the full suite has exactly 1 failure, the new test. Before this change, that weakening went unnoticed.
  • At head d3f93f3:
    • integration suite: OK (274 tests, 2466 assertions);
    • SecurityLoggingTest|JsonLineFormatterTest: OK (68 tests, 479 assertions);
    • run-controls.php: 48 of 48 negative controls detected.
  • The fragment and pseudonym controls were first checked against the code without the new tests, in an independent review's scratch tree, and went undetected there. The tests close those two gaps.
• Unit tests, PHPStan, PHP-CS-Fixer, Deptrac and the architecture tests are clean.
• CI on the exact head is the reference.

Interaction with other PRs

• feat(organizations): membership administration API (FND-010C) #19 adds tenant routes, so the pinned map must gain its three membership routes when both are on main. Whichever merges second applies this resolution (an independent review simulated it):
  • RouteSecurityTest::TENANT_PERMISSIONS: add api_v1_organization_member_add → MembershipAdd, api_v1_organization_member_change_role → MembershipChangeRole, api_v1_organization_member_remove → MembershipRemove, between api_v1_organization_member and api_v1_organization_members. This file merges without a textual conflict, so the pin test fails if it is forgotten.
  • AuditTrailTest.php: take feat(organizations): membership administration API (FND-010C) #19's side (shared helper trait), then pass $disabled->id to whileAuditFailsFor().
  • Control counts in ADR-019, authentication.md and dev-environment.md: count the merged controls.php (grep -c "^ \['") rather than adding the numbers; CI must show N of N. Verified for feat(organizations): membership administration API (FND-010C) #19 at 1565f72 and this branch at b92ba2b: 71 controls, all detected, integration OK (313 tests).
• fix(auth): match no account for an e-mail address with a NUL byte #28 is on main (61c3d94) and merged into this branch (b92ba2b): the documented count is 50 (38 + fix(auth): match no account for an e-mail address with a NUL byte #28's 2 + these 10); the authentication test table keeps both this branch's text and fix(auth): match no account for an e-mail address with a NUL byte #28's console input rule. Locally: 50 of 50 negative controls detected.
• fix(auth): end sessions when a user is disabled #26 and fix(frontend): keep the user signed in when signing out fails #32 are on main (e03d151) and merged into this branch (2ce29c7): the documented count is 53 (main's 43 and these 10). Locally and in CI 40186922451: integration OK (278 tests), 53 of 53 negative controls detected. With feat(organizations): membership administration API (FND-010C) #19 as well, the total is 74 (verified in a scratch simulation of the whole queue on e03d151).
• feat(organizations): membership administration API (FND-010C) #19 also adds controls at the end of controls.php. These ones are inserted elsewhere to keep conflicts small.

Known limitations

• While active, two failing-audit triggers apply to the whole database, because new IDs are unknown in advance. Integration tests run serially, so this is safe today.