fix(security): the setup wizard's #[NoAdminRequired] removed a layer, not added one

[rubenvdlinde]

Why

Hydra Gates are diff-scoped, and the enablement PR touched only CI files — so gate-9 reported green in ~1.5 s having read no lib/. Re-run against the full tree (gate script ConductionNL/.github@ad00986, ajv installed), origin/development @ ae7659f: 16 failing gates, including gate-9 semantic-auth: 3 attribute-vs-body mismatches — all three SetupController methods.

Measurement note: full-tree, not in-CI. The Hydra Gates job never runs npm ci, and this repo pins hydra-gates-ref: v1.0.1, which predates #147/#148/#149 — so the in-CI figure can legitimately differ.

The defect

All three methods carried #[NoAdminRequired] over a body that calls requireAdmin(). The class docblock defended it explicitly:

Each method carries #[NoAdminRequired] so NC middleware accepts the request, but the body enforces an explicit IGroupManager::isAdmin gate (ADR-005 — do not rely on the SecurityMiddleware default alone)

That reasoning is inverted. #[NoAdminRequired] does not add a check — it removes one. It tells NC's SecurityMiddleware to stop requiring admin. So the body gate was not a second layer; it was the only layer, standing in front of endpoints that

• persist app config including registry_url and the write-only registry_token, and
• run seed-templates, which writes OpenRegister ApplicationTemplate objects in an admin request context.

This is the exact shape gate-9 was built for (decidesk#44).

The fix

#[AuthorizedAdminSetting(AdminSettings::class)] instead. Now:

layer	before	after
NC middleware	admin check disabled	enforces admin before dispatch
method body	requireAdmin()	requireAdmin() (kept)

Effective policy is unchanged for admins and strictly stricter for everyone else. requireAdmin() stays in every body, so the 403 payload a non-admin would see is unchanged for anyone who still reaches it — this is defence in depth as the original comment intended, rather than the single layer it actually produced.

gate-5 (route-auth) stays green: #[AuthorizedAdminSetting] is an accepted auth posture, so all three methods remain explicitly annotated.

Verification

• php -l clean; no test references SetupController.
• Hydra gates, full tree: 16 → 15 failing. gate-9 3 → PASS; no other gate moved (gate-5 still PASS).

Not fixed here (reported separately as a gate bug)

gate-14 route-reachability reports 7 "unrouted" methods in this repo — dashboard#page, dashboard#catchAll, settings#index/create/load, preferences#getPreference/setPreference. All seven are false positives. They are contributed by OCA\OpenRegister\AppHost\Routes::standard(), and gate-14 parses only the literal array in appinfo/routes.php, so it cannot see routes supplied by the shared AppHost builder. Same cause in shillinq (7), doriath (5) and planix (3).

No gate was silenced, baselined, excluded or weakened.

🤖 Generated with Claude Code

---

CI state on this branch (read before merging)

Everything green except one job:

quality / Hydra Gates   FAIL — 1 gate
  [gate-46] spec-anchor-existence: FAIL — 6 unresolved @spec target(s)
quality / Quality Report  FAIL  (depends on the above)

gate-9 and gate-47 now pass. phpstan, phpcs, phpmd, psalm, PHPUnit on all six PHP × NC matrix cells, frontend, licence and security jobs: all pass.

The 6 are pre-existing and are not something this change introduced — they are SetupController's @spec anchors pointing at openspec/changes/openbuild-first-time-setup/tasks.md#task-21|31|41, headings that do not exist. gate-46 is diff-scoped, so any PR touching this file surfaces them.

I fixed them properly (promote the change delta to openspec/specs/first-time-setup/, re-point all six), measured it, and backed it out: the promotion brings 7 new Scenarios under gate-19 e2e-coverage, openbuild has no setup-wizard Playwright spec, and the only ways to close that were an @e2e exclude (a suppression) or seven e2e specs I could not run and verify. Full analysis, the exact anchors, and the measured 15 → 16 trade are in #128.

So this PR is deliberately not merged from my side: the one red job is red because of a file I had to touch, and a human should decide whether to land it ahead of #128 or after.

[github-actions]

Quality Report — ConductionNL/openbuild @ 6261019

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	❌
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
test-l10n		✅
composer			✅	✅ 101/101
npm			✅	✅ 654/654
PHPUnit					✅
Newman					⏭️
Playwright					⏭️
Hydra gates					❌

---

Quality workflow — 2026-08-05 12:45 UTC

Download the full PDF report from the workflow artifacts.

[github-actions]

Quality Report — ConductionNL/openbuild @ e8746f0

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
test-l10n		✅
composer			✅	✅ 101/101
npm			✅	✅ 654/654
PHPUnit					✅
Newman					⏭️
Playwright					⏭️
Hydra gates					❌

---

Quality workflow — 2026-08-05 13:05 UTC

Download the full PDF report from the workflow artifacts.

[github-actions]

Quality Report — ConductionNL/openbuild @ a9cfd1c

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
test-l10n		✅
composer			✅	✅ 101/101
npm			✅	✅ 654/654
PHPUnit					✅
Newman					⏭️
Playwright					⏭️
Hydra gates					❌

---

Quality workflow — 2026-08-05 13:09 UTC

Download the full PDF report from the workflow artifacts.

[github-actions]

Quality Report — ConductionNL/openbuild @ ae43ef0

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
test-l10n		✅
composer			✅	✅ 101/101
npm			✅	✅ 654/654
PHPUnit					✅
Newman					⏭️
Playwright					⏭️
Hydra gates					❌

---

Quality workflow — 2026-08-05 20:08 UTC

Download the full PDF report from the workflow artifacts.

[rubenvdlinde]

Held — blocked by gate-46 on pre-existing @spec debt in the files this PR touches.

Hydra Gates on the PR head is a real, scoped run:

[hydra-gates] Scope: diff vs origin/development — N changed file(s)
[gate-46] spec-anchor-existence: FAIL — unresolved @spec target(s)

The gates are diff-scoped per ADR-020, so touching a file legitimately pulls its existing dangling @spec anchors into scope. That is the intended "you touched it, you own it" behaviour, not a gate malfunction — and it means no base run can excuse it, since the finding is scoped to files only this PR touched.

I also want to be explicit about what I did not do: I did not cite development as "also red". A push to the base branch scopes 0 files and Hydra Gates passes in ~20s having inspected nothing. That is a vacuous green and proves nothing — only a genuinely red base is evidence.

What this needs: repoint the unresolved anchors onto headings that actually exist. The technique is straightforward and worth doing properly — see openregister#2355 for a worked example, including the check that matters:

resolve each @spec anchor against the spec's own headings, stripping the
"Requirement: " prefix, AND include a negative control — the OLD anchor must
come back MISS. Without that control a green result only tells you the
checker is permissive.

That control is the whole point: it is what distinguishes "the anchors resolve" from "my slugifier matches everything". When I first ran it against openregister I got 5 MISS out of 5 — the anchors were fine and my checker was wrong (it was not stripping the Requirement: prefix).

Not adding a waiver, a baseline entry, or a require-full-coverage flip to get past this — each of those converts a gate that is inconvenient once into a gate that is silent permanently.

The underlying quality work in this PR is sound and measured; it is only the anchor debt in the touched files standing between it and a merge.