mirror of
https://github.com/siteboon/claudecodeui.git
synced 2026-07-07 06:02:43 +08:00
fix: address code scanning findings
- enforce a second image trust boundary in the provider builders: buildClaudeUserContent/buildCodexInputItems only accept files inside the upload store or the run's working directory (CodeQL path injection) - drop an always-true selectedProject check in handleSubmit - remove the unused resume option from spawnCursor
This commit is contained in:
@@ -30,7 +30,7 @@ function isWorkspaceTrustPrompt(text = '') {
|
||||
|
||||
async function spawnCursor(command, options = {}, ws) {
|
||||
return new Promise(async (resolve, reject) => {
|
||||
const { sessionId, projectPath, cwd, resume, toolsSettings, skipPermissions, model, sessionSummary, images } = options;
|
||||
const { sessionId, projectPath, cwd, toolsSettings, skipPermissions, model, sessionSummary, images } = options;
|
||||
const resolvedModel = await providerModelsService.resolveResumeModel('cursor', sessionId, model);
|
||||
let capturedSessionId = sessionId; // Track session ID throughout the process
|
||||
let sessionCreatedSent = false; // Track if we've already sent session-created event
|
||||
|
||||
@@ -90,6 +90,25 @@ export function resolveImageAbsolutePath(cwd: string | undefined, imagePath: str
|
||||
return path.resolve(cwd || process.cwd(), imagePath);
|
||||
}
|
||||
|
||||
function isPathInsideDirectory(candidate: string, directory: string): boolean {
|
||||
const relative = path.relative(path.resolve(directory), candidate);
|
||||
return relative.length > 0 && !relative.startsWith('..') && !path.isAbsolute(relative);
|
||||
}
|
||||
|
||||
/**
|
||||
* Second layer of the image trust boundary (the first is the chat.send filter
|
||||
* in the websocket gateway): provider builders only reference files that live
|
||||
* in the global upload store or inside the run's working directory — places
|
||||
* the agent could already access on its own. Anything else (e.g. `~/.ssh`) is
|
||||
* refused, so a caller-supplied descriptor can never leak arbitrary files.
|
||||
*/
|
||||
export function isAllowedImageSourcePath(resolvedPath: string, cwd?: string): boolean {
|
||||
return (
|
||||
isPathInsideDirectory(resolvedPath, getGlobalImageAssetsDir()) ||
|
||||
isPathInsideDirectory(resolvedPath, cwd || process.cwd())
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves the media type for one image, preferring the uploaded mime type and
|
||||
* falling back to the file extension.
|
||||
@@ -246,8 +265,14 @@ export async function buildClaudeUserContent(
|
||||
continue;
|
||||
}
|
||||
|
||||
const resolvedPath = resolveImageAbsolutePath(cwd, descriptor.path);
|
||||
if (!isAllowedImageSourcePath(resolvedPath, cwd)) {
|
||||
console.warn(`[Images] Refusing to read image outside allowed roots: ${descriptor.path}`);
|
||||
continue;
|
||||
}
|
||||
|
||||
try {
|
||||
const bytes = await fs.readFile(resolveImageAbsolutePath(cwd, descriptor.path));
|
||||
const bytes = await fs.readFile(resolvedPath);
|
||||
blocks.push({
|
||||
type: 'image',
|
||||
source: {
|
||||
@@ -277,9 +302,16 @@ type CodexInputItem =
|
||||
export function buildCodexInputItems(prompt: string, images: unknown, cwd?: string): CodexInputItem[] {
|
||||
const items: CodexInputItem[] = [{ type: 'text', text: prompt }];
|
||||
for (const descriptor of normalizeImageDescriptors(images)) {
|
||||
const resolvedPath = resolveImageAbsolutePath(cwd, descriptor.path);
|
||||
if (!isAllowedImageSourcePath(resolvedPath, cwd)) {
|
||||
// Same trust boundary as buildClaudeUserContent — the Codex runtime
|
||||
// reads this file, so it must stay within the allowed roots.
|
||||
console.warn(`[Images] Refusing to attach image outside allowed roots: ${descriptor.path}`);
|
||||
continue;
|
||||
}
|
||||
items.push({
|
||||
type: 'local_image',
|
||||
path: resolveImageAbsolutePath(cwd, descriptor.path),
|
||||
path: resolvedPath,
|
||||
});
|
||||
}
|
||||
return items;
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
appendImagesInputTag,
|
||||
buildClaudeUserContent,
|
||||
buildCodexInputItems,
|
||||
isAllowedImageSourcePath,
|
||||
normalizeImageDescriptors,
|
||||
parseImagesInputTag,
|
||||
resolveImageMediaType,
|
||||
@@ -186,3 +187,31 @@ test('buildCodexInputItems emits text plus absolute local_image paths', () => {
|
||||
assert.ok(path.isAbsolute(imageItem.path));
|
||||
assert.equal(imageItem.path, path.resolve(cwd, '.cloudcli/assets/pic.jpg'));
|
||||
});
|
||||
|
||||
test('isAllowedImageSourcePath only accepts the upload store and the run cwd', () => {
|
||||
const cwd = path.join(os.tmpdir(), 'some-project');
|
||||
const uploadStore = path.join(os.homedir(), '.cloudcli', 'assets');
|
||||
|
||||
assert.equal(isAllowedImageSourcePath(path.join(uploadStore, 'shot.png'), cwd), true);
|
||||
assert.equal(isAllowedImageSourcePath(path.join(cwd, 'docs', 'diagram.png'), cwd), true);
|
||||
|
||||
assert.equal(isAllowedImageSourcePath(path.join(os.homedir(), '.ssh', 'id_rsa'), cwd), false);
|
||||
assert.equal(isAllowedImageSourcePath(path.join(cwd, '..', 'other-project', 'x.png'), cwd), false);
|
||||
// The roots themselves are directories, not readable image files.
|
||||
assert.equal(isAllowedImageSourcePath(cwd, cwd), false);
|
||||
});
|
||||
|
||||
test('provider builders refuse descriptors outside the allowed roots', async () => {
|
||||
const cwd = path.join(os.tmpdir(), 'codex-project');
|
||||
const outsidePath = path.join(os.homedir(), '.ssh', 'id_rsa.png');
|
||||
|
||||
const codexItems = buildCodexInputItems('prompt', [{ path: outsidePath }], cwd);
|
||||
assert.deepEqual(codexItems, [{ type: 'text', text: 'prompt' }]);
|
||||
|
||||
const claudeContent = await buildClaudeUserContent(
|
||||
'prompt',
|
||||
[{ path: outsidePath, mimeType: 'image/png' }],
|
||||
cwd,
|
||||
);
|
||||
assert.deepEqual(claudeContent, [{ type: 'text', text: 'prompt' }]);
|
||||
});
|
||||
|
||||
@@ -586,9 +586,8 @@ export function useChatComposerState({
|
||||
if (textareaRef.current) {
|
||||
textareaRef.current.style.height = 'auto';
|
||||
}
|
||||
if (selectedProject) {
|
||||
safeLocalStorage.removeItem(`draft_input_${selectedProject.projectId}`);
|
||||
}
|
||||
// selectedProject is guaranteed by the guard at the top of handleSubmit.
|
||||
safeLocalStorage.removeItem(`draft_input_${selectedProject.projectId}`);
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user