fix(rbac): openbuild authorised the automation write, OpenRegister refused it, the user got a 500 (#173)

[rubenvdlinde]

What this is

The fix for #173 — the root cause of the red E2E Tests (Playwright) job on development.

Opened for review, not for merge, per Ruben's instruction: it changes where an authorization boundary lives, which is an architecture decision.

The measurement first

#173 was filed from a reading of the code and said so. Its second comment then reported a CI failure as evidence of a different first blocker. I re-measured everything on a live instance (NC 34, openregister 0.2.17-unstable.36), printing the status code on every probe. One half stands, the other is retracted.

RETRACTED — "a granted editor cannot see the application" (#171, and #173's second comment)

probe	result
GET /apps/openregister/api/objects/openbuild/application as rbac-editor	200, 5 rows — OR multitenancy does not drop the row
GET /apps/openbuild/api/applications as rbac-editor, before any grant	200, [] — correct, permissions was null
same, after granting owners:['user:rbac-owner'], editors:['user:rbac-editor'] on a wizard-created app	1 row for rbac-editor, 1 row for rbac-owner

The grant works, on the wizard path, for both roles. The original claim came from a Playwright locator timing out on an app-picker option — see "the second cause" below for why that locator could never have matched. A locator that finds nothing says nothing about the API underneath it. This retraction is written into tests/e2e/support/appRoles.ts, where the next person will look.

CONFIRMED — openbuild grants the write, OpenRegister refuses it, the user gets a 500

Against a fixture app whose permissions are {owners:['user:rbac-owner'], editors:['user:rbac-editor']}:

caller	call	before
rbac-editor	POST /apps/openregister/api/objects/openbuild/automation	403 "does not have permission to 'create' objects in schema 'Automation'"
rbac-editor	POST /apps/openbuild/api/automations/{uuid}/enable	403 insufficient_permission — correct
rbac-owner	same	500 internal_error "User 'rbac-owner' does not have permission to 'update' objects in schema 'Automation'"

The shape is one write authorised twice, by two layers, in two different vocabularies. openbuild's PermissionResolver says "you are an owner of this application on this version"; OpenRegister then re-litigates the same write against a coarse schema-level group ACL (create/update/delete: ["admin"]) and refuses; withAutomation()'s outer catch (Throwable) turns the refusal into a 500, so it does not even read as a permission problem.

The change

#173's option (b): CRUD moves behind openbuild endpoints that authorise per Application and then write in system context.

• POST /api/automations, PUT /api/automations/{uuid}, DELETE /api/automations/{uuid}. Each is #[NoAdminRequired]; each runs matchesCaller(..., allowAdminBypass: false) before any write or compile side effect.
• update pins applicationSlug/versionUuid to the stored values, so a caller holding a role on application A cannot re-parent B's automation by posting A's slug.
• destroy removes the compiled artifacts before the definition — the reverse order leaves the instance acting on a rule nobody can see or edit any more.
• The internal saves pass _rbac: false, with the reason at each call site.
• AutomationEditDialog.vue and AutomationsPage.vue point at the new endpoints.

The automation schema deliberately stays ["admin"]. That gate is the backstop that makes this controller the only way in for a non-admin, so the boundary is in one place instead of two. Option (a) — widening the schema to authenticated — would let any authenticated user rewrite any automation on any application straight over OR REST with no per-application filter anywhere.

This departs from the ADR-022 default and is documented as such at both the route table and the controller docblock. That is the decision to review.

Verification — negative controls run FIRST

caller	call	result
rbac-outsider	CREATE on draft	403
rbac-viewer	CREATE on draft	403
rbac-editor	CREATE on an app they hold no role on	403
rbac-viewer	DELETE	403
rbac-editor	ENABLE on production	403 — correct, owners only

Then the requirement:

caller	call	before	after
rbac-editor	CREATE on draft	403	201
rbac-editor	ENABLE on draft	unreachable	200, enabled=true
rbac-owner	ENABLE on production	500	200, enabled=true
rbac-editor	UPDATE, body claiming a different app + bogus version	n/a	200, both fields pinned back
rbac-editor	DELETE	403	200

The e2e spec

automations-rbac.spec.ts now builds its fixture rather than assuming one that only ever existed on a developer's box, resets the rows it authors, targets the row it seeded rather than .first(), and reuses globalSetup's stored sessions instead of two interactive logins.

A second, independent cause — and it is fleet-wide

Even once the app existed, getByRole('option', { name: /rbac.?automations.?app/i }) could not match it. NcSelect renders options through NcEllipsisedOption, which splits any label of 10+ characters into two <span>s for middle-ellipsis; the option role computes its accessible name from its contents, so the algorithm inserts a space at the element boundary:

• RBAC Automations App → announced as RBAC Autom ations App
• production (10 chars) → produ ction

That is an upstream @nextcloud/vue accessibility defect — a screen reader reads it the same mangled way. Filed as ConductionNL/.github#350. Matched here via the component's own title attribute; not by loosening the regex, which would hide the defect and risk matching the wrong option in a longer list.

What I did NOT verify, stated plainly

I have not seen both browser tests green in one run. On my local rig — a bind-mounted WSL container — the SPA boot alone is 12.8 s (measured: goto 12760 ms, .automations-page 164 ms, overlay dismissal 4132 ms, row visible 939 ms), and the config's per-test budget is timeout: 30_000. Test 1 (editor authors + enables on a draft version) passed end-to-end through the UI, twice — which is the half that was outright impossible before this change. Test 2 needs two sessions and runs 31–34 s on that rig.

I did not widen the timeout, skip anything, or add mode: 'serial'. CI warms the bundle in ci-seed.sh and is the real verdict. If test 2 is still budget-bound there, the honest follow-up is to make AutomationsPage render before its status calls resolve, or to split the test — not to raise the number.

⚠️ Note when reading this PR's own gate cells: gate-19 is diff-scoped, so a green gate-19 here says nothing about repo-wide @e2e coverage.

Closes #171. Refs #173.

[github-actions]

Quality Report — ConductionNL/openbuild @ 345bb43

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

---

Quality workflow — 2026-08-11 13:49 UTC

Download the full PDF report from the workflow artifacts.

[rubenvdlinde]

Not a review, and not a merge — this PR is Ruben's to land. One CI reading
that is easy to misattribute, recorded here so the next person does not have to
re-derive it.

The red E2E cell on this PR is not this PR's. Its only failure is
tests/e2e/builder-host.spec.ts:44 (detail page must render the seeded body
text), and that same test fails on #175, #176, #179 and this one — four
branches whose diffs do not overlap and none of which touches that file. The
last development run before they were cut (31459739568, 04:50) was
2 failed / 181 passed and did not include it.

The likely input: openbuild's E2E job installs
additional-apps: [{"repo":"ConductionNL/openregister","ref":"development"}] —
a moving ref, which advanced 6 commits between that run and the ~13:50 PR
runs.

The part that matters for this PR: #175, #176 and #179 all still show the two
automations-rbac.spec.ts failures. This branch shows neither — only the
shared one. So the fix here does clear both reds it set out to clear, and the
remaining cell is environmental.

Evidence: gh api repos/ConductionNL/openbuild/actions/jobs/<id>/logs for jobs
93791451153 (this PR), 93818991209 (#176), 93733358066 (#175), 93802145089 (#179).

[rubenvdlinde]

Review note (not a review): this PR's green E2E demonstrates its harness changes, not its RBAC fix

Posting a measurement rather than an opinion, because the fleet-board notes on this repo currently say the opposite and a reviewer reading them would be misled.

The standing claim was: "PR #177 shows only the builder-host failure, i.e. its RBAC fix genuinely clears the two automations-rbac reds." The observation is right. The attribution is not.

What the log shows

gh api repos/ConductionNL/openbuild/actions/jobs/93791451153/logs — 262,498 bytes, HTTP 200 (not a BlobNotFound stub):

1 failed / 68 skipped / 182 passed
✓ automations-rbac … editor authors + enables on a draft version   (14.4s)
✓ automations-rbac … editor gets 403 on production; owner succeeds (13.4s)

Positive controls on the same log: grep -c automations-rbac → 2, grep -c 'Running … tests' → 1, and grep -c cn-support-dialog → 0.

Why that green is not evidence about the PHP change

Neither test ever issues an enable request in the failing state, so no code path lib/Controller/AutomationsController.php touches is exercised by the transition from red to green. On development today the two failures are:

• :124 — dies at line 130 clicking the application combobox, blocked by <div … data-testid-modal="cn-support-dialog" class="dialog__modal cn-support-dialog modal-mask"> subtree intercepts pointer events. That dialog is opened by an HTTP 200 carrying {"value": null} from /api/preferences/support-dialog-seen — openbuild is one of the few apps that actually serves that route, and getPreference() returns 200 with a null value for an unset key, which useSupportDialog reads as "not seen yet". Not an error dialog, not downstream of the 500.
• :152 — dies at line 159 on getByRole('option', {name:/rbac.?automations.?app/i}) with no locator resolved to line at all, i.e. zero candidates. On development rbac-automations-app appears in exactly 3 places, all constants and comments inside the spec itself; ci-seed.sh seeds only hello-world.

This PR fixes both — with suppressSupportDialog(page) and with the fixture-building beforeAll. Those are the two hunks the green cell demonstrates.

A 500 from the RBAC defect would have surfaced as expect(response.status()).toBe(403) → received 500. That string appears in none of the six job logs I read.

What this does and does not imply

It does not argue against the PHP fix — the underlying defect (openbuild authorises per-object, OpenRegister re-litigates against a coarse schema ACL, the outer catch (Throwable) turns the refusal into a 500) was measured directly by HTTP probe with status codes printed, and that measurement stands on its own.

It means only this: a PR that changes the product and the harness in one commit cannot tell you which one turned the cell green. If you want the RBAC fix's evidence, it has to come from the direct probes, not from this E2E cell.

I have not merged, modified or rebased anything here — this PR and #173 are review-only per instruction. Bumping phpcsutils for CVE-2026-65954 went into a separate PR that touches nothing else.

[github-actions]

Quality Report — ConductionNL/openbuild @ c5107fb

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

---

Quality workflow — 2026-08-11 23:42 UTC

Download the full PDF report from the workflow artifacts.

[github-actions]

Quality Report — ConductionNL/openbuild @ 3a4e554

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

---

Quality workflow — 2026-08-12 00:18 UTC

Download the full PDF report from the workflow artifacts.

[github-actions]

Quality Report — ConductionNL/openbuild @ 2e7c297

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

---

Quality workflow — 2026-08-12 07:38 UTC

Download the full PDF report from the workflow artifacts.