mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 14:17:40 +00:00
fix(pkg-helper): harden direct fallback parsing
This commit is contained in:
1 parent
a61bbe85f6
commit
9b16a57a4a
2 files changed
+201
-34
No files matched your search
@@ -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) {
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in new issue
Block a user