fix(federation): delete the orphaned publish wrapper and test the seam that ships (gate-57) - #447
Merged
rubenvdlinde merged 1 commit intoAug 7, 2026
Conversation
…m that ships
Gate 57 (orphaned-write-capability) named
`FederationService::publishEntryForFederation()` — zero non-test production
callers. Tracing the callers showed the capability is NOT missing, which is
the opposite of what a dead write-capability usually means.
`PublicationService::publish()` is live through
`PublicationController::publish()`, routed as `publication#publish`
(`PUT /api/publication/{objectType}/{uuid}/publish`, appinfo/routes.php:195).
The federation wrapper was a pass-through to that same service call, minus
two things the routed path has:
* the per-object `authorizeEntry()` IDOR guard (ADR-005), and
* the optional ISO-8601 `$when` moment — its signature was
`(string $objectType, string $uuid)`, so a scheduled publication was
unreachable through it.
So it was not an unwired capability waiting for a caller; it was a second,
weaker publish seam that nothing used. Wiring a route to it would have
duplicated a live capability and widened the auth surface. It is deleted,
along with its private `getPublicationService()` helper and the now-unused
import.
The two unit tests that named it were its ONLY callers anywhere in the repo.
They are replaced by tests on the seam that actually ships — there was no
`PublicationControllerTest` at all, so the live publish path and its IDOR
guard were untested:
* an admin publishes and `$when` is FORWARDED to the service;
* a non-owning aanbod-beheerder gets 403 and `publish()` is never reached;
* a peer-sourced (federated mirror) entry is refused even for an admin.
Verified both directions, in a disposable nextcloud:32-apache container
(PHP 8.3, the app bind-mounted at custom_apps/ so the deployed code is the
code under test):
| | result |
|---|---|
| gate 57 before | 1 finding — publishEntryForFederation |
| gate 57 after | 0 findings |
| new tests, guard intact | 3/3 pass (17/17 across both files) |
| new tests, guard removed + `$when` dropped (mutant) | 3/3 FAIL |
The mutant run is the positive control: all three assertions are about the
guard and the forwarded argument, not about the mock.
openspec/specs/federated-catalog-sync/spec.md named the deleted wrapper as
the publish seam. It now names `PublicationController::publish()` and states
that federation does not own a second publish entry point — the requirement's
real invariant (visibility enforced by the OpenRegister public RBAC read gate
on `publicatiedatum`) is unchanged and still satisfied.
phpcs: 0 errors on both changed lib/ files.
Contributor
Quality Report — ConductionNL/softwarecatalog @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-vue-demi | ✅ | ||||
| test-l10n | ✅ | ||||
| composer | ✅ | ✅ 121/121 | |||
| npm | ✅ | ✅ 853/853 | |||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ✅ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-07 06:33 UTC
Download the full PDF report from the workflow artifacts.
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
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.
What gate 57 found, and why the obvious reading was wrong
Gate 57 (orphaned-write-capability) named
FederationService::publishEntryForFederation()— zero non-test production callers.The obvious reading of a dead write capability is "the feature is missing". Here it was the opposite.
PublicationService::publish()is live, throughPublicationController::publish(), routed aspublication#publish(PUT /api/publication/{objectType}/{uuid}/publish,appinfo/routes.php:195). Catalog entries can be published; the wrapper was a second seam that nothing used.It was also the weaker of the two. Compared with the routed path it was missing:
authorizeEntry()IDOR guard (ADR-005) — admin, or an aanbod-beheerder whose organisation owns the entry; peer-sourced mirrors refused;$whenmoment — its signature was(string $objectType, string $uuid), so a scheduled publication could not be expressed through it at all.Wiring a route to it would therefore have duplicated a live capability and widened the auth surface. It is deleted, with its private
getPublicationService()helper and the now-unused import.The tests moved to where the code ships
The two
testPublishEntryForFederation*tests were the only callers of the method anywhere in the repo — they constructed the service and invoked something no production path could reach. Deleting them exposed that there was noPublicationControllerTestat all: the live publish path and its IDOR guard were untested.tests/Unit/Controller/PublicationControllerTest.phpnow covers:$whenis forwarded to the service;publish()is never reached;Verified both directions
Run in a disposable
nextcloud:32-apachecontainer (PHP 8.3) with the app bind-mounted atcustom_apps/softwarecatalog, so the deployed code is the code under test. No shared dev instance was touched.publishEntryForFederation$whendropped (mutant)The mutant run is the positive control: all three assertions are about the guard and the forwarded argument, not about the mock shape.
php -lclean.phpcs: 0 errors on both changedlib/files (the two remaining warnings are the pre-existing missing-@spec-on-class notices both classes already carried).Spec
openspec/specs/federated-catalog-sync/spec.mdnamed the deleted wrapper as the publish seam. It now namesPublicationController::publish()and states explicitly that federation does not own a second publish entry point. The requirement's actual invariant — visibility enforced by the OpenRegister public RBAC read gate{group:public, match:{publicatiedatum:{$lte:$now}}}— is unchanged and still satisfied.No waiver, no
@orphaned-write-capability exclude, no baseline entry: the gate goes green because the dead code is gone.🤖 Generated with Claude Code