fix(scope): make the pending schema ref single-use so it cannot leak between operations - #2792
Merged
rubenvdlinde merged 1 commit intoAug 22, 2026
Conversation
ObjectService::setSchema() records the raw ref so a later setRegister() can re-resolve it inside the register the caller names. It was never cleared after being consumed, and ObjectService is reused for many operations in one process — so a ref left behind by an operation that set a schema and never set a register was re-resolved against the NEXT caller's register and refused it. Measured on the shared instance right after the scoping landed: a pipelinq repair step seeding trustConfiguration was told 'posJournalEntryOutbound is not carried by register trust-configuration' — a slug from an entirely unrelated operation. Four repair steps across two apps failed that way. The ref is now consumed and cleared by the first setRegister() that follows, which keeps order-independence for the setSchema/setRegister pair while confining the state to the operation that created it. Two regression tests; must-fail control confirms the first fails against the leaking version.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| composer | ✅ | ✅ 175/175 | |||
| npm | ✅ | ✅ 528/528 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ✅ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-22 11:07 UTC
Download the full PDF report from the workflow artifacts.
This was referenced Aug 22, 2026
rubenvdlinde
added a commit
that referenced
this pull request
Aug 22, 2026
* fix(scope): clear the pending schema ref on the save path too #2792 made the pending ref single-use and cleared it in find(). saveObject() and patchObject() do not go through find() — they call setContextFromParameters(), which was left as it was. So the leak is still live on the write path, which is where it is most visible. MEASURED on buildiq's E2E, 2026-08-22 12:26 UTC (after #2792 landed at 11:09): POST /index.php/apps/docudesk/api/templates Failed to create template: Schema slug "application" is not carried by register "docudesk" (id 19), which carries 9 schema(s). 1 schema(s) elsewhere on this instance carry this slug; none of them is served here, because naming a register makes it a boundary. Docudesk asked for register 19 / schema 18 (`template`) — the seed writes that pair and verifies it on read-back immediately before. `application` is openbuild's schema, left pending by an unrelated earlier call on the same shared ObjectService instance. setRegister() then consumed it and re-resolved it inside docudesk's register, which correctly refused a schema that register does not carry. The refusal is right; the ref should never have been there. find() already guards this, with the same reasoning in its own comment. The fix belongs in the shared helper rather than a second copy at the third call site: when a caller supplies BOTH register and schema, it has named the pair outright, so any pending ref is stale by definition and is dropped before resolution. Order-independence is untouched — a caller that genuinely does setSchema($s)->setRegister($r) still re-resolves — and the boundary is exactly as strict, because the schema is still resolved inside the register named here. Two failing fleet E2E suites trace to this: buildiq's template create (above) and integriq's flow halting immediately after its `write` step. * test: the pending-ref leak on the write path, reproduced find() already has a regression test for this leak. saveObject() and patchObject() do not go through find(), so nothing covered the path that was still broken. Drives setContextFromParameters() by reflection rather than saveObject(): the leak lives entirely in context resolution, and the rest of a save needs a dozen more collaborators that would obscure what is being asserted. Paired control, against the parent commit's lib/: without the fix SchemaNotInRegisterException: Schema slug "application" is not carried by register "docudesk" (id 19) … with the fix OK (2 tests, 2 assertions) That exception is the production message verbatim, from buildiq's E2E on 2026-08-22 12:26 UTC. The wider tests/Unit/Service/ suite reports 8 errors both WITH and WITHOUT this change (LeafProvidersMetadataTest, DbalObjectSourceProviderTest, TextExtractionServiceTest) — pre-existing, measured both directions rather than assumed. --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #2774, fixing a regression it introduced.
ObjectService::setSchema()records the raw ref so a latersetRegister()can re-resolve it inside the register the caller names — that is what made the boundary hold regardless of the order the two setters are called in. But the ref was never cleared after being consumed, and ObjectService is reused for many operations in one process. A ref left behind by an operation that set a schema and never set a register was therefore re-resolved against the next caller's register, and refused it.Measured on the shared dev instance minutes after #2774 landed, during
occ upgrade: a pipelinq repair step seedingtrustConfigurationwas told "Schema slug posJournalEntryOutbound is not carried by register trust-configuration" — a slug from an entirely unrelated operation. Four repair steps across two apps failed that way; the error text was accurate about the boundary and wrong about which schema anyone had asked for, which is what gave the leak away.The pending ref is now consumed and cleared by the first
setRegister()that follows. Order-independence for thesetSchema/setRegisterpair is unchanged; the state is simply confined to the operation that created it.Two regression tests: the ref is cleared once consumed, and a
setRegister()with no pending ref leaves an already-resolved schema untouched. Must-fail control: the first fails against the leaking version and passes against this one. ObjectServiceTest 141 green; lint/phpcs/psalm/phpstan exit 0; phpmd's 16 findings are byte-identical to clean development.🤖 Generated with Claude Code