diff --git a/CHANGELOG.md b/CHANGELOG.md index 899415b..f4ab607 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ Downstream agents should note **agent-core** upgrades in their own changelogs. ## [Unreleased] +### Security + +- **execution**: validate `fluid_log_path` before `os.Stat` / `os.ReadFile` (CodeQL `go/path-injection`); only absolute paths under `/tmp/fluid/`. + ## [0.1.0] - 2026-05-26 ### Added diff --git a/execution/agent.go b/execution/agent.go index dc6fd74..dc2a933 100644 --- a/execution/agent.go +++ b/execution/agent.go @@ -233,8 +233,9 @@ func (a *Agent) startFileLogForwarder(meta map[string]interface{}, payload map[s return func() {} } logPath, _ := payload["fluid_log_path"].(string) - logPath = strings.TrimSpace(logPath) - if logPath == "" { + safePath, err := safeFluidLogPath(logPath) + if err != nil { + log.Printf("fluid_log_path rejected: %v", err) return func() {} } @@ -249,7 +250,7 @@ func (a *Agent) startFileLogForwarder(meta map[string]interface{}, payload map[s return default: } - info, err := os.Stat(logPath) + info, err := os.Stat(safePath) if err != nil { time.Sleep(1 * time.Second) continue @@ -259,7 +260,7 @@ func (a *Agent) startFileLogForwarder(meta map[string]interface{}, payload map[s lastSize = 0 } if size > lastSize { - b, err := os.ReadFile(logPath) + b, err := os.ReadFile(safePath) if err == nil { chunk := b[int(lastSize):] for _, line := range splitLogLines(string(chunk)) { diff --git a/execution/logpath.go b/execution/logpath.go new file mode 100644 index 0000000..0584cff --- /dev/null +++ b/execution/logpath.go @@ -0,0 +1,41 @@ +package execution + +import ( + "fmt" + "path/filepath" + "strings" +) + +// fluidLogPathRoot is where the control plane places per-run logs (see UseCaseEngine.FluidVars). +const fluidLogPathRoot = "/tmp/fluid" + +// safeFluidLogPath validates fluid_log_path from skill payloads before os.Stat/os.ReadFile. +// Only absolute paths under /tmp/fluid/ are allowed (no traversal via ..). +func safeFluidLogPath(raw string) (string, error) { + raw = strings.TrimSpace(raw) + if raw == "" { + return "", fmt.Errorf("empty log path") + } + if strings.Contains(raw, "\x00") { + return "", fmt.Errorf("invalid log path") + } + if !filepath.IsAbs(raw) { + return "", fmt.Errorf("log path must be absolute") + } + + cleaned := filepath.Clean(raw) + root := filepath.Clean(fluidLogPathRoot) + if cleaned != root && !strings.HasPrefix(cleaned, root+string(filepath.Separator)) { + return "", fmt.Errorf("log path must be under %s", fluidLogPathRoot) + } + + rel, err := filepath.Rel(root, cleaned) + if err != nil { + return "", fmt.Errorf("log path: %w", err) + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("log path escapes allowed directory") + } + + return cleaned, nil +} diff --git a/execution/logpath_test.go b/execution/logpath_test.go new file mode 100644 index 0000000..6d31be2 --- /dev/null +++ b/execution/logpath_test.go @@ -0,0 +1,32 @@ +package execution + +import ( + "path/filepath" + "testing" +) + +func TestSafeFluidLogPath(t *testing.T) { + runID := "550e8400-e29b-41d4-a716-446655440000" + valid := filepath.Join(fluidLogPathRoot, runID, "logs", "use-case.log") + + got, err := safeFluidLogPath(valid) + if err != nil { + t.Fatalf("expected valid path: %v", err) + } + if got != filepath.Clean(valid) { + t.Fatalf("got %q want %q", got, filepath.Clean(valid)) + } + + cases := []string{ + "", + "relative/log", + "/etc/passwd", + "/tmp/fluid/../etc/passwd", + "/tmp/fluid/../../etc/passwd", + } + for _, p := range cases { + if _, err := safeFluidLogPath(p); err == nil { + t.Fatalf("expected error for %q", p) + } + } +}