Add OpenSpec changes: extend-schemas-in-register-service + validate-self-folder-access (anonymisation scaffolding)

[rjzondervan]

Adding specifications for two issues:

• [OpenSpec] [openregister] extend-schemas-in-register-service #1428
• [OpenSpec] [openregister] validate-self-folder-access #1342

These specifications are primarily for fixing functionality to enable extending schema's within a register when requested, and update the behaviour of openregister regarding connecting folders to an object.

These issues address functionality that will later be needed in the Anonymisation project (Docudesk), for connecting dossiers to custom folders in the Nextcloud file system, and fixing the functionality of Docudesk to choose proper schemas in the app settings.

[WilcoLouwerse]

🟡 Missing PR description

This PR makes non-trivial changes (12 files, +1260 lines) but has no description. Please add a description covering:

• What changed and why
• Any behavioral differences reviewers should know about
• Links to related issues, specs, or prior art

A reviewer cannot confidently verify intent against the diff without this context.

[github-actions]

Quality Report — ConductionNL/openregister @ de05318

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

---

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

Download the full PDF report from the workflow artifacts.

[MWest2020]

[extend-schemas] Tegenspraak over properties-stripping — bewust?

Drie documenten in dezelfde change spreken elkaar tegen:

• proposal.md (Capabilities → schemas): "with properties filtered, matching current controller behavior"
• design.md (Decision 5): "properties stripping is deferred to the consumer — the serializer matches the controller's current behavior, which does NOT strip properties"
• specs/register-service-extensions/spec.md (Requirement: schemas extension): "The properties field of each schema MUST be preserved — the serializer MUST NOT strip it."

Niet blocking, maar lijkt me iets om vóór implementatie eenduidig te krijgen — anders krijgt de uitvoerder twee tegenstrijdige instructies. Welke is de bedoeling: behouden (design + spec) of strippen (proposal)?

[MWest2020]

[cross-cutting] Inconsistente artefactstructuur tussen de twee changes — opzettelijk?

validate-self-folder-access/ heeft een plan.json (416 regels, machine-leesbaar met acceptance_criteria / files_likely_affected / status) plus tasks.md (vrijwel dezelfde inhoud in markdown).

extend-schemas-in-register-service/ heeft alleen tasks.md, geen plan.json.

Niet blocking, maar: is plan.json het nieuwe format dat we breed willen, alleen voor security-changes, of een experiment van één van beide? En zo ja, wil je de duplicatie met tasks.md daarbinnen oplossen (één bron, niet twee)?

[MWest2020]

[extend-schemas] HTTP-response shape change voor orphan IDs — staat niet in proposal.md

design.md Decision 5 en spec.md (Requirement: Missing schema ID) leggen helder uit dat de orphan-ID-retentie een deliberate divergence is van het huidige RegistersController::index()-gedrag (dat orphans stilletjes dropt). Dat verandert dus de response shape van /api/registers?_extend=schemas voor de edge case.

In proposal.md zie ik dat niet terug — alleen "no breaking changes to the HTTP API" zonder nuance.

Niet blocking — design en spec dekken het — maar voor reviewers die alleen de proposal lezen is dit een verrassing. Eén regel onder "What Changes" of "Impact" zou helpen?

[MWest2020]

[beide changes] Open Questions — al beantwoord, of bewust open gehouden?

Beide design.md's hebben een Open Questions-sectie die de spec minder definitief maakt:

extend-schemas/design.md:

• "Should unknown _extend values warn?" — wordt elders al beantwoord (silently ignored, matches controller). Mag dichtgespijkerd?
• "Do we want findSerialized / findAllSerialized?" — deze staan al in tasks.md 2.2/2.3 en in spec.md als MUST. Lijkt me beslist?

validate-self-folder-access/design.md:

• "Should we also verify the folder is a Folder (vs a File)?" — spec scenario "Binding to a file fails with 403" antwoordt al ja. Mag weg.
• "Controller-level HTTP-403 mapping op elk endpoint of base class?" — implementeerder moet dit weten vóór sectie 5 start.

Niet blocking, maar: zijn deze bewust open gelaten voor de implementator om te beslissen, of ben je vergeten ze door te halen toen het antwoord vorm kreeg?

[MWest2020]

[extend-schemas] Acceptance criterion 7.3 verifieert een non-event — bedoeling?

tasks.md 7.3:

Manually verify (in a local dev environment) that DocuDesk's admin settings schema dropdown remains empty on the current OpenRegister build after this change — confirming no accidental fix-through-magic.

Omdat DocuDesk in deze change niet wordt aangepast (expliciet non-goal), is "dropdown blijft leeg" per definitie waar — er is geen kanaal waardoor het wel zou kunnen werken. De intentie snap ik (sanity check dat de fix niet per ongeluk doorheen lekt), maar dit is eerder iets voor de DocuDesk follow-up PR dan een acceptance criterion voor deze change.

Niet blocking — overwegen om weg te halen of te herformuleren?

[WilcoLouwerse]

Inline findings for PR #1427 — Thorough review.

[WilcoLouwerse]

🔴 Blocker — PR title says "anonymisation project" but ships zero anonymisation content

The PR title is "Add specs for the anonymisation project". A full-text search across the 1332-line diff for anonymi returns zero matches. The two changes shipped are (a) extend-schemas-in-register-service — refactoring _extend honour for RegisterService, no anonymisation logic; (b) validate-self-folder-access — folder access-control hardening, no anonymisation logic. Neither proposal mentions anonymisation, pseudonymisation, GDPR Art. 4(5), reidentification, or PII handling.

The newly-added PR description clarifies what the changes are (extending schemas + folder access control) but doesn't bridge them back to the "anonymisation project" framing in the title.

Impact: Reviewers and stakeholders sourced into this PR by the title (security officers, DPOs, the anonymisation-project sponsor) will not find what they were looking for. Cross-references from other tickets / Specter outputs / project trackers that point at "PR 1427 — anonymisation specs" become misleading. If the changes here are scaffolding for a later anonymisation change, that intent is invisible in the artefacts. A reviewer cannot evaluate "does this serve the anonymisation project?" because the proposals don't claim that lineage.

Suggested fix: Either (a) rename the PR title to honestly reflect the contents — e.g. "Add OpenSpec changes for register _extend honour + @self.folder access control" — or (b) add an explicit paragraph at the top of each proposal.md (and the PR description) explaining how this scaffolding feeds the anonymisation project (e.g. "the dossier-anonymisation pipeline depends on accurate schema expansion via DI and on hardened folder binding to prevent cross-tenant leaks of pseudonymised records").

[WilcoLouwerse]

3 blockers require fixes before merge — canonical spec landed before archive, plan.json shipped at proposal time, and PR title says "anonymisation" but ships none. Both proposals are well-reasoned (especially the validate-self-folder-access design narrative) — the issues are workflow-packaging and missing definitions (default-deny invariant, "self" definition, wire-format breaking change, DocuDesk cross-repo link); 7 🟡 concerns + 3 🟢 minors detailed inline. Note: quality / Security (composer) and quality / Integration Tests (Newman) show as failing on this commit but are unrelated to this docs-only PR.

[rjzondervan]

Addressed in b684e6c

Thanks both for the careful read. Net: −1020 / +185 lines, all in openspec/. TL;DR — three blockers fixed, contradictions resolved, line numbers and plan.json / canonical-spec artefacts removed.

Blockers

#	Concern	Fix
1	Canonical spec landed before archive	Deleted openspec/specs/self-folder-access-control/spec.md — will be regenerated by /opsx:archive after the change merges and is implemented.
2	plan.json shipped at proposal time	Deleted both plan.json files. Tracking issues already exist (#1428, #1342); the JSON itself was Claude-generated and got committed by mistake. Going forward, /opsx:plan-to-issues runs separately and the result is not committed.
3	PR title says "anonymisation" but ships zero anonymisation content	Added an "Anonymisation lineage" paragraph at the top of both proposals explaining the dependency chain (DocuDesk's anonymisation pipeline depends on schema expansion + folder-bind hardening). PR title renamed to "Add OpenSpec changes: extend-schemas-in-register-service + validate-self-folder-access (anonymisation scaffolding)".

Concerns (Wilco)

• 🟡 tasks.md format → reformatted both files to the flat - [ ] N. / - [ ] N.M checkbox tree per openspec/AGENTS.md.
• 🟡 Default-deny posture → added a top-line "Default-deny invariant for @self.folder binds" requirement. Anything that doesn't end in "and isReadable() returned true for this user" is a denial.
• 🟡 "self" definition → added a "Definition of self" requirement: explicit IUser arg first, then IUserSession::getUser(), otherwise deny. No implied identities.
• 🟡 Rate-limit / alert hook → reframed in the proposal: this change ships the audit input, not the alarm. Rate-limiting / anomaly detection / operator alerting are explicitly out-of-scope follow-up work.
• 🟡 N+1 schema lookup → added a risk row to extend-schemas/design.md. Pre-existing in the controller's loop, preserved verbatim by this refactor — not introduced. Batched findByIds + per-request cache deferred to a separate follow-up so this change stays a behaviour-preserving refactor.
• 🟡 Orphan-ID retention is wire-format breaking → upgraded the framing in proposal What Changes and the design risk row. Now explicitly called out as a wire-format change for typed JSON consumers (Go/Java/Kotlin) requiring a discriminated decoder; will land in the changelog.
• 🟡 No DocuDesk link → added the follow-up description inline in extend-schemas/proposal.md ("DocuDesk follow-up: swap $register->jsonSerialize() for $registerSerializer->serialize($register, ['schemas']) in RegisterDiscoveryService::serializeRegister(). Filed as a follow-up issue once this change merges; not blocking on it."). Will open the DocuDesk issue post-merge.

Concerns (Mark)

• 🟡 properties stripping contradiction → reconciled: spec wins (preserve). Updated proposal and design to match. The serializer is intentionally non-opinionated about properties; consumer-side filtering (DocuDesk's filterSchemaProperties) stays in DocuDesk.
• 🟡 Inconsistent artefact structure → resolved by deleting both plan.json files (see Blocker [Changelog CI] Add Changelog for Version 0.1.2 #2). Both changes now have the same artefact set: proposal.md + design.md + specs/ + tasks.md.
• 🟡 Orphan-ID HTTP shape change not in proposal → fixed (see Wilco's wire-format point above).
• 🟡 Open Questions already answered → converted to "Resolved Questions" sections in both design.md files. The genuinely open one (getNodeById() rootFolder fallback deprecation) moved to a separate "Deferred" subsection.
• 🟡 Acceptance criterion 7.3 verifies a non-event → dropped. Will live as a check on the DocuDesk follow-up PR, where it actually exercises a code change.

Minors (Wilco)

• 🟢 Bare ## Requirements → removed; only ## ADDED Requirements remains.
• 🟢 ADR-014 hedge → ADR-014 is per-app i18n; the right reference is ADR-007 (Security and Auth). Updated.
• 🟢 Hardcoded line numbers → dropped from all four proposal/design files. Symbol references (class + method) remain.

Quality CI

The two failing checks (composer security, Newman) on the prior commit are not docs-related and are about pre-existing default-branch state. This docs-only follow-up shouldn't change them.

[WilcoLouwerse]

✅ Resolved in b684e6c — PR title renamed to "Add OpenSpec changes: extend-schemas-in-register-service + validate-self-folder-access (anonymisation scaffolding)" and both proposals now carry an ## Anonymisation lineage section explaining the dossier-anonymisation dependency chain. Title and content are aligned.

[WilcoLouwerse]

All 12 inline findings + the PR-title finding cleanly resolved in b684e6c3 — premature artefacts removed, default-deny invariant + "self" definition added, breaking-change framing upgraded, and the Anonymisation lineage paragraphs make the title/content alignment honest. Zero new findings; both proposals are merge-ready.