diff --git a/docs/14-skills-runtime.md b/docs/14-skills-runtime.md index b0132459..c41f4c44 100644 --- a/docs/14-skills-runtime.md +++ b/docs/14-skills-runtime.md @@ -1,6 +1,6 @@ # 14 - Skills Runtime Environment -How skills access Python, Node.js, and system tools inside the Docker container. Covers image variants, pre-installed packages, runtime installation, and security constraints. +How skills access Python, Node.js, and system tools inside Docker containers and bare-metal gateway deployments. Covers image variants, pre-installed packages, runtime installation, and security constraints. --- @@ -90,6 +90,24 @@ PATH=/app/data/.runtime/npm-global/bin:/app/data/.runtime/pip/bin:$PATH 2. **Node.js**: `npm install -g ` installs to `/app/data/.runtime/npm-global/`. `NODE_PATH` includes both system globals (`/usr/local/lib/node_modules`) and runtime globals. 3. **Persistence**: Packages installed at runtime persist across tool calls within the same container lifecycle (volume-backed). +### Bare-Metal Ubuntu/Debian + +When the gateway runs directly on Ubuntu/Debian instead of inside the Alpine Docker image: + +1. `pip:` still runs `pip3 install --break-system-packages `. +2. `npm:` runs `npm install -g ` with a GoClaw-owned prefix at `{runtimeDir}/npm-global` instead of `/usr/lib/node_modules`. +3. Bare system package names use `sudo -n apt-get install -y --no-install-recommends `. +4. Compatibility aliases: `pip3` installs `python3-pip`; `github-cli` installs `gh`. +5. Installed apt packages are recorded in `{runtimeDir}/system-packages.json` so the System Packages table can show the user-facing name (`github-cli`) while checking the real apt package (`gh`). +6. `/tmp/pkg.sock` is Docker/Alpine-only and is not required on bare-metal Ubuntu/Debian. + +Default `{runtimeDir}` resolution: + +1. `RUNTIME_DIR`, when set. +2. `GOCLAW_DATA_DIR/.runtime`, when `GOCLAW_DATA_DIR` is set. +3. `/var/lib/goclaw/data/.runtime` on bare-metal Linux. +4. `/app/data/.runtime` in Docker-style runtime. + ### Agent Guidance The system prompt and UI should treat runtime availability as variant-dependent: @@ -258,7 +276,7 @@ exclude_deps: # filter false positives from auto-scan; ignored when deps: is | Prefix | Effect | Example | |--------|--------|---------| | `pip:` | Python pip install | `pip:psycopg2-binary`, `pip:requests>=2.31` | -| `npm:` | Global npm install | `npm:typescript` | +| `npm:` | Global npm install under GoClaw runtime prefix | `npm:typescript`, `npm:@aiagentwiki/cli` | | `github:` | GitHub Releases installer (admin) | `github:cli/cli@v2.40.0` | | `system:` | apk package via pkg-helper | `system:ffmpeg` | | (bare) | Treated as system binary | `pandoc` | diff --git a/docs/packages-apk.md b/docs/packages-apk.md index 8d86cdda..73b57019 100644 --- a/docs/packages-apk.md +++ b/docs/packages-apk.md @@ -202,6 +202,10 @@ HTTP status mapping (via `packages_updates.go`): `/app/pkg-helper` is not running, or `/tmp/pkg.sock` does not exist. +For Docker Alpine deployments this is an error. For bare-metal Ubuntu/Debian +deployments, `/tmp/pkg.sock` is expected to be absent; package install should +use the apt path instead of apk/pkg-helper. + 1. Check container logs: `docker logs 2>&1 | grep pkg-helper` 2. Verify the binary exists: `docker exec ls -la /app/pkg-helper` 3. If missing, the Docker image was NOT rebuilt after the pkg-helper v2 upgrade. @@ -209,6 +213,26 @@ HTTP status mapping (via `packages_updates.go`): 4. If the binary exists but the socket is missing, check that the container entrypoint starts the helper before the gateway: `ENTRYPOINT ["/app/entrypoint.sh"]`. +### Bare-metal Ubuntu/Debian package table + +On bare-metal Ubuntu/Debian, system package install does not write the Alpine +`apk-packages` persist file. GoClaw records successful apt installs in +`{runtimeDir}/system-packages.json` and lists versions via `dpkg-query`. + +Alias examples: + +- Installing `pip3` records display name `pip3`, apt package `python3-pip`. +- Installing `github-cli` records display name `github-cli`, apt package `gh`. + +The System Packages table should show the display name users installed, not the +underlying Debian package alias. + +### Bare-metal npm global prefix + +On bare-metal Ubuntu/Debian, Node packages installed from the Packages page use +`{runtimeDir}/npm-global` as `NPM_CONFIG_PREFIX`. This avoids writing to +`/usr/lib/node_modules`, which is root-owned on standard Ubuntu installs. + Logging: the gateway emits `slog.Info("package.update.apk.unavailable")` when the helper socket is unreachable. Grep for this key to confirm the symptom. diff --git a/internal/skills/dep_checker.go b/internal/skills/dep_checker.go index 49564137..2ba9df20 100644 --- a/internal/skills/dep_checker.go +++ b/internal/skills/dep_checker.go @@ -19,6 +19,7 @@ func CheckSkillDeps(m *SkillManifest) (bool, []string) { if m == nil || m.IsEmpty() { return true, nil } + ensureNpmGlobalEnv() var missing []string @@ -121,6 +122,7 @@ func checkNodePackages(packages []string, scriptsDir string) []string { defer cancel() cmd := exec.CommandContext(ctx, "node", "-e", sb.String()) + cmd.Env = npmCommandEnv() if scriptsDir != "" { cmd.Dir = scriptsDir } diff --git a/internal/skills/dep_installer.go b/internal/skills/dep_installer.go index afda08e0..1b31041e 100644 --- a/internal/skills/dep_installer.go +++ b/internal/skills/dep_installer.go @@ -119,7 +119,12 @@ func InstallSingleDep(ctx context.Context, dep string) (bool, string) { } defer release() } - cmd := exec.CommandContext(ctx, "npm", "install", "-g", pkg) + if err := os.MkdirAll(npmGlobalPrefix(), 0o750); err != nil { + return false, fmt.Sprintf("npm prefix setup: %v", err) + } + ensureNpmGlobalEnv() + cmd := exec.CommandContext(ctx, npmBinary, "install", "-g", pkg) + cmd.Env = npmCommandEnv() out, err := cmd.CombinedOutput() if err != nil { msg := fmt.Sprintf("%s: %v", strings.TrimSpace(string(out)), err) @@ -127,9 +132,7 @@ func InstallSingleDep(ctx context.Context, dep string) (bool, string) { return false, msg } default: - // System package via pkg-helper (root-privileged Unix socket). - // pkg-helper handles persist to apk-packages file. - ok, errMsg := apkViaHelper(ctx, "install", dep) + ok, errMsg := installSystemPackage(ctx, dep) if !ok { return false, errMsg } @@ -165,9 +168,9 @@ func InstallDeps(ctx context.Context, manifest *SkillManifest, missing []string) slog.Info("skills: installing system packages", "pkgs", sysPkgs) var successful []string for _, pkg := range sysPkgs { - ok, errMsg := apkViaHelper(ctx, "install", pkg) + ok, errMsg := installSystemPackage(ctx, pkg) if !ok { - result.Errors = append(result.Errors, fmt.Sprintf("apk %s: %s", pkg, errMsg)) + result.Errors = append(result.Errors, fmt.Sprintf("system %s: %s", pkg, errMsg)) } else { successful = append(successful, pkg) } @@ -196,9 +199,16 @@ func InstallDeps(ctx context.Context, manifest *SkillManifest, missing []string) // Npm packages: install one by one for partial-success resilience. if len(npmPkgs) > 0 { slog.Info("skills: installing npm packages", "pkgs", npmPkgs) + if err := os.MkdirAll(npmGlobalPrefix(), 0o750); err != nil { + result.Errors = append(result.Errors, fmt.Sprintf("npm prefix setup: %v", err)) + cleanCaches(ctx) + return result, nil + } + ensureNpmGlobalEnv() var successful []string for _, pkg := range npmPkgs { - cmd := exec.CommandContext(ctx, "npm", "install", "-g", pkg) + cmd := exec.CommandContext(ctx, npmBinary, "install", "-g", pkg) + cmd.Env = npmCommandEnv() if out, err := cmd.CombinedOutput(); err != nil { result.Errors = append(result.Errors, fmt.Sprintf("npm %s: %s (%v)", pkg, strings.TrimSpace(string(out)), err)) } else { @@ -266,7 +276,9 @@ func UninstallPackage(ctx context.Context, dep string) (bool, string) { } case strings.HasPrefix(dep, "npm:"): pkg := strings.TrimPrefix(dep, "npm:") - cmd := exec.CommandContext(ctx, "npm", "uninstall", "-g", pkg) + ensureNpmGlobalEnv() + cmd := exec.CommandContext(ctx, npmBinary, "uninstall", "-g", pkg) + cmd.Env = npmCommandEnv() out, err := cmd.CombinedOutput() if err != nil { msg := fmt.Sprintf("%s: %v", strings.TrimSpace(string(out)), err) @@ -274,8 +286,7 @@ func UninstallPackage(ctx context.Context, dep string) (bool, string) { return false, msg } default: - // System package via pkg-helper. Helper handles persist file removal. - ok, errMsg := apkViaHelper(ctx, "uninstall", dep) + ok, errMsg := uninstallSystemPackage(ctx, dep) if !ok { return false, errMsg } diff --git a/internal/skills/dep_installer_test.go b/internal/skills/dep_installer_test.go index 6bf49294..10dd17bd 100644 --- a/internal/skills/dep_installer_test.go +++ b/internal/skills/dep_installer_test.go @@ -2,6 +2,8 @@ package skills import ( "context" + "os" + "path/filepath" "sync" "sync/atomic" "testing" @@ -19,6 +21,24 @@ func TestSharedPackageLocker_NilPath(t *testing.T) { } } +func TestInstallSingleDepNpmUsesWritableRuntimePrefix(t *testing.T) { + runtimeDir := t.TempDir() + t.Setenv("RUNTIME_DIR", runtimeDir) + t.Setenv("NPM_CONFIG_PREFIX", "") + t.Setenv("FIXTURE_NPM_EXIT", "0") + useFixtureNpm(t) + + ok, msg := InstallSingleDep(context.Background(), "npm:@aiagentwiki/cli") + if !ok { + t.Fatalf("InstallSingleDep failed: %s", msg) + } + + wantPrefix := filepath.Join(runtimeDir, "npm-global") + if _, err := os.Stat(wantPrefix); err != nil { + t.Fatalf("npm prefix %q was not created: %v", wantPrefix, err) + } +} + // TestSetSharedPackageLocker_InjectsAndReturns verifies that // SetSharedPackageLocker stores the locker and sharedPackageLocker retrieves it. func TestSetSharedPackageLocker_InjectsAndReturns(t *testing.T) { diff --git a/internal/skills/npm_update_checker.go b/internal/skills/npm_update_checker.go index b7eaf0f7..0a8eb082 100644 --- a/internal/skills/npm_update_checker.go +++ b/internal/skills/npm_update_checker.go @@ -57,11 +57,13 @@ func (c *NpmUpdateChecker) Check(ctx context.Context, knownETags map[string]stri slog.Info("package.update.npm.unavailable", "reason", "npm not found") return UpdateCheckResult{Source: "npm", Available: false} } + ensureNpmGlobalEnv() cctx, cancel := context.WithTimeout(ctx, 30*time.Second) defer cancel() cmd := exec.CommandContext(cctx, npmBinary, "outdated", "--global", "--json") + cmd.Env = npmCommandEnv() cmd.WaitDelay = 2 * time.Second var stdout, stderr bytes.Buffer cmd.Stdout = &stdout diff --git a/internal/skills/npm_update_executor.go b/internal/skills/npm_update_executor.go index 13cd5df5..4e51d097 100644 --- a/internal/skills/npm_update_executor.go +++ b/internal/skills/npm_update_executor.go @@ -5,6 +5,7 @@ import ( "context" "fmt" "log/slog" + "os" "os/exec" "time" ) @@ -46,7 +47,12 @@ func (e *NpmUpdateExecutor) Update(ctx context.Context, name, toVersion string, // suffixes, so the only "@" in the token is our version separator. target := name + "@" + toVersion + if err := os.MkdirAll(npmGlobalPrefix(), 0o750); err != nil { + return fmt.Errorf("npm prefix setup: %w", err) + } + ensureNpmGlobalEnv() cmd := exec.CommandContext(cctx, npmBinary, "install", "--global", target) + cmd.Env = npmCommandEnv() cmd.WaitDelay = 2 * time.Second var stdout, stderr bytes.Buffer cmd.Stdout = &stdout diff --git a/internal/skills/package_lister.go b/internal/skills/package_lister.go index e34f627e..78f614c1 100644 --- a/internal/skills/package_lister.go +++ b/internal/skills/package_lister.go @@ -11,6 +11,10 @@ import ( "time" ) +var packageListCommandCombinedOutput = func(ctx context.Context, name string, args ...string) ([]byte, error) { + return exec.CommandContext(ctx, name, args...).CombinedOutput() +} + // PackageInfo describes a single installed package. type PackageInfo struct { Name string `json:"name"` @@ -40,14 +44,14 @@ type InstalledPackages struct { const listTimeout = 15 * time.Second -// ListInstalledPackages queries apk, pip3, and npm for installed packages. -// Only returns user-installed packages (filters out base Alpine packages for system). +// ListInstalledPackages queries system, pip3, and npm for installed packages. +// System packages are limited to packages installed through GoClaw. func ListInstalledPackages(ctx context.Context) *InstalledPackages { ctx, cancel := context.WithTimeout(ctx, listTimeout) defer cancel() result := &InstalledPackages{} - result.System = listApkUserPackages(ctx) + result.System = listSystemUserPackages(ctx) result.Pip = listPipPackages(ctx) result.Npm = listNpmPackages(ctx) if gh := DefaultGitHubInstaller(); gh != nil { @@ -67,14 +71,17 @@ func ListInstalledPackages(ctx context.Context) *InstalledPackages { return result } +func listSystemUserPackages(ctx context.Context) []PackageInfo { + if IsAlpineRuntime() { + return listApkUserPackages(ctx) + } + return listDebianUserPackages(ctx) +} + // listApkUserPackages returns packages from the apk-packages persist file // (user-installed on-demand packages only, not base Alpine). func listApkUserPackages(ctx context.Context) []PackageInfo { - runtimeDir := os.Getenv("RUNTIME_DIR") - if runtimeDir == "" { - runtimeDir = "/app/data/.runtime" - } - listFile := filepath.Join(runtimeDir, "apk-packages") + listFile := filepath.Join(packageRuntimeDir(), "apk-packages") f, err := os.Open(listFile) if err != nil { @@ -111,7 +118,7 @@ func listApkUserPackages(ctx context.Context) []PackageInfo { // Uses "apk list --installed" which works without root and gives versioned output. func getApkVersion(ctx context.Context, name string) string { // Output format: "github-cli-2.72.0-r6 aarch64 {github-cli} (MIT) [installed]" - out, err := exec.CommandContext(ctx, "apk", "list", "--installed", name).Output() + out, err := packageListCommandCombinedOutput(ctx, "apk", "list", "--installed", name) if err != nil { return "" } @@ -134,7 +141,7 @@ func getApkVersion(ctx context.Context, name string) string { // listPipPackages returns pip3-installed packages via JSON output. func listPipPackages(ctx context.Context) []PackageInfo { - out, err := exec.CommandContext(ctx, "pip3", "list", "--format", "json").CombinedOutput() + out, err := packageListCommandCombinedOutput(ctx, "pip3", "list", "--format", "json") if err != nil { return nil } @@ -156,7 +163,10 @@ func listPipPackages(ctx context.Context) []PackageInfo { // listNpmPackages returns globally installed npm packages. func listNpmPackages(ctx context.Context) []PackageInfo { - out, err := exec.CommandContext(ctx, "npm", "list", "-g", "--json", "--depth=0").CombinedOutput() + ensureNpmGlobalEnv() + cmd := exec.CommandContext(ctx, npmBinary, "list", "-g", "--json", "--depth=0") + cmd.Env = npmCommandEnv() + out, err := cmd.CombinedOutput() if err != nil && len(out) == 0 { return nil } @@ -176,3 +186,35 @@ func listNpmPackages(ctx context.Context) []PackageInfo { } return pkgs } + +func listDebianUserPackages(ctx context.Context) []PackageInfo { + records, err := readSystemPackageRecords() + if err != nil || len(records) == 0 { + return nil + } + + pkgs := make([]PackageInfo, 0, len(records)) + for _, record := range records { + if record.Manager != "apt" || record.Package == "" { + continue + } + version := getDebianPackageVersion(ctx, record.Package) + if version == "" { + continue + } + name := record.Name + if name == "" { + name = record.Package + } + pkgs = append(pkgs, PackageInfo{Name: name, Version: version}) + } + return pkgs +} + +func getDebianPackageVersion(ctx context.Context, name string) string { + out, err := packageListCommandCombinedOutput(ctx, "dpkg-query", "-W", "-f=${Version}", name) + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} diff --git a/internal/skills/package_lister_test.go b/internal/skills/package_lister_test.go index b850c555..622357bd 100644 --- a/internal/skills/package_lister_test.go +++ b/internal/skills/package_lister_test.go @@ -3,8 +3,10 @@ package skills import ( "context" "encoding/json" + "errors" "os" "path/filepath" + "reflect" "strings" "testing" "time" @@ -412,6 +414,37 @@ func TestListInstalledPackages_HasThreeCategories(t *testing.T) { } } +func TestListDebianUserPackagesShowsRequestedAliasName(t *testing.T) { + t.Setenv("RUNTIME_DIR", t.TempDir()) + overrideAlpineRuntime(false) + t.Cleanup(func() { overrideAlpineRuntime(false) }) + + if err := addSystemPackageRecord("github-cli", "gh", "apt"); err != nil { + t.Fatalf("addSystemPackageRecord: %v", err) + } + + orig := packageListCommandCombinedOutput + packageListCommandCombinedOutput = func(_ context.Context, name string, args ...string) ([]byte, error) { + if name != "dpkg-query" { + return nil, errors.New("unexpected command") + } + wantArgs := []string{"-W", "-f=${Version}", "gh"} + if !reflect.DeepEqual(args, wantArgs) { + t.Fatalf("args = %#v, want %#v", args, wantArgs) + } + return []byte("2.72.0-1\n"), nil + } + t.Cleanup(func() { packageListCommandCombinedOutput = orig }) + + got := listDebianUserPackages(context.Background()) + if len(got) != 1 { + t.Fatalf("got %d packages, want 1: %#v", len(got), got) + } + if got[0].Name != "github-cli" || got[0].Version != "2.72.0-1" { + t.Fatalf("package = %#v, want github-cli 2.72.0-1", got[0]) + } +} + // TestListInstalledPackages_ContextTimeout tests timeout behavior. func TestListInstalledPackages_ContextTimeout(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond) diff --git a/internal/skills/package_runtime_paths.go b/internal/skills/package_runtime_paths.go new file mode 100644 index 00000000..7ecd91bb --- /dev/null +++ b/internal/skills/package_runtime_paths.go @@ -0,0 +1,94 @@ +package skills + +import ( + "os" + "path/filepath" + "runtime" + "strings" +) + +func packageRuntimeDir() string { + if v := strings.TrimSpace(os.Getenv("RUNTIME_DIR")); v != "" { + return v + } + if v := strings.TrimSpace(os.Getenv("GOCLAW_DATA_DIR")); v != "" { + return filepath.Join(v, ".runtime") + } + if runtime.GOOS != "windows" && !IsAlpineRuntime() { + return "/var/lib/goclaw/data/.runtime" + } + return filepath.Join("/app/data", ".runtime") +} + +func npmGlobalPrefix() string { + if v := strings.TrimSpace(os.Getenv("NPM_CONFIG_PREFIX")); v != "" { + return v + } + return filepath.Join(packageRuntimeDir(), "npm-global") +} + +func npmGlobalBinDir() string { + if runtime.GOOS == "windows" { + return npmGlobalPrefix() + } + return filepath.Join(npmGlobalPrefix(), "bin") +} + +func npmGlobalNodePath() string { + return filepath.Join(npmGlobalPrefix(), "lib", "node_modules") +} + +func ensureNpmGlobalEnv() { + prependProcessPath(npmGlobalBinDir()) +} + +func npmCommandEnv() []string { + prefix := npmGlobalPrefix() + binDir := npmGlobalBinDir() + nodePath := npmGlobalNodePath() + + env := make([]string, 0, len(os.Environ())+3) + for _, e := range os.Environ() { + switch { + case strings.HasPrefix(e, "NPM_CONFIG_PREFIX="): + continue + case strings.HasPrefix(e, "PATH="): + continue + case strings.HasPrefix(e, "NODE_PATH="): + continue + } + env = append(env, e) + } + + pathValue := prependPathValue(os.Getenv("PATH"), binDir) + nodePathValue := prependPathValue(os.Getenv("NODE_PATH"), nodePath) + env = append(env, + "NPM_CONFIG_PREFIX="+prefix, + "PATH="+pathValue, + "NODE_PATH="+nodePathValue, + ) + return env +} + +func prependProcessPath(dir string) { + if strings.TrimSpace(dir) == "" { + return + } + _ = os.Setenv("PATH", prependPathValue(os.Getenv("PATH"), dir)) +} + +func prependPathValue(current, dir string) string { + if strings.TrimSpace(dir) == "" { + return current + } + parts := filepath.SplitList(current) + for _, p := range parts { + if p == dir { + return current + } + } + if current == "" { + return dir + } + return dir + string(os.PathListSeparator) + current +} diff --git a/internal/skills/package_runtime_paths_test.go b/internal/skills/package_runtime_paths_test.go new file mode 100644 index 00000000..45ea7342 --- /dev/null +++ b/internal/skills/package_runtime_paths_test.go @@ -0,0 +1,63 @@ +package skills + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestNpmCommandEnvUsesRuntimePrefix(t *testing.T) { + runtimeDir := t.TempDir() + t.Setenv("RUNTIME_DIR", runtimeDir) + t.Setenv("NPM_CONFIG_PREFIX", "") + t.Setenv("NODE_PATH", "") + t.Setenv("PATH", "/usr/bin") + + env := npmCommandEnv() + wantPrefix := filepath.Join(runtimeDir, "npm-global") + wantBin := npmGlobalBinDir() + wantNodePath := filepath.Join(wantPrefix, "lib", "node_modules") + + if !envContainsExact(env, "NPM_CONFIG_PREFIX="+wantPrefix) { + t.Fatalf("npmCommandEnv missing NPM_CONFIG_PREFIX=%q", wantPrefix) + } + if !envContainsPrefixValue(env, "PATH=", wantBin) { + t.Fatalf("npmCommandEnv PATH does not start with %q", wantBin) + } + if !envContainsPrefixValue(env, "NODE_PATH=", wantNodePath) { + t.Fatalf("npmCommandEnv NODE_PATH does not start with %q", wantNodePath) + } +} + +func TestEnsureNpmGlobalEnvPrependsProcessPath(t *testing.T) { + runtimeDir := t.TempDir() + t.Setenv("RUNTIME_DIR", runtimeDir) + t.Setenv("NPM_CONFIG_PREFIX", "") + t.Setenv("PATH", "/usr/bin") + + ensureNpmGlobalEnv() + + wantBin := npmGlobalBinDir() + if got := os.Getenv("PATH"); !strings.HasPrefix(got, wantBin+string(os.PathListSeparator)) { + t.Fatalf("PATH = %q, want prefix %q", got, wantBin) + } +} + +func envContainsExact(env []string, want string) bool { + for _, item := range env { + if item == want { + return true + } + } + return false +} + +func envContainsPrefixValue(env []string, key, wantPrefix string) bool { + for _, item := range env { + if strings.HasPrefix(item, key) { + return strings.HasPrefix(strings.TrimPrefix(item, key), wantPrefix) + } + } + return false +} diff --git a/internal/skills/system_package_installer.go b/internal/skills/system_package_installer.go new file mode 100644 index 00000000..7af05d69 --- /dev/null +++ b/internal/skills/system_package_installer.go @@ -0,0 +1,96 @@ +package skills + +import ( + "context" + "fmt" + "log/slog" + "os/exec" + "regexp" + "strings" +) + +var ( + debPackageNameRE = regexp.MustCompile(`^[a-z0-9][a-z0-9+.-]*$`) + systemLookPath = exec.LookPath + systemCommandCombinedOutput = runSystemCommandCombinedOutput + aptSystemPackageAliases = map[string]string{"pip3": "python3-pip", "github-cli": "gh"} + errSystemPackageMgrUnavailable = "system package manager unavailable on this runtime" +) + +func runSystemCommandCombinedOutput(ctx context.Context, name string, args ...string) ([]byte, error) { + return exec.CommandContext(ctx, name, args...).CombinedOutput() +} + +func installSystemPackage(ctx context.Context, requested string) (bool, string) { + if IsAlpineRuntime() { + return apkViaHelper(ctx, "install", requested) + } + pkg, err := resolveDebianPackageName(requested) + if err != nil { + return false, err.Error() + } + if _, err := systemLookPath("apt-get"); err != nil { + return false, errSystemPackageMgrUnavailable + } + if ok, msg := runAptCommand(ctx, "install", pkg); !ok { + return false, msg + } + if err := addSystemPackageRecord(requested, pkg, "apt"); err != nil { + slog.Warn("skills: system package record add failed", "package", requested, "resolved", pkg, "error", err) + } + return true, "" +} + +func uninstallSystemPackage(ctx context.Context, requested string) (bool, string) { + if IsAlpineRuntime() { + return apkViaHelper(ctx, "uninstall", requested) + } + pkg, err := resolveDebianPackageName(requested) + if err != nil { + return false, err.Error() + } + if _, err := systemLookPath("apt-get"); err != nil { + return false, errSystemPackageMgrUnavailable + } + if ok, msg := runAptCommand(ctx, "remove", pkg); !ok { + return false, msg + } + if err := removeSystemPackageRecord(requested, pkg, "apt"); err != nil { + slog.Warn("skills: system package record remove failed", "package", requested, "resolved", pkg, "error", err) + } + return true, "" +} + +func resolveDebianPackageName(requested string) (string, error) { + pkg := strings.ToLower(strings.TrimSpace(requested)) + if alias, ok := aptSystemPackageAliases[pkg]; ok { + pkg = alias + } + if !debPackageNameRE.MatchString(pkg) { + return "", fmt.Errorf("invalid Debian package name: %s", requested) + } + return pkg, nil +} + +func runAptCommand(ctx context.Context, action, pkg string) (bool, string) { + args := []string{"-n", "env", "DEBIAN_FRONTEND=noninteractive", "apt-get"} + switch action { + case "install": + args = append(args, "install", "-y", "--no-install-recommends", pkg) + case "remove": + args = append(args, "remove", "-y", pkg) + default: + return false, "unsupported apt action" + } + out, err := systemCommandCombinedOutput(ctx, "sudo", args...) + if err != nil { + msg := strings.TrimSpace(string(out)) + if msg == "" { + msg = err.Error() + } else { + msg = fmt.Sprintf("%s: %v", msg, err) + } + return false, msg + } + return true, "" +} diff --git a/internal/skills/system_package_installer_test.go b/internal/skills/system_package_installer_test.go new file mode 100644 index 00000000..cab7e24e --- /dev/null +++ b/internal/skills/system_package_installer_test.go @@ -0,0 +1,112 @@ +package skills + +import ( + "context" + "errors" + "reflect" + "strings" + "testing" +) + +func withSystemPackageTestHooks(t *testing.T, alpine bool, lookPathErr error, run func(context.Context, string, ...string) ([]byte, error)) { + t.Helper() + t.Setenv("RUNTIME_DIR", t.TempDir()) + overrideAlpineRuntime(alpine) + origLookPath := systemLookPath + origRun := systemCommandCombinedOutput + systemLookPath = func(file string) (string, error) { + if lookPathErr != nil { + return "", lookPathErr + } + return "/usr/bin/" + file, nil + } + systemCommandCombinedOutput = run + t.Cleanup(func() { + systemLookPath = origLookPath + systemCommandCombinedOutput = origRun + overrideAlpineRuntime(false) + }) +} + +func TestResolveDebianPackageNameAliases(t *testing.T) { + tests := map[string]string{ + "pip3": "python3-pip", + "github-cli": "gh", + "ripgrep": "ripgrep", + "libstdc++": "libstdc++", + } + for input, want := range tests { + got, err := resolveDebianPackageName(input) + if err != nil { + t.Fatalf("resolveDebianPackageName(%q): %v", input, err) + } + if got != want { + t.Fatalf("resolveDebianPackageName(%q) = %q, want %q", input, got, want) + } + } +} + +func TestResolveDebianPackageNameRejectsUnsafeNames(t *testing.T) { + for _, input := range []string{"", "../curl", "pkg/name", "@scope/pkg", "curl;reboot", "-flag"} { + if got, err := resolveDebianPackageName(input); err == nil { + t.Fatalf("resolveDebianPackageName(%q) = %q, want error", input, got) + } + } +} + +func TestInstallSystemPackageUsesAptOnNonAlpine(t *testing.T) { + var gotName string + var gotArgs []string + withSystemPackageTestHooks(t, false, nil, func(_ context.Context, name string, args ...string) ([]byte, error) { + gotName = name + gotArgs = append([]string(nil), args...) + return nil, nil + }) + + ok, msg := installSystemPackage(context.Background(), "pip3") + + if !ok || msg != "" { + t.Fatalf("installSystemPackage failed: ok=%v msg=%q", ok, msg) + } + if gotName != "sudo" { + t.Fatalf("command = %q, want sudo", gotName) + } + wantArgs := []string{"-n", "env", "DEBIAN_FRONTEND=noninteractive", "apt-get", "install", "-y", "--no-install-recommends", "python3-pip"} + if !reflect.DeepEqual(gotArgs, wantArgs) { + t.Fatalf("args = %#v, want %#v", gotArgs, wantArgs) + } +} + +func TestUninstallSystemPackageUsesAptRemoveOnNonAlpine(t *testing.T) { + var gotArgs []string + withSystemPackageTestHooks(t, false, nil, func(_ context.Context, _ string, args ...string) ([]byte, error) { + gotArgs = append([]string(nil), args...) + return nil, nil + }) + + ok, msg := uninstallSystemPackage(context.Background(), "github-cli") + + if !ok || msg != "" { + t.Fatalf("uninstallSystemPackage failed: ok=%v msg=%q", ok, msg) + } + wantArgs := []string{"-n", "env", "DEBIAN_FRONTEND=noninteractive", "apt-get", "remove", "-y", "gh"} + if !reflect.DeepEqual(gotArgs, wantArgs) { + t.Fatalf("args = %#v, want %#v", gotArgs, wantArgs) + } +} + +func TestInstallSystemPackageReportsMissingApt(t *testing.T) { + withSystemPackageTestHooks(t, false, errors.New("missing"), func(context.Context, string, ...string) ([]byte, error) { + t.Fatal("command should not run") + return nil, nil + }) + + ok, msg := installSystemPackage(context.Background(), "ripgrep") + + if ok { + t.Fatal("installSystemPackage succeeded, want failure") + } + if !strings.Contains(msg, errSystemPackageMgrUnavailable) { + t.Fatalf("msg = %q, want unavailable", msg) + } +} diff --git a/internal/skills/system_package_records.go b/internal/skills/system_package_records.go new file mode 100644 index 00000000..f61e2329 --- /dev/null +++ b/internal/skills/system_package_records.go @@ -0,0 +1,96 @@ +package skills + +import ( + "encoding/json" + "errors" + "os" + "path/filepath" + "strings" +) + +type systemPackageRecord struct { + Name string `json:"name"` + Package string `json:"package"` + Manager string `json:"manager"` +} + +func systemPackageRecordsPath() string { + return filepath.Join(packageRuntimeDir(), "system-packages.json") +} + +func addSystemPackageRecord(requested, resolved, manager string) error { + record := systemPackageRecord{ + Name: normalizeSystemPackageDisplayName(requested), + Package: strings.ToLower(strings.TrimSpace(resolved)), + Manager: strings.ToLower(strings.TrimSpace(manager)), + } + if record.Name == "" || record.Package == "" || record.Manager == "" { + return nil + } + + records, err := readSystemPackageRecords() + if err != nil { + return err + } + for i, existing := range records { + if existing.Manager == record.Manager && (existing.Name == record.Name || existing.Package == record.Package) { + records[i] = record + return writeSystemPackageRecords(records) + } + } + records = append(records, record) + return writeSystemPackageRecords(records) +} + +func removeSystemPackageRecord(requested, resolved, manager string) error { + wantName := normalizeSystemPackageDisplayName(requested) + wantPackage := strings.ToLower(strings.TrimSpace(resolved)) + wantManager := strings.ToLower(strings.TrimSpace(manager)) + + records, err := readSystemPackageRecords() + if err != nil { + return err + } + filtered := records[:0] + for _, record := range records { + if record.Manager == wantManager && (record.Name == wantName || record.Package == wantPackage) { + continue + } + filtered = append(filtered, record) + } + return writeSystemPackageRecords(filtered) +} + +func readSystemPackageRecords() ([]systemPackageRecord, error) { + data, err := os.ReadFile(systemPackageRecordsPath()) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return nil, nil + } + return nil, err + } + if len(strings.TrimSpace(string(data))) == 0 { + return nil, nil + } + var records []systemPackageRecord + if err := json.Unmarshal(data, &records); err != nil { + return nil, err + } + return records, nil +} + +func writeSystemPackageRecords(records []systemPackageRecord) error { + path := systemPackageRecordsPath() + if err := os.MkdirAll(filepath.Dir(path), 0o750); err != nil { + return err + } + data, err := json.MarshalIndent(records, "", " ") + if err != nil { + return err + } + return os.WriteFile(path, append(data, '\n'), 0o600) +} + +func normalizeSystemPackageDisplayName(name string) string { + return strings.ToLower(strings.TrimSpace(name)) +}