feat(apphost): own store discovery — GenericStoreService + controller base (ADR-080) - #2310
Merged
Merged
Conversation
… base (ADR-080) A "store" is a remote OpenRegister instance exposing installable items over its objects API. OpenRegister is already the SERVER of that protocol; this makes it the client too, instead of each app growing its own proxy. Generalised from openbuild's RemoteTemplateStoreService, parameterised by a per-app StoreDescriptor (appId, schema, defaultRegister, cardFields). The engine owns, once: isConfigured/search/resolve, the SSRF guard, redirect refusal, bounded timeouts, Bearer-only token use, and the four-outcome contract. INSTALL is deliberately absent. Cloning an application template, enabling a connector adapter and instantiating an agent template are different operations with different authorization, so each app keeps its own install action and calls resolve() for the payload. That is the ADR-080 Decision 3 seam. The guard is now a direct SecurityService::assertSafeFetchUrl() call. openbuild reached it through a dynamic class-string with a weaker local fallback if the class was missing — reasonable there, unnecessary here, and one fewer degraded path to get wrong. On the tests: the first run had every happy path returning store_unreachable, because assertSafeFetchUrl resolves DNS and fails CLOSED, and the fixture hostname does not resolve. The SSRF negative controls were passing for the wrong reason — they only proved that EVERYTHING was rejected. They now use literal public IPs so the positive control is real and offline, and a dedicated test pins the fail-closed-on-unresolvable behaviour so the next reader does not rediscover it. 14/14; full AppHost suite 167/167.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ❌ | ||||
| phpmd | ❌ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| composer | ✅ | ✅ 174/174 | |||
| npm | ✅ | ✅ 713/713 | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ✅ |
Quality workflow — 2026-08-03 18:53 UTC
Download the full PDF report from the workflow artifacts.
…ervice, consumed by composition (ADR-080) A cross-app abstract controller broke in three places: NC's router reflects every controller during route MATCHING (absent OR = 500 on every route in the consuming app), leaf test suites stub OR rather than autoloading it so the subclass could not load, and phpstan rejects 'extends unknown class' and refuses to let it be ignored. All three come from `extends` being resolved by the autoloader rather than the container. GenericStoreService stays and is injected. A service type-hint has none of those problems and is the shape leaf apps already use for ObjectService.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ❌ | ||||
| phpmd | ❌ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| composer | ✅ | ✅ 174/174 | |||
| npm | ✅ | ✅ 713/713 | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ✅ |
Quality workflow — 2026-08-03 19:03 UTC
Download the full PDF report from the workflow artifacts.
…phpmd fetch() sat exactly at the cyclomatic threshold (10) and over NPath (216 vs 200). Splitting the body decode out is not just a metric fix — an unparseable body is a DIFFERENT outcome from an unreachable registry, and having the two guard chains in one method is what made them easy to collapse. StaticAccess on SecurityService::assertSafeFetchUrl is suppressed with a reason: it is static upstream, and calling it directly is the point of moving this client into OpenRegister (the app-local copy reached it via a dynamic class-string with a weaker fallback).
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| composer | ✅ | ✅ 174/174 | |||
| npm | ✅ | ✅ 713/713 | |||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ✅ |
Quality workflow — 2026-08-03 19:16 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
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.
A "store" is a remote OpenRegister instance exposing installable items over its objects API. OpenRegister is already the server of that protocol; this makes it the client too, instead of each app growing its own proxy.
What lands
AppHost/Service/GenericStoreService—isConfigured/search/resolve, the SSRF guard, redirect refusal, bounded timeouts, Bearer-only token use, four-outcome contractAppHost/Service/StoreDescriptor— per-app parameters (appId, schema, defaultRegister, cardFields)AppHost/Controller/GenericStoreControllerBase— auth guard, slug validation, error envelopeGeneralised from openbuild's
RemoteTemplateStoreService, which this replaces (see the companion openbuild PR).Install is deliberately absent
Cloning an application template, enabling a connector adapter and instantiating an agent template are different operations with different authorization. Each app keeps its own install action and calls
resolve()for the payload — the ADR-080 Decision 3 seam.The guard is now a direct call
openbuild reached
SecurityService::assertSafeFetchUrl()through a dynamic class-string with a weaker local fallback if the class was missing. Reasonable there; unnecessary here, and one fewer degraded path to get wrong.Worth reading the test file for
The first run had every happy path returning
store_unreachable, becauseassertSafeFetchUrlresolves DNS and fails closed, and the fixture hostname does not resolve. The SSRF negative controls were passing for the wrong reason — they only proved that everything was rejected.They now use literal public IPs so the positive control is real and offline, and a dedicated test pins the fail-closed-on-unresolvable behaviour so the next reader does not rediscover it.
Also asserted: a card never carries the install payload (no
manifest, notoken), andresolve()trusts the returned slug rather than the filter, so a registry ignoring an unknown query param cannot hand back an arbitrary first row.Verification
14/14 new tests · full AppHost suite 167/167.
Purely additive — three new classes plus one test file, no existing file touched.
Companion PRs: hydra (ADR-079/080), nextcloud-vue, openbuild, openconnector.