Skip to content

Commit 8cbb976

Browse files
Sophclaude
andcommitted
fix: validate session ID in Pi captureTranscript before writing
Pi's ParseHookEvent calls captureTranscript on agent_end, which builds the destination path from the hook-supplied session ID and writes the transcript — all before the lifecycle dispatcher validates the ID in DispatchLifecycleEvent. A "../"-laden ID could thus write outside the .entire/tmp/pi cache directory. Validate the session ID at the top of captureTranscript (the choke point where it becomes a path, covering both call sites); an unsafe ID returns "" — the function's existing "no capture" signal — so dispatch behavior is unchanged. Also corrects the gosec comment on the write that wrongly claimed the ID was already validated. Taint is the local Pi hook payload (same-privilege), so severity is low; this is defense-in-depth consistent with the other hook hardening in this branch. The sibling cacheSessionID writes a fixed filename, not an ID-derived path, so it needs no guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: ddfd40f73b22
1 parent 0e4b26f commit 8cbb976

2 files changed

Lines changed: 46 additions & 1 deletion

File tree

cmd/entire/cli/agent/pi/lifecycle.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/entireio/cli/cmd/entire/cli/agent"
1616
"github.com/entireio/cli/cmd/entire/cli/logging"
1717
"github.com/entireio/cli/cmd/entire/cli/paths"
18+
"github.com/entireio/cli/cmd/entire/cli/validation"
1819
)
1920

2021
// Hook names — these match Pi's native event names exactly (snake_case),
@@ -286,6 +287,15 @@ func captureTranscript(ctx context.Context, sessionID, piSessionFile string) str
286287
if sessionID == "" || piSessionFile == "" {
287288
return ""
288289
}
290+
// sessionID comes from the hook payload (or the locally cached active
291+
// session) and is used to build dst below, before the lifecycle dispatcher
292+
// validates it. Validate here at the choke point so an unsafe ID cannot
293+
// write the transcript outside the cache directory; "" signals no capture.
294+
if err := validation.ValidateSessionID(sessionID); err != nil {
295+
logging.Warn(ctx, "pi: refusing to capture transcript for unsafe session ID",
296+
slog.String("session_id", sessionID), slog.String("err", err.Error()))
297+
return ""
298+
}
289299
dir := resolveSessionDir(ctx)
290300
if err := os.MkdirAll(dir, 0o750); err != nil {
291301
logging.Warn(ctx, "pi: capture transcript mkdir failed",
@@ -300,7 +310,7 @@ func captureTranscript(ctx context.Context, sessionID, piSessionFile string) str
300310
slog.String("src", piSessionFile), slog.String("err", err.Error()))
301311
return ""
302312
}
303-
//nolint:gosec // G703: dst constructed from validated session ID inside .entire/tmp
313+
//nolint:gosec // G703: dst is sessionID (validated above) under .entire/tmp/pi
304314
if err := os.WriteFile(dst, data, 0o600); err != nil {
305315
logging.Warn(ctx, "pi: capture transcript write failed",
306316
slog.String("dst", dst), slog.String("err", err.Error()))

cmd/entire/cli/agent/pi/lifecycle_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,41 @@ func TestCaptureTranscript_MissingInputs(t *testing.T) {
211211
}
212212
}
213213

214+
// TestCaptureTranscript_RejectsTraversalSessionID verifies that captureTranscript
215+
// refuses an unsafe session ID. captureTranscript runs inside ParseHookEvent,
216+
// before the lifecycle dispatcher validates the ID, so it must guard the
217+
// transcript write itself — otherwise a "../"-laden ID escapes the cache dir.
218+
func TestCaptureTranscript_RejectsTraversalSessionID(t *testing.T) {
219+
// Cannot use t.Parallel — t.Chdir.
220+
dir := t.TempDir()
221+
t.Chdir(dir)
222+
223+
src := filepath.Join(dir, "src.jsonl")
224+
if err := os.WriteFile(src, []byte("payload\n"), 0o600); err != nil {
225+
t.Fatal(err)
226+
}
227+
228+
// A sentinel outside the cache dir that the traversal would target.
229+
victim := filepath.Join(dir, "victim.json")
230+
if err := os.WriteFile(victim, []byte("SAFE"), 0o600); err != nil {
231+
t.Fatal(err)
232+
}
233+
234+
for _, bad := range []string{"../victim", "/etc/passwd", "..", "a/b"} {
235+
if got := captureTranscript(context.Background(), bad, src); got != "" {
236+
t.Errorf("captureTranscript(%q) = %q, want \"\" (unsafe ID must be refused)", bad, got)
237+
}
238+
}
239+
240+
got, err := os.ReadFile(victim)
241+
if err != nil {
242+
t.Fatal(err)
243+
}
244+
if string(got) != "SAFE" {
245+
t.Errorf("sentinel was overwritten via traversal: %q", string(got))
246+
}
247+
}
248+
214249
func TestGetSupportedHooks(t *testing.T) {
215250
t.Parallel()
216251
got := (&PiAgent{}).GetSupportedHooks()

0 commit comments

Comments
 (0)