feat(team): open persisted attachments in editor
This commit is contained in:
parent
63e16d1043
commit
e88d3a1e98
7 changed files with 148 additions and 13 deletions
|
|
@ -217,6 +217,7 @@ export class TeamAttachmentStore {
|
||||||
id: entry.id,
|
id: entry.id,
|
||||||
data: buffer.toString('base64'),
|
data: buffer.toString('base64'),
|
||||||
mimeType,
|
mimeType,
|
||||||
|
filePath,
|
||||||
});
|
});
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
if ((error as NodeJS.ErrnoException).code === 'ENOENT') continue;
|
if ((error as NodeJS.ErrnoException).code === 'ENOENT') continue;
|
||||||
|
|
|
||||||
|
|
@ -11,7 +11,7 @@
|
||||||
import * as fs from 'fs';
|
import * as fs from 'fs';
|
||||||
import * as path from 'path';
|
import * as path from 'path';
|
||||||
|
|
||||||
import { getClaudeBasePath, getHomeDir } from './pathDecoder';
|
import { getAppDataPath, getClaudeBasePath, getHomeDir } from './pathDecoder';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Sensitive file patterns that should never be accessible.
|
* Sensitive file patterns that should never be accessible.
|
||||||
|
|
@ -101,6 +101,7 @@ export function matchesSensitivePattern(normalizedPath: string): boolean {
|
||||||
* Allowed directories:
|
* Allowed directories:
|
||||||
* - The project path itself
|
* - The project path itself
|
||||||
* - The ~/.claude directory (for session data)
|
* - The ~/.claude directory (for session data)
|
||||||
|
* - The app-owned data directory (attachments, task attachments)
|
||||||
*
|
*
|
||||||
* @param normalizedPath - The normalized absolute path to check
|
* @param normalizedPath - The normalized absolute path to check
|
||||||
* @param projectPath - The project root path (can be null for global access)
|
* @param projectPath - The project root path (can be null for global access)
|
||||||
|
|
@ -114,12 +115,19 @@ export function isPathWithinAllowedDirectories(
|
||||||
const normalizedTarget = normalizeForCompare(normalizedPath, isWindows);
|
const normalizedTarget = normalizeForCompare(normalizedPath, isWindows);
|
||||||
const claudeDir = getClaudeBasePath();
|
const claudeDir = getClaudeBasePath();
|
||||||
const normalizedClaudeDir = normalizeForCompare(claudeDir, isWindows);
|
const normalizedClaudeDir = normalizeForCompare(claudeDir, isWindows);
|
||||||
|
const appDataDir = getAppDataPath();
|
||||||
|
const normalizedAppDataDir = normalizeForCompare(appDataDir, isWindows);
|
||||||
|
|
||||||
// Always allow access to ~/.claude for session data
|
// Always allow access to ~/.claude for session data
|
||||||
if (isPathWithinRoot(normalizedTarget, normalizedClaudeDir)) {
|
if (isPathWithinRoot(normalizedTarget, normalizedClaudeDir)) {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Allow app-owned persisted data such as message attachment files.
|
||||||
|
if (isPathWithinRoot(normalizedTarget, normalizedAppDataDir)) {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
// If project path provided, allow access within project
|
// If project path provided, allow access within project
|
||||||
if (projectPath) {
|
if (projectPath) {
|
||||||
const normalizedProjectPath = normalizeForCompare(projectPath, isWindows);
|
const normalizedProjectPath = normalizeForCompare(projectPath, isWindows);
|
||||||
|
|
@ -137,7 +145,7 @@ export function isPathWithinAllowedDirectories(
|
||||||
* Security checks performed:
|
* Security checks performed:
|
||||||
* 1. Path must be absolute
|
* 1. Path must be absolute
|
||||||
* 2. Path traversal prevention (no ..)
|
* 2. Path traversal prevention (no ..)
|
||||||
* 3. Must be within allowed directories (project or ~/.claude)
|
* 3. Must be within allowed directories (project, ~/.claude, or app data)
|
||||||
* 4. Must not match sensitive file patterns
|
* 4. Must not match sensitive file patterns
|
||||||
*
|
*
|
||||||
* @param filePath - The file path to validate
|
* @param filePath - The file path to validate
|
||||||
|
|
|
||||||
|
|
@ -2,6 +2,7 @@ import { useEffect, useState } from 'react';
|
||||||
|
|
||||||
import { useAppTranslation } from '@features/localization/renderer';
|
import { useAppTranslation } from '@features/localization/renderer';
|
||||||
import { FileIcon } from '@renderer/components/team/editor/FileIcon';
|
import { FileIcon } from '@renderer/components/team/editor/FileIcon';
|
||||||
|
import { useStore } from '@renderer/store';
|
||||||
import { isImageMime } from '@renderer/utils/attachmentUtils';
|
import { isImageMime } from '@renderer/utils/attachmentUtils';
|
||||||
import { Loader2 } from 'lucide-react';
|
import { Loader2 } from 'lucide-react';
|
||||||
|
|
||||||
|
|
@ -22,6 +23,7 @@ export const AttachmentDisplay = ({
|
||||||
attachments,
|
attachments,
|
||||||
}: AttachmentDisplayProps): React.JSX.Element | null => {
|
}: AttachmentDisplayProps): React.JSX.Element | null => {
|
||||||
const { t } = useAppTranslation('team');
|
const { t } = useAppTranslation('team');
|
||||||
|
const revealFileInEditor = useStore((s) => s.revealFileInEditor);
|
||||||
const [state, setState] = useState<{
|
const [state, setState] = useState<{
|
||||||
loaded: AttachmentFileData[];
|
loaded: AttachmentFileData[];
|
||||||
loading: boolean;
|
loading: boolean;
|
||||||
|
|
@ -74,10 +76,16 @@ export const AttachmentDisplay = ({
|
||||||
return {
|
return {
|
||||||
meta,
|
meta,
|
||||||
dataUrl: isImage ? `data:${data.mimeType};base64,${data.data}` : undefined,
|
dataUrl: isImage ? `data:${data.mimeType};base64,${data.data}` : undefined,
|
||||||
|
filePath: data.filePath ?? meta.filePath,
|
||||||
isImage,
|
isImage,
|
||||||
};
|
};
|
||||||
})
|
})
|
||||||
.filter(Boolean) as { meta: AttachmentMeta; dataUrl: string | undefined; isImage: boolean }[];
|
.filter(Boolean) as {
|
||||||
|
meta: AttachmentMeta;
|
||||||
|
dataUrl: string | undefined;
|
||||||
|
filePath: string | undefined;
|
||||||
|
isImage: boolean;
|
||||||
|
}[];
|
||||||
|
|
||||||
if (items.length === 0) return null;
|
if (items.length === 0) return null;
|
||||||
|
|
||||||
|
|
@ -107,10 +115,29 @@ export const AttachmentDisplay = ({
|
||||||
: undefined
|
: undefined
|
||||||
}
|
}
|
||||||
/>
|
/>
|
||||||
|
) : item.filePath ? (
|
||||||
|
<button
|
||||||
|
key={item.meta.id}
|
||||||
|
type="button"
|
||||||
|
className="flex size-20 flex-col items-center justify-center gap-1 rounded-md border border-[var(--color-border)] bg-[var(--color-surface-raised)] transition-colors hover:border-[var(--color-border-emphasis)] hover:bg-[var(--color-surface)]"
|
||||||
|
title={item.meta.filename}
|
||||||
|
aria-label={`Open ${item.meta.filename}`}
|
||||||
|
onClick={(event) => {
|
||||||
|
event.stopPropagation();
|
||||||
|
revealFileInEditor(item.filePath!);
|
||||||
|
}}
|
||||||
|
onKeyDown={(event) => event.stopPropagation()}
|
||||||
|
>
|
||||||
|
<FileIcon fileName={item.meta.filename} className="size-5" />
|
||||||
|
<span className="max-w-[72px] truncate text-[9px] text-[var(--color-text-muted)]">
|
||||||
|
{item.meta.filename}
|
||||||
|
</span>
|
||||||
|
</button>
|
||||||
) : (
|
) : (
|
||||||
<div
|
<div
|
||||||
key={item.meta.id}
|
key={item.meta.id}
|
||||||
className="flex size-20 flex-col items-center justify-center gap-1 rounded-md border border-[var(--color-border)] bg-[var(--color-surface-raised)]"
|
className="flex size-20 flex-col items-center justify-center gap-1 rounded-md border border-[var(--color-border)] bg-[var(--color-surface-raised)]"
|
||||||
|
title={item.meta.filename}
|
||||||
>
|
>
|
||||||
<FileIcon fileName={item.meta.filename} className="size-5" />
|
<FileIcon fileName={item.meta.filename} className="size-5" />
|
||||||
<span className="max-w-[72px] truncate text-[9px] text-[var(--color-text-muted)]">
|
<span className="max-w-[72px] truncate text-[9px] text-[var(--color-text-muted)]">
|
||||||
|
|
|
||||||
|
|
@ -591,6 +591,8 @@ export interface AttachmentFileData {
|
||||||
id: string;
|
id: string;
|
||||||
data: string;
|
data: string;
|
||||||
mimeType: AttachmentMediaType;
|
mimeType: AttachmentMediaType;
|
||||||
|
/** Absolute path to the persisted attachment file when available. */
|
||||||
|
filePath?: string;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Lightweight metadata for a single tool call (for UI display in tooltips). */
|
/** Lightweight metadata for a single tool call (for UI display in tooltips). */
|
||||||
|
|
|
||||||
|
|
@ -1,11 +1,16 @@
|
||||||
import * as fs from 'fs';
|
import * as fs from 'fs';
|
||||||
import * as os from 'os';
|
import * as os from 'os';
|
||||||
import * as path from 'path';
|
import * as path from 'path';
|
||||||
|
|
||||||
import { afterEach, describe, expect, it } from 'vitest';
|
import { afterEach, describe, expect, it } from 'vitest';
|
||||||
|
|
||||||
import { TeamAttachmentStore } from '../../../src/main/services/team/TeamAttachmentStore';
|
import { TeamAttachmentStore } from '../../../src/main/services/team/TeamAttachmentStore';
|
||||||
import { TeamTaskAttachmentStore } from '../../../src/main/services/team/TeamTaskAttachmentStore';
|
import { TeamTaskAttachmentStore } from '../../../src/main/services/team/TeamTaskAttachmentStore';
|
||||||
|
import {
|
||||||
|
type ElectronUserDataMigrationApp,
|
||||||
|
getLegacyElectronUserDataCandidates,
|
||||||
|
migrateElectronUserDataDirectory,
|
||||||
|
shouldCopyElectronUserDataEntry,
|
||||||
|
} from '../../../src/main/utils/electronUserDataMigration';
|
||||||
import {
|
import {
|
||||||
getAppDataPath,
|
getAppDataPath,
|
||||||
getBackupsBasePath,
|
getBackupsBasePath,
|
||||||
|
|
@ -13,12 +18,6 @@ import {
|
||||||
getMcpServerBasePath,
|
getMcpServerBasePath,
|
||||||
setAppDataBasePath,
|
setAppDataBasePath,
|
||||||
} from '../../../src/main/utils/pathDecoder';
|
} from '../../../src/main/utils/pathDecoder';
|
||||||
import {
|
|
||||||
getLegacyElectronUserDataCandidates,
|
|
||||||
migrateElectronUserDataDirectory,
|
|
||||||
shouldCopyElectronUserDataEntry,
|
|
||||||
type ElectronUserDataMigrationApp,
|
|
||||||
} from '../../../src/main/utils/electronUserDataMigration';
|
|
||||||
|
|
||||||
class FakeElectronApp implements ElectronUserDataMigrationApp {
|
class FakeElectronApp implements ElectronUserDataMigrationApp {
|
||||||
setPathCalls: { name: string; value: string }[] = [];
|
setPathCalls: { name: string; value: string }[] = [];
|
||||||
|
|
@ -407,6 +406,7 @@ describe('electron userData migration', () => {
|
||||||
id: 'att-1',
|
id: 'att-1',
|
||||||
data: Buffer.from('message attachment').toString('base64'),
|
data: Buffer.from('message attachment').toString('base64'),
|
||||||
mimeType: 'text/plain',
|
mimeType: 'text/plain',
|
||||||
|
filePath: path.join(currentPath, 'data', 'attachments', 'team-a', 'msg-1', 'att-1--note.txt'),
|
||||||
},
|
},
|
||||||
]);
|
]);
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -7,11 +7,14 @@ import * as os from 'os';
|
||||||
import * as path from 'path';
|
import * as path from 'path';
|
||||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
|
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
|
||||||
|
|
||||||
import { getHomeDir, setClaudeBasePathOverride } from '../../../src/main/utils/pathDecoder';
|
|
||||||
|
|
||||||
import {
|
import {
|
||||||
isPathWithinRoot,
|
getHomeDir,
|
||||||
|
setAppDataBasePath,
|
||||||
|
setClaudeBasePathOverride,
|
||||||
|
} from '../../../src/main/utils/pathDecoder';
|
||||||
|
import {
|
||||||
isPathWithinAllowedDirectories,
|
isPathWithinAllowedDirectories,
|
||||||
|
isPathWithinRoot,
|
||||||
isWindowsReservedFileName,
|
isWindowsReservedFileName,
|
||||||
validateFileName,
|
validateFileName,
|
||||||
validateFilePath,
|
validateFilePath,
|
||||||
|
|
@ -22,14 +25,18 @@ import {
|
||||||
describe('pathValidation', () => {
|
describe('pathValidation', () => {
|
||||||
const homeDir = getHomeDir();
|
const homeDir = getHomeDir();
|
||||||
const claudeDir = path.join(homeDir, '.claude');
|
const claudeDir = path.join(homeDir, '.claude');
|
||||||
|
const appDataBasePath = path.join(homeDir, '.agent-teams-ai-test');
|
||||||
|
const appDataPath = path.join(appDataBasePath, 'data');
|
||||||
const testProjectPath = path.resolve('/home/user/my-project');
|
const testProjectPath = path.resolve('/home/user/my-project');
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
setClaudeBasePathOverride(claudeDir);
|
setClaudeBasePathOverride(claudeDir);
|
||||||
|
setAppDataBasePath(appDataBasePath);
|
||||||
});
|
});
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
setClaudeBasePathOverride(null);
|
setClaudeBasePathOverride(null);
|
||||||
|
setAppDataBasePath(null);
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('isPathWithinAllowedDirectories', () => {
|
describe('isPathWithinAllowedDirectories', () => {
|
||||||
|
|
@ -48,6 +55,15 @@ describe('pathValidation', () => {
|
||||||
).toBe(true);
|
).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should allow paths within app-owned data directory', () => {
|
||||||
|
expect(
|
||||||
|
isPathWithinAllowedDirectories(
|
||||||
|
path.join(appDataPath, 'attachments', 'team-a', 'msg-1', 'att-1--note.txt'),
|
||||||
|
testProjectPath
|
||||||
|
)
|
||||||
|
).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
it('should reject paths outside allowed directories', () => {
|
it('should reject paths outside allowed directories', () => {
|
||||||
expect(isPathWithinAllowedDirectories('/etc/passwd', testProjectPath)).toBe(false);
|
expect(isPathWithinAllowedDirectories('/etc/passwd', testProjectPath)).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,81 @@
|
||||||
|
import React, { act } from 'react';
|
||||||
|
import { createRoot } from 'react-dom/client';
|
||||||
|
|
||||||
|
import { AttachmentDisplay } from '@renderer/components/team/attachments/AttachmentDisplay';
|
||||||
|
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||||
|
|
||||||
|
const storeMocks = vi.hoisted(() => ({
|
||||||
|
revealFileInEditor: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('@features/localization/renderer', () => ({
|
||||||
|
useAppTranslation: () => ({ t: (key: string) => key }),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('@renderer/components/team/editor/FileIcon', () => ({
|
||||||
|
FileIcon: ({ fileName }: { fileName: string }) => React.createElement('span', null, fileName),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('@renderer/store', () => ({
|
||||||
|
useStore: (selector: (state: { revealFileInEditor: (filePath: string) => void }) => unknown) =>
|
||||||
|
selector({ revealFileInEditor: storeMocks.revealFileInEditor }),
|
||||||
|
}));
|
||||||
|
|
||||||
|
describe('AttachmentDisplay', () => {
|
||||||
|
afterEach(() => {
|
||||||
|
document.body.innerHTML = '';
|
||||||
|
vi.unstubAllGlobals();
|
||||||
|
storeMocks.revealFileInEditor.mockReset();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('opens persisted non-image attachments in the built-in editor', async () => {
|
||||||
|
vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true);
|
||||||
|
const getAttachments = vi.fn().mockResolvedValue([
|
||||||
|
{
|
||||||
|
id: 'att-1',
|
||||||
|
data: Buffer.from('verification').toString('base64'),
|
||||||
|
mimeType: 'text/markdown',
|
||||||
|
filePath: '/app/data/attachments/team-a/msg-1/att-1--verification.md',
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
Object.defineProperty(window, 'electronAPI', {
|
||||||
|
configurable: true,
|
||||||
|
value: { teams: { getAttachments } },
|
||||||
|
});
|
||||||
|
|
||||||
|
const host = document.createElement('div');
|
||||||
|
document.body.appendChild(host);
|
||||||
|
const root = createRoot(host);
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
root.render(
|
||||||
|
<AttachmentDisplay
|
||||||
|
teamName="team-a"
|
||||||
|
messageId="msg-1"
|
||||||
|
attachments={[
|
||||||
|
{
|
||||||
|
id: 'att-1',
|
||||||
|
filename: 'verification.md',
|
||||||
|
mimeType: 'text/markdown',
|
||||||
|
size: 12,
|
||||||
|
},
|
||||||
|
]}
|
||||||
|
/>
|
||||||
|
);
|
||||||
|
});
|
||||||
|
await act(async () => {
|
||||||
|
await Promise.resolve();
|
||||||
|
});
|
||||||
|
|
||||||
|
const button = host.querySelector<HTMLButtonElement>('button[aria-label="Open verification.md"]');
|
||||||
|
expect(button).not.toBeNull();
|
||||||
|
|
||||||
|
await act(async () => {
|
||||||
|
button?.click();
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(storeMocks.revealFileInEditor).toHaveBeenCalledWith(
|
||||||
|
'/app/data/attachments/team-a/msg-1/att-1--verification.md'
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
Loading…
Reference in a new issue