Release: merge development into beta by github-actions[bot] · Pull Request #4580 · ConductionNL/openregister

github-actions · 2026-10-11T07:20:38Z

Automated PR to sync development changes to beta for beta release.

Merging this PR will trigger the beta release workflow.

Reminder: Add a major, minor, or patch label to this PR to control the version bump. Default is patch.

Changelist

Snapshot of development at 8e7bbd8 (16 Oct 2026) against beta at b996608: 46 PRs, 13 more bundled inside them. The chore(sync) PR that carries beta back into development is left out. PRs merged to development after this snapshot are not listed yet.

Mostly the case, draft and forms engine work (decisions 179 to 229), example sets and RBAC refinements, plus the migration-pack editor (#4602) that stackiq's CMDB import uses to show and edit its mapping packs.

…ressions

…draft-lifecycle

…n methods

…before the save

… goes through submit()

…m-destination-validator

…ecycle

…es _status

… prettier-formatted

…ver rate bands A quantity accrues over ordered rate bands, optionally capped (ADR-118 C16): the shape of a stepped daily penalty, tiered pricing or a bracket table, so it is configuration for each of them. The validator now treats a lit operand as data, so a literal rate schedule is no longer refused as malformed.

metadata.completeVia = {app, action} makes the portal seam refuse a direct completion with 409 complete-via-app, naming the action, after the party check and before anything is stored. Only the named app completes the task in-process (PortalTaskService::complete(viaApp: ...)). A portal reads the same key off the task row and opens the app's action instead of the upload form, so an app's own checks (a frozen checklist, a signature, a payment) cannot be skipped by completing the task through the seam. Change a-portal-task-names-the-app-that-completes-it.

…, failing closed A property can cite rows of another schema by a key property (referenceSchema + referenceKey, e.g. a code) and refuse their delete with onDelete RESTRICT. An unreadable citation blocks the delete as unverifiable; a citing table that does not exist yet counts as no citation. The property validator accepts the two keys and refuses referenceKey with any other action. uuid references are unchanged. Generic reference guard, capability C25 of hydra's procedures-are-configuration (decision 182).

…them by key Decision 186: a reusable decision table lives in OpenRegister as a generic capability (ADR-118 C15). A core decision-tables register, refused at save when the evaluator cannot run a table or its key is taken; DecisionTableStore resolves a table by key or uuid and evaluates it for apps; the decision-table flow step takes tableRef as an alternative to an inline table.

…browser e2e, @SPEC on three responder methods)

WilcoLouwerse · 2026-10-15T15:02:41Z

+	 *
+	 * @spec openspec/changes/draft-owner-activation-and-lifetime/specs/form-destination/spec.md#requirement-a-draft-must-not-be-changed-by-somebody-who-cannot-read-it
+	 */
+	public function keptFrom(ObjectEntity $object, Schema $schema, ?string $userId): bool {


🔴 Bulk save lets any user with update rights overwrite another user's draft

A draft is hidden from other users on every read path (SQL RBAC predicate) and ObjectService::refuseSomebodyElsesDraft() refuses a single-object save over someone else's draft; keptFrom() (this method) is the only guard and it has exactly one caller. The bulk path does neither. SaveObjects::resolveSafeguardActionForRow() finds the existing row with _rbac: false, so the draft is "found" and the row becomes an update; the per-row check is PermissionHandler::hasPermission(action: update, objectOwner: draftOwner, object: draft), which has no draft rule. Any user who may update in the schema can therefore POST /api/bulk/{register}/{schema}/save with the draft's uuid and replace its property values (_status is carried forward, so it stays a draft and the owner finds it changed). Reproduces the exact hole fixed for the single path in 853d562, on the bulk route (BulkController::save is #[NoAdminRequired]).

Fix: In SaveObjects safeguard step, after the existing entity is resolved and $effectiveRbac === true, reject the row (same recordSafeguardRejection) when (new DraftStatusPolicy())->keptFrom(object: $existingEntity, schema: $rowSchema, userId: $currentUser?->getUID()) is true. Keep the answer indistinguishable from "not found/denied" as on the single path.

Done when: a bulk save by user B carrying the uuid of user A's draft (schema without draftsVisible) returns the row as rejected, the draft's stored data and _updated are unchanged, and a bulk save by A (the owner) still updates it. Unit seam: the permission step of SaveObjects::safeguardRows (new test beside tests/Unit/Service/Object/SaveObjectDraftStatusTest.php).

Verification: grep -rn "keptFrom" lib --include=*.php → only ObjectService.php:2339 and the definition; grep -n "Draft\|keptFrom" lib/Service/Object/SaveObjects.php → no draft handling besides ObjectEventGate dispatch

Tracked in #4648 so it survives the next re-cut of this sync PR.

🔧 Proposed fix in #4655, commit 728c921 (see the PR body for what changed). The thread stays open until it is merged.

WilcoLouwerse · 2026-10-15T15:02:41Z

+		// the owner's own drafts in view on every path.
+		$drafts = new DraftStatusPolicy();
+		if ($schema !== null && $drafts->draftsVisibleToOthers(schema: $schema) === false) {
+			$reachable = '(' . $reachable . ' AND ' . $drafts->notADraftSql(columnPrefix: $columnPrefix) . ')';


🟡 A table that could not be healed still gets _status in the RBAC predicate

The self-healing change promises that a table whose metadata columns cannot be added "degrades quietly": MagicSearchHandler leaves the lifecycle predicates out for the columns MagicMetadataColumnHealer::missingFor() reports (REQ-MTH-002). reachableRowSqlFor() does not consult it: for every schema without draftsVisible it ANDs (_status IS NULL OR _status <> 'draft') into the read predicate unconditionally. For a non-admin on a degraded table (no ALTER permission, or an ALTER deferred inside an open MySQL transaction, both cases the healer handles by marking the column missing) the query therefore fails with an unknown-column error, while admins (RBAC bypass) get the quiet answer. The same call also feeds buildRbacPredicateForAlias() for related-row joins.

Fix: Give MagicRbacHandler the healer (or a missingFor(table) callback) and skip notADraftSql() when _status is in the missing list, as draftConditionSql() does; the table cannot hold a draft without the column.

Done when: a unit test with the healer reporting _status missing asserts buildRbacRowPredicateSql() contains no _status for a non-admin, and still contains it when the column exists.

Verification: grep -n "metadataColumns\|missingFor\|MagicMetadataColumnHealer" lib/Db/MagicMapper/MagicRbacHandler.php → no match

🔧 Proposed fix in #4655, commit 363e607 (see the PR body for what changed). The thread stays open until it is merged.

WilcoLouwerse · 2026-10-15T15:02:41Z

+
+		// An administrator's re-import is the explicit decision REQ-LCA-003
+		// waits for, for every authorization block either side carries.
+		if ($this->decidedVia() !== null) {


🟡 Under an administrator's decision, a local authorization block disappears when the shipped schema declares none

With a decision in effect, line 165 adds every authorization root found in live OR incoming to the decisions, so AuthorizationBlocks::settle() turns the block's state into UPSTREAM; foldUpstream() with the block absent from incoming then drops it from the merged definition. Reproduced with the classes directly: baseline {title}, live {title, authorization: {read:[staff], update:[staff]}}, incoming {title} under ShippedAuthorizationDecision::run() gives merged {"title":"T2"} and replacedLocal: ["authorization"]. A schema an administrator locked down locally, in an app whose file declares no authorization for it, becomes default-open after --import (and after an example-set install, which runs the same decision). The design (D-3) only discusses a shipped block replacing a local one; open task 3.3 in the same change is still undecided, and every other change in this release treats an ambiguous authorization as deny.

Fix: Only decide roots the incoming definition carries ($this->blocks->roots($incoming)); a root present only in live keeps its normal state (LOCAL is preserved and reported).

Done when: a GuardedDescriptorMergeTest case with incoming lacking authorization and a local block, run under a decision, keeps the local block; the existing replace-when-shipped-has-it cases stay green.

Verification: php /tmp/claude-1000/-home-wilco-hydra/fe360b05-60b2-49fc-a902-849b84a31cd5/scratchpad/t4580.php (calls GuardedDescriptorMerge with a ShippedAuthorizationDecision) → merged lacks authorization, replacedLocal ["authorization"]

🔧 Proposed fix in #4655, commit 03e338e (see the PR body for what changed). The thread stays open until it is merged.

WilcoLouwerse · 2026-10-15T15:02:41Z

+	 *
+	 * @spec openspec/changes/form-destination-validator/specs/form-destination/spec.md#requirement-a-list-must-hide-drafts-unless-asked
+	 */
+	public function sql(string $mode, string $columnPrefix, ?string $owner): ?string {


🟡 The draft lens is spelled once for lists but not for the raw-SQL aggregation fast path

AggregationRunner builds its own WHERE (_deleted, _archived predicates, with a comment saying it must reproduce the list path's defaults) and takes the access rule from MagicMapper::rbacRowPredicateSql(). For an administrator that predicate is null, and no draft predicate is added, so a tile or aggregate counts every user's drafts while the list under it (DraftListLens::sql() EXCLUDE mode) hides them. With a schema's draftsListed: false the same mismatch applies to a user's own drafts (the RBAC owner-admit keeps them in the count; the list's NONE lens drops them). The change's own task 2b.8 says list total and facets must agree; aggregates are the third reader and were not wired to DraftListLens.

Fix: Add the draft predicate to the aggregation fast path from DraftListLens::sql() (mode from modeFor() with no query, so EXCLUDE/NONE), next to the _archived predicate, skipped when the table lacks _status.

Done when: a unit test over the aggregation fast path asserts the generated WHERE contains the draft predicate for an admin and that a count over a table with one draft and one active row returns 1.

Verification: grep -n "_status\|DraftListLens" lib/Service/Aggregation/AggregationRunner.php → no match (while _archived is handled at lines 2554-2556)

🔧 Proposed fix in #4655, commit d4aa263 (see the PR body for what changed). The thread stays open until it is merged.

WilcoLouwerse · 2026-10-15T15:02:41Z

+	 *
+	 * @spec openspec/changes/seed-objects-follow-the-shipped-version/specs/data-import-export/spec.md
+	 */
+	public function advanceVersion(ObjectEntity $entity, ?string $carried): void {


🟢 Any caller who may update an object can set its version to an arbitrary higher value

SaveObject::updateObject now passes @self.version from the request body to advanceVersion(), which stores it when it parses and is higher than the stored one. The seed-import comparison ("update a stored seed only when the shipped version is higher") then reads that object as newer than every later shipped version, so the app's seed fix never reaches it. The caller needs update rights on the object anyway, so this is integrity of the seed channel, not escalation.

Fix: Take the carried version only on the import path (an explicit flag from the seed importer), or ignore @self.version from the HTTP boundary.

Verification: grep -n "carriedVersion" lib/Service/Object/SaveObject.php → 4436-4441, read straight from $selfData

WilcoLouwerse · 2026-10-15T15:02:43Z

+		// Free-form marks a consuming app reads (REQ-ATTR-007), validated at
+		// scan time; an empty map is no declaration and is not forwarded.
+		if ($attribute->annotations !== []) {
+			$taxonomy['annotations'] = $attribute->annotations;


🟢 #[McpTool] annotations may carry MCP's reserved hint keys

The free-form annotations map is forwarded in the descriptor under the key annotations, which tools/list hands to external MCP clients unchanged (McpToolsService::listTools() returns descriptors raw), and annotations is the key the MCP specification uses for tool hints (readOnlyHint, destructiveHint, idempotentHint, openWorldHint, title). malformedAnnotation() only checks "non-empty string key, scalar value", so annotations: ['readOnlyHint' => true] on a write tool is accepted and a client that auto-approves read-only tools would trust it, while OpenRegister's own hint and grant checks say otherwise. Authors are trusted code, so this is hardening, not an exploit.

Fix: reject the five reserved MCP hint keys in malformedAnnotation() (or forward the map under a namespaced key such as x-annotations).

Done when: AttributeToolScannerTest asserts a tool declaring readOnlyHint inside annotations is skipped and logged.

Verification: sed -n 395,405p lib/Mcp/AttributeToolScanner.php; sed -n 98,128p lib/Service/Mcp/McpToolsService.php → only key/scalar shape checked; descriptor returned as is

WilcoLouwerse · 2026-10-15T15:02:43Z

+	 * GET would, runs the same doelbinding guard, withholds `session`, `request`
+	 * and `ipAddress`, and removes from each entry's `changed` the properties
+	 * the caller may not see on the object. That per-object read check is what
+	 * closes the wave-3 C7 IDOR now, instead of the admin gate: an arbitrary


🟡 GET .../{id}/logs still returns the raw audit rows that #4614 says a reader must not get

#4614 makes .../audit-trails readable by anyone who may read the object, and takes care to withhold session, request and ipAddress, to strip from changed the properties the caller may not see, and to run the doelbinding guard. The older route GET /api/objects/{register}/{schema}/{id}/logs (ObjectsController::logs, unchanged, @NoAdminRequired) answers the same question for the same caller: it resolves the object with RBAC and returns ObjectService::getLogs() rows unfiltered. A reader who may see the object but not one of its properties gets that property's old and new values, the IP address and the session id from /logs, so the redaction (and the 'nobody else' sentence in the new docblock and spec) is one URL away from void. The anchored line is the new docblock sentence; the leaking route is lib/Controller/ObjectsController.php::logs() (line 4784).

Fix: Make ObjectsController::logs() delegate to ObjectHistoryForReaders::page() for non-admins (same 404/purpose behaviour), or make it admin-only; keep its response shape if existing UI callers rely on it. Add the route to the spec requirement text so both URLs are named.

Done when: A non-admin who may read an object but not property x calls /logs and gets entries without x, session, request and ipAddress; an unreadable object gives 404.

Verification: sed -n 4780,4790p lib/Controller/ObjectsController.php; grep -n 'function findLogs' -A20 lib/Service/Object/GetObject.php | grep -n 'findAll' → ObjectsController::logs() is #[NoAdminRequired]-style, resolves the object with RBAC only and returns AuditTrailMapper::findAll() rows raw: no WITHHELD_FIELDS, no HistoryEntryRedactor, no PurposeGuard

Severity: ObjectsController::logs() exists unchanged on beta, so the exposure is not new in this promotion. It is filed because #4614 now establishes that object history is redacted for non-admin readers, and this sibling route still returns the raw rows.

🔧 Proposed fix in #4655, commit 4e900c4 (see the PR body for what changed). The thread stays open until it is merged.

WilcoLouwerse · 2026-10-15T15:02:43Z

+	 *
+	 * @var string
+	 */
+	private const USER_SETTINGS_HASH = '#user-settings';


🟡 #4587 moved the per-person settings into the user settings dialog but left the pointers to the old place

#4587 deletes OpenRegisterPersonal, personalSettings.js and the <personal> entry in info.xml, so Nextcloud's personal settings page no longer shows anything of OpenRegister. These live copies still point there: (1) the 'Reconnect your account' notification (Notifier.php::prepareCredentialRelinkNeeded) says 'open Connected accounts in your personal settings' and its link is settings.PersonalSettings.index?section=additional, so a person whose connection broke lands on a page without the account list; (2) the admin hint in PushNotificationsConfiguration.vue:107 ('Users can opt in ... from their personal settings', with its nl.json translation); (3) the requirement 'A person can connect, see and repair a connection from personal settings' in openspec/specs/credential-oauth2-connect/spec.md:208; (4) stale header comments in BrowserNotificationsSection.vue:13 ('TODO mount ... registers NO PHP Personal settings class') and OAuth2Endpoints.php:20. The anchored line is the new hash constant; the notification link is on Notifier.php:628, not a changed line.

Fix: Point the notification link at the app page with #user-settings (as the OAuth callback now does) and reword the message and the admin hint to 'user settings'; update the spec requirement text and the two comments; update l10n keys with the new English strings.

Done when: git grep -n -i 'personal settings' over lib/, src/ and openspec/specs/ returns no live copy that describes where OpenRegister's per-person settings are, and the relink notification link opens the dialog.

Verification: git grep -n "settings.PersonalSettings.index\|in your personal settings\|from their personal settings\|personal settings SHALL\|No PHP Personal settings\|NO PHP Personal settings" -- lib src openspec/specs → lib/Notification/Notifier.php:621,628; src/views/settings/sections/PushNotificationsConfiguration.vue:107; openspec/specs/credential-oauth2-connect/spec.md:208; src/components/userSettings/BrowserNotificationsSection.vue:13; lib/Service/Credential/OAuth2Endpoints.php:20; l10n/nl.json:63

🔧 Proposed fix in #4655, commit 5faeef9 (see the PR body for what changed). The thread stays open until it is merged.

WilcoLouwerse · 2026-10-15T15:02:43Z

+		// into this schema against what it is about to become. An owning app
+		// set to `refuse` blocks the save; otherwise the broken forms are
+		// unpublished after the save, and the response lists them.
+		$formAssessment = $this->assessDependentForms(existing: $existingSchema, data: $data);


🟡 The 'saving a schema re-checks its forms' rule holds for SchemasController and nothing else

The spec says 'When a schema's properties or required list are saved, OpenRegister SHALL dispatch FormDestinationDependentsEvent'. The dispatch lives in SchemasController::applyUpdate() only (publishing a draft goes through it too). Schemas are also written by the configuration importer (Service/Configuration/ImportHandler, which is how an app upgrade ships a new required property), SchemaImportController, the AI SchemaTool and the MCP SchemasToolProvider; none of them assess the published forms, so a form can keep published and then 422 every resident at submit time with nobody notified. Separately, formDependentsCheck() swallows any Throwable from the container and returns null, which silently skips an owning app's refuse setting without a log line.

Fix: Move the assess/apply step into one service called by every schema write that changes properties or required (or state in the spec that only the controller path is covered), and log at warning level when the check cannot be built.

Done when: A schema import that adds a required property to a destination schema unpublishes (or refuses, per formDestinationBreak) the dependent forms; a container failure is logged.

Verification: grep -rn 'FormDependentsCheck\|FormDestinationDependentsEvent' lib --include=*.php -l; grep -rln 'schemaMapper->update\|updateFromArray' lib --include=*.php → FormDependentsCheck is referenced only by SchemasController; ImportHandler, SchemaImportController, SchemaTool, SchemasToolProvider, SchemaService also update schemas; formDependentsCheck() returns null on any Throwable and the check is skipped without a log line

🔧 Proposed fix in #4655, commit 1f4eafb (partly: the PR body says what is left). The thread stays open until it is merged.

WilcoLouwerse · 2026-10-15T15:02:43Z

+	 *
+	 * @spec openspec/changes/special-right-to-manage-grants/specs/object-ownership/spec.md
+	 */
+	public function letsItselfIn(?string $admittedAs, ?string $userId, array $userGroups, ?string $user, ?string $group): bool {


🟡 The "may not let itself in" check ignores remote principals

letsItselfIn() compares only the user and group grantee against the caller, but ObjectSharingService::GRANTABLE_TYPES also accepts remote and remote_group (ObjectSharingService.php:94-95). A grant_manage holder can therefore grant read to its own federated cloud id, or a remote group, and so read the record by one extra call, which is exactly what the docblock above says the guard prevents. The grant also outlives a rotation of the holder's own credentials.

Fix: refuse a holder-admitted remote / remote_group grant outright (no way to prove it is not the caller), or limit GRANTABLE_TYPES for holders to user and group.

Done when: a unit test in GrantManageSpecialRightTest grants remote as a holder and asserts the refusal.

Verification: sed -n 123,133p lib/Service/Rbac/GrantManageGuard.php; sed -n 91,96p lib/Service/Rbac/ObjectSharingService.php → only $user/$group compared; remote/remote_group grantable

Found by the persistence audit that ran alongside this review.

🔧 Proposed fix in #4655, commit e19745b (see the PR body for what changed). The thread stays open until it is merged.

WilcoLouwerse

Verdict: REQUEST_CHANGES (Strict)

Not ready for beta: six blockers in code that reached development without any review, most in the new forms, drafts, frozen-file and deferral work. Under Strict every 🟡 below blocks as well.

🔧 Fix PR: #4655 — 23 of 33 findings, one commit each

Merge it into development, then re-request the review. The other findings stay on their threads.

🔴 Blockers (6)

Bulk save overwrites another user's draft — lib/Service/Object/DraftStatusPolicy.php:295 (introduced in #4566, #4600). A draft is hidden from other users on every read path (SQL RBAC predicate) and ObjectService::refuseSomebodyElsesDraft() refuses a single-object save over someone else's draft; keptFrom() (this method) is the only guard and it has exactly one caller. Tracked in #4648. (fix proposed in #4655)
A required file property can never be satisfied by an upload token — lib/Service/Form/FormSubmitService.php:644 (introduced in #4563). mapPayload() moves an upload token's file into $plan['files'] and leaves the property out of $plan['object']. Tracked in #4646. (fix proposed in #4655)
Review policy fails open for visibility when it cannot be read — lib/Service/Decision/ReviewPolicyResolver.php:98 (introduced in #4595). When the schema cannot be read, the resolver returns ownership: decider with no supervisors and calls that "fail closed", but hideOthers stays false, so DecisionVisibility::isHidden() returns false and another reviewer sees the verdict, bases and decisio Tracked in #4647. (fix proposed in #4655)
Frozen-file guard ignores writes made through a share — lib/Listener/FrozenNodeWriteListener.php:172 (introduced in #4549). refusalFor() returns null unless $node->getPath() starts with /openregister/files/. Tracked in #4649. (fix proposed in #4655)
Any admin-UI file-settings save clears the instance review default — lib/Service/Settings/FileSettingsHandler.php:259 (introduced in #4595). updateFileSettingsOnly() rebuilds the whole fileManagement value from the request and writes ReviewPolicy::SETTINGS_KEY => $this->reviewDefault($fileData), which is null when the request has no anonymisationReview key. Tracked in #4650. (fix proposed in #4655)
Deferred-chunk limit is 32000, but NC 32 and 33 accept 4000 — lib/Service/Deferral/ListenerDeferralService.php:82 (introduced in #4616). MAX_ARGUMENT_LENGTH = 32000 is documented as OC\BackgroundJob\JobList::MAX_ARGUMENT_JSON_LENGTH. Tracked in #4651. (fix proposed in #4655)

🟡 Concerns (19)

RBAC draft predicate ignores unhealed _status — lib/Db/MagicMapper/MagicRbacHandler.php:359 (introduced in #4566, #4610). The self-healing change promises that a table whose metadata columns cannot be added "degrades quietly": MagicSearchHandler leaves the lifecycle predicates out for the columns MagicMetadataColumnHealer::missingFor() reports (REQ-MTH-002). (fix proposed in #4655)
Forced re-import deletes local authorization the file lacks — lib/Service/ShippedBaseline/GuardedDescriptorMerge.php:165 (introduced in #4618). With a decision in effect, line 165 adds every authorization root found in live OR incoming to the decisions, so AuthorizationBlocks::settle() turns the block's state into UPSTREAM; foldUpstream() with the block absent from incoming then drops it fro (fix proposed in #4655)
Aggregation fast path counts drafts the list hides — lib/Service/Object/DraftListLens.php:175 (introduced in #4566, #4600). AggregationRunner builds its own WHERE (_deleted, _archived predicates, with a comment saying it must reproduce the list path's defaults) and takes the access rule from MagicMapper::rbacRowPredicateSql(). (fix proposed in #4655)
Upload endpoint ignores the form's audience — lib/Controller/FormsController.php:274 (introduced in #4563). submit() answers 401 when the visitor is anonymous and the form's audience is not public, but upload() (also #[PublicPage]) never reads $form['audience']. (fix proposed in #4655)
Idempotency key is neither subject-bound nor race-safe — lib/Service/Form/FormSubmitService.php:268 (introduced in #4563). The key scope is the form id only, so on a public form anyone who presents the same Idempotency-Key within 24 hours gets the first submitter's stored answer (reference, id, and every confirmation property, which can be personal data). (fix proposed in #4655)
Draft cleanup deletes without re-checking the draft — lib/Service/Object/DraftExpiry.php:368 (introduced in #4600). expiredIn() reads every page of expired draft uuids first; remove() then deletes them one by one (up to 1000 per run) with _rbac: false and no re-read. (fix proposed in #4655)
Dependent form reported unpublished when nothing was unpublished — lib/Service/Form/FormDependentsCheck.php:158 (introduced in #4563). unpublish() calls the closure only when $form->unpublish !== null and otherwise falls through to notify the author and return unpublished. (fix proposed in #4655)
Task wake still overwrites another listener's signal — lib/Service/Flow/FlowTaskBridge.php:369 (introduced in #4541). The docblock says the signal now carries the outcome bag "so it cannot erase what another listener on the same event signalled". (fix proposed in #4655)
Decision ownership check is skipped in two cases — lib/Service/Decision/RelationDecisionService.php:106 (introduced in #4595). decide() reads $current and runs the ownership refusal only when $current !== null, outside the transaction. (fix proposed in #4655)
Failure to arm a statutory expiry timer is only logged — lib/Service/Case/CasePlanItemTimers.php:219 (introduced in #4544). arm() catches every Throwable from FlowTimerService::arm and returns false, and onEntered() just counts. (fix proposed in #4655)
Email recipient reference may point at any object in the organisation — lib/Service/Notification/ReferencedAddressNotifier.php:308 (introduced in #4559). findReferenced() reads the referenced object with _rbac: false and _multitenancy: false by id only; sameTenant() then requires equal organisation, and passes for any object with no organisation.
Aborted delete or rename reads as success to internal callers — lib/Listener/FrozenNodeWriteListener.php:123 (introduced in #4549). For delete and rename the listener calls abortOperation(). (fix proposed in #4655)
A later listener's allow() overrides an earlier refusal — lib/Event/DecisionChangeRequestedEvent.php:129 (introduced in #4595). DecisionChangeRequestedEvent is a vetoable question ("may this user change this decision?"), but allow() sets allowed = true and clears the reason regardless of an earlier refuse(). (fix proposed in #4655)
Object GET /logs bypasses the audit-history redaction — lib/Controller/AuditTrailController.php:569 (introduced in #4614). #4614 makes .../audit-trails readable by anyone who may read the object, and takes care to withhold session, request and ipAddress, to strip from changed the properties the caller may not see, and to run the doelbinding guard. (fix proposed in #4655)
Copies still send users to the removed personal settings form — lib/Controller/CredentialOauth2Controller.php:102 (introduced in #4587). #4587 deletes OpenRegisterPersonal, personalSettings.js and the <personal> entry in info.xml, so Nextcloud's personal settings page no longer shows anything of OpenRegister. (fix proposed in #4655)
Dependent-form re-check runs on one schema write path only — lib/Controller/SchemasController.php:1281 (introduced in #4563). The spec says 'When a schema's properties or required list are saved, OpenRegister SHALL dispatch FormDestinationDependentsEvent'. (fix proposed in #4655)
The "may not let itself in" check ignores remote principals — lib/Service/Rbac/GrantManageGuard.php:123 (introduced in #4601). letsItselfIn() compares only the user and group grantee against the caller, but ObjectSharingService::GRANTABLE_TYPES also accepts remote and remote_group (ObjectSharingService.php:94-95). (fix proposed in #4655)
Delegated grant changes leave no audit-trail row — lib/Service/Rbac/ObjectOwnershipService.php:366 (introduced in #4601), no changed line within reach, so no thread. A grant_manage holder can grant, revoke and set the owning group, and an admin can change the organisation's grant_manage list, but none of those writes an OpenRegister audit-trail entry: ObjectSharingService has no audit call at all, setOwnerGroup() writes the block and returns, and OrganisationController::update records nothing for the right. reassign() in the same class does write one (ownership_handover, via AuditTrailMapper::createAuditTrail), so who widened access to a record is the one thing the trail cannot answer. Fix: write an audit row (actor, object or organisation, principal before/after) for grant, revoke, owning-group change and grant_manage list change, the way ownership_handover is written. Done when: grep -n -i audit lib/Service/Rbac/ObjectSharingService.php finds the write, and a test asserts the row per action. Verification: grep -c -i -E "auditTrail|AuditTrailMapper" lib/Service/Rbac/ObjectSharingService.php → 0; ObjectOwnershipService.php:95 AUDIT_ACTION = 'ownership_handover' used only by reassign() Found by the persistence audit that ran alongside this review.
Guard failure imports shipped authorization unguarded — lib/Service/ShippedBaseline/ShippedConfigurationGuard.php:201 (introduced in #4618), no changed line within reach, so no thread. When the guard throws, the catch returns definition => $incoming with guarded: false: the app's shipped authorization block replaces whatever the administrator set on this instance, which is the one thing the guard exists to prevent. The comment calls that "a known state", but for an authorization block it is fail-open towards the shipped policy. Authorization changes taken because only the app changed them (applied) are also never written to the audit trail. Fix: on guard failure keep the instance's current authorization (or skip importing that key) and log; record applied authorization changes on the trail as recordReplaced does for explicit decisions. Done when: a test makes the guard throw and asserts the instance's authorization block is unchanged after the import. Verification: sed -n 201,216p lib/Service/ShippedBaseline/ShippedConfigurationGuard.php → catch returns 'definition' => $incoming, 'guarded' => false Found by the persistence audit that ran alongside this review. (fix proposed in #4655)

🟢 Minor (8)

Client-supplied @self.version raises stored version — lib/Service/Object/ObjectVersionHandler.php:158 (introduced in #4635). SaveObject::updateObject now passes @self.version from the request body to advanceVersion(), which stores it when it parses and is higher than the stored one.
Direct-invoke caller config is not limited to the node's config keys — lib/Service/Flow/FlowRunService.php:1090 (introduced in #4553). executeNode() merges the caller's config over the published step's config with caller keys winning, without intersecting with IFlowNodeConfigKeys::configKeys().
Upload tokens: orphan .bin never purged, size unbounded without maxSize — lib/Service/Form/FormUploadStore.php:133 (introduced in #4563). issue() writes <token>.bin and then <token>.json; purge() walks .json files only, so a .bin whose .json write failed is never deleted.
Remove restores a change's path even if edited since — lib/Service/ExampleSet/ExampleSetChanges.php:251 (introduced in #4603, #4632). ExampleSetChanges::revert() writes the recorded old value back (or removes the path) without checking that the object still holds what the set wrote.
Organisation lookup errors read as gone or as absent — lib/Service/ExampleSet/ExampleSetInstance.php:159 (introduced in #4603). delete() treats any Throwable from findByUuid() as "already gone" and returns true, so a database error makes remove forget the organisation uuid while the organisation stays.
Listener builds MagicMapper on every file write instance-wide — lib/AppInfo/Application.php:2995 (introduced in #4549). FrozenNodeWriteListener is registered on four node events that fire for every file operation on the instance (all apps, thumbnails, uploads).
--check help says it writes nothing, but it imports registers — lib/Command/ExampleSetInstallCommand.php:86 (introduced in #4603). The --check option is described as "Validate every object against its schema and write nothing".
#[McpTool] annotations may carry MCP's reserved hint keys — lib/Mcp/AttributeToolScanner.php:303 (introduced in #4554). The free-form annotations map is forwarded in the descriptor under the key annotations, which tools/list hands to external MCP clients unchanged (McpToolsService::listTools() returns descriptors raw), and annotations is the key the MCP specification

✅ Verified clean

The five new migrations (Version1Date20261009150000 … 20261011140000): idempotent guards, additive and nullable columns, no two PRs on one table.
#4589: a roles block that resolves to nothing grants nothing (PermissionHandler.php:3622, fail-closed).
#4601: non-admins cannot change the organisation's grant_manage list; patch routes through update.
Draft privacy on the single-object read and write paths (MagicRbacHandler.php:336-363, ObjectService.php:2321); the bulk path is the exception (🔴 above).
#4571 graduated/bandRate: band boundaries, zero, negative quantity and the open last band match the spec and are asserted.
OAuth2 connect: safeReturnUrl() still blocks open redirects; the archived spec deltas are synced into openspec/specs; every info.xml job, command and repair class exists.
The beta release computes its version from tags, so the stale info.xml version on development is not a defect.

Scope decision: promotion of 296 commits, 0 direct. 37 merged PRs: 34 by rubenvdlinde with no review at all, read at full depth in four slices (data/RBAC/migrations, process engines, platform services, controllers/frontend/specs) plus an integration pass over the 59 files touched by two or more PRs; 3 by WilcoLouwerse (#4602, #4642, #4643), trusted on their earlier self-review after a spot-check (no open threads). Residual: the 53 files only those three touched were not re-read; per slice, most test files, l10n and parity JSON were skimmed, not read.
Gates: quality / Hydra Gates and quality / Quality Report green on b9e5d8df; what they cover (lint, licence headers, l10n parity, unit suite) was not re-checked.
Persistence audit: run on this checkout; its grant-related findings are posted above (remote grantees, missing audit rows, the shipped-authorization guard). Two audit items were refuted against the spec and dropped (a holder naming another owning group; a stored form without a mapping). LOW items not posted: occ openregister:example-set:install has no environment guard; the portal assertion uses one HS256 secret with no kid and no jti record; exports with a null retentionDays are kept forever.
Verification: every 🔴 was re-checked on the head by reading the code (nothing was run: no vendor tree, no rig). The Nextcloud 32/33 job-argument limit (plat-f3) was read from the server tags v32.0.0, v33.0.0 and v34.0.0.

…ly as a mock A fresh instance refused the zuiddrecht set: brp and kvk are mock-only descriptors, and the real-configuration re-import correctly refuses a mock. A requires entry now names the descriptor it wants: a slug for the real one, {"register": "<slug>", "mock": true} for the mock, imported through reimportMock() under openregister.mock and recorded for remove as before. A plain slug never falls back to the mock. zuiddrecht declares brp and kvk as mock requirements.

…erty (#4656) ObjectTreeService walks descendants, ancestors or both along the parent property a schema already declares (x-openregister-hierarchy.parent, else x-openregister-party.parentProperty), one query per level, with levels, cycle protection, the declared depth, the requested depth and the node cap, and the caller's read access on every node. Exposed as GET /api/objects/{register}/{schema}/{id}/tree. Decision 250.

… and nothing else (#4657) BrpPersonProvider::countResidentsAtAddressObject() asks Haal Centraal ZoekMetAdresseerbaarObjectIdentificatie with one field and answers { total, meta }. It asks only when the brp-haalcentraal source carries configuration.residentCount: true, and the call reaches the connector with withholdBodies so the residents' numbers are never stored. The router gains sourceConfiguration() and the withholdBodies call option. This is the openregister half of portaliq identity-registered-details T07. Assisted-by: ClaudeCode:claude-opus-5-5

…ock-register fix(example-sets): a set may require a register OpenRegister ships only as a mock

…roperty, and a config field may be a dot path The rehearsal R2 install logged 16,038 'meta is not a valid attribute' errors: shillinq's _meta property has a _meta column, and every magic-table reader took each _ column for object metadata, so MagicStatisticsHandler failed the whole row and the other readers dropped the property. MagicMetadataColumn now decides: a property the schema declares is a property unless ObjectEntity has that attribute. An unknown metadata column is skipped and noted once at debug. opencatalogi's publiccode schema was rejected on every import because the save-time check compared description.nl.longDescription with the top-level keys; the reader walks dot paths, so the check now accepts a path whose first segment is a property. example-set --check counted changes into the per-schema object count (12 dossiq cases against 10 declared); changes are now counted and printed apart.

… bulk writer asks it too check:strict's phpmd flagged static access, a long variable name and the bulk writer's method length; the write-side rule moves into MagicMetadataColumn::skippedOnWrite().

fix(magic-tables,example-sets): underscore properties read as properties, dot-path config fields, check counts like install

Bumps [zod](https://github.com/colinhacks/zod) from 4.6.5 to 4.6.7. - [Release notes](https://github.com/colinhacks/zod/releases) - [Commits](colinhacks/zod@v4.6.5...v4.6.7) --- updated-dependencies: - dependency-name: zod dependency-version: 4.6.7 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bumps [@playwright/test](https://github.com/microsoft/playwright) from 1.63.0 to 1.64.0. - [Release notes](https://github.com/microsoft/playwright/releases) - [Commits](microsoft/playwright@v1.63.0...v1.64.0) --- updated-dependencies: - dependency-name: "@playwright/test" dependency-version: 1.64.0 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bumps [smalot/pdfparser](https://github.com/smalot/pdfparser) from 2.12.5 to 2.13.0. - [Release notes](https://github.com/smalot/pdfparser/releases) - [Commits](smalot/pdfparser@v2.12.5...v2.13.0) --- updated-dependencies: - dependency-name: smalot/pdfparser dependency-version: 2.13.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

…#4625) Bumps [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser) from 8.68.0 to 8.71.2. - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/parser/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v8.71.2/packages/parser) --- updated-dependencies: - dependency-name: "@typescript-eslint/parser" dependency-version: 8.71.2 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bumps [vue-material-design-icons](https://github.com/robcresswell/vue-material-design-icons) from 5.3.1 to 5.4.0. - [Release notes](https://github.com/robcresswell/vue-material-design-icons/releases) - [Commits](robcresswell/vue-material-design-icons@5.3.1...5.4.0) --- updated-dependencies: - dependency-name: vue-material-design-icons dependency-version: 5.4.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bumps [webonyx/graphql-php](https://github.com/webonyx/graphql-php) from 15.37.3 to 15.38.1. - [Release notes](https://github.com/webonyx/graphql-php/releases) - [Changelog](https://github.com/webonyx/graphql-php/blob/master/CHANGELOG.md) - [Commits](webonyx/graphql-php@v15.37.3...v15.38.1) --- updated-dependencies: - dependency-name: webonyx/graphql-php dependency-version: 15.38.1 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

rjzondervan · 2026-10-16T09:47:48Z

+		if ($mapping === null) {
+			foreach ($payload as $name => $value) {
+				$name = (string)$name;
+				if (str_starts_with($name, '_') === false && str_starts_with($name, '@') === false) {


🔴 Blocker — A form payload's id picks the uuid, and a scoped draft save then writes over any draft with RBAC off

pairsOf() drops only _ and @ keys when a write has no mapping, so a submitter's id reaches $plan['object']. writeAll() passes uuid: null for a fresh submit or a new draft, and SaveObject::extractUuidAndSelfData() (SaveObject.php:3538-3539) then takes $data['id'] as the uuid. SaveObject looks that uuid up with _rbac: false and, when it exists, takes the update branch (handleObjectUpdate, SaveObject.php:3330).

For a scope-value owner (decision 195), writeAll() saves with _rbac: false. refuseSomebodyElsesDraft() returns early when RBAC is off, and DraftStatusPolicy::resolveStatus() accepts draft → draft. So a portal resident whose form has no mapping can send { "id": "<another person's draft uuid>", ... } to saveDraft() and overwrite that draft in the destination schema. The victim can be another resident or a Nextcloud user. The draft keeps its owner, so the victim later finds and submits content they did not type. The comment at lines 685-697 ("Updating or sending it was authorised by DraftFinder::own() before this write") does not hold on this path: own() is only asked when $draftId is set.

The same key affects a fresh submit under RBAC. A uuid the caller cannot read goes down checkSavePermissions()'s create branch (ObjectService.php:2275-2287). It is judged by the schema's create rule (for an anonymous visitor on a public form: the public create rule) and then stored over the hidden object. refuseSomebodyElsesDraft() closes that only for drafts.

I could not check whether the in-process callers (the portal) ever pass mapping: null. The spec explicitly allows it ("without a mapping the payload minus _ and @ keys is the object"), and FormsController::payload() forwards id too. Uncertain, so a blocker under Strict.

Impact: a form submitter (anonymous on a public form, or a portal resident through a scoped draft) can change existing objects of the destination schema that they cannot read, including other people's drafts.

Suggested fix: in pairsOf(), also drop id and uuid when no mapping is given, so a submit never chooses its uuid. The only way to name an existing object is then $draftId, which goes through DraftFinder::own(). As a backstop, in writeAll() refuse when $uuid === null and the object still carries an id.

Done when:

a new FormSubmitServiceTest case: saveDraft() with a scoped owner, mapping: null and payload id = another owner's draft uuid either creates a new object with a fresh uuid or is refused, and the other draft's data is unchanged. The test goes red with the id filter removed

the same for submit() with an anonymous subject and the id of an existing object

Reachability: a scope-value owner is only created through DraftOwner::portalSubject() / scoped() by an in-process caller; no caller in this repository creates one, so the write path is reached through the portal app's use of saveDraft(). Reported as a blocker under Strict because the payload id is not stripped on a write that runs with RBAC off.

Tracked as #4664: filed rather than fixed here because the fix lands through development and this release PR is re-cut after every merge.

rjzondervan · 2026-10-16T09:47:48Z

+                // A draft is removed for good, with its files: a tombstone of
+                // unsent input serves nobody, and on an archival schema the
+                // purge would refuse it for ever (decisions 180 and 216).
+                if ($objectToDelete->isDraft() === true) {


🔴 Blocker — Bulk delete ignores every draft rule

The draft delete rules live only in deleteObject(): the lookup under the read RBAC (which hides somebody else's draft), isDeletableDraft() for archival schemas, and permanent = true here. deleteObjects(), behind POST /api/bulk/{register}/{schema}/delete (BulkController::delete, @NoAdminRequired), has none of them.

Contract branch (spec draft-owner-activation-and-lifetime) Single delete Bulk delete

Only somebody who can read a draft may change it ✓ RBAC lookup hides it → 404 ✗ filterUuidsForPermissions() → hasPermission(delete, objectOwner), which has no draft rule

A draft's delete is permanent, with its files ✓ line 3190 ✗ deleteHandler->deleteObject() without permanent, so a soft-deleted tombstone keeps the unsent input

One draft.removed audit entry without content ✓ written in the permanent branch of DeleteObject::delete() ✗ not permanent, so no entry

A draft is deletable on an archival schema ✓ isDeletableDraft() ✗ rejectIfArchivalImmutable() refuses it, so the owner cannot bulk-discard their own draft there

So any user with delete rights in a schema can remove another user's draft by uuid, and the removed draft's content stays in the trash.

Sibling of the open thread at DraftStatusPolicy.php:295 (bulk save over somebody else's draft). That thread covers the bulk save path, not the bulk delete path. The root cause is the same: PermissionHandler::hasPermission() has no draft rule. Putting keptFrom() into hasPermission() when an object is passed would close both threads and f3.

Suggested fix: in deleteObjects(), for each pre-resolved row that isDraft(): skip it (reason OBJECT_NOT_PERMITTED, the same answer as an unreadable row) when $_rbac is on and DraftStatusPolicy::keptFrom() is true; otherwise pass permanent: true and skip the archival gate, as deleteObject() does.

Done when:

a deleteObjects() test: user B bulk-deleting user A's draft gets it in skipped_uuids and the draft still exists; A bulk-deleting it removes it permanently and writes one draft.removed entry

the same test goes red with the new branch removed

Tracked as #4665: filed rather than fixed here because the fix lands through development and this release PR is re-cut after every merge.

rjzondervan · 2026-10-16T09:47:48Z

@@ -91,7 +91,7 @@ class ReadableAuditTrailLister {
 	 *
 	 * @var string[]
 	 */
-	private const WITHHELD_FIELDS = ['session', 'request', 'ipAddress'];
+	public const WITHHELD_FIELDS = ['session', 'request', 'ipAddress'];


🔴 Blocker — /api/audit-trails/readable shows other users' draft contents

A draft's saves write ordinary create/update audit rows, and their changed column holds every property's old and new value (AuditTrailMapper.php:878-884). GET /api/audit-trails/readable (@NoAdminRequired) decides per row through mayRead() → PermissionHandler::hasPermission(action: 'read', objectOwner, object). hasPermission() has no draft rule (the draft rule lives only in the SQL predicate, MagicRbacHandler::reachableRowSqlFor()). Any user with read rights on the schema therefore reads, in this list, what another user typed into a draft that the object API hides from them ("a draft answers to its owner only", decision 180). WITHHELD_FIELDS removes session, request and ipAddress but keeps changed.

Sibling of the open thread at DraftStatusPolicy.php:295, which has the same root cause (hasPermission() has no draft rule) but is about a different path.

Impact: draft content (possibly personal data a resident has not yet decided to send) is readable by colleagues while the draft lives.

Suggested fix: add the draft rule to PermissionHandler::hasPermission() when an object is passed: refuse read (and every other action) when (new DraftStatusPolicy())->keptFrom($object, $schema, $userId) is true. That one change also closes f2 and the bulk-save thread.

Done when:

a ReadableAuditTrailListerTest case: user B, with read rights on the schema, does not get the audit rows of user A's draft. It goes red without the draft rule

user A still gets them

Tracked as #4666: filed rather than fixed here because the fix lands through development and this release PR is re-cut after every merge.

rjzondervan · 2026-10-16T09:47:48Z

+
+		$receivedAt = DateTimeImmutable::createFromMutable($this->time->getDateTime());
+		// Creation side effects run now, once (decision 227): the created event, then the receipt.
+		(new ObjectEventGate(events: $this->events))->leftDraft(object: $created[0]['object'], receivedAt: $receivedAt);


🟡 Concern — Two concurrent leave-draft requests both announce the creation

The spec (object-events) says a listener on ObjectCreatedEvent "SHALL receive every object exactly once". promoteDraft() and DraftActivation::activate() (line 142) first check that the object is still a draft (DraftFinder::own() / ownDraft(), a plain read), then save, then call leftDraft() without asking whether the save left draft. Two requests on one draft, such as a double click or a client retry (with no Idempotency-Key or with different keys), can both pass the check before either commits. The second save then loads the now-active row, and DraftStatusPolicy::resolveStatus() returns early at line 114 because active === active, so it raises no refusal. The save goes through as an ordinary update: ObjectEventGate::updated() sends ObjectUpdatedEvent, and leftDraft() sends ObjectCreatedEvent and ObjectActivatedEvent a second time. The result is duplicated webhooks, flows, created notification rules and receipts. Nothing locks the draft, and the save has no version check.

Suggested fix: let the save report whether it used the promotion allowance, for example DraftStatusPolicy::consumePromotion(uuid): bool, or check isset(self::$promotions[$uuid]) after the write. Call leftDraft() only when the allowance was used, and otherwise answer as for a draft that is gone (404/409). Better still, refuse the write in resolveStatus() when an allowance exists for a uuid that is no longer a draft.

Done when:

a FormSubmitServiceTest and a DraftActivationTest case where saveObject returns an object whose stored status was already active (the allowance was not used): no ObjectCreatedEvent is dispatched. Both go red with the check removed

rjzondervan · 2026-10-16T09:47:48Z

+	 */
+	private function expiredIn(Register $register, Schema $schema, DateTimeImmutable $cutoff): array {
+		$expired = [];
+		for ($page = 0; $page < self::MAX_PAGES; $page++) {


🟡 Concern — Drafts beyond the 5,000-read cap are neither expired nor counted as deferred

The spec says: "at most 5,000 drafts read per register and schema. What is left SHALL be counted as deferred and SHALL wait for the next run." expiredIn() reads every draft (_drafts=true), with no date filter, in PAGE_SIZE pages up to MAX_PAGES, and filters by age in PHP. When a schema holds more than 5,000 drafts, the expired ones after the 5,000th row are never read. They are not counted as expired or deferred, so the log and occ openregister:drafts:expired report less than is really there. Whether a later run ever reaches them depends on the list's default sort. With a stable order, the same first 5,000 rows are read on every run, so expired drafts further down are never removed.

Suggested fix: filter in the query (@self[updated] before the cutoff, with a fallback to created), so the 5,000 rows read are 5,000 expired drafts. When the last page is full, count what remains as deferred (or report "more than N").

Done when:

a DraftExpiryTest case with an objects fake that returns 5,000 fresh drafts followed by expired ones: the expired ones are removed, or counted as deferred, and never missing from the counts

rjzondervan · 2026-10-16T09:47:51Z

+			// A reference by key only refuses: the other actions rewrite or
+			// delete by uuid and would act on the wrong value.
+			$referenceKey = ($property['referenceKey'] ?? ($property['items']['referenceKey'] ?? null));
+			if ($referenceKey !== null && strtoupper((string)$property['onDelete']) !== 'RESTRICT') {


🟢 Minor — Validator and service disagree on referenceKey: uuid

ReferentialIntegrityService::extractReferenceKey() treats referenceKey empty or uuid as a plain uuid reference (all onDelete actions apply). The validator refuses any non-null referenceKey unless onDelete is RESTRICT, so referenceKey: uuid with CASCADE is refused at save although the service would honour it. Use the same normalisation in both (e.g. a shared static ReferenceKey::of($property)).

rjzondervan · 2026-10-16T09:47:51Z

+	 * @spec openspec/changes/a-task-may-wait-on-another-task/specs/flow-tasks/spec.md#requirement-a-task-may-wait-on-another-task-and-waits-out-of-sight
+	 */
+	private function applyBlockedExclusion(IQueryBuilder $qb, TaskInboxCriteria $criteria): void {
+		if ($criteria->includeBlocked === true || $criteria->objectUuid !== null || $criteria->runUuid !== null) {


🟡 A blocked portal task is still listed and completable in the portal

#4595 (a-task-may-wait-on-another-task) says an inbox read SHALL leave out every blocked task, in its results and its total. The exclusion lives in applyBlockedExclusion(), which only applyInboxPredicates() calls. An external (portal) task is excluded from every Nextcloud inbox (applyExternalExclusion()), so the portal list is the only inbox it has. That list is PortalTaskService::listForSubject() (extended by #4569) over TaskMapper::findOpenExternalForParty() / countOpenExternalForParty(), and applyPartyPredicates() filters on assignee, performer type and is_terminal only. Composed:

A portal task created with blockedBy shows up in the citizen's task list and count the moment it is created, not when its blocker closes. The released re-announcement from TaskBlockerReleaseListener then announces a task the party has been looking at all along.

PortalTaskService::complete() → TaskService::complete() never reads blockedBy (the only blocker check in TaskService is refuseHandWrittenState() on create), so the party can complete the task the case meant to hold back.

Each row goes through TaskInboxService::row() without openBlockers (PortalTaskService.php:137), so every row runs its own blocker query. design.md D-3 says "No query per row". inbox() batches; the portal list does not.

Neither PR saw the other: #4569 merged on 11 Oct, #4595 on 12 Oct.

rjzondervan · 2026-10-16T09:47:51Z

+	 * @spec openspec/changes/draft-creation-side-effects/specs/object-events/spec.md#requirement-creation-side-effects-must-be-held-for-a-draft-and-run-once-when-it-leaves-draft
+	 */
+	public function created(ObjectEntity $object): void {
+		if ($object->isDraft() === true) {


🟡 Personal data in a draft cannot be found by a data-subject request

Three PRs combine here. None of them covers this alone:

feat(objects): a draft is the destination object, in status draft (decision 180) #4566 makes a draft the destination object. It holds the half-filled form input, which is often a name, address, BSN or e-mail.

feat(objects): a draft holds its creation side effects until it leaves draft (decision 227) #4633 holds every ObjectCreatedEvent/ObjectUpdatedEvent listener from a draft until it leaves draft. ObjectChangeListener is one of them (the spec names it). It queues text extraction and entity recognition, and those write openregister_entities / openregister_entity_relations.

feat(objects): a draft's owner makes it definitive, and a schema sets a draft lifetime (decision 216) #4600 lets a schema keep drafts forever (draftLifetimeDays: false).

The data-subject machinery finds a person's objects only through that index: DataSubjectRequestService::findSubjectObjects() → discover() → matchEntities() reads openregister_entities joined to openregister_entity_relations. Erasure preview and export use the same path (ErasurePreviewService::preview() line 128). A draft is never in that index, so access and erasure requests miss it. On a schema with draftLifetimeDays: false that lasts indefinitely. Even when the entity index does hit a draft, loadObject() reads it with _rbac: true, and the draft predicate hides it from anyone but its owner and administrators.

Removing a draft does not fully remove its data either. Every save of a draft writes a full create/update audit row with its values (SaveObject lines 4018-4027 and 6352). The removal spec keeps those rows on purpose ("The entries the draft's own saves wrote before the removal are not changed"), but the delete requirement promises "no tombstone of unsent input is kept" (openspec/changes/draft-owner-activation-and-lifetime/specs/form-destination/spec.md:189). Both statements cannot be true.

The draft-creation-side-effects proposal lists text extraction as deliberately held, with no mention of GDPR. I did not check whether a DSAR export has a second, non-index discovery path. Grep found none (findSubjectObjects is the entry point used by erasure).

rjzondervan · 2026-10-16T09:47:51Z

+
+		$blockers = [];
+		foreach ($referencing as $refObj) {
+			if ($refObj->getDeleted() !== null && empty($refObj->getDeleted()) === false) {


🟡 Another user's hidden draft blocks a RESTRICT delete

#4570 adds RESTRICT by key: keyReferenceBlockers() → findReferencingByKey() → findReferencingInMagicTable(). The $ref RESTRICT path that already existed reaches the same raw SQL. That SQL filters _deleted IS NULL and nothing else, and the loop here skips only deleted rows. #4566 adds drafts: a draft is visible to its owner only and expires after 30 days. A draft holds whatever a half-filled form cited, such as a code from a code list.

Together: user A's draft that cites code X stops user B from deleting X. B gets a 409 whose blockers[].objectUuid names an object B cannot open, list or resolve. B can only wait until A sends the draft or it expires. A pending draft can also block a planned deletion. The draft spec calls a draft "not yet a record: it holds unsent input", and the delete path then counts it as a live referrer. CASCADE/SET_NULL on uuid references reach into other users' drafts in the same way.

Pick one rule for drafts in referential integrity and apply it to both lookups (uuid and key). Either drafts do not block (they cannot be seen and will expire), or they block without revealing their uuid to a caller who cannot read them.

rjzondervan · 2026-10-16T09:47:51Z

 - **Word-family text extraction now captures all content and supports legacy `.doc` and `.odt`.** `TextExtractionService::extractWord()` was rewritten around a recursive PhpWord element walker (`walkWordElements`, depth-capped at `MAX_WORD_DEPTH = 50`) that descends tables (rows → cells, including nested tables and in-cell text-runs / list items), section headers and footers, and document-level footnotes/endnotes — content the previous 2-level traversal silently dropped (notably entire tables, because a PhpWord `Table` exposes `getRows()`, not `getElements()`, so the old loop skipped it). The PhpWord reader is now selected from the file MIME / extension (`resolveWordReader`): `.docx` → `Word2007`, legacy `.doc` → `MsDoc`, `.odt` → `ODText` — previously the default `Word2007` reader was always used, so `.doc` and `.odt` failed to load even when routed. `application/vnd.oasis.opendocument.text` is added to the Word routing in `isWordDocument()`. (`text-extraction-word-completeness`)

 ### Behaviour changes
+- **A draft no longer fires `ObjectCreatedEvent`, `ObjectUpdatedEvent` or `ObjectDeletedEvent` (`draft-creation-side-effects`, decision 227).** A draft is an object in status `draft`. Flows, webhooks, activity, push, metrics and notifications no longer run on a draft's saves or its delete. They run once, when the draft leaves draft: `ObjectCreatedEvent` is dispatched then, followed by `ObjectActivatedEvent`. A listener on `ObjectCreatedEvent` needs no status check. Do creation work on `ObjectCreatedEvent` only. A listener that does it on `ObjectActivatedEvent` too runs twice. Objects that never were a draft behave as before.


🟡 CHANGELOG omits this release's upgrade-visible behaviour changes

The release adds three CHANGELOG bullets, all about drafts. These changes ship to beta with no entry:

fix(rbac): a roles block that resolves to nothing grants nothing #4589: a schema/register roles block that resolves to nothing used to leave the schema open and now grants nothing on read/create/update/delete (PermissionHandler::grantNothingWhenEmpty()). On upgrade this can take access away from users. It needs an upgrade note like the drafts one.

feat(files): a frozen or archived object's files refuse every write, through the files API and through Files or WebDAV #4549: every file write, create, delete and rename under an archived or frozen object's folder is now refused, also through WebDAV/Files (FrozenNodeWriteListener on four node events). Clients that attach files to archived objects start getting errors.

fix(settings): personal settings move into the user settings dialog #4587: the personal settings section (<personal>OpenRegisterPersonal) is removed and moved into the in-app user settings dialog. Nextcloud's personal settings no longer show it.

feat(audit): an object's history is readable by whoever may read the object #4614: an object's audit trail is now readable by anyone who may read the object. That is an exposure change admins should know about.

New public API surface: POST /api/forms/{formId}/submit|uploads|draft, POST /api/forms/validate (feat(forms): a form submits into its destination object (decision 179) #4563), POST /api/objects/{register}/{schema}/{id}/activate (feat(objects): a draft's owner makes it definitive, and a schema sets a draft lifetime (decision 216) #4600), GET .../{id}/tree (feat(relations): read an object's tree along its declared parent property #4656), /api/record-templates CRUD (train: land 10 openregister PRs (12 Oct) #4595), POST /api/migration-packs/preview (feat(migration-packs): view and edit migration packs in OpenRegister #4602), and the example-set occ commands.

git grep -n -i 'roles block\|frozen\|archived object\|personal settings' CHANGELOG.md returns nothing.

rjzondervan

Verdict: REQUEST_CHANGES (Strict)

Not ready for beta. Five blockers sit in code no second person reviewed, three of them in the draft model: the draft rule lives in the SQL read predicate only, and PermissionHandler::hasPermission() has none, so bulk delete, the readable audit trail and (on Wilco's open thread) bulk save all reach other users' drafts. Adding the draft rule to hasPermission() when an object is passed would close all three. Two more blockers landed after Wilco's review (#4656, #4661). Under Strict every 🟡 below blocks as well.

🔴 Blockers (5)

A form payload's id picks the uuid, and a scoped draft save then writes over any draft with RBAC off — lib/Service/Form/FormSubmitService.php:592 (introduced in #4563, #4566). With no mapping pairsOf() passes the payload's id through, SaveObject takes it as the uuid, and a scoped draft save writes with _rbac: false, so any object in the destination schema can be overwritten. Tracked in #4664.
Bulk delete ignores every draft rule: others' drafts deleted, soft-deleted, no draft.removed — lib/Service/ObjectService.php:3190 (introduced in #4566, #4600). filterUuidsForPermissions() reads across all magic tables and checks only the schema's delete permission, so a bulk delete removes other users' drafts that a single delete would refuse. Tracked in #4665.
/api/audit-trails/readable shows other users' draft contents — lib/Service/Audit/ReadableAuditTrailLister.php:94 (introduced in #4566, #4614). The lister resolves objects across all magic tables and asks hasPermission('read'), which has no draft rule, so anyone who may read the schema sees the changed values of another user's draft. Tracked in #4666.
Tree fields returns writeOnly, property-restricted and encrypted values raw — lib/Service/Relation/ObjectTreeService.php:479 (introduced in #4656). GET …/tree?fields=a,b copies each named key straight from the raw object onto every node, with no property RBAC, writeOnly redaction or decryption. Tracked in #4667.
Bulk writer lets a _-named schema property overwrite metadata columns — lib/Db/MagicMapper/MagicBulkHandler.php:217 (introduced in #4661). A schema property whose name is a metadata column (_owner, _organisation, _status, …) passes skippedOnWrite(), and line 233 writes the request's value into that column after line 178 stamped it. Tracked in #4668.

🟡 Concerns (35)

Two concurrent leave-draft requests both announce the creation — lib/Service/Form/FormSubmitService.php:340 (introduced in #4633, #4600). DraftStatusPolicy returns early when the asked status equals the current one before the promotion check, and leftDraft() runs unconditionally, so two concurrent leave-draft requests both release the creation events.
Drafts beyond the 5,000-read cap are neither expired nor counted deferred — lib/Service/Object/DraftExpiry.php:284 (introduced in #4600). The expiry query reads at most 5,000 drafts with no date filter and checks age in PHP afterwards, so drafts past the cap are neither expired nor counted as deferred.
A draft save never claims its upload tokens — lib/Service/Form/FormSubmitService.php:229 (introduced in #4563). saveDraft() maps upload tokens into $plan['files'] and writes the files onto the draft, but unlike submitAll() (line 280) it never calls $this->uploads->claim().
A required generated identifier blocks every submit and activation — lib/Service/Object/DraftStatusPolicy.php:229 (introduced in #4600, #4633). serverFilledExcusals() excuses a property from required when it has a default, computed, or x-openregister.serverSet.
Advance-a-case step moves the oldest realisation of a repeating item — lib/Service/Case/Nodes/CaseAdvanceNode.php:218 (introduced in #4595). advance() transitions the first row whose key matches, and rows sort by position then id, so with a repeating item it moves the oldest (terminal) realisation.
Case step with a configured uuid anchors the plan to the wrong register/schema — lib/Service/Case/Nodes/CaseNodeBase.php:185 (introduced in #4595). The configured uuid template replaces the target object, so the case can be about another object than the item the step received.
Items ensured as active never arm their due/expiry timers — lib/Service/Case/CasePlanEnsurer.php:347 (introduced in #4544, #4595). ensure() inserts missing items directly in their state, bypassing the state machine, so no CaseItemTransitionedEvent fires and their due/expiry timers are never armed (sibling of the open CasePlanItemTimers.php:219 thread).
System-created plans realise flows as a non-user system:<app> — lib/Service/Case/CasePlanService.php:191 (introduced in #4544). createPlanAsSystem() passes system:<app> as the actor to insertPlan(), which runs cascade->evaluate(actor: 'system:<app>').
bandRate skips schedule validation for a unit of zero or less — lib/Service/Calculation/RateSchedule.php:137 (introduced in #4571). rateAt() returns null for $unit <= 0 before normalise() runs, so a malformed schedule (empty, not a list, falling bounds, an open band first) is not refused when the unit is 0 or negative.
An empty own property fills a deep-link placeholder with nothing — lib/Service/Search/ObjectSearchResultFormatter.php:196 (introduced in #4555). An empty own property is substituted as '', so no literal {name} survives and the deep-link fallback never triggers.
Duplicate decision-table key check fails open — lib/Listener/DecisionTableValidationListener.php:205 (introduced in #4595). keyTakenBy() answers "no clash" (null) when the search for an existing key throws, so the save goes through with a duplicate key.
Admin-UI re-import silently replaces local authorization blocks — lib/Controller/RegisterDescriptorController.php:152 (introduced in #4618). #4618 routes the Register descriptors panel's button through the new reimport() (via: 'admin settings: Register descriptors').
OpenRegister descriptor re-import uses a different configuration id than install — lib/Service/RegisterDescriptorService.php:290 (introduced in #4618). importDescriptorFile() calls importFromApp(appId: $appId, ...) with $appId = 'openregister', forced, with the descriptor's own version.
Remove reads any lookup error as already deleted — lib/Service/ExampleSet/ExampleSetStore.php:288 (introduced in #4603, #4612). ExampleSetStore catches every Throwable on the lookup and reads it as "already gone", and ExampleSetStoreTest.php:154 locks that in (sibling of the open ExampleSetInstance.php:159 thread).
Reinstall duplicates an object when its read fails — lib/Service/ExampleSet/ExampleSetInstaller.php:652 (introduced in #4603). kept() gets null from store->get() on any Throwable, so a reinstall whose read failed creates a second object and overwrites the record id (sibling of the open ExampleSetInstance.php:159 thread).
Change and lookup take the first match, not exactly one — lib/Service/ExampleSet/ExampleSetChanges.php:332 (introduced in #4603). REQ-EXS-011 says a change has 'a where that finds exactly one existing object by business key'; REQ-EXS-002 says a lookup fills 'the id of the one row a lookup by business key finds'.
Reinstall overwrites user edits to reference fields — lib/Service/ExampleSet/ExampleSetInstaller.php:386 (introduced in #4632). refreshReferences() resolves every top-level field whose declaration holds {{object:}} or {{lookup:}} and writes each one whose stored value differs.
Install, remove, install leaves a required register empty — lib/Service/ExampleSet/ExampleSetPrerequisites.php:120 (introduced in #4603, #4658). Remove soft-deletes the objects of the recorded import jobs (ExampleSetRegisters::removeImport()) but leaves the register and its schemas, which the same install created.
Remove deletes an organisation others started using — lib/Service/ExampleSet/ExampleSetInstance.php:163 (introduced in #4603). ExampleSetInstance::delete() deletes the organisation by uuid when the record lists it.
Generated config value deleted even after it was changed — lib/Service/ExampleSet/ExampleSetAppConfig.php:146 (introduced in #4603). revert() skips the 'changed since' check for a generated entry (($entry['generated'] ?? false) === false && $changed === true) and then always calls deleteKey() for it, because had is ignored when generated is true.
Local edits and shipped seed versions share one sequence — lib/Service/Configuration/ImportHandler.php:3612 (introduced in #4635). The shipped @self.version is compared with the object's own version, which every local save bumps, so a local edit can mask a shipped update and a later shipped version overwrites the edit.
Pack editor shows untranslated validator and engine messages — lib/Service/MigrationPack/PackDefinitionValidator.php:205 (introduced in #4602). #4602 rewrote these messages for the editor ('Mapping row 4', 'the number the pack editor shows in its table'), and explainError() shows details.errors as a list under the save error (MigrationPackDetail.vue:446).
Tree endpoint skips the doelbinding (PurposeGuard) check — lib/Controller/ObjectRelationsController.php:507 (introduced in #4656). tree() authorises with ObjectReadAccess::readable() only.
An access-check failure reads as a complete tree — lib/Service/Relation/ObjectTreeService.php:437 (introduced in #4656). The class promises "The answer says which bound stopped it, so a caller can refuse a partial tree instead of reading it as the whole one".
Node cap bounds the answer, not the work — lib/Service/Relation/ObjectTreeService.php:302 (introduced in #4656). The node cap is checked after childrenOf() has fetched every child row of the level with no LIMIT, so it bounds the answer but not the work.
Ancestor nodes name the uuid of an ancestor the caller may not read — lib/Service/Relation/ObjectTreeService.php:390 (introduced in #4656). For each ancestor node, parent is the grandparent uuid read from the ancestor's raw data (line 390), whether or not the caller may read the grandparent.
RESTRICT by key never blocks a scalar reference on MySQL/MariaDB — lib/Service/Object/ReferentialIntegrityService.php:1164 (introduced in #4570). findReferencingInMagicTable() quotes the column with "…", which MySQL/MariaDB read as a string literal, so the new key-RESTRICT path never finds a scalar reference there (the helper is already on beta).
Resident count needs an integriq that beta does not ship — lib/Service/Integration/ExternalIntegrationRouter.php:737 (introduced in #4657). dispatch() passes withholdBodies: true as a named argument to integriq's CallService::call.
An oversized deferred entry silently loses its listener side effect — lib/Service/Deferral/ListenerDeferralService.php:309 (introduced in #4616). An entry larger than the job argument budget is continued with one error log line: the notification / webhook / projection it carried never runs, and nothing retries it.
Object-folder endpoints have no frozen/archived guard — lib/Controller/FilesController.php:315 (introduced in #4549). REQ-OAS-007: every write to a frozen or archived object's files "SHALL be refused… the API SHALL answer 409". #4549 wires FileWriteGuard into FilesController::ensureObjectAccess() only.
Draft activation answers the raw entity, writeOnly values included — lib/Controller/ObjectDraftController.php:182 (introduced in #4600). activate() returns $activated->jsonSerialize(): the saved entity straight from saveObject(), never passed through RenderObject.
A blocked portal task is still listed and completable in the portal — lib/Db/TaskMapper.php:592 (introduced in #4595, #4569). applyPartyPredicates() has no blocked_by predicate (only the inbox path calls applyBlockedExclusion()), so a blocked task is still listed and completable in the portal.
Personal data in a draft cannot be found by a data-subject request — lib/Service/Object/ObjectEventGate.php:86 (introduced in #4566, #4600, #4633). Draft side effects are held by ObjectEventGate, including the entity-index listener, so personal data in a draft never reaches openregister_entities, the only source a data-subject request searches.
Another user's hidden draft blocks a RESTRICT delete — lib/Service/Object/ReferentialIntegrityService.php:1121 (introduced in #4566, #4570). The RESTRICT lookup filters on _deleted only, so another user's hidden draft blocks the delete and blockers[].objectUuid names a draft the deleter cannot open.
CHANGELOG omits this release's upgrade-visible behaviour changes — CHANGELOG.md:37 (introduced in #4589, #4549, #4587, #4614, #4563, #4600, #4656, #4595, #4602). The release adds three CHANGELOG bullets, all about drafts, for 47 PRs; frozen files, the roles-block change and the moved personal settings are not mentioned.

🟢 Minor (16)

Draft sweep counts are logged at info, hidden at the default log level — lib/BackgroundJob/DraftExpiryJob.php:123 (introduced in #4600). The sweep's per-schema and total counts are logged at info, below the default log level, so the promised "counted zero" never reaches the log.
/api/forms/validate reads any schema across organisations — lib/Controller/FormsController.php:126 (introduced in #4563). Any signed-in user can name any schema by id, uuid or slug.
The PATCH refusal of somebody else's draft is a 403, so a draft id is an oracle there — lib/Service/ObjectService.php:2345 (introduced in #4600). The form path takes care that "anything else answers the same 404, so a draft id is not an oracle" (FormSubmitService, DraftFinder, ObjectDraftController).
Decision tables resolve across every organisation — lib/Service/Dmn/DecisionTableStore.php:158 (introduced in #4595). lookup() (and the duplicate check in DecisionTableValidationListener::keyTakenBy()) reads the decision-tables register with _rbac: false, _multitenancy: false.
Reader's object history keeps another object's uuid from a cascade — lib/Service/Audit/HistoryEntryRedactor.php:159 (introduced in #4614). filterChanged() withholds properties but not changed.cascadeContext, which AuditTrailMapper sets to another object's uuid.
Hidden decision still shows through anonymized/anonymizedValue — lib/Service/Decision/DecisionVisibility.php:59 (introduced in #4595). MASKED_FIELDS blanks bases, skipAnonymization and decisionRecord, and sets decision to hidden.
OAS still reads an unresolved roles block as open — lib/Service/Rbac/EffectiveAuthorization.php, no changed line within reach, so no thread (introduced in #4589). #4589 makes PermissionHandler::expandRoles() answer a roles block that resolves to nothing with four empty verbs (deny-all).
--check also creates OpenRegister organisations — lib/Service/ExampleSet/ExampleSetPrerequisites.php:74 (introduced in #4603). check() runs ensureOrganisations(), so --check creates organisations, and REQ-EXS-004 and REQ-EXS-016 contradict each other on it.
Import jobs attributed to the set by a time window — lib/Service/ExampleSet/ExampleSetRegisters.php:138 (introduced in #4603). recordedImport() diffs jobIds() before and after the import, across openregister and openregister.mock.
Install record kept in a non-lazy app-config value — lib/Service/ExampleSet/ExampleSetRecord.php:193 (introduced in #4603). write() stores the whole record (every object id, change, file, app-config entry and preference) with setValueString() without lazy: true.
Pack import answers 201 when it replaced a pack — lib/Controller/MigrationPacksController.php:293 (introduced in #4602). import() knows whether it created or replaced a pack ($result->created, logged as created/replaced), but answers 201 either way with only the pack in the body.
New constants split DEFAULT_FIELDS from its docblock — lib/Service/Integration/Providers/BrpPersonProvider.php:109 (introduced in #4657). The three new constants sit between DEFAULT_FIELDS and its docblock, which now documents the wrong constant.
Listener never checks the object folder node itself — lib/Listener/FrozenNodeWriteListener.php:223 (introduced in #4549). candidateOwnerFolder() starts at $node->getParent(), so when the node IS the <uuid> object folder (delete or rename/move of the whole folder) no candidate is found and the operation is allowed.
retentionDays accepts 0, negatives and non-numbers silently — lib/Service/Export/ExportProfileService.php:420 (introduced in #4595). (int)$data['retentionDays'] stores 0, -5 or "abc" silently, and the recorder then clamps exports to expire one minute after production.
blockedBy may name any task, readable or not — lib/Service/Task/TaskBlockerGuard.php:106 (introduced in #4595). assertDeclarable() resolves the blocker with the unscoped TaskMapper::findByUuid(); there is no check that the caller may see it.
Validator and service disagree on referenceKey: uuid — lib/Service/Schemas/PropertyValidatorHandler.php:982 (introduced in #4570). ReferentialIntegrityService::extractReferenceKey() treats referenceKey empty or uuid as a plain uuid reference (all onDelete actions apply).

✅ Verified clean

BRP resident count (#4657): answers, logs and the call log carry a number only, no name, BSN or address id; there is no HTTP route in OpenRegister for it.
Migration-pack endpoints are admin-gated; example sets are CLI-only; pack and set loading realpath-checks fragment files (no path traversal).
The five migrations in this release are guarded, none returns null, and info.xml is bumped over beta; no double ObjectCreatedEvent on draft promotion.

Tracking issues

#4664 — A form payload's id picks the uuid, and a scoped draft save then writes over any draft with RBAC off
#4665 — Bulk delete ignores every draft rule: others' drafts deleted, soft-deleted, no draft.removed
#4666 — /api/audit-trails/readable shows other users' draft contents
#4667 — Tree fields returns writeOnly, property-restricted and encrypted values raw
#4668 — Bulk writer lets a _-named schema property overwrite metadata columns

Scope decision: full, provenance-driven review of the whole release, in five slices (forms and drafts; decisions, cases, flow, RBAC and audit; example sets, migration packs and import; database, relations, BRP and files; a cross-PR integration pass). 47 merged PRs, 0 direct commits: none had an approving review by someone other than its author, so all code was read at full depth. Wilco's 31 open threads were not repeated; siblings at other sites are marked as such.
Not read: about 560 files, mostly l10n/, tests/, openspec/ and src/ outside the migration-pack editor; translation completeness, lint, types and the unit suite are left to the green quality checks.
CI: every required check is green on 8e7bbd8; deploy / Deploy Documentation is red (not required) and E2E (Playwright) was skipped.

🤖 Generated with Claude Code

rubenvdlinde added 30 commits October 10, 2026 18:34

refactor(forms): phpmd: payload pairs in their own method, named supp…

b26e555

…ressions

Merge branch 'build/d179-form-destination-validator' into build/d180-…

74bd9a0

…draft-lifecycle

refactor(objects): phpmd: draft move and write-or-promote in their ow…

118b56a

…n methods

fix(forms): a draft is written only by its owner, checked explicitly …

fb3a024

…before the save

fix(forms): psalm, phpstan and the lookup audit rule; one destination…

5cac086

… goes through submit()

Merge remote-tracking branch 'origin/development' into build/d179-for…

c4c7311

…m-destination-validator

Merge build/d179-form-destination-validator into build/d180-draft-lif…

e1ab4e2

…ecycle

fix(objects): psalm on draftData; the aggregation fixture table carri…

4339dbe

…es _status

style(forms): prettier on the e2e spec

f3a6342

style(objects): prettier on the e2e spec

a820830

fix(objects): the e2e spec without the merge markers a820830 carried,…

ff7b169

… prettier-formatted

Merge remote-tracking branch 'origin/development' into shep/4541

09a8313

Merge remote-tracking branch 'origin/development' into shep/4544

0341d8f

Merge remote-tracking branch 'origin/development' into shep/4546

cdecc14

Merge remote-tracking branch 'origin/development' into shep/4547

ae39acf

Merge remote-tracking branch 'origin/development' into shep/4549

bf9e3a2

Merge remote-tracking branch 'origin/development' into shep/4553

279dbfc

Merge remote-tracking branch 'origin/development' into shep/4555

0e56e51

Merge remote-tracking branch 'origin/development' into shep/4559

35744f1

Merge remote-tracking branch 'origin/development' into shep/4563

b56f336

Merge remote-tracking branch 'origin/development' into shep/4569

2c58a31

Merge remote-tracking branch 'origin/development' into shep/4570

fdb9328

Merge remote-tracking branch 'origin/development' into shep/4571

5d6d194

Merge remote-tracking branch 'origin/development' into shep/4572

8004458

Merge remote-tracking branch 'origin/development' into shep/4534

f7211fa

fix(ci): clear development's red after #4573 (prettier on the folder-…

acd1872

…browser e2e, @SPEC on three responder methods)

WilcoLouwerse reviewed Oct 15, 2026

View reviewed changes

WilcoLouwerse requested changes Oct 15, 2026

View reviewed changes

rubenvdlinde and others added 14 commits October 15, 2026 19:12

refactor(example-sets): a mock import is its own method, not a flag

b637c7c

Merge pull request #4658 from ConductionNL/fix/example-set-requires-m…

4d6fdde

…ock-register fix(example-sets): a set may require a register OpenRegister ships only as a mock

refactor(magic-tables): the column classifier is an instance, and the…

f28140d

… bulk writer asks it too check:strict's phpmd flagged static access, a long variable name and the bulk writer's method length; the write-side rule moves into MagicMetadataColumn::skippedOnWrite().

Merge pull request #4661 from ConductionNL/fix/g4-zuiddrecht-r2-findings

f474f0d

fix(magic-tables,example-sets): underscore properties read as properties, dot-path config fields, check counts like install

rjzondervan reviewed Oct 16, 2026

View reviewed changes

rjzondervan requested changes Oct 16, 2026

View reviewed changes

Contract branch (spec draft-owner-activation-and-lifetime)	Single delete	Bulk delete
Only somebody who can read a draft may change it	✓ RBAC lookup hides it → 404	✗ `filterUuidsForPermissions()` → `hasPermission(delete, objectOwner)`, which has no draft rule
A draft's delete is permanent, with its files	✓ line 3190	✗ `deleteHandler->deleteObject()` without `permanent`, so a soft-deleted tombstone keeps the unsent input
One `draft.removed` audit entry without content	✓ written in the permanent branch of `DeleteObject::delete()`	✗ not permanent, so no entry
A draft is deletable on an archival schema	✓ `isDeletableDraft()`	✗ `rejectIfArchivalImmutable()` refuses it, so the owner cannot bulk-discard their own draft there

Conversation

github-actions Bot commented Oct 11, 2026 • edited by WilcoLouwerse Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelist

Features (21)

Fixes (14)

Trains (bundled PRs) (2)

Dependencies and security (7)

Maintenance (CI, gates, specs, docs) (2)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WilcoLouwerse left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Verdict: REQUEST_CHANGES (Strict)

🔧 Fix PR: #4655 — 23 of 33 findings, one commit each

🔴 Blockers (6)

🟡 Concerns (19)

🟢 Minor (8)

✅ Verified clean

Uh oh!

Choose a reason for hiding this comment

🔴 Blocker — A form payload's id picks the uuid, and a scoped draft save then writes over any draft with RBAC off

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

🔴 Blocker — Bulk delete ignores every draft rule

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

🔴 Blocker — /api/audit-trails/readable shows other users' draft contents

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

github-actions Bot commented Oct 11, 2026 •

edited by WilcoLouwerse

Loading

WilcoLouwerse left a comment •

edited

Loading

🔴 Blocker — A form payload's `id` picks the uuid, and a scoped draft save then writes over any draft with RBAC off

🔴 Blocker — `/api/audit-trails/readable` shows other users' draft contents

rjzondervan left a comment •

edited

Loading