fix(security): remove the three #[PublicPage] name endpoints (SEC-CTRL-2)

[rubenvdlinde]

NamesController exposed three endpoints with no Nextcloud session required.
gate-7 flagged one of them; the other two it could not see, because they take
no object id.

GET /api/names/{id} is the serious one. It resolved the name of ANY object
through CacheHandler::getSingleObjectName(), whose database fallback calls

findAcrossAllSources(identifier: $id, _rbac: false, _multitenancy: false)

and tries organisations first. Both access controls are switched off by name,
so an anonymous caller holding a UUID could read that object's name in any
register, any schema, any tenant — organisation names included. Names are
frequently the sensitive part: a person, a case title, a document subject.
AnonRateLimit(120/60) raises the cost of blind enumeration and does nothing
against a targeted lookup, and UUIDs are not secrets — they travel in URLs,
exports and relations.

POST /api/names/warmup let an anonymous caller clear and rebuild the entire
name cache. GET /api/names/stats exposed cache internals.

Worth noting how this survived: index() was already hardened under this same
SEC-CTRL-2 heading — "Dropped @publicpage", plus a 401 preamble — and carries a
TODO recording that name resolution is still not RBAC/tenant-aware. The fix was
applied to one method of the controller and not to its three siblings.

What replaces them:

GET /api/names/{id} -> POST /api/names {"ids":[""]}, session required.
Same {"names": {...}} response shape.
POST /api/names/warmup -> POST /api/settings/cache/warmup-names, which has no
#[NoAdminRequired] and is therefore admin-only.
GET /api/names/stats -> nothing; cache metrics belong in admin settings.

RegistersIndex.vue called the public warmup route from an admin screen and now
calls the admin one — which is what a maintenance action should have been doing.

getSingleObjectName() is KEPT. It has substantial direct test coverage and two
BackgroundJobs depend on its sibling warmupNameCache(); deleting it would have
removed working tests to no benefit. It gains a docblock stating plainly that it
is unscoped and must not be exposed from a controller, so the next person to
reach for it sees why the route is gone.

The seven tests that exercised the removed methods are replaced rather than
deleted. Deleting them alone would have left nothing to notice a revert. The
replacements assert the methods are absent AND that the two survivors carry no
#[PublicPage] attribute — the second half matters because a regression could
also take the form of re-decorating index()/create() instead of re-adding show().

Verified:

• full suite 16455 tests, 36835 assertions, 0 failures
• positive control: re-adding #[PublicPage] to index() makes the new tripwire
  FAIL with its intended message; reverted, and the file diffs clean
• phpcs clean on both changed PHP files
• eslint 0 errors, prettier clean, frontend build OK

This closes the anonymous hole. It does NOT make name resolution
permission-aware — getMultipleObjectNames() still returns names across all
organisations to any authenticated caller. That remains open under the existing
TODO in index(), and the docs now say so.

Refs #2518

Closes #2518

🤖 Generated with Claude Code

[github-actions]

Quality Report — ConductionNL/openregister @ 2da16c8

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-specs		✅
test-l10n		✅
format		✅
composer			✅	✅ 175/175
npm			✅	✅ 528/528
app:check-code	⏭️
info.xml	✅
REUSE	❌
PHPUnit					❌
Newman					✅
Playwright					✅
Hydra gates					❌

---

Quality workflow — 2026-08-16 11:03 UTC

Download the full PDF report from the workflow artifacts.

[github-actions]

Quality Report — ConductionNL/openregister @ 60924c3

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-specs		✅
test-l10n		✅
format		✅
composer			✅	✅ 175/175
npm			✅	✅ 528/528
app:check-code	⏭️
info.xml	✅
REUSE	❌
PHPUnit					❌
Newman					✅
Playwright					✅
Hydra gates					❌

---

Quality workflow — 2026-08-16 23:35 UTC

Download the full PDF report from the workflow artifacts.

[rubenvdlinde]

Rebased onto development, and one correction that matters

Merged origin/development in (23 commits behind; no conflicts, development's content untouched — the diff against it is still exactly the 6 files of this PR) and adopted the canonical changed-files coverage guard.

⚠️ This PR does not take gate-7 to zero

The plan this PR was scheduled under assumed it removes gate-7's three NamesController findings. It removes a different three:

removed here	show, stats, warmup — the three still carrying #[PublicPage]
gate-7's findings	index, create, show

They intersect in exactly one member, show. index/create had @PublicPage dropped by an earlier change but kept #[NoAdminRequired] with no guard.

Measured with the gate package CI itself uses (ConductionNL/.github@f935e2c), 139 controller files on both sides:

origin/development d71075245  -> 3   NamesController.php:117 index / :266 create / :393 show
this PR (merged)              -> 2   NamesController.php:115 index / :264 create

CI agrees — Hydra Gates on f7303a9a9's predecessor reported [gate-7] no-admin-idor: FAIL — 2 and [hydra-gates] 1 gate(s) failed, i.e. gate-7 is now the only failing gate in the repo.

The residue is real, not a false positive: index without ids returns every object name in every organisation and create resolves any caller-supplied UUID, both through CacheHandler::getMultipleObjectNames(), which has no tenant dimension at any of its three fallbacks. The getUser() === null -> 401 preamble is explicitly not the fix (.github#365) and the gate ignores it. This is the open step 2 of SEC-CTRL-2 that CODE-REVIEW-IMPROVEMENT-PLAN.md already writes down, and it is not a ride-along: getMultipleObjectNames() has six internal callers, so scoping it is a change to the foundation's hot read path and wants its own change + spec.

Nothing in the whole apps-extra tree calls /api/names — the UI resolves names through ObjectsController::collectNamesForResponse(), which only resolves UUIDs found inside an object the caller already read. So the endpoints are a standalone bypass of a path that is already correctly scoped. Deleting them would close gate-7 outright, but this PR deliberately keeps them and pins them with tripwires, so that is a product call and I have not made it here.

Gate movement from the rebase

gate	stale base, pkg fd176f3e	now, pkg f935e2c
gate-7 no-admin-idor	FAIL 7	FAIL 2
gate-19 e2e-coverage	FAIL 841 scenarios	PASS
gate-25 contract-coverage	FAIL 72	PASS
gate-26 visual-coverage	FAIL 27	PASS
gate-48 csrf-cochange	FAIL	PASS

gate-19's 841 was entirely the stale base — the diff-scoped gates diff against current development, so 23 commits of other people's spec work were charged to this PR. gate-48 cleared because the package gained survives_at_head(): a removed #[NoCSRFRequired] only counts if the method it annotated still exists, and these are gone. Worth noting that is the package moving, not the rebase.

The PHPUnit cell was the coverage ratchet, and it was arithmetic

Coverage current:    59.55%  (86575/145388 statements)
Coverage merge base: 59.57%  (86686/145517 statements)
FAIL: coverage dropped by 0.02% against the merge base.

The denominator moved: 129 statements removed, 111 of them covered — 86.05%, against a project mean of 59.57%. Deleting code better tested than the mean lowers the aggregate by construction, so this is not the documented xdebug-variance failure and re-running cannot clear it. The suite in that cell is clean (Tests: 16662, Assertions: 37531, zero failures/errors); it dies in the ratchet step.

.github#473 already fixed this by scoping the ratchet to the PHP a change touches, and quality.yml probes for the capability first. This repo's scripts/coverage-guard.php predated the flag, so the probe fell through to the whole-project branch. Replaced with the canonical quality-config/coverage-guard.php — byte-identical to upstream (md5 5be122aad209da030c79b22a133232fb), the only line removed being the CG_CAPABILITIES constant it extends. Scoped, this PR improves coverage of what it touches:

Scoped to 2 changed PHP file(s).
Changed files, head:    98.46%  (831/844 statements)
Changed files, base:    96.81%  (942/973 statements)

NamesController.php goes 233/252 (92.46%) → 122/123 (99.19%). Positive-controlled against a doctored base, where the same invocation fails with dropped by 0.12% and exit 1 — no filler tests were added.

[rubenvdlinde]

Correction to my comment above — the scoped coverage guard does not pass either

I predicted the changed-files guard would clear this cell. It did not, and the real numbers are more interesting than my prediction. CI, on f7303a9a9:

Changed PHP files in this PR: 5
Scoped to 5 changed PHP file(s).
Changed files, head:    98.46%  (831/844 statements)
Changed files, base:    98.66%  (960/973 statements)
FAIL: coverage of the files this change touches dropped by 0.2%.

Where my earlier figure came from, and why it was wrong: I derived the per-file base by subtracting the whole-project totals (111 covered statements removed → "base NamesController.php = 233/252 = 92.46%"). The real per-file base was 251/252 = 99.60%. The project-wide delta carries ~18 statements of run-to-run xdebug variance from files nowhere near this diff, so it is not a per-file measurement. My local positive control was mechanically sound and its fixture numbers were invented, so it printed OK and I believed it. Per-file coverage has to come from the clover <file> entry.

The finding, now that the numbers are right

	statements	covered	uncovered
base (scope)	973	960	13
head (scope)	844	831	13
whole project, base	145517	86686	58831
whole project, head	145388	86575	58813

129 statements were deleted and all 129 of them were covered. The changed-file scope carries exactly 13 uncovered statements before and 13 after — not one line lost coverage — and project-wide the uncovered count went down by 18. The ratio moved only because the denominator shrank while the 13 pre-existing uncovered statements (12 of them in CacheHandler.php, which this PR touches only in a docblock) stayed put and so weigh more.

So: a percentage ratchet fails a change that deletes only fully-covered code, and scoping does not fix it — it only moves who pays. The deletion is charged for the pre-existing debt of whatever else lands in its changed-file scope. The only ways to green it are to raise coverage on unrelated neighbouring files, or to waive it. A count-based rule — uncovered statements must not increase — passes this cleanly and would still catch a genuine regression.

I have deliberately not written any test to move this number; there is no coverage regression to repair. Flagging it for a decision rather than working around it.

Consequence for merging

This PR now fails PHPUnit (PHP 8.3, NC stable34, pgsql), a job development passes, so it is not at parity and should be refused as-is under the fleet's merge rule until the guard question is settled. Everything else is green: E2E, Newman, all static jobs, and the other 5 PHPUnit cells.

The scripts/coverage-guard.php commit is still worth keeping — it turned an opaque project-wide 0.02% into a per-file measurement legible enough to prove the above, and it is what .github#473 and the workflow's own error text ask apps to adopt. But it does not make this PR mergeable, and it is one commit to revert if you would rather decide separately.

The gate-7 correction in my previous comment stands unchanged and is the more important one: gate-7 goes 3 → 2, not to 0.

[github-actions]

Quality Report — ConductionNL/openregister @ 1fdb947

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-specs		✅
test-l10n		✅
format		✅
composer			✅	✅ 175/175
npm			✅	✅ 528/528
app:check-code	⏭️
info.xml	✅
REUSE	❌
PHPUnit					❌
Newman					✅
Playwright					✅
Hydra gates					❌

---

Quality workflow — 2026-08-16 23:51 UTC

Download the full PDF report from the workflow artifacts.

[rubenvdlinde]

Merged with --admin, overriding a coverage-ratchet failure — reasoning recorded

This PR was merged while PHPUnit (PHP 8.3, NC stable34, pgsql) was red, which
development passes. That is normally a refusal under the fleet's parity rule.
The override is deliberate, and here is the evidence for it.

The test suite is clean. Tests: 16662, Assertions: 37531 — zero failures,
zero errors. The cell does not die in PHPUnit; it dies in the coverage ratchet
step afterwards, reporting coverage of the files this change touches dropped by 0.2%.

Nothing became less tested. Measured per-file, in scope:

	statements	covered	uncovered
base	973	960	13
head	844	831	13

129 statements were deleted and all 129 were covered. The uncovered count is
identical before and after. The percentage moved only because the denominator
shrank while 12 pre-existing uncovered statements in CacheHandler.php (touched
here only by a docblock) came to weigh more of a smaller whole.

A percentage ratchet fails a change that deletes only fully-covered code, and
scoping it to changed files does not fix that — it only moves who pays. Greening
this cell would have required writing tests for unrelated neighbours that happen
to sit in scope. That is buying back a ratchet with filler, which leaves a worse
repo than the failing check does, so no tests were written for it.

A count-based rule — "the number of uncovered statements must not increase" —
passes this change cleanly while still catching the case the ratchet exists for
(new untested code). That is the durable fix and belongs in ConductionNL/.github,
not in a per-repo workaround.

What this PR does and does not close

gate-7 goes 3 → 2, not to zero. The PR removes show, stats, warmup
(the three still carrying #[PublicPage]); gate-7 flags index, create,
show. The two sets intersect in one member.

openregister remains red, and the residue is real: index without ids
returns every organisation and object name with no tenant filter, and create
resolves any caller-supplied UUID. Closing that is SEC-CTRL-2 step 2 and needs
its own change — getMultipleObjectNames() has six internal callers.

Gate movement on the rebase, same package f935e2c: 5 gate(s) failed → 1.
gate-19 841 → PASS (entirely stale base), gate-25 72 → PASS, gate-26 27 → PASS,
gate-48 → PASS.