From ad22f5fc30b75ce22f48bf97ae1aabcfa345fe2f Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Fri, 15 May 2026 22:08:41 -0700 Subject: [PATCH] fix: open markdown links over ssh (#2067) --- .../src/components/editor/MarkdownPreview.tsx | 24 ++- src/renderer/src/store/slices/editor.test.ts | 140 +++++++++++++++++- src/renderer/src/store/slices/editor.ts | 62 ++++++-- 3 files changed, 202 insertions(+), 24 deletions(-) diff --git a/src/renderer/src/components/editor/MarkdownPreview.tsx b/src/renderer/src/components/editor/MarkdownPreview.tsx index 29cda7cd5..e1eb444d1 100644 --- a/src/renderer/src/components/editor/MarkdownPreview.tsx +++ b/src/renderer/src/components/editor/MarkdownPreview.tsx @@ -67,6 +67,7 @@ import { openHttpLink } from '@/lib/http-link-routing' import { isLocalPathOpenBlocked, showLocalPathOpenBlockedToast } from '@/lib/local-path-open-guard' import { markdownPreviewUrlTransform } from './markdown-preview-url-transform' import { settingsForRuntimeOwner } from '@/runtime/runtime-rpc-client' +import { statRuntimePath } from '@/runtime/runtime-file-client' import { buildMarkdownTableOfContents } from './markdown-table-of-contents' import { MarkdownTableOfContentsPanel } from './MarkdownTableOfContentsPanel' import { getDiffCommentLineLabel, isMarkdownComment } from '@/lib/diff-comment-compat' @@ -752,7 +753,7 @@ export default function MarkdownPreview({ ) } - const handleClick = (event: React.MouseEvent): void => { + const handleClick = async (event: React.MouseEvent): Promise => { if (!href) { return } @@ -885,6 +886,27 @@ export default function MarkdownPreview({ const relativePath = absolutePath.slice(targetWorktree.path.length + 1) const language = detectLanguage(absolutePath) + try { + const stats = await statRuntimePath( + { + settings: settingsForRuntimeOwner( + useAppStore.getState().settings, + sourceRuntimeEnvironmentId + ), + worktreeId: targetWorktree.id, + worktreePath: targetWorktree.path, + connectionId: getConnectionId(targetWorktree.id) ?? undefined + }, + absolutePath + ) + if (stats.isDirectory) { + toast.error(`Cannot open directory: ${relativePath}`) + return + } + } catch { + toast.error(`File not found: ${relativePath}`) + return + } // Why: line targets like #L10 and path.ts:10 should reveal in Monaco, // not open a preview tab or a literal path with the suffix included. diff --git a/src/renderer/src/store/slices/editor.test.ts b/src/renderer/src/store/slices/editor.test.ts index c9e842d01..384377ff0 100644 --- a/src/renderer/src/store/slices/editor.test.ts +++ b/src/renderer/src/store/slices/editor.test.ts @@ -1375,6 +1375,7 @@ describe('createEditorSlice activateMarkdownLink', () => { const openFileUriMock = vi.fn() const pathExistsMock = vi.fn() const authorizeExternalPathMock = vi.fn() + const fsStatMock = vi.fn() const runtimeEnvironmentCallMock = vi.fn() const runtimeEnvironmentTransportCallMock = vi.fn() @@ -1386,6 +1387,14 @@ describe('createEditorSlice activateMarkdownLink', () => { pathExistsMock.mockReset() pathExistsMock.mockResolvedValue(true) authorizeExternalPathMock.mockReset() + fsStatMock.mockReset() + fsStatMock.mockImplementation(async ({ filePath }: { filePath: string }) => { + const exists = await pathExistsMock(filePath) + if (!exists) { + throw new Error('File not found') + } + return { size: 1, isDirectory: false, mtime: 1 } + }) runtimeEnvironmentCallMock.mockReset() runtimeEnvironmentTransportCallMock.mockReset() runtimeEnvironmentCallMock.mockResolvedValue({ @@ -1411,13 +1420,7 @@ describe('createEditorSlice activateMarkdownLink', () => { }, fs: { authorizeExternalPath: authorizeExternalPathMock, - stat: vi.fn(async ({ filePath }: { filePath: string }) => { - const exists = await pathExistsMock(filePath) - if (!exists) { - throw new Error('File not found') - } - return { size: 1, isDirectory: false, mtime: 1 } - }) + stat: fsStatMock }, runtimeEnvironments: { call: runtimeEnvironmentTransportCallMock @@ -1496,6 +1499,78 @@ describe('createEditorSlice activateMarkdownLink', () => { ]) }) + it('stats SSH markdown links through the source worktree connection before opening', async () => { + const store = createEditorStore() + pathExistsMock.mockResolvedValue(true) + store.setState({ + repos: [ + { + id: 'repo1', + path: '/repo', + displayName: 'Repo', + badgeColor: '#000', + addedAt: 0, + connectionId: 'ssh-1' + } + ], + worktreesByRepo: { + repo1: [ + { + id: 'wt-1', + repoId: 'repo1', + path: '/repo', + branch: 'refs/heads/main', + head: 'abc', + isBare: false, + isMainWorktree: true, + displayName: 'main', + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 0 + } + ] + } + } as Partial) + + await store.getState().activateMarkdownLink('./guide.md', { + sourceFilePath: '/repo/docs/note.md', + worktreeId: 'wt-1', + worktreeRoot: '/repo' + }) + + expect(fsStatMock).toHaveBeenCalledWith({ + filePath: '/repo/docs/guide.md', + connectionId: 'ssh-1' + }) + expect(store.getState().openFiles).toEqual([ + expect.objectContaining({ + filePath: '/repo/docs/guide.md', + mode: 'edit', + isPreview: true + }) + ]) + }) + + it('does not open linked markdown directories as files', async () => { + const store = createEditorStore() + fsStatMock.mockResolvedValueOnce({ size: 1, isDirectory: true, mtime: 1 }) + + await store.getState().activateMarkdownLink('./guide.md', { + sourceFilePath: '/repo/docs/note.md', + worktreeId: 'wt-1', + worktreeRoot: '/repo' + }) + + expect(store.getState().openFiles).toEqual([]) + expect(toastErrorMock).toHaveBeenCalledWith('Cannot open directory: docs/guide.md') + }) + it('can open a file without adopting the currently active runtime owner', () => { const store = createEditorStore() store.setState({ @@ -1644,6 +1719,57 @@ describe('createEditorSlice activateMarkdownLink', () => { expect(openFileUriMock).not.toHaveBeenCalled() }) + it('blocks external file URLs from SSH markdown sources', async () => { + const store = createEditorStore() + store.setState({ + repos: [ + { + id: 'repo1', + path: '/repo', + displayName: 'Repo', + badgeColor: '#000', + addedAt: 0, + connectionId: 'ssh-1' + } + ], + worktreesByRepo: { + repo1: [ + { + id: 'wt-1', + repoId: 'repo1', + path: '/repo', + branch: 'refs/heads/main', + head: 'abc', + isBare: false, + isMainWorktree: true, + displayName: 'main', + comment: '', + linkedIssue: null, + linkedPR: null, + linkedLinearIssue: null, + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 0, + lastActivityAt: 0 + } + ] + } + } as Partial) + + await store.getState().activateMarkdownLink('file:///tmp/image.png', { + sourceFilePath: '/repo/docs/note.md', + worktreeId: 'wt-1', + worktreeRoot: '/repo' + }) + + expect(authorizeExternalPathMock).not.toHaveBeenCalled() + expect(store.getState().openFiles).toEqual([]) + expect(toastErrorMock).toHaveBeenCalledWith( + 'Opening remote paths in the local OS is not available.' + ) + }) + it('activates same-file line anchors via setActiveFile without opening a new tab', async () => { const store = createEditorStore() pathExistsMock.mockResolvedValue(true) diff --git a/src/renderer/src/store/slices/editor.ts b/src/renderer/src/store/slices/editor.ts index 7c18af097..19f96cfec 100644 --- a/src/renderer/src/store/slices/editor.ts +++ b/src/renderer/src/store/slices/editor.ts @@ -5,6 +5,7 @@ import { joinPath } from '@/lib/path' import { toast } from 'sonner' import { resolveMarkdownLinkTarget } from '@/components/editor/markdown-internal-links' import { openHttpLink } from '@/lib/http-link-routing' +import { isLocalPathOpenBlocked, showLocalPathOpenBlockedToast } from '@/lib/local-path-open-guard' import { detectLanguage } from '@/lib/language-detect' import type { GitBranchChangeEntry, @@ -32,7 +33,7 @@ import { import { deleteRuntimePath, deleteRuntimeRelativePath, - runtimePathExists + statRuntimePath } from '@/runtime/runtime-file-client' import { settingsForRuntimeOwner } from '@/runtime/runtime-rpc-client' import { findWorktreeById, getRepoIdFromWorktreeId } from './worktree-helpers' @@ -586,6 +587,13 @@ function deleteUntouchedUntitledFile(state: AppState, file: OpenFile): void { .catch(() => {}) } +function getWorktreeConnectionId(state: AppState, worktreeId: string): string | undefined { + const worktree = findWorktreeById(state.worktreesByRepo ?? {}, worktreeId) + const repoId = worktree?.repoId ?? getRepoIdFromWorktreeId(worktreeId) + const repo = (state.repos ?? []).find((candidate) => candidate.id === repoId) + return repo?.connectionId ?? undefined +} + export const createEditorSlice: StateCreator = (set, get) => ({ editorDrafts: {}, setEditorDraft: (fileId, content) => @@ -2317,11 +2325,23 @@ export const createEditorSlice: StateCreator = (s setPendingEditorReveal: (reveal) => set({ pendingEditorReveal: reveal }), activateMarkdownLink: async (rawHref, ctx) => { + const initialState = get() const sourceRuntimeEnvironmentId = ctx.runtimeEnvironmentId ?? - get().openFiles.find((file) => file.filePath === ctx.sourceFilePath)?.runtimeEnvironmentId ?? + initialState.openFiles.find((file) => file.filePath === ctx.sourceFilePath) + ?.runtimeEnvironmentId ?? null - const sourceSettings = settingsForRuntimeOwner(get().settings, sourceRuntimeEnvironmentId) + const sourceSettings = settingsForRuntimeOwner( + initialState.settings, + sourceRuntimeEnvironmentId + ) + const sourceConnectionId = getWorktreeConnectionId(initialState, ctx.worktreeId) + const fileContext = { + settings: sourceSettings, + worktreeId: ctx.worktreeId, + worktreePath: ctx.worktreeRoot, + connectionId: sourceConnectionId + } const target = resolveMarkdownLinkTarget(rawHref, ctx.sourceFilePath, ctx.worktreeRoot) if (!target) { return @@ -2336,17 +2356,28 @@ export const createEditorSlice: StateCreator = (s if (target.kind === 'file') { const { line, column } = target if (target.relativePath === undefined) { - if (sourceSettings?.activeRuntimeEnvironmentId?.trim()) { + if (isLocalPathOpenBlocked(sourceSettings, { connectionId: sourceConnectionId })) { // Why: a file:// link outside the worktree is a client-local escape - // hatch. Remote runtime editors must not authorize/open client paths - // as though the server could read them. - toast.error('External local file links are not available for remote runtime files yet.') + // hatch. Remote runtime/SSH editors must not treat server paths as client paths. + showLocalPathOpenBlockedToast() return } // Why: terminal file links already authorize clicked external paths // before opening them in Orca. Markdown file:// links need the same // user-gesture authorization so /tmp screenshots can use ImageViewer. await window.api.fs.authorizeExternalPath({ targetPath: target.absolutePath }) + } else { + let stats: { isDirectory: boolean } + try { + stats = await statRuntimePath(fileContext, target.absolutePath) + } catch { + toast.error(`File not found: ${target.relativePath}`) + return + } + if (stats.isDirectory) { + toast.error(`Cannot open directory: ${target.relativePath}`) + return + } } get().openFile( @@ -2372,18 +2403,17 @@ export const createEditorSlice: StateCreator = (s // target.kind === 'markdown' const { absolutePath, relativePath, line, column } = target - const exists = await runtimePathExists( - { - settings: sourceSettings, - worktreeId: ctx.worktreeId, - worktreePath: ctx.worktreeRoot - }, - absolutePath - ) - if (!exists) { + let stats: { isDirectory: boolean } + try { + stats = await statRuntimePath(fileContext, absolutePath) + } catch { toast.error(`File not found: ${relativePath}`) return } + if (stats.isDirectory) { + toast.error(`Cannot open directory: ${relativePath}`) + return + } const state = get() const existing = state.openFiles.find(