Skip to content

Commit 78fad62

Browse files
lazypowerclaude
andcommitted
Address Copilot review on #4
- routes_test.go: check UpsertNode error in TestUnmarkEmptyExtractionsRoute so a failed seed surfaces as the real cause instead of a misleading "unexpected unmark" assertion. - extract_test.go: handle os.Create and Write errors in writeDummyTranscript so a create failure fails the test cleanly instead of panicking on the deferred Close. - extract.go: validate session-id before globbing in findTranscript. Path separators or ".." would let auto-discovery escape ~/.claude/projects. Added validateSessionIDForGlob + table-driven tests covering the cases (uuid ok, plain slug ok, forward/backslash rejected, ".."/absolute path rejected). Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
1 parent ddc255b commit 78fad62

3 files changed

Lines changed: 65 additions & 6 deletions

File tree

internal/cli/extract.go

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,15 @@ func runBackfillEmpty(client *hooks.Client) error {
134134
}
135135

136136
// findTranscript searches ~/.claude/projects/*/<session-id>.jsonl for a
137-
// Claude Code transcript matching the given session id.
137+
// Claude Code transcript matching the given session id. The sessionID is
138+
// validated first — path separators or ".." would let a glob pattern escape
139+
// ~/.claude/projects, which is surprising for "auto-discovery". Callers who
140+
// genuinely need to point at a transcript outside that tree should pass
141+
// --transcript explicitly.
138142
func findTranscript(sessionID string) (string, error) {
143+
if err := validateSessionIDForGlob(sessionID); err != nil {
144+
return "", fmt.Errorf("%w — pass --transcript to point at a specific file", err)
145+
}
139146
home, err := os.UserHomeDir()
140147
if err != nil {
141148
return "", fmt.Errorf("resolve home dir: %w", err)
@@ -153,3 +160,20 @@ func findTranscript(sessionID string) (string, error) {
153160
}
154161
return matches[0], nil
155162
}
163+
164+
// validateSessionIDForGlob rejects session IDs that would let the
165+
// auto-discovery glob escape ~/.claude/projects. Real Claude Code session
166+
// IDs are UUIDs, but continuity imports from other sources so we don't
167+
// require that — we just refuse anything that would traverse the filesystem.
168+
func validateSessionIDForGlob(sessionID string) error {
169+
if sessionID == "" {
170+
return fmt.Errorf("session-id is empty")
171+
}
172+
if strings.ContainsAny(sessionID, `/\`) {
173+
return fmt.Errorf("session-id %q contains a path separator", sessionID)
174+
}
175+
if sessionID == "." || sessionID == ".." || strings.Contains(sessionID, "..") {
176+
return fmt.Errorf("session-id %q contains path traversal", sessionID)
177+
}
178+
return nil
179+
}

internal/cli/extract_test.go

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,19 @@ func writeDummyTranscript(t *testing.T) string {
5050
{"type": "assistant", "message": map[string]any{"role": "assistant", "content": "Sure, I can help."}},
5151
{"type": "user", "message": map[string]any{"role": "user", "content": "Another message here"}},
5252
}
53-
f, _ := os.Create(path)
53+
f, err := os.Create(path)
54+
if err != nil {
55+
t.Fatalf("create transcript: %v", err)
56+
}
5457
defer f.Close()
5558
for _, e := range entries {
5659
data, _ := json.Marshal(e)
57-
f.Write(data)
58-
f.Write([]byte("\n"))
60+
if _, err := f.Write(data); err != nil {
61+
t.Fatalf("write transcript: %v", err)
62+
}
63+
if _, err := f.Write([]byte("\n")); err != nil {
64+
t.Fatalf("write transcript: %v", err)
65+
}
5966
}
6067
return path
6168
}
@@ -143,3 +150,29 @@ func TestExtractCLITranscriptMissing(t *testing.T) {
143150
t.Errorf("unexpected error: %v", err)
144151
}
145152
}
153+
154+
func TestValidateSessionIDForGlob(t *testing.T) {
155+
cases := []struct {
156+
name string
157+
sessionID string
158+
wantErr bool
159+
}{
160+
{"empty", "", true},
161+
{"uuid", "0f5d812c-69e1-40d0-9eef-5436f5721a80", false},
162+
{"plain slug", "my-session", false},
163+
{"forward slash", "foo/bar", true},
164+
{"backslash", `foo\bar`, true},
165+
{"parent traversal", "..", true},
166+
{"embedded traversal", "foo..bar", true},
167+
{"dot", ".", true},
168+
{"absolute escape", "/etc/passwd", true},
169+
}
170+
for _, tc := range cases {
171+
t.Run(tc.name, func(t *testing.T) {
172+
err := validateSessionIDForGlob(tc.sessionID)
173+
if (err != nil) != tc.wantErr {
174+
t.Errorf("validateSessionIDForGlob(%q) err=%v wantErr=%v", tc.sessionID, err, tc.wantErr)
175+
}
176+
})
177+
}
178+
}

internal/server/routes_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,12 +396,14 @@ func TestUnmarkEmptyExtractionsRoute(t *testing.T) {
396396
srv.db.MarkExtracted("empty-sess")
397397
srv.db.InitSession("real-sess", "proj")
398398
srv.db.MarkExtracted("real-sess")
399-
srv.db.UpsertNode(&store.MemNode{
399+
if err := srv.db.UpsertNode(&store.MemNode{
400400
URI: "mem://user/preferences/x",
401401
NodeType: "leaf",
402402
Category: "preferences",
403403
SourceSession: "real-sess",
404-
})
404+
}); err != nil {
405+
t.Fatalf("UpsertNode: %v", err)
406+
}
405407

406408
req := httptest.NewRequest("POST", "/api/sessions/unmark-empty-extractions", nil)
407409
w := httptest.NewRecorder()

0 commit comments

Comments
 (0)