From 6ac314b15dac7f1580bb950ea92bf2a10a8ad726 Mon Sep 17 00:00:00 2001 From: lxowalle Date: Wed, 8 Apr 2026 17:29:37 +0800 Subject: [PATCH] fix isolation path handling for relative hooks Preserve relative command and working-directory semantics when Linux isolation wraps subprocesses, and restore absolute argv path exposure to avoid startup regressions. Add hook coverage and docs updates so isolation-enabled process hooks keep working as configured. --- pkg/agent/hook_process_test.go | 125 ++++++++++++++++++++++++++ pkg/isolation/README.md | 2 +- pkg/isolation/README_CN.md | 2 +- pkg/isolation/platform_linux.go | 130 +++++++++++++++++++++++---- pkg/isolation/platform_linux_test.go | 101 ++++++++++++++++++++- 5 files changed, 341 insertions(+), 19 deletions(-) diff --git a/pkg/agent/hook_process_test.go b/pkg/agent/hook_process_test.go index 50f89811f..9b1f0d2e8 100644 --- a/pkg/agent/hook_process_test.go +++ b/pkg/agent/hook_process_test.go @@ -7,10 +7,13 @@ import ( "fmt" "os" "path/filepath" + "runtime" "strings" "testing" "time" + "github.com/sipeed/picoclaw/pkg/config" + "github.com/sipeed/picoclaw/pkg/isolation" "github.com/sipeed/picoclaw/pkg/providers" ) @@ -178,6 +181,75 @@ func TestAgentLoop_MountProcessHook_ApprovalDeny(t *testing.T) { } } +func TestAgentLoop_MountProcessHook_IsolationSupportsRelativeDirAndCommand(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("linux-only isolation path handling") + } + + provider := &llmHookTestProvider{} + al, agent, cleanup := newHookTestLoop(t, provider) + defer cleanup() + + root := t.TempDir() + t.Setenv(config.EnvHome, filepath.Join(root, "picoclaw-home")) + binDir := filepath.Join(root, "bin") + hookDir := filepath.Join(root, "hooks") + if err := os.MkdirAll(binDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(hookDir, 0o755); err != nil { + t.Fatal(err) + } + writeFakeBwrap(t, filepath.Join(binDir, "bwrap")) + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + linkTestBinary(t, os.Args[0], filepath.Join(hookDir, "hook-helper")) + + cfg := config.DefaultConfig() + cfg.Isolation.Enabled = true + isolation.Configure(cfg) + t.Cleanup(func() { isolation.Configure(config.DefaultConfig()) }) + + cwd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + relHookDir, err := filepath.Rel(cwd, hookDir) + if err != nil { + t.Fatal(err) + } + + if err := al.MountProcessHook(context.Background(), "ipc-relative", ProcessHookOptions{ + Command: []string{"./hook-helper", "-test.run=TestProcessHook_HelperProcess", "--"}, + Dir: relHookDir, + Env: processHookHelperEnv("rewrite", ""), + InterceptLLM: true, + }); err != nil { + t.Fatalf("MountProcessHook failed with relative dir/command under isolation: %v", err) + } + + resp, err := al.runAgentLoop(context.Background(), agent, processOptions{ + SessionKey: "session-relative", + Channel: "cli", + ChatID: "direct", + UserMessage: "hello", + DefaultResponse: defaultResponse, + EnableSummary: false, + SendResponse: false, + }) + if err != nil { + t.Fatalf("runAgentLoop failed: %v", err) + } + if resp != "provider content|ipc" { + t.Fatalf("expected process-hooked llm content, got %q", resp) + } + provider.mu.Lock() + lastModel := provider.lastModel + provider.mu.Unlock() + if lastModel != "process-model" { + t.Fatalf("expected process model, got %q", lastModel) + } +} + func processHookHelperCommand() []string { return []string{os.Args[0], "-test.run=TestProcessHook_HelperProcess", "--"} } @@ -193,6 +265,59 @@ func processHookHelperEnv(mode, eventLog string) []string { return env } +func writeFakeBwrap(t *testing.T, path string) { + t.Helper() + script := `#!/bin/sh +set -eu +workdir= +while [ "$#" -gt 0 ]; do + case "$1" in + --) + shift + break + ;; + --chdir) + workdir="$2" + shift 2 + ;; + --bind|--ro-bind) + shift 3 + ;; + --proc|--dev) + shift 2 + ;; + --die-with-parent|--unshare-ipc) + shift + ;; + *) + shift + ;; + esac +done +if [ -n "$workdir" ]; then + cd "$workdir" +fi +exec "$@" +` + if err := os.WriteFile(path, []byte(script), 0o755); err != nil { + t.Fatalf("write fake bwrap: %v", err) + } +} + +func linkTestBinary(t *testing.T, source, target string) { + t.Helper() + if err := os.Symlink(source, target); err == nil { + return + } + data, err := os.ReadFile(source) + if err != nil { + t.Fatalf("read test binary: %v", err) + } + if err := os.WriteFile(target, data, 0o755); err != nil { + t.Fatalf("create hook helper binary: %v", err) + } +} + func waitForFileContains(t *testing.T, path, substring string) { t.Helper() diff --git a/pkg/isolation/README.md b/pkg/isolation/README.md index 623bb492b..de16ce505 100644 --- a/pkg/isolation/README.md +++ b/pkg/isolation/README.md @@ -147,7 +147,7 @@ Capabilities: Default mounts include the instance root plus the minimum runtime system paths such as `/usr`, `/bin`, `/lib`, `/lib64`, and `/etc/resolv.conf`. -At runtime, PicoClaw also adds the executable path, its directory, the working directory, and absolute path arguments when needed. +At runtime, PicoClaw also adds the executable path, its directory, the effective working directory, and absolute path arguments when needed. There is no automatic fallback when `bwrap` is missing. diff --git a/pkg/isolation/README_CN.md b/pkg/isolation/README_CN.md index 4592a7fe8..0529a84bd 100644 --- a/pkg/isolation/README_CN.md +++ b/pkg/isolation/README_CN.md @@ -147,7 +147,7 @@ Linux 后端当前依赖 `bwrap`（`bubblewrap`）。 默认映射包括实例根，以及 `/usr`、`/bin`、`/lib`、`/lib64`、`/etc/resolv.conf` 等最小运行时系统路径。 -运行时还会按需补充可执行文件本身、其所在目录、当前工作目录，以及命令行中的绝对路径参数。 +运行时还会按需补充可执行文件本身、其所在目录、生效后的工作目录，以及命令行中的绝对路径参数。 缺少 `bwrap` 时不会自动回退。 diff --git a/pkg/isolation/platform_linux.go b/pkg/isolation/platform_linux.go index d8b9e417c..9a282a4ad 100644 --- a/pkg/isolation/platform_linux.go +++ b/pkg/isolation/platform_linux.go @@ -3,10 +3,12 @@ package isolation import ( + "errors" "fmt" "os" "os/exec" "path/filepath" + "strings" "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/logger" @@ -42,32 +44,40 @@ func applyPlatformIsolation(cmd *exec.Cmd, isolation config.IsolationConfig, roo originalPath := cmd.Path originalArgs := append([]string{}, cmd.Args...) - originalDir := cmd.Dir + _, execDir, err := resolveLinuxWorkingDir(cmd.Dir, originalPath) + if err != nil { + return err + } + resolvedPath, err := resolveLinuxCommandPath(originalPath, execDir) + if err != nil { + return err + } // Start from the configured mount plan, then add only the executable, its - // resolved path, and the working directory. Any additional host paths must be - // exposed explicitly via config instead of being inferred from argv. + // resolved path, the effective working directory, and any absolute path + // arguments needed to preserve the original command semantics. plan := BuildLinuxMountPlan(root, isolation.ExposePaths) - plan = ensureLinuxMountRule(plan, originalPath, originalPath, "ro") - plan = ensureLinuxMountRule(plan, filepath.Dir(originalPath), filepath.Dir(originalPath), "ro") - if resolved, resolveErr := filepath.EvalSymlinks(originalPath); resolveErr == nil && resolved != originalPath { + plan = ensureLinuxMountRule(plan, resolvedPath, resolvedPath, "ro") + plan = ensureLinuxMountRule(plan, filepath.Dir(resolvedPath), filepath.Dir(resolvedPath), "ro") + if resolved, resolveErr := filepath.EvalSymlinks(resolvedPath); resolveErr == nil && resolved != resolvedPath { plan = ensureLinuxMountRule(plan, resolved, resolved, "ro") plan = ensureLinuxMountRule(plan, filepath.Dir(resolved), filepath.Dir(resolved), "ro") } - if originalDir != "" { - plan = ensureLinuxMountRule(plan, originalDir, originalDir, "rw") - if resolved, resolveErr := filepath.EvalSymlinks(originalDir); resolveErr == nil && resolved != originalDir { + if execDir != "" { + plan = ensureLinuxMountRule(plan, execDir, execDir, "rw") + if resolved, resolveErr := filepath.EvalSymlinks(execDir); resolveErr == nil && resolved != execDir { plan = ensureLinuxMountRule(plan, resolved, resolved, "rw") } } + plan = appendLinuxArgumentMounts(plan, originalArgs[1:]) logger.DebugCF("isolation", "linux isolation mount plan", map[string]any{ "root": root, - "command": originalPath, - "working_dir": originalDir, + "command": resolvedPath, + "working_dir": execDir, "mounts": formatLinuxMountPlan(plan), }) - bwrapArgs, err := buildLinuxBwrapArgs(originalPath, originalArgs, originalDir, plan) + bwrapArgs, err := buildLinuxBwrapArgs(originalPath, resolvedPath, originalArgs, execDir, plan) if err != nil { return err } @@ -106,8 +116,9 @@ func cleanupPendingPlatformResources(cmd *exec.Cmd) { // line that re-executes the original process inside the isolated mount view. func buildLinuxBwrapArgs( originalPath string, + resolvedPath string, originalArgs []string, - originalDir string, + execDir string, plan []MountRule, ) ([]string, error) { bwrapArgs := []string{ @@ -124,16 +135,103 @@ func buildLinuxBwrapArgs( } bwrapArgs = append(bwrapArgs, flag, rule.Source, rule.Target) } - if originalDir != "" { - bwrapArgs = append(bwrapArgs, "--chdir", originalDir) + if execDir != "" { + bwrapArgs = append(bwrapArgs, "--chdir", execDir) } - bwrapArgs = append(bwrapArgs, "--", originalPath) + execPath := originalPath + if isRelativeCommandPath(originalPath) { + execPath = resolvedPath + } + bwrapArgs = append(bwrapArgs, "--", execPath) if len(originalArgs) > 1 { bwrapArgs = append(bwrapArgs, originalArgs[1:]...) } return bwrapArgs, nil } +func resolveLinuxWorkingDir(originalDir, originalPath string) (string, string, error) { + if originalDir != "" { + resolved, err := filepath.Abs(originalDir) + if err != nil { + return "", "", fmt.Errorf("resolve command dir %s: %w", originalDir, err) + } + return resolved, resolved, nil + } + if !isRelativeCommandPath(originalPath) { + return "", "", nil + } + wd, err := os.Getwd() + if err != nil { + return "", "", fmt.Errorf("resolve current working dir: %w", err) + } + return "", wd, nil +} + +func resolveLinuxCommandPath(originalPath, execDir string) (string, error) { + if filepath.IsAbs(originalPath) || !isRelativeCommandPath(originalPath) { + return filepath.Clean(originalPath), nil + } + base := execDir + if base == "" { + var err error + base, err = os.Getwd() + if err != nil { + return "", fmt.Errorf("resolve current working dir: %w", err) + } + } + return filepath.Clean(filepath.Join(base, originalPath)), nil +} + +func appendLinuxArgumentMounts(plan []MountRule, args []string) []MountRule { + for _, arg := range args { + path, ok := linuxArgumentPath(arg) + if !ok { + continue + } + clean := filepath.Clean(path) + if info, err := os.Stat(clean); err == nil { + mode := "ro" + if info.IsDir() { + mode = "rw" + } + plan = ensureLinuxMountRule(plan, clean, clean, mode) + if resolved, resolveErr := filepath.EvalSymlinks(clean); resolveErr == nil && resolved != clean { + plan = ensureLinuxMountRule(plan, resolved, resolved, mode) + } + continue + } else if !errors.Is(err, os.ErrNotExist) { + continue + } + parent := filepath.Dir(clean) + if parent == clean { + continue + } + if _, err := os.Stat(parent); err == nil { + plan = ensureLinuxMountRule(plan, parent, parent, "rw") + } + } + return plan +} + +func linuxArgumentPath(arg string) (string, bool) { + if filepath.IsAbs(arg) { + return arg, true + } + idx := strings.IndexRune(arg, '=') + if idx <= 0 || idx == len(arg)-1 { + return "", false + } + value := arg[idx+1:] + if !filepath.IsAbs(value) { + return "", false + } + return value, true +} + +func isRelativeCommandPath(path string) bool { + return !filepath.IsAbs(path) && strings.ContainsRune(path, filepath.Separator) +} + // ensureLinuxMountRule appends a mount rule unless another rule already owns // the same target path. func ensureLinuxMountRule(plan []MountRule, source, target, mode string) []MountRule { diff --git a/pkg/isolation/platform_linux_test.go b/pkg/isolation/platform_linux_test.go index 12a90c0ba..a082b1663 100644 --- a/pkg/isolation/platform_linux_test.go +++ b/pkg/isolation/platform_linux_test.go @@ -21,7 +21,7 @@ func TestBuildLinuxBwrapArgs_IncludesNamespaceFlagsAndExec(t *testing.T) { t.Fatal(err) } plan := BuildLinuxMountPlan(root, []config.ExposePath{{Source: binaryDir, Target: binaryDir, Mode: "ro"}}) - args, err := buildLinuxBwrapArgs(binaryPath, []string{binaryPath, "--flag"}, root, plan) + args, err := buildLinuxBwrapArgs(binaryPath, binaryPath, []string{binaryPath, "--flag"}, root, plan) if err != nil { t.Fatalf("buildLinuxBwrapArgs() error = %v", err) } @@ -47,3 +47,102 @@ func TestBuildLinuxBwrapArgs_IncludesNamespaceFlagsAndExec(t *testing.T) { t.Fatalf("bwrap args missing required items: %v", args) } } + +func TestResolveLinuxWorkingDir_ResolvesRelativeDir(t *testing.T) { + cwd := t.TempDir() + previous, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + defer func() { + if chdirErr := os.Chdir(previous); chdirErr != nil { + t.Fatalf("restore cwd: %v", chdirErr) + } + }() + if err := os.Chdir(cwd); err != nil { + t.Fatal(err) + } + + resolvedDir, execDir, err := resolveLinuxWorkingDir("./hooks", "./hook.sh") + if err != nil { + t.Fatalf("resolveLinuxWorkingDir() error = %v", err) + } + want := filepath.Join(cwd, "hooks") + if resolvedDir != want || execDir != want { + t.Fatalf("resolveLinuxWorkingDir() = (%q, %q), want (%q, %q)", resolvedDir, execDir, want, want) + } +} + +func TestResolveLinuxCommandPath_UsesExecDirForRelativeCommand(t *testing.T) { + execDir := filepath.Join(t.TempDir(), "hooks") + got, err := resolveLinuxCommandPath("./hook.sh", execDir) + if err != nil { + t.Fatalf("resolveLinuxCommandPath() error = %v", err) + } + want := filepath.Join(execDir, "hook.sh") + if got != want { + t.Fatalf("resolveLinuxCommandPath() = %q, want %q", got, want) + } +} + +func TestBuildLinuxBwrapArgs_UsesResolvedPathForRelativeCommand(t *testing.T) { + root := t.TempDir() + execDir := filepath.Join(root, "hooks") + if err := os.MkdirAll(execDir, 0o755); err != nil { + t.Fatal(err) + } + resolvedPath := filepath.Join(execDir, "hook.sh") + if err := os.WriteFile(resolvedPath, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatal(err) + } + plan := []MountRule{ + {Source: execDir, Target: execDir, Mode: "rw"}, + {Source: resolvedPath, Target: resolvedPath, Mode: "ro"}, + } + args, err := buildLinuxBwrapArgs("./hook.sh", resolvedPath, []string{"./hook.sh"}, execDir, plan) + if err != nil { + t.Fatalf("buildLinuxBwrapArgs() error = %v", err) + } + hasExecDir := false + for _, arg := range args { + if arg == execDir { + hasExecDir = true + break + } + } + if !hasExecDir { + t.Fatalf("buildLinuxBwrapArgs() missing resolved chdir: %v", args) + } + for i := range args { + if args[i] == "--" { + if i+1 >= len(args) || args[i+1] != resolvedPath { + t.Fatalf("buildLinuxBwrapArgs() exec path = %v, want %q after --", args, resolvedPath) + } + return + } + } + t.Fatalf("buildLinuxBwrapArgs() missing exec delimiter: %v", args) +} + +func TestAppendLinuxArgumentMounts_AddsAbsoluteArgumentPaths(t *testing.T) { + root := t.TempDir() + input := filepath.Join(root, "input.txt") + if err := os.WriteFile(input, []byte("data"), 0o644); err != nil { + t.Fatal(err) + } + output := filepath.Join(root, "out", "result.txt") + if err := os.MkdirAll(filepath.Dir(output), 0o755); err != nil { + t.Fatal(err) + } + + plan := appendLinuxArgumentMounts(nil, []string{input, "--output=" + output}) + if len(plan) != 2 { + t.Fatalf("appendLinuxArgumentMounts() len = %d, want 2", len(plan)) + } + if plan[0].Source != input || plan[0].Mode != "ro" { + t.Fatalf("appendLinuxArgumentMounts()[0] = %+v, want source=%q mode=ro", plan[0], input) + } + if plan[1].Source != filepath.Dir(output) || plan[1].Mode != "rw" { + t.Fatalf("appendLinuxArgumentMounts()[1] = %+v, want source=%q mode=rw", plan[1], filepath.Dir(output)) + } +}