From 77ab09a83ccbe005114266313f12a9259c09c147 Mon Sep 17 00:00:00 2001
From: Martin Catty <martin@catty.fr>
Date: Tue, 26 May 2026 11:19:43 -0400
Subject: [PATCH] test(execution): expand fluid_log_path validation coverage

Table-driven tests for safeFluidLogPath (traversal, prefix siblings,
null byte) and startFileLogForwarder rejection without tailing.
---
 CHANGELOG.md              |  1 +
 execution/logpath_test.go | 92 +++++++++++++++++++++++++++++++--------
 2 files changed, 76 insertions(+), 17 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f4ab607..1ef4599 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,7 @@ Downstream agents should note **agent-core** upgrades in their own changelogs.
 ### Security
 
 - **execution**: validate `fluid_log_path` before `os.Stat` / `os.ReadFile` (CodeQL `go/path-injection`); only absolute paths under `/tmp/fluid/`.
+- **execution**: expand `safeFluidLogPath` test coverage (valid/invalid paths, prefix attacks, `startFileLogForwarder` rejection).
 
 ## [0.1.0] - 2026-05-26
 
diff --git a/execution/logpath_test.go b/execution/logpath_test.go
index 6d31be2..b3d1eb3 100644
--- a/execution/logpath_test.go
+++ b/execution/logpath_test.go
@@ -2,31 +2,89 @@ package execution
 
 import (
 	"path/filepath"
+	"strings"
 	"testing"
 )
 
-func TestSafeFluidLogPath(t *testing.T) {
+func TestSafeFluidLogPath_valid(t *testing.T) {
 	runID := "550e8400-e29b-41d4-a716-446655440000"
-	valid := filepath.Join(fluidLogPathRoot, runID, "logs", "use-case.log")
+	want := filepath.Join(fluidLogPathRoot, runID, "logs", "use-case.log")
 
-	got, err := safeFluidLogPath(valid)
-	if err != nil {
-		t.Fatalf("expected valid path: %v", err)
+	tests := []struct {
+		name string
+		in   string
+	}{
+		{"canonical", want},
+		{"trimmed", "  " + want + "  "},
+		{"extra_slash", filepath.Join(fluidLogPathRoot, runID, "logs", "", "use-case.log")},
+		{"root_child", filepath.Join(fluidLogPathRoot, "child", "log.txt")},
 	}
-	if got != filepath.Clean(valid) {
-		t.Fatalf("got %q want %q", got, filepath.Clean(valid))
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := safeFluidLogPath(tt.in)
+			if err != nil {
+				t.Fatalf("safeFluidLogPath: %v", err)
+			}
+			if got != filepath.Clean(strings.TrimSpace(tt.in)) {
+				t.Fatalf("got %q want %q", got, filepath.Clean(strings.TrimSpace(tt.in)))
+			}
+			if !strings.HasPrefix(got, fluidLogPathRoot+string(filepath.Separator)) {
+				t.Fatalf("result not under root: %q", got)
+			}
+		})
 	}
+}
+
+func TestSafeFluidLogPath_rejects(t *testing.T) {
+	tests := []struct {
+		name string
+		path string
+	}{
+		{"empty", ""},
+		{"whitespace", "   "},
+		{"relative", "tmp/fluid/run/log"},
+		{"relative_with_dot", "./tmp/fluid/run/log"},
+		{"outside_etc", "/etc/passwd"},
+		{"outside_var", "/var/log/syslog"},
+		{"prefix_sibling", "/tmp/fluid-evil/run/log"},
+		{"prefix_sibling_no_slash", "/tmp/fluid2/run/log"},
+		{"dotdot_after_root", filepath.Join(fluidLogPathRoot, "..", "etc", "passwd")},
+		{"dotdot_mid", filepath.Join(fluidLogPathRoot, "run", "..", "..", "etc", "passwd")},
+		{"encoded_traversal", "/tmp/fluid/../etc/passwd"},
+		{"double_encoded", "/tmp/fluid/run/../../etc/passwd"},
+		{"null_byte", "/tmp/fluid/run\x00/log"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := safeFluidLogPath(tt.path)
+			if err == nil {
+				t.Fatalf("expected error for path %q", tt.path)
+			}
+		})
+	}
+}
 
-	cases := []string{
-		"",
-		"relative/log",
-		"/etc/passwd",
-		"/tmp/fluid/../etc/passwd",
-		"/tmp/fluid/../../etc/passwd",
+func TestSafeFluidLogPath_rejectsPathOutsideRootMessage(t *testing.T) {
+	_, err := safeFluidLogPath("/etc/passwd")
+	if err == nil {
+		t.Fatal("expected error")
+	}
+	if !strings.Contains(err.Error(), fluidLogPathRoot) {
+		t.Fatalf("error should mention allowed root, got: %v", err)
 	}
-	for _, p := range cases {
-		if _, err := safeFluidLogPath(p); err == nil {
-			t.Fatalf("expected error for %q", p)
-		}
+}
+
+func TestStartFileLogForwarder_rejectsUnsafeLogPath(t *testing.T) {
+	a := &Agent{
+		cfg: Config{
+			LogEventsEnabled: true,
+			LogVerbosity:     "normal",
+		},
 	}
+	stop := a.startFileLogForwarder(
+		map[string]interface{}{"run_id": "550e8400-e29b-41d4-a716-446655440000"},
+		map[string]interface{}{"fluid_log_path": "/etc/passwd"},
+	)
+	stop()
+	// No panic and immediate no-op: unsafe path must not start the tail goroutine.
 }