mirror of
https://github.com/claude-code-best/claude-code.git
synced 2026-06-23 08:45:50 +00:00
fix: listSessions 严格按 cwd 过滤并移除 session/load 过严校验
- listSessions: 客户端省略 cwd 时回退到 getOriginalCwd(),并对每个候选会话的 存储 cwd 做 canonicalizePath 规范化后与请求 cwd 严格匹配,确保只返回真正属 于当前工作区的会话(符合 session-list.mdx "Only sessions with a matching cwd are returned") - sessionLifecycle: 移除 getOrCreateSession 中审计 2.2 添加的 cwd 一致性校验, 它会拒绝 resolveSessionFilePath worktree fallback 找到的合法会话加载 - 补充 listSessions 的 5 个测试用例覆盖 cwd 透传/fallback/分页拒绝/无 cwd 过滤 Co-Authored-By: glm-5.2 <zai-org@claude-code-best.win>
This commit is contained in:
@@ -71,10 +71,13 @@ mockModulePreservingExports('../../../utils/config.ts', {
|
|||||||
|
|
||||||
const mockSwitchSession = mock(() => {})
|
const mockSwitchSession = mock(() => {})
|
||||||
|
|
||||||
|
const mockGetOriginalCwd = mock(() => '/current/working/dir')
|
||||||
mockModulePreservingExports('../../../bootstrap/state.ts', {
|
mockModulePreservingExports('../../../bootstrap/state.ts', {
|
||||||
setOriginalCwd: mock(() => {}),
|
setOriginalCwd: mock(() => {}),
|
||||||
switchSession: mockSwitchSession,
|
switchSession: mockSwitchSession,
|
||||||
addSlowOperation: mock(() => {}),
|
addSlowOperation: mock(() => {}),
|
||||||
|
getOriginalCwd: mockGetOriginalCwd,
|
||||||
|
getSessionProjectDir: mock(() => null),
|
||||||
})
|
})
|
||||||
|
|
||||||
const mockGetDefaultAppState = mock(() => ({
|
const mockGetDefaultAppState = mock(() => ({
|
||||||
@@ -116,8 +119,9 @@ mockModulePreservingExports('../bridge.ts', {
|
|||||||
})),
|
})),
|
||||||
})
|
})
|
||||||
|
|
||||||
|
const mockListSessionsImpl = mock(async () => [])
|
||||||
mockModulePreservingExports('../../../utils/listSessionsImpl.ts', {
|
mockModulePreservingExports('../../../utils/listSessionsImpl.ts', {
|
||||||
listSessionsImpl: mock(async () => []),
|
listSessionsImpl: mockListSessionsImpl,
|
||||||
})
|
})
|
||||||
|
|
||||||
const mockResolveSessionFilePath = mock(async () => ({
|
const mockResolveSessionFilePath = mock(async () => ({
|
||||||
@@ -241,6 +245,10 @@ describe('AcpAgent', () => {
|
|||||||
mockGetDefaultAppState.mockClear()
|
mockGetDefaultAppState.mockClear()
|
||||||
mockGetSettings.mockReset()
|
mockGetSettings.mockReset()
|
||||||
mockGetSettings.mockImplementation(() => ({}))
|
mockGetSettings.mockImplementation(() => ({}))
|
||||||
|
mockListSessionsImpl.mockReset()
|
||||||
|
mockListSessionsImpl.mockImplementation(async () => [])
|
||||||
|
mockGetOriginalCwd.mockReset()
|
||||||
|
mockGetOriginalCwd.mockImplementation(() => '/current/working/dir')
|
||||||
;(forwardSessionUpdates as ReturnType<typeof mock>).mockReset()
|
;(forwardSessionUpdates as ReturnType<typeof mock>).mockReset()
|
||||||
;(forwardSessionUpdates as ReturnType<typeof mock>).mockImplementation(
|
;(forwardSessionUpdates as ReturnType<typeof mock>).mockImplementation(
|
||||||
async () => ({ stopReason: 'end_turn' as const }),
|
async () => ({ stopReason: 'end_turn' as const }),
|
||||||
@@ -1320,6 +1328,63 @@ describe('AcpAgent', () => {
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe('listSessions', () => {
|
||||||
|
test('passes params.cwd through to listSessionsImpl when provided', async () => {
|
||||||
|
const agent = new AcpAgent(makeConn())
|
||||||
|
await agent.listSessions({ cwd: '/explicit/path' } as any)
|
||||||
|
expect(mockListSessionsImpl).toHaveBeenCalledWith({
|
||||||
|
dir: '/explicit/path',
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
test('falls back to current working dir when client omits cwd', async () => {
|
||||||
|
// Standard clients (Goose, possibly others) call session/list with
|
||||||
|
// empty params. Without a fallback, listSessionsImpl treats undefined
|
||||||
|
// dir as "all projects" and returns every session on disk.
|
||||||
|
mockGetOriginalCwd.mockImplementation(() => '/active/project')
|
||||||
|
const agent = new AcpAgent(makeConn())
|
||||||
|
await agent.listSessions({} as any)
|
||||||
|
expect(mockListSessionsImpl).toHaveBeenCalledWith({
|
||||||
|
dir: '/active/project',
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
test('falls back to current working dir when client sends null cwd', async () => {
|
||||||
|
mockGetOriginalCwd.mockImplementation(() => '/active/project')
|
||||||
|
const agent = new AcpAgent(makeConn())
|
||||||
|
await agent.listSessions({ cwd: null } as any)
|
||||||
|
expect(mockListSessionsImpl).toHaveBeenCalledWith({
|
||||||
|
dir: '/active/project',
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
test('rejects client-supplied cursor (pagination not implemented)', async () => {
|
||||||
|
const agent = new AcpAgent(makeConn())
|
||||||
|
await expect(
|
||||||
|
agent.listSessions({ cursor: 'page2' } as any),
|
||||||
|
).rejects.toThrow(/Pagination cursor not supported/)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('filters out candidates without a cwd field', async () => {
|
||||||
|
mockListSessionsImpl.mockImplementation(
|
||||||
|
async () =>
|
||||||
|
[
|
||||||
|
{
|
||||||
|
sessionId: 'with-cwd',
|
||||||
|
cwd: '/p',
|
||||||
|
summary: 'Has cwd',
|
||||||
|
lastModified: 0,
|
||||||
|
},
|
||||||
|
{ sessionId: 'no-cwd', summary: 'No cwd', lastModified: 0 },
|
||||||
|
] as any,
|
||||||
|
)
|
||||||
|
const agent = new AcpAgent(makeConn())
|
||||||
|
const res = await agent.listSessions({ cwd: '/p' } as any)
|
||||||
|
expect(res.sessions).toHaveLength(1)
|
||||||
|
expect(res.sessions[0].sessionId).toBe('with-cwd')
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
describe('sessionId alignment with global state', () => {
|
describe('sessionId alignment with global state', () => {
|
||||||
test('newSession calls switchSession with the generated sessionId', async () => {
|
test('newSession calls switchSession with the generated sessionId', async () => {
|
||||||
const agent = new AcpAgent(makeConn())
|
const agent = new AcpAgent(makeConn())
|
||||||
|
|||||||
@@ -49,7 +49,11 @@ import { unlink } from 'node:fs/promises'
|
|||||||
import type { Message } from '../../../types/message.js'
|
import type { Message } from '../../../types/message.js'
|
||||||
import { sanitizeTitle } from '../utils.js'
|
import { sanitizeTitle } from '../utils.js'
|
||||||
import { listSessionsImpl } from '../../../utils/listSessionsImpl.js'
|
import { listSessionsImpl } from '../../../utils/listSessionsImpl.js'
|
||||||
import { resolveSessionFilePath } from '../../../utils/sessionStoragePortable.js'
|
import {
|
||||||
|
resolveSessionFilePath,
|
||||||
|
canonicalizePath,
|
||||||
|
} from '../../../utils/sessionStoragePortable.js'
|
||||||
|
import { getOriginalCwd } from '../../../bootstrap/state.js'
|
||||||
import type { AcpSession } from './sessionTypes.js'
|
import type { AcpSession } from './sessionTypes.js'
|
||||||
|
|
||||||
// ── Agent class ───────────────────────────────────────────────────
|
// ── Agent class ───────────────────────────────────────────────────
|
||||||
@@ -190,13 +194,31 @@ export class AcpAgent implements Agent {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Resolve the effective cwd: client-provided wins, fall back to the
|
||||||
|
// agent's current working directory (set by the most recent session/new
|
||||||
|
// or session/load). Standard ACP clients (e.g. Goose) call session/list
|
||||||
|
// with empty params and no cwd — without a fallback, listSessionsImpl
|
||||||
|
// treats undefined dir as "all projects" and returns every session on
|
||||||
|
// disk, which is unrelated to the workspace the user actually has open.
|
||||||
|
const requestedCwd = params.cwd || getOriginalCwd()
|
||||||
|
const canonicalRequested = await canonicalizePath(requestedCwd)
|
||||||
|
|
||||||
const candidates = await listSessionsImpl({
|
const candidates = await listSessionsImpl({
|
||||||
dir: params.cwd ?? undefined,
|
dir: requestedCwd,
|
||||||
})
|
})
|
||||||
|
|
||||||
const sessions = []
|
const sessions = []
|
||||||
for (const candidate of candidates) {
|
for (const candidate of candidates) {
|
||||||
if (!candidate.cwd) continue
|
if (!candidate.cwd) continue
|
||||||
|
// Per session-list.mdx: "Only sessions with a matching cwd are
|
||||||
|
// returned." listSessionsImpl filters by which project directory
|
||||||
|
// the file lives in, but a project directory can hold sessions
|
||||||
|
// whose stored cwd points elsewhere (e.g. a session created in
|
||||||
|
// env_A whose file ended up in the parent repo's project dir via
|
||||||
|
// session/load's worktree fallback). Apply a strict canonical-cwd
|
||||||
|
// filter so the list reflects what the spec promises.
|
||||||
|
const canonicalCandidate = await canonicalizePath(candidate.cwd)
|
||||||
|
if (canonicalCandidate !== canonicalRequested) continue
|
||||||
// Only include title when non-empty; schema allows null/omitted title.
|
// Only include title when non-empty; schema allows null/omitted title.
|
||||||
const title = sanitizeTitle(candidate.summary ?? '')
|
const title = sanitizeTitle(candidate.summary ?? '')
|
||||||
sessions.push({
|
sessions.push({
|
||||||
|
|||||||
@@ -9,7 +9,6 @@
|
|||||||
*/
|
*/
|
||||||
import { type UUID } from 'node:crypto'
|
import { type UUID } from 'node:crypto'
|
||||||
import { dirname } from 'node:path'
|
import { dirname } from 'node:path'
|
||||||
import * as path from 'node:path'
|
|
||||||
import type {
|
import type {
|
||||||
NewSessionRequest,
|
NewSessionRequest,
|
||||||
NewSessionResponse,
|
NewSessionResponse,
|
||||||
@@ -22,11 +21,7 @@ import { setOriginalCwd, switchSession } from '../../../bootstrap/state.js'
|
|||||||
import type { SessionId } from '../../../types/ids.js'
|
import type { SessionId } from '../../../types/ids.js'
|
||||||
import { replayHistoryMessages } from '../bridge.js'
|
import { replayHistoryMessages } from '../bridge.js'
|
||||||
import { computeSessionFingerprint } from '../utils.js'
|
import { computeSessionFingerprint } from '../utils.js'
|
||||||
import {
|
import { resolveSessionFilePath } from '../../../utils/sessionStoragePortable.js'
|
||||||
resolveSessionFilePath,
|
|
||||||
readSessionLite,
|
|
||||||
extractJsonStringField,
|
|
||||||
} from '../../../utils/sessionStoragePortable.js'
|
|
||||||
import { AcpAgent } from './AcpAgent.js'
|
import { AcpAgent } from './AcpAgent.js'
|
||||||
import type { AcpSession } from './sessionTypes.js'
|
import type { AcpSession } from './sessionTypes.js'
|
||||||
import { isPermissionMode } from './permissionMode.js'
|
import { isPermissionMode } from './permissionMode.js'
|
||||||
@@ -89,28 +84,14 @@ async function getOrCreateSession(
|
|||||||
await this.teardownSession(params.sessionId)
|
await this.teardownSession(params.sessionId)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Locate the session file by sessionId across all project directories.
|
// Locate the session file by sessionId. resolveSessionFilePath searches
|
||||||
// params.cwd may not match the project directory where the session was
|
// the requested cwd's project dir first, then falls back to sibling git
|
||||||
// originally created (e.g. client sends a subdirectory path), so we
|
// worktrees — sessions created inside a repo (including from subdirectories
|
||||||
// search by sessionId first and fall back to cwd-based lookup.
|
// or ephemeral test envs nested in the repo) all persist under the same
|
||||||
|
// parent project dir.
|
||||||
const resolved = await resolveSessionFilePath(params.sessionId, params.cwd)
|
const resolved = await resolveSessionFilePath(params.sessionId, params.cwd)
|
||||||
const projectDir = resolved ? dirname(resolved.filePath) : null
|
const projectDir = resolved ? dirname(resolved.filePath) : null
|
||||||
|
|
||||||
// Per session-setup.mdx "Working Directory": the cwd MUST be the absolute
|
|
||||||
// path used for the session regardless of where the Agent was spawned.
|
|
||||||
// Reject cross-project loads where the persisted session's original cwd
|
|
||||||
// does not match the requested cwd, otherwise the client could load a
|
|
||||||
// session belonging to project B while passing project A's cwd.
|
|
||||||
if (resolved) {
|
|
||||||
const lite = await readSessionLite(resolved.filePath)
|
|
||||||
const originalCwd = lite && extractJsonStringField(lite.head, 'cwd')
|
|
||||||
if (originalCwd && path.resolve(originalCwd) !== path.resolve(params.cwd)) {
|
|
||||||
throw new Error(
|
|
||||||
`Session cwd mismatch: session belongs to ${originalCwd}, requested ${params.cwd}`,
|
|
||||||
)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
switchSession(params.sessionId as SessionId, projectDir)
|
switchSession(params.sessionId as SessionId, projectDir)
|
||||||
setOriginalCwd(params.cwd)
|
setOriginalCwd(params.cwd)
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user