Four codecontext sidecar wrappers — get_file_analysis (required file_path), get_symbol_info, get_dependencies, and get_semantic_neighborhoods (optional) — forwarded file_path to the HTTP sidecar unchanged. The sidecar's internal file index is keyed on absolute paths, so any relative path from the model returned "File not found in graph". Three back-to-back failures observed in one chat on 2026-05-22 17:56 UTC, ~48 s of wasted tool budget. ## Resolver Add resolveProjectPath(projectRoot, rawPath) in codecontext_client.ts: trim check → absolute/relative branch (both go through resolve() so dot-segments normalise) → realpath with ENOENT fallthrough → escape check using the realpathed value. Error shape mirrors the existing target_dir escape error byte-for-byte; only the field name differs. Wired into callCodecontext at the args-spread site, guarded on file_path presence + non-empty. All four wrappers benefit from one call site; wrappers without file_path (overview, framework, watch, search) are unaffected. ## Schema trim .trim() added to all four file_path Zod schemas: get_file_analysis: z.string().trim().min(1) get_symbol_info: z.string().trim().optional() get_dependencies: z.string().trim().optional() get_semantic_neighborhoods: z.string().trim().optional() Absorbs trailing newlines / whitespace from model output before the resolver sees the value. ## Adversarial review fixes Adversarial pass surfaced two P2 findings: 1. Absolute path with `..` resolving outside the project root (e.g. `<projectRoot>/../etc/passwd`) that ENOENTs at realpath would slip through the literal prefix-check: the raw string starts with `<projectRoot>/`. Fix: resolve() the absolute branch's candidate too, so dot-segments normalise before the prefix check. 2. No symlink-escape test coverage. Realpath's stated purpose (catching in-project symlinks pointing outside the project) was never tested. Added: create a tmpdir outside projectRoot, symlink projectRoot/evil-link → outside file, assert rejection. ## Tests codecontext_client.test.ts: 19 tests (10 baseline + 9 new file_path resolution cases). Cases cover: relative→absolute, absolute-inside, relative-escape, absolute-outside, ENOENT-fallthrough, empty-string, wrapper-without-file_path, absolute-with-`..`-ENOENT, symlink-leaving-root. codecontext_tools.test.ts: one assertion updated to expect the resolved-absolute file_path on the wire (previously asserted the raw relative path passed through, which is exactly the bug being fixed). Full suite: 301 passed, 7 skipped. ## Affected / unaffected - get_codebase_overview, get_framework_analysis, watch_changes, search_symbols: no file_path arg → resolver guard skips them. No behavior change. - get_semantic_neighborhoods IS in SYNTHESIS_TOOLS — previously-failing relative-path calls will now successfully synthesize. Desirable, not a regression. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
400 lines
14 KiB
TypeScript
400 lines
14 KiB
TypeScript
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
|
import { mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises';
|
|
import { join } from 'node:path';
|
|
import { tmpdir } from 'node:os';
|
|
import { callCodecontext } from '../codecontext_client.js';
|
|
|
|
// ---- fixtures ---------------------------------------------------------------
|
|
|
|
let workDir: string;
|
|
let projectDir: string;
|
|
let outsideDir: string;
|
|
|
|
beforeEach(async () => {
|
|
// Shared workspace so projectDir and outsideDir are siblings but the
|
|
// realpath escape check still treats outsideDir as outside the project.
|
|
workDir = await mkdtemp(join(tmpdir(), 'codecontext-test-'));
|
|
projectDir = join(workDir, 'project');
|
|
outsideDir = join(workDir, 'outside');
|
|
await mkdir(projectDir);
|
|
await mkdir(outsideDir);
|
|
});
|
|
|
|
afterEach(async () => {
|
|
await rm(workDir, { recursive: true, force: true });
|
|
vi.restoreAllMocks();
|
|
});
|
|
|
|
function mockJSONResponse(body: unknown, status = 200): Response {
|
|
return new Response(JSON.stringify(body), {
|
|
status,
|
|
headers: { 'content-type': 'application/json' },
|
|
});
|
|
}
|
|
|
|
// ---- tests ------------------------------------------------------------------
|
|
|
|
describe('callCodecontext — target_dir validation', () => {
|
|
it('rejects when target_dir does not exist', async () => {
|
|
const fetcher = vi.fn();
|
|
await expect(
|
|
callCodecontext(
|
|
{
|
|
toolName: 'get_codebase_overview',
|
|
args: { target_dir: '/nonexistent/path/deliberately/missing' },
|
|
projectPath: projectDir,
|
|
},
|
|
fetcher as unknown as typeof fetch,
|
|
),
|
|
).rejects.toThrow(/target_dir does not exist/);
|
|
expect(fetcher).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it('rejects when target_dir is outside the project root', async () => {
|
|
const fetcher = vi.fn();
|
|
await expect(
|
|
callCodecontext(
|
|
{
|
|
toolName: 'get_codebase_overview',
|
|
args: { target_dir: outsideDir },
|
|
projectPath: projectDir,
|
|
},
|
|
fetcher as unknown as typeof fetch,
|
|
),
|
|
).rejects.toThrow(/escapes project root/);
|
|
expect(fetcher).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it('injects projectPath as target_dir when args.target_dir is undefined', async () => {
|
|
const fetcher = vi.fn().mockResolvedValue(
|
|
mockJSONResponse({ result: 'overview text', error: null }),
|
|
);
|
|
await callCodecontext(
|
|
{
|
|
toolName: 'get_codebase_overview',
|
|
args: { include_stats: true },
|
|
projectPath: projectDir,
|
|
},
|
|
fetcher as unknown as typeof fetch,
|
|
);
|
|
expect(fetcher).toHaveBeenCalledTimes(1);
|
|
const body = JSON.parse(fetcher.mock.calls[0]![1]!.body as string);
|
|
expect(body.target_dir).toBe(projectDir);
|
|
expect(body.include_stats).toBe(true);
|
|
});
|
|
});
|
|
|
|
describe('callCodecontext — HTTP request shape', () => {
|
|
it('POSTs to /v1/<toolName> with JSON content-type', async () => {
|
|
const fetcher = vi.fn().mockResolvedValue(
|
|
mockJSONResponse({ result: 'ok', error: null }),
|
|
);
|
|
await callCodecontext(
|
|
{
|
|
toolName: 'search_symbols',
|
|
args: { query: 'User', limit: 5 },
|
|
projectPath: projectDir,
|
|
},
|
|
fetcher as unknown as typeof fetch,
|
|
);
|
|
expect(fetcher).toHaveBeenCalledTimes(1);
|
|
const [url, init] = fetcher.mock.calls[0]!;
|
|
expect(url).toMatch(/\/v1\/search_symbols$/);
|
|
expect(init.method).toBe('POST');
|
|
expect(init.headers['Content-Type']).toBe('application/json');
|
|
const body = JSON.parse(init.body);
|
|
expect(body).toMatchObject({ query: 'User', limit: 5, target_dir: projectDir });
|
|
});
|
|
});
|
|
|
|
describe('callCodecontext — result handling', () => {
|
|
it('returns { result, truncated: false } when codecontext result is under the 32 kB limit', async () => {
|
|
const fetcher = vi.fn().mockResolvedValue(
|
|
mockJSONResponse({ result: 'a short markdown report', error: null }),
|
|
);
|
|
const out = await callCodecontext(
|
|
{
|
|
toolName: 'get_codebase_overview',
|
|
args: {},
|
|
projectPath: projectDir,
|
|
},
|
|
fetcher as unknown as typeof fetch,
|
|
);
|
|
expect(out.truncated).toBe(false);
|
|
expect(out.result).toBe('a short markdown report');
|
|
});
|
|
|
|
it('truncates and marks truncated: true when result exceeds 32 kB', async () => {
|
|
const bigResult = 'x'.repeat(40_000);
|
|
const fetcher = vi.fn().mockResolvedValue(
|
|
mockJSONResponse({ result: bigResult, error: null }),
|
|
);
|
|
const out = await callCodecontext(
|
|
{
|
|
toolName: 'get_codebase_overview',
|
|
args: {},
|
|
projectPath: projectDir,
|
|
},
|
|
fetcher as unknown as typeof fetch,
|
|
);
|
|
expect(out.truncated).toBe(true);
|
|
expect(out.result).toMatch(/\[truncated, 8000 chars omitted; narrow with file_path/);
|
|
expect(out.result.length).toBeLessThan(bigResult.length);
|
|
});
|
|
});
|
|
|
|
describe('callCodecontext — error paths', () => {
|
|
it('throws an actionable error when codecontext reports an empty-file parser failure', async () => {
|
|
const fetcher = vi.fn().mockResolvedValue(
|
|
mockJSONResponse({
|
|
result: null,
|
|
error:
|
|
'failed to refresh analysis: failed to analyze directory: ' +
|
|
'failed to parse file /opt/boolab/.opencode/node_modules/foo/index.js: content is empty',
|
|
}),
|
|
);
|
|
await expect(
|
|
callCodecontext(
|
|
{ toolName: 'get_codebase_overview', args: {}, projectPath: projectDir },
|
|
fetcher as unknown as typeof fetch,
|
|
),
|
|
).rejects.toThrow(/codecontext parse failure.*\.codecontextignore/);
|
|
});
|
|
|
|
it('throws a generic error when codecontext reports other errors', async () => {
|
|
const fetcher = vi.fn().mockResolvedValue(
|
|
mockJSONResponse({ result: null, error: 'symbol_name is required' }),
|
|
);
|
|
await expect(
|
|
callCodecontext(
|
|
{ toolName: 'get_symbol_info', args: {}, projectPath: projectDir },
|
|
fetcher as unknown as typeof fetch,
|
|
),
|
|
).rejects.toThrow(/codecontext error: symbol_name is required/);
|
|
});
|
|
|
|
it('throws on HTTP non-2xx response', async () => {
|
|
const fetcher = vi.fn().mockResolvedValue(
|
|
new Response('upstream gateway boom', { status: 502 }),
|
|
);
|
|
await expect(
|
|
callCodecontext(
|
|
{ toolName: 'get_codebase_overview', args: {}, projectPath: projectDir },
|
|
fetcher as unknown as typeof fetch,
|
|
),
|
|
).rejects.toThrow(/codecontext HTTP 502/);
|
|
});
|
|
|
|
it('translates a fetcher AbortError to a "timed out" error', async () => {
|
|
// The catch branch in callCodecontext maps any AbortError (whether it
|
|
// came from our internal 30s setTimeout or from the fetcher itself) to a
|
|
// "timed out" message. Exercising the catch directly is cleaner than
|
|
// wrangling vi.useFakeTimers with realpath's microtask scheduling.
|
|
const abortingFetcher = vi.fn().mockImplementation(() => {
|
|
const err = new Error('The user aborted a request.');
|
|
err.name = 'AbortError';
|
|
return Promise.reject(err);
|
|
});
|
|
await expect(
|
|
callCodecontext(
|
|
{ toolName: 'get_codebase_overview', args: {}, projectPath: projectDir },
|
|
abortingFetcher as unknown as typeof fetch,
|
|
),
|
|
).rejects.toThrow(/timed out after 30000ms/);
|
|
});
|
|
});
|
|
|
|
// ---- v1.13.18: file_path resolution tests -----------------------------------
|
|
|
|
describe('callCodecontext — file_path resolution', () => {
|
|
// Case 1: relative path resolves to absolute under project root
|
|
it('resolves a relative file_path to an absolute path inside project root', async () => {
|
|
// Create a real file so realpath can canonicalise it
|
|
const fileName = 'src_module.ts';
|
|
await writeFile(join(projectDir, fileName), '// hello');
|
|
const fetcher = vi.fn().mockResolvedValue(
|
|
mockJSONResponse({ result: 'file analysis', error: null }),
|
|
);
|
|
await callCodecontext(
|
|
{
|
|
toolName: 'get_file_analysis',
|
|
args: { file_path: fileName },
|
|
projectPath: projectDir,
|
|
},
|
|
fetcher as unknown as typeof fetch,
|
|
);
|
|
expect(fetcher).toHaveBeenCalledTimes(1);
|
|
const body = JSON.parse(fetcher.mock.calls[0]![1]!.body as string);
|
|
// Should be the resolved absolute path
|
|
expect(body.file_path).toBe(join(projectDir, fileName));
|
|
});
|
|
|
|
// Case 2: absolute path inside project root → realpathed → forwarded
|
|
it('passes through an absolute file_path inside project root', async () => {
|
|
const fileName = 'absolute_target.ts';
|
|
const absPath = join(projectDir, fileName);
|
|
await writeFile(absPath, '// absolute');
|
|
const fetcher = vi.fn().mockResolvedValue(
|
|
mockJSONResponse({ result: 'analysis', error: null }),
|
|
);
|
|
await callCodecontext(
|
|
{
|
|
toolName: 'get_file_analysis',
|
|
args: { file_path: absPath },
|
|
projectPath: projectDir,
|
|
},
|
|
fetcher as unknown as typeof fetch,
|
|
);
|
|
const body = JSON.parse(fetcher.mock.calls[0]![1]!.body as string);
|
|
expect(body.file_path).toBe(absPath);
|
|
});
|
|
|
|
// Case 3: relative escape path → rejected with same error shape as target_dir escape
|
|
it('rejects a relative file_path that escapes the project root', async () => {
|
|
const fetcher = vi.fn();
|
|
await expect(
|
|
callCodecontext(
|
|
{
|
|
toolName: 'get_file_analysis',
|
|
args: { file_path: '../../etc/passwd' },
|
|
projectPath: projectDir,
|
|
},
|
|
fetcher as unknown as typeof fetch,
|
|
),
|
|
).rejects.toThrow(/escapes project root/);
|
|
expect(fetcher).not.toHaveBeenCalled();
|
|
});
|
|
|
|
// Case 4: absolute path outside project root → rejected
|
|
it('rejects an absolute file_path outside the project root', async () => {
|
|
const fetcher = vi.fn();
|
|
await expect(
|
|
callCodecontext(
|
|
{
|
|
toolName: 'get_file_analysis',
|
|
// /etc/passwd is outside any tmpdir project root
|
|
args: { file_path: '/etc/passwd' },
|
|
projectPath: projectDir,
|
|
},
|
|
fetcher as unknown as typeof fetch,
|
|
),
|
|
).rejects.toThrow(/escapes project root/);
|
|
expect(fetcher).not.toHaveBeenCalled();
|
|
});
|
|
|
|
// Case 5: nonexistent file (ENOENT) → forwarded as un-realpath'd absolute
|
|
it('forwards a nonexistent file_path as absolute without throwing', async () => {
|
|
const missingPath = join(projectDir, 'does_not_exist.ts');
|
|
const fetcher = vi.fn().mockResolvedValue(
|
|
mockJSONResponse({ result: null, error: 'File not found in graph: ' + missingPath }),
|
|
);
|
|
// The resolver should NOT throw; the error comes back from the sidecar
|
|
await expect(
|
|
callCodecontext(
|
|
{
|
|
toolName: 'get_file_analysis',
|
|
args: { file_path: 'does_not_exist.ts' },
|
|
projectPath: projectDir,
|
|
},
|
|
fetcher as unknown as typeof fetch,
|
|
),
|
|
).rejects.toThrow(/File not found in graph/);
|
|
// Wire was still called — resolver forwarded the path
|
|
expect(fetcher).toHaveBeenCalledTimes(1);
|
|
const body = JSON.parse(fetcher.mock.calls[0]![1]!.body as string);
|
|
// Should receive the absolute (non-realpathed) path
|
|
expect(body.file_path).toBe(missingPath);
|
|
});
|
|
|
|
// Case 6: empty string → skipped by guard, reaches wire unmodified
|
|
// Note: Zod .trim().min(1) in get_file_analysis rejects empty before the
|
|
// shim is reached in production. At the shim layer, the guard
|
|
// `file_path.trim() !== ''` skips the resolver for empty strings so that
|
|
// optional-file_path wrappers treat '' as "not provided". This is a
|
|
// deliberate design; callers that require file_path validate at the Zod layer.
|
|
it('skips resolver for empty string file_path (treated as not provided)', async () => {
|
|
const fetcher = vi.fn().mockResolvedValue(
|
|
mockJSONResponse({ result: 'analysis', error: null }),
|
|
);
|
|
// Should succeed — empty string is treated as "no file_path"
|
|
await callCodecontext(
|
|
{
|
|
toolName: 'get_file_analysis',
|
|
args: { file_path: '' },
|
|
projectPath: projectDir,
|
|
},
|
|
fetcher as unknown as typeof fetch,
|
|
);
|
|
expect(fetcher).toHaveBeenCalledTimes(1);
|
|
const body = JSON.parse(fetcher.mock.calls[0]![1]!.body as string);
|
|
// Empty string passes through unchanged (resolver not invoked)
|
|
expect(body.file_path).toBe('');
|
|
});
|
|
|
|
// Case 7: wrapper without file_path (e.g. get_codebase_overview) → resolver not invoked
|
|
it('does not invoke file_path resolver when file_path is absent from args', async () => {
|
|
const fetcher = vi.fn().mockResolvedValue(
|
|
mockJSONResponse({ result: 'overview', error: null }),
|
|
);
|
|
await callCodecontext(
|
|
{
|
|
toolName: 'get_codebase_overview',
|
|
args: { include_stats: true },
|
|
projectPath: projectDir,
|
|
},
|
|
fetcher as unknown as typeof fetch,
|
|
);
|
|
expect(fetcher).toHaveBeenCalledTimes(1);
|
|
const body = JSON.parse(fetcher.mock.calls[0]![1]!.body as string);
|
|
// No file_path in the wire body
|
|
expect('file_path' in body).toBe(false);
|
|
});
|
|
|
|
// Case 8: absolute path with `..` that resolves outside project root, even
|
|
// when the literal path is ENOENT. Without resolve() in the absolute branch
|
|
// the prefix check false-positives because the raw `<projectDir>/../etc/x`
|
|
// literal starts with `<projectDir>/`.
|
|
it('rejects absolute file_path with `..` resolving outside project root (ENOENT branch)', async () => {
|
|
const fetcher = vi.fn();
|
|
const escapingAbsolute = `${projectDir}/../etc/non_existent_passwd`;
|
|
await expect(
|
|
callCodecontext(
|
|
{
|
|
toolName: 'get_file_analysis',
|
|
args: { file_path: escapingAbsolute },
|
|
projectPath: projectDir,
|
|
},
|
|
fetcher as unknown as typeof fetch,
|
|
),
|
|
).rejects.toThrow(/escapes project root/);
|
|
expect(fetcher).not.toHaveBeenCalled();
|
|
});
|
|
|
|
// Case 9: in-project symlink targeting outside the project root. This is the
|
|
// canonical realpath defense — realpath must canonicalise the symlink and
|
|
// the escape check must reject. Without this test, a symlink-out hole could
|
|
// regress silently.
|
|
it('rejects file_path that resolves through a symlink leaving project root', async () => {
|
|
const outsideDir = await mkdtemp(join(tmpdir(), 'codecontext-outside-'));
|
|
try {
|
|
const evilTarget = join(outsideDir, 'secrets.txt');
|
|
await writeFile(evilTarget, 'top secret');
|
|
await symlink(evilTarget, join(projectDir, 'evil-link'));
|
|
const fetcher = vi.fn();
|
|
await expect(
|
|
callCodecontext(
|
|
{
|
|
toolName: 'get_file_analysis',
|
|
args: { file_path: 'evil-link' },
|
|
projectPath: projectDir,
|
|
},
|
|
fetcher as unknown as typeof fetch,
|
|
),
|
|
).rejects.toThrow(/escapes project root/);
|
|
expect(fetcher).not.toHaveBeenCalled();
|
|
} finally {
|
|
await rm(outsideDir, { recursive: true, force: true });
|
|
}
|
|
});
|
|
});
|