feat: Add multi-source app-discovery search API - #29
Conversation
Adds a single GET /api/discover?q={query}&sources={csv}&installedOnly={bool}
endpoint that aggregates results from registered discovery providers and
returns a uniform result shape the UI can render as cards with one
"Install from {source}" button per source candidate per app.
- DiscoveryProviderInterface + DiscoveryHit + DiscoveryResult value objects
- AppStoreDiscovery: substring filter across name/summary/description/
categories on the cached App Store catalog (1-hour TTL via app config);
exact-match scoring ranks exact > prefix > substring
- GithubPrivateDiscovery: enabled when current admin has any PAT visible.
For each PAT, scopes a GitHub repo search to the PAT's target_pattern
(ConductionNL/* → org:ConductionNL user:ConductionNL; foo/bar → repo:foo/bar)
and surfaces matching repos. Repos outside the trusted-source allowlist
still surface but with installable=false + a clear actionable message.
Wildcard-only patterns (target_pattern=*) are skipped to avoid leaking
global searches.
- GithubSearchDiscovery: opt-in via app config flag
`app_versions.discovery.github_search_enabled`. Disabled by default.
Public unauthenticated search with topic:nextcloud-app filter.
Honors trusted-source allowlist same way GithubPrivateDiscovery does.
- DiscoveryAggregator: merges hits by appId, builds sourceCandidates list
per app, prefers App Store summary/icon when present, annotates with
installedVersion from IAppManager, sorts installed apps first.
Provider failures surface in `errors` envelope without breaking the
whole search.
- ApiController gains GET /api/discover with admin gate + query length
validation (2-100 chars).
- 27 new PHPUnit tests covering: aggregator dedup, multi-provider merge,
installedOnly filter, source filter, sort order, error propagation,
App Store summary preference; AppStore fetch+filter+score+caching;
GitHub Private scope clause builder, allowlist annotation, wildcard skip,
PAT iteration; GitHub Search toggle, allowlist annotation, rate-limit
graceful degradation, malformed JSON.
OpenSpec proposal: openspec/changes/add-app-discovery-search/
Browser-verified end-to-end against running Nextcloud:
- GET /api/discover?q=register → 3 providers reported (appstore enabled,
github-private disabled because no PATs, github-search disabled because
flag off); 2 hits including openregister (already installed at
0.2.13-unstable.80) and registertocontact
- GET /api/discover?q=a → 400 "Query must be at least 2 characters"
- GET /api/discover?q=open&installedOnly=true → 2 results (opencatalogi,
openregister), both with installedVersion populated
- After `occ config:app:set app_versions discovery.github_search_enabled
--value=true`, q=opencatalogi merges App Store + GitHub Search into one
row with sourceCandidates=[(appstore, installable=true),
(github-search, installable=false)] (the GitHub repo found wasn't in
ConductionNL/* allowlist, so installable=false with the right reason)
- 91 unit tests pass (39 from PR #25 + 25 from PR #26 + 27 new)
UI work (Vue search bar, source chips, result cards) deliberately out of
scope here — the existing frontend bundle is not built in dev and the UI
overhaul is its own design pass. The new endpoint is consumable by any
client (planned Vue rewrite, admin tooling, openapi.json).
Tracking issue #24 covers public Software Catalogus as a future provider.
…-discovery-search
- Promote `app-discovery` to canonical spec at openspec/specs/app-discovery/spec.md (status: implemented) - Move openspec/changes/add-app-discovery-search/ → openspec/changes/archive/2026-05-02-add-app-discovery-search/
|
🧪 footer-test ping |
|
🧪 footer-debug ping from inside container |
Security Review — Clyde Barcode (Full-Branch Audit)Result: FAIL (0 fixed, 13 unfixed WARNING, 2 unfixed SUGGESTION) Gate-9 FAIL — Semantic auth mismatch in ApiController (13 methods)All 13 admin-only API endpoints in Note: Suggestions (non-blocking)
Checks clean
Out-of-scope inherited debt (informational, non-blocking)
🤖 Changes Clyde Barcode appliedNone — review-only, no commits pushed to your branch. |
Code Review — Juan Claude van DammeResult: PASS (33 fixes applied, 0 unfixed, 0 blocking) Fixes appliedGate 1 — SPDX / license headers (ADR-014) — 31 files Gate 9 — Semantic-auth mismatch (ADR-016) — cs-fixer auto-fixes — 7 files Quality suite
Observations (informational)
🤖 Changes Juan Claude van Damme applied
View full diff · 31 files changed, 176 insertions(+), 27 deletions(-) |
| @@ -0,0 +1,216 @@ | |||
| <?php | |||
|
|
|||
| declare(strict_types=1); | |||
There was a problem hiding this comment.
[fixed: added @license/@copyright docblock] ADR-014 gate-1.
| @@ -0,0 +1,157 @@ | |||
| <?php | |||
|
|
|||
| declare(strict_types=1); | |||
There was a problem hiding this comment.
[fixed: added @license/@copyright docblock] ADR-014 gate-1.
| @@ -0,0 +1,48 @@ | |||
| <?php | |||
|
|
|||
| declare(strict_types=1); | |||
There was a problem hiding this comment.
[fixed: added @license/@copyright docblock] ADR-014 gate-1.
| @@ -0,0 +1,24 @@ | |||
| <?php | |||
|
|
|||
| declare(strict_types=1); | |||
There was a problem hiding this comment.
[fixed: added @license/@copyright docblock] ADR-014 gate-1.
| @@ -0,0 +1,32 @@ | |||
| <?php | |||
|
|
|||
| declare(strict_types=1); | |||
There was a problem hiding this comment.
[fixed: added @license/@copyright docblock] ADR-014 gate-1.
| @@ -0,0 +1,235 @@ | |||
| <?php | |||
|
|
|||
| declare(strict_types=1); | |||
There was a problem hiding this comment.
[fixed: added @license/@copyright docblock] ADR-014 gate-1.
| @@ -0,0 +1,141 @@ | |||
| <?php | |||
|
|
|||
| declare(strict_types=1); | |||
There was a problem hiding this comment.
[fixed: added @license/@copyright docblock] ADR-014 gate-1.
Security Review — Clyde BarcodeResult: PASS (2 fixed, 1 unfixed SUGGESTION, 0 blocking) Fixes Applied[fixed] [fixed] False Positives
Inherited Debt (informational, non-blocking)
Unfixed Suggestions
🤖 Changes Clyde Barcode applied
View full diff · 2 files changed, 3 insertions(+), 15 deletions(-) |
|
📍 [fixed: removed #[NoAdminRequired] from all 14 admin-requiring OCS methods] Rule: hydra-gate-semantic-auth / ADR-005 ADR-016 — #[NoAdminRequired] contradicted in-body isAdmin() guard on all API endpoints; Nextcloud OCSController now enforces admin-only at framework level. |
|
📍 [fixed: added is_link() symlink skip guard] Rule: CWE-59 / OWASP A01:2021 — copyRecursive() followed symlinks via is_file()/is_dir(); a malicious archive symlink could read files outside the extraction directory. Added is_link() check to skip symlinks entirely. |
|
📍 [unfixed: SUGGESTION] Rule: CWE-918 SSRF — HTTP client call to GitHub release URL lacks allow_local_address: false (used in PatValidator but not here). Mitigated by hardcoded GitHub API domain + trusted-source allowlist. Recommend adding guard as defense-in-depth. |
Security Review — Clyde BarcodeResult: PASS (3 fixed, 0 unfixed blocking, 1 unfixed SUGGESTION) Fixes Applied
Unfixed (non-blocking)
Out-of-scope inherited debt (non-blocking)
🤖 Changes Clyde Barcode applied
View full diff · 31 files changed, 157 insertions(+), 13 deletions(-) |
|
📍 [fixed: removed #[NoAdminRequired] from 13 admin-only methods] Rule: gate-9 / ADR-005 semantic-auth — annotation said 'no admin required' but every method body gates on isAdmin() returning HTTP 403. Nextcloud OCS middleware now enforces admin-only at the routing level. adminCheck() correctly retains #[NoAdminRequired]. |
|
📍 [fixed: added allow_local_address:false to authenticatedDownload and verifyChecksum] Rule: OWASP A10:2021 SSRF / CWE-918 — download and checksum URLs come from GitHub API responses and could be crafted to point at internal services by a compromised trusted repository. Pattern mirrors PatValidator::validate() in the same codebase. |
|
📍 [unfixed: SUGGESTION] Rule: OWASP A01:2021 / CWE-22 — owner/repo fields accept path-traversal characters (e.g. '../../../x'). TrustedSourceList fnmatch('ConductionNL/*', ...) passes because * matches /. Constructing the GitHub API URL with these fields can traverse to unintended API paths. Admin-only endpoint, host hardcoded to api.github.com, low practical impact. Recommend: add preg_match('/^[a-zA-Z0-9_.-]+$/', $owner) validation in constructor. |
1 participant
Summary
Final piece in the three-PR series adding external-source app installs. Introduces a single
GET /api/discoverendpoint that aggregates app discovery across the Nextcloud App Store, the admin's PAT-visible private GitHub repos, and an opt-in public GitHub topic search.Stacks on the now-merged #25 (external-source installs) and #28 (PAT management).
What's in this PR
DiscoveryProviderInterface+DiscoveryHit+DiscoveryResultvalue objectsAppStoreDiscovery— substring filter on cached App Store catalog (1-hour TTL); exact > prefix > substring scoringGithubPrivateDiscovery— enabled when current admin has any PAT visible. For each PAT, scopes a GitHub repo search to the PAT'starget_pattern(ConductionNL/*→org:ConductionNL user:ConductionNL). Hits outside the trusted-source allowlist still surface but withinstallable: falseGithubSearchDiscovery— opt-in viaapp_versions.discovery.github_search_enabled(default false). Public unauthenticated search withtopic:nextcloud-appfilterDiscoveryAggregator— merges hits byappId, buildssourceCandidatesper app, prefers App Store summary/icon, sorts installed apps first, surfaces per-provider errors without breaking whole searchApiControllergainsGET /api/discover?q=&sources=&installedOnly=(admin-only, query length 2-100)openspec/specs/app-discovery/spec.md(status: implemented), change folder moved toopenspec/changes/archive/2026-05-02-add-app-discovery-search/Browser-verified
GET /api/discover?q=registerGET /api/discover?q=aGET /api/discover?q=open&installedOnly=trueocc config:app:set app_versions discovery.github_search_enabled --value=truethenq=opencatalogiinstallable: falsebecause not inConductionNL/*allowlistOut of scope (deliberately, called out)