Skip to content

feat: Add external-source install path with trusted-source allowlist - #25

Merged
rubenvdlinde merged 2 commits into
developmentfrom
feature/add-external-source-installs
May 2, 2026
Merged

feat: Add external-source install path with trusted-source allowlist#25
rubenvdlinde merged 2 commits into
developmentfrom
feature/add-external-source-installs

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Summary

Adds a parallel install path for apps from sources outside the Nextcloud App Store. The signed App Store flow is unchanged; the new path trades the Nextcloud code-signing chain for an explicit trusted-source allowlist + content integrity checks.

This is proposal 1 of 3 in a series:

  1. This PR — the install mechanism for external (GitHub) sources
  2. (Next) add-github-pat-management — encrypted PAT storage so private repos can be installed
  3. (After) add-app-discovery-search — multi-source search UI to find apps to install

Tracking issue #24 covers the future Software Catalogus discovery source.

What's in this PR

Install layer

  • SourceInterface + SourceRegistry with two drivers: AppStoreSource (refactor of existing logic) and GithubReleaseSource (new, public GitHub Releases API)
  • SourceBinding — sticky per-app source persisted as app_versions.source.{appId} JSON in app config
  • TrustedSourceList — fnmatch-style allowlist, defaults to [\"ConductionNL/*\"], configurable via app_versions.trusted_sources
  • ExternalReleaseInstallerService — parallel to SelectedReleaseInstallerService but skips Nextcloud cert/signature verification; instead enforces appId match, version match, and optional SHA-256 against a sibling .sha256 asset
  • InstallFinalizer — extracted post-extract logic (migrations, repair steps, config writes) shared by both installers so the two paths cannot drift on upgrade semantics

API

  • GET /api/sources — list registered sources + the trusted-source globs
  • GET /api/source/{appId}/binding — read the bound source for an app
  • POST /api/source/{appId}/bind — bind a source (rejected if not in allowlist)
  • GET /api/app/{appId}/versions?source=... — optional one-off override
  • POST /api/app/{appId}/versions/{version}/install — accepts optional source body param
  • ApiController now uses constructor injection for IGroupManager / IUserSession / ServerVersion / InstallerService instead of \OC::\$server->get()

Specs

OpenSpec proposal at openspec/changes/add-external-source-installs/:

  • proposal.md, design.md, tasks.md
  • New capability spec specs/external-sources/spec.md
  • Delta specs/version-management/spec.md adding source binding to the existing capability

Tests

  • 39 PHPUnit unit tests covering source binding, allowlist matching (default, custom, malformed), GitHub source HTTP behaviour (success, 404, rate limit, network error, malformed JSON, multi-asset disambiguation), source-id parsing
  • New tests/bootstrap-unit-only.php + tests/phpunit-unit-only.xml so unit tests can run without the full Nextcloud server bootstrap
  • Replaces the stale ApiTest::testIndex that referenced a non-existent method

Browser verification

Manually verified against the running Nextcloud (admin login, app installed via occ app:enable):

Endpoint Expected Actual
GET /api/admin-check isAdmin true ok
GET /api/sources 2 drivers + ConductionNL/* default ok
POST /api/source/randomapp/bind {owner:randomuser} 403 + allowlist message ok
POST /api/source/openregister/bind {owner:ConductionNL} 200, binding written ok
GET /api/app/openregister/versions real GitHub releases (0.2.15-beta.*, etc.) ok
GET /api/app/larpingapp/versions (unbound) App Store versions (fallback) ok
GET /api/app/openregister/versions?source=appstore App Store versions, binding unchanged ok

Test plan

  • Tests pass: vendor/bin/phpunit -c tests/phpunit-unit-only.xml --no-coverage (39 tests, 85 assertions)
  • No regression in the existing App Store install flow (SelectedReleaseInstallerService shares InstallFinalizer with the new path; runLazyRegistration order preserved)
  • Verify in your env that an actual install of an external release works end-to-end (I stopped short of a real install in the dev env to avoid mutating shared state — pre-flight runs cleanly with allowlist + integrity checks reachable)
  • Confirm app_versions.trusted_sources allowlist is the right default for production ([\"ConductionNL/*\"])

Notes / open items for follow-up

  • Auto-pin observed SHA-256 to the binding so a maintainer rewriting a GitHub release cannot ship altered bytes silently. Tracked in design.md (deferred to follow-up)
  • Sigstore / Cosign signature verification — not in this proposal
  • Auto-update from external sources (cron poll for new releases) — future work
  • Pre-existing psalm baseline: 174 errors, mostly IConfig::getAppValue deprecation warnings used throughout the codebase. Not a regression introduced by this PR

Introduces a parallel install flow for non-App-Store sources (initial driver:
public GitHub releases). The signed App Store path is unchanged; the new path
trades the Nextcloud code-signing chain for: a trusted-source allowlist
(default ConductionNL/*), strict appId/version match against the extracted
appinfo/info.xml, and optional SHA-256 verification when the release publishes
a sibling .sha256 asset.

- New SourceInterface + SourceRegistry + AppStoreSource/GithubReleaseSource
  drivers; per-app source binding persisted as JSON in app config
- TrustedSourceList enforces the allowlist with fnmatch-style globs
- ExternalReleaseInstallerService runs the same maintenance-mode/backup-on-
  failure flow as the signed installer; both share post-extract migrations
  via new InstallFinalizer (so signed and external paths cannot drift on
  upgrade semantics)
- ApiController gains GET /api/sources, GET/POST /api/source/{appId}/binding
  and accepts an optional source override on version-list and install; injects
  IGroupManager/IUserSession/ServerVersion via constructor (was \OC::server)
- 39 PHPUnit unit tests covering source binding, allowlist matching, GitHub
  source HTTP behaviour, source-id parsing; new bootstrap-unit-only.php so
  unit tests run without the full Nextcloud server bootstrap

OpenSpec proposal: openspec/changes/add-external-source-installs/

Browser-verified end-to-end against running Nextcloud:
- /api/admin-check → ok
- /api/sources → returns 2 drivers + ConductionNL/* default allowlist
- POST /api/source/randomapp/bind {owner: randomuser} → 403 with allowlist message
- POST /api/source/openregister/bind {owner: ConductionNL} → 200, binding written
- GET /api/app/openregister/versions → returns real GitHub releases
- GET /api/app/larpingapp/versions (unbound) → falls through to App Store
- ?source=appstore override returns App Store versions without changing binding
- Promote `external-sources` to canonical spec at openspec/specs/external-sources/spec.md (status: implemented)
- Merge ADDED/MODIFIED requirements into openspec/specs/version-management/spec.md (status: idea → implemented): adds Source binding, Explicit source override, and rewrites Fetch Available Versions to query the bound source
- Move openspec/changes/add-external-source-installs/ → openspec/changes/archive/2026-05-02-add-external-source-installs/ per OpenSpec convention
@rubenvdlinde
rubenvdlinde merged commit 9a739cd into development May 2, 2026
0 of 7 checks passed
@rubenvdlinde
rubenvdlinde deleted the feature/add-external-source-installs branch May 2, 2026 14:59
rubenvdlinde added a commit that referenced this pull request May 2, 2026
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.
MWest2020 pushed a commit that referenced this pull request Jul 16, 2026
… / GitHub quick buttons' (#25) from feat/source-binding-ui into main

Reviewed-on: https://codeberg.org/Conduction/app-versions/pulls/25
rubenvdlinde pushed a commit that referenced this pull request Jul 23, 2026
… / GitHub quick buttons' (#25) from feat/source-binding-ui into main

Reviewed-on: https://codeberg.org/Conduction/app-versions/pulls/25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant