mirror of
https://github.com/chenhg5/cc-connect.git
synced 2026-10-25 17:36:43 +08:00
When multiple cc-connect projects share a host, a stored AgentSessionID can point at a session file that lives in a DIFFERENT project's ~/.claude/projects/<key>/ directory. Resuming it would silently load the wrong project's conversation history into the current project. Adds an opt-in SessionIDValidator interface (default-agent helper included) so the engine asks the agent whether a stored ID is valid BEFORE calling StartSession. If the agent reports the ID does not belong to this project, the engine clears the stored ID and starts fresh — preventing the cross-project context leak. The ClaudeCode agent implements ValidateSessionID by checking for a <sessionID>.jsonl file under the per-project directory derived from the agent's workDir. Fresh implementation supersedes the stale #604 branch (which mixed #599 validation with the already-merged #603 duplicate-session prune CLI). No prune-CLI changes are included here. Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
1 parent
5e2f3b9eba
commit
30c7605f95
5 files changed
+297
No files matched your search
@@ -406,6 +406,50 @@ func (a *Agent) SetPlatformPrompt(prompt string) {
|
||||
a.platformPrompt = prompt
|
||||
}
|
||||
|
||||
// ValidateSessionID reports whether the given session ID exists in this
|
||||
// agent's per-project session store (issue #599). It is the agent-side
|
||||
// implementation of core.SessionIDValidator: when the engine is about to
|
||||
// resume a session whose stored ID was inherited from another project, the
|
||||
// engine calls this and — on a false return — clears the ID and starts a
|
||||
// fresh session instead of reloading the wrong conversation.
|
||||
func (a *Agent) ValidateSessionID(_ context.Context, sessionID string) bool {
|
||||
if sessionID == "" {
|
||||
return false
|
||||
}
|
||||
homeDir, err := os.UserHomeDir()
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
a.mu.RLock()
|
||||
workDir := a.workDir
|
||||
a.mu.RUnlock()
|
||||
return validateSessionIDInProject(homeDir, workDir, sessionID)
|
||||
}
|
||||
|
||||
// validateSessionIDInProject checks whether sessionID has a .jsonl file
|
||||
// under the per-project directory that Claude Code derives from workDir.
|
||||
// Split out from (*Agent).ValidateSessionID so the logic can be unit
|
||||
// tested without touching the real $HOME.
|
||||
func validateSessionIDInProject(homeDir, workDir, sessionID string) bool {
|
||||
if sessionID == "" {
|
||||
return false
|
||||
}
|
||||
if homeDir == "" || workDir == "" {
|
||||
return false
|
||||
}
|
||||
absWorkDir, err := filepath.Abs(workDir)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
projectDir := findProjectDir(homeDir, absWorkDir)
|
||||
if projectDir == "" {
|
||||
return false
|
||||
}
|
||||
path := filepath.Join(projectDir, sessionID+".jsonl")
|
||||
_, err = os.Stat(path)
|
||||
return err == nil
|
||||
}
|
||||
|
||||
// StartSession creates a persistent interactive Claude Code session.
|
||||
func (a *Agent) StartSession(ctx context.Context, sessionID string) (core.AgentSession, error) {
|
||||
a.mu.Lock()
|
||||
|
||||
@@ -765,3 +765,112 @@ func TestExtractStringContent(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// ── Issue #599 — cross-project session context leakage ────────
|
||||
//
|
||||
// The original PR (#604) regressed when the engine loaded a stored
|
||||
// AgentSessionID that actually belonged to a different project's
|
||||
// workspace. The fix is to ask the agent, via core.SessionIDValidator,
|
||||
// whether the ID has a session file under THIS project's per-project
|
||||
// directory. These tests pin the helper directly.
|
||||
|
||||
// TestValidateSessionIDInProject_ValidSession ensures a known-good
|
||||
// sessionID in the project's directory validates true.
|
||||
func TestValidateSessionIDInProject_ValidSession(t *testing.T) {
|
||||
homeDir := t.TempDir()
|
||||
workDir := filepath.Join(homeDir, "Documents", "myproject")
|
||||
projectKey := encodeClaudeProjectKey(workDir)
|
||||
projectDir := filepath.Join(homeDir, ".claude", "projects", projectKey)
|
||||
if err := os.MkdirAll(projectDir, 0o755); err != nil {
|
||||
t.Fatalf("create project dir: %v", err)
|
||||
}
|
||||
sessionID := "abc123-def456"
|
||||
if err := os.WriteFile(filepath.Join(projectDir, sessionID+".jsonl"), []byte("{}"), 0o644); err != nil {
|
||||
t.Fatalf("write session file: %v", err)
|
||||
}
|
||||
if !validateSessionIDInProject(homeDir, workDir, sessionID) {
|
||||
t.Errorf("validateSessionIDInProject(%q, %q) = false, want true", workDir, sessionID)
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateSessionIDInProject_InvalidSession ensures a sessionID that
|
||||
// has no file in the project's directory returns false (the engine should
|
||||
// then start fresh).
|
||||
func TestValidateSessionIDInProject_InvalidSession(t *testing.T) {
|
||||
homeDir := t.TempDir()
|
||||
workDir := filepath.Join(homeDir, "Documents", "myproject")
|
||||
projectKey := encodeClaudeProjectKey(workDir)
|
||||
projectDir := filepath.Join(homeDir, ".claude", "projects", projectKey)
|
||||
if err := os.MkdirAll(projectDir, 0o755); err != nil {
|
||||
t.Fatalf("create project dir: %v", err)
|
||||
}
|
||||
// A different session file is present, but not the one we ask about.
|
||||
if err := os.WriteFile(filepath.Join(projectDir, "other-session.jsonl"), []byte("{}"), 0o644); err != nil {
|
||||
t.Fatalf("write other session file: %v", err)
|
||||
}
|
||||
if validateSessionIDInProject(homeDir, workDir, "abc123-def456") {
|
||||
t.Errorf("validateSessionIDInProject for missing session = true, want false")
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateSessionIDInProject_EmptySessionID ensures the empty ID is
|
||||
// rejected outright; the engine should never try to resume an empty ID
|
||||
// anyway, but the helper must still return false defensively.
|
||||
func TestValidateSessionIDInProject_EmptySessionID(t *testing.T) {
|
||||
if validateSessionIDInProject(t.TempDir(), "/tmp", "") {
|
||||
t.Error("validateSessionIDInProject(empty) = true, want false")
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateSessionIDInProject_ProjectDirMissing ensures a workDir that
|
||||
// has no corresponding ~/.claude/projects/<key> directory returns false —
|
||||
// Claude Code has never been invoked in that workspace, so any stored
|
||||
// session ID could not possibly belong to it.
|
||||
func TestValidateSessionIDInProject_ProjectDirMissing(t *testing.T) {
|
||||
homeDir := t.TempDir()
|
||||
if validateSessionIDInProject(homeDir, "/nonexistent/path", "some-session-id") {
|
||||
t.Error("validateSessionIDInProject for missing project dir = true, want false")
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateSessionIDInProject_CrossProjectLeak is the regression for
|
||||
// issue #599: a session ID created under project A's directory must NOT
|
||||
// validate as belonging to project B, even when B is also configured. The
|
||||
// original bug let one project silently resume another project's
|
||||
// conversation history.
|
||||
func TestValidateSessionIDInProject_CrossProjectLeak(t *testing.T) {
|
||||
homeDir := t.TempDir()
|
||||
projectsBase := filepath.Join(homeDir, ".claude", "projects")
|
||||
|
||||
projectA := filepath.Join(homeDir, "work", "projectA")
|
||||
projectB := filepath.Join(homeDir, "work", "projectB")
|
||||
dirA := filepath.Join(projectsBase, encodeClaudeProjectKey(projectA))
|
||||
dirB := filepath.Join(projectsBase, encodeClaudeProjectKey(projectB))
|
||||
for _, d := range []string{dirA, dirB} {
|
||||
if err := os.MkdirAll(d, 0o755); err != nil {
|
||||
t.Fatalf("create %s: %v", d, err)
|
||||
}
|
||||
}
|
||||
|
||||
sessionID := "shared-session-id"
|
||||
if err := os.WriteFile(filepath.Join(dirA, sessionID+".jsonl"), []byte("{}"), 0o644); err != nil {
|
||||
t.Fatalf("write session in projectA: %v", err)
|
||||
}
|
||||
|
||||
// Project B should NOT see projectA's session.
|
||||
if validateSessionIDInProject(homeDir, projectB, sessionID) {
|
||||
t.Error("validateSessionIDInProject leaked project A's session into project B")
|
||||
}
|
||||
// Sanity: project A still sees its own.
|
||||
if !validateSessionIDInProject(homeDir, projectA, sessionID) {
|
||||
t.Error("validateSessionIDInProject rejected project A's own session")
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgent_ImplementsSessionIDValidator is a compile-time check that the
|
||||
// production *Agent satisfies core.SessionIDValidator. If the interface
|
||||
// or its method signature drifts, this test stops compiling before the
|
||||
// regression can ship.
|
||||
func TestAgent_ImplementsSessionIDValidator(t *testing.T) {
|
||||
var _ core.SessionIDValidator = (*Agent)(nil)
|
||||
}
|
||||
@@ -3170,6 +3170,20 @@ func (e *Engine) getOrCreateInteractiveStateWith(sessionKey string, p Platform,
|
||||
// is unbound, force a fresh start instead of attaching to whichever CLI
|
||||
// conversation happens to be "latest" in this workspace.
|
||||
startSessionID := session.GetAgentSessionID()
|
||||
// Cross-project session leakage guard (issue #599): if a session ID was
|
||||
// inherited from a different project's workspace (e.g. another
|
||||
// cc-connect project that happens to share a Session row), the agent
|
||||
// can detect the mismatch and we should clear the ID rather than
|
||||
// resume a conversation that has nothing to do with this project.
|
||||
if startSessionID != "" {
|
||||
if validator, ok := agent.(SessionIDValidator); ok && !validator.ValidateSessionID(e.ctx, startSessionID) {
|
||||
slog.Warn("session ID does not belong to this project, clearing it",
|
||||
"session_key", sessionKey, "invalid_session_id", startSessionID)
|
||||
session.SetAgentSessionID("", agent.Name())
|
||||
sessions.Save()
|
||||
startSessionID = ""
|
||||
}
|
||||
}
|
||||
isResume := startSessionID != ""
|
||||
startAt := time.Now()
|
||||
agentSession, err := agent.StartSession(e.ctx, startSessionID)
|
||||
|
||||
@@ -160,6 +160,22 @@ type SystemPromptSupporter interface {
|
||||
HasSystemPromptSupport() bool
|
||||
}
|
||||
|
||||
// SessionIDValidator is an optional interface for agents that can validate
|
||||
// whether a stored session ID actually belongs to the current project's
|
||||
// session store. The engine uses this to prevent cross-project session
|
||||
// context leakage (issue #599): a stale ID from another project's workspace
|
||||
// would otherwise resume the wrong conversation history.
|
||||
//
|
||||
// Implementations should return false when:
|
||||
// - the session ID is empty
|
||||
// - the session file does not exist under the agent's per-project store
|
||||
// - the agent cannot determine the current project directory
|
||||
//
|
||||
// The engine treats a false return as "clear the stored ID and start fresh".
|
||||
type SessionIDValidator interface {
|
||||
ValidateSessionID(ctx context.Context, sessionID string) bool
|
||||
}
|
||||
|
||||
// TypingIndicator is an optional interface for platforms that can show a
|
||||
// "processing" indicator (typing bubble, emoji reaction, etc.) while the
|
||||
// agent is working. StartTyping is called when processing begins and returns
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
package core
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// validatingAgent wraps a controllableAgent and adds an opt-in
|
||||
// SessionIDValidator so we can pin the engine's behavior for issue #599:
|
||||
// when the stored session ID is rejected by the agent, the engine must
|
||||
// start a fresh session instead of resuming the wrong one.
|
||||
type validatingAgent struct {
|
||||
controllableAgent
|
||||
validateFunc func(ctx context.Context, sessionID string) bool
|
||||
}
|
||||
|
||||
func (a *validatingAgent) ValidateSessionID(ctx context.Context, sessionID string) bool {
|
||||
if a.validateFunc == nil {
|
||||
return true // default: trust whatever ID the Session has
|
||||
}
|
||||
return a.validateFunc(ctx, sessionID)
|
||||
}
|
||||
|
||||
var _ SessionIDValidator = (*validatingAgent)(nil)
|
||||
|
||||
// TestIssue599_InvalidSessionIDClearedBeforeResume pins the regression for
|
||||
// cross-project session leakage: when the agent rejects the stored
|
||||
// session ID, the engine must clear the ID and call StartSession with
|
||||
// "" (fresh start) rather than passing the bad ID through.
|
||||
func TestIssue599_InvalidSessionIDClearedBeforeResume(t *testing.T) {
|
||||
var startedWith string
|
||||
sess := newControllableSession("fresh-id")
|
||||
agent := &validatingAgent{
|
||||
controllableAgent: controllableAgent{nextSession: sess},
|
||||
validateFunc: func(_ context.Context, sessionID string) bool {
|
||||
// Reject whatever ID the Session carries — simulate a
|
||||
// cross-project leak.
|
||||
return false
|
||||
},
|
||||
}
|
||||
agent.startSessionFn = func(_ context.Context, sessionID string) (AgentSession, error) {
|
||||
startedWith = sessionID
|
||||
return sess, nil
|
||||
}
|
||||
|
||||
p := &stubPlatformEngine{n: "test"}
|
||||
e := NewEngine("test", agent, []Platform{p}, "", LangEnglish)
|
||||
key := "test:user1"
|
||||
|
||||
// Simulate a stored cross-project session ID.
|
||||
s := &Session{AgentSessionID: "leaked-id-from-other-project"}
|
||||
|
||||
e.getOrCreateInteractiveStateWith(key, p, "ctx", s, e.sessions, nil, "")
|
||||
|
||||
if startedWith != "" {
|
||||
t.Errorf("StartSession called with %q, want \"\" (fresh start; leaked id must NOT be passed through)", startedWith)
|
||||
}
|
||||
}
|
||||
|
||||
// TestIssue599_ValidSessionIDPreserved is the negative case: when the
|
||||
// agent says the ID is valid, the engine must pass it through to
|
||||
// StartSession so the resume actually resumes.
|
||||
func TestIssue599_ValidSessionIDPreserved(t *testing.T) {
|
||||
var startedWith string
|
||||
sess := newControllableSession("resumed-id")
|
||||
agent := &validatingAgent{
|
||||
controllableAgent: controllableAgent{nextSession: sess},
|
||||
validateFunc: func(_ context.Context, _ string) bool {
|
||||
return true
|
||||
},
|
||||
}
|
||||
agent.startSessionFn = func(_ context.Context, sessionID string) (AgentSession, error) {
|
||||
startedWith = sessionID
|
||||
return sess, nil
|
||||
}
|
||||
|
||||
p := &stubPlatformEngine{n: "test"}
|
||||
e := NewEngine("test", agent, []Platform{p}, "", LangEnglish)
|
||||
key := "test:user1"
|
||||
|
||||
s := &Session{AgentSessionID: "valid-id-abc"}
|
||||
|
||||
e.getOrCreateInteractiveStateWith(key, p, "ctx", s, e.sessions, nil, "")
|
||||
|
||||
if startedWith != "valid-id-abc" {
|
||||
t.Errorf("StartSession called with %q, want %q (resume path)", startedWith, "valid-id-abc")
|
||||
}
|
||||
}
|
||||
|
||||
// TestIssue599_AgentWithoutValidatorNotBlocked ensures the validation
|
||||
// gate is opt-in: agents that do not implement SessionIDValidator
|
||||
// continue to work as before (the existing assumption is that engine
|
||||
// callers only persist valid IDs).
|
||||
func TestIssue599_AgentWithoutValidatorNotBlocked(t *testing.T) {
|
||||
var startedWith string
|
||||
sess := newControllableSession("resumed-id")
|
||||
agent := &controllableAgent{nextSession: sess}
|
||||
agent.startSessionFn = func(_ context.Context, sessionID string) (AgentSession, error) {
|
||||
startedWith = sessionID
|
||||
return sess, nil
|
||||
}
|
||||
|
||||
p := &stubPlatformEngine{n: "test"}
|
||||
e := NewEngine("test", agent, []Platform{p}, "", LangEnglish)
|
||||
key := "test:user1"
|
||||
|
||||
s := &Session{AgentSessionID: "any-id"}
|
||||
|
||||
e.getOrCreateInteractiveStateWith(key, p, "ctx", s, e.sessions, nil, "")
|
||||
|
||||
if startedWith != "any-id" {
|
||||
t.Errorf("StartSession called with %q, want %q (no validator = pass through)", startedWith, "any-id")
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user