docs: document ResolveSessionFile security contract + add regression tests · Entire

docs: document ResolveSessionFile security contract + add regression tests

026f6bb→main· Soph·1mo ago·3 files·+81 added/-0 removed

Document on the Agent.ResolveSessionFile interface that some agents use agentSessionID as a directory component or (Codex/Pi) return it verbatim when absolute, so callers sourcing the ID from untrusted data MUST validate it first — the resume/rewind choke points do.

Add contract-pinning tests for the two unusual agents: Codex (absolute returned verbatim) and Copilot (ID as directory component). Each asserts both the agent behavior and that ValidateSessionID rejects the dangerous shape, so the guard cannot regress out from under these agents.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

Sessions

383105db9469View transcript

Changes

3

65 unmodified lines

GetSessionDir(repoPath string) (string, error)

// ResolveSessionFile returns the path to the session transcript file.
//
// SECURITY CONTRACT: agentSessionID is used to build a filesystem path and
// some implementations use it as a directory component or (Codex/Pi) return
// it verbatim when absolute. Callers that source agentSessionID from
// untrusted data (e.g. checkpoint metadata on the shared
// entire/checkpoints/v1 branch, hook input) MUST validate it with
// validation.ValidateSessionID first. The resume/rewind restore paths do
// this at their choke points (transcript.resolveTranscriptPath and
// strategy.RestoreLogsOnly); do not call this with unvalidated input.
ResolveSessionFile(sessionDir, agentSessionID string) string

// ReadSession reads session data from agent's storage.

Mcmd/entire/cli/agent/agent.go+9

package codex

import (
    "testing"

"github.com/entireio/cli/cmd/entire/cli/validation"
)

// TestResolveSessionFile_AbsoluteVerbatim_GuardedByValidator pins the security
// contract for Codex's ResolveSessionFile: an absolute agentSessionID is
// returned verbatim (a deliberate feature for agent-recorded transcript paths),
// which makes it a path-traversal footgun if fed untrusted input. Callers that
// source the ID from untrusted data (checkpoint metadata, hook input) must
// reject it first via validation.ValidateSessionID.
//
// This test fails if either the verbatim behavior changes silently OR the shared
// validator stops rejecting absolute IDs — i.e. it guards the resume/rewind fix
// from regressing out from under this agent.
func TestResolveSessionFile_AbsoluteVerbatim_GuardedByValidator(t *testing.T) {
    t.Parallel()

ag := &CodexAgent{}
    const abs = "/etc/evil.jsonl"

if got := ag.ResolveSessionFile("/home/u/.codex/sessions", abs); got != abs {
        t.Fatalf("ResolveSessionFile returned %q, want verbatim %q (behavior change — re-check the validator guard)", got, abs)
    }
    if err := validation.ValidateSessionID(abs); err == nil {
        t.Fatalf("ValidateSessionID(%q) = nil; the validator MUST reject absolute IDs to guard this footgun", abs)
    }
}

Acmd/entire/cli/agent/codex/security_contract_test.go+31

package copilotcli

import (
    "path/filepath"
    "strings"
    "testing"

"github.com/entireio/cli/cmd/entire/cli/validation"
)

// TestResolveSessionFile_DirComponent_GuardedByValidator pins the security
// contract for Copilot's ResolveSessionFile: it uses agentSessionID as a
// directory component (<dir>/<id>/events.jsonl). A bare ".." therefore escapes
// the session directory even though it contains no path separator, so the
// shared validator must reject it. Callers sourcing the ID from untrusted data
// must validate first.
//
// This test fails if the validator stops rejecting ".." (regressing the
// resume/rewind guard) or if the layout changes such that the ID is no longer a
// directory component without a matching guard update.
func TestResolveSessionFile_DirComponent_GuardedByValidator(t *testing.T) {
    t.Parallel()

ag := &CopilotCLIAgent{}
    sessionDir := "/home/user/.copilot/session-state"

// A ".." id used as a directory component escapes sessionDir.
    escaped := ag.ResolveSessionFile(sessionDir, "..")
    rel, err := filepath.Rel(sessionDir, escaped)
    if err != nil {
        t.Fatalf("filepath.Rel(%q, %q) error: %v", sessionDir, escaped, err)
    }
    if !strings.HasPrefix(rel, "..") {
        t.Fatalf("expected %q to escape %q, but it did not (rel=%q)", escaped, sessionDir, rel)
    }

// The shared validator is the guard that prevents that id from reaching here.
    if err := validation.ValidateSessionID(".."); err == nil {
        t.Fatal(`ValidateSessionID("..") = nil; the validator MUST reject ".." to guard this directory-component footgun`)
    }
}

Acmd/entire/cli/agent/copilotcli/security_contract_test.go+41