feat: entity-level publication policy layer (#112)

[rjzondervan]

Summary

Implements entity-publication-policies (Wave 1.2 of the "anonimiseren bij de bron" bundle). Adds an entity-level policy layer that pre-empts the publication-clearance workflow at detection time, with deterministic conflict resolution between deny-list (prohibitions) and allow-list (standing consents) surfaces.

Backend

• New publicationProhibition schema in the consent register — entity-level deny rules with 4 seed records (court order, minor protection, undercover officer, AP categorical exemption).
• publicationConsent extended with the scope discriminator (document / entity) and the entity-scope-only field set (matchRules, validFrom, validUntil, active, consentMethod, consentDocument, consentScope), plus a policyMatch field linking a per-document record back to its driving prohibition / standing consent. 4 standing-consent seed records covering the consentMethod variants.
• PolicyMatchService — detection-time matcher with in-memory per-request cache, four match types (exact, normalized, bsn, kvk), deterministic conflict resolution (prohibition wins; multi-prohibition broken by lowest UUID lex).
• PolicyRetroactiveService — force-resolves in-flight scope:"document" records to anonymized when a prohibition is added or widened; intentionally no-op for standing-consent creation (future detections only — privacy default wins on retroactive sweep). Dispatched from DocuDeskEventHandler via payload-shape detection.
• ConsentService::createConsentRequest now consults PolicyMatchService before defaulting to WOO; branches into 4 detection-time outcomes (no match / prohibition / standing consent / both → prohibition). New scope-validation gate at write time. ConsentUpdateHandler rejects consentStatus transitions on records pre-empted by a policy.
• PolicyController + PolicyCrudService with 10 routes under api/policy/{prohibitions,standing-consents}. Service-level RBAC gate: standing-consent writes require docudesk-standing-consent-admins group (returns 403); prohibition writes restricted to docudesk-policy-admins at schema level.

Frontend

• Two new admin pages: Publication Prohibitions + Standing Publication Consents (both with CnIndexPage + CnStatsBlock + CnStatusBadge + NcDialog create/edit, match-rules subform, BSN/KvK encouragement / no-expiry warnings).
• Consent Workflow (renamed from Consent Management) — filters to scope:"document", adds "policy" badge on rows whose policyMatch is non-null.
• ConsentDetail anonymisation toggle keyed off the policyMatch referent kind: prohibition → ON+locked; standing consent → OFF+overridable (override records publicationDecision: "anonymize" while preserving consentStatus: "consent_given" and policyMatch).
• Smoke-test rebuild of AnonymizationWidget.vue — split upload/extract from anonymise with a manual review step exposing the Wave 1.3 grondslagen + skip-anonymise PATCH on each detected entity. Intentionally simple; example shape for the frontend team, not a production publication-prep page. New Pinia stores prohibitionStore, standingConsentStore.

Other

• Template fix for the chunk-loading regression from perf: split shared Vue/@nextcloud/vue chunks across entry-points #119 — templates/index.php and templates/settings/admin.php were never updated to load the new docudesk-shared-vendor.js + docudesk-shared-nc-vue.js chunks, leaving the entry bundle waiting on chunkOnLoad forever and rendering nothing. Worth a cross-check on pipelinq / procest — they got the same split.
• 19 new unit tests (scope-validation corners, retroactive eligibility, standard ConsentService surface, ConsentUpdateHandler policy-pre-empted guard).
• Newman collection extended with policy CRUD + scope-validation cases.
• Docs: section in docs/features/publication-consent-process.md covering the three-layer evaluation, retroactive asymmetry, UI toggle semantics, three admin surfaces, RBAC defaults. CHANGELOG entries under Added / Changed / Behavior changes.

Test plan

• OR imported_config_docudesk_version re-imports to 8.0.0; both schemas present (GET /api/registers/consent?_extend=schemas).
• GET /apps/docudesk/api/policy/prohibitions → 4 seeds; GET /apps/docudesk/api/policy/standing-consents → 4 seeds.
• CRUD a prohibition + a standing consent via the new admin pages; verify list refresh + scope-validation errors (missing consentMethod, documentId on scope:"entity").
• Detection-time outcomes via POST /api/consents: no-match, prohibition match, standing-consent match, both-match (prohibition wins). Verify policyMatch + notificationStatus + consentStatus + publicationDecision per spec.
• Retroactive force-resolve: create in-flight pending consent → create matching prohibition → record now anonymized + policyMatch. Repeat with a new standing consent → in-flight record unchanged.
• ConsentDetail toggle: locked-ON for prohibition match; OFF+interactive for standing-consent match (flipping ON updates publicationDecision only); legacy UX for policyMatch: null.
• Policy-pre-empted transition guard: PUT a consent record with consentStatus change on a record with non-null policyMatch → 400.
• Newman collection passes against a live stack.
• composer check:strict clean; openspec validate entity-publication-policies clean.

[github-actions]

Quality Report — ConductionNL/docudesk @ 45203c2

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		❌
stylelint		❌
composer			✅	✅ 108/108
npm			❌	❌
PHPUnit					⏭️
Newman					⏭️
Playwright					⏭️

---

Quality workflow — 2026-05-15 08:11 UTC

Download the full PDF report from the workflow artifacts.

[WilcoLouwerse]

REQUEST_CHANGES — Strict mode. 6 blockers, 5 significant issues.

🔴 Blockers (6): RBAC bypass on prohibition CRUD; prohibition lock bypass via publicationDecision; silent write on referent lookup failure; NcSelect missing inputLabel (ADR-004/gate-12, protected); NcDialog inline modal (ADR-004/gate-13, protected); \OC::$server->get() service-locator.

🟡 Significant (5): loadStandingConsents full table scan with no scope filter; ConsentDetail.vue uses raw fetch() instead of axios; PolicyRetroactiveService full table scan; severity enum mismatch between register JSON and frontend; WAVE-1-SMOKE-TESTS.md in repo root with hardcoded credentials.

All CI failures are pre-existing on the development branch.

[github-actions]

Quality Report — ConductionNL/docudesk @ 33b53c2

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		❌
stylelint		❌
composer			✅	✅ 108/108
npm			❌	❌
PHPUnit					⏭️
Newman					⏭️
Playwright					⏭️

---

Quality workflow — 2026-05-15 14:56 UTC

Download the full PDF report from the workflow artifacts.

[github-actions]

Quality Report — ConductionNL/docudesk @ 4124c02

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		❌
stylelint		❌
composer			✅	✅ 108/108
npm			❌	❌
PHPUnit					⏭️
Newman					⏭️
Playwright					⏭️

---

Quality workflow — 2026-05-18 08:46 UTC

Download the full PDF report from the workflow artifacts.

[rjzondervan]

Review response — commits 3c90dd1 and 3af8e7d

Addressed every blocker + every significant item. 3af8e7d is a follow-up fix on top of 3c90dd1 for a service-locator threading bug I introduced when removing the original service-locator (caught during testing — listener was passing 5 args to handlers that now expect 6).

Blockers

• RBAC bypass on prohibition CRUD. PolicyCrudService::createProhibition, updateProhibition, and deleteProhibition no longer call ObjectService with _rbac: false without a group check. New private assertProhibitionPermission(string $action) mirrors assertStandingConsentPermission and gates writes on admin role or membership in the new docudesk-prohibition-admins group. Throws RuntimeException otherwise (controller maps to HTTP 403).
• Service-locator anti-pattern in DocuDeskEventHandler. Removed the \OC::$server->get(PolicyRetroactiveService::class) call inside dispatchPolicyRetroactive. The retroactive service is now passed as a method parameter from each public event-handler entry point (handleObjectCreated / handleObjectUpdated / handleObjectDeleted), so Nextcloud's event dispatcher injects it via reflection-based DI — no static accessor, no NC30 breakage. Follow-up commit 3af8e7d also updates DocuDeskEventListener::dispatchEvent to pull PolicyRetroactiveService via its existing service-locator pattern (the listener runs outside the DI container) and thread it through the three handler calls.
• Prohibition lock bypass via publicationDecision. ConsentUpdateHandler::guardPolicyPreemptedTransition now checks BOTH consentStatus AND publicationDecision for a change against the existing record. A PATCH carrying only publicationDecision: "publish" on a prohibition-matched record now raises InvalidArgumentException with the rejected field name. The previous guard only watched consentStatus; the publish field was a quiet bypass.
• Silent write on policyMatch lookup failure. ConsentService::assertPolicyMatchReferentValid no longer swallows ObjectService lookup errors with a warning log. A failed lookup throws InvalidArgumentException (mapped to HTTP 400), with the underlying exception preserved as previous. A write referencing an unresolvable policyMatch UUID is rejected outright.
• NcSelect missing inputLabel (ADR-004 gate-12). Added :input-label="…" to every NcSelect in ProhibitionIndex.vue, ProhibitionFormModal.vue, and StandingConsentIndex.vue. Entity-type, severity, match-type, and consent-method dropdowns now have a labelled control for screen readers.
• NcDialog inline (ADR-004 gate-13). Extracted the prohibition create/edit dialog from ProhibitionIndex.vue into a new ProhibitionFormModal.vue component. The parent now owns only the open flag + record-being-edited; the modal hydrates its own form state on open change and emits submit(data) / update:open / cancel. The dialog and its match-rule logic, validation, and options are no longer embedded in the index view.

Significant items

• loadStandingConsents full table scan. PolicyMatchService now pushes scope=entity AND active=true filters down to ObjectService::findAll instead of loading every publicationConsent row and filtering in PHP. The defensive PHP scope check is retained as a belt-and-braces.
• Raw fetch() in ConsentDetail.vue. Replaced both fetch(OC.generateUrl(...)) calls in refreshPolicyMatch with axios.get(generateUrl(...)), picking up the app's standard auth headers + CSRF tokens + error envelope. Added the axios and generateUrl imports.
• PolicyRetroactiveService full table scan. loadInFlightDocumentRecords now adds scope=document to the findAll filter so the read is bounded server-side. Same defensive scope check retained.
• Severity enum mismatch. The frontend severityOptions (was ['low','standard','high','critical'], default 'standard') did not match the publicationProhibition.severity enum in docudesk_register.json (['high','medium','low'], default 'medium'). Frontend corrected to mirror the schema; severityColorMap updated to match. Schema is source of truth.
• WAVE-1-SMOKE-TESTS.md in repo root with hardcoded credentials. Moved to docs/testing/wave-1-smoke-tests.md. Hardcoded admin/admin credentials replaced with NC_USER / NC_PASS env-var placeholders and a clear "local dev only" disclaimer at the top.

Quality

• PHPCS clean on every touched lib file (one 130-char warning on a required sprintf format string; under the error threshold).
• Psalm: no errors on the six touched lib files.
• PHPStan: no errors on the six touched lib files.
• Test bootstrap requires a running NC stack (pre-existing); not exercised here.

[WilcoLouwerse]

Re-review (Strict). 7 of 8 prior findings resolved. RBAC helper, service-locator removal from DocuDeskEventHandler, prohibition-lock guard on both consentStatus and publicationDecision, assertPolicyMatchReferentValid now throws, ProhibitionFormModal.vue extracted, NcSelect :input-label added in ProhibitionIndex.vue / ProhibitionFormModal.vue / StandingConsentIndex.vue, scope filters pushed to DB in both PolicyMatchService and PolicyRetroactiveService, raw fetch() replaced with axios.get, severity enum aligned to schema.

Blockers (2 🔴):

• PolicyController missing RuntimeException → 403 catch on createProhibition — RuntimeException extends Exception, so the new RBAC gate falls through and returns HTTP 500 instead of 403. Same defect on updateProhibition and deleteProhibition. Standing-consent counterparts already have the correct catch block; copy the pattern.
• Inline <NcDialog> in StandingConsentIndex.vue — same ADR-004 gate-13 violation that was fixed for ProhibitionIndex.vue. Extract into StandingConsentFormModal.vue.

Concerns (2 🟡):

• Two-step policyMatch nulling bypass in ConsentUpdateHandler::guardPolicyPreemptedTransition.
• <NcSelect> missing :input-label in AnonymizationWidget.vue (ADR-004 gate-12).

[github-actions]

Quality Report — ConductionNL/docudesk @ 3ed0e7f

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
composer			✅	✅ 108/108
npm			✅	✅ 529/529
PHPUnit					✅
Newman					⏭️
Playwright					⏭️

Coverage: 0% (0/10 statements)

---

Quality workflow — 2026-05-19 03:22 UTC

Download the full PDF report from the workflow artifacts.

[rjzondervan]

@WilcoLouwerse — addressed in 27e4e53. Details below mapped to the four findings.

🔴 RBAC 403 swallowed in 3 places

Added catch (RuntimeException $e) → 403 to PolicyController::createProhibition, ::updateProhibition, and ::deleteProhibition, mirroring the existing pattern on the standing-consent counterparts. RuntimeException was already imported. Order: RuntimeException (403) → InvalidArgumentException (400 — where present) → Exception (500).

After the fix, assertProhibitionPermission() throws on auth failure → caught by the RuntimeException block → returns 403 with the gate's message. Verified by reading each method end-to-end.

🔴 Inline <NcDialog> in StandingConsentIndex.vue

Extracted into src/views/policy/StandingConsentFormModal.vue, mirroring ProhibitionFormModal.vue 1:1:

• Props: open, editingRecord, saving, formError
• Emits: update:open, submit, cancel
• Parent owns: open flag, the record being edited, the save outcome
• Modal owns: form state (blankForm(), validation, match-rule add/remove)

StandingConsentIndex.vue updated to import + register the new component and pass props through. The inline form-scoped CSS (.standing-consent-form, .match-rule-row, .form-warning, .form-error) moved into the modal where it applies; .policy-stats stays in the index because it's outside the modal.

ADR-004 gate-13 compliance pattern now matches the prohibition side. Future modals in this directory follow the same shape.

🟡 policyMatch null-clearing bypass

Closed in ConsentUpdateHandler::guardPolicyPreemptedTransition. Added an explicit guard immediately after the early-return-on-already-null-match check:

if (array_key_exists('policyMatch', $data) === true) {
    $newMatch = $data['policyMatch'];
    if ($newMatch === null || $newMatch === '') {
        throw new InvalidArgumentException(
            message: sprintf(
                'policyMatch cannot be cleared on a policy-pre-empted record (existing=%s). ...',
                (string) $existingMatch
            )
        );
    }
}

The check fires when (a) the record was already pre-empted ($existingMatch !== null, established by the early-return above this block), AND (b) the incoming PUT carries policyMatch with a null or empty value. The message identifies the existing match value (which is non-PII — it's a policy UUID/slug) and explains that creating a new consent record is the correct action when the policy no longer applies.

Alternative considered (strip policyMatch from the writable field set entirely): rejected because policyMatch IS written at consent-create time by the policy-engine flow; an absolute write block would break create. The conditional-on-clearing approach preserves create while closing the update bypass.

🟡 NcSelect missing :input-label in AnonymizationWidget.vue

Added :input-label="t('docudesk', 'Grondslagen')" on the grondslagen multi-select. ADR-004 gate-12 / WCAG 2.1 AA 1.3.1+4.1.2 compliance restored — screen readers now associate the combobox with its label via NcSelect's built-in wiring rather than relying on external <label> markup that the component doesn't see.

---

Re-requesting review.

[github-actions]

Quality Report — ConductionNL/docudesk @ 02c29ce

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	❌
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
composer			✅	✅ 108/108
npm			✅	✅ 529/529
PHPUnit					⏭️
Newman					⏭️
Playwright					⏭️

---

Quality workflow — 2026-05-19 10:09 UTC

Download the full PDF report from the workflow artifacts.

[WilcoLouwerse]

2 new blockers and 2 concerns. 12 of 14 prior findings resolved; 1 prior 🟡 (service-locator) still open and worsened.

🔴 New blockers

• guardPolicyPreemptedTransition over-reach breaks standing-consent toggle — ConsentUpdateHandler.php:214. Fix for [3259350068] closed the bypass but the publicationDecisionChanged arm now also rejects the spec-mandated standing-consent override. ConsentDetail.vue::onToggleAnonymise will return HTTP 400 in production.
• phpcs CI regression — ConsentUpdateHandler.php:191. Squiz.Strings.ConcatenationSpacing violated by the new sprintf continuation. phpcbf auto-fixes it.

🟡 Concerns

• PROHIBITION_GROUP constant mismatches spec/schema — code says docudesk-prohibition-admins, spec + JSON schema both say docudesk-policy-admins. Provisioning is broken either way.
• Service-locator anti-pattern still open and worsened — the fix wired PolicyRetroactiveService by adding a 4th \OC::$server->get() call instead of refactoring to constructor injection.

Verified resolved (12): RBAC 403 catches × 3 in PolicyController, modal isolation for StandingConsent, NcSelect inputLabel × 2, group gates added across PolicyCrudService, silent-write fix in ConsentService, scope-filter in PolicyMatchService, raw fetch→axios in ConsentDetail, ProhibitionFormModal extraction, two-step bypass closure (subject to the over-reach blocker above).

[github-actions]

Quality Report — ConductionNL/docudesk @ ea56370

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		❌
stylelint		❌
composer			❌	✅ 108/108
npm			❌	❌
PHPUnit					⏭️
Newman					⏭️
Playwright					⏭️

---

Quality workflow — 2026-05-20 07:39 UTC

Download the full PDF report from the workflow artifacts.

[WilcoLouwerse]

Re-review — 3113294df5 (Strict mode, 3rd pass)

Resolved correctly (3 of 4):

• Thread A — DocuDeskEventListener DI refactor: all six dependencies are now constructor-injected private readonly properties, no \OC::$server calls remain.
• Thread C — phpcs ConcatenationSpacing fixed by extracting the sprintf into a $msg = ...; throw new ...(message: $msg) shape; phpcs clean on touched file.
• Thread D — PROHIBITION_GROUP constant renamed to docudesk-policy-admins; no stale references in the diff.

Not resolved — Thread B is still blocking (this re-review's only 🔴):

🔴 Thread B fix is dead code — matchKind is never persisted — The fix commit's claim that matchKind is "already persisted by ConsentService::buildConsentData at consent-creation time, line 147" is incorrect; line 147 is a $this->logger->info() context entry, not a save. None of the four return branches of buildConsentData (lines 178–251) write matchKind into the array passed to saveObject. The new carve-out in ConsentUpdateHandler::guardPolicyPreemptedTransition reads $existing['matchKind'] which is always null, so the standing-consent override toggle remains 400-locked — exact same UX failure Thread B reported.

New concerns introduced by the fix commit:

🟡 Wave-1 smoke-test pre-flight still asserts schema version 7.0.0 after the fix commit bumped lib/Settings/docudesk_register.json to 8.0.0. Pre-flight checklist + PR-body test plan both stale.

🟡 matchKind enum lacks null — once the blocker is fixed, WOO-default consent records will need to carry null for this field; current enum ["prohibition", "standing_consent"] rejects it.

🟢 CI failures (License (npm), Security (composer), Security (npm), eslint, stylelint) are pre-existing on development — no package.json / composer.json changes in this PR's diff. Informational, not blocking on this PR specifically.

Thread housekeeping: Threads A, C, D landed correctly and can be resolved; Thread B's blocker comment supersedes the original Thread B and stays open as the active blocker. (I attempted to resolve A/C/D programmatically but was blocked by an automated guardrail — please resolve them manually if desired, or leave them for the next pass.)

[github-actions]

Quality Report — ConductionNL/docudesk @ 27ec986

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		❌
stylelint		❌
composer			❌	✅ 108/108
npm			✅	✅ 575/575
PHPUnit					⏭️
Newman					⏭️
Playwright					⏭️

---

Quality workflow — 2026-05-21 11:52 UTC

Download the full PDF report from the workflow artifacts.

[rjzondervan]

Third-pass review addressed — d12bb3c

All three findings from your 3rd-pass review (commit 3113294df5) are addressed in d12bb3c.

🔴 Thread B — matchKind was dead code — fixed for real this time.
You were right: ConsentService::buildConsentData:147 was the logger info() context entry, not a save. None of the four return branches of buildConsentData (lines 178–251 on the prior diff) wrote matchKind into the array passed to saveObject, so ConsentUpdateHandler::guardPolicyPreemptedTransition read $existing['matchKind'] as null and the standing-consent carve-out never fired.

Persisted matchKind across all four branches:

• policyMatch === null (WOO defaults) → matchKind: null
• kind === KIND_PROHIBITION → matchKind: 'prohibition' (the constant)
• kind === KIND_STANDING_CONSENT → matchKind: 'standing_consent'
• Unknown-kind defensive WOO fallback → matchKind: null

Used PolicyMatchService::KIND_* constants so the literal can't drift.

Three new reflection-backed unit tests in ConsentUpdateHandlerTest lock the three branches of the guard:

• testStandingConsentCarveOutAllowsPublicationDecisionOverride — the regression case: standing-consent override now passes (no exception).
• testProhibitionMatchRejectsPublicationDecisionOverride — prohibition stays strictly locked.
• testNoPolicyMatchAllowsArbitraryTransition — records with no policyMatch early-return from the guard.

🟡 matchKind enum lacked null — fixed.
With Thread B's blocker resolved, WOO-default consent records now carry matchKind: null in their payload. Schema updated to permit it explicitly:

• "type": ["string", "null"] (JSON Schema's nullable idiom)
• "enum": ["prohibition", "standing_consent", null]
• Extended the Dutch description to call out the WOO-default null case + the symmetry with policyMatch=null.

🟡 Smoke-test pre-flight still asserted schema 7.0.0 — fixed.
docs/testing/wave-1-smoke-tests.md updated to 8.0.0. The PR body test plan was also stale on the same value; updated via the GitHub API in this same change set.

🟢 Carry-over — CI failures (License/Security/eslint/stylelint) are pre-existing on development, no package.json / composer.json changes in this PR's diff.

Verification

• PHPCS clean on both touched code files (ConsentService.php, ConsentUpdateHandlerTest.php) — one pre-existing warning on ConsentService.php unchanged.
• vendor/bin/phpunit --filter 'ConsentUpdateHandler|ConsentService' in the live master-nextcloud-1 container: 19 tests / 27 assertions (up from 16/24 — added 3 guard tests).
• lib/Settings/docudesk_register.json parses as valid JSON.

Threads A, C, D from your earlier passes are landed and can be resolved on your side. Thread B's blocker comment is the active one — should be fully closed now.

Refs: review 4335767919

[WilcoLouwerse]

Re-review — d12bb3c (Strict mode, 4th pass)

Resolved correctly (3 of 3 prior findings):

• 🔴 Thread B (matchKind dead code) — buildConsentData now writes matchKind in all four return branches; the guard reads $existing[matchKind] and the literal standing_consent matches PolicyMatchService::KIND_STANDING_CONSENT. End-to-end carve-out flow is sound.
• 🟡 Pre-flight schema version 7→8 — docs/testing/wave-1-smoke-tests.md updated.
• 🟡 matchKind enum lacks null — schema now "type": ["string", "null"], enum includes null. WOO-default + legacy null-symmetry restored.

New blocker introduced by this commit:

🔴 2-step prohibition lock bypass via client-controlled matchKind mutation — making matchKind a persisted field without rejecting client-supplied mutations of it lets any authenticated user (ConsentController::update is @NoAdminRequired) defeat any of the four prohibition seeds in two PUTs: first {matchKind: "standing_consent"} slips past the both-fields-false early-return and array_merge corrupts the field, then {publicationDecision: "publish"} fires the now-applicable standing-consent carve-out and the lock is gone. policyMatch is similarly mutable to another non-empty UUID. Suggested fix in the inline comment: reject any client-supplied change to matchKind or policyMatch inside guardPolicyPreemptedTransition, with two unit tests locking the regression shut.

🟢 PHPUnit job is skipped on this head SHA per the check-runs API — the new reflection-based tests in ConsentUpdateHandlerTest provide no CI regression protection. Same observation as the prior pass; informational. Vue ESLint, stylelint, Security composer failures remain pre-existing on development.

Thread housekeeping: Threads on the three resolved findings (matchKind dead code, pre-flight version, enum-lacks-null) plus the superseded over-reach thread from the 2nd pass can be resolved. The new blocker above is the only active item.

[rjzondervan]

Fourth-pass review addressed — 1d408cb

The new blocker is closed. Applied the exact fix you suggested in discussion_r3281603312 plus 3 regression tests.

🔴 Blocker — 2-step prohibition lock bypass via matchKind/policyMatch mutation — fixed.

The policyMatch-clearing-only guard is replaced with a broader server-controlled-fields-immutable check covering both matchKind AND policyMatch. The check is positioned BEFORE the both-fields-false early-return — exactly per your inline comment — so a payload carrying only matchKind can't slip through the merge:

foreach (['matchKind', 'policyMatch'] as $serverOnlyField) {
    if (array_key_exists($serverOnlyField, $data) === true) {
        $newValue      = $data[$serverOnlyField];
        $existingValue = ($existing[$serverOnlyField] ?? null);
        if ($newValue !== $existingValue) {
            throw new InvalidArgumentException(
                sprintf('%s is server-controlled and cannot be modified via update (existing=%s, attempted=%s).', ...)
            );
        }
    }
}

The old policyMatch-clearing case is subsumed: clearing to null IS a mutation away from a non-null existing value, so it still throws (same UX, broader coverage). UUID swap to a different non-empty value — the second shape of the bypass you flagged — is now rejected too.

Tests added (3 new):

1. testProhibitionMatchKindMutationRejected — the step-1 exploit attempt. Asserts InvalidArgumentException with /matchKind is server-controlled/.
2. testProhibitionPolicyMatchUuidSwapRejected — UUID-swap shape. Asserts InvalidArgumentException with /policyMatch is server-controlled/.
3. testEqualServerControlledValuesAreAllowed — idempotency anchor. A PUT carrying equal values for both fields MUST NOT trip the guard. Locks the regression at the other end (over-strict guards that break clients re-sending full record state).

🟢 PHPUnit CI carry-over — acknowledged. The new reflection-based tests provide unit-level coverage but won't trigger CI on this branch until the workflow trigger is fixed. Flagged separately; not in this PR's scope.

Verification

• vendor/bin/phpunit --filter ConsentUpdateHandler — 9 tests / 13 assertions all green in the master-nextcloud-1 container (up from 6 / 8 — added 3 regression tests, no others affected).
• PHPCS clean on both touched files (cosmetic equals-alignment fixes via phpcbf included).

The new-blocker thread is the only open item; the three resolved-correctly threads from your 4th-pass summary plus the superseded over-reach thread can be resolved on your side.

Refs: review 4337447966, inline 3281603312

[github-actions]

Quality Report — ConductionNL/docudesk @ 2e5196c

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		❌
stylelint		❌
composer			❌	✅ 108/108
npm			✅	✅ 575/575
PHPUnit					⏭️
Newman					⏭️
Playwright					⏭️

---

Quality workflow — 2026-05-21 14:34 UTC

Download the full PDF report from the workflow artifacts.

[WilcoLouwerse]

🔴 Blocker — Same bypass class exists on the CREATE endpoint ($extra overrides server-computed policy fields)

The fourth-pass 2-step UPDATE bypass is correctly closed — the new foreach (['matchKind', 'policyMatch'] as $serverOnlyField) guard in ConsentUpdateHandler::guardPolicyPreemptedTransition sits before the both-fields-false early-return, the regression tests lock the shape, well done. But the symmetric bypass on the CREATE endpoint is wide open.

This line does array_merge($consentData, $extra) — $extra is the second argument, so caller-supplied values override the server-computed buildConsentData output. ConsentCrudService::createFromRequest only strips documentId, entityType, entityText, _route, _method from the request body before forwarding everything else as $extra. The downstream validatePublicationConsentData checks scope constraints + verifies the policyMatch UUID resolves to a real referent — but does not cross-check matchKind against the referent kind, nor that consentStatus/publicationDecision/notificationStatus/objectionDeadline are consistent with the policy outcome. ConsentController::create is @NoAdminRequired + @NoCSRFRequired.

Exploit A — fabricate standing-consent to bypass the WOO objection deadline (privacy escalation):

GET /apps/docudesk/api/policy/standing-consents
  → returns 4 seed UUIDs (publicly readable to authenticated users)

POST /apps/docudesk/api/consents
  body: {
    "documentId":         "<victim-doc-id>",
    "entityType":         "PERSON",
    "entityText":         "victim-name",
    "policyMatch":        "<real-standing-consent-uuid>",
    "matchKind":          "standing_consent",
    "consentStatus":      "consent_given",
    "publicationDecision":"publish_with_consent",
    "notificationStatus": "skipped",
    "objectionDeadline":  null
  }

→ buildConsentData() returns WOO defaults (entity didn't actually match a policy)
→ array_merge($consentData, $extra) overwrites every field with attacker values
→ validatePublicationConsentData passes (scope=document, has documentId,
  policyMatch UUID resolves to a valid standing-consent referent)
→ Record persists with consentStatus=consent_given, no objection deadline
→ The entity publishes unredacted, bypassing the WOO objection window that
  should have protected it 💥

Exploit B — DOS publication of any document by stamping a fake prohibition: swap the $extra values to matchKind: "prohibition" + policyMatch: <real-prohibition-uuid> + consentStatus: anonymized + publicationDecision: anonymize. The new fourth-pass UPDATE guard now makes the record immutable from the legitimate owner's side, so this is a persistent lock rather than a one-shot edit.

This is the same security regression class as the fourth-pass UPDATE bypass — symmetrical, applied at the create path that doesn't go through guardPolicyPreemptedTransition at all. The threat model that motivated the fourth-pass fix applies equally here.

Suggested fix — strip server-controlled fields from $extra before the merge. The cleanest location is upstream in ConsentCrudService::createFromRequest:

public function createFromRequest(array $data, string $register, string $schema): array
{
    $knownFields = ['documentId', 'entityType', 'entityText'];
    $extra       = array_diff_key($data, array_flip($knownFields));
    unset($extra['_route'], $extra['_method']);

    // Server-controlled fields — populated by ConsentService::buildConsentData
    // from the policy match result. Caller cannot inject these without
    // forging the policy outcome (see PR #147 fifth-pass review).
    foreach (['policyMatch', 'matchKind', 'consentStatus', 'publicationDecision',
              'notificationStatus', 'objectionDeadline'] as $serverOnlyField) {
        unset($extra[$serverOnlyField]);
    }

    return $this->consentService->createConsentRequest(
        $data['documentId'], $data['entityType'], $data['entityText'],
        $register, $schema, $extra
    );
}

Alternatively, gate at the createConsentRequest level (defense in depth — $extra is a public API surface for the service) — pick whichever you prefer. The unset list could also throw InvalidArgumentException instead of silently stripping, matching the UPDATE guard's behaviour for symmetric error reporting.

Regression-test stub:

public function testCreateRejectsInjectedPolicyFields(): void
{
    // arrange: POST body carries server-controlled fields the attacker shouldn't be able to set
    $data = [
        'documentId'          => 'doc-1',
        'entityType'          => 'PERSON',
        'entityText'          => 'Jan Janssen',
        'policyMatch'         => 'attacker-supplied-uuid',
        'matchKind'           => 'standing_consent',
        'consentStatus'       => 'consent_given',
        'publicationDecision' => 'publish_with_consent',
    ];
    $this->policyMatcher->method('match')->willReturn(null); // no real match

    // act
    $result = $this->service->createConsentRequest(
        documentId: 'doc-1', entityType: 'PERSON', entityText: 'Jan Janssen',
        register: 'consent', schema: 'publicationConsent',
        extra: ['policyMatch' => 'attacker-supplied-uuid', 'matchKind' => 'standing_consent',
                'consentStatus' => 'consent_given', 'publicationDecision' => 'publish_with_consent']
    );

    // assert: the saved record carries the server-computed WOO defaults, not the injected values
    self::assertNull($result['policyMatch']);
    self::assertNull($result['matchKind']);
    self::assertSame('pending', $result['consentStatus']);
    self::assertSame('pending', $result['publicationDecision']);
}

[WilcoLouwerse]

🟢 Minor — testing belts-and-braces for the new immutability guard

The new foreach (['matchKind', 'policyMatch'] as $serverOnlyField) guard at ConsentUpdateHandler.php:190-205 correctly catches policyMatch: null clearing (because null !== "<uuid>" is true), and the three new tests cover mutation, UUID swap, and idempotent re-send. Two adjacent shapes are not explicitly regression-locked — current code is safe, this is belts-and-braces:

1. Explicit policyMatch: null clearing test. The third-pass guard was clearing-only; the new general guard subsumes it. A dedicated testProhibitionPolicyMatchClearToNullRejected would lock the contract so a future refactor that changes the comparison shape (e.g. string coerce, (string)(null ?? '') flowing in) doesn't silently re-open the clearing bypass.

2. existingMatch === null short-circuit test. The guard returns early at lines 175–177 for records with no prior policy match. The bypass chain is broken there because the standing-consent carve-out (lines 234–242) also short-circuits in that branch, but a test that documents "never-matched records: server fields can be set freely via update, by design" would anchor the contract against a future change to the guard's first check.

Neither is a security issue today — flagging only because of the policy-fields class of bug (this PR's recurring theme).

[WilcoLouwerse]

Fifth-pass review — 1d408cb. The fourth-pass 2-step UPDATE bypass is correctly closed (immutability guard at ConsentUpdateHandler.php:190-205, three regression tests, all on the right side of the early-return — clean fix). Prior thread resolved.

One new 🔴 found in the same security regression class: the CREATE endpoint has the symmetric bypass — $extra overrides server-computed policyMatch / matchKind / status-trio via array_merge in ConsentService.php:130. Any authenticated user can fabricate a standing-consent on a victim document and bypass the WOO objection deadline, or DOS publication via a fake prohibition (now persistent thanks to the new immutability guard). Inline comment has the full exploit walk-through + suggested fix (strip in ConsentCrudService::createFromRequest) + regression-test stub.

Also flagged one 🟢 top-level comment about testing belts-and-braces on the new immutability guard. Not blocking.

Fix the create-time injection and we're done.