From 2cbf838158034bbca06a8fe3ced8041467dffd80 Mon Sep 17 00:00:00 2001 From: Duy /zuey/ Date: Thu, 16 Apr 2026 15:09:48 +0700 Subject: [PATCH] feat(packages): GitHub Releases binary installer (#898) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(packages): add GitHub Releases binary installer New runtime source `github:owner/repo[@tag]` for installing Linux CLI binaries from GitHub Releases. Admin-only, SHA256-verified, ELF-validated. Backend: - GitHub API client with 10-min cache + rate-limit mapping - SSRF-guarded streaming downloader (HTTPS + host allowlist, re-validated on every redirect hop, literal-IP rejection) - Checksums.txt / SHA256SUMS lookup with constant-time verify - Archive extract (tar.gz / zip / raw) with path-traversal + zip-bomb guards, symlink skip - ELF magic + 64-bit class + runtime-arch validation - Atomic manifest persistence (temp + rename) HTTP: - POST /v1/packages/install accepts github: spec - GET /v1/packages/github-releases for picker UI (viewer+, arch-filtered) - Extended InstalledPackages response with github field - github-bin runtime probe Infra: - Dockerfile creates /app/data/.runtime/bin (goclaw:goclaw 0755) - docker-entrypoint.sh prepends bin dir to PATH - Env-only config (never config.json): token, max size, org allowlist, bin dir, manifest path UI: - GitHub Binaries section + release picker modal - Dismissable musl/glibc compatibility warning (localStorage) - i18n keys across en/vi/zh Docs: docs/packages-github.md user guide + 14-skills-runtime.md cross-ref. Closes #741 * refactor(packages): revert validPkgName broadening + drop unused sentinel Code review cleanup: - validPkgName regex had `:` added defensively, but github: specs are validated separately via skills.ParseGitHubSpec before reaching this check — the broadening was dead attack surface. - Drop unused ErrUnknownArchive sentinel + the `_ = ErrUnknownArchive` stub in extractRaw. * feat(packages): per-user rate limit on /v1/packages/github-releases Cap picker endpoint at 30 req/min/user (burst 10) to protect the shared GitHub API quota. Key is userID (header X-GoClaw-User-Id) or remote IP for anonymous callers. Returns 429 + Retry-After: 60 when tripped. Standalone token-bucket limiter (stale-entry cleanup every 5 min) lives in internal/http rather than importing internal/gateway, which would create a package cycle. * fix(ui): guard split()[0] for noUncheckedIndexedAccess strict TS CI pnpm build failed on TS2345: `.split('@')[0]` returns `string | undefined` under strict index access. Default to empty string to satisfy the type checker; runtime behaviour unchanged because the downstream regex rejects empty strings. * fix(packages): address Claude review — medium + low findings Medium - rate limiter: atomic.Int64 lastSeen + amortized sweep replaces goroutine-based cleanup → fixes data race on lastSeen and the goroutine leak when tests swap the package-level limiter. - checksum pipeline: slog.Warn on ReadFile and ParseChecksums failures (previously silent). "asset not listed" stays warn+proceed but is now documented as the publisher's choice — ELF validation remains the final gate. - downloader: drop http.Client.Timeout (30s capped the whole request including body read, aborting large downloads on slow links). Context deadline from install timeout (5 min) is the correct bound. Low / style / UI - extractRaw honors maxUncompressed (ErrFileTooLarge on overflow) so the helper is safe outside the hot path. - cmd/gateway_github_installer.go: drop the explicit cfg.Defaults() call — NewGitHubInstaller already invokes it. - GitHubPackageEntry: remove unpopulated InstalledBy field + document why. - owner regex tightened to 39-char GitHub limit (was 40). - mu lock comment corrected: serializes only the disk-write phase. - UI: shared stripPrefixAndTag helper + owner regex mirrors the backend 39-char cap; destructure-with-default kills the split()[0] ?? "" awkwardness while still satisfying noUncheckedIndexedAccess. Verified: go build (pg + sqliteonly) · go vet · go test -race ./internal/skills ./internal/http · pnpm build. * fix(packages): address Claude review round 2 Medium - validRepoPath now rejects trailing hyphens in the owner segment and caps at 39 chars, matching gitHubSpecRE exactly. Previously a subtle drift between the two validators could let `foo-/repo` slip to the GitHub API and surface as a 502 instead of a clean 400. - handleGitHubReleases no longer forwards raw err.Error() from the upstream call. Maps sentinel errors: ErrGitHubRateLimited → 429 + Retry-After ErrGitHubNotFound → 404 ErrGitHubUnauthorized → 502 "github authentication failed" default → 502 "failed to fetch releases" Avoids leaking rate-limit reset timestamps / server internals to viewer-tier callers. Low / UX - Install response now returns the manifest entry for github: specs (new lookupGitHubEntry helper; nil-safe fallback to {ok:true}). Lets the UI display "installed: lazygit v0.42.0" without a list refresh. - gitHubSpecRE tag segment capped at 1..255 chars (git ref-name bound). UI isValidFullSpec mirrors the same cap. * fix(packages): address Claude review round 3 Medium - github_api: URL-encode owner, repo, and tag via url.PathEscape when building API paths. Previously a tag containing '#' would be stripped as a URL fragment and '?' would inject a query parameter, silently hitting the wrong release. Low / polish - Uninstall via full "github:owner/repo[@tag]" spec now falls back to manifest lookup by owner/repo, handling packages whose binary name differs from the repo name (cli/cli → gh). - GitHubClient.cache sweeps expired entries opportunistically when the map grows past 256 entries (prevents theoretical unbounded growth over long uptime). - handleInstall for github: specs now calls GitHubInstaller.Install directly and returns the freshly-created manifest entry, eliminating the double manifest read via List() from the lookupGitHubEntry helper. - pickBinaries comment corrected — actual behavior excludes paths matched by nonBinaryPathRE rather than enforcing a single-depth limit. * fix(packages): address Claude review round 4 (style + ordering) All 4 findings are Low severity: - github_api.go: replace interface{} with any across the cache type, cacheGet return, cacheSet param, and doJSON out param. - doJSON: rename local `url` to `apiURL` to avoid shadowing the "net/url" package import used by GetRelease/ListReleases. - Uninstall: save the updated manifest BEFORE removing binaries on disk. If saveManifest fails we now bail out without leaving a manifest entry that still claims binaries which have been deleted (a retried Uninstall would otherwise hit ErrPackageNotInstalled after the first attempt wiped the files). Disk removal stays best-effort and warn-on-error, which matches the idempotent intent. - pickBinaries: inline comment corrected to reflect actual behavior — depth is not enforced; nonBinaryPathRE filter + downstream ELF validation are the real gates. * fix(packages): address Claude review round 5 All 3 findings are Low severity: - handleInstall github fast-path now wraps the context with skills.InstallTimeout (5 min) before calling gh.Install and emits the same "skills: installing dep" / "dep installed" / "github install failed" log lines as the generic InstallSingleDep path, so operator-observability is identical between github: and pip:/npm: install flows. - installTimeout promoted to exported InstallTimeout so the http layer shares the single source of truth rather than duplicating the 5-minute constant. - cacheMaxEntries comment clarifies it is a soft sweep trigger, not a hard cap — when every entry is still within TTL the map can briefly exceed the threshold by one insert. * fix(packages): address Claude review round 6 (final Lows) Both findings are Low severity (reviewer marked the PR "ready to merge" already): - github_installer: "no checksum asset available" downgraded from slog.Warn to slog.Info. Many popular upstream releases (jq, fzf, older ripgrep, etc.) ship no checksum file at all — that is publisher policy, not a problem with the install. The suspicious cases (checksum file unreadable, unparseable, or missing this asset) stay at Warn so they stand out. - handleGitHubReleases response now uses a narrow assetPreview DTO (name + size_bytes) instead of embedding the full GitHubAsset type which also carried browser_download_url. The picker UI never rendered the URL; trimming the response keeps the viewer-tier surface minimal. UI AssetPreview interface realigned to match. * fix(packages): address Claude review round 7 Narrow the GET /v1/packages GitHub entry to a viewer-safe projection (repo/tag/binaries/name/installed_at), mirroring the assetPreview fix from round 6. Strips asset_url, sha256, and asset_name from the list response — viewer-level callers no longer see CDN download URLs or checksum metadata for installed packages. UI types realigned; the removed fields were never rendered. Finding #2 (install writes binary before manifest save) left as noted — reviewer confirmed informational only, self-heals on retry, no security impact since binaries pass ELF validation before being written. * fix(packages): address Claude review round 8 Map HTTP 429 (GitHub secondary rate limits — abuse detection, unauthenticated bursts, search) to ErrGitHubRateLimited in the API client so the picker endpoint renders 429 "rate limit reached" with Retry-After: 60 instead of falling through to 502 "failed to fetch releases". Primary rate limits (403 + X-RateLimit-Remaining: 0) were already handled; this covers the secondary class documented at https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api#about-secondary-rate-limits * fix(packages): address Claude review round 9 Two defensive hardenings flagged as Very Low: - ParseChecksums: strip leading `./` from checksum filenames. `sha256sum ./file` emits `./file` in the name column; the caller looks up by bare asset basename so `./`-prefixed entries would silently miss. Real release checksums almost never use this form, but the guard is essentially free. - doJSON: cap response body at 8 MiB via io.LimitReader before JSON decode. Current GitHub list/release payloads are well under this (~1 MiB at per_page=100). Guards against future call sites or a misbehaving upstream returning an oversized document. * fix(cron): eliminate cross-test race on runLoopTickInterval `Service.Stop()` closes stopChan but does not wait for the runLoop goroutine to exit. In the test suite, test A's `defer cs.Stop()` can return before the spawned runLoop has reached `ticker := time.NewTicker(runLoopTickInterval)`. If test B then calls `setFastTick()` to mutate the package-level var, the race detector correctly flags it: Read at runLoopTickInterval by goroutine A (runLoop ticker init) Previous write by goroutine B (setFastTick in test B) Fix: snapshot `runLoopTickInterval` inside `Start()` under the mutex before spawning the goroutine, and pass the value as a parameter to `runLoop`. The spawned goroutine no longer reads the package-level var, so the cross-test window is closed. Production behavior unchanged. Verified: `go test -race -count=3 ./internal/cron/...` passes three times in a row; the CI failure on PR #898 reproduced before the fix and is gone after. * fix(packages): address review P0/P1/P2 + new DoS vector P0.1 — UI uninstall 400: parseAndValidatePackage now accepts github: (manifest Name form, no owner/repo) in addition to the full spec. UI sends github:${pkg.name} from the manifest; dispatcher already tolerated bare names — the HTTP validator was the only gate rejecting them. Install path re-validates strictly via ParseGitHubSpec and bare-name install returns 400 now (was 500). P1.1 — ExtractArchive raw-ELF fallback name: add ExtractArchiveAs( path, fallbackName, max). Installer passes parsed.Repo so raw (non-archive) ELF assets no longer end up recorded as /tmp/goclaw-gh-asset-XXXX.bin — that basename would leak into the manifest Binaries entry and break PATH lookup. P1.3 — Archive entry count cap: maxArchiveEntries = 10_000 + ErrTooManyEntries sentinel. Tar: count ALL headers seen (incl. symlinks/dirs we skip) to block the gzip-bomb-of-headers DoS — header bytes don't count against maxUncompressed for zero-size entries. Zip: pre-check via peekZipEntryCount reads the EOCD record manually and rejects oversized archives BEFORE zip.OpenReader allocates []*zip.File of declared capacity (this was a fresh red-team finding; stdlib would otherwise alloc ~1GB for a crafted 200MB zip claiming 4M entries). P1.4 — Rate-limit install/uninstall: packagesWriteLimiter (10/min/user, burst 3). Admin-only mitigates but a compromised token could otherwise flood upstream (GitHub/pip/npm) or spam manifest mutations. P1.6 — Non-Linux early reject: ErrUnsupportedOS guard at the top of Install(). Windows/macOS hosts no longer waste bandwidth fetching a Linux asset just to fail at the ELF machine check. P1.7 — Manifest fsync: OpenFile → Write → Sync → Close → Rename → dir Sync, with tmp cleanup on every error path. POSIX doesn't guarantee durability via rename alone; XFS / ext4 with async journal can reorder. P2.1 — Belt-and-suspenders zip runtime break when cumulative bytes reach the cap (pre-declared check already covers it but the streaming loop now bails immediately). P2.6 — Binary-name collision warn: slog.Warn when a different repo already owns the basename we're about to overwrite. Last-writer- wins unchanged; operator now gets a signal instead of silence. Hardening — rate-limit key: rateLimitKeyFromRequest prefers store.UserIDFromContext over the raw X-GoClaw-User-Id header so an admin can't rotate the header mid-session to dodge the bucket. Header/IP fallback retained for pre-auth / test callers. Tests: 9 new cases on parseAndValidatePackage (github full/bare/ empty/traversal/injection/space/leading-hyphen); TestExtractArchiveAs_RawELFUsesFallbackName; TestExtractTarGz_EntryCountCap + TestExtractZip_EntryCountCap; TestPeekZipEntryCount (DoS pre-check path). Verified: go build ./... && go build -tags sqliteonly ./... && go vet ./... && go test -race ./internal/skills/... ./internal/http/... --------- Co-authored-by: viettranx --- Dockerfile | 6 +- cmd/gateway_github_installer.go | 63 ++ cmd/gateway_http_wiring.go | 3 +- docker-entrypoint.sh | 6 +- docs/14-skills-runtime.md | 14 + docs/packages-github.md | 145 +++++ internal/cron/service.go | 7 +- internal/cron/service_execution.go | 11 +- internal/http/packages.go | 175 +++++ internal/http/packages_rate_limiter.go | 154 +++++ internal/http/packages_rate_limiter_test.go | 76 +++ internal/http/packages_test.go | 59 +- internal/skills/archive_extract.go | 362 +++++++++++ internal/skills/archive_extract_test.go | 372 +++++++++++ internal/skills/dep_installer.go | 56 +- internal/skills/github_api.go | 215 ++++++ internal/skills/github_api_test.go | 81 +++ internal/skills/github_checksum.go | 95 +++ internal/skills/github_checksum_test.go | 76 +++ internal/skills/github_default_installer.go | 27 + internal/skills/github_download.go | 125 ++++ internal/skills/github_download_test.go | 70 ++ internal/skills/github_installer.go | 610 ++++++++++++++++++ internal/skills/github_installer_test.go | 147 +++++ internal/skills/package_lister.go | 34 +- internal/skills/runtime_check.go | 12 + ui/web/src/i18n/locales/en/packages.json | 19 + ui/web/src/i18n/locales/vi/packages.json | 19 + ui/web/src/i18n/locales/zh/packages.json | 19 + .../packages/github-binaries-section.tsx | 326 ++++++++++ .../src/pages/packages/hooks/use-packages.ts | 12 + ui/web/src/pages/packages/packages-page.tsx | 7 + 32 files changed, 3384 insertions(+), 19 deletions(-) create mode 100644 cmd/gateway_github_installer.go create mode 100644 docs/packages-github.md create mode 100644 internal/http/packages_rate_limiter.go create mode 100644 internal/http/packages_rate_limiter_test.go create mode 100644 internal/skills/archive_extract.go create mode 100644 internal/skills/archive_extract_test.go create mode 100644 internal/skills/github_api.go create mode 100644 internal/skills/github_api_test.go create mode 100644 internal/skills/github_checksum.go create mode 100644 internal/skills/github_checksum_test.go create mode 100644 internal/skills/github_default_installer.go create mode 100644 internal/skills/github_download.go create mode 100644 internal/skills/github_download_test.go create mode 100644 internal/skills/github_installer.go create mode 100644 internal/skills/github_installer_test.go create mode 100644 ui/web/src/pages/packages/github-binaries-section.tsx diff --git a/Dockerfile b/Dockerfile index b7ab822c..c768a836 100644 --- a/Dockerfile +++ b/Dockerfile @@ -144,7 +144,8 @@ RUN chmod +x /app/docker-entrypoint.sh && \ # while pip/npm subdirs are goclaw-owned (runtime installs by the app process). # Symlink .claude → data volume so Claude CLI credentials persist across container recreates. RUN mkdir -p /app/workspace /app/data/.runtime/pip /app/data/.runtime/npm-global/lib \ - /app/data/.runtime/pip-cache /app/data/.claude /app/skills /app/tsnet-state /app/.goclaw \ + /app/data/.runtime/pip-cache /app/data/.runtime/bin /app/data/.claude /app/skills \ + /app/tsnet-state /app/.goclaw \ && ln -s /app/data/.claude /app/.claude \ && touch /app/data/.runtime/apk-packages \ && chown -R goclaw:goclaw /app/workspace /app/skills /app/tsnet-state /app/.goclaw \ @@ -152,7 +153,8 @@ RUN mkdir -p /app/workspace /app/data/.runtime/pip /app/data/.runtime/npm-global && chown root:goclaw /app/data/.runtime /app/data/.runtime/apk-packages \ && chmod 0750 /app/data/.runtime \ && chmod 0640 /app/data/.runtime/apk-packages \ - && chown -R goclaw:goclaw /app/data/.runtime/pip /app/data/.runtime/npm-global /app/data/.runtime/pip-cache /app/data/.claude + && chown -R goclaw:goclaw /app/data/.runtime/pip /app/data/.runtime/npm-global /app/data/.runtime/pip-cache /app/data/.runtime/bin /app/data/.claude \ + && chmod 0755 /app/data/.runtime/bin # Default environment ENV GOCLAW_CONFIG=/app/config.json \ diff --git a/cmd/gateway_github_installer.go b/cmd/gateway_github_installer.go new file mode 100644 index 00000000..3f91e4fb --- /dev/null +++ b/cmd/gateway_github_installer.go @@ -0,0 +1,63 @@ +package cmd + +import ( + "log/slog" + "os" + "path/filepath" + "strconv" + "strings" + + "github.com/nextlevelbuilder/goclaw/internal/skills" +) + +// initGitHubInstaller constructs the process-wide GitHub Releases installer +// from environment variables and registers it via skills.SetDefaultGitHubInstaller. +// +// Config comes ONLY from env vars (token is a secret — never from config.json): +// +// GOCLAW_PACKAGES_GITHUB_TOKEN optional PAT (boosts rate limit, enables private repos) +// GOCLAW_PACKAGES_MAX_ASSET_SIZE_MB default 200 +// GOCLAW_PACKAGES_GITHUB_ALLOWED_ORGS comma-separated allowlist (empty = all allowed) +// GOCLAW_PACKAGES_GITHUB_BIN_DIR default /app/data/.runtime/bin +// GOCLAW_PACKAGES_GITHUB_MANIFEST default {BIN_DIR}/../github-packages.json +func initGitHubInstaller() { + cfg := &skills.GitHubPackagesConfig{ + Token: os.Getenv("GOCLAW_PACKAGES_GITHUB_TOKEN"), + BinDir: os.Getenv("GOCLAW_PACKAGES_GITHUB_BIN_DIR"), + ManifestPath: os.Getenv("GOCLAW_PACKAGES_GITHUB_MANIFEST"), + } + if v := os.Getenv("GOCLAW_PACKAGES_MAX_ASSET_SIZE_MB"); v != "" { + if n, err := strconv.Atoi(v); err == nil && n > 0 { + cfg.MaxAssetSizeMB = n + } + } + if v := os.Getenv("GOCLAW_PACKAGES_GITHUB_ALLOWED_ORGS"); v != "" { + for _, o := range strings.Split(v, ",") { + if o = strings.TrimSpace(o); o != "" { + cfg.AllowedOrgs = append(cfg.AllowedOrgs, o) + } + } + } + + // NewGitHubInstaller calls cfg.Defaults() — no need to invoke it here. + client := skills.NewGitHubClient(cfg.Token) + installer := skills.NewGitHubInstaller(client, cfg) + skills.SetDefaultGitHubInstaller(installer) + + // Best-effort ensure bin dir + manifest dir exist (entrypoint may run as root + // while Go process runs as goclaw — respect pre-existing permissions). + if err := os.MkdirAll(cfg.BinDir, 0o755); err != nil { + slog.Warn("github.installer: mkdir bin dir failed", "path", cfg.BinDir, "error", err) + } + if err := os.MkdirAll(filepath.Dir(cfg.ManifestPath), 0o755); err != nil { + slog.Warn("github.installer: mkdir manifest dir failed", "path", cfg.ManifestPath, "error", err) + } + + slog.Info("packages: github installer enabled", + "bin_dir", cfg.BinDir, + "manifest", cfg.ManifestPath, + "allowed_orgs", cfg.AllowedOrgs, + "max_asset_mb", cfg.MaxAssetSizeMB, + "token_set", cfg.Token != "", + ) +} diff --git a/cmd/gateway_http_wiring.go b/cmd/gateway_http_wiring.go index cc07eb1a..b497cf1d 100644 --- a/cmd/gateway_http_wiring.go +++ b/cmd/gateway_http_wiring.go @@ -133,7 +133,8 @@ func (d *gatewayDeps) wireHTTPHandlersOnServer( d.server.SetUsageHandler(httpapi.NewUsageHandler(d.pgStores.Snapshots, d.pgStores.DB)) } - // Runtime package management (install/uninstall system/pip/npm packages) + // Runtime package management (install/uninstall system/pip/npm/github packages) + initGitHubInstaller() d.server.SetPackagesHandler(httpapi.NewPackagesHandler()) // API documentation (OpenAPI spec + Swagger UI at /docs) diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index 1eaa8896..3b7e36c1 100644 --- a/docker-entrypoint.sh +++ b/docker-entrypoint.sh @@ -7,7 +7,7 @@ RUNTIME_DIR="/app/data/.runtime" # Non-fatal: on first start with a fresh volume the directory may not be # writable yet (volume initialisation race on some Docker runtimes). # The app starts fine without .runtime; package installs will fail gracefully. -mkdir -p "$RUNTIME_DIR/pip" "$RUNTIME_DIR/npm-global/lib" "$RUNTIME_DIR/pip-cache" || true +mkdir -p "$RUNTIME_DIR/pip" "$RUNTIME_DIR/npm-global/lib" "$RUNTIME_DIR/pip-cache" "$RUNTIME_DIR/bin" || true # Fix .runtime ownership for split root/goclaw access. # .runtime itself must be root-owned so pkg-helper (root) can write apk-packages. @@ -16,7 +16,7 @@ mkdir -p "$RUNTIME_DIR/pip" "$RUNTIME_DIR/npm-global/lib" "$RUNTIME_DIR/pip-cach if [ "$(id -u)" = "0" ] && [ -d "$RUNTIME_DIR" ]; then chown root:goclaw "$RUNTIME_DIR" 2>/dev/null || true chmod 0750 "$RUNTIME_DIR" 2>/dev/null || true - chown -R goclaw:goclaw "$RUNTIME_DIR/pip" "$RUNTIME_DIR/npm-global" "$RUNTIME_DIR/pip-cache" 2>/dev/null || true + chown -R goclaw:goclaw "$RUNTIME_DIR/pip" "$RUNTIME_DIR/npm-global" "$RUNTIME_DIR/pip-cache" "$RUNTIME_DIR/bin" 2>/dev/null || true fi # Fix workspace directory ownership: handle dirs created by root in previous @@ -38,7 +38,7 @@ export PIP_CACHE_DIR="$RUNTIME_DIR/pip-cache" # NODE_PATH includes both pre-installed system globals and runtime-installed globals. export NPM_CONFIG_PREFIX="$RUNTIME_DIR/npm-global" export NODE_PATH="/usr/local/lib/node_modules:$RUNTIME_DIR/npm-global/lib/node_modules:${NODE_PATH:-}" -export PATH="$RUNTIME_DIR/npm-global/bin:$RUNTIME_DIR/pip/bin:$PATH" +export PATH="$RUNTIME_DIR/bin:$RUNTIME_DIR/npm-global/bin:$RUNTIME_DIR/pip/bin:$PATH" # System packages: re-install on-demand packages persisted across recreates. # After chown above, root owns .runtime and can create this file. diff --git a/docs/14-skills-runtime.md b/docs/14-skills-runtime.md index eadf8682..e142833f 100644 --- a/docs/14-skills-runtime.md +++ b/docs/14-skills-runtime.md @@ -189,6 +189,20 @@ To add a new package to the Docker image: For packages only needed by specific skills, prefer runtime installation (Option B) to keep the image lean. +### GitHub Releases Installer + +For CLI tools distributed as GitHub Releases (lazygit, starship, ripgrep, gh, etc.) +that aren't packaged via apk/pip/npm, use the `github:` runtime installer: + +``` +github:owner/repo[@tag] +``` + +Admin-only, SHA256-verified, ELF-validated, with a release-picker UI. Binaries +land in `/app/data/.runtime/bin/` (on `$PATH`). See +[`docs/packages-github.md`](./packages-github.md) for syntax, configuration, +security posture, and troubleshooting (especially musl/glibc compatibility). + --- ## 8. Skill Search (v3) diff --git a/docs/packages-github.md b/docs/packages-github.md new file mode 100644 index 00000000..229a7832 --- /dev/null +++ b/docs/packages-github.md @@ -0,0 +1,145 @@ +# GitHub Binary Installer + +Install CLI tools directly from GitHub Releases at runtime. Covers Go, Rust, +shell, and other binary-distributed tools not available via `apk` / `pip` / `npm`. + +Closes [#741](https://github.com/nextlevelbuilder/goclaw/issues/741). + +## Install Syntax + +``` +github:owner/repo[@tag] +``` + +Examples: +- `github:cli/cli` → latest release +- `github:jesseduffield/lazygit@v0.42.0` → specific version +- `github:sharkdp/fd@v9.0.0` → specific version with dot separator + +## How It Works + +1. Fetches release metadata from the GitHub API +2. Auto-selects asset matching `linux` + current arch (amd64 / arm64) +3. Streams download to a temp file, enforcing a max size cap +4. Verifies SHA256 if the publisher ships `checksums.txt` / `SHA256SUMS` +5. Validates ELF magic bytes + 64-bit class + machine matches runtime arch +6. Extracts archive safely (tar.gz / zip / raw binary) with path-traversal + zip-bomb guards +7. Installs to `/app/data/.runtime/bin/` (prepended to `$PATH`) +8. Persists a manifest for later listing + uninstall + +## Usage + +### Web UI + +1. Admin Settings → Packages page +2. Scroll to **GitHub Binaries** section +3. Enter `owner/repo[@tag]` or click **Browse releases** to pick a version +4. Click **Install** + +### HTTP API + +```bash +# Install +curl -X POST http://gateway/v1/packages/install \ + -H "Authorization: Bearer $ADMIN_TOKEN" \ + -H "Content-Type: application/json" \ + -d '{"package": "github:jesseduffield/lazygit@v0.42.0"}' + +# List installed (includes pip/npm/system + github) +curl http://gateway/v1/packages -H "Authorization: Bearer $ADMIN_TOKEN" + +# Browse releases (picker UI uses this) +curl 'http://gateway/v1/packages/github-releases?repo=cli/cli&limit=10' \ + -H "Authorization: Bearer $VIEWER_TOKEN" + +# Uninstall +curl -X POST http://gateway/v1/packages/uninstall \ + -H "Authorization: Bearer $ADMIN_TOKEN" \ + -H "Content-Type: application/json" \ + -d '{"package": "github:lazygit"}' +``` + +## Admin Configuration + +All configuration is driven by environment variables — **never** place the +token in `config.json`. + +| Env var | Default | Notes | +|---------|---------|-------| +| `GOCLAW_PACKAGES_GITHUB_TOKEN` | `""` | Optional PAT: rate 60/hr → 5000/hr + private repo access | +| `GOCLAW_PACKAGES_MAX_ASSET_SIZE_MB` | `200` | Applies to both download cap and 2× uncompressed cap | +| `GOCLAW_PACKAGES_GITHUB_ALLOWED_ORGS` | `""` | Comma-separated allowlist (empty = all orgs allowed) | +| `GOCLAW_PACKAGES_GITHUB_BIN_DIR` | `/app/data/.runtime/bin` | Where extracted binaries land | +| `GOCLAW_PACKAGES_GITHUB_MANIFEST` | `{bin_dir}/../github-packages.json` | Manifest path | + +Token scopes: +- public-only repos: no scopes required +- private repos: `repo` +- org-SSO-enforced repos: must be SSO-authorized PAT + +## Security + +- HTTPS-only downloads with SSRF host allowlist: + `github.com`, `api.github.com`, `objects.githubusercontent.com`, + `release-assets.githubusercontent.com`, `codeload.github.com` +- Every redirect hop re-validated (blocks redirect-based host escape) +- Literal IP hostnames (v4 / v6) always rejected (blocks cloud-metadata access) +- SHA256 verification when publisher ships `checksums.txt` / `SHA256SUMS` (constant-time compare) +- ELF magic + 64-bit class + machine-arch validation before `chmod +x` +- Path-traversal prevention in archive extraction (rejects `..`, absolute, Windows drive, null byte) +- Zip-bomb guard (cumulative uncompressed bytes capped at 2× max asset size) +- Symlink / hardlink entries skipped, never written +- Admin-only API + master-scope guard on install/uninstall +- Picker endpoint `/v1/packages/github-releases` throttled per user (30 req/min, burst 10) to protect GitHub API quota; anonymous fallback keyed by remote IP. Response is `429 Too Many Requests` with `Retry-After: 60` when tripped. +- Token never logged (startup log prints `token_set=bool`) + +## Troubleshooting + +### "glibc not found" / segfault on execution + +GoClaw runs on Alpine Linux (musl libc). Many Go/Rust binaries target glibc. + +**Fix:** pick a musl-compatible release asset. Look for names containing: +- `*-musl.tar.gz` (explicit musl) +- `*-linux-static*` (fully static) +- Go binaries with `CGO_ENABLED=0` typically work out of the box + +Known-good musl releases: +- `ripgrep`: `ripgrep-*-x86_64-unknown-linux-musl.tar.gz` +- `starship`: `starship-x86_64-unknown-linux-musl.tar.gz` +- `gh`: `gh_*_linux_amd64.tar.gz` (static) + +### "no matching asset found" + +Asset naming doesn't fit the heuristic. Open the release page and confirm assets +exist for `linux` + your arch. Workaround: file an upstream issue asking for +standard `linux_amd64` / `linux_arm64` naming. + +### "arch mismatch" + +Binary is `amd64` but runtime is `arm64` (or vice versa). Pick a release asset +matching the host arch — the release picker UI filters automatically. + +### "rate limit exceeded" + +Anonymous GitHub API is capped at 60 req/hr. Set +`GOCLAW_PACKAGES_GITHUB_TOKEN` to bump to 5000/hr. + +### "checksum mismatch" + +Hard-fail. Indicates tampered download or publisher re-signing without updating +the release. Do not force-install; report upstream. + +## Limitations (Phase 1) + +- Linux-only (Lite/Desktop editions not yet supported) +- Docker edition only (runtime dir `/app/data/.runtime/bin`) +- Installs all top-level executables in an archive (no interactive picker if + archive contains multiple binaries) +- No version history / rollback — re-installing replaces in place +- Global manifest (not per-tenant) + +## See Also + +- [`docs/14-skills-runtime.md`](./14-skills-runtime.md) — Overview of the runtime packages system +- Issue [#741](https://github.com/nextlevelbuilder/goclaw/issues/741) — Original feature request diff --git a/internal/cron/service.go b/internal/cron/service.go index c872d125..b12bc046 100644 --- a/internal/cron/service.go +++ b/internal/cron/service.go @@ -84,7 +84,12 @@ func (cs *Service) Start() error { cs.stopChan = make(chan struct{}) cs.running = true - go cs.runLoop(cs.stopChan) + // Snapshot the tick interval before spawning so the goroutine doesn't + // race with tests that mutate the package-level `runLoopTickInterval` + // after a previous Stop() returned but the runLoop goroutine hasn't yet + // executed its ticker construction. + tick := runLoopTickInterval + go cs.runLoop(cs.stopChan, tick) slog.Info("cron service started", "jobs", len(cs.store.Jobs)) return nil diff --git a/internal/cron/service_execution.go b/internal/cron/service_execution.go index 78ca4e81..fd8a7a5d 100644 --- a/internal/cron/service_execution.go +++ b/internal/cron/service_execution.go @@ -139,10 +139,17 @@ func (cs *Service) recordRunLocked(jobID string, err error, resultText string) { // runLoopTickInterval is the cron run loop tick rate. Production default = 1s. // Tests override this via the setFastTick(t) helper to avoid waiting >1s per // scheduled-job test. Production behavior is unchanged. +// +// The value is read synchronously inside Start() before the runLoop goroutine +// is spawned — runLoop itself takes `tick` as a parameter so it never reads +// this package-level var. This avoids a cross-test race where test A's Stop() +// returns before its runLoop has executed the ticker-construction line, and +// test B subsequently calls setFastTick(), mutating the var while test A's +// goroutine is still racing to read it. var runLoopTickInterval = 1 * time.Second -func (cs *Service) runLoop(stopChan chan struct{}) { - ticker := time.NewTicker(runLoopTickInterval) +func (cs *Service) runLoop(stopChan chan struct{}, tick time.Duration) { + ticker := time.NewTicker(tick) defer ticker.Stop() for { diff --git a/internal/http/packages.go b/internal/http/packages.go index bf2a4451..97ffb2de 100644 --- a/internal/http/packages.go +++ b/internal/http/packages.go @@ -1,8 +1,14 @@ package http import ( + "context" + "errors" + "log/slog" "net/http" "regexp" + "runtime" + "strconv" + "strings" "github.com/nextlevelbuilder/goclaw/internal/permissions" "github.com/nextlevelbuilder/goclaw/internal/skills" @@ -10,9 +16,20 @@ import ( ) // validPkgName allows alphanumeric, hyphens, underscores, dots, @, / (for scoped npm). +// `github:` specs are validated separately (via skills.ParseGitHubSpec) and bypass this regex. // Rejects names starting with - to prevent argument injection. var validPkgName = regexp.MustCompile(`^[a-zA-Z0-9@][a-zA-Z0-9._+\-/@]*$`) +// validGitHubBareName matches bare manifest names used on the uninstall path +// (e.g. "gh", "lazygit", "ripgrep-13"). Must not contain `/` or `@` — those +// forms are handled by the full-spec branch via skills.ParseGitHubSpec. +var validGitHubBareName = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]*$`) + +// validRepoPath matches "owner/repo" used by the releases endpoint. +// Owner rules mirror skills.gitHubSpecRE — GitHub caps usernames/orgs at 39 chars, +// no leading/trailing hyphen. +var validRepoPath = regexp.MustCompile(`^([A-Za-z0-9](?:[A-Za-z0-9-]{0,37})?[A-Za-z0-9]|[A-Za-z0-9])/[A-Za-z0-9][A-Za-z0-9._-]*$`) + // PackagesHandler handles runtime package management HTTP endpoints. type PackagesHandler struct{} @@ -27,6 +44,7 @@ func (h *PackagesHandler) RegisterRoutes(mux *http.ServeMux) { mux.HandleFunc("POST /v1/packages/install", h.adminAuth(h.handleInstall)) mux.HandleFunc("POST /v1/packages/uninstall", h.adminAuth(h.handleUninstall)) mux.HandleFunc("GET /v1/packages/runtimes", h.readAuth(h.handleRuntimes)) + mux.HandleFunc("GET /v1/packages/github-releases", h.readAuth(h.handleGitHubReleases)) mux.HandleFunc("GET /v1/shell-deny-groups", h.readAuth(h.handleDenyGroups)) } @@ -63,6 +81,25 @@ func parseAndValidatePackage(w http.ResponseWriter, r *http.Request) string { return "" } + // github: packages carry the scheme prefix through the whole pipeline. + // Accept two forms: + // 1. Full spec "github:owner/repo[@tag]" — install + uninstall path. + // 2. Bare manifest name "github:" — uninstall path only (the UI + // table surfaces canonical Name which may differ from repo — e.g. + // cli/cli → gh). Install will re-validate via ParseGitHubSpec and + // return a clear ErrInvalidGitHubSpec for bare-name form. + if strings.HasPrefix(body.Package, "github:") { + if _, err := skills.ParseGitHubSpec(body.Package); err == nil { + return body.Package + } + bare := strings.TrimPrefix(body.Package, "github:") + if validGitHubBareName.MatchString(bare) { + return body.Package + } + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid github spec"}) + return "" + } + // Strip prefix for validation, then validate the bare package name. name := body.Package for _, prefix := range []string{"pip:", "npm:"} { @@ -90,10 +127,56 @@ func (h *PackagesHandler) handleInstall(w http.ResponseWriter, r *http.Request) if !requireMasterScope(w, r) { return } + if !enforcePackagesWriteLimit(w, r, "/v1/packages/install") { + return + } pkg := parseAndValidatePackage(w, r) if pkg == "" { return } + + // Fast path for github: specs — call the installer directly so we can + // return the freshly-created manifest entry without a second disk read + // via List(). Other prefixes fall through to the generic dispatcher. + // Uses the same InstallTimeout + top-level log line as InstallSingleDep + // for operator-observability parity across install paths. + if strings.HasPrefix(pkg, "github:") { + gh := skills.DefaultGitHubInstaller() + if gh == nil { + writeJSON(w, http.StatusServiceUnavailable, map[string]any{ + "ok": false, "error": "github installer not configured", + }) + return + } + slog.Info("skills: installing dep", "dep", pkg) + ctx, cancel := context.WithTimeout(r.Context(), skills.InstallTimeout) + defer cancel() + entry, err := gh.Install(ctx, pkg) + if err != nil { + slog.Error("skills: github install failed", "dep", pkg, "error", err) + // Classify client-side errors as 400 so the UI can show a clear + // validation message instead of a generic 500. The bare-name + // form (github:) is accepted by parseAndValidatePackage to + // keep the uninstall path alive; Install re-validates strictly + // via ParseGitHubSpec and this branch surfaces that to the user. + status := http.StatusInternalServerError + switch { + case errors.Is(err, skills.ErrInvalidGitHubSpec), + errors.Is(err, skills.ErrGitHubOrgNotAllowed), + errors.Is(err, skills.ErrUnsupportedOS), + errors.Is(err, skills.ErrNoMatchingAsset): + status = http.StatusBadRequest + } + writeJSON(w, status, map[string]any{ + "ok": false, "error": err.Error(), + }) + return + } + slog.Info("skills: dep installed", "dep", pkg) + writeJSON(w, http.StatusOK, map[string]any{"ok": true, "entry": entry}) + return + } + ok, errMsg := skills.InstallSingleDep(r.Context(), pkg) if !ok { writeJSON(w, http.StatusInternalServerError, map[string]any{"ok": false, "error": errMsg}) @@ -111,6 +194,9 @@ func (h *PackagesHandler) handleUninstall(w http.ResponseWriter, r *http.Request if !requireMasterScope(w, r) { return } + if !enforcePackagesWriteLimit(w, r, "/v1/packages/uninstall") { + return + } pkg := parseAndValidatePackage(w, r) if pkg == "" { return @@ -128,6 +214,95 @@ func (h *PackagesHandler) handleRuntimes(w http.ResponseWriter, _ *http.Request) writeJSON(w, http.StatusOK, skills.CheckRuntimes()) } +// handleGitHubReleases proxies the GitHub Releases API for the picker UI. +// GET /v1/packages/github-releases?repo=owner/repo&limit=10 +// Auth: viewer+ (read-only, no secrets exposed). +// Throttled via per-user rate limiter to protect the shared GitHub API quota. +func (h *PackagesHandler) handleGitHubReleases(w http.ResponseWriter, r *http.Request) { + if !enforceGitHubReleasesLimit(w, r) { + return + } + gh := skills.DefaultGitHubInstaller() + if gh == nil || gh.Client == nil { + writeJSON(w, http.StatusServiceUnavailable, map[string]string{"error": "github installer not configured"}) + return + } + repo := r.URL.Query().Get("repo") + if !validRepoPath.MatchString(repo) { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid repo; expected owner/repo"}) + return + } + parts := strings.SplitN(repo, "/", 2) + owner, repoName := parts[0], parts[1] + + if !gh.AllowedOrg(owner) { + // Return 404 rather than 403 so allowlist membership is not enumerable. + writeJSON(w, http.StatusNotFound, map[string]string{"error": "not found"}) + return + } + + limit := 10 + if s := r.URL.Query().Get("limit"); s != "" { + if n, err := strconv.Atoi(s); err == nil && n >= 1 && n <= 50 { + limit = n + } + } + + releases, err := gh.Client.ListReleases(r.Context(), owner, repoName, limit) + if err != nil { + // Map sentinel errors to generic client-safe messages. Avoid surfacing + // raw GitHub API error bodies (may include rate-limit timestamps, + // server-side internals) to viewer-level callers. + switch { + case errors.Is(err, skills.ErrGitHubRateLimited): + w.Header().Set("Retry-After", "60") + writeJSON(w, http.StatusTooManyRequests, map[string]string{"error": "github rate limit reached"}) + case errors.Is(err, skills.ErrGitHubNotFound): + writeJSON(w, http.StatusNotFound, map[string]string{"error": "repository not found"}) + case errors.Is(err, skills.ErrGitHubUnauthorized): + writeJSON(w, http.StatusBadGateway, map[string]string{"error": "github authentication failed"}) + default: + writeJSON(w, http.StatusBadGateway, map[string]string{"error": "failed to fetch releases"}) + } + return + } + + // assetPreview is a deliberately-narrow projection of GitHubAsset exposed + // to viewer-level callers of the picker. The full GitHubAsset type carries + // the CDN download URL which the UI never renders — keep the response + // surface minimal (name + size_bytes are all the picker needs). + type assetPreview struct { + Name string `json:"name"` + SizeBytes int64 `json:"size_bytes"` + } + type releaseDTO struct { + Tag string `json:"tag"` + Name string `json:"name"` + PublishedAt string `json:"published_at"` + Prerelease bool `json:"prerelease"` + MatchingAssets []assetPreview `json:"matching_assets"` + AllAssetsCount int `json:"all_assets_count"` + } + out := make([]releaseDTO, 0, len(releases)) + for _, rel := range releases { + if rel.Draft { + continue + } + dto := releaseDTO{ + Tag: rel.TagName, + Name: rel.Name, + PublishedAt: rel.PublishedAt.UTC().Format("2006-01-02T15:04:05Z"), + Prerelease: rel.Prerelease, + AllAssetsCount: len(rel.Assets), + } + if pick, perr := skills.SelectAsset(rel.Assets, "linux", runtime.GOARCH); perr == nil && pick != nil { + dto.MatchingAssets = []assetPreview{{Name: pick.Name, SizeBytes: pick.SizeBytes}} + } + out = append(out, dto) + } + writeJSON(w, http.StatusOK, map[string]any{"releases": out}) +} + // handleDenyGroups returns all registered shell deny groups with name, description, and default state. func (h *PackagesHandler) handleDenyGroups(w http.ResponseWriter, _ *http.Request) { type groupInfo struct { diff --git a/internal/http/packages_rate_limiter.go b/internal/http/packages_rate_limiter.go new file mode 100644 index 00000000..f2f7a9fd --- /dev/null +++ b/internal/http/packages_rate_limiter.go @@ -0,0 +1,154 @@ +package http + +import ( + "log/slog" + "net" + "net/http" + "sync" + "sync/atomic" + "time" + + "github.com/nextlevelbuilder/goclaw/internal/store" + "golang.org/x/time/rate" +) + +// perKeyRateLimiter is a minimal per-key token-bucket limiter used to cap +// external GitHub API usage initiated through /v1/packages/github-releases. +// Key is userID (header X-GoClaw-User-Id) or RemoteAddr when anonymous. +// +// Stale-entry eviction is amortized: every perKeyRateLimiterSweepInterval +// accepted requests trigger an inline scan. This avoids a background +// goroutine (which leaked in tests when the package-level instance was +// swapped out) and sidesteps data races on per-entry state. +type perKeyRateLimiter struct { + limiters sync.Map // key → *perKeyEntry + rps rate.Limit + burst int + callCounter atomic.Int64 +} + +type perKeyEntry struct { + limiter *rate.Limiter + lastSeen atomic.Int64 // unix nanoseconds; updated on every Allow=true +} + +// perKeyRateLimiterSweepInterval controls how often (per accepted call) the +// stale-entry sweep runs. Power-of-two for cheap modulo. +const perKeyRateLimiterSweepInterval = 1024 + +// perKeyRateLimiterStaleAfter is the idle window before an entry is evicted. +const perKeyRateLimiterStaleAfter = 10 * time.Minute + +// newPerKeyRateLimiter: rpm is requests per minute, burst is max burst size. +// rpm <= 0 disables (always allows). +func newPerKeyRateLimiter(rpm, burst int) *perKeyRateLimiter { + if burst <= 0 { + burst = 5 + } + r := rate.Limit(0) + if rpm > 0 { + r = rate.Limit(float64(rpm) / 60.0) + } + return &perKeyRateLimiter{rps: r, burst: burst} +} + +// Allow reports whether the request is within budget. +func (rl *perKeyRateLimiter) Allow(key string) bool { + if rl.rps == 0 { + return true // disabled + } + nowNs := time.Now().UnixNano() + + // Prepare a fresh entry up front; LoadOrStore discards it on existing keys. + fresh := &perKeyEntry{limiter: rate.NewLimiter(rl.rps, rl.burst)} + fresh.lastSeen.Store(nowNs) + + v, _ := rl.limiters.LoadOrStore(key, fresh) + entry := v.(*perKeyEntry) + if !entry.limiter.Allow() { + return false + } + entry.lastSeen.Store(nowNs) + + if rl.callCounter.Add(1)%perKeyRateLimiterSweepInterval == 0 { + rl.sweepStale() + } + return true +} + +// sweepStale evicts entries older than perKeyRateLimiterStaleAfter. +// Safe for concurrent invocation — sync.Map.Range + atomic lastSeen guarantee +// data-race freedom. +func (rl *perKeyRateLimiter) sweepStale() { + cutoffNs := time.Now().Add(-perKeyRateLimiterStaleAfter).UnixNano() + rl.limiters.Range(func(k, v any) bool { + if v.(*perKeyEntry).lastSeen.Load() < cutoffNs { + rl.limiters.Delete(k) + } + return true + }) +} + +// githubReleasesLimiter caps calls to the picker endpoint to protect the +// shared upstream GitHub API quota. 30 req/min/user with burst 10 leaves +// plenty of headroom for UX while preventing quota exhaustion. +var githubReleasesLimiter = newPerKeyRateLimiter(30, 10) + +// packagesWriteLimiter throttles POST /install and /uninstall. Admin-only +// endpoints, but a compromised admin token could otherwise flood the upstream +// GitHub API / pip / npm or spam manifest mutations. 10 req/min/user with +// burst 3 comfortably covers real admin workflows while breaking automated +// abuse. +var packagesWriteLimiter = newPerKeyRateLimiter(10, 3) + +// rateLimitKeyFromRequest returns the authenticated user ID if present (from +// the request context populated by enrichContext/requireAuth), else falls +// back to the raw X-GoClaw-User-Id header, else the remote IP. +// +// Preferring context over header means an admin with a leaked token can't +// rotate the header mid-session to dodge the per-user bucket — the context +// is bound to the API-key owner / session principal that survived auth. +func rateLimitKeyFromRequest(r *http.Request) string { + if uid := store.UserIDFromContext(r.Context()); uid != "" { + return "uid:" + uid + } + if uid := extractUserID(r); uid != "" { + return "uid:" + uid + } + host, _, err := net.SplitHostPort(r.RemoteAddr) + if err != nil || host == "" { + host = r.RemoteAddr + } + return "ip:" + host +} + +// enforceGitHubReleasesLimit returns true if the request is allowed; false (after +// writing 429) if throttled. +func enforceGitHubReleasesLimit(w http.ResponseWriter, r *http.Request) bool { + key := rateLimitKeyFromRequest(r) + if githubReleasesLimiter.Allow(key) { + return true + } + slog.Warn("security.rate_limited", "endpoint", "/v1/packages/github-releases", "key", key) + w.Header().Set("Retry-After", "60") + writeJSON(w, http.StatusTooManyRequests, map[string]string{ + "error": "rate limit exceeded; try again in 60 seconds", + }) + return false +} + +// enforcePackagesWriteLimit caps POST /install + /uninstall per user. +// Returns true when the request is within budget; false (after writing 429) +// when throttled. +func enforcePackagesWriteLimit(w http.ResponseWriter, r *http.Request, endpoint string) bool { + key := rateLimitKeyFromRequest(r) + if packagesWriteLimiter.Allow(key) { + return true + } + slog.Warn("security.rate_limited", "endpoint", endpoint, "key", key) + w.Header().Set("Retry-After", "60") + writeJSON(w, http.StatusTooManyRequests, map[string]string{ + "error": "rate limit exceeded; try again in 60 seconds", + }) + return false +} diff --git a/internal/http/packages_rate_limiter_test.go b/internal/http/packages_rate_limiter_test.go new file mode 100644 index 00000000..8871370b --- /dev/null +++ b/internal/http/packages_rate_limiter_test.go @@ -0,0 +1,76 @@ +package http + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +func TestPerKeyRateLimiter_AllowThenBlock(t *testing.T) { + rl := newPerKeyRateLimiter(60, 2) // 1 rps, burst 2 + + // First two requests for key A succeed (burst). + for i := 0; i < 2; i++ { + if !rl.Allow("A") { + t.Fatalf("request %d should be allowed", i) + } + } + // Third immediate request should be blocked. + if rl.Allow("A") { + t.Error("third immediate request should be rate-limited") + } + // Independent key is unaffected. + if !rl.Allow("B") { + t.Error("different key should not be rate-limited") + } +} + +func TestPerKeyRateLimiter_Disabled(t *testing.T) { + rl := newPerKeyRateLimiter(0, 5) + for i := 0; i < 100; i++ { + if !rl.Allow("x") { + t.Fatalf("disabled limiter should always allow (i=%d)", i) + } + } +} + +func TestRateLimitKeyFromRequest(t *testing.T) { + // Authenticated user: prefix with uid:. + r := httptest.NewRequest(http.MethodGet, "/x", nil) + r.Header.Set("X-GoClaw-User-Id", "alice") + if got := rateLimitKeyFromRequest(r); got != "uid:alice" { + t.Errorf("want uid:alice, got %s", got) + } + + // Anonymous: fall back to IP. + r2 := httptest.NewRequest(http.MethodGet, "/x", nil) + r2.RemoteAddr = "203.0.113.5:54321" + if got := rateLimitKeyFromRequest(r2); got != "ip:203.0.113.5" { + t.Errorf("want ip:203.0.113.5, got %s", got) + } +} + +func TestEnforceGitHubReleasesLimit_Writes429(t *testing.T) { + // Swap the package-level limiter for a tight one, restore after. + prev := githubReleasesLimiter + githubReleasesLimiter = newPerKeyRateLimiter(60, 1) // burst 1 + defer func() { githubReleasesLimiter = prev }() + + r := httptest.NewRequest(http.MethodGet, "/x", nil) + r.Header.Set("X-GoClaw-User-Id", "bob") + + w1 := httptest.NewRecorder() + if !enforceGitHubReleasesLimit(w1, r) { + t.Fatal("first call should pass") + } + w2 := httptest.NewRecorder() + if enforceGitHubReleasesLimit(w2, r) { + t.Fatal("second call should be throttled") + } + if w2.Code != http.StatusTooManyRequests { + t.Errorf("want 429, got %d", w2.Code) + } + if w2.Header().Get("Retry-After") == "" { + t.Error("Retry-After header missing") + } +} diff --git a/internal/http/packages_test.go b/internal/http/packages_test.go index 8442d82b..6bd58c91 100644 --- a/internal/http/packages_test.go +++ b/internal/http/packages_test.go @@ -118,9 +118,62 @@ func TestParseAndValidatePackage(t *testing.T) { wantStatusErr: http.StatusBadRequest, }, { - name: "starts with @", - body: `{"package":"@scope/pkg"}`, - wantEmpty: false, + name: "starts with @", + body: `{"package":"@scope/pkg"}`, + wantEmpty: false, + }, + // github: scheme — full-spec form (install + uninstall) + { + name: "github full spec", + body: `{"package":"github:cli/cli"}`, + wantEmpty: false, + }, + { + name: "github full spec with tag", + body: `{"package":"github:cli/cli@v2.45.0"}`, + wantEmpty: false, + }, + // github: scheme — bare-name form (uninstall path: UI sends github:${pkg.name}) + { + name: "github bare name (uninstall path)", + body: `{"package":"github:gh"}`, + wantEmpty: false, + }, + { + name: "github bare name with dots/hyphens", + body: `{"package":"github:ripgrep-13.0"}`, + wantEmpty: false, + }, + // github: scheme — rejected forms (injection vectors) + { + name: "github empty after prefix", + body: `{"package":"github:"}`, + wantEmpty: true, + wantStatusErr: http.StatusBadRequest, + }, + { + name: "github traversal", + body: `{"package":"github:../evil"}`, + wantEmpty: true, + wantStatusErr: http.StatusBadRequest, + }, + { + name: "github with shell injection", + body: `{"package":"github:evil;rm -rf"}`, + wantEmpty: true, + wantStatusErr: http.StatusBadRequest, + }, + { + name: "github with space", + body: `{"package":"github:evil tool"}`, + wantEmpty: true, + wantStatusErr: http.StatusBadRequest, + }, + { + name: "github starts with hyphen", + body: `{"package":"github:-flag"}`, + wantEmpty: true, + wantStatusErr: http.StatusBadRequest, }, } diff --git a/internal/skills/archive_extract.go b/internal/skills/archive_extract.go new file mode 100644 index 00000000..ae5ddf30 --- /dev/null +++ b/internal/skills/archive_extract.go @@ -0,0 +1,362 @@ +package skills + +import ( + "archive/tar" + "archive/zip" + "bytes" + "compress/gzip" + "encoding/binary" + "errors" + "fmt" + "io" + "io/fs" + "log/slog" + "os" + "path" + "path/filepath" + "strings" +) + +// Sentinel errors for archive extraction. +var ( + ErrUnsafePath = errors.New("archive: unsafe path rejected") + ErrZipBomb = errors.New("archive: uncompressed size exceeds limit (zip bomb protection)") + ErrFileTooLarge = errors.New("archive: single file exceeds limit") + ErrTooManyEntries = errors.New("archive: entry count exceeds limit (DoS protection)") +) + +// maxArchiveEntries caps regular file entries per archive. Chosen to cover +// real-world tooling (Go toolchain has ~4k, Python wheel ~6k) while +// preventing a 400M×1-byte file entry DoS where `append` growth + per-entry +// struct alloc (~64B) would OOM the server. +const maxArchiveEntries = 10_000 + +// ArchiveFile is a single extracted entry held in memory. +type ArchiveFile struct { + Name string + Mode fs.FileMode + Size int64 + Content []byte +} + +// Magic byte sequences for format detection. +var ( + magicGzip = []byte{0x1f, 0x8b} + magicZip = []byte{0x50, 0x4b, 0x03, 0x04} + magicELF = []byte{0x7f, 0x45, 0x4c, 0x46} +) + +// ExtractArchive detects the format by magic bytes + extension fallback and extracts. +// maxUncompressed caps total uncompressed bytes (zip-bomb protection). +// Raw (non-archive) inputs are named after filepath.Base(path). Use +// ExtractArchiveAs when the path is a temp file and a logical name should be +// recorded instead. +func ExtractArchive(path string, maxUncompressed int64) ([]ArchiveFile, error) { + return ExtractArchiveAs(path, "", maxUncompressed) +} + +// ExtractArchiveAs is ExtractArchive with a caller-supplied fallbackName for +// raw (non-archive) binary inputs. This matters when `path` is a temp file +// like `/tmp/goclaw-gh-asset-XXXX.bin` — without a logical name the resulting +// ArchiveFile.Name leaks the temp filename into downstream install logic. +// Empty fallbackName falls back to filepath.Base(path). +func ExtractArchiveAs(path, fallbackName string, maxUncompressed int64) ([]ArchiveFile, error) { + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer f.Close() + + var head [4]byte + n, _ := io.ReadFull(f, head[:]) + if _, err := f.Seek(0, io.SeekStart); err != nil { + return nil, err + } + prefix := head[:n] + + rawName := fallbackName + if rawName == "" { + rawName = filepath.Base(path) + } + + switch { + case bytes.HasPrefix(prefix, magicGzip): + return extractTarGz(f, maxUncompressed) + case bytes.HasPrefix(prefix, magicZip): + return extractZip(path, maxUncompressed) + case bytes.HasPrefix(prefix, magicELF): + return extractRaw(f, rawName, maxUncompressed) + } + // Fallback on extension. + lower := strings.ToLower(path) + switch { + case strings.HasSuffix(lower, ".tar.gz"), strings.HasSuffix(lower, ".tgz"): + return extractTarGz(f, maxUncompressed) + case strings.HasSuffix(lower, ".zip"): + return extractZip(path, maxUncompressed) + } + // Last resort: treat as raw binary. + return extractRaw(f, rawName, maxUncompressed) +} + +// sanitizePath rejects absolute, parent-escaping, and Windows-drive paths. +// Returns a cleaned relative path safe to join with an install dir. +func sanitizePath(name string) (string, error) { + if name == "" { + return "", fmt.Errorf("%w: empty", ErrUnsafePath) + } + // Reject null bytes explicitly. + if strings.ContainsRune(name, 0x00) { + return "", fmt.Errorf("%w: null byte", ErrUnsafePath) + } + // Reject Windows drive prefix like "C:\". + if len(name) >= 2 && name[1] == ':' { + return "", fmt.Errorf("%w: windows drive %q", ErrUnsafePath, name) + } + // Normalize both separators. + normalized := strings.ReplaceAll(name, "\\", "/") + // Reject absolute. + if strings.HasPrefix(normalized, "/") { + return "", fmt.Errorf("%w: absolute path %q", ErrUnsafePath, name) + } + // Reject any "../" component. + for _, part := range strings.Split(normalized, "/") { + if part == ".." { + return "", fmt.Errorf("%w: traversal component in %q", ErrUnsafePath, name) + } + } + cleaned := path.Clean(normalized) + // path.Clean of a non-absolute path should remain non-absolute. + if strings.HasPrefix(cleaned, "../") || cleaned == ".." || strings.HasPrefix(cleaned, "/") { + return "", fmt.Errorf("%w: escapes base after clean %q → %q", ErrUnsafePath, name, cleaned) + } + return cleaned, nil +} + +// extractTarGz streams a gzip'd tar and returns all regular-file entries. +func extractTarGz(r io.Reader, maxUncompressed int64) ([]ArchiveFile, error) { + gzr, err := gzip.NewReader(r) + if err != nil { + return nil, fmt.Errorf("gzip: %w", err) + } + defer gzr.Close() + tr := tar.NewReader(gzr) + + var out []ArchiveFile + var total int64 + // iterations counts ALL tar headers seen (including symlinks/dirs we + // skip). A crafted archive can inflate a tiny gzip payload into billions + // of zero-size symlink headers (header bytes count against + // maxUncompressed only for regular-file payloads); without a header cap, + // the loop itself becomes the DoS. + var iterations int + for { + hdr, err := tr.Next() + if errors.Is(err, io.EOF) { + break + } + if err != nil { + return nil, fmt.Errorf("tar: %w", err) + } + iterations++ + if iterations > maxArchiveEntries { + return nil, ErrTooManyEntries + } + + switch hdr.Typeflag { + case tar.TypeReg, tar.TypeRegA: + // supported + case tar.TypeSymlink, tar.TypeLink: + slog.Warn("archive: skipping link entry", "name", hdr.Name, "type", string(hdr.Typeflag)) + continue + default: + // Skip directories and other special types. + continue + } + + clean, err := sanitizePath(hdr.Name) + if err != nil { + return nil, err + } + + if hdr.Size < 0 { + return nil, fmt.Errorf("tar: negative size for %q", hdr.Name) + } + if total+hdr.Size > maxUncompressed { + return nil, ErrZipBomb + } + total += hdr.Size + + buf := make([]byte, 0, hdr.Size) + w := bytes.NewBuffer(buf) + // Use limited reader so a truncated tar doesn't spin forever. + if _, err := io.Copy(w, io.LimitReader(tr, hdr.Size)); err != nil { + return nil, fmt.Errorf("tar read %q: %w", hdr.Name, err) + } + out = append(out, ArchiveFile{ + Name: clean, + Mode: fs.FileMode(hdr.Mode) & fs.ModePerm, + Size: hdr.Size, + Content: w.Bytes(), + }) + } + return out, nil +} + +// peekZipEntryCount reads only the end-of-central-directory record and +// returns the declared entry count WITHOUT allocating per-entry structs. +// Returns (-1, nil) when EOCD lookup fails gracefully — callers should fall +// back to the stdlib parser which has its own stricter format checks. +// +// This is a critical DoS guard: zip.OpenReader alloc's a []*File of declared +// capacity BEFORE our higher-level pre-check runs. A crafted zip claiming +// 4M entries in a 200MB file could otherwise pin ~1GB of heap per call. +func peekZipEntryCount(filePath string) (int, error) { + f, err := os.Open(filePath) + if err != nil { + return -1, err + } + defer f.Close() + st, err := f.Stat() + if err != nil { + return -1, err + } + size := st.Size() + if size < 22 { + return -1, nil // too small for any valid zip EOCD + } + // EOCD record is 22 bytes + variable-length comment (up to 65535 bytes). + // Scan the last ≤65557 bytes from the back for the EOCD signature. + const eocdSigMax = 65557 + scanFrom := int64(0) + if size > eocdSigMax { + scanFrom = size - eocdSigMax + } + buf := make([]byte, size-scanFrom) + if _, err := f.ReadAt(buf, scanFrom); err != nil && !errors.Is(err, io.EOF) { + return -1, err + } + // EOCD magic: 0x06054b50 (little-endian on-disk: 50 4b 05 06). + sig := []byte{0x50, 0x4b, 0x05, 0x06} + for i := len(buf) - 22; i >= 0; i-- { + if buf[i] == sig[0] && bytes.Equal(buf[i:i+4], sig) { + // EOCD layout (offsets from record start): + // 10: total number of entries in central directory (u16) + // A value of 0xFFFF indicates ZIP64 — we conservatively bail + // and let the stdlib parser decide (it handles ZIP64 and has + // its own entries-vs-size sanity check). + n := binary.LittleEndian.Uint16(buf[i+10 : i+12]) + if n == 0xFFFF { + return -1, nil + } + return int(n), nil + } + } + return -1, nil +} + +// extractZip opens a zip file and returns all regular file entries. +func extractZip(filePath string, maxUncompressed int64) ([]ArchiveFile, error) { + // Pre-check entry count BEFORE zip.OpenReader to prevent the stdlib + // from pre-allocating [N]*zip.File for a crafted declared count. + if n, err := peekZipEntryCount(filePath); err == nil && n > maxArchiveEntries { + return nil, ErrTooManyEntries + } + + zr, err := zip.OpenReader(filePath) + if err != nil { + return nil, fmt.Errorf("zip open: %w", err) + } + defer zr.Close() + + // Post-open recheck covers ZIP64 (where peek returns -1 and the stdlib + // parsed the true count) plus declared-vs-actual mismatches. + if len(zr.File) > maxArchiveEntries { + return nil, ErrTooManyEntries + } + var sum uint64 + for _, f := range zr.File { + if f.Mode().IsDir() { + continue + } + sum += f.UncompressedSize64 + if sum > uint64(maxUncompressed) { + return nil, ErrZipBomb + } + } + + var out []ArchiveFile + var total int64 + for _, f := range zr.File { + if f.Mode().IsDir() { + continue + } + if f.Mode()&fs.ModeSymlink != 0 { + slog.Warn("archive: skipping symlink entry", "name", f.Name) + continue + } + // Guard against exhausting the cap through streaming reads alone + // (pre-check above covers declared counts — this covers runtime). + if total >= maxUncompressed { + return nil, ErrZipBomb + } + clean, err := sanitizePath(f.Name) + if err != nil { + return nil, err + } + + if f.UncompressedSize64 > uint64(maxUncompressed) { + return nil, ErrFileTooLarge + } + rc, err := f.Open() + if err != nil { + return nil, fmt.Errorf("zip open %q: %w", f.Name, err) + } + + // Also enforce streaming cap in case declared size lies. + lr := io.LimitReader(rc, maxUncompressed-total+1) + buf, err := io.ReadAll(lr) + rc.Close() + if err != nil { + return nil, fmt.Errorf("zip read %q: %w", f.Name, err) + } + if int64(len(buf)) > maxUncompressed-total { + return nil, ErrZipBomb + } + total += int64(len(buf)) + + out = append(out, ArchiveFile{ + Name: clean, + Mode: f.Mode().Perm(), + Size: int64(len(buf)), + Content: buf, + }) + } + return out, nil +} + +// extractRaw reads the entire file as a single binary entry. +// maxBytes guards against oversized inputs (belt-and-braces with the downloader +// cap) so this helper is safe to call from any context. +func extractRaw(f *os.File, name string, maxBytes int64) ([]ArchiveFile, error) { + if maxBytes <= 0 { + maxBytes = 200 * 1024 * 1024 + } + b, err := io.ReadAll(io.LimitReader(f, maxBytes+1)) + if err != nil { + return nil, err + } + if int64(len(b)) > maxBytes { + return nil, ErrFileTooLarge + } + clean, err := sanitizePath(name) + if err != nil { + return nil, err + } + return []ArchiveFile{{ + Name: clean, + Mode: 0o755, + Size: int64(len(b)), + Content: b, + }}, nil +} diff --git a/internal/skills/archive_extract_test.go b/internal/skills/archive_extract_test.go new file mode 100644 index 00000000..db898da1 --- /dev/null +++ b/internal/skills/archive_extract_test.go @@ -0,0 +1,372 @@ +package skills + +import ( + "archive/tar" + "archive/zip" + "bytes" + "compress/gzip" + "errors" + "io" + "os" + "path/filepath" + "strings" + "testing" +) + +// writeTarGz builds a .tar.gz file with the given entries. +func writeTarGz(t *testing.T, entries map[string]string) string { + t.Helper() + var buf bytes.Buffer + gz := gzip.NewWriter(&buf) + tw := tar.NewWriter(gz) + for name, body := range entries { + hdr := &tar.Header{Name: name, Mode: 0o755, Size: int64(len(body)), Typeflag: tar.TypeReg} + if err := tw.WriteHeader(hdr); err != nil { + t.Fatal(err) + } + if _, err := tw.Write([]byte(body)); err != nil { + t.Fatal(err) + } + } + tw.Close() + gz.Close() + + f, err := os.CreateTemp("", "goclaw-test-*.tar.gz") + if err != nil { + t.Fatal(err) + } + if _, err := f.Write(buf.Bytes()); err != nil { + t.Fatal(err) + } + f.Close() + t.Cleanup(func() { os.Remove(f.Name()) }) + return f.Name() +} + +// writeZip builds a .zip with the given entries. +func writeZip(t *testing.T, entries map[string]string) string { + t.Helper() + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + for name, body := range entries { + w, err := zw.Create(name) + if err != nil { + t.Fatal(err) + } + w.Write([]byte(body)) + } + zw.Close() + + f, err := os.CreateTemp("", "goclaw-test-*.zip") + if err != nil { + t.Fatal(err) + } + if _, err := f.Write(buf.Bytes()); err != nil { + t.Fatal(err) + } + f.Close() + t.Cleanup(func() { os.Remove(f.Name()) }) + return f.Name() +} + +func TestExtractTarGz_HappyPath(t *testing.T) { + path := writeTarGz(t, map[string]string{ + "lazygit": "ELF\x7fhello", + "LICENSE": "MIT", + "README.md": "readme", + }) + files, err := ExtractArchive(path, 10*1024*1024) + if err != nil { + t.Fatal(err) + } + if len(files) != 3 { + t.Fatalf("want 3 files, got %d", len(files)) + } + var gotNames []string + for _, f := range files { + gotNames = append(gotNames, f.Name) + } + for _, want := range []string{"lazygit", "LICENSE", "README.md"} { + found := false + for _, g := range gotNames { + if g == want { + found = true + break + } + } + if !found { + t.Errorf("missing %s in %v", want, gotNames) + } + } +} + +func TestExtractZip_HappyPath(t *testing.T) { + path := writeZip(t, map[string]string{ + "rg": "binary-content", + "doc.md": "doc", + }) + files, err := ExtractArchive(path, 1024*1024) + if err != nil { + t.Fatal(err) + } + if len(files) != 2 { + t.Fatalf("want 2, got %d", len(files)) + } +} + +// -------- Security: path traversal -------- + +func TestSanitizePath_Malicious(t *testing.T) { + bad := []string{ + "../../../etc/passwd", + "/etc/passwd", + "..", + "../outside", + "safe/../../etc/shadow", + "C:\\Windows\\cmd.exe", + "with\x00null", + "", + } + for _, p := range bad { + if _, err := sanitizePath(p); err == nil { + t.Errorf("sanitizePath(%q) should reject", p) + } + } + ok := []string{"lazygit", "dir/sub/file", "nested/tool.bin"} + for _, p := range ok { + if _, err := sanitizePath(p); err != nil { + t.Errorf("sanitizePath(%q) should accept: %v", p, err) + } + } +} + +func TestExtractTarGz_PathTraversal(t *testing.T) { + path := writeTarGz(t, map[string]string{"../../../etc/evil": "pwn"}) + _, err := ExtractArchive(path, 1024) + if !errors.Is(err, ErrUnsafePath) { + t.Errorf("want ErrUnsafePath, got %v", err) + } +} + +func TestExtractZip_PathTraversal(t *testing.T) { + path := writeZip(t, map[string]string{"../escape.txt": "pwn"}) + _, err := ExtractArchive(path, 1024) + if !errors.Is(err, ErrUnsafePath) { + t.Errorf("want ErrUnsafePath, got %v", err) + } +} + +// -------- Security: zip bomb / size cap -------- + +func TestExtractTarGz_ZipBomb_ByCumulativeSize(t *testing.T) { + entries := map[string]string{} + body := strings.Repeat("A", 1024) + for i := 0; i < 100; i++ { + entries[fileNameN(i)] = body // 100 KB total + } + path := writeTarGz(t, entries) + _, err := ExtractArchive(path, 10*1024) // 10 KB cap + if !errors.Is(err, ErrZipBomb) { + t.Errorf("want ErrZipBomb, got %v", err) + } +} + +func TestExtractZip_ZipBomb(t *testing.T) { + entries := map[string]string{} + body := strings.Repeat("X", 1024) + for i := 0; i < 100; i++ { + entries[fileNameN(i)] = body + } + path := writeZip(t, entries) + _, err := ExtractArchive(path, 10*1024) + if !errors.Is(err, ErrZipBomb) { + t.Errorf("want ErrZipBomb, got %v", err) + } +} + +// -------- Security: ELF validation -------- + +func TestValidateELF_NonELFRejected(t *testing.T) { + vectors := map[string][]byte{ + "PDF": []byte("%PDF-1.4\n"), + "shell": []byte("#!/bin/bash\necho hi\n"), + "PE": []byte("MZ\x90\x00"), + "machO": {0xcf, 0xfa, 0xed, 0xfe, 0x07, 0x00, 0x00, 0x01}, + "truncated": {0x7f, 0x45, 0x4c}, + "empty": {}, + } + for name, v := range vectors { + if err := validateELF(v); err == nil { + t.Errorf("vector %s should be rejected as non-ELF", name) + } + } +} + +// -------- Security: extractRaw for unknown bytes -------- + +func TestExtractRaw_NonArchive(t *testing.T) { + tmp, err := os.CreateTemp("", "raw-*.bin") + if err != nil { + t.Fatal(err) + } + defer os.Remove(tmp.Name()) + tmp.Write([]byte("plain bytes")) + tmp.Close() + files, err := ExtractArchive(tmp.Name(), 1024) + if err != nil { + t.Fatal(err) + } + if len(files) != 1 || string(files[0].Content) != "plain bytes" { + t.Errorf("unexpected files: %+v", files) + } +} + +// -------- Helpers -------- + +func fileNameN(i int) string { + // Avoid importing "strconv" for tiny helper. + return filepath.Join("d", []string{"a", "b", "c"}[i%3], "f", rune2s(i)) +} +func rune2s(i int) string { + var buf [6]byte + n := 0 + if i == 0 { + return "0" + } + for i > 0 { + buf[n] = byte('0' + i%10) + n++ + i /= 10 + } + // reverse + var out []byte + for k := n - 1; k >= 0; k-- { + out = append(out, buf[k]) + } + return string(out) +} + +// Ensure the compress/bytes/io imports are kept — used only in helpers above. +var _ = io.EOF + +// -------- P1.1: Raw ELF uses caller-supplied logical name -------- + +func TestExtractArchiveAs_RawELFUsesFallbackName(t *testing.T) { + // Write a tiny "ELF" (magic bytes only — format parsing is done by + // validateELF in callers, extractRaw just copies bytes). + tmp, err := os.CreateTemp("", "goclaw-gh-asset-*.bin") + if err != nil { + t.Fatal(err) + } + defer os.Remove(tmp.Name()) + tmp.Write([]byte{0x7f, 0x45, 0x4c, 0x46, 0x02, 0x01, 0x01, 0x00}) // \x7fELF header stub + tmp.Close() + + // Without fallback → takes temp basename (the bug we're fixing). + files, err := ExtractArchive(tmp.Name(), 1024) + if err != nil { + t.Fatal(err) + } + if !strings.HasPrefix(filepath.Base(files[0].Name), "goclaw-gh-asset-") { + t.Errorf("expected temp basename leakage when no fallback, got %q", files[0].Name) + } + + // With fallback → records logical name. + files, err = ExtractArchiveAs(tmp.Name(), "lazygit", 1024) + if err != nil { + t.Fatal(err) + } + if files[0].Name != "lazygit" { + t.Errorf("ExtractArchiveAs with fallbackName=lazygit recorded %q, want %q", + files[0].Name, "lazygit") + } +} + +// -------- P1.3: Archive entry count cap (DoS protection) -------- + +func TestExtractTarGz_EntryCountCap(t *testing.T) { + // Build a tar archive with more than maxArchiveEntries tiny entries. + var buf bytes.Buffer + gz := gzip.NewWriter(&buf) + tw := tar.NewWriter(gz) + for i := 0; i < maxArchiveEntries+5; i++ { + name := "f" + rune2s(i) + hdr := &tar.Header{Name: name, Mode: 0o644, Size: 1, Typeflag: tar.TypeReg} + if err := tw.WriteHeader(hdr); err != nil { + t.Fatal(err) + } + tw.Write([]byte{'x'}) + } + tw.Close() + gz.Close() + + f, err := os.CreateTemp("", "many-*.tar.gz") + if err != nil { + t.Fatal(err) + } + defer os.Remove(f.Name()) + f.Write(buf.Bytes()) + f.Close() + + _, err = ExtractArchive(f.Name(), 100*1024*1024) // Large size cap — only entry cap should trip. + if !errors.Is(err, ErrTooManyEntries) { + t.Errorf("want ErrTooManyEntries, got %v", err) + } +} + +func TestExtractZip_EntryCountCap(t *testing.T) { + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + for i := 0; i < maxArchiveEntries+5; i++ { + w, err := zw.Create("f" + rune2s(i)) + if err != nil { + t.Fatal(err) + } + w.Write([]byte{'x'}) + } + zw.Close() + + f, err := os.CreateTemp("", "many-*.zip") + if err != nil { + t.Fatal(err) + } + defer os.Remove(f.Name()) + f.Write(buf.Bytes()) + f.Close() + + _, err = ExtractArchive(f.Name(), 100*1024*1024) + if !errors.Is(err, ErrTooManyEntries) { + t.Errorf("want ErrTooManyEntries, got %v", err) + } +} + +// TestPeekZipEntryCount covers the DoS-preemption path: the EOCD-level check +// must reject oversized archives BEFORE zip.OpenReader allocates +// []*zip.File of declared capacity. +func TestPeekZipEntryCount(t *testing.T) { + // Small zip: peek returns exact count. + small := writeZip(t, map[string]string{"a": "1", "b": "2", "c": "3"}) + n, err := peekZipEntryCount(small) + if err != nil { + t.Fatalf("peek small: %v", err) + } + if n != 3 { + t.Errorf("peek small count = %d, want 3", n) + } + + // Non-zip file: peek returns -1 gracefully (no false positive). + raw, err := os.CreateTemp("", "raw-*.bin") + if err != nil { + t.Fatal(err) + } + defer os.Remove(raw.Name()) + raw.Write([]byte("not a zip file, just random bytes")) + raw.Close() + n, err = peekZipEntryCount(raw.Name()) + if err != nil { + t.Fatalf("peek raw: %v", err) + } + if n != -1 { + t.Errorf("peek raw should return -1 (EOCD not found), got %d", n) + } +} diff --git a/internal/skills/dep_installer.go b/internal/skills/dep_installer.go index f1fcfca1..140b6b40 100644 --- a/internal/skills/dep_installer.go +++ b/internal/skills/dep_installer.go @@ -12,7 +12,10 @@ import ( "time" ) -const installTimeout = 5 * time.Minute +// InstallTimeout is the wall-clock cap applied to a single package install. +// Exported so HTTP handlers that bypass InstallSingleDep (e.g. the github: +// fast path) can wrap their context with the same deadline. +const InstallTimeout = 5 * time.Minute // pkgHelperSocket is the Unix socket path for the root-privileged pkg-helper. const pkgHelperSocket = "/tmp/pkg.sock" @@ -46,12 +49,23 @@ func AggregateMissingDeps(skillDirs map[string]string) (*SkillManifest, []string // InstallSingleDep installs one dependency (format: "pip:pkg", "npm:pkg", or plain binary name). // Returns (ok, errorMessage). Logs progress via slog so the Log page can show install status. func InstallSingleDep(ctx context.Context, dep string) (bool, string) { - ctx, cancel := context.WithTimeout(ctx, installTimeout) + ctx, cancel := context.WithTimeout(ctx, InstallTimeout) defer cancel() slog.Info("skills: installing dep", "dep", dep) switch { + case strings.HasPrefix(dep, "github:"): + gh := DefaultGitHubInstaller() + if gh == nil { + return false, "github installer not configured" + } + if _, err := gh.Install(ctx, dep); err != nil { + slog.Error("skills: github install failed", "dep", dep, "error", err) + return false, err.Error() + } + slog.Info("skills: dep installed", "dep", dep) + return true, "" case strings.HasPrefix(dep, "pip:"): pkg := strings.TrimPrefix(dep, "pip:") cmd := exec.CommandContext(ctx, "pip3", "install", "--no-cache-dir", "--break-system-packages", pkg) @@ -87,7 +101,7 @@ func InstallSingleDep(ctx context.Context, dep string) (bool, string) { // InstallDeps installs missing packages by category. // Uses PIP_TARGET and NPM_CONFIG_PREFIX from env (set by docker-entrypoint.sh). func InstallDeps(ctx context.Context, manifest *SkillManifest, missing []string) (*InstallResult, error) { - ctx, cancel := context.WithTimeout(ctx, installTimeout) + ctx, cancel := context.WithTimeout(ctx, InstallTimeout) defer cancel() result := &InstallResult{} @@ -156,12 +170,46 @@ func InstallDeps(ctx context.Context, manifest *SkillManifest, missing []string) // UninstallPackage removes one package (format: "pip:pkg", "npm:pkg", or plain apk name). // Returns (ok, errorMessage). func UninstallPackage(ctx context.Context, dep string) (bool, string) { - ctx, cancel := context.WithTimeout(ctx, installTimeout) + ctx, cancel := context.WithTimeout(ctx, InstallTimeout) defer cancel() slog.Info("skills: uninstalling package", "dep", dep) switch { + case strings.HasPrefix(dep, "github:"): + gh := DefaultGitHubInstaller() + if gh == nil { + return false, "github installer not configured" + } + // Accept either "github:name" (manifest name only) or the full + // "github:owner/repo[@tag]". For the full form we look up the manifest + // entry by owner/repo so packages whose binary name differs from the + // repo name (e.g. cli/cli → gh) can still be uninstalled via spec. + name := strings.TrimPrefix(dep, "github:") + if spec, err := ParseGitHubSpec(dep); err == nil { + name = spec.Repo + if entries, lerr := gh.List(); lerr == nil { + want := spec.Owner + "/" + spec.Repo + for _, e := range entries { + if strings.EqualFold(e.Repo, want) { + name = e.Name + break + } + } + } + } else if slash := strings.Index(name, "/"); slash >= 0 { + // Tolerate bare "owner/repo" without the scheme prefix. + name = name[slash+1:] + if at := strings.IndexByte(name, '@'); at >= 0 { + name = name[:at] + } + } + if err := gh.Uninstall(ctx, name); err != nil { + slog.Error("skills: github uninstall failed", "dep", dep, "error", err) + return false, err.Error() + } + slog.Info("skills: package uninstalled", "dep", dep) + return true, "" case strings.HasPrefix(dep, "pip:"): pkg := strings.TrimPrefix(dep, "pip:") cmd := exec.CommandContext(ctx, "pip3", "uninstall", "-y", pkg) diff --git a/internal/skills/github_api.go b/internal/skills/github_api.go new file mode 100644 index 00000000..de331fa7 --- /dev/null +++ b/internal/skills/github_api.go @@ -0,0 +1,215 @@ +package skills + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "strconv" + "strings" + "sync" + "time" +) + +// Sentinel errors returned by the GitHub API client. +var ( + ErrGitHubNotFound = errors.New("github: release not found") + ErrGitHubUnauthorized = errors.New("github: unauthorized (check token)") + ErrGitHubRateLimited = errors.New("github: rate limited") + ErrGitHubServer = errors.New("github: server error") +) + +// GitHubAsset describes a single release asset. +type GitHubAsset struct { + Name string `json:"name"` + DownloadURL string `json:"browser_download_url"` + SizeBytes int64 `json:"size"` + ContentType string `json:"content_type"` +} + +// GitHubRelease is a simplified projection of the GitHub release payload. +type GitHubRelease struct { + TagName string `json:"tag_name"` + Name string `json:"name"` + PublishedAt time.Time `json:"published_at"` + Prerelease bool `json:"prerelease"` + Draft bool `json:"draft"` + Assets []GitHubAsset `json:"assets"` +} + +// releaseCacheEntry is a single cached release lookup. +type releaseCacheEntry struct { + data any + expiresAt time.Time +} + +// GitHubClient is a minimal REST client for the GitHub Releases API. +// Supports optional bearer token (private repos + higher rate limit) and +// an in-memory 10-minute TTL cache keyed by "owner/repo:tag". +type GitHubClient struct { + Token string + BaseURL string // default "https://api.github.com" — overridable for tests + HTTPClient *http.Client + + mu sync.Mutex + cache map[string]releaseCacheEntry + ttl time.Duration +} + +// NewGitHubClient creates a client. If httpClient is nil, a default with 30s timeout is used. +func NewGitHubClient(token string) *GitHubClient { + return &GitHubClient{ + Token: token, + BaseURL: "https://api.github.com", + HTTPClient: &http.Client{Timeout: 30 * time.Second}, + cache: make(map[string]releaseCacheEntry), + ttl: 10 * time.Minute, + } +} + +func (c *GitHubClient) cacheGet(key string) (any, bool) { + c.mu.Lock() + defer c.mu.Unlock() + e, ok := c.cache[key] + if !ok || time.Now().After(e.expiresAt) { + return nil, false + } + return e.data, true +} + +// cacheMaxEntries is a SOFT sweep trigger, not a hard cap: once the map +// reaches this size we scan for expired entries and drop them before +// inserting the new one. If every entry is still live the map can briefly +// exceed the threshold — in practice the 10-minute TTL keeps growth bounded +// by the request rate. Prevents unbounded growth from many distinct repos +// being queried over long uptime. +const cacheMaxEntries = 256 + +func (c *GitHubClient) cacheSet(key string, v any) { + c.mu.Lock() + defer c.mu.Unlock() + if len(c.cache) >= cacheMaxEntries { + now := time.Now() + for k, e := range c.cache { + if now.After(e.expiresAt) { + delete(c.cache, k) + } + } + } + c.cache[key] = releaseCacheEntry{data: v, expiresAt: time.Now().Add(c.ttl)} +} + +// GetRelease fetches a single release by tag. If tag is empty, "latest" is used. +func (c *GitHubClient) GetRelease(ctx context.Context, owner, repo, tag string) (*GitHubRelease, error) { + key := fmt.Sprintf("rel:%s/%s:%s", owner, repo, tag) + if v, ok := c.cacheGet(key); ok { + r := v.(*GitHubRelease) + return r, nil + } + + var path string + if tag == "" { + path = fmt.Sprintf("/repos/%s/%s/releases/latest", + url.PathEscape(owner), url.PathEscape(repo)) + } else { + // PathEscape the tag so characters valid in git refs but URL-special + // (#, ?, %, +) don't silently corrupt the path (# → fragment, ? → query). + path = fmt.Sprintf("/repos/%s/%s/releases/tags/%s", + url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(tag)) + } + + var rel GitHubRelease + if err := c.doJSON(ctx, path, &rel); err != nil { + return nil, err + } + c.cacheSet(key, &rel) + return &rel, nil +} + +// ListReleases returns the most recent releases (at most `limit`, max 100). +func (c *GitHubClient) ListReleases(ctx context.Context, owner, repo string, limit int) ([]GitHubRelease, error) { + if limit <= 0 { + limit = 10 + } + if limit > 100 { + limit = 100 + } + key := fmt.Sprintf("list:%s/%s:%d", owner, repo, limit) + if v, ok := c.cacheGet(key); ok { + return v.([]GitHubRelease), nil + } + path := fmt.Sprintf("/repos/%s/%s/releases?per_page=%d", + url.PathEscape(owner), url.PathEscape(repo), limit) + var releases []GitHubRelease + if err := c.doJSON(ctx, path, &releases); err != nil { + return nil, err + } + c.cacheSet(key, releases) + return releases, nil +} + +// doJSON performs a GET + JSON decode, mapping status codes to sentinel errors. +func (c *GitHubClient) doJSON(ctx context.Context, path string, out any) error { + // Avoid shadowing the "net/url" package import used elsewhere in this file. + apiURL := c.BaseURL + path + req, err := http.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) + if err != nil { + return err + } + req.Header.Set("Accept", "application/vnd.github+json") + req.Header.Set("X-GitHub-Api-Version", "2022-11-28") + if c.Token != "" { + req.Header.Set("Authorization", "Bearer "+c.Token) + } + + resp, err := c.HTTPClient.Do(req) + if err != nil { + return fmt.Errorf("github: http request failed: %w", err) + } + defer resp.Body.Close() + + switch { + case resp.StatusCode == http.StatusOK: + // fall through + case resp.StatusCode == http.StatusNotFound: + return ErrGitHubNotFound + case resp.StatusCode == http.StatusUnauthorized: + return ErrGitHubUnauthorized + case resp.StatusCode == http.StatusForbidden: + // Rate limit check + remaining := resp.Header.Get("X-RateLimit-Remaining") + if remaining == "0" { + reset := resp.Header.Get("X-RateLimit-Reset") + if n, errConv := strconv.ParseInt(reset, 10, 64); errConv == nil { + return fmt.Errorf("%w (resets at %s)", ErrGitHubRateLimited, time.Unix(n, 0).UTC().Format(time.RFC3339)) + } + return ErrGitHubRateLimited + } + return ErrGitHubUnauthorized + case resp.StatusCode == http.StatusTooManyRequests: + // GitHub secondary rate limits (abuse detection, search, unauthenticated + // bursts) return 429 rather than 403+X-RateLimit-Remaining:0. Map both + // onto the same sentinel so the HTTP handler renders a 429 "rate limit + // reached" instead of a 502 "failed to fetch releases". + return ErrGitHubRateLimited + case resp.StatusCode >= 500: + return ErrGitHubServer + default: + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024)) + return fmt.Errorf("github: unexpected status %d: %s", resp.StatusCode, strings.TrimSpace(string(body))) + } + + // Cap the response body at a generous 8 MiB. GitHub's release/list + // payloads are well under this (a 100-release list with rich asset + // metadata sits around 1 MiB). Belt-and-braces in case a future caller + // adds a path that could return a much larger document, or a + // man-in-the-middle / misbehaving upstream sends an oversized body. + const maxAPIResponseBytes = 8 * 1024 * 1024 + if err := json.NewDecoder(io.LimitReader(resp.Body, maxAPIResponseBytes)).Decode(out); err != nil { + return fmt.Errorf("github: decode response: %w", err) + } + return nil +} diff --git a/internal/skills/github_api_test.go b/internal/skills/github_api_test.go new file mode 100644 index 00000000..d27d0314 --- /dev/null +++ b/internal/skills/github_api_test.go @@ -0,0 +1,81 @@ +package skills + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "testing" +) + +func TestGitHubClient_GetRelease(t *testing.T) { + hits := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits++ + if r.Header.Get("Authorization") != "Bearer testtoken" { + t.Errorf("expected bearer token, got %q", r.Header.Get("Authorization")) + } + switch r.URL.Path { + case "/repos/cli/cli/releases/latest": + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{ + "tag_name": "v2.45.0", + "name": "v2.45.0", + "published_at": "2025-01-01T00:00:00Z", + "prerelease": false, + "assets": [ + {"name": "gh_linux_amd64.tar.gz", "browser_download_url": "https://github.com/cli/cli/releases/download/v2.45.0/gh.tar.gz", "size": 1024} + ] + }`)) + case "/repos/missing/repo/releases/latest": + w.WriteHeader(http.StatusNotFound) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer srv.Close() + + c := NewGitHubClient("testtoken") + c.BaseURL = srv.URL + + rel, err := c.GetRelease(context.Background(), "cli", "cli", "") + if err != nil { + t.Fatal(err) + } + if rel.TagName != "v2.45.0" || len(rel.Assets) != 1 { + t.Errorf("unexpected release %+v", rel) + } + + // Cache hit shouldn't increment hits. + first := hits + rel2, err := c.GetRelease(context.Background(), "cli", "cli", "") + if err != nil { + t.Fatal(err) + } + if rel2.TagName != rel.TagName { + t.Error("cache returned different release") + } + if hits != first { + t.Errorf("expected cache hit, got %d → %d", first, hits) + } + + _, err = c.GetRelease(context.Background(), "missing", "repo", "") + if !errors.Is(err, ErrGitHubNotFound) { + t.Errorf("want ErrGitHubNotFound, got %v", err) + } +} + +func TestGitHubClient_RateLimited(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("X-RateLimit-Remaining", "0") + w.Header().Set("X-RateLimit-Reset", "9999999999") + w.WriteHeader(http.StatusForbidden) + })) + defer srv.Close() + c := NewGitHubClient("") + c.BaseURL = srv.URL + _, err := c.GetRelease(context.Background(), "x", "y", "v1") + if !errors.Is(err, ErrGitHubRateLimited) { + t.Errorf("want ErrGitHubRateLimited, got %v", err) + } +} diff --git a/internal/skills/github_checksum.go b/internal/skills/github_checksum.go new file mode 100644 index 00000000..654c1269 --- /dev/null +++ b/internal/skills/github_checksum.go @@ -0,0 +1,95 @@ +package skills + +import ( + "crypto/subtle" + "errors" + "fmt" + "regexp" + "strings" +) + +// ErrChecksumMismatch is returned when a computed SHA256 doesn't match the expected value. +var ErrChecksumMismatch = errors.New("github: checksum mismatch") + +// checksumLineRE parses ` ` or ` *` lines. +var checksumLineRE = regexp.MustCompile(`(?m)^([a-fA-F0-9]{64})[ \t]+\*?(\S.*)$`) + +// FindChecksumAsset scans a release's assets looking for a checksum file. +// Returns nil if none found. +// Lookup order: +// 1. .sha256 +// 2. checksums.txt +// 3. SHA256SUMS / SHA256SUMS.txt +// 4. *_checksums.txt (pattern match) +// 5. *.sha256sums +func FindChecksumAsset(release *GitHubRelease, assetName string) *GitHubAsset { + if release == nil { + return nil + } + targetPerAsset := strings.ToLower(assetName + ".sha256") + byLower := func(n string) string { return strings.ToLower(n) } + + // Pass 1: exact per-asset .sha256 companion. + for i := range release.Assets { + if byLower(release.Assets[i].Name) == targetPerAsset { + return &release.Assets[i] + } + } + // Pass 2: common aggregate names. + priorities := []string{"checksums.txt", "sha256sums", "sha256sums.txt", "sha256sum.txt"} + for _, p := range priorities { + for i := range release.Assets { + if byLower(release.Assets[i].Name) == p { + return &release.Assets[i] + } + } + } + // Pass 3: fuzzier match — trailing `_checksums.txt` / `-checksums.txt` / `.sha256sums`. + for i := range release.Assets { + n := byLower(release.Assets[i].Name) + if strings.HasSuffix(n, "_checksums.txt") || strings.HasSuffix(n, "-checksums.txt") { + return &release.Assets[i] + } + if strings.HasSuffix(n, ".sha256sums") { + return &release.Assets[i] + } + } + return nil +} + +// ParseChecksums parses a checksums.txt-style file into a map of filename → SHA256. +// Accepts both ` ` (two spaces) and ` *` (binary prefix). +// Unrecognized lines are ignored (comments, empty lines). +func ParseChecksums(content []byte) (map[string]string, error) { + matches := checksumLineRE.FindAllSubmatch(content, -1) + out := make(map[string]string, len(matches)) + for _, m := range matches { + sha := strings.ToLower(string(m[1])) + name := strings.TrimSpace(string(m[2])) + // `sha256sum ./file` emits `./file` in the name column. Strip the + // leading `./` so the lookup in the caller (keyed by bare asset + // basename) matches. Real release checksums almost never use this + // form, but the guard is defensive and essentially free. + name = strings.TrimPrefix(name, "./") + // In "checksums.txt" the name may be just a basename. Keep as-is; + // caller looks up by asset name which matches basename. + out[name] = sha + } + if len(out) == 0 { + return nil, fmt.Errorf("no valid checksum entries found") + } + return out, nil +} + +// VerifyChecksum does a constant-time comparison between expected and actual hex strings. +func VerifyChecksum(expected, actual string) error { + e := strings.ToLower(strings.TrimSpace(expected)) + a := strings.ToLower(strings.TrimSpace(actual)) + if len(e) == 0 || len(a) == 0 { + return ErrChecksumMismatch + } + if subtle.ConstantTimeCompare([]byte(e), []byte(a)) != 1 { + return fmt.Errorf("%w (expected %s, got %s)", ErrChecksumMismatch, e, a) + } + return nil +} diff --git a/internal/skills/github_checksum_test.go b/internal/skills/github_checksum_test.go new file mode 100644 index 00000000..e6062bed --- /dev/null +++ b/internal/skills/github_checksum_test.go @@ -0,0 +1,76 @@ +package skills + +import ( + "errors" + "testing" +) + +func TestParseChecksums(t *testing.T) { + input := []byte(`# comment +abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890 lazygit_Linux_x86_64.tar.gz +1111111111111111111111111111111111111111111111111111111111111111 *lazygit_Linux_arm64.tar.gz +not a valid line at all +`) + m, err := ParseChecksums(input) + if err != nil { + t.Fatal(err) + } + if len(m) != 2 { + t.Fatalf("want 2 entries, got %d: %+v", len(m), m) + } + if m["lazygit_Linux_x86_64.tar.gz"] == "" { + t.Error("missing amd64 entry") + } + if m["lazygit_Linux_arm64.tar.gz"] == "" { + t.Error("missing arm64 entry (binary prefix *)") + } +} + +func TestParseChecksums_Empty(t *testing.T) { + if _, err := ParseChecksums([]byte("")); err == nil { + t.Error("empty file should error") + } +} + +func TestVerifyChecksum(t *testing.T) { + if err := VerifyChecksum("aBc", "abc"); err != nil { + t.Errorf("case-insensitive match failed: %v", err) + } + err := VerifyChecksum("abc", "xyz") + if !errors.Is(err, ErrChecksumMismatch) { + t.Errorf("want ErrChecksumMismatch, got %v", err) + } + if err := VerifyChecksum("", "abc"); err == nil { + t.Error("empty expected should fail") + } +} + +func TestFindChecksumAsset(t *testing.T) { + rel := &GitHubRelease{ + Assets: []GitHubAsset{ + {Name: "binary.tar.gz"}, + {Name: "binary.tar.gz.sha256"}, + {Name: "checksums.txt"}, + }, + } + a := FindChecksumAsset(rel, "binary.tar.gz") + if a == nil || a.Name != "binary.tar.gz.sha256" { + t.Errorf("should prefer per-asset .sha256, got %v", a) + } + + rel2 := &GitHubRelease{ + Assets: []GitHubAsset{ + {Name: "binary.tar.gz"}, + {Name: "SHA256SUMS"}, + }, + } + a = FindChecksumAsset(rel2, "binary.tar.gz") + if a == nil || a.Name != "SHA256SUMS" { + t.Errorf("should fall back to SHA256SUMS, got %v", a) + } + + rel3 := &GitHubRelease{Assets: []GitHubAsset{{Name: "binary.tar.gz"}}} + if FindChecksumAsset(rel3, "binary.tar.gz") != nil { + t.Error("no checksum asset should return nil") + } +} diff --git a/internal/skills/github_default_installer.go b/internal/skills/github_default_installer.go new file mode 100644 index 00000000..f4d9f3e3 --- /dev/null +++ b/internal/skills/github_default_installer.go @@ -0,0 +1,27 @@ +package skills + +import "sync" + +// defaultGitHubInstaller is the process-wide installer used by free functions +// (InstallSingleDep, UninstallPackage, ListInstalledPackages). It is set once +// at startup via SetDefaultGitHubInstaller and read without locking thereafter. +var ( + defaultGitHubInstallerMu sync.RWMutex + defaultGitHubInstaller *GitHubInstaller +) + +// SetDefaultGitHubInstaller registers the installer used by prefix dispatch in +// the top-level install/uninstall/list helpers. Safe to call multiple times +// (replaces the existing installer). +func SetDefaultGitHubInstaller(i *GitHubInstaller) { + defaultGitHubInstallerMu.Lock() + defer defaultGitHubInstallerMu.Unlock() + defaultGitHubInstaller = i +} + +// DefaultGitHubInstaller returns the registered installer, or nil if unset. +func DefaultGitHubInstaller() *GitHubInstaller { + defaultGitHubInstallerMu.RLock() + defer defaultGitHubInstallerMu.RUnlock() + return defaultGitHubInstaller +} diff --git a/internal/skills/github_download.go b/internal/skills/github_download.go new file mode 100644 index 00000000..6f18db70 --- /dev/null +++ b/internal/skills/github_download.go @@ -0,0 +1,125 @@ +package skills + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "errors" + "fmt" + "io" + "net" + "net/http" + "net/url" + "os" + "strings" +) + +// Sentinel errors for the downloader. +var ( + ErrNotHTTPS = errors.New("github.download: non-HTTPS URL rejected") + ErrHostNotAllowed = errors.New("github.download: host not in allowlist") + ErrAssetTooLarge = errors.New("github.download: asset exceeds max size") + ErrTooManyRedirect = errors.New("github.download: too many redirects") +) + +// allowedDownloadHosts is the SSRF allowlist for asset downloads. +var allowedDownloadHosts = map[string]bool{ + "github.com": true, + "api.github.com": true, + "objects.githubusercontent.com": true, + "release-assets.githubusercontent.com": true, + "codeload.github.com": true, +} + +// validateDownloadURL ensures the URL is HTTPS and the host is allowlisted. +// Also blocks private/loopback IPs when the host is an IP literal. +func validateDownloadURL(rawURL string) error { + u, err := url.Parse(rawURL) + if err != nil { + return fmt.Errorf("github.download: parse url: %w", err) + } + if u.Scheme != "https" { + return ErrNotHTTPS + } + host := strings.ToLower(u.Hostname()) + if !allowedDownloadHosts[host] { + return fmt.Errorf("%w: %s", ErrHostNotAllowed, host) + } + // Block literal IPs as hostname (prevents raw-IP SSRF via rebinding tricks). + if ip := net.ParseIP(host); ip != nil { + return fmt.Errorf("%w: literal IP %s", ErrHostNotAllowed, host) + } + return nil +} + +// DownloadAsset streams an asset over HTTPS to a temp file, validating the URL, +// enforcing a max byte cap, and computing SHA256 as it writes. +// Caller must remove the temp file. +func (c *GitHubClient) DownloadAsset(ctx context.Context, assetURL string, maxBytes int64) (string, string, error) { + if err := validateDownloadURL(assetURL); err != nil { + return "", "", err + } + if maxBytes <= 0 { + maxBytes = 200 * 1024 * 1024 + } + + // Build a client that validates every redirect hop. + // No Timeout here — it caps the whole request including body read, which + // would abort large (hundreds of MB) downloads on modest connections. + // The caller's context carries the correct deadline (install timeout). + client := &http.Client{ + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= 5 { + return ErrTooManyRedirect + } + return validateDownloadURL(req.URL.String()) + }, + } + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, assetURL, nil) + if err != nil { + return "", "", err + } + req.Header.Set("Accept", "application/octet-stream") + if c.Token != "" { + req.Header.Set("Authorization", "Bearer "+c.Token) + } + + resp, err := client.Do(req) + if err != nil { + return "", "", fmt.Errorf("github.download: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(io.LimitReader(resp.Body, 1024)) + return "", "", fmt.Errorf("github.download: status %d: %s", + resp.StatusCode, strings.TrimSpace(string(body))) + } + + tmp, err := os.CreateTemp("", "goclaw-gh-asset-*.bin") + if err != nil { + return "", "", err + } + tmpName := tmp.Name() + _ = tmp.Chmod(0o600) + + h := sha256.New() + // Read up to maxBytes+1 so we can detect overflow. + limited := io.LimitReader(resp.Body, maxBytes+1) + n, err := io.Copy(io.MultiWriter(tmp, h), limited) + cerr := tmp.Close() + if err != nil { + os.Remove(tmpName) + return "", "", fmt.Errorf("github.download: copy: %w", err) + } + if cerr != nil { + os.Remove(tmpName) + return "", "", cerr + } + if n > maxBytes { + os.Remove(tmpName) + return "", "", ErrAssetTooLarge + } + return tmpName, hex.EncodeToString(h.Sum(nil)), nil +} diff --git a/internal/skills/github_download_test.go b/internal/skills/github_download_test.go new file mode 100644 index 00000000..e5f8d96e --- /dev/null +++ b/internal/skills/github_download_test.go @@ -0,0 +1,70 @@ +package skills + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestValidateDownloadURL_SSRF(t *testing.T) { + blocked := []string{ + "http://github.com/foo", // plain HTTP + "https://internal.example.com/x", // not allowlisted + "https://github.com.attacker.com/x", // prefix attack + "https://127.0.0.1/metadata", // literal IP + "https://[::1]/x", // IPv6 literal + "https://169.254.169.254/latest/meta-data", // cloud metadata + "https://metadata.google.internal/x", // GCP metadata + "ftp://github.com/foo", // non-HTTPS scheme + } + for _, u := range blocked { + if err := validateDownloadURL(u); err == nil { + t.Errorf("should reject %q", u) + } + } + allowed := []string{ + "https://github.com/org/repo/releases/download/v1/asset.tar.gz", + "https://objects.githubusercontent.com/release-assets/123", + "https://api.github.com/repos/org/repo/releases/latest", + } + for _, u := range allowed { + if err := validateDownloadURL(u); err != nil { + t.Errorf("should allow %q: %v", u, err) + } + } +} + +func TestDownloadAsset_MaxSize(t *testing.T) { + // Spin up a fake allowlisted server by pointing the allowlist entry to a + // test server via DNS override isn't feasible inside pure Go tests; instead + // temporarily mutate allowedDownloadHosts for this single test. + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // 2 KiB payload. + w.Write([]byte(strings.Repeat("A", 2048))) + })) + defer srv.Close() + + // Use github.com allowlist entry by swapping DNS via URL rewriting is + // more complex; instead we call the internal copy helper directly by + // temporarily whitelisting 127.0.0.1. The SSRF validator blocks literal + // IPs so this test focuses solely on the overflow branch. We inline the + // download loop logic from DownloadAsset to simulate overflow without + // hitting the SSRF block. + // Exercise: cap at 1024 against 2048-byte response → overflow. + client := NewGitHubClient("") + // Save + restore allowlist. + prev := allowedDownloadHosts + allowedDownloadHosts = map[string]bool{"127.0.0.1": true} + defer func() { allowedDownloadHosts = prev }() + // validateDownloadURL blocks literal IP. Emulate by pointing URL host + // to a registered name — simplest path: call DownloadAsset with + // srv.URL which has host "127.0.0.1:PORT"; validator rejects literal IP + // regardless of allowlist. So instead assert the host rejection path. + _, _, err := client.DownloadAsset(context.Background(), srv.URL, 1024) + if !errors.Is(err, ErrHostNotAllowed) { + t.Errorf("want ErrHostNotAllowed for literal-IP host, got %v", err) + } +} diff --git a/internal/skills/github_installer.go b/internal/skills/github_installer.go new file mode 100644 index 00000000..98d88f2a --- /dev/null +++ b/internal/skills/github_installer.go @@ -0,0 +1,610 @@ +package skills + +import ( + "bytes" + "context" + "debug/elf" + "encoding/json" + "errors" + "fmt" + "log/slog" + "os" + "path/filepath" + "regexp" + "runtime" + "sort" + "strings" + "sync" + "time" +) + +// Sentinel errors for the GitHub installer. +var ( + ErrInvalidGitHubSpec = errors.New("github: invalid spec (expected github:owner/repo[@tag])") + ErrGitHubOrgNotAllowed = errors.New("github: org not in allowlist") + ErrNoMatchingAsset = errors.New("github: no matching asset for runtime") + ErrNotELF = errors.New("github: not an ELF binary") + ErrUnsupportedELFClass = errors.New("github: only 64-bit ELF supported") + ErrELFArchMismatch = errors.New("github: ELF architecture mismatch") + ErrNoBinaryInArchive = errors.New("github: no executable found in archive") + ErrPackageNotInstalled = errors.New("github: package not installed") + ErrUnsupportedOS = errors.New("github: install only supported on Linux") +) + +// GitHubSpec is the parsed form of a "github:owner/repo[@tag]" identifier. +type GitHubSpec struct { + Owner string + Repo string + Tag string // empty string means "latest" +} + +// gitHubSpecRE validates the supported identifier format. +// Owner: GitHub usernames are capped at 39 chars, alnum + hyphen, no leading/trailing hyphen. +// Repo: alnum + `.`/`_`/`-`. +// Tag: capped at 255 chars (git ref-name upper bound), excludes NUL/whitespace. +var gitHubSpecRE = regexp.MustCompile(`^github:([A-Za-z0-9](?:[A-Za-z0-9-]{0,37})?[A-Za-z0-9]|[A-Za-z0-9])/([A-Za-z0-9][A-Za-z0-9._-]*)(?:@([^\s\x00]{1,255}))?$`) + +// ParseGitHubSpec parses an identifier string. +func ParseGitHubSpec(s string) (*GitHubSpec, error) { + m := gitHubSpecRE.FindStringSubmatch(s) + if m == nil { + return nil, ErrInvalidGitHubSpec + } + return &GitHubSpec{Owner: m[1], Repo: m[2], Tag: m[3]}, nil +} + +// GitHubPackagesConfig holds tunables for the installer. +// Token is sourced from env var only (never config.json plaintext). +type GitHubPackagesConfig struct { + Token string // optional GitHub personal access token + BinDir string // where to install binaries (default /app/data/.runtime/bin) + ManifestPath string // manifest file path (default {BinDir}/../github-packages.json) + AllowedOrgs []string // lowercase list; empty = all allowed + MaxAssetSizeMB int // default 200 +} + +// Defaults fills in zero-valued fields. +func (c *GitHubPackagesConfig) Defaults() { + if c.BinDir == "" { + c.BinDir = "/app/data/.runtime/bin" + } + if c.ManifestPath == "" { + c.ManifestPath = filepath.Join(filepath.Dir(c.BinDir), "github-packages.json") + } + if c.MaxAssetSizeMB <= 0 { + c.MaxAssetSizeMB = 200 + } + // Normalize allowed orgs to lowercase, trim whitespace, drop empties. + out := c.AllowedOrgs[:0] + for _, o := range c.AllowedOrgs { + o = strings.ToLower(strings.TrimSpace(o)) + if o != "" { + out = append(out, o) + } + } + c.AllowedOrgs = out +} + +// MaxAssetBytes returns MaxAssetSizeMB as a byte count. +func (c *GitHubPackagesConfig) MaxAssetBytes() int64 { + return int64(c.MaxAssetSizeMB) * 1024 * 1024 +} + +// GitHubInstaller orchestrates end-to-end install + uninstall + list. +type GitHubInstaller struct { + Client *GitHubClient + Config *GitHubPackagesConfig + + mu sync.Mutex // serializes the final disk-write phase: bin dir writes + manifest mutation + // (download, extraction, and ELF validation intentionally run outside the lock) +} + +// NewGitHubInstaller constructs an installer. +func NewGitHubInstaller(client *GitHubClient, cfg *GitHubPackagesConfig) *GitHubInstaller { + if cfg == nil { + cfg = &GitHubPackagesConfig{} + } + cfg.Defaults() + return &GitHubInstaller{Client: client, Config: cfg} +} + +// AllowedOrg returns true if owner passes allowlist (empty slice = all allowed). +func (i *GitHubInstaller) AllowedOrg(owner string) bool { + if len(i.Config.AllowedOrgs) == 0 { + return true + } + owner = strings.ToLower(owner) + for _, a := range i.Config.AllowedOrgs { + if a == owner { + return true + } + } + return false +} + +// -------- Asset selection -------- + +var ( + excludeSuffixRE = regexp.MustCompile(`(?i)\.(sha256|sig|asc|minisig|pem|pub|cert|crt)$`) + excludeNameRE = regexp.MustCompile(`(?i)(source[\s_-]?code|source\.tar\.gz|source\.zip)`) + linuxRE = regexp.MustCompile(`(?i)linux`) + amd64RE = regexp.MustCompile(`(?i)(amd64|x86[-_]?64|x64)`) + arm64RE = regexp.MustCompile(`(?i)(arm64|aarch64)`) +) + +// SelectAsset picks the best asset for target OS + arch. +// Heuristic (in order): +// 1. exclude checksum/signature suffix files +// 2. exclude "source code" archives +// 3. filter by OS ("linux") +// 4. filter by arch +// 5. prefer .tar.gz/.tgz > .zip > raw +// 6. tiebreak by shortest name +func SelectAsset(assets []GitHubAsset, goos, goarch string) (*GitHubAsset, error) { + candidates := make([]GitHubAsset, 0, len(assets)) + for _, a := range assets { + if excludeSuffixRE.MatchString(a.Name) { + continue + } + if excludeNameRE.MatchString(a.Name) { + continue + } + candidates = append(candidates, a) + } + + if goos == "linux" { + candidates = filterAssets(candidates, linuxRE) + } + switch goarch { + case "amd64": + candidates = filterAssets(candidates, amd64RE) + case "arm64": + candidates = filterAssets(candidates, arm64RE) + } + + if len(candidates) == 0 { + return nil, enrichNoMatch(assets, goos, goarch) + } + + sort.SliceStable(candidates, func(i, j int) bool { + pi, pj := extPriority(candidates[i].Name), extPriority(candidates[j].Name) + if pi != pj { + return pi < pj + } + return len(candidates[i].Name) < len(candidates[j].Name) + }) + pick := candidates[0] + return &pick, nil +} + +func filterAssets(in []GitHubAsset, re *regexp.Regexp) []GitHubAsset { + out := in[:0:0] + for _, a := range in { + if re.MatchString(a.Name) { + out = append(out, a) + } + } + return out +} + +// extPriority gives lower numbers to preferred archive formats. +func extPriority(name string) int { + n := strings.ToLower(name) + switch { + case strings.HasSuffix(n, ".tar.gz"), strings.HasSuffix(n, ".tgz"): + return 0 + case strings.HasSuffix(n, ".zip"): + return 1 + default: + return 2 + } +} + +func enrichNoMatch(all []GitHubAsset, goos, goarch string) error { + names := make([]string, 0, len(all)) + for _, a := range all { + names = append(names, a.Name) + } + return fmt.Errorf("%w (os=%s, arch=%s); available: %s", + ErrNoMatchingAsset, goos, goarch, strings.Join(names, ", ")) +} + +// -------- Manifest -------- + +// GitHubPackageEntry records metadata about an installed package. +// +// Note: there is no InstalledBy field — the install call doesn't currently +// thread a user ID through, and an always-empty audit field is more misleading +// than useful. Add it back if/when the request context is plumbed in. +type GitHubPackageEntry struct { + Name string `json:"name"` + Repo string `json:"repo"` + Tag string `json:"tag"` + Binaries []string `json:"binaries"` + SHA256 string `json:"sha256"` + AssetURL string `json:"asset_url"` + AssetName string `json:"asset_name"` + AssetSizeBytes int64 `json:"asset_size_bytes"` + InstalledAt time.Time `json:"installed_at"` +} + +// GitHubManifest is the persisted state for installed GitHub packages. +type GitHubManifest struct { + Version int `json:"version"` + Packages []GitHubPackageEntry `json:"packages"` +} + +// loadManifest returns an empty manifest if file missing. +func (i *GitHubInstaller) loadManifest() (*GitHubManifest, error) { + b, err := os.ReadFile(i.Config.ManifestPath) + if err != nil { + if os.IsNotExist(err) { + return &GitHubManifest{Version: 1}, nil + } + return nil, err + } + var m GitHubManifest + if err := json.Unmarshal(b, &m); err != nil { + return nil, fmt.Errorf("parse manifest: %w", err) + } + if m.Version == 0 { + m.Version = 1 + } + return &m, nil +} + +// saveManifest writes atomically via temp + fsync + rename + dir fsync. +// The two fsyncs ensure durability across crashes/power-loss: without +// file fsync the rename commits a possibly-empty inode; without dir fsync +// the rename itself may be reordered on XFS/ext4 with journal-async. +func (i *GitHubInstaller) saveManifest(m *GitHubManifest) error { + dir := filepath.Dir(i.Config.ManifestPath) + if err := os.MkdirAll(dir, 0o755); err != nil { + return err + } + b, err := json.MarshalIndent(m, "", " ") + if err != nil { + return err + } + tmp := i.Config.ManifestPath + ".tmp" + f, err := os.OpenFile(tmp, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o640) + if err != nil { + return err + } + if _, err := f.Write(b); err != nil { + f.Close() + os.Remove(tmp) + return err + } + if err := f.Sync(); err != nil { + f.Close() + os.Remove(tmp) + return err + } + if err := f.Close(); err != nil { + os.Remove(tmp) + return err + } + if err := os.Rename(tmp, i.Config.ManifestPath); err != nil { + os.Remove(tmp) + return err + } + // Best-effort dir fsync — opening a dir as O_RDONLY works on Linux/macOS + // but some filesystems (e.g. Windows via WSL paths) may reject it. We + // log and proceed — the rename has already happened. + if d, derr := os.Open(dir); derr == nil { + _ = d.Sync() + d.Close() + } + return nil +} + +// List returns all installed packages from manifest. +func (i *GitHubInstaller) List() ([]GitHubPackageEntry, error) { + i.mu.Lock() + defer i.mu.Unlock() + m, err := i.loadManifest() + if err != nil { + return nil, err + } + return m.Packages, nil +} + +// -------- ELF validation -------- + +// validateELF checks magic bytes, 64-bit class, and machine matches runtime. +func validateELF(content []byte) error { + if len(content) < 4 { + return ErrNotELF + } + if !bytes.Equal(content[:4], []byte{0x7f, 0x45, 0x4c, 0x46}) { + return ErrNotELF + } + f, err := elf.NewFile(bytes.NewReader(content)) + if err != nil { + return fmt.Errorf("invalid ELF: %w", err) + } + defer f.Close() + + if f.Class != elf.ELFCLASS64 { + return ErrUnsupportedELFClass + } + wantMachine := elf.EM_X86_64 + if runtime.GOARCH == "arm64" { + wantMachine = elf.EM_AARCH64 + } + if f.Machine != wantMachine { + return fmt.Errorf("%w (binary=%v, runtime=%v)", ErrELFArchMismatch, f.Machine, wantMachine) + } + return nil +} + +// -------- Binary picker -------- + +var nonBinaryPathRE = regexp.MustCompile(`(?i)(^|/)(man|docs?|contrib|completions|examples?|tests?|licenses?)/`) + +// pickBinaries selects executable entries from an extracted archive. +// Preference order: +// 1. entries with basename == repo name (common case: `lazygit` binary in lazygit archive) +// 2. all executable entries whose path doesn't match nonBinaryPathRE +// (man/docs/contrib/completions/examples/tests/licenses are excluded) +// 3. any ELF-magic entry under a non-excluded path +func pickBinaries(files []ArchiveFile, repoName string) []ArchiveFile { + // Filter out clearly-not-binary paths first. + var candidates []ArchiveFile + for _, f := range files { + if nonBinaryPathRE.MatchString(f.Name) { + continue + } + candidates = append(candidates, f) + } + + // Try exact basename match to repo name. + var named []ArchiveFile + for _, f := range candidates { + base := filepath.Base(f.Name) + if base == repoName { + named = append(named, f) + } + } + if len(named) > 0 { + return named + } + + // Otherwise: any executable-looking entry that survived the nonBinaryPathRE + // filter above. Depth is not enforced here — ELF validation in the caller + // is the final gate before chmod +x. + var execs []ArchiveFile + for _, f := range candidates { + if isLikelyExecutable(f) { + execs = append(execs, f) + } + } + return execs +} + +func isLikelyExecutable(f ArchiveFile) bool { + // Executable bit set OR ELF magic present. + if f.Mode&0o111 != 0 { + return true + } + if len(f.Content) >= 4 && bytes.Equal(f.Content[:4], []byte{0x7f, 0x45, 0x4c, 0x46}) { + return true + } + return false +} + +// -------- Install + Uninstall -------- + +// Install runs the full pipeline: parse → check org → fetch release → select asset → +// download → verify → extract → validate ELF → write to bin dir → update manifest. +func (i *GitHubInstaller) Install(ctx context.Context, spec string) (*GitHubPackageEntry, error) { + // The installer ships only Linux ELF asset selection + validation. Guard + // non-Linux callers (Windows/macOS desktop host) up front so we don't + // waste bandwidth fetching a Linux asset that's going to be rejected at + // the ELF-machine check later. + if runtime.GOOS != "linux" { + return nil, fmt.Errorf("%w (got %s)", ErrUnsupportedOS, runtime.GOOS) + } + parsed, err := ParseGitHubSpec(spec) + if err != nil { + return nil, err + } + if !i.AllowedOrg(parsed.Owner) { + return nil, fmt.Errorf("%w: %s", ErrGitHubOrgNotAllowed, parsed.Owner) + } + + release, err := i.Client.GetRelease(ctx, parsed.Owner, parsed.Repo, parsed.Tag) + if err != nil { + return nil, err + } + asset, err := SelectAsset(release.Assets, "linux", runtime.GOARCH) + if err != nil { + return nil, err + } + + maxBytes := i.Config.MaxAssetBytes() + tmpPath, sha, err := i.Client.DownloadAsset(ctx, asset.DownloadURL, maxBytes) + if err != nil { + return nil, err + } + defer os.Remove(tmpPath) + + // Checksum verification (if publisher provides it). + // Failure modes that silently proceed are noisy-logged so an operator can + // detect a modified checksum file being served alongside a tampered asset. + if ca := FindChecksumAsset(release, asset.Name); ca != nil { + checksumPath, _, cerr := i.Client.DownloadAsset(ctx, ca.DownloadURL, 1<<20) // 1 MiB cap + if cerr == nil { + defer os.Remove(checksumPath) + data, rerr := os.ReadFile(checksumPath) + if rerr != nil { + slog.Warn("github.installer: read checksum file failed", + "checksum_asset", ca.Name, "error", rerr) + } else { + sums, perr := ParseChecksums(data) + if perr != nil { + slog.Warn("github.installer: parse checksum file failed", + "checksum_asset", ca.Name, "error", perr) + } else if expected, ok := sums[asset.Name]; ok { + if verr := VerifyChecksum(expected, sha); verr != nil { + return nil, verr + } + } else { + // Publisher ships a checksum file that omits this asset. + // Could be benign (asset added later, different file set) + // or a MITM replacing the checksum file with an entry-free + // one. We warn loudly; ELF validation remains the final gate. + slog.Warn("github.installer: asset not listed in checksum file", + "asset", asset.Name, "checksum_asset", ca.Name) + } + } + } else { + slog.Warn("github.installer: failed to fetch checksum file", "error", cerr) + } + } else { + // Not a problem with the install — many upstream publishers simply + // don't ship checksum files (jq, fzf, older ripgrep, etc.). Downgraded + // from Warn so the suspicious cases (read/parse/asset-not-listed + // errors, which stay at Warn above) stand out cleanly. + slog.Info("github.installer: no checksum asset available", "asset", asset.Name) + } + + // Pass the repo name as the fallback logical name so raw (non-archive) + // ELF assets don't end up recorded under the temp filename + // "goclaw-gh-asset-XXXX.bin". + files, err := ExtractArchiveAs(tmpPath, parsed.Repo, 2*maxBytes) + if err != nil { + return nil, err + } + + binaries := pickBinaries(files, parsed.Repo) + if len(binaries) == 0 { + return nil, fmt.Errorf("%w: %s", ErrNoBinaryInArchive, asset.Name) + } + for idx := range binaries { + if err := validateELF(binaries[idx].Content); err != nil { + return nil, err + } + } + + // Commit to disk under lock. + i.mu.Lock() + defer i.mu.Unlock() + + if err := os.MkdirAll(i.Config.BinDir, 0o755); err != nil { + return nil, fmt.Errorf("create bin dir: %w", err) + } + + // Load manifest first so we can detect basename collisions with other + // installed packages. Current policy is still last-writer-wins (changing + // that would break re-install), but an operator needs to know when two + // packages are fighting over the same binary name. + m, err := i.loadManifest() + if err != nil { + return nil, err + } + entryRepo := parsed.Owner + "/" + parsed.Repo + + binNames := make([]string, 0, len(binaries)) + for _, b := range binaries { + name := filepath.Base(b.Name) + // Warn if another package in the manifest already owns this binary name. + for _, p := range m.Packages { + if !strings.EqualFold(p.Repo, entryRepo) { + for _, pb := range p.Binaries { + if pb == name { + slog.Warn("github.installer: binary name collision — overwriting", + "binary", name, "existing_package", p.Name, + "existing_repo", p.Repo, "new_repo", entryRepo) + } + } + } + } + dst := filepath.Join(i.Config.BinDir, name) + if err := os.WriteFile(dst, b.Content, 0o755); err != nil { + return nil, fmt.Errorf("write binary %s: %w", name, err) + } + binNames = append(binNames, name) + } + + entry := GitHubPackageEntry{ + Name: canonicalPackageName(parsed, binNames), + Repo: entryRepo, + Tag: release.TagName, + Binaries: binNames, + SHA256: sha, + AssetURL: asset.DownloadURL, + AssetName: asset.Name, + AssetSizeBytes: asset.SizeBytes, + InstalledAt: time.Now().UTC(), + } + + // Replace existing entry with same name. + found := false + for idx := range m.Packages { + if m.Packages[idx].Name == entry.Name { + m.Packages[idx] = entry + found = true + break + } + } + if !found { + m.Packages = append(m.Packages, entry) + } + if err := i.saveManifest(m); err != nil { + return nil, err + } + return &entry, nil +} + +// canonicalPackageName uses repo name unless a single binary has a different name. +func canonicalPackageName(spec *GitHubSpec, binNames []string) string { + if len(binNames) == 1 && binNames[0] != spec.Repo { + return binNames[0] + } + return spec.Repo +} + +// Uninstall removes installed binaries + manifest entry. +// name matches GitHubPackageEntry.Name. +func (i *GitHubInstaller) Uninstall(ctx context.Context, name string) error { + _ = ctx + i.mu.Lock() + defer i.mu.Unlock() + + m, err := i.loadManifest() + if err != nil { + return err + } + idx := -1 + for k, p := range m.Packages { + if p.Name == name { + idx = k + break + } + } + if idx < 0 { + return ErrPackageNotInstalled + } + binaries := m.Packages[idx].Binaries + // Persist the manifest BEFORE touching the files on disk. If saveManifest + // fails we bail out without orphaning binaries on a manifest that still + // claims them as installed (a retried Uninstall would otherwise hit + // ErrPackageNotInstalled after the first attempt wiped the files). + m.Packages = append(m.Packages[:idx], m.Packages[idx+1:]...) + if err := i.saveManifest(m); err != nil { + return err + } + // Remove-after-save is best-effort: a missing file is fine (idempotent), + // any other error is warned and the manifest still reflects the truth + // that the entry is no longer tracked. + for _, b := range binaries { + // Only remove files within our configured bin dir (defense in depth). + path := filepath.Join(i.Config.BinDir, filepath.Base(b)) + if rerr := os.Remove(path); rerr != nil && !os.IsNotExist(rerr) { + slog.Warn("github.installer: remove binary failed", "path", path, "error", rerr) + } + } + return nil +} diff --git a/internal/skills/github_installer_test.go b/internal/skills/github_installer_test.go new file mode 100644 index 00000000..3d226fce --- /dev/null +++ b/internal/skills/github_installer_test.go @@ -0,0 +1,147 @@ +package skills + +import ( + "errors" + "strings" + "testing" +) + +func TestParseGitHubSpec(t *testing.T) { + cases := []struct { + in string + ok bool + owner string + repo string + tag string + }{ + {"github:cli/cli@v2.45.0", true, "cli", "cli", "v2.45.0"}, + {"github:cli/cli", true, "cli", "cli", ""}, + {"github:jesseduffield/lazygit@v0.42.0", true, "jesseduffield", "lazygit", "v0.42.0"}, + {"github:sharkdp/fd@v9.0.0+build.1", true, "sharkdp", "fd", "v9.0.0+build.1"}, + {"github:owner/repo.subpath@v1", true, "owner", "repo.subpath", "v1"}, + {"github:a/b", true, "a", "b", ""}, + {"github:Org-1/Repo_2@x-y.z", true, "Org-1", "Repo_2", "x-y.z"}, + {"pip:foo", false, "", "", ""}, + {"github:/repo", false, "", "", ""}, + {"github:owner/", false, "", "", ""}, + {"github:-bad/repo", false, "", "", ""}, + {"github:bad-/repo", false, "", "", ""}, + {"github:owner/repo@", false, "", "", ""}, + {"", false, "", "", ""}, + } + for _, tc := range cases { + got, err := ParseGitHubSpec(tc.in) + if tc.ok { + if err != nil { + t.Errorf("%q: unexpected error %v", tc.in, err) + continue + } + if got.Owner != tc.owner || got.Repo != tc.repo || got.Tag != tc.tag { + t.Errorf("%q: got %+v, want {%s %s %s}", tc.in, got, tc.owner, tc.repo, tc.tag) + } + } else { + if err == nil { + t.Errorf("%q: expected error, got %+v", tc.in, got) + } + } + } +} + +func TestSelectAsset(t *testing.T) { + // Realistic-ish asset lists. + lazygit := []GitHubAsset{ + {Name: "lazygit_0.42.0_Linux_x86_64.tar.gz"}, + {Name: "lazygit_0.42.0_Linux_arm64.tar.gz"}, + {Name: "lazygit_0.42.0_Darwin_x86_64.tar.gz"}, + {Name: "lazygit_0.42.0_Windows_x86_64.zip"}, + {Name: "checksums.txt"}, + } + starship := []GitHubAsset{ + {Name: "starship-x86_64-unknown-linux-musl.tar.gz"}, + {Name: "starship-aarch64-unknown-linux-musl.tar.gz"}, + {Name: "starship-x86_64-pc-windows-msvc.zip"}, + {Name: "starship-x86_64-unknown-linux-musl.tar.gz.sha256"}, + } + noMatch := []GitHubAsset{ + {Name: "tool-Darwin-arm64.tar.gz"}, + {Name: "Source code (zip)"}, + } + + t.Run("lazygit amd64", func(t *testing.T) { + a, err := SelectAsset(lazygit, "linux", "amd64") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(a.Name, "Linux_x86_64") { + t.Errorf("got %s", a.Name) + } + }) + t.Run("lazygit arm64", func(t *testing.T) { + a, err := SelectAsset(lazygit, "linux", "arm64") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(a.Name, "Linux_arm64") { + t.Errorf("got %s", a.Name) + } + }) + t.Run("starship musl amd64", func(t *testing.T) { + a, err := SelectAsset(starship, "linux", "amd64") + if err != nil { + t.Fatal(err) + } + if a.Name != "starship-x86_64-unknown-linux-musl.tar.gz" { + t.Errorf("got %s", a.Name) + } + }) + t.Run("no match", func(t *testing.T) { + _, err := SelectAsset(noMatch, "linux", "amd64") + if !errors.Is(err, ErrNoMatchingAsset) { + t.Errorf("want ErrNoMatchingAsset, got %v", err) + } + if !strings.Contains(err.Error(), "tool-Darwin-arm64") { + t.Errorf("error should list available assets, got: %v", err) + } + }) +} + +func TestAllowedOrg(t *testing.T) { + empty := NewGitHubInstaller(nil, &GitHubPackagesConfig{}) + if !empty.AllowedOrg("anyone") { + t.Error("empty allowlist should permit all orgs") + } + locked := NewGitHubInstaller(nil, &GitHubPackagesConfig{AllowedOrgs: []string{"GoodOrg", " digitop "}}) + if !locked.AllowedOrg("goodorg") { + t.Error("goodorg should be allowed (case-insensitive)") + } + if !locked.AllowedOrg("Digitop") { + t.Error("digitop should be allowed after trim+lowercase") + } + if locked.AllowedOrg("evil") { + t.Error("evil should be rejected") + } +} + +func TestConfigDefaults(t *testing.T) { + c := &GitHubPackagesConfig{} + c.Defaults() + if c.BinDir == "" || c.ManifestPath == "" || c.MaxAssetSizeMB != 200 { + t.Errorf("unexpected defaults: %+v", c) + } + if c.MaxAssetBytes() != 200*1024*1024 { + t.Errorf("MaxAssetBytes wrong: %d", c.MaxAssetBytes()) + } +} + +func TestCanonicalPackageName(t *testing.T) { + spec := &GitHubSpec{Owner: "cli", Repo: "cli"} + if canonicalPackageName(spec, []string{"gh"}) != "gh" { + t.Error("single differing binary name should win") + } + if canonicalPackageName(spec, []string{"cli"}) != "cli" { + t.Error("matching binary name should use repo") + } + if canonicalPackageName(spec, []string{"a", "b"}) != "cli" { + t.Error("multi binaries should fall back to repo name") + } +} diff --git a/internal/skills/package_lister.go b/internal/skills/package_lister.go index 39365fa5..e34f627e 100644 --- a/internal/skills/package_lister.go +++ b/internal/skills/package_lister.go @@ -17,11 +17,25 @@ type PackageInfo struct { Version string `json:"version"` } +// GitHubPackageListEntry is a viewer-safe projection of GitHubPackageEntry. +// Deliberately omits AssetURL / SHA256 / AssetName so viewer-level callers of +// GET /v1/packages don't receive CDN download URLs or checksum metadata — +// those are install-time details the UI never renders. Mirrors the same +// narrowing applied to the release-picker endpoint (`assetPreview`). +type GitHubPackageListEntry struct { + Name string `json:"name"` + Repo string `json:"repo"` + Tag string `json:"tag"` + Binaries []string `json:"binaries"` + InstalledAt time.Time `json:"installed_at"` +} + // InstalledPackages groups installed packages by manager. type InstalledPackages struct { - System []PackageInfo `json:"system"` - Pip []PackageInfo `json:"pip"` - Npm []PackageInfo `json:"npm"` + System []PackageInfo `json:"system"` + Pip []PackageInfo `json:"pip"` + Npm []PackageInfo `json:"npm"` + GitHub []GitHubPackageListEntry `json:"github,omitempty"` } const listTimeout = 15 * time.Second @@ -36,6 +50,20 @@ func ListInstalledPackages(ctx context.Context) *InstalledPackages { result.System = listApkUserPackages(ctx) result.Pip = listPipPackages(ctx) result.Npm = listNpmPackages(ctx) + if gh := DefaultGitHubInstaller(); gh != nil { + if entries, err := gh.List(); err == nil { + result.GitHub = make([]GitHubPackageListEntry, 0, len(entries)) + for _, e := range entries { + result.GitHub = append(result.GitHub, GitHubPackageListEntry{ + Name: e.Name, + Repo: e.Repo, + Tag: e.Tag, + Binaries: e.Binaries, + InstalledAt: e.InstalledAt, + }) + } + } + } return result } diff --git a/internal/skills/runtime_check.go b/internal/skills/runtime_check.go index 0293f3fb..b39298ca 100644 --- a/internal/skills/runtime_check.go +++ b/internal/skills/runtime_check.go @@ -63,6 +63,18 @@ func CheckRuntimes() *RuntimeStatus { } status.Runtimes = append(status.Runtimes, pkgInfo) + // Check github-bin runtime directory (where GitHub-installed binaries live). + ghInfo := RuntimeInfo{Name: "github-bin"} + binDir := "/app/data/.runtime/bin" + if gh := DefaultGitHubInstaller(); gh != nil && gh.Config != nil && gh.Config.BinDir != "" { + binDir = gh.Config.BinDir + } + if fi, err := os.Stat(binDir); err == nil && fi.IsDir() { + ghInfo.Available = true + ghInfo.Version = binDir + } + status.Runtimes = append(status.Runtimes, ghInfo) + return status } diff --git a/ui/web/src/i18n/locales/en/packages.json b/ui/web/src/i18n/locales/en/packages.json index 7a7b37e3..16c739c6 100644 --- a/ui/web/src/i18n/locales/en/packages.json +++ b/ui/web/src/i18n/locales/en/packages.json @@ -22,6 +22,25 @@ "title": "Node Packages", "placeholder": "Package name (e.g. typescript)" }, + "github": { + "title": "GitHub Binaries", + "placeholder": "owner/repo[@tag] (e.g. cli/cli@v2.45.0)", + "browse": "Browse releases", + "muslWarning": "Alpine Linux uses musl libc. Binaries compiled against glibc may fail at runtime. Prefer static or musl-compatible releases.", + "muslDismiss": "Dismiss", + "pickerTitle": "Select release from {{repo}}", + "pickerEmpty": "No releases found for {{repo}}", + "pickerLoading": "Loading releases...", + "pickerMatchingAssets": "{{count}} matching / {{total}} total", + "pickerPrerelease": "Pre-release", + "pickerSelect": "Select", + "columns": { + "repo": "Repository", + "tag": "Version", + "binaries": "Binaries", + "installedAt": "Installed" + } + }, "actions": { "install": "Install", "uninstall": "Uninstall", diff --git a/ui/web/src/i18n/locales/vi/packages.json b/ui/web/src/i18n/locales/vi/packages.json index f231638a..8e112434 100644 --- a/ui/web/src/i18n/locales/vi/packages.json +++ b/ui/web/src/i18n/locales/vi/packages.json @@ -22,6 +22,25 @@ "title": "Gói Node", "placeholder": "Tên gói (vd: typescript)" }, + "github": { + "title": "Binary GitHub", + "placeholder": "owner/repo[@tag] (vd: cli/cli@v2.45.0)", + "browse": "Xem release", + "muslWarning": "Alpine Linux dùng musl libc. Binary biên dịch với glibc có thể fail runtime. Chọn binary static hoặc release musl-compatible.", + "muslDismiss": "Đóng", + "pickerTitle": "Chọn release từ {{repo}}", + "pickerEmpty": "Không tìm thấy release cho {{repo}}", + "pickerLoading": "Đang tải releases...", + "pickerMatchingAssets": "{{count}} phù hợp / {{total}} tổng", + "pickerPrerelease": "Pre-release", + "pickerSelect": "Chọn", + "columns": { + "repo": "Repository", + "tag": "Phiên bản", + "binaries": "Binaries", + "installedAt": "Ngày cài" + } + }, "actions": { "install": "Cài đặt", "uninstall": "Gỡ bỏ", diff --git a/ui/web/src/i18n/locales/zh/packages.json b/ui/web/src/i18n/locales/zh/packages.json index fbb5722c..a4848c76 100644 --- a/ui/web/src/i18n/locales/zh/packages.json +++ b/ui/web/src/i18n/locales/zh/packages.json @@ -22,6 +22,25 @@ "title": "Node 软件包", "placeholder": "包名（例如 typescript）" }, + "github": { + "title": "GitHub 二进制", + "placeholder": "owner/repo[@tag]（例如 cli/cli@v2.45.0）", + "browse": "浏览版本", + "muslWarning": "Alpine Linux 使用 musl libc。使用 glibc 编译的二进制文件可能在运行时失败。优先选择静态或 musl 兼容版本。", + "muslDismiss": "关闭", + "pickerTitle": "从 {{repo}} 选择版本", + "pickerEmpty": "{{repo}} 没有发行版", + "pickerLoading": "正在加载发行版...", + "pickerMatchingAssets": "{{count}} 匹配 / {{total}} 总计", + "pickerPrerelease": "预发行版", + "pickerSelect": "选择", + "columns": { + "repo": "仓库", + "tag": "版本", + "binaries": "二进制", + "installedAt": "安装时间" + } + }, "actions": { "install": "安装", "uninstall": "卸载", diff --git a/ui/web/src/pages/packages/github-binaries-section.tsx b/ui/web/src/pages/packages/github-binaries-section.tsx new file mode 100644 index 00000000..d0868dba --- /dev/null +++ b/ui/web/src/pages/packages/github-binaries-section.tsx @@ -0,0 +1,326 @@ +import { useState } from "react"; +import { useTranslation } from "react-i18next"; +import { useQuery } from "@tanstack/react-query"; +import { Loader2, Download, Trash2, Info, X, GitBranch } from "lucide-react"; +import { Button } from "@/components/ui/button"; +import { Alert, AlertDescription } from "@/components/ui/alert"; +import { Dialog, DialogContent, DialogHeader, DialogTitle } from "@/components/ui/dialog"; +import { ConfirmDialog } from "@/components/shared/confirm-dialog"; +import { useHttp } from "@/hooks/use-ws"; +import { queryKeys } from "@/lib/query-keys"; + +// Viewer-safe projection — backend strips asset_url / sha256 / asset_name from +// the GET /v1/packages response (see GitHubPackageListEntry in Go). The UI +// only renders repo / tag / binaries / installed_at, so those extra fields +// were never needed on this side. +export interface GitHubPackageEntry { + name: string; + repo: string; + tag: string; + binaries: string[]; + installed_at: string; +} + +interface AssetPreview { + name: string; + size_bytes: number; +} + +interface ReleaseDTO { + tag: string; + name: string; + published_at: string; + prerelease: boolean; + matching_assets: AssetPreview[] | null; + all_assets_count: number; +} + +interface Props { + packages: GitHubPackageEntry[] | null | undefined; + onInstall: (pkg: string) => Promise<{ ok: boolean }>; + onUninstall: (pkg: string) => Promise<{ ok: boolean }>; +} + +const MUSL_DISMISS_KEY = "packages.musl_warning_dismissed"; + +// Owner: GitHub usernames are capped at 39 chars (alnum + hyphen, no leading/trailing hyphen). +// Repo: alnum + `.`/`_`/`-`. Mirrors the backend `gitHubSpecRE`. +const OWNER_REPO_RE = + /^([A-Za-z0-9](?:[A-Za-z0-9-]{0,37})?[A-Za-z0-9]|[A-Za-z0-9])\/[A-Za-z0-9][A-Za-z0-9._-]*$/; + +function stripPrefixAndTag(spec: string): string { + // Destructuring with a default satisfies TS `noUncheckedIndexedAccess` + // (split is guaranteed to return ≥1 element at runtime, but TS types it + // as `string | undefined`). + const [name = ""] = spec.replace(/^github:/, "").split("@"); + return name; +} + +function isValidRepo(spec: string): boolean { + return OWNER_REPO_RE.test(stripPrefixAndTag(spec)); +} + +function isValidFullSpec(spec: string): boolean { + // owner/repo OR owner/repo@tag (prefix `github:` optional in the input box). + // Tag capped at 255 chars to mirror the backend regex. + return /^([A-Za-z0-9](?:[A-Za-z0-9-]{0,37})?[A-Za-z0-9]|[A-Za-z0-9])\/[A-Za-z0-9][A-Za-z0-9._-]*(@[^\s]{1,255})?$/.test( + spec.replace(/^github:/, "") + ); +} + +export function GitHubBinariesSection({ packages, onInstall, onUninstall }: Props) { + const { t } = useTranslation("packages"); + const [input, setInput] = useState(""); + const [installing, setInstalling] = useState(false); + const [pickerOpen, setPickerOpen] = useState(false); + const [pickerRepo, setPickerRepo] = useState(""); + const [uninstallTarget, setUninstallTarget] = useState(null); + const [dismissed, setDismissed] = useState(() => { + try { + return window.localStorage.getItem(MUSL_DISMISS_KEY) === "1"; + } catch { + return false; + } + }); + + const handleDismiss = () => { + setDismissed(true); + try { + window.localStorage.setItem(MUSL_DISMISS_KEY, "1"); + } catch { + /* ignore */ + } + }; + + const handleBrowse = () => { + if (!isValidRepo(input)) return; + setPickerRepo(stripPrefixAndTag(input)); + setPickerOpen(true); + }; + + const handleInstall = async () => { + const spec = input.trim(); + if (!isValidFullSpec(spec)) return; + setInstalling(true); + const full = spec.startsWith("github:") ? spec : `github:${spec}`; + const res = await onInstall(full); + setInstalling(false); + if (res.ok) setInput(""); + }; + + return ( +
+
+ +

{t("github.title")}

+
+ + {!dismissed && ( + + + + {t("github.muslWarning")} + + + + )} + +
+ setInput(e.target.value)} + onKeyDown={(e) => e.key === "Enter" && handleInstall()} + disabled={installing} + /> +
+ + +
+
+ +
+ + + + + + + + + + + + {!packages?.length ? ( + + + + ) : ( + packages.map((pkg) => ( + + + + + + + + )) + )} + +
+ {t("github.columns.repo")} + + {t("github.columns.tag")} + + {t("github.columns.binaries")} + + {t("github.columns.installedAt")} + + {t("table.actions")} +
+ {t("table.empty")} +
{pkg.repo}{pkg.tag}{pkg.binaries?.join(", ")} + {new Date(pkg.installed_at).toLocaleDateString()} + + +
+
+ + setPickerOpen(false)} + onSelect={(tag) => { + setInput(`${pickerRepo}@${tag}`); + setPickerOpen(false); + }} + /> + + setUninstallTarget(null)} + title={t("confirmUninstall.title")} + description={t("confirmUninstall.description", { name: uninstallTarget })} + confirmLabel={t("actions.uninstall")} + variant="destructive" + onConfirm={async () => { + if (uninstallTarget) { + await onUninstall(`github:${uninstallTarget}`); + setUninstallTarget(null); + } + }} + /> +
+ ); +} + +interface PickerProps { + repo: string; + open: boolean; + onClose: () => void; + onSelect: (tag: string) => void; +} + +function GitHubReleasePicker({ repo, open, onClose, onSelect }: PickerProps) { + const { t } = useTranslation("packages"); + const http = useHttp(); + const { data, isFetching } = useQuery({ + queryKey: [...queryKeys.packages.all, "github-releases", repo], + queryFn: () => + http.get<{ releases: ReleaseDTO[] }>( + `/v1/packages/github-releases?repo=${encodeURIComponent(repo)}&limit=10` + ), + enabled: open && !!repo, + staleTime: 10 * 60 * 1000, + }); + + return ( + !o && onClose()}> + + + {t("github.pickerTitle", { repo })} + + {isFetching ? ( +
+ + {t("github.pickerLoading")} +
+ ) : !data?.releases?.length ? ( +
+ {t("github.pickerEmpty", { repo })} +
+ ) : ( +
+ {data.releases.map((rel) => ( + +
+ + ))} + + )} + + + ); +} diff --git a/ui/web/src/pages/packages/hooks/use-packages.ts b/ui/web/src/pages/packages/hooks/use-packages.ts index f0954bda..be1127f4 100644 --- a/ui/web/src/pages/packages/hooks/use-packages.ts +++ b/ui/web/src/pages/packages/hooks/use-packages.ts @@ -10,10 +10,22 @@ export interface PackageInfo { version: string; } +// Viewer-safe projection — mirrors GitHubPackageListEntry on the Go side. +// asset_url / sha256 / asset_name are deliberately stripped from the list +// response and are not exposed to viewer-level callers. +export interface GitHubPackageInfo { + name: string; + repo: string; + tag: string; + binaries: string[]; + installed_at: string; +} + export interface InstalledPackages { system: PackageInfo[] | null; pip: PackageInfo[] | null; npm: PackageInfo[] | null; + github?: GitHubPackageInfo[] | null; } interface InstallResult { diff --git a/ui/web/src/pages/packages/packages-page.tsx b/ui/web/src/pages/packages/packages-page.tsx index bcb61e75..484b7089 100644 --- a/ui/web/src/pages/packages/packages-page.tsx +++ b/ui/web/src/pages/packages/packages-page.tsx @@ -7,6 +7,7 @@ import { Alert, AlertDescription, AlertTitle } from "@/components/ui/alert"; import { Button } from "@/components/ui/button"; import { usePackages, type PackageInfo } from "./hooks/use-packages"; import { usePackageRuntimes } from "./hooks/use-package-runtimes"; +import { GitHubBinariesSection } from "./github-binaries-section"; type ActionStatus = "idle" | "loading" | "success" | "error"; @@ -103,6 +104,12 @@ export function PackagesPage() { onInstall={(pkg) => installPackage(`npm:${pkg}`, t)} onUninstall={(pkg) => uninstallPackage(`npm:${pkg}`, t)} /> + + installPackage(pkg, t)} + onUninstall={(pkg) => uninstallPackage(pkg, t)} + /> ); }