fix(security): wave-3 critical fixes - #669
Open
rubenvdlinde wants to merge 1990 commits into
Open
Conversation
Contributor
Author
|
Auto-merge blocked: unrelated histories between development and main. The security fixes are committed and pushed to Recommended resolution: Admin-merge or manually resolve by rebasing main from development, then merge this PR. Alternatively, a Nextcloud App release tag can be cut directly from |
…come Pipelinq products (ADR-003)' (#182) from refactor/retire-leges into development
…oss-app migration; decision schema is a dependency hub — sequence carefully)
- manifest: translate remaining Dutch page/widget titles (Objections, Appeals, Proposals, Approval routes, Enforcement strategy, Objection advisory committees, Processing time, Deadline monitoring, …) and the entire first-run setup wizard (Welcome / Initialise register / Load example demo data / Done). - CasesOnMapView: build the marker set client-side from each case's GeoJSON `geometry` field (OR's server-side maps-overview does not surface the string-typed geometry here), and give CnMapWidget a center + default OSM basemap so the standalone /map page renders 10 cases instead of a blank pane. - TermijnDashboard: English heading + filter labels (was 'AWB termijnbewaking dashboard' / 'Filter by zaaktype').
The Tenants index columns (name/oin/domain/groupId/isActive) did not exist on the tenant schema (which has displayName/legalName/kvkNumber/status/tier/…), so the 'name' column fell back to the object UUID and the others rendered blank. Point the columns at the real fields with explicit English labels.
New user tutorial: pair two instances, share a whole case type (confidential cases auto-withheld), share a single confidential case on its own, read/edit across the federation with write-back, and automate sharing with a federate-share flow. Cross-links the OpenRegister Federation reference and the map tutorial.
…der-nav # Conflicts: # appinfo/info.xml # src/manifest.json
… chrome, tutorials + Tenants/CaseTypes fixes' (#184) from feat/cases-folder-nav into development
# Conflicts: # src/main.js # src/manifest.d/50-zaakportaal.json # src/manifest.d/60-leverancier.json # src/manifest.json # src/menu-layout.json
Nextcloud 34's app-management page inlines the raw app.svg and recolors it via 'fill: currentcolor' on the <svg> element. A fill carried on <path>/<style>/class beats the inherited value, so the icon rendered white-on-white and vanished from the list. Moving the fill to the <svg> element lets Nextcloud recolor the icon; visuals are unchanged everywhere the icon is shown on a dark/colored surface.
# Conflicts: # lib/Settings/procest_register.json # openspec/specs/supplier-portal/spec.md # src/registry.js
# Conflicts: # package-lock.json # package.json
…026-07-10) # Conflicts: # appinfo/info.xml # package-lock.json # package.json # src/customComponents.js # src/manifest.d/50-besluitvorming.json # src/manifest.d/bezwaar-beroep-cards.json # src/menu-layout.json # src/registry.js # src/views/MyWorkCards.vue
# Conflicts: # src/main.js
… root' (#186) from fix/app-icon-svg-level-fill into development
…logs v6 build fix Nav: drop the vestigial CasesGroup so "Cases" is a single top-level item routing straight to the cases index. Per-case-type filtering already lives on the index folder sidebar, so the group + "All cases" child was redundant. Index columns: fetch the caseType/statusType collections when they are registered but still empty. registerObjectType seeds collections[type] as a truthy empty array, so the old `!collection` guard treated an unfetched type as already loaded and never fired the lookup, leaving Case type / Status cells showing raw UUIDs on the manifest index page. Build: the local nc-vue source now pulls useAppInstaller -> @nextcloud/password-confirmation, which needs @nextcloud/dialogs v6's spawnDialog. Bump @nextcloud/dialogs 3.2.0 -> 6.4.2, add @nextcloud/password-confirmation 5.3.2, alias @nextcloud/dialogs/style.css to dist for the local-lib build, and bump @conduction/nextcloud-vue to beta.174. Default `npm run build` compiles clean again.
…der-nav # Conflicts: # package-lock.json # package.json
…-column labels' (#189) from feat/cases-folder-nav into development
…height - Allowlist the base-map tile hosts in the CSP (relaxCspForMapTiles). Leaflet loads tiles as <img>, and NC's default img-src blocks third-party tile servers. The OSM host only worked incidentally because the optional Nextcloud 'maps' app pushes a default policy — so the map broke on any instance without it. - Declare the Standard / Humanitarian / Terrain basemaps in the cases mapConfig; these mirror the CSP allowlist above (keep the two in step). - webpack: LOCAL_LIB_PATH env override so a build can target another checkout of the nextcloud-vue src (e.g. a worktree on a feature branch) without disturbing the shared sibling checkout. - Bump @conduction/nextcloud-vue to beta.180 (map controls + leaflet.css fix).
…ppHost (gate-64 / ADR-040) (#752) * fix(apphost): register OpenRegister's autoloader before referencing AppHost Nextcloud registers apps in sorted order: OC_App::getEnabledApps() does sort($apps) and Coordinator::registerApps() walks that list calling OC_App::registerAutoloading($appId, $path) and then $app->register() for one app at a time, so every app registers before the PSR-4 prefix of every alphabetically-later app exists. `procest` sorts after `openregister`, so this happens to hold today — by alphabet, not by design. The class_exists() guard in AppHostRegistrar cannot tell 'OpenRegister absent' from 'OpenRegister's prefix not registered yet': both answer FALSE, and both silently skip the entire AppHost engine — health, metrics, preferences, deep links, the SPA page/catch-all, the seven dashboard widgets and the MCP provider. Fix: register OpenRegister's prefix ourselves before the guard. registerAutoloading() touches only the autoloader and is idempotent, so on the current ordering this costs nothing. IAppManager::loadApp() is deliberately NOT used: it marks OpenRegister loaded and calls Coordinator::bootApp(), booting it before its own register() has run. Caught by hydra gate-64 (apphost-autoload-prelude), ADR-040. * fix(apphost): make the prelude branch-free and declare OC_App to psalm Two CI findings on the prelude, both real: 1. psalm UndefinedClass on \OC_App. It is Nextcloud's server-private legacy bootstrap class, absent from nextcloud/ocp, and there is no OCP interface for registering another app's autoloader. Declared as a suppressed referencedClass in psalm.xml, the same way doriath declares it. 2. The coverage ratchet. `return true` after the call plus `return false` in the catch gave the method a branch that NO environment can exercise both sides of — whichever runs, the other is dead in that run — so the class could never reach full line coverage. No caller ever consumed the return value either: what callers depend on is the class_exists() guard that follows the call. The method is now void with a single statement in the try and a comment-only catch, so every executable line runs in every environment. The tests now assert the two things that are actually observable: that control returns to the caller at all (a Throwable escaping would fail the test, and in production would abort the whole register()), and that a second call does not stack another autoloader. phpmd StaticAccess on the new composition-root call is documented on the calling method rather than baselined. * docs(spec): give the load-order prelude its own capability spec The prelude requirement was appended to an existing legacy spec, which pulls every scenario in that file into gate-19's diff scope and demands e2e coverage for scenarios this change never touched. It is also not the same capability: apphost-adoption / skill-requirement-enforcement describe what the wiring DOES, this describes whether the wiring happens at all. Moved to openspec/specs/apphost-autoload-prelude/spec.md, deliberately with no scenarios: both behaviours live in the app-registration phase, which completes before the first request is dispatched, so neither is reachable from a browser, and the absent-OpenRegister path cannot be set up on an instance that needs OpenRegister to serve the app at all. They are asserted in the unit test named in the spec, so no @e2e exclusion is claimed for either. * test: cover the prelude's degraded path, which no instance could reach The coverage ratchet was right and the code was wrong. Clover for scholiq shows it exactly: line 100 (the registerAutoloading call) count=2, line 101 (the catch) count=0. The catch was never entered — because every instance this suite runs on HAS OpenRegister installed, so getAppPath() never throws. The never-rethrow branch, which is the entire reason this class exists, had never once been executed by a test. register() now takes an optional app id. Production callers pass nothing and get 'openregister'; the new test passes an id that cannot resolve, so getAppPath() throws and the catch runs. The literal stays AT the registerAutoloading call site rather than becoming a signature default, so it remains visible to a reader and to hydra gate-64, which reads that call's arguments. The new test asserts something real rather than merely not throwing: a prelude whose app cannot be resolved must leave spl_autoload_functions() untouched.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+32
to
+36
| uses: ConductionNL/.github/.github/workflows/release-beta.yml@main | ||
| with: | ||
| app-name: procest | ||
| channel: dev | ||
| secrets: inherit |
…eBuilder tests (#747) The Coverage Baseline Check on development asks for this: measured coverage improved past the committed baseline (29.6 -> 29.69, +0.09), and the ratchet wants the gain locked in so it cannot silently be given back. The improvement is the StufMessageBuilder suite added while retiring the IV3 report (#740). Raising the floor is the whole point of the mechanism — the earlier attempt in that PR went the other way and was correctly refused by Coverage Baseline Protection.
…ommand injection) (#755) `quality / Security (composer)` is now red on every procest PR: Advisory ID: PKSA-rdkp-vv9z-mjkg CVE: CVE-2026-67434 Title: OS Command injection Affected versions: <3.13.6|>=4.0.0,<4.0.2 Reported at: 2026-08-05T23:53:11+00:00 GHSA-hmqg-cxww-wqhq Worth noting why development's last runs are green: the advisory was published YESTERDAY, and roave/security-advisories is installed as `dev-latest` on each run. So the same lockfile was clean on 2026-08-05 and is vulnerable on 2026-08-06 with no commit in between. Those green runs are not evidence the lockfile is fine — they are evidence of when they ran. The `^3.9` constraint in composer.json already permits the fixed version, so this is a lockfile move only: 1 update, 0 installs, 0 removals, and the diff touches exactly one version string. No composer.json change is needed. Verified after the bump: `phpcs --version` reports 3.13.6, and phpcs still runs and still reports over lib/AppInfo — a linter that silently stopped starting would look exactly like a clean run, which is the trap this repo has hit before (exit 255 in platform_check.php printing no findings).
(#756) npm audit on `development` (measured with --package-lock-only; the Dependabot alert count is computed on the stale default branch `main` and is meaningless here): before: 1 critical, 2 high, 7 moderate, 5 low (15) after: 0 critical, 0 high, 5 moderate, 5 low (10) Bumps: - vitest + @vitest/coverage-v8 1.6.1 -> ^3.2.7 (same version on both). Clears the CRITICAL Vitest UI arbitrary file read/exec (<=3.2.5) and drags vite 5.4.21 -> 7.3.6, clearing the HIGH `server.fs.deny` bypass (GHSA-fx2h-pf6j-xcff, <=6.4.2). 3.2.7 is outside every current advisory range and is one major less disruptive than vitest 4. - @cyclonedx/cyclonedx-npm 4.2.1 -> ^6.0.0. Clears the HIGH shell injection via --workspace (2.1.0 - 4.2.1). - @vitejs/plugin-vue 5.2.4 -> ^6.0.0. Not cosmetic and not optional: v5 declares `peer vite ^5.0.0 || ^6.0.0`, so once vite hoisted to 7.3.6 the tree carried an invalid peer (`npm ls` ELSPROBLEMS) on a plugin vitest.config.js actually requires at line 40. v6 declares `^5 || ^6 || ^7 || ^8`, which makes the single hoisted vite valid for every consumer. Removes the blanket `minimatch: ^3.1.2` override. test-exclude@7, pulled in by @vitest/coverage-v8 3.x, does `const { minimatch } = require(...)` — the v10 named export — so forcing v3's bare-function export yields `TypeError: minimatch is not a function`. Deleted rather than widened to `>=`, which would force v10 everywhere. Verified nothing in the tree now resolves below 3.1.2 (3.1.5, 9.0.9, 10.2.6 present). composer: squizlabs/php_codesniffer 3.13.5 -> 3.13.6 for CVE-2026-67434 / GHSA-hmqg-cxww-wqhq (OS command injection, <3.13.6). Advisory published 2026-08-05, i.e. after this branch was last measured clean. All lockfile operations were done with `npx npm@10.8.2` to match CI (node 20 / npm 10.8.2); local npm 11 prunes optional entries CI needs.
) development already carries the ADR-040 prelude (lib/AppInfo/ OpenRegisterAutoloader.php, its spec, and unit tests) and gate-64 passes there. What it does not have is any assertion that the endpoints the prelude EXISTS FOR actually answer, and that gap is the whole point: the failure this guards is silent by construction. `health#index` and `metrics#index` are not procest classes. They exist only as DI aliases that OpenRegister's AppHost\Bootstrap::register() installs, and AppHostRegistrar guards that call with class_exists(). If the prelude were removed or reordered the guard would answer false, the registration would be SKIPPED, and the app would boot, route and look healthy — smoke.spec.ts passes either way. STATUS CODE IS NOT THE DISCRIMINATOR, AND ASSUMING IT WAS COST A REWRITE. The first version asserted `status < 500` and `status !== 404`. Both pass on a completely broken adoption, because procest's SPA catch-all answers ANY unmatched path with the app shell: GET /api/health -> 200 application/json GET /api/definitely-not-a-route -> 200 text/html <-- measured So a route that stops resolving returns 200 HTML, not 404 and not 500. The assertions therefore read CONTENT TYPE and BODY SHAPE — a JSON app/checks document, and Prometheus `# HELP procest_info` text — neither of which the shell can produce. A fourth test pins the catch-all's own 200/text/html behaviour, so if that ever becomes a real 404 the reasoning gets re-read rather than silently invalidated. Verified earlier today against the running dev instance: 4 passed. Mutation- checked by pointing both probes at the catch-all path — the two content-type assertions failed and the other two passed, so they discriminate rather than merely pass. The instance is down as of this commit, so CI is the re-run. This is the e2e half of what became procest#751; the implementation half of that PR is superseded by what landed on development, and its OpenRegisterAutoloader is the better design (an injectable app id, so the degraded branch is reachable from a unit test).
…es (#757) axe-core sat in `dependencies`, declaring an accessibility *testing* library as an application runtime dependency. Measured: nothing under src/ imports axe-core, and the built production bundle does not contain it — 0 hits for axe own signature rule id `aria-allowed-attr` across the built js/, while that string is present in node_modules/axe-core/axe.min.js (positive control proving the grep can match). What put it in every app manifest is @conduction/nextcloud-vue, which declares axe-core as an OPTIONAL peerDependency. nc-vue does use it, but only in `src/testing/a11y.js` — a testing helper never imported from `src/index.js`, so it never reaches an app bundle. nc-vue own file header states axe-core "is a devDependency" and that consumers wanting the a11y assertion "add axe-core to their OWN devDependencies". This change follows that instruction. The E2E workflow installs axe-core in its own dedicated step, so the a11y gate is unaffected. Not a bundle-size fix; the bundle is byte-identical. It stops a test dependency being declared as production surface (SBOM, `npm ci --omit=dev`, advisory triage). An optional peer is satisfied by a devDependency, so nothing breaks. Verified: npm ci + production build exit 0. The E2E job failed once and then passed on a re-run with no change — flaky, and it tested a prebuilt frontend that this diff cannot alter.
) min-version is enforced at install time, so a 32 floor makes occ app:enable refuse on stable31, which this repo's own CI runs. The e2e seed then fails with "is not installed or enabled". The reason the floor was raised no longer holds: openregister#2372 removed every eager reference to its ContextChat provider, so the class is only loaded behind interface_exists() guards and never read on an older server. openregister#2380 restored its own 28 floor on that evidence.
…ings" (#760) gate-63 (ADR-079) blocks EVERY procest manifest edit, not just one PR. It reports PASS on development only because it SKIPS when the manifest is not in the diff — so the first PR to touch src/manifest.json inherits two failures it did not cause. procest#758 hit exactly that. D1 page 'Settings' is a type:settings page whose id and title claim the platform meaning of Settings, while app configuration belongs at /settings/admin/<app>. D4 a settings-foldout entry labelled 'Settings' inside a foldout button already called Settings — the nav literally renders Settings > Settings. FIXED BY RENAMING, NOT DELETING, and the distinction matters. The gate's WARN suggests deleting the in-app page as a duplicate of lib/Settings/AdminSettings .php. It is not a duplicate: CaseTypesMenu routes to this same page, so it HOSTS the case-type management surface — the one admin-settings.spec.ts asserts renders "its management surface and add control". Deleting it would have removed a live surface to satisfy a lint. D1's own wording allows the safe remedy: "a domain page that happens to be called settings must be RENAMED". page id Settings -> ProcestConfiguration page title Settings -> Configuration section Settings -> Configuration SettingsMenu label Settings -> Configuration (D4) THE ROUTE PATH /settings IS DELIBERATELY UNCHANGED. It is a bookmarkable URL, the gate keys on id/title rather than path, and pages.spec.ts navigates to /index.php/apps/procest/settings three times. A page id IS its vue-router route name, so the rename has real callers: src/views/DoorlooptijdDashboard.vue pushed { name: 'Settings' } from the "Go to Settings" empty-state button — updated, or that button would have navigated nowhere. tests/e2e/navigation.spec.ts targets the gear foldout by testid and still passes: the foldout legitimately says "Settings". Its comment warned about colliding with the SettingsMenu entry, which is precisely the collision D4 describes and this commit removes — comment updated to say so. ALSO FIXES the pre-existing gate-45 finding this pulled into scope: three skeleton loaders in DoorlooptijdDashboard.vue animate indefinitely with no prefers-reduced-motion fallback (WCAG 2.3.3). An indefinite pulse is what someone with vestibular sensitivity sets that preference to avoid. The animation is now dropped under the media query and replaced with a dimmed static state, so a skeleton still reads as "not real data yet" rather than becoming indistinguishable from a loaded card. Verified: ALL 59 APPLICABLE GATES GREEN, all 59 ran; check:manifest passes (58 pages, 0 ajv errors). gate-63's remaining output is the non-blocking duplicate-home WARN, which stands as a genuine question for the PO rather than something to silence by deleting a working screen.
The case detail page had no view of effort at all. This adds an "Hours booked" KPI summing Shillinq's UrenRegistratie.hours for entries whose subject is this case. It works because ConductionNL/shillinq#463 gave UrenRegistratie a subjectApp/subjectId pair, so an hour can finally name the domain object it was worked on; before that there was nothing to filter by. HOURS, NOT MONEY — hydra ADR-081's line, not a limitation of the widget. A domain app CLASSIFIES and must not sum money or hold a ledger-shaped array; that is exactly what the deleted case.kosten field was. Hours are effort, not currency, so a sum of them belongs here. The cost half deliberately does NOT ship with this. UrenRegistratie stores no employer-cost amount: it has recognisedRate, a RateCard snapshot of what is BILLED, not what an hour COSTS the employer. Per ADR-081 the employer cost is hrmq's, now reachable at POST /api/employees/cost-rate (ConductionNL/hrmq#78), and composing hours x that rate is an aggregation that belongs in Shillinq. A widget summing recognisedRate would render a confident, plausible, wrong number — revenue labelled as cost. VERIFIED AGAINST THE RUNNING INSTANCE before writing the widget, because a cross-register read from a procest widget had no precedent here (all 81 existing register references are procest's own): GET /api/objects/shillinq/UrenRegistratie 200, 187 entries GET ...?subjectApp=procest&subjectId=abc 200, 0 results manifest schema metric enum count | sum, with a field 0 results rather than an error is the right answer today: nothing writes subjectApp yet, so every case correctly shows no hours. THE GATES CAUGHT THREE REAL BUGS, all of which would have shipped visibly broken UI: gate-55 the cell I first chose OVERLAPPED case-related, which spans (8,8,4,7) — I had read only the four kpi entries and assumed y=8 was free. Moved to (0,10,4,2), the genuine gap between case-process and initiator, then verified by intersecting EVERY layout pair rather than eyeballing it twice. gate-60 'ClockOutline' is not registered, so it renders with NO icon. There are TWO registries and they disagree — gate-60 reads src/icons.js, gate-55 reads nc-vue's widgetIcons.js. 'ProgressClock' satisfies the first and not the second; 'History' is in both. gate-55 the pre-existing 'Creation' icon on case-assistant had the same defect and sat on this very page, so it came into scope with this edit. Fixed to 'Lightbulb' rather than left for someone else. ALL 59 APPLICABLE GATES GREEN, all 59 ran. check:manifest passes. The manifest round-trips byte-identically through a tab-indented dump, verified before rewriting, so the diff carries no reformatting.
…ally runs (#737) * fix(security): give the two immutability guards a call site that actually runs Both of procest's immutability rules were enforced by nothing. REQ-SUB-007 — `BewijsstukService::assertMutable()` was implemented and unit tested with ZERO production callers (hydra gate-6, orphan-auth). An authorization check that is never invoked is identical to having no check at all (OWASP A01:2021). A bewijsstuk linked to a vaststelling could be edited or deleted freely. REQ-IC-8 — `ChecklistRunImmutabilityListener` was worse than orphaned. It was never referenced by any registrar, so it was never subscribed to any event and never ran; and it declared the POST-persist `ObjectUpdatedEvent`, which OpenRegister dispatches AFTER `updateObjectEntity()` has committed the row, with no surrounding transaction. Even had it been registered, throwing from there could not have undone the mutation it objected to. The fix, for both: subscribe to OpenRegister's PRE-persist, stoppable `ObjectUpdatingEvent` / `ObjectDeletingEvent`. `stopPropagation()` makes MagicMapper raise `HookStoppedException` before anything is written — the same mechanism `LocationBagValidationListener` already uses and documents. This is the reachable enforcement point because the frontend writes through OpenRegister's generic objects API (ADR-022), not through a procest route; there is no bewijsstuk route to guard. `BewijsstukImmutabilityListener` reads the STORED state (`getOldObject()` on update, the entity itself on delete), never the incoming payload — otherwise a caller could clear `immutable` in the same request that mutates the document and walk through the guard. There is a test for exactly that bypass. Proof, not assertion — each test was re-run with lib/ reverted: - revert A (assertMutable has no caller, i.e. the shipped state): the 3 rejection tests fail, the 4 positive controls still pass. - revert B (guard reads the caller payload instead of the stored row): the bypass test fails. - revert C (checklist listener restored to post-persist ObjectUpdatedEvent): the pre-persist rejection test fails. Clean tree: 52/52 green in tests/Unit/Listener. The subsidieverlening-keten spec note is updated to say which half of REQ-SUB-007 now runs and which half still does not; the spec stays `partial` (verifyHash, the archief-trigger and the Docudesk PDF/A handover are still unwired, per the 2026-07-16 decision in procest#229). * fix(quality): satisfy phpmd coupling and phpstan on the immutability guards Two failures my previous commit introduced — phpstan and phpmd were both green on development before it, so these are mine, not pre-existing. phpmd CouplingBetweenObjects: the three new imports pushed `ObjectListenerRegistrar` to 14 dependencies against a limit of 13. Rather than raise the threshold, the immutability registrations move into their own `ImmutabilityListenerRegistrar`, which is what that class's own docblock says should happen ("Subsystem-scoped listeners live in their own registrars") and is the same shape as the bezwaar and workflow registrars. phpstan: `is_array($payload) === false` is always false — `jsonSerialize()` is declared `array`, so the guard was dead code. Removed rather than annotated. No suppression, no threshold change, no baseline entry. Revert control A re-run after the rework: with `assertMutable()`'s call removed the 3 rejection tests still fail and the 4 positive controls still pass. Clean tree: 52/52 in tests/Unit/Listener, gate-6 clean, phpcs/phpmd/phpstan clean on every changed file.
Coverage Baseline Check measured 29.69% on development push run 31074667383 while .coverage-baseline said 29.6, so the job failed on the non-empty git diff left by --update-baseline. This is a genuine improvement, not a wobble: runs 31049337245 and 31041326272 both measured exactly 29.6, so coverage moved with the code in between. Raises the baseline to the measured value, as the job's own error text asks. No threshold weakened, no waiver.
This was referenced Aug 8, 2026
Contributor
Author
|
Blocker: CONFLICTING, and it is now the single remediation path for Recording what depends on it, since three other open PRs now point at this one:
Landing this sync fixes |
…exist (#766) * fix(settings): PUT /api/settings 500s — the routed update() does not exist `Routes::standard()` routes `settings#update` (PUT /api/settings) into this app's controller namespace. OpenRegister's AppHost substitutes its generic controller only when the leaf does NOT ship a class of that name — see `AppHost\Bootstrap::aliasControllerUnlessLeafDefinesIt()`. Procest ships its own `SettingsController`, so the alias is skipped and the generic is never constructed: every method the canonical table routes to `settings#` is owed locally. Procest implemented `index/create/load` and no `update()`. The router matches the URL, the dispatcher reflects the method, and the request dies with a 500 — not a 404. `src/store/modules/enforcement.js::saveLhsMatrix()` sends exactly that request, so saving the LHS matrix has been failing. Fix mirrors `GenericSettingsControllerBase`: `update()` is the canonical write, `create()` is the legacy POST alias that delegates to it. Auth posture is unchanged — both carry the same `#[AuthorizedAdminSetting]` the write already had. Found by a full-tree hydra-gates run (gate-14 route-reachability). CI's diff-scoped run cannot see it: the file has not changed, so it is never in scope. Can-fail proof: `CanonicalRouteMethodContractTest` fails on the pre-fix tree naming `SettingsController::update()` exactly, and passes after. It asserts on each individual method, never on the controller class merely existing, and carries a positive control that fails if the route-table scan matches nothing. The same test deliberately does NOT flag Health/Metrics/Preferences: those classes are absent from lib/Controller on purpose so the AppHost alias binds. That is the other side of the same seam, and creating them would break it. * test(settings): cover the settings write path, clearing the coverage ratchet The fix added one statement to lib/ (`create()`'s delegating return) with no test reaching it, and `Guard coverage baseline` failed exactly as designed: Coverage merge base: 29.85% (13668/45783 statements) FAIL: coverage dropped against the merge base by less than 0.01% — too little to show in the percentage, but a real loss in the counts. merge base 13668/45783 -> head 13668/45784 statements. Covered rather than waived: no baseline edit, no threshold change, no `continue-on-error`. The guard was right — SettingsController had no unit test at all, which is why a routed method could go missing unnoticed in the first place. The tests assert the ITEM: that the write reaches `SettingsService::updateSettings()` with the request's own parameters, and that the response carries the STORED config rather than the submission. A test asserting only "200" or "is a JSONResponse" would pass against a controller that wrote nothing — which is the shape of defect this PR exists to fix. CAN-FAIL PROOF (2 mutations, measured): - `create()` stops delegating and returns an empty success: FAIL — "create() must produce the same written result as update()". - `update()` returns the submission instead of the service's result: 2 FAILs — "must return the config the service actually stored". - restored: OK (3 tests, 6 assertions). A third test pins that an EMPTY submission still reaches the service, because an early return on an empty payload is indistinguishable from a successful no-op write at the call site.
…ed matrix (#767) The floor has been flipped twice this week (#759 raised it, #761 was closed, #762 reverted it) because nothing tied the declared range to the tested one. This raises it to 32 per the fleet-wide alignment on PHP 8.3, and adds the test that makes the next flip impossible to land silently. WHAT I CHECKED RATHER THAN ASSUMED The rationale already sitting in info.xml was stale, and #762 reverted the value while leaving that rationale in place — so the file declared 28 and explained 32. Both the info.xml comment and code-quality.yml claimed "openregister declares min-version=32". Measured today against the canonical repo, ConductionNL/openregister@development declares: <nextcloud min-version="28" max-version="34"/> openregister#2380 undid it. So the openconnector#1172/#1173 rule — min-version must be >= the max of every <app> dependency's floor — imposes NO constraint here: procest declares no <app> dependencies at all, and the app it depends on in practice has a floor of 28. Both stale comments are corrected rather than repeated. #762's own stated premise is also false at this tip. It reverted the floor because "this repo tests stable31"; code-quality.yml pins nextcloud-test-refs to exactly ["stable32"], and the stable31 leg was REMOVED. WHY 32 IS RIGHT ON TODAY'S EVIDENCE 1. Nothing below 32 is tested. stable32 is the only leg, so 28-31 was an advertised App Store range with zero exercise behind it. 2. info.xml declares <php min-version="8.3"/> two lines above. Nextcloud 28 does not support PHP 8.3, so the pair the two lines jointly advertise is not a configuration that can exist. CI MATRIX: unchanged, and now checked. No leg targets NC < 32 — stable32 is the only entry — so there is nothing to drop. max-version stays 34, the fleet-wide value everywhere except openconnector (35). CAN-FAIL PROOF for NextcloudFloorMatrixTest (3 mutations, measured): - floor 32 -> 33: 2 failures — "runs against stable32" and "no CI leg runs at or above it". - matrix ["stable32"] -> ["stable31"] at floor 32: 2 failures, naming stable31. This is literally the state #762 claimed to be in, so the test would have caught that PR. - both restored: OK (3 tests, 25 assertions). It asserts on every individual ref, not on the matrix merely being non-empty, and a separate positive control fails if either scan matches nothing — an unparsed matrix would otherwise make every assertion pass vacuously.
… through an always-skipped test (#765) * test(e2e): stop 18 backend-only scenarios laundering gate-19 coverage through an always-skipped test tests/e2e/spec-coverage/document-zaakdossier.spec.ts carried a test tagged with 18 @e2e anchors whose body ended in an unconditional `test.skip(true, 'Backend-enforced — asserted in PHPUnit/Newman, not UI')`. It reported *skipped* on every run in every environment, while still giving gate-19 the traceability link for all 18 scenarios — REQ-ZAK-001a..c, 002a..d, 003a..d, 007a..b, 008b, 009a..b, 010a..b were recorded as e2e-covered by a test that never executed. Those scenarios are genuinely backend-only (service guards, HTTP status contracts, Range framing, a repair step), so their traceability moves to the mechanism gate-19 provides for exactly this: a reason-bearing `@e2e exclude` on each scenario in the spec, naming the PHPUnit/Newman suite that does assert it. The placeholder test is removed. Three further tests ended in an unconditional `test.skip(true, 'Requires a seeded case fixture …')`. The blocker is real but permanent — nothing in this repo seeds a dossier fixture for Playwright — so they become `test.fixme` referencing #764 rather than skips that read as transient. Net: no scenario loses its traceability (18 excluded with a written reason, 10 still tagged by this file), and the suite no longer reports coverage from a test that cannot run. Refs #764 * test(e2e): TEMPORARY sentinel — positive control that the E2E job can report red Removed in the next commit. * test(e2e): remove the temporary sentinel Positive control done: with the sentinel present the E2E job reported 1 failed / 87 passed / 38 skipped and concluded 'failure', so the 87 passed / 0 failed / 38 skipped result on this branch is a real verdict rather than a job that cannot go red.
* fix(e2e): add a globalTimeout under the 45m CI cap Fleet-wide Playwright instrument sweep, ConductionNL/.github#188. Neither change can alter a verdict; both change whether you can see why a verdict happened. This repo's `trace` is already `retain-on-failure`, so only the timeout half applies here. No repo in the fleet set `globalTimeout`. The shared quality.yml Playwright job is `timeout-minutes: 45`, and a job cancelled by that cap produces no verdict and no artifacts: the trace upload is `if: failure()` and the report upload is `if: always()`, and neither runs on a cancelled job, while `gh pr checks` still renders it as "fail". Runs cancelled at ~45m16s have been observed in this fleet. Measured overhead in that job before the `Run Playwright tests` step starts is 2.0-2.4 min, so 38m leaves ~7 min of margin while guaranteeing a tally and its artifacts. * fix(e2e): apply the same fix to the config CI actually loads The shared quality.yml resolves its config as `${playwright-test-path}/playwright.config.ts` and only falls back to the app-root `playwright.config.ts` when that file is absent (quality.yml ~L2218). This repo ships tests/e2e/playwright.config.ts, so THAT is the file every CI run has been using — the app-root config fixed in the previous commit is the one developers load by hand, not the one the gate reads. Applies the identical `retain-on-failure` + `globalTimeout: 38 * 60_000` change here. ConductionNL/.github#188.
…ects (#770) Two tools in the same pipeline gave OPPOSITE instructions about where an `@spec` tag should point, and following the one that runs FIRST manufactured findings for the one that runs SECOND. `SpecTagSniff` runs as a blocking `PHP Quality (phpcs)` job and told every developer, in its file docblock and in its own warning text: @SPEC openspec/changes/{change-name}/tasks.md#task-N A change directory is temporary by definition — completing a change moves it to `openspec/changes/archive/<date>-<name>/`, and renaming or dropping one removes the target outright. Every tag written to that instruction dangles from that moment on, and gate-46 (spec-anchor-existence) reports it. The developer who wrote the tag had followed this sniff's own advice. Measured on portaliq: 100 unresolved gate-46 targets, and 260 of its 385 live tags pointing into a change directory. The sniff ships identically in 20 ConductionNL repos, so grinding the tags without fixing the sniff regenerates them at the rate changes are archived. This changes the docblock example and BOTH warning messages to the canonical form gate-46 and the project rule agree on: @SPEC openspec/specs/{capability}/spec.md#requirement-{slug} The method-level message previously carried no guidance at all, so a developer reading it had only the class message to copy from; it now names the same canonical shape. Behaviour is unchanged: severity stays WARNING (verified via phpcs — an untagged class and public method still report 0 errors / 2 warnings, and a tagged file still reports nothing), and an `openspec/changes/...` target is still accepted, since this sniff only checks that a tag is PRESENT. No `@spec` tags are repointed here — this repo's existing tags are untouched. Refs ConductionNL/.github#228
…alls that never ran (#774) * fix(auth): declare the auth posture on five routed methods, stop reroute contradicting its own body gate-5 flagged all five WorkflowDefinitionController endpoints as having no declared auth posture. They were already admin-only — a Nextcloud controller method with no auth attribute is — but an undeclared posture is indistinguishable from a forgotten one, which is the point of the gate. Declared with AuthorizedAdminSetting, matching the sibling seam CaseDefinitionController: workflow definitions are case-type configuration and are edited from /settings/workflow-definitions. gate-9 flagged RoutingController::reroute() and it was right. The docblock said "Requires the caller to be a server admin ... Non-admin callers receive 403", the body enforced exactly that via IGroupManager::isAdmin(), and the method still declared @NoAdminRequired. Replaced with an @auth admin-only declaration that states what the body does. This is a posture declaration, not a waiver: the endpoint stays exactly as restrictive as it was. gate-7 flagged AssistantController::availability(). Whether an LLM backend is wired up is deployment information, so the probe now answers only for an authenticated session — the same fail-closed shape converse() already uses two methods below it. gate-49 flagged SubstitutionController::revoke() and ::actions(), which called service methods that can throw with no try/catch and no @throws. Both now translate to a JSONResponse with an accurate status, mirroring create(). Deliberately untouched: submitResult()'s isAdmin(). It is an admin bypass inside a real per-object guard; "fixing" it would lock out the field inspectors the endpoint exists for. * fix(or): two findAll() call sites passed the wrong shape and failed silently OpenRegister's ObjectService::findAll() takes ONE config array (`findAll(array $config = [], bool $_rbac = true, bool $_multitenancy = true)`). Two procest call sites passed ($register, $schema, $filters) positionally. That is a TypeError against `array $config` — and both sites sit inside a `catch (Throwable)` that turns the failure into a success-shaped return, so neither ever reported anything. 54 other call sites in lib/ already use the correct shape. - BezwaarDecisionListener::hasPublishedDecision() caught the TypeError and returned `true` = "a published decision exists". The guard that is supposed to block a bezwaar entering "Beslissing op bezwaar" without a published bezwaarDecision has therefore never once blocked anything. - RoleResolverService::loadCaseRoles() caught it, logged a warning and returned an empty list, so stored case roles were never loaded and rule resolution silently fell through to its other sources. gate-61 (ADR-078) additionally flagged the listener for running an unbounded findAll() on the write path and performing a saveObject() inside another object's write. The probe is now bounded — it answers a yes/no question, so it never needs the whole set — and fails OPEN when it hits the bound, because reverting on an incomplete scan would block a legitimate transition. The revert itself stays inline and says why: it is a transition guard, not follow-up work, and a deferred revert would publish the invalid status to every reader and to the notification and audit listeners firing off the same write. gate-23 (WARN-only) flagged PdokLocatieserverService::callDirect() for reaching api.pdok.nl with its own transport. It cannot be rerouted through openconnector's PDOK adapter yet — that adapter returns 404 on `lookup?id=`, the exact call this shim makes (openconnector#1197) — so this is the honest partial: the raw `fopen()` stream wrapper is replaced with Nextcloud's IClientService. The wrapper ignores the instance's proxy and certificate configuration and is unavailable outright when allow_url_fopen is off, so on a hardened or proxied deployment this path failed in a way that looked like PDOK being down. Behaviour, timeout, Accept header and the RuntimeException-carrying- the-status contract are preserved. * test(bezwaar): cover the guard that was dead, asserting on the call SHAPE The coverage ratchet on PR #774 was right: the fix added 21 statements and no tests. These are those tests, and they are the ones that would have caught the original defect. BezwaarDecisionListenerTest asserts on the ARGUMENT the listener hands to OpenRegister — one config array, with register/schema/bezwaar under `filters`, and a `limit` — not merely that a call happened. A test asserting only "findAll was called" would have passed against the broken positional call too, which is exactly how this stayed green while the guard never fired. Verified the tests can fail: restoring the old three-argument positional call turns 6 passing tests into 3 failures (testProbeCallsFindAllWithASingleConfigArray, testProbeIsBounded, testRevertsWhenNoDecidedDecisionExists); restoring the fix returns them to green. So these assertions are load-bearing, not decorative. Also covered: the decidesk-delegated `decisionRef` path, the fail-open at the probe bound, the no-op on unrelated statuses, and the new 401 on AssistantController::availability(). tests/Stubs/Event/ObjectUpdatedEventStub.php is the post-persist counterpart of the existing ObjectUpdatingEventStub, loaded by tests/bootstrap.php only when the real openregister class is absent and self-skipping via class_exists() when it is present — the same pattern the surrounding stubs already use. Full unit suite locally: 1739 tests, 6077 assertions, 0 failures.
* fix(ci): stop tracking .phpunit.result.cache (gate-29 gitignore-then-commit) The file is gitignored but was still tracked from before the ignore rule landed. Untrack it so the gate's tracked-file-behind-an-ignore-rule check passes. * fix(specs): retarget/repair 4 dangling @SPEC anchors (gate-46 spec-anchor-existence) - woo-publication design.md: add a resolvable (Fallback) anchor tag to the "Fallback: catalog discovery is best-effort" heading - DigidSamlAdapterInterface / LogDigidSamlAdapter: retarget from the archived zaakportaal-01-schema-foundation change to the canonical zaakportaal-mijngemeente spec's DigiD/eHerkenning requirement - processMiningApi.js / processMiningShaping.js: retarget from the now-archived process-mining-bottlenecks change tasks to its synced canonical spec - RedactionAssistDialog.vue / DocumentAssessmentTable.vue: retarget the 10 woo-llm-anonymisation change-task references to the canonical spec's human-review requirement (the one still-valid #task-4-1 reference is left untouched) * fix(schema): add missing property titles/descriptions (gate-51 schema-property-titles) 23 schema properties across procest_register.json (caseFederatedActivity entries, casetransfer custodyAuditTrail) and 70-cmmn-case-model.json (CMMN plan-item / sentry structure: type, name, description, entry/exit criteria id/standardEvent/caseFileEvent/field/operator/value) were missing a human-friendly description. Added real, CMMN/domain-grounded descriptions rather than placeholder text. * fix(relations): correct/add external-register $ref values (gate-54 relation-dialect) - case.decisions: $ref "Decision" -> "decision". Verified against decidesk's real lib/Settings/decidesk_register.json: the schema's JSON key is "Decision" but its declared `slug` is "decision", and a $ref resolves against the slug (per the gate's own documented reasoning). - case.vergunningaanvraagRef: was relation-shaped (format:uuid + a relation description) but had no $ref at all. Added $ref:"vergunningaanvraag" + x-external-register:"dso", verified against openregister/lib/Settings/dso_register.json (schema key and slug are both "vergunningaanvraag", hosted in openregister's own "dso" mock register) and matching the file-name-implies-register-name convention already used by the pipelinq/decidesk external refs in this file. caseType.productsOrServices's $ref:"product" is left unchanged — verified against pipelinq's real register (key and slug both "product", matching ADR-003 verbatim). That finding and the (now correct but still externally-unresolvable) vergunningaanvraagRef finding remain red: the checker's schema-key candidate set is built only from the current app's own register files, so a genuinely-external cross-app/cross-register slug can never resolve from procest's side. Not a defect. * fix(nav): restore SubstitutionMenu, delete 8 dead removals entries (gate-53 effective-manifest-crossref) - SubstitutionMenu (route /substitution) was wrongly removed as a "duplicate" of SubstitutionAdminMenu (/substitution-admin) in 01c1a89. They are different pages for different roles: SubstitutionSettings is the self-service page every user manages their own substitutions on; SubstitutionAdmin is the coordinator-only console listing all substitutions. Non-coordinators had no menu path to their own self-service page. Restored. - Deleted 8 removals entries that matched no merged menu entry at all (dead cruft from earlier portal/inspection retirements): BezwaarDecisions, BezwaarAdviceRequests, Subsidies, BesluitvormingGroup, LeverancierDashboard, MijnZaken, MijnNotificaties, Inspecties. 8 findings remain intentionally red (BezwaarBeroepGroup, Bezwaren, Beroepen, SubsidiesGroup, CaseMap, Voorstellen, Advice, BesluitvormingAgenda) — each is a deliberate, documented retirement (case-type-navigation commit d6824aa; consume-decidesk-besluitvorming-leaf commit d2df51b) superseded by a runtime surface (Cases' folderSidebar + map viewMode, CaseDetail's BesluitvormingLeafTab) that this gate's static .menu-tree walk cannot see. Restoring them as static entries would reintroduce the exact anti-pattern both changes explicitly rejected; deleting their routes would violate each change's own ADR-044 hard invariant to keep them deep-link/e2e routable. Documented in _meta.removalsCoverageNote for the next reader. * fix(e2e): replace networkidle waits with deterministic waits (gate-58 e2e-networkidle) networkidle never settles on Nextcloud (NC's notification poll keeps a request in flight for the whole session), so all 3 waits silently burned their entire timeout budget behind a .catch(() => {}). Replaced with waitUntil:'domcontentloaded' + explicit element waits: - deelzaak-case-email.spec.ts: the goto is immediately followed by real URL/text assertions with their own timeouts, so the networkidle wait was redundant on top of them. - docs-screenshots.spec.ts (2 sites): ported the app's own established technique from tests/e2e/visual/_visual-helpers.ts's waitForContentReady() — wait on the actual content region (main / .app-content / #content-vue, or #content/.section for the NC core admin-settings page), then let any loading spinner clear.
* fix(a11y): gate-32 semantic-controls — add keyboard equivalents to click targets Non-semantic elements (div/li/a) with @click but no role/tabindex/keyboard handler are unreachable and inoperable for keyboard and screen-reader users (WCAG 2.1.1 / 4.1.2). Adds role="button" tabindex="0" plus real @keydown.enter / @keydown.space handlers that call the same action as the click handler — not decorative attributes. Modal/dialog backdrop overlays (click-outside-to-close) get the same treatment so the dismiss action has a keyboard equivalent. The six "View all" widget footer links move from a bare <a @click.prevent> to a real <a :href> pointing at the actual destination URL (computed via generateUrl), which is both a genuinely better fix (native middle-click / open-in-new-tab support) and clears the gate since a real href exempts an <a> from the semantic-controls check. TaskCreateDialog.vue additionally gets an aria-label on its icon-only close button (gate-39 button-name, same file). gate-32: 40 findings -> 0. * fix(a11y): gate-39 button-name — add accessible names to icon-only buttons Icon-only NcButton/button elements with no aria-label/title/text content announce as just "button" to screen readers (WCAG 2.2 AA SC 4.1.2). Adds a translated aria-label naming the actual action ("Edit {name}", "Delete {name}", "Remove guard", "Close step configuration", etc.) rather than a generic label, using the row's own name field for specificity where one is available. gate-39: 24 findings -> 0. * fix(a11y): gate-34 window-confirm — replace window.confirm() with CnConfirmDialog Native window.confirm() calls break Nextcloud theming and are inaccessible to assistive tech (they bypass the app's dialog stack entirely). Replaces all three call sites with @conduction/nextcloud-vue's shared CnConfirmDialog (NcDialog-wrapped, its own file in the design-system package — importing it does not trigger gate-13 modal-isolation since no <NcModal>/<NcDialog> tag is written inline in these components). Each site now opens the dialog on the original trigger, performs the delete in a `@confirm` handler, and reports success/failure back via setResult() instead of a bare console.error / silent catch. These three files also carry their gate-43 table-headers (<th scope=>) and, for DeelzaakList.vue, gate-45 prefers-reduced-motion fixes — committed together since they land in the same file. gate-34: 3 findings -> 0. * fix(a11y): gate-43 table-headers — add scope= to <th> elements <th> without a scope declaration leaves screen readers unable to associate data cells with their column/row header (WCAG 2.2 AA SC 1.3.1). Adds scope="col" to header-row <th> cells and scope="row" to the row-header <th> in LhsMatrixAdmin.vue's severity/behavior matrix table. ComplaintAnalyticsDashboard.vue's "By category" table had no <th> at all (rule=table-without-th) — adds a real <thead> naming the two actual columns (Category, Count) instead of a header-less table. Self-closed spacer/action <th /> columns (row-actions menus, drag handles) are left alone — they carry no accessible name so scope= on them is inert, per the gate's own exemption for headers with no name. gate-43: 27 findings -> 0. * fix(a11y): gate-45 prefers-reduced-motion — add reduced-motion fallbacks <style> blocks with a transition/animation and no @media (prefers-reduced-motion: reduce) override force motion on users who have asked the OS to minimise it (WCAG 2.2 SC 2.3.3). Adds a media query per affected selector that sets transition/animation: none — a real override, not an empty block satisfying the gate's text match. Skeleton-loading shimmer keyframe animations and hover/click transitions both get the treatment. The remaining ~17 files flagged by this gate had their reduced-motion fix land in an earlier commit because they were also touched for gate-32/39/43 in the same file. gate-45: 25 findings -> 0. * fix(a11y): gate-55 detail-page-discipline — use a registered manifest icon AiOversightDetail's data widget declared icon "RobotOutline", which is not in the shared icon registry (nextcloud-vue CnWidgetGrid/widgetIcons.js) and renders the '?' fallback per ADR-062 rule 8. Swaps it for "ShieldCheckOutline" — already in the registry and semantically fitting for an AI-oversight / human-review audit entry (EU AI Act Art. 14 evidence). gate-55: 1 finding -> 0. The gate-16 spec-coverage regression this branch introduced (8 new frontend methods/computed properties added while fixing gate-34/32 lacked @SPEC tags) was fixed in the earlier gate-32/gate-34 commits by giving each new method the same @SPEC tag as its sibling methods in the same file. * fix(l10n): add the 14 strings the a11y pass introduced, EN and NL The accessible names and the CnConfirmDialog copy added for gate-39/gate-34 were new source strings, and neither l10n file had them. That failed two CI jobs the gate suite does not see — 'l10n coverage (en.json)' and 'quality / Frontend Check (test:l10n)' — and would have shipped raw English keys to a Dutch-locale user. en.json extracted with the repo's own 'test:l10n:write'; nl.json translated by hand. 'Remove guard' is 'Voorwaarde verwijderen' rather than a literal 'bewaker' — in the transition editor a guard is a precondition on the transition, not a person. node tests/l10n/check-l10n.js now reports both halves OK; before this commit it reported 14 keys missing from en.json and then 4 missing Dutch translations. * fix(spec): retarget the signalering-widgets @SPEC tag at its canonical spec All 36 tags pointed at openspec/changes/retrofit-2026-05-24-signalering-widgets/tasks.md, a change directory that was archived to openspec/changes/archive/2026-05-31-retrofit-2026-05-24-signalering-widgets. The target has not existed for months, which is why gate-46 reports them as unresolved. Retargeted at openspec/specs/signalering-widgets/spec.md — the canonical home, which exists — per the rule that a @SPEC tag names a spec, never a change dir. 30 of the 36 predate this branch; the a11y pass added 6 more by copying the surrounding docblock. Diff-scoping means touching these files makes all of them this PR's to answer for, and CI reported exactly that: 10 unresolved findings from 1 distinct target. Measured, not assumed: after this commit no @SPEC target under src/ named signalering-widgets is missing from disk.⚠️ NOT fixed here, and not this PR's: 48 OTHER @SPEC targets under src/ are also missing — mostly archived retrofit-2026-05-* change dirs, plus two that name a canonical spec which was never written (openspec/specs/process-mining-bottlenecks/spec.md, openspec/specs/realtime-updates-ui/spec.md). They are diff-scoped out today and will surface on the next PR that touches each file. That is a separate sweep, and the two missing canonical specs need writing, not retargeting. * fix(register): drop x-external-register — nothing reads it PR #776 added "x-external-register" next to three cross-register $refs as part of a gate-54 fix. Measured, it is inert in both directions: - OpenRegister does not read it: no occurrence of x-external-register anywhere under openregister/lib/. - gate-54 does not read it either: check_relation_dialect.py resolves a $ref against _global_schema_keys(), the union of schema KEYS and declared SLUGS across *register*.json and register.d/*.json in the same settings dir. There is no external-register branch. Proof it changed nothing: gate-54 reports the same 2 findings with and without the key. The 'decision' $ref in that commit was a REAL fix and is untouched — "Decision" -> "decision" resolves against decidesk's declared slug. Only the no-op key is removed; an unread key introduced under a 'fix(relations)' subject reads like a mechanism and is not one. The remaining 2 findings STAY RED, honestly: caseType.productsOrServices -> 'product' (Pipelinq's product register) and case.vergunningaanvraagRef -> 'vergunningaanvraag' (the DSO register) both name schemas owned by another app. Neither is in procest's 85-schema register set, so the gate cannot resolve them and has no dialect that would let it. Both properties are live — 6 and 7 code references respectively — so deleting them would break working relations, and adding foreign schemas to procest's own register would misstate ownership. Each property's description already documents the owning register. This needs a cross-register $ref dialect in the gate, not a change here. * fix(case): wire BesluitvormingLeafTab — it was registered but never placed Found while verifying the gate-53 rationale merged in #776, which states that the standalone Besluitvorming nav (Voorstellen / Advice / Agenda) was 'replaced with a BesluitvormingLeafTab sidebar tab on CaseDetail' and that the replacement is 'live, wired, and menu-reachable'. The first half was true; the second was not. Measured: - src/components/tabs/BesluitvormingLeafTab.vue exists. - src/registry.js:412 registers it, in the same shape as CaseNotesTab. - CaseDetail.config.sidebar.tabs listed audit, version-history, notes, sharing, email — and nothing else. So the component was resolvable by name and rendered by nobody. The consume-decidesk-besluitvorming-leaf change removed three menu entries and registered a tab it never placed: users lost the nav and gained no replacement. Being in the registry is availability, not placement — the manifest decides what renders, and it did not mention this tab. The component's own header says it is meant to be surfaced 'through the path', which is exactly the wiring added here, and it already handles decidesk being absent by rendering a quiet unavailable notice rather than a broken tab. Icon 'Gavel' is registered in src/icons.js, so gate-60 stays green (28 manifests, 0 failures). This does NOT move gate-53: still the same 8 findings, because that gate walks the static .menu tree and a per-object sidebar tab is not in it. The point is the lost surface, not the gate — the gate is what led me to look. * fix(register): express the two cross-app relations in the x-external-register dialect (gate-54 2 -> 0) .github#286 landed after my earlier commit and adds exactly the notion that was missing: check_relation_dialect.py now has _is_external_ref(), so a property owned by another app's register is expressible. Verified against a fresh clone of ConductionNL/.github@main, package 365fa31d09a26f980e6dc76cb0800575ef005a4e — `_is_external_ref` exists at lib/check_relation_dialect.py:341. It did NOT exist at 651e5c5, which is why my previous commit removed the key as inert. That measurement was correct at the time and is now superseded. The dialect is `x-external-register: <app>` on the property, carrying the BARE identifier, and NO $ref — the checker still reports a property that declares both, because OpenRegister resolves $ref within one register set and can never reach another app's schema: caseType.productsOrServices x-external-register: pipelinq, items lose $ref case.vergunningaanvraagRef x-external-register: dso, loses $ref case.decisions is deliberately NOT given the key: `decision` is one of procest's own 85 schemas and resolves locally, so it is not a cross-app reference. Adding the key there would now be a finding rather than a fix. Bidirectional proof, same checker and invocation both ways: as committed -> 0 findings $ref put back on vergunningaanvraagRef -> "$ref 'vergunningaanvraag' does not resolve to a schema key in the register set (case-exact)" restored -> 0 findings Both properties stay live — 6 and 7 code references — and their descriptions already name the owning register.
Hard pin to the vue3 dist-tag head (2.2.0-vue3.7), up from 2.2.0-vue3.3. Verified: - lockfile control (pin unchanged): 0-line diff, so the 8-line lock change is attributable to this bump alone; no other package re-resolved - installed off disk after npm ci: one copy, 2.2.0-vue3.7, peer vue ^3.5.0 - build: exit 0, 3 warnings before and after - vitest: 32 files / 330 passed before and after - bundle: 66,317,565 -> 66,327,229 bytes (+9,664, +0.01%)
chore(deps): pin @conduction/nextcloud-vue to 2.2.0-vue3.7
Follow-up to #778, which pinned 2.2.0-vue3.7. The vue3 dist-tag moved twice more while the fleet wave was running (vue3.7 -> vue3.8 -> vue3.9). The fleet converges on 2.2.0-vue3.9. Verified on npm 10.8.2, the version CI runs: - control (pin unchanged): 0-line diff - npm ci: exit 0 - installed off disk: one copy, 2.2.0-vue3.9, peer vue ^3.5.0 - build: exit 0, 3 warnings at 2.2.0-vue3.7 and at 2.2.0-vue3.9 - vitest: 32 files / 330 passed at both versions - bundle: 66,326,990 -> 66,333,000 bytes (+6,010, +0.01%) 2.2.0-vue3.9 is not pre-verified against our apps the way 2.2.0-vue3.7 was, so the run above is the verification. No regression.
chore(deps): pin @conduction/nextcloud-vue to 2.2.0-vue3.9
…all (#780) * fix(pwa): the service worker swallowed every PDOK-via-openconnector call `E2E Tests (Playwright)` was red on `development` with three `page.evaluate: TypeError: Failed to fetch` failures in `spec-coverage/pdok-via-openconnector.spec.ts`, thrown from inside Nextcloud core's patched `window.fetch` (`core-main.js:1:80554` is the `await t(e,n)` in that wrapper — checked against stable32's shipped `dist/core-main.js`). It is not a test problem and not a Playwright problem. Measured on a CI runner with NO `page.route()` registered at all, the same fetch still throws; and with the page loaded OUTSIDE the service worker's scope, it returns 404 and Playwright's route handler fires normally. TWO PRODUCT DEFECTS, both in the mobiel-inspectie-offline PWA layer: 1. `public/service-worker.js` decided "is this a map tile?" with `/(brtachtergrondkaart|wmts|pdok|service\.pdok\.nl)/i.test(url.host + url.pathname)` — a substring test that also runs over the SAME-ORIGIN path. Since migrate-pdok-to-openconnector this app's own address lookups live at `/apps/openconnector/api/pdok/{suggest,lookup,free, reverse}`, so every one of them matched on the literal `pdok` and was answered cache-first out of the map-tile cache. The worker is registered at app-root scope, so this hit every procest page. Replaced with an exact third-party host allow-list: a request back to this Nextcloud is never a tile. 2. A Service Worker inherits the CSP of its OWN script response. `DashboardController::serviceWorker()` sent Nextcloud's default `default-src 'none'` with no `connect-src`, under which EVERY `fetch()` the worker makes is blocked. Measured on Nextcloud 32: `fetch(request)`, `fetch(request.url)` and `fetch(url, {mode:'same-origin'})` all threw inside the worker. Both strategies (`cacheFirst` / `networkFirst`) therefore always fell through to `Response.error()` — the worker could only ever break a request, never serve one, and the offline sync cache could never be populated in the first place. Now sends `connect-src 'self' https://service.pdok.nl`. Together these made `src/services/pdokService.js` REJECT rather than degrade: `handleNetworkError()` rethrows on an error with no HTTP status, so the address field broke outright instead of surfacing the 503/404 warning `openspec/specs/pdok-consumer/spec.md` requires. Why it was CI-only: the worker's scope is `generateUrl('/apps/procest/')`, which keeps the `/index.php` prefix unless `front_controller_active` is set. On CI (`php -S`) the spec's `/index.php/apps/procest/dashboard` is INSIDE that scope and the worker controls the document; on the docker images every developer uses, the same URL is OUTSIDE it and the worker is invisible. The specs now navigate into the worker's own scope and wait for `navigator.serviceWorker.controller`, so the coin flip is gone. Test hardening in the same files (each mutation-verified): - assert the ITEM, not the container. `expect(Array.isArray(result))` was true for the empty array an unintercepted server answer produces, and `expect(status).toBe(404)` passed on Nextcloud's own 404 — openconnector is genuinely absent on CI. Both fulfilled responses now carry a marker header that a real server answer cannot produce, and the payload assertion names `Lauriergracht 116`. - `addressesRegisterAvailable()` returned `status() !== 404`, so 401 from an unauthenticated context read as "the register is installed" and the test went on to seed against it. Now only a 2xx counts. - that same test resolved `process.env.NEXTCLOUD_URL || 'http://localhost:8080'` — the SHARED dev container — so it seeded and deleted OpenRegister objects in somebody else's environment whenever the suite was pointed elsewhere. Now uses the single `BASE_URL` resolver. New `spec-coverage/service-worker-scope.spec.ts` guards defect 2 directly. A third test that would have guarded defect 1 from the response alone was written, found unable to fail under the planted defect, and left out rather than kept as a check that always passes. Refs #719 * test(pwa): unit-cover the service-worker script CSP The coverage ratchet caught the 4 new statements in DashboardController::serviceWorker() as untested — correctly: the CSP on that response is the difference between a Service Worker that can fetch and one whose every request is blocked, and nothing else in the suite looks at it. Proven able to fail: with the CSP block removed, testServiceWorkerScriptGrantsConnectSrc reports "Failed asserting that 'default-src 'none';base-uri 'none';manifest-src 'self';frame-ancestors 'none'' contains \"connect-src\"."
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wave-3 critical security fixes for procest. All 4 REAL findings + 1 PARTIAL (SSRF half) + BONUS fail-open fixed.
deliverToSubscriptionnow validates callback URL with https-only allowlist + RFC1918/loopback/link-local CIDR block before any outbound POST.SettingsService.getPublicSettings()redactsai_api_keyandappointment_backend_api_keyto'***'for non-admin callers;SettingsController.index()now uses the redacted variant unless caller is admin.scopeGrantCovers()now verifies full scope name (zaken.aanmaken) not just suffix (.aanmaken) — addsCOMPONENT_SCOPE_PREFIXmap; prevents intra-component scope confusion.ParaferingService+ParaferingController(operated entirely in-memory, no persistence, client-supplied audit trail forgeable). Live engine isParafeerActieService. Routes removed.callAiModelvalidates admin-configuredai_model_urlagainst scheme allowlist + DNS pin + CIDR block before curl. TLS/redirect claims from report were FALSE — not fixed.ZrcController.checkZaakReadAccess: changedcatch(\Throwable)-return-null(fail-open) toreturn permissionDeniedResponse()— any unexpected exception now denies access (fail-closed).Test plan
Ref: /tmp/triage-procest.md