diff --git a/src/main/services/team/TeamAttachmentStore.ts b/src/main/services/team/TeamAttachmentStore.ts index 8b78943f..1b0b05ae 100644 --- a/src/main/services/team/TeamAttachmentStore.ts +++ b/src/main/services/team/TeamAttachmentStore.ts @@ -217,6 +217,7 @@ export class TeamAttachmentStore { id: entry.id, data: buffer.toString('base64'), mimeType, + filePath, }); } catch (error) { if ((error as NodeJS.ErrnoException).code === 'ENOENT') continue; diff --git a/src/main/utils/pathValidation.ts b/src/main/utils/pathValidation.ts index 9e58601d..5590b9cd 100644 --- a/src/main/utils/pathValidation.ts +++ b/src/main/utils/pathValidation.ts @@ -11,7 +11,7 @@ import * as fs from 'fs'; import * as path from 'path'; -import { getClaudeBasePath, getHomeDir } from './pathDecoder'; +import { getAppDataPath, getClaudeBasePath, getHomeDir } from './pathDecoder'; /** * Sensitive file patterns that should never be accessible. @@ -101,6 +101,7 @@ export function matchesSensitivePattern(normalizedPath: string): boolean { * Allowed directories: * - The project path itself * - The ~/.claude directory (for session data) + * - The app-owned data directory (attachments, task attachments) * * @param normalizedPath - The normalized absolute path to check * @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 claudeDir = getClaudeBasePath(); const normalizedClaudeDir = normalizeForCompare(claudeDir, isWindows); + const appDataDir = getAppDataPath(); + const normalizedAppDataDir = normalizeForCompare(appDataDir, isWindows); // Always allow access to ~/.claude for session data if (isPathWithinRoot(normalizedTarget, normalizedClaudeDir)) { 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 (projectPath) { const normalizedProjectPath = normalizeForCompare(projectPath, isWindows); @@ -137,7 +145,7 @@ export function isPathWithinAllowedDirectories( * Security checks performed: * 1. Path must be absolute * 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 * * @param filePath - The file path to validate diff --git a/src/renderer/components/team/attachments/AttachmentDisplay.tsx b/src/renderer/components/team/attachments/AttachmentDisplay.tsx index dd03cf65..0cff8048 100644 --- a/src/renderer/components/team/attachments/AttachmentDisplay.tsx +++ b/src/renderer/components/team/attachments/AttachmentDisplay.tsx @@ -2,6 +2,7 @@ import { useEffect, useState } from 'react'; import { useAppTranslation } from '@features/localization/renderer'; import { FileIcon } from '@renderer/components/team/editor/FileIcon'; +import { useStore } from '@renderer/store'; import { isImageMime } from '@renderer/utils/attachmentUtils'; import { Loader2 } from 'lucide-react'; @@ -22,6 +23,7 @@ export const AttachmentDisplay = ({ attachments, }: AttachmentDisplayProps): React.JSX.Element | null => { const { t } = useAppTranslation('team'); + const revealFileInEditor = useStore((s) => s.revealFileInEditor); const [state, setState] = useState<{ loaded: AttachmentFileData[]; loading: boolean; @@ -74,10 +76,16 @@ export const AttachmentDisplay = ({ return { meta, dataUrl: isImage ? `data:${data.mimeType};base64,${data.data}` : undefined, + filePath: data.filePath ?? meta.filePath, 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; @@ -107,10 +115,29 @@ export const AttachmentDisplay = ({ : undefined } /> + ) : item.filePath ? ( + ) : (
diff --git a/src/shared/types/team.ts b/src/shared/types/team.ts index cff49c08..7d690285 100644 --- a/src/shared/types/team.ts +++ b/src/shared/types/team.ts @@ -591,6 +591,8 @@ export interface AttachmentFileData { id: string; data: string; 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). */ diff --git a/test/main/utils/electronUserDataMigration.test.ts b/test/main/utils/electronUserDataMigration.test.ts index 3b00d4ff..a7ee7e17 100644 --- a/test/main/utils/electronUserDataMigration.test.ts +++ b/test/main/utils/electronUserDataMigration.test.ts @@ -1,11 +1,16 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; - import { afterEach, describe, expect, it } from 'vitest'; import { TeamAttachmentStore } from '../../../src/main/services/team/TeamAttachmentStore'; import { TeamTaskAttachmentStore } from '../../../src/main/services/team/TeamTaskAttachmentStore'; +import { + type ElectronUserDataMigrationApp, + getLegacyElectronUserDataCandidates, + migrateElectronUserDataDirectory, + shouldCopyElectronUserDataEntry, +} from '../../../src/main/utils/electronUserDataMigration'; import { getAppDataPath, getBackupsBasePath, @@ -13,12 +18,6 @@ import { getMcpServerBasePath, setAppDataBasePath, } from '../../../src/main/utils/pathDecoder'; -import { - getLegacyElectronUserDataCandidates, - migrateElectronUserDataDirectory, - shouldCopyElectronUserDataEntry, - type ElectronUserDataMigrationApp, -} from '../../../src/main/utils/electronUserDataMigration'; class FakeElectronApp implements ElectronUserDataMigrationApp { setPathCalls: { name: string; value: string }[] = []; @@ -407,6 +406,7 @@ describe('electron userData migration', () => { id: 'att-1', data: Buffer.from('message attachment').toString('base64'), mimeType: 'text/plain', + filePath: path.join(currentPath, 'data', 'attachments', 'team-a', 'msg-1', 'att-1--note.txt'), }, ]); diff --git a/test/main/utils/pathValidation.test.ts b/test/main/utils/pathValidation.test.ts index 9965f72f..de8bbb77 100644 --- a/test/main/utils/pathValidation.test.ts +++ b/test/main/utils/pathValidation.test.ts @@ -7,11 +7,14 @@ import * as os from 'os'; import * as path from 'path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { getHomeDir, setClaudeBasePathOverride } from '../../../src/main/utils/pathDecoder'; - import { - isPathWithinRoot, + getHomeDir, + setAppDataBasePath, + setClaudeBasePathOverride, +} from '../../../src/main/utils/pathDecoder'; +import { isPathWithinAllowedDirectories, + isPathWithinRoot, isWindowsReservedFileName, validateFileName, validateFilePath, @@ -22,14 +25,18 @@ import { describe('pathValidation', () => { const homeDir = getHomeDir(); 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'); beforeEach(() => { setClaudeBasePathOverride(claudeDir); + setAppDataBasePath(appDataBasePath); }); afterEach(() => { setClaudeBasePathOverride(null); + setAppDataBasePath(null); }); describe('isPathWithinAllowedDirectories', () => { @@ -48,6 +55,15 @@ describe('pathValidation', () => { ).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', () => { expect(isPathWithinAllowedDirectories('/etc/passwd', testProjectPath)).toBe(false); }); diff --git a/test/renderer/components/team/attachments/AttachmentDisplay.test.tsx b/test/renderer/components/team/attachments/AttachmentDisplay.test.tsx new file mode 100644 index 00000000..a942c317 --- /dev/null +++ b/test/renderer/components/team/attachments/AttachmentDisplay.test.tsx @@ -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( + + ); + }); + await act(async () => { + await Promise.resolve(); + }); + + const button = host.querySelector('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' + ); + }); +});