diff --git a/internal/skills/apk_helper_call_test.go b/internal/skills/apk_helper_call_test.go index 67da4eab..00f131b7 100644 --- a/internal/skills/apk_helper_call_test.go +++ b/internal/skills/apk_helper_call_test.go @@ -6,6 +6,8 @@ import ( "encoding/json" "fmt" "net" + "os" + "path/filepath" "runtime" "strings" "sync/atomic" @@ -132,6 +134,108 @@ func TestApkHelperCall_DialFail(t *testing.T) { } } +func TestApkHelperCallFallback_ParsesStdoutJSONWithStderrLogs(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell fixture requires Unix") + } + + helper := writePkgHelperFixture(t, ` +echo 'time=2026-06-15 level=INFO msg="pkg-helper: installing"' >&2 +printf '%s\n' '{"ok":true,"data":"installed"}' +`) + + ok, code, data, errMsg := apkHelperCallFallback(context.Background(), helper, "install", "curl") + + if !ok { + t.Fatalf("ok = false, want true (code=%q err=%q)", code, errMsg) + } + if code != "" { + t.Errorf("code = %q, want empty", code) + } + if data != "installed" { + t.Errorf("data = %q, want %q", data, "installed") + } + if errMsg != "" { + t.Errorf("errMsg = %q, want empty", errMsg) + } +} + +func TestApkHelperCallFallback_ParsesErrorJSONDespiteExitStatus(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell fixture requires Unix") + } + + helper := writePkgHelperFixture(t, ` +echo 'time=2026-06-15 level=ERROR msg="pkg-helper: install failed"' >&2 +printf '%s\n' '{"ok":false,"error":"package not found","code":"not_found"}' +exit 1 +`) + + ok, code, _, errMsg := apkHelperCallFallback(context.Background(), helper, "install", "missing") + + if ok { + t.Fatal("ok = true, want false") + } + if code != "not_found" { + t.Errorf("code = %q, want %q", code, "not_found") + } + if errMsg != "package not found" { + t.Errorf("errMsg = %q, want %q", errMsg, "package not found") + } +} + +func TestApkHelperCallFallback_InvalidStdoutIsHelperError(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell fixture requires Unix") + } + + helper := writePkgHelperFixture(t, ` +printf '%s\n' 'not-json' +`) + + ok, code, _, errMsg := apkHelperCallFallback(context.Background(), helper, "install", "curl") + + if ok { + t.Fatal("ok = true, want false for invalid helper response") + } + if code != "helper_error" { + t.Errorf("code = %q, want %q", code, "helper_error") + } + if !strings.Contains(errMsg, "invalid response") || !strings.Contains(errMsg, "stdout: not-json") { + t.Errorf("errMsg = %q, want invalid response with stdout detail", errMsg) + } +} + +func TestFirstExecutableFileFindsBundledFallbackCandidate(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("executable-bit fixture requires Unix") + } + + helper := writePkgHelperFixture(t, `exit 0`) + got, ok := firstExecutableFile([]string{ + filepath.Join(t.TempDir(), "missing-helper"), + helper, + }) + + if !ok { + t.Fatal("firstExecutableFile did not find executable fallback candidate") + } + if got != helper { + t.Fatalf("firstExecutableFile = %q, want %q", got, helper) + } +} + +func writePkgHelperFixture(t *testing.T, body string) string { + t.Helper() + + path := filepath.Join(t.TempDir(), "pkg-helper") + script := "#!/bin/sh\n" + strings.TrimLeft(body, "\n") + if err := os.WriteFile(path, []byte(script), 0o755); err != nil { + t.Fatalf("write helper fixture: %v", err) + } + return path +} + // TestApkHelperCall_ValidResponse verifies a well-formed canned response is // parsed correctly into (ok, code, data, errMsg). func TestApkHelperCall_ValidResponse(t *testing.T) { diff --git a/internal/skills/dep_installer.go b/internal/skills/dep_installer.go index 7f7bfe71..2588fd1c 100644 --- a/internal/skills/dep_installer.go +++ b/internal/skills/dep_installer.go @@ -2,6 +2,7 @@ package skills import ( "bufio" + "bytes" "context" "encoding/json" "fmt" @@ -39,6 +40,10 @@ const InstallTimeout = 5 * time.Minute // pkgHelperSocket is the Unix socket path for the root-privileged pkg-helper. const pkgHelperSocket = "/tmp/pkg.sock" +// pkgHelperBundledPath is where the Docker image copies the helper binary. +// /app is not part of PATH, so direct-exec fallback must check it explicitly. +const pkgHelperBundledPath = "/app/pkg-helper" + // apkHelperCallFunc is the package-level hook for apkHelperCall, allowing tests // to inject a stub without starting a real Unix socket server. Production code // always uses the default value (apkHelperCall). Tests replace it per-case and @@ -305,33 +310,10 @@ func UninstallPackage(ctx context.Context, dep string) (bool, string) { func apkHelperCall(ctx context.Context, action, pkg string) (ok bool, code, data, errMsg string) { conn, err := net.DialTimeout("unix", pkgHelperSocket, 5*time.Second) if err != nil { - // Fallback: try executing pkg-helper directly if available - if path, lookErr := exec.LookPath("pkg-helper"); lookErr == nil { - cmd := exec.CommandContext(ctx, path, action) - if pkg != "" { - cmd.Args = append(cmd.Args, pkg) - } - out, execErr := cmd.CombinedOutput() - - var resp struct { - OK bool `json:"ok"` - Error string `json:"error"` - Code string `json:"code"` - Data string `json:"data"` - } - if unmarshalErr := json.Unmarshal(out, &resp); unmarshalErr == nil { - if resp.Code == "" && !resp.OK { - resp.Code = "system_error" - } - return resp.OK, resp.Code, resp.Data, resp.Error - } - - if execErr != nil { - return false, "system_error", "", fmt.Sprintf("pkg-helper fallback failed: %v: %s", execErr, strings.TrimSpace(string(out))) - } - return true, "", string(out), "" + if path, found := findPkgHelperBinary(); found { + return apkHelperCallFallback(ctx, path, action, pkg) } - + return false, "helper_unavailable", "", fmt.Sprintf("pkg-helper unavailable: %v", err) } defer conn.Close() @@ -360,22 +342,103 @@ func apkHelperCall(ctx context.Context, action, pkg string) (ok bool, code, data return false, "helper_error", "", "pkg-helper: no response" } - var resp struct { - OK bool `json:"ok"` - Error string `json:"error"` - Code string `json:"code"` - Data string `json:"data"` - } - if err := json.Unmarshal(scanner.Bytes(), &resp); err != nil { + resp, err := parsePkgHelperResponse(scanner.Bytes()) + if err != nil { return false, "helper_error", "", fmt.Sprintf("pkg-helper: invalid response: %v", err) } + return resp.OK, resp.Code, resp.Data, resp.Error +} + +type pkgHelperResponse struct { + OK bool `json:"ok"` + Error string `json:"error"` + Code string `json:"code"` + Data string `json:"data"` +} + +func parsePkgHelperResponse(out []byte) (pkgHelperResponse, error) { + var resp pkgHelperResponse + if err := json.Unmarshal(out, &resp); err != nil { + return pkgHelperResponse{}, err + } // Default missing code to system_error for v1-era helpers that omit the field. if resp.Code == "" && !resp.OK { resp.Code = "system_error" } + return resp, nil +} - return resp.OK, resp.Code, resp.Data, resp.Error +func apkHelperCallFallback(ctx context.Context, helperPath, action, pkg string) (ok bool, code, data, errMsg string) { + cmd := exec.CommandContext(ctx, helperPath, action) + if pkg != "" { + cmd.Args = append(cmd.Args, pkg) + } + + var stderr bytes.Buffer + cmd.Stderr = &stderr + out, execErr := cmd.Output() + + resp, parseErr := parsePkgHelperResponse(out) + if parseErr == nil { + if execErr != nil && resp.OK { + return false, "system_error", "", fmt.Sprintf("pkg-helper fallback exited after successful response: %v", execErr) + } + return resp.OK, resp.Code, resp.Data, resp.Error + } + + detail := helperFallbackOutputDetail(out, stderr.String()) + if execErr != nil { + return false, "system_error", "", fmt.Sprintf("pkg-helper fallback failed: %v%s", execErr, detail) + } + return false, "helper_error", "", fmt.Sprintf("pkg-helper fallback invalid response: %v%s", parseErr, detail) +} + +func helperFallbackOutputDetail(stdout []byte, stderr string) string { + var parts []string + if trimmed := strings.TrimSpace(string(stdout)); trimmed != "" { + parts = append(parts, "stdout: "+trimmed) + } + if trimmed := strings.TrimSpace(stderr); trimmed != "" { + parts = append(parts, "stderr: "+trimmed) + } + if len(parts) == 0 { + return "" + } + return ": " + strings.Join(parts, "; ") +} + +func findPkgHelperBinary() (string, bool) { + if path, err := exec.LookPath("pkg-helper"); err == nil { + return path, true + } + return firstExecutableFile(pkgHelperFallbackPaths()) +} + +func pkgHelperFallbackPaths() []string { + paths := []string{pkgHelperBundledPath} + if exe, err := os.Executable(); err == nil { + paths = append([]string{filepath.Join(filepath.Dir(exe), "pkg-helper")}, paths...) + } + return paths +} + +func firstExecutableFile(paths []string) (string, bool) { + seen := make(map[string]struct{}, len(paths)) + for _, path := range paths { + if path == "" { + continue + } + if _, ok := seen[path]; ok { + continue + } + seen[path] = struct{}{} + info, err := os.Stat(path) + if err == nil && !info.IsDir() && info.Mode().Perm()&0111 != 0 { + return path, true + } + } + return "", false } // apkViaHelper is the legacy 2-return-value wrapper used by InstallSingleDep,