Skip to content

feat: Add encrypted PAT storage for GitHub private-repo installs - #28

Merged
rubenvdlinde merged 3 commits into
developmentfrom
feature/add-github-pat-management
May 2, 2026
Merged

feat: Add encrypted PAT storage for GitHub private-repo installs#28
rubenvdlinde merged 3 commits into
developmentfrom
feature/add-github-pat-management

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Re-opened against development after admin-merge of #25 deleted the original base branch. Original PR #26 (closed) carries the full review history.

Summary

Adds Personal Access Token (PAT) management so admins can install Nextcloud apps from private GitHub repositories. Per-admin by default, optional share-with-admins, encrypted at rest, validated for least-privilege scope on upload, plaintext never returned via API.

Stacks on top of the now-merged #25 (external-source install path). Final piece in the original 3-PR series alongside #27.

What's in this PR

  • lib/Db/Pat.php entity + PatMapper
  • Migration Version1000Date20260502120000 creates app_versions_pats table (with shortened index names + bool default omitted to satisfy MigrationService validation)
  • PatManager.useToken()ICrypto encrypt-on-create + decrypt-only-inside-callback. Plaintext never appears in API responses (only first-4/last-4 char hint)
  • PatValidator probes GET https://api.github.com/user:
    • Classic PAT (ghp_*): rejects any scope outside repo / public_repo
    • Fine-grained PAT (github_pat_*): accepted with unverifiable_scope warning
    • Captures github-authentication-token-expiration header into expires_at
  • PatResolver — owner-priority + glob matching + expiry skip
  • GithubReleaseSource and ExternalReleaseInstallerService attach Authorization: Bearer <token> when a PAT matches the binding's owner/repo
  • PatDeeplinkBuilder — full prefill for classic; page link + structured instructions for fine-grained
  • UserDeletedListener sweeps PATs owned by deleted users
  • ApiController gains GET/POST/PATCH/DELETE /api/pats and GET /api/pats/deeplink
  • 25 new PHPUnit tests (validator, manager, resolver, deeplink) — 64 total pass
  • OpenSpec archive: canonical openspec/specs/pat-management/spec.md (status: implemented), change folder moved to openspec/changes/archive/2026-05-02-add-github-pat-management/

Browser-verified

Endpoint Result
Migration runs Table created
GET /api/pats empty list initially
GET /api/pats/deeplink?kind=classic URL with scopes=repo and host description
GET /api/pats/deeplink?kind=fine-grained Page link + 5 read-only instructions
POST /api/pats empty fields 400
POST /api/pats invalid token 400 "Token is invalid or revoked"
DELETE /api/pats/9999 404
Proposal 1 path (regression) 100 GitHub releases, no regression

Test plan

  • vendor/bin/phpunit -c tests/phpunit-unit-only.xml --no-coverage (64 tests)
  • Upload real classic PAT with repo scope → accepted
  • Upload classic PAT with broader scopes → rejected
  • Bind a private repo, list versions returns private releases
  • Delete a Nextcloud user → their PATs disappear

Adds Personal Access Token (PAT) management so admins can install Nextcloud
apps from private GitHub repositories. PATs are per-admin by default, can
optionally be shared with other admins, are validated for least-privilege
scope on upload, encrypted at rest via ICrypto, and never returned via the
API in plaintext.

- New table app_versions_pats (migration Version1000Date20260502120000)
- Pat entity + PatMapper with findVisibleTo / findOwnedBy / deleteByOwner
- PatManager.useToken() decrypts only inside a callback; plaintext is
  immediately discarded afterwards. Token never appears in API responses
  (only first 4 + last 4 chars as `tokenHint`)
- PatValidator probes GET https://api.github.com/user:
  - Classic PAT (ghp_*): rejects any scope outside repo / public_repo by
    parsing X-OAuth-Scopes
  - Fine-grained PAT (github_pat_*): GitHub does not expose configured
    permissions; accepted with explicit `unverifiable_scope` warning
  - 401 → "Token is invalid or revoked"; 403 → rate limit message
  - Captures `github-authentication-token-expiration` header into expires_at
- PatResolver picks the highest-priority non-expired PAT visible to the
  current uid that matches the binding's owner/repo (owner-owned > shared,
  then most-specific glob)
- GithubReleaseSource and ExternalReleaseInstallerService now resolve a PAT
  for the bound owner/repo and attach `Authorization: Bearer <token>` to
  GitHub API + asset-download + sha256 fetches when a PAT is present.
  Public-only path (no PAT) unchanged.
- PatDeeplinkBuilder generates prefilled GitHub URLs:
  - Classic: full prefill (scopes=repo, description with NC host)
  - Fine-grained: page link + structured instructions array
- UserDeletedListener removes PATs owned by a deleted Nextcloud user
- ApiController gains GET/POST/PATCH/DELETE /api/pats and GET /api/pats/deeplink
- 25 new PHPUnit tests for PatValidator (scope detection, expiry parsing,
  rejection paths, case-insensitive headers), PatManager (encryption
  roundtrip, hint builder, useToken callback semantics), PatResolver
  (pattern matching, owner priority, expired PAT skip), and
  PatDeeplinkBuilder (classic prefill, fine-grained instructions)

OpenSpec proposal: openspec/changes/add-github-pat-management/

Browser-verified end-to-end:
- Migration runs and creates table (with shortened index names; default-false
  on bool column omitted to satisfy MigrationService validation)
- GET /api/pats returns empty list initially
- GET /api/pats/deeplink?kind=classic returns prefilled URL with scopes=repo
  and a description including the Nextcloud host
- GET /api/pats/deeplink?kind=fine-grained returns the page link plus a
  read-only-permissions instructions array
- POST /api/pats with empty fields → 400 "label, targetPattern and token are required"
- POST /api/pats with invalid token (ghp_definitelyinvalid...) → 400
  "Token is invalid or revoked" (verified the validator hits api.github.com,
  reads 401, and surfaces a useful message rather than the catch-all)
- DELETE /api/pats/{unknown id} → 404
- Proposal 1 path unchanged: openregister GitHub-bound versions list still
  returns 100 releases, no regression
- Promote `pat-management` to canonical spec at openspec/specs/pat-management/spec.md (status: implemented)
- Move openspec/changes/add-github-pat-management/ → openspec/changes/archive/2026-05-02-add-github-pat-management/
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