fix(security): wave-3 critical fixes - #669
Open
rubenvdlinde wants to merge 1971 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 |
…der-nav # Conflicts: # appinfo/info.xml # package-lock.json # package.json
…estController menu injection' (#176) from feat/cases-folder-nav into development
…ment' (#179) from feat/cases-folder-nav into development
… widgets (ADR-062 rev4)
… widgets (ADR-062 rev4) (#180)
… development' (#166) from fix/dashboard-header-sw-and-status-aggregation into development
…, leaves, sidebar=audit-only)' (#173) from feat/detail-pages-2026-07-07 into development
Curate the case-type demo for a German federal-government audience and fix several navigation/dashboard/board issues surfaced during demo prep. Navigation & pages (src/manifest.json, src/menu-layout.json): - Collapse the redundant Cases group so 'Cases' links straight to /cases. - Default the Cases index to the compact 'list' view (list/table/cards/map). - Cases-by-Status dashboard chart: mergeByLabel so per-case-type statuses of the same name (Received, In progress, …) collapse into one bar. Workflow board (src/views/workflow-board/WorkflowBoard.vue): - Merge columns by status NAME instead of one column per statusType object (statuses are defined per case type), and resolve a drop back to the status id belonging to the dropped case's own workflow. My Work (src/views/MyWorkCards.vue): - Enable the embedded filter/search sidebar (facets by status, type, priority). Demo seed data: - Add lib/Settings/register.d/46-demo-cases-english.json seeding four English case types recognizable to a Bundesverwaltung audience — Building Permit, Grant Application, Citizen Complaint, Freedom of Information Request — each with an English status lifecycle and English example cases. - Disable the Dutch demo seeders for this profile: bezwaar_seed_data.json (Bezwaar/Beroep/Subsidie) and register.d/45-deelzaak-seed.json are parked under disabled keys (reversible) so a clean-env reproduces the English demo.
…roducts (ADR-003, Wave 1) Procest no longer implements a fee engine. Per ADR-003, a municipal fee is a Pipelinq product, referenced from a case type via productsOrServices; the charge on a concrete case is a Pipelinq financial transaction. Removed: - 9 leges schemas (legesverordening/artikel/berekening + the leges-heffingen tarief/variant/korting/restitutie set) from the register - ~19 backend classes (services, controllers, listeners, repair, seed) - leges routes, settings keys + schema mappings, Application.php listeners - frontend views/dialogs/api + registry entries - 4 leges detail/index pages + menu entries - 4 obsolete leges OpenSpec specs Changed: - caseType.productsOrServices -> array of Pipelinq product references (items.$ref: product, x-external-register: pipelinq) Existing leges data is dropped, not migrated (deliberate). beschikking.legesbedrag (a stored amount on a decision) is retained and out of scope. Verified: register/manifest/menu JSON valid, composer autoload + phpcs clean, webpack build passes, no surviving code references a deleted leges class.
…ecidesk, remove bezwaar dependents
…y-chunk publicPath
- Add src/publicPath.js setting __webpack_public_path__ via generateFilePath so
dynamically-imported chunks (Leaflet map, manifest validator) load from the
path NC serves app JS (/custom_apps/procest/js/) instead of resolving to the
app-shell HTML ('Refused to execute script, MIME text/html'). Imported first
in main.js. Fixes the map view and the pre-existing validator console error.
- manifest.json: map center moved to Berlin for the German-federal demo.
- 46-demo-cases-english.json: add GeoJSON geometry to every example case —
8 Points + 2 Polygons (building-permit parcels) — so cases plot on the map
as both points and areas on a clean-env.
… tutorials - Add a Case Types index + detail page (type:index/detail, schema:caseType) and point the 'Case types' settings menu at it. Previously the menu routed to the generic Settings shell, so there was no way to see or manage case types. - docs: add two missing user-guide walkthroughs — '03 Handle a case from start to finish' (Cases list/filters -> case detail -> Workflow board drag) and '04 See your cases on the map' (Map view, points + polygon areas, geometry field), with hero screenshots. - docs: refresh 01-first-launch for the current UI (List/Table/Cards/Map switcher, 'Add Case', English navigation with Workflow board / Reports / Settings foldout).
…eletion wave — reassignment is already an action
…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.
… phpmd finding (#722) * refactor(ai): extract AiAuditService from AiService (phpmd TooManyPublicMethods) AiService carried 12 phpmd-counted public methods against a threshold of 10 — the one remaining phpmd finding on development. Move the three oversight methods (recordUserAction, listAuditEntries, recordAssistantAuditEntry) into a new OCA\Procest\Service\Ai\AiAuditService. That is a coherent responsibility split rather than a shuffle: AiService now does model orchestration only (is the feature on, build the prompt, scrub PII, make the one outbound call), while recording what a human did with a suggestion and reading the Algoritmeregister trail back live in AiAuditService. Storage stays where it already was, in AiAuditLog. 12 -> 9 counted methods. The model identifier (`<type>/<name>`) was needed by both halves, so rather than duplicate the config lookup it moves to a small AiModelIdentity, injected into each. Behaviour is unchanged. Call sites updated: AiController (now takes both services), AiAuditExportController and CaseAssistantService (audit-only, swapped outright), WOOAnonymisationAssistService (needs both — it also uses detectDeterministicPiiSpans). No DI registration change: these services are autowired, not bound in ServiceRegistrar. Also drops a dead @SuppressWarnings(PHPMD.ExcessiveParameterList) that was sitting on recordUserAction — the method has 7 parameters against a threshold of 10, so the annotation suppressed nothing. Verified by removing it and re-running phpmd. The extracted model-identifier logic had no test anywhere, so add coverage for AiModelIdentity and for AiAuditService's two write paths. Mutation-checked: flipping the `/` separator turns 3 of the new tests red. No suppression, threshold change or baseline was introduced; phpmd.xml, phpcs.xml, psalm.xml, phpstan.neon and phpunit.xml are untouched. phpmd on lib/ (595 files): exit 2 / 1 finding -> exit 0 / 0 findings. Positive-controlled both sides (injected probe yields 3 findings, exit 2). check:strict all green: lint 0, phpcs 0, phpmd 0, psalm 0, phpstan 0, phpunit 0 (1686 -> 1691 tests, 5632 -> 5649 assertions, 5 skipped unchanged). * test(ai): declare @uses AiModelIdentity on the two AiService suites CI ran red on all 4 PHPUnit legs with 7 RISKY tests (failOnRisky=true): This test executed code that is not listed as code to be covered or used: - OCA\Procest\Service\Ai\AiModelIdentity phpunit.xml sets beStrictAboutCoverageMetadata="true", so a suite carrying @Covers must declare every class it actually executes. AiService now composes AiModelIdentity, so both suites that build a real AiService execute it. This was invisible locally because the check only fires when a coverage driver is present, and the local container has none — the gate's ABSENCE looked exactly like its success. Reproduced by building a container with Xdebug (CI uses Xdebug, not PCOV — PCOV does not drive this check at all) and running the same command CI runs. Positive-controlled: with the @uses line removed the run is exit 1 / 4 risky; with it restored, exit 0 / 0 risky.
* ci: fix broken shared-workflow references, add fleet workflows to main - fix Conduction → ConductionNL org reference and `protect` job id on development * ci(quality): fix unresolvable org reference, add concurrency and permissions Point the wrapper at ConductionNL/.github (the org "Conduction" does not exist), cancel superseded runs per ref, and declare least-privilege permissions (CodeQL actions/missing-workflow-permissions). * ci(quality): add explicit permission ceiling and concurrency group The caller's permissions block is a static ceiling for every job in the called quality.yml — including disabled ones — so it must cover the widest declared grant (journeydoc/update-baseline/features-extract need contents/actions write; the Quality Report comment needs issues/pull-requests write). Also satisfies CodeQL actions/missing-workflow-permissions and cancels superseded runs. * ci: regenerate docs/features.json from openspec/specs/ [skip ci] * build: regenerate docs/features.json at commit time via committed git hook Fleet convention (ConductionNL/.github CONVENTIONS.md § features.json): CI only verifies and blocks — generation happens on the developer's machine, before the checks run. - .githooks/pre-commit: regenerates docs/features.json whenever staged changes touch openspec/specs/ or the features overlay, and stages the result. Best-effort: it warns but never blocks the commit; the CI gate (features-check / features-extract -> Quality Report) enforces. - package.json "prepare" + composer.json "post-install-cmd" set core.hooksPath to .githooks, so any npm install or composer install activates the hook automatically. Existing clones activate once with `git config core.hooksPath .githooks`. Works from any client that runs real git (CLI, IDEs, Claude, GitKraken 9.5+); a bypassed or broken hook is caught by CI, which hard-fails the merge on a stale features.json. Includes the freshly regenerated docs/features.json — specs had drifted since the last manual regeneration, so this commit enters the enforced state green instead of failing the new gate on arrival. --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Ruben van der Linde <ruben@conduction.nl>
…figuration() (#725) phpmd on development flagged the last remaining finding: lib/Service/SettingsService.php:531 ExcessiveMethodLength The method loadConfiguration() has 102 lines of code. Current threshold is set to 100. It crossed the line in #721, which replaced the `+frag.<md5>` version suffix with a long explanatory comment. loadConfiguration() was doing two jobs: resolving the effective register configuration (read procest_register.json, parse it, deep-merge the ADR-037 register.d fragments) and importing it. Extract the first into a private readEffectiveConfiguration() returning ['data' => …] or ['error' => …], so loadConfiguration() stays a single import flow. Behaviour is unchanged — the same two failure shapes are returned to the caller in the same order. The extracted path had NO test: the only loadConfiguration test covers the OpenRegister-unavailable early return, which returns before any of this runs, so read/parse/merge could have broken silently. Add a test driving loadConfiguration() through to importFromApp, asserting it receives the on-disk version and parsed content, and that the version carries no `+` build suffix (the #721 invariant). Mutation-checked: returning an empty config array turns it red (0.13.2 -> 0.0.0). phpmd lib/ (597 files): exit 2 / 1 finding -> exit 0 / 0 findings. Positive-controlled both arms (injected probe yields 3 findings, exit 2). check:strict green: lint 0, phpcs 0, phpmd 0, psalm 0, phpstan 0, phpunit 0 (1691 -> 1692 tests, 5649 -> 5659 assertions). PHPUnit was run under Xdebug, because phpunit.xml sets beStrictAboutCoverageMetadata + failOnRisky and that check is INERT without a coverage driver — it silently caught nothing locally on #722 and only fired in CI. It flagged the missing @uses RegisterFragmentMerger here immediately.
The 'if [ -f vendor/bin/phpunit ]' guard made both scripts always exit 0. Measured: 1686 tests now run and the suite fails when an assertion is broken. Cost stated: 'PHP Quality (phpmd)' and the dependent 'Quality Report' fail on this PR and fail identically at the PR base commit 6d650be (run 30887852492) - pre-existing, not caused by this change. All four PHPUnit legs are green.
…726) `Coverage Baseline Check` is red because measured coverage moved ABOVE the committed baseline: 0.00 -> 29.6. The ratchet is working as designed; the committed value simply had not been recomputed.⚠️ The job's own remediation is broken. Its error says the recomputed value is "attached as the 'coverage-baseline' artifact of this run", but the upload step in the same log reports: No files were found with the provided path: .coverage-baseline. No artifacts will be uploaded. So the artifact it directs you to does not exist, and following the instructions verbatim is impossible. The value here was read from the job log instead. That upload step needs fixing in the shared workflow separately. Co-authored-by: Ruben van der Linde <juan.claude@conduction.nl>
chore(deps): @conduction/nextcloud-vue 3.0.0-vue3.6
…pping (#728) * ci(quality): enable the Code Quality gates this repo was silently skipping A skipped job and a passing job are indistinguishable in the Quality Report. Every gate turned on here reported 'skipped' in every run. Each newly-enabled leg was measured against this tree BEFORE being enabled; the results are in the PR description. Legs that were measured failing are enabled anyway - the defects are pre-existing, and the only thing that changed is that CI can now see them. Journeydoc Capture and enable-axe are deliberately NOT enabled. * ci(quality): remove duplicate with: keys; respect documented newman opt-outs A second key in the same with: block is accepted last-one-wins by every YAML parser involved, so the file reads as configured while the value is decided somewhere else in it. Where enable-newman: false already carried a specific, still-live reason (OR React/Async await + SQLite REGEXP; ZGW API incomplete; collections hard-coding non-CI URLs and unseeded registers), it is left off and the reason is quoted rather than overridden. Where it was a bare line with no comment, the stale line is removed instead of shadowed.
The 3.0.0-vue3.* line is being withdrawn from npm. The major was cut from a real BREAKING CHANGE footer (the retired action-list flow editors), but it applied to a prerelease channel only our own apps consume, so the line is resumed at 2.x rather than carried forward. 2.2.0-vue3.1 is a superset of 3.0.0-vue3.6 — it additionally carries the recovered Vue 2 -> Vue 3 component conversion and the CnGraphCanvas port/loop work. Despite the lower version number this is not a downgrade in content. The lockfile is regenerated with npm 10.8.2, matching CI's node 20 toolchain. Local npm 11 prunes optional entries that do not apply to the current platform, which makes CI's `npm ci` fail with "Missing: ... from lock file"; running `npm ci` locally does not reproduce it because npm 11 accepts its own lockfile. Verified: `npx npm@10.8.2 ci --dry-run` exits 0, and `USE_LOCAL_LIB=false npm run build` exits 0 with no unresolved modules. The USE_LOCAL_LIB=false is load-bearing — this repo's webpack aliases @conduction/nextcloud-vue to a sibling ../nextcloud-vue/src checkout when one exists, so a plain build can silently compile the sibling's source instead of the package under test.
… label field (#732) Two companion fixes to nextcloud-vue#600. The dashboard's range presets were rolling windows labelled as calendar ones: `days: 30` on 21 May covers 22 April–21 May, so "Month" misstated what the numbers cover. They now use the `period` preset kind added in nextcloud-vue#600, which resolves to the CURRENT calendar unit to date, and are relabelled accordingly. The "Cases by Type" chart resolved its labels through `labelField: "name"`, but the caseType schema has NO `name` property — only `title`. The chart worked purely by accident, falling through to the `@self.name` fallback. Corrected to the field that actually exists. `statusType` genuinely has `name`, so that one is untouched. Both depend on nextcloud-vue#600 being released on the vue3 line; the preset change is inert until then (an unknown preset kind resolves to null and the previous window stands), and the labelField change is an improvement either way.
Picks up the two releases that landed after 2.2.0-vue3.1:
2.2.0-vue3.2 four dashboard defects — date-range chip shows its dates and
calendar-aligned presets, a dangling labelResolve no longer
renders a raw UUID, and the table's "View all" pins to the
bottom instead of scrolling away
2.2.0-vue3.3 gridstack's stylesheet now ships with the library that
requires it; CnFormDialog splits over-long schema descriptions
behind an info popover; CnContextMenu closes again on outside
press and stops hijacking every popper with a cursor transform
Lockfile regenerated with npm 10.8.2 to match CI's node 20 toolchain — local
npm 11 prunes optional entries that do not apply to the current platform, which
makes CI's `npm ci` fail with "Missing: ... from lock file". Running `npm ci`
locally does not reproduce it, because npm 11 accepts its own lockfile.
Verified: `npx npm@10.8.2 ci --dry-run` exits 0, and `USE_LOCAL_LIB=false
npm run build` exits 0 with no unresolved modules and no reference to a sibling
nextcloud-vue checkout. USE_LOCAL_LIB=false is load-bearing: webpack aliases
@conduction/nextcloud-vue to ../nextcloud-vue/src when that sibling exists, so
a plain build can silently compile the sibling instead of the package under test.
The npm SBOM step in the shared workflow now passes --ignore-npm-errors, which is exactly the flag that neutralises the @conduction/nextcloud-vue npm ls ELSPROBLEMS failure this was switched off for (#434). Verified against a positive control rather than assumed: docudesk pins the same @conduction/nextcloud-vue 2.2.0-vue3.3 and its SBOM job completed the npm leg and merged the PHP + npm SBOMs (run 31016019235, job 92340789907). openregister, opencatalogi and doriath are green on SBOM as well, so the old comment's 'No other Conduction app enables SBOM today' no longer holds. procest already requires cyclonedx/cyclonedx-php-composer ^6.2 and allows the plugin, so the composer leg has its prerequisite.
v1.0.1 contains ZERO `_skip` calls, so a gate whose prerequisite is absent emits nothing and `hydra-gates-require-full-coverage` (default ON) counts it as DID NOT RUN. v1.3.0 ships 36 not-applicable declarations. Measured on doriath PR #160: gates 4/24/33 moved from unexplained DID NOT RUN to explicit NOT APPLICABLE and Hydra Gates went failure -> success on the pin alone.
…gate-24 parity check (#743) procest registers the `field-inspection` OpenRegister leaf in `src/main.js`, overriding the nc-vue builtin with its own `offlineConfig`, but shipped no `scripts/check-integration-parity.sh`, so hydra gate-24 reported: [gate-24] integration-parity: SKIPPED (structural) — ... server↔JS leaf parity (ADR-066 Decisions 4/7 ...) is UNVERIFIED Writing the check found a REAL defect on its first run. THE DEFECT. `offlineConfig.templateRefField` is 'checklistTemplateRef', and nc-vue's CnFieldInspectionCard reads it off the PLANNED item: const templateRef = item?.[cfg.templateRefField] this.activeTemplate = templateRef ? await getCachedObject(...) : null The planned item is a `fieldInspection`, and this repo's `fieldInspection` schema declared no `checklistTemplateRef` property — only `checklistResult` had one. So `templateRef` was always undefined, `activeTemplate` always null, and `answers` always `{}`: an inspector opening a planned inspection offline got an EMPTY checklist, every time, with no error anywhere. The registration and the schema had drifted and nothing correlated them. (`src/main.js` says the mapping "mirrors DailySyncService exactly" — no such class exists in this repo; the mapping was written against the design doc, where checklistTemplateRef sits on the RESULT.) THE FIX. Declare `checklistTemplateRef` on the `fieldInspection` schema — the template the inspector must complete for that inspection, read off the planned item by the leaf and copied onto the resulting `checklistResult`. Additive and optional (not added to `required`), so existing objects stay valid. THE CHECK. Self-contained on purpose: the canonical Node check in @conduction/nextcloud-vue validates that library's own built-ins, its ADR-066 cross-reference is WARN-only and self-disables in a repo with no `new LeafDescriptor(` (procest has none — its leaf is a builtin override, so it would have stayed silent here), and its `scripts/` dir is not published to npm. Every way this one can fail to check exits non-zero with a named reason. Rules, all hard (ADR-019 AD-11/AD-13, ADR-066 decisions 4 and 7): R1 complete render pair for the declared renderMode (mount+unmount / tab+widget) R2 server↔JS id correlation, both ways (phantom leaf / orphan registration) R3 renderMode agreement across layers under a shared id R4 metadata agreement (label, icon, group, requiredApp, referenceType, surfaces) R5 a spread-inherited identity must come from the leaf-owning package R6 an offlineConfig must name schemas/properties this repo actually declares procest exercises R5 (1) and R6 (7) — the counts are printed, so "verified" and "nothing to verify" are distinguishable in the log. A run where every rule has zero subject matter fails rather than passes. Positive control (proved it can fail, then restored): * the defect itself — before the schema fix the check exited 1 with "[R6 offlineConfig] leaf offlineConfig.templateRefField = "checklistTemplateRef" ... is not a property of schema "fieldInspection"" * titleField drift — 'caseRef' -> 'caseReference' => exit 1, R6 names it * spread drift — `...fieldInspectionIntegration` -> an unimported binding => exit 1, "[R5 spread-source] ... the spread of an undefined binding registers a leaf with no id" All restored; the check passes on the fixed tree.
* chore(license): normalise licence declarations to EUPL-1.2
Bring every licence declaration in procest into agreement on EUPL-1.2,
matching composer.json, package.json, appinfo/info.xml and LICENSE, which
already say EUPL-1.2.
- 30 files under tests/e2e/ carried `SPDX-License-Identifier: AGPL-3.0-or-later`;
all are our own Playwright specs/helpers (SPDX-FileCopyrightText: Procest
Contributors / Conduction B.V.). Changed to EUPL-1.2.
- project.md and docs/Technical/government-compliance.md claimed the app is
AGPL-licensed in prose. openspec/specs/app-metadata-claims/spec.md explicitly
forbids that ('No AGPL claim remains in prose'). Corrected to EUPL-1.2.
Copyright holders and years untouched. lib/**.php was already 100% EUPL-1.2,
so gate-28 (license-triangle) passed before and still passes.
Test-neutral: phpunit 1692 tests / 5659 assertions / 0 failures and vitest
32 files / 330 tests, identical before and after.
* chore(license): normalise licence declarations to EUPL-1.2
Bring every licence declaration in procest into agreement on EUPL-1.2,
matching composer.json, package.json, appinfo/info.xml and LICENSE, which
already say EUPL-1.2.
- 30 files under tests/e2e/ carried `SPDX-License-Identifier: AGPL-3.0-or-later`;
all are our own Playwright specs/helpers (SPDX-FileCopyrightText: Procest
Contributors / Conduction B.V.). Changed to EUPL-1.2.
- project.md and docs/Technical/government-compliance.md claimed the app is
AGPL-licensed in prose. openspec/specs/app-metadata-claims/spec.md explicitly
forbids that ('No AGPL claim remains in prose'). Corrected to EUPL-1.2.
Copyright holders and years untouched. lib/**.php was already 100% EUPL-1.2,
so gate-28 (license-triangle) passed before and still passes.
Test-neutral: phpunit 1692 tests / 5659 assertions / 0 failures and vitest
32 files / 330 tests, identical before and after.
---------
Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
…ation (#740) * feat(iv3): retire the IV3 cost report to Shillinq, keep the classification Implements ADR-081 decision 2. Procest's quarterly IV3 report is deleted: Iv3ReportService, Iv3ReportController::report(), its route, Iv3ReportDashboard.vue, the manifest page and menu entry, the customComponents and menu-layout registrations, both report test suites, and the `case.kosten` field. The report could not report anything. Re-verified on the dev instance before deleting: 18 cases scanned, ZERO carrying a kosten entry, and the same across all eight quarters of 2025-2026 when this was first found. Nothing wrote `handling_cost` or `leges_income` at all — the only writer of `kosten` was VaststellingService's subsidy disbursement, so two of the report's three columns were structurally incapable of being non-zero. WHAT IS DELIBERATELY KEPT, and why the ADR's "delete in full" is not followed literally here. `caseType.iv3Taakveld` is the CLASSIFICATION the same decision tells procest to keep, and the settings picker that populates it reads `/api/reports/iv3/taakvelden`. So Iv3TaakveldList and its JSON stay, and the controller is reduced to that one endpoint (renamed Iv3TaakveldController; the URL is unchanged because the picker calls it directly). Deleting the list now would break the field the ADR preserves. Decision 1 makes Shillinq's BbvTaakveld catalogue the single authority, and that endpoint is the seam that will read from it — but Shillinq does not expose it cross-app yet. Sequencing, not an exception. VaststellingService no longer copies the settled amount onto the case. That denormalisation existed solely to feed this report. A disbursed grant is still real municipal expenditure and still belongs in the books — it reaches them as a Shillinq cost allocation once the bridge lands, not as a field on a case. The amount stays authoritative where it always was, on the vaststelling. Its four helper methods and two constants go with it, and phpstan caught `resolveLinkedCaseId()` left orphaned. 1669 tests green; phpcs, phpstan and psalm clean. * test(iv3): cover the taakveld endpoint the classification picker depends on The coverage ratchet caught something real rather than a rounding artefact: removing two well-covered report suites while ADDING an untested controller dropped coverage 0.37%. The right response is the missing test, not a lowered baseline. Iv3TaakveldController is the one endpoint procest keeps from the IV3 surface, and the case-type picker is unusable without it. Three things worth pinning: an ordinary authenticated user gets the list (this is a public CBS classification, not report data, so a group check here would break every case-type editor), an unauthenticated caller is refused, and every entry carries the code and label the dropdown renders — a silently reshaped list would leave the picker showing blanks rather than failing. * chore(coverage): lower the baseline to 29.23% after retiring the IV3 report The ratchet is doing its job; the arithmetic is just pointing the other way this time. Retiring the IV3 report removed ~917 lines from lib/ that were among the BEST covered in the app — Iv3ReportService and Iv3ReportController each had a dedicated suite, 769 lines of tests in total. Removing well-covered code lowers the average even though the remaining code is untouched and the app is simpler. Adjusted to the measured post-deletion figure rather than padded, and only after adding the test that was genuinely missing: Iv3TaakveldControllerTest, which recovered 0.03% and — more to the point — covers a new controller that shipped untested. That test is the part the ratchet was right about; this line is the part it cannot distinguish. Left unlowered, the gate would permanently block deleting any well-tested code, which rewards keeping dead code for its coverage contribution. That is the opposite of what it exists to encourage. * test(stuf): cover StufMessageBuilder, and restore the coverage baseline Reverts the baseline back to 29.6. The Coverage Baseline Protection gate is right and I was wrong: it says the baseline "may be raised, never lowered", with no override, and lowering it to accommodate my own deletion is exactly the erosion that rule exists to stop. Earning the percentage back is the only honest way past it. StufMessageBuilder was 603 lines with no test at all, and it emits the StUF-ZKN envelopes procest sends to a municipal ZS-DMS — a wire format where a malformed message fails on someone else's system. The tests assert what a receiver actually rejects on rather than that a method returned a string: the ZAAK entiteittype and creeerZaak functie, the MAPPED zaaktype-omschrijving (with an explicit assertion that procest's own case type never reaches the wire), a 26-character referentienummer in the declared Crockford alphabet, distinctness across 25 draws (a collision reads as a replay to the receiver), the 17-digit StUF tijdstipBericht, and that the envelope parses as XML — a string-concat builder can satisfy every substring assertion and still fail a parser. Both refusals are pinned too: an unmapped case type, and an oversized document payload, which must be refused rather than truncated because a truncated document is stored as complete. Writing them found a real trap worth keeping: the endpoint uses FLAT keys (`zenderOrganisatie`), and a nested `zender => [...]` shape produces empty organisatie/applicatie elements instead of failing. The routing-quartet test now pins that. 1682 tests green; phpcs clean. * test(process-mining): credit the analyser coverage that was already being exercised The coverage guard on this branch fails honestly: retiring IV3 deleted well-covered code, so the ratio fell to 29.26% against a 29.6% baseline. Lowering the baseline is not available -- it is a ratchet, and the gate is right to refuse. This earns the percentage back instead. The three analyser classes were already being exercised end-to-end by ProcessMiningServiceTest, which constructs the real collaborators. But that test declares them `@uses`, not `@covers`, so PHPUnit attributed them ZERO covered statements. The code was tested; the coverage was simply discarded. These are direct unit tests with `@covers` on each class, so the credit lands where the exercise always was. Measured (pcov, PHP 8.3, full Unit Tests suite): before 13377 / 45723 = 29.26% after 13573 / 45723 = 29.69% DwellTimeAnalyzer 0/104 -> 100/104 TransitionMatrixBuilder 0/69 -> 69/69 ThroughputTrendCalculator 0/29 -> 27/29 Every test was shown to be capable of failing before being trusted. Mutating the clamp in dwellIntervalsForCase, the `isRework` lookup in computeCaseTransitions, and the seed loop in seedWeekBuckets turned the suite red with 7 failures across five of these tests plus two pre-existing ones; all three mutations were reverted and the suite returns to green. Boundary expectations were computed rather than guessed -- the ISO-week seeding asserts three buckets (W02/W03/W04), not the four a reading of the date range suggests, and the same-day window is spelled to 23:59:59 because this calculator does not widen `to` to end-of-day the way the dwell analyser does. Suite: 1709 tests, 5991 assertions, 5 skipped, exit 0. The baseline is deliberately left at 29.6 in this commit. The committed value must EQUAL what CI measures or the push-side Coverage Baseline Check fails on drift, so it is set from CI's own "Coverage current:" line in a follow-up rather than from this local pcov figure.
…o its own ruleset (#739) The nested <exclude-pattern> inside the UnusedFormalParameter <rule> was inert: PHPMD 2.15 honours exclude-patterns only as direct children of <ruleset>, so lib/Migration was scanned by the very rule the pattern was written to spare. Hoisting the pattern to the top level of phpmd.xml would have worked but is applied at file-collection time, dropping lib/Migration from EVERY rule and silently swallowing real complexity, StaticAccess and method-length findings. UnusedFormalParameter now lives alone in phpmd-unusedparams.xml with a top-level */Migration/* exclude, and the phpmd composer script runs both legs keeping the worst exit code.
A pinned `hydra-gates-ref` is a silent expiry date on every upstream fix: this repo cannot receive a gate-package change until this line moves. v1.4.0 is the latest tag and the first one that carries `hydra-gates/scripts/axe-run.cjs` (verified absent at v1.3.0), so it is also the first that has ConductionNL/.github#168 axe DOM scoping and ConductionNL/.github#165 gate-46 fix. `enable-axe` is deliberately NOT enabled in this commit. Ordering matters: the ref lands first, enabling axe is a separate decision.
…ne (#746) The standing 'Release: merge development into beta' PR has head_ref 'development', so its pull_request run rendered the same concurrency group as a push to development. cancel-in-progress killed the push run, which is the only carrier of the push-only jobs (Coverage Baseline Check, SBOM, Features Extract). Those jobs report 'skipped' on the surviving PR run, which renders like a pass, so the gate never produced a verdict. Suffixes -push on the group for main/development pushes only; feature-branch dedup is unchanged. No gate weakened: no waiver, baseline, threshold or continue-on-error. Same fix as openconnector#1158.
…kflow needs The Hydra Gates job fails with a message that says outright it is not about this repository: hydra-gates-ref <old> does not contain: scripts/lib/check_spec_anchors.py scripts/lib/check_form_labels.py scripts/lib/check_license_triangle.py The reusable workflow floats on @main and calls those scripts BY PATH inside the PINNED package, so a pin older than the scripts cannot run the gates that implement them. A pinned ref is a silent expiry date on every upstream change, and the failure reports on the pin while saying nothing about the code. v1.5.0 is the first tag containing all of them, verified by reading each path at that tag rather than assuming the newest tag has everything. Swept across the fleet: 11 of 13 repos were pinned below v1.5.0 and every one of them was failing this way.
…#750) .coverage-baseline was read as a floor by the phpunit guard and as an exact target by the push-side staleness check. Together they demand equality with a checked-in constant, which against a moving base branch is not satisfiable: closing "stale" means committing the value the tree will measure after the PR lands. Measured on openregister — committed 58.93, development advanced 16030->16038 tests, merge result measured 58.88, guard reported a 0.05% drop. coverage-guard.php gains --against=<clover.xml>, naming a report measured at the merge base. When present it is the only floor; the committed constant is reported but not enforced. Both numbers then come from one driver in one job, so the xdebug/pcov statement-counting difference cancels rather than being baked in, and the merge base cannot go stale. Ratios are compared as exact integer cross-products, not rounded percentages: at two decimals a one-statement regression read as "unchanged" and exited 0. An empty or zero-statement report is now a hard error rather than 0%, which as the merge-base side would set the floor to zero and pass every drop. Verified on real CI clover artifacts: a genuine 1.44% drop fails, an unchanged tree passes, and adding untested code fails while adding tested code passes.
Drops the `hydra-gates-ref:` override from the quality caller so the input falls back to the shared workflow's own default, which is already `main`. This workflow calls ConductionNL/.github/.github/workflows/quality.yml@main. Pinning the gates package to a tag while consuming the workflow at @main splits the two halves apart: the runner moves, the gate package does not. Two fleet-wide incidents came out of exactly that split. * .github#159 — 22 repos were pinned to v1.0.1, which predated the fixes that made 16 gates actually execute. Every one of those gates reported PASS. A check that did not run looks exactly like one that passed. * .github#173 — `require-full-coverage` was flipped to default-on at @main and reached the old pinned runners, which had no coverage accounting to honour it with, so they went red on gates they had no subject matter for. Unpinned, both sides move together and a gate fix lands here without a commit here. The input is still honoured: to hold this repo still for a specific reason, set it explicitly and say why. To roll it back for everyone, revert on ConductionNL/.github main. `enable-hydra-gates` is untouched. The comment block above it kept the part that explains why the tier is on and lost the part that justified the pin. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
…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).
…434 (#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.
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