| 1 | package jobs |
| 2 | |
| 3 | import ( |
| 4 | "context" |
| 5 | "io" |
| 6 | "os" |
| 7 | "path/filepath" |
| 8 | "strings" |
| 9 | "testing" |
| 10 | |
| 11 | "reasonix/internal/event" |
| 12 | "reasonix/internal/store" |
| 13 | ) |
| 14 | |
| 15 | // TestValidatePathSegment exhaustively covers the segment validator that guards |
| 16 | // StartForSession against #6932 (path traversal in artifact paths). |
| 17 | func TestValidatePathSegment(t *testing.T) { |
| 18 | cases := []struct { |
| 19 | name string |
| 20 | field string |
| 21 | input string |
| 22 | wantErr bool |
| 23 | }{ |
| 24 | // parentSession: empty is the documented unscoped default. |
| 25 | {"empty parentSession is allowed", "parentSession", "", false}, |
| 26 | // kind: empty is rejected because id is built from kind. |
| 27 | {"empty kind is rejected", "kind", "", true}, |
| 28 | // Typical safe values. |
| 29 | {"simple lowercase kind", "kind", "task", false}, |
| 30 | {"hyphenated kind", "kind", "bash-bg", false}, |
| 31 | {"underscored kind", "kind", "bash_bg", false}, |
| 32 | {"digits-only kind", "kind", "123", false}, |
| 33 | {"unicode kind (no separators)", "kind", "任务", false}, |
| 34 | {"parentSession with timestamp", "parentSession", "20260724-142525abc", false}, |
| 35 | // Path separators: must reject on any field. |
| 36 | {"forward slash", "kind", "task/evil", true}, |
| 37 | {"back slash", "kind", "task\\evil", true}, |
| 38 | {"only slash", "parentSession", "/", true}, |
| 39 | {"only backslash", "parentSession", "\\", true}, |
| 40 | // Traversal: must reject both `.` and `..` even without separators, |
| 41 | // because filepath.Clean treats them as parent references. |
| 42 | {"single dot", "kind", ".", true}, |
| 43 | {"double dot", "kind", "..", true}, |
| 44 | {"traversal prefix", "parentSession", "..", true}, |
| 45 | {"parentSession with traversal", "parentSession", "../../etc", true}, |
| 46 | {"kind with embedded traversal", "kind", "task/../../etc", true}, |
| 47 | // Control characters and NUL. |
| 48 | {"NUL byte", "kind", "task\x00evil", true}, |
| 49 | {"newline", "kind", "task\nevil", true}, |
| 50 | {"tab", "kind", "task\tevil", true}, |
| 51 | {"DEL char", "kind", "task\x7fevil", true}, |
| 52 | } |
| 53 | for _, tc := range cases { |
| 54 | t.Run(tc.name, func(t *testing.T) { |
| 55 | err := validatePathSegment(tc.input, tc.field) |
| 56 | if tc.wantErr && err == nil { |
| 57 | t.Fatalf("validatePathSegment(%q, %q) = nil, want error", tc.input, tc.field) |
| 58 | } |
| 59 | if !tc.wantErr && err != nil { |
| 60 | t.Fatalf("validatePathSegment(%q, %q) = %v, want nil", tc.input, tc.field, err) |
| 61 | } |
| 62 | }) |
| 63 | } |
| 64 | } |
| 65 | |
| 66 | // TestStartForSession_RejectsPathTraversalParentSession ensures a malicious |
| 67 | // parentSession like "../../etc" does not create files outside the manager's |
| 68 | // temp root. See #6932. |
| 69 | func TestStartForSession_RejectsPathTraversalParentSession(t *testing.T) { |
| 70 | m := NewManager(event.Discard) |
| 71 | defer m.Close() |
| 72 | |
| 73 | // Snapshot the temp root before; afterwards the directory listing must be |
| 74 | // unchanged. This proves no traversal payload created any subdirectory. |
| 75 | beforeEntries, err := os.ReadDir(m.tempRoot) |
| 76 | if err != nil { |
| 77 | t.Fatalf("read temp root before: %v", err) |
| 78 | } |
| 79 | |
| 80 | // "../../etc" with a leading traversal walks two parents up from |
| 81 | // m.tempRoot and then descends into "etc". The exact resolution does not |
| 82 | // matter; what matters is that the validator rejects it before any |
| 83 | // mkdir or open happens, anywhere. |
| 84 | ran := false |
| 85 | j := m.StartForSession("../../etc", "task", "escape attempt", func(_ context.Context, _ io.Writer) (string, error) { |
| 86 | ran = true |
| 87 | return "", nil |
| 88 | }) |
| 89 | if ran { |
| 90 | t.Fatal("run goroutine executed for an invalid parentSession; the fix failed") |
| 91 | } |
| 92 | if j.status != Failed { |
| 93 | t.Fatalf("job status = %q, want %q (artifactErr=%q)", j.status, Failed, j.artifactErr) |
| 94 | } |
| 95 | if !strings.Contains(j.artifactErr, "parentSession") { |
| 96 | t.Fatalf("artifactErr should reference parentSession, got %q", j.artifactErr) |
| 97 | } |
| 98 | if j.artifactPath != "" { |
| 99 | t.Fatalf("artifactPath = %q, want empty (no file should be created)", j.artifactPath) |
| 100 | } |
| 101 | |
| 102 | afterEntries, err := os.ReadDir(m.tempRoot) |
| 103 | if err != nil { |
| 104 | t.Fatalf("read temp root after: %v", err) |
| 105 | } |
| 106 | if len(afterEntries) != len(beforeEntries) { |
| 107 | names := make([]string, 0, len(afterEntries)) |
| 108 | for _, e := range afterEntries { |
| 109 | names = append(names, e.Name()) |
| 110 | } |
| 111 | t.Fatalf("temp root gained %d new entries; want no change. New entries: %v", len(afterEntries)-len(beforeEntries), names) |
| 112 | } |
| 113 | } |
| 114 | |
| 115 | // TestStartForSession_RejectsPathTraversalKind ensures a malicious kind |
| 116 | // containing path separators is rejected before any artifact is created. |
| 117 | // See #6932. |
| 118 | func TestStartForSession_RejectsPathTraversalKind(t *testing.T) { |
| 119 | m := NewManager(event.Discard) |
| 120 | defer m.Close() |
| 121 | |
| 122 | ran := false |
| 123 | j := m.StartForSession("safe-session", "../../../etc", "escape via kind", func(_ context.Context, _ io.Writer) (string, error) { |
| 124 | ran = true |
| 125 | return "", nil |
| 126 | }) |
| 127 | if ran { |
| 128 | t.Fatal("run goroutine executed for an invalid kind; the fix failed") |
| 129 | } |
| 130 | if j.status != Failed { |
| 131 | t.Fatalf("job status = %q, want %q (artifactErr=%q)", j.status, Failed, j.artifactErr) |
| 132 | } |
| 133 | if !strings.Contains(j.artifactErr, "kind") { |
| 134 | t.Fatalf("artifactErr should reference kind, got %q", j.artifactErr) |
| 135 | } |
| 136 | if j.artifactPath != "" { |
| 137 | t.Fatalf("artifactPath = %q, want empty", j.artifactPath) |
| 138 | } |
| 139 | } |
| 140 | |
| 141 | // TestStartForSession_AcceptsValidInput is a regression guard: the validator |
| 142 | // must not break legitimate callers that use typical session ids and kinds. |
| 143 | func TestStartForSession_AcceptsValidInput(t *testing.T) { |
| 144 | m := NewManager(event.Discard) |
| 145 | defer m.Close() |
| 146 | |
| 147 | ran := false |
| 148 | release := make(chan struct{}) |
| 149 | j := m.StartForSession("session-20260724-142525abc", "task", "normal call", func(ctx context.Context, _ io.Writer) (string, error) { |
| 150 | ran = true |
| 151 | select { |
| 152 | case <-release: |
| 153 | return "ok", nil |
| 154 | case <-ctx.Done(): |
| 155 | return "", ctx.Err() |
| 156 | } |
| 157 | }) |
| 158 | j.mu.Lock() |
| 159 | status := j.status |
| 160 | artifactErr := j.artifactErr |
| 161 | artifactPath := j.artifactPath |
| 162 | j.mu.Unlock() |
| 163 | if status != Running { |
| 164 | t.Fatalf("job status = %q, want Running (artifactErr=%q)", status, artifactErr) |
| 165 | } |
| 166 | if artifactPath == "" { |
| 167 | t.Fatal("artifactPath empty; expected a path under the temp root") |
| 168 | } |
| 169 | if !strings.HasPrefix(artifactPath, m.tempRoot) { |
| 170 | t.Fatalf("artifactPath = %q does not start with temp root %q", artifactPath, m.tempRoot) |
| 171 | } |
| 172 | |
| 173 | // Wait for run to finish and confirm cleanup paths still work. |
| 174 | close(release) |
| 175 | res := m.WaitForSession(context.Background(), "session-20260724-142525abc", []string{j.ID}, 5) |
| 176 | if len(res) != 1 { |
| 177 | t.Fatalf("WaitForSession returned %d results, want 1", len(res)) |
| 178 | } |
| 179 | if res[0].Output != "ok" { |
| 180 | t.Fatalf("run output = %q, want %q", res[0].Output, "ok") |
| 181 | } |
| 182 | if !ran { |
| 183 | t.Fatal("run callback never executed") |
| 184 | } |
| 185 | } |
| 186 | |
| 187 | // TestStartForSession_AcceptsSetActiveSessionPathDir guards against a previous |
| 188 | // regression where a defense-in-depth containment check in openArtifactLocked |
| 189 | // rejected legitimate artifact directories produced by SetActiveSessionPath. |
| 190 | // That path resolves to <root>/<id>.jobs (an absolute directory outside the |
| 191 | // manager's temp root), so a temp-root containment check was a false positive. |
| 192 | // validatePathSegment confines only the temp-root fallback; persistent artifact |
| 193 | // directories come from the trusted transcript path bound by the store layer. |
| 194 | // See #6932. |
| 195 | func TestStartForSession_AcceptsSetActiveSessionPathDir(t *testing.T) { |
| 196 | root := t.TempDir() |
| 197 | sessionPath := filepath.Join(root, "a.jsonl") |
| 198 | |
| 199 | m := NewManager(event.Discard) |
| 200 | defer m.Close() |
| 201 | m.SetActiveSessionPath("session-a", sessionPath) |
| 202 | |
| 203 | j := m.StartForSession("session-a", "bash", "regression", func(_ context.Context, _ io.Writer) (string, error) { |
| 204 | return "ok", nil |
| 205 | }) |
| 206 | <-j.done |
| 207 | if j.artifactErr != "" { |
| 208 | t.Fatalf("artifactErr = %q, want empty (SetActiveSessionPath dir must be accepted)", j.artifactErr) |
| 209 | } |
| 210 | if j.artifactPath == "" { |
| 211 | t.Fatal("artifactPath empty; expected a path under the session dir") |
| 212 | } |
| 213 | // The log file must live next to the session transcript. |
| 214 | wantDir := store.SessionJobsDir(sessionPath) |
| 215 | if !strings.HasPrefix(j.artifactPath, wantDir) { |
| 216 | t.Fatalf("artifactPath = %q, want prefix %q", j.artifactPath, wantDir) |
| 217 | } |
| 218 | } |
| 219 |