mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
fix(projects): list symlinked folders as folders in the remote Add Project browser (#29658)
A symlink to a directory was listed as a file on SSH and managed hosts, so projects behind a linked path could not be reached. Follow links when classifying entries; broken links and loops stay files. Co-authored-by: Gideon <jjin6573@naver.com>
This commit is contained in:
@@ -70,7 +70,7 @@ describe('registerSshBrowseHandler', () => {
|
||||
]
|
||||
})
|
||||
expect(exec).toHaveBeenCalledWith(
|
||||
expect.stringContaining('\ncd "$HOME" && pwd && command ls -1Ap\n')
|
||||
expect.stringContaining('\ncd "$HOME" && pwd && entries=$(command ls -1A) && ')
|
||||
)
|
||||
expect(channel.listenerCount('data')).toBe(0)
|
||||
expect(channel.listenerCount('exit')).toBe(0)
|
||||
@@ -160,7 +160,7 @@ describe('registerSshBrowseHandler', () => {
|
||||
entries: []
|
||||
})
|
||||
expect(exec).toHaveBeenCalledWith(
|
||||
expect.stringContaining("\ncd '/tmp/it'\\''s here' && pwd && command ls -1Ap\n")
|
||||
expect.stringContaining("\ncd '/tmp/it'\\''s here' && pwd && entries=$(command ls -1A) && ")
|
||||
)
|
||||
})
|
||||
|
||||
@@ -203,7 +203,7 @@ describe('registerSshBrowseHandler', () => {
|
||||
expect(exec).toHaveBeenCalledTimes(2)
|
||||
expect(exec).toHaveBeenNthCalledWith(
|
||||
1,
|
||||
expect.stringContaining("\ncd 'C:/Users/alice' && pwd && command ls -1Ap\n")
|
||||
expect.stringContaining("\ncd 'C:/Users/alice' && pwd && entries=$(command ls -1A) && ")
|
||||
)
|
||||
expect(exec.mock.calls[1]?.[0]).toMatch(/^powershell\.exe /)
|
||||
expect(exec.mock.calls[1]?.[1]).toEqual({ wrapCommand: false })
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
import { mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
|
||||
import { RuntimeServerEnvironmentCommands } from './runtime-server-environment-commands'
|
||||
|
||||
const dirLinkType = process.platform === 'win32' ? 'junction' : 'dir'
|
||||
|
||||
describe('RuntimeServerEnvironmentCommands.browseDirectory', () => {
|
||||
let root: string
|
||||
|
||||
beforeEach(async () => {
|
||||
root = await mkdtemp(join(tmpdir(), 'orca-server-browse-symlink-'))
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
await rm(root, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
it.skipIf(process.platform === 'win32')(
|
||||
'lists a symlinked directory as a directory and keeps broken links and loops as files',
|
||||
async () => {
|
||||
await mkdir(join(root, 'real-dir'))
|
||||
await writeFile(join(root, 'real-file.txt'), 'x\n')
|
||||
await symlink(join(root, 'real-dir'), join(root, 'linked-dir'), dirLinkType)
|
||||
await symlink(join(root, 'real-file.txt'), join(root, 'linked-file'))
|
||||
await symlink(join(root, 'missing-target'), join(root, 'broken-link'))
|
||||
await symlink(join(root, 'loop-b'), join(root, 'loop-a'))
|
||||
await symlink(join(root, 'loop-a'), join(root, 'loop-b'))
|
||||
|
||||
const result = await new RuntimeServerEnvironmentCommands().browseDirectory(root)
|
||||
|
||||
expect(result.entries).toEqual([
|
||||
{ name: 'linked-dir', isDirectory: true, isSymlink: true },
|
||||
{ name: 'real-dir', isDirectory: true, isSymlink: false },
|
||||
{ name: 'broken-link', isDirectory: false, isSymlink: true },
|
||||
{ name: 'linked-file', isDirectory: false, isSymlink: true },
|
||||
{ name: 'loop-a', isDirectory: false, isSymlink: true },
|
||||
{ name: 'loop-b', isDirectory: false, isSymlink: true },
|
||||
{ name: 'real-file.txt', isDirectory: false, isSymlink: false }
|
||||
])
|
||||
}
|
||||
)
|
||||
|
||||
it('navigates into a symlinked directory', async () => {
|
||||
await mkdir(join(root, 'real-dir', 'project'), { recursive: true })
|
||||
await symlink(join(root, 'real-dir'), join(root, 'linked-dir'), dirLinkType)
|
||||
|
||||
const result = await new RuntimeServerEnvironmentCommands().browseDirectory(
|
||||
join(root, 'linked-dir')
|
||||
)
|
||||
|
||||
expect(result.resolvedPath).toBe(join(root, 'linked-dir'))
|
||||
expect(result.entries).toEqual([{ name: 'project', isDirectory: true, isSymlink: false }])
|
||||
})
|
||||
})
|
||||
@@ -1,6 +1,7 @@
|
||||
import { readdir, stat } from 'node:fs/promises'
|
||||
import { homedir } from 'node:os'
|
||||
import { isAbsolute, resolve } from 'node:path'
|
||||
import type { Dirent } from 'node:fs'
|
||||
import { isAbsolute, join, resolve } from 'node:path'
|
||||
import type { DirEntry, FilesystemPathFlavor } from '../../shared/filesystem-entry-types'
|
||||
import { sortDirEntries } from '../../shared/file-name-sort'
|
||||
import { probeGitAvailability } from '../git/git-availability'
|
||||
@@ -24,6 +25,21 @@ function resolveServerBrowsePath(pathValue: string): string {
|
||||
return resolve(homedir(), trimmed)
|
||||
}
|
||||
|
||||
// Why: Dirent.isDirectory() is false for a symlink, so a linked folder would be unreachable; follow it, and treat broken links or loops (ENOENT/ELOOP) as files.
|
||||
async function isBrowseEntryDirectory(dirPath: string, entry: Dirent): Promise<boolean> {
|
||||
if (entry.isDirectory()) {
|
||||
return true
|
||||
}
|
||||
if (!entry.isSymbolicLink()) {
|
||||
return false
|
||||
}
|
||||
try {
|
||||
return (await stat(join(dirPath, entry.name))).isDirectory()
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
export class RuntimeServerEnvironmentCommands {
|
||||
async browseDirectory(pathValue: string): Promise<{
|
||||
resolvedPath: string
|
||||
@@ -39,13 +55,15 @@ export class RuntimeServerEnvironmentCommands {
|
||||
throw new Error(`${dirPath} is not a directory`)
|
||||
}
|
||||
const entries = await readdir(dirPath, { withFileTypes: true })
|
||||
const mapped = entries
|
||||
.filter((entry) => entry.name !== '.' && entry.name !== '..')
|
||||
.map((entry) => ({
|
||||
name: entry.name,
|
||||
isDirectory: entry.isDirectory(),
|
||||
isSymlink: entry.isSymbolicLink()
|
||||
}))
|
||||
const mapped = await Promise.all(
|
||||
entries
|
||||
.filter((entry) => entry.name !== '.' && entry.name !== '..')
|
||||
.map(async (entry) => ({
|
||||
name: entry.name,
|
||||
isDirectory: await isBrowseEntryDirectory(dirPath, entry),
|
||||
isSymlink: entry.isSymbolicLink()
|
||||
}))
|
||||
)
|
||||
sortDirEntries(mapped)
|
||||
return {
|
||||
resolvedPath: dirPath,
|
||||
|
||||
@@ -0,0 +1,98 @@
|
||||
import { spawn } from 'node:child_process'
|
||||
import { EventEmitter } from 'node:events'
|
||||
import { chmod, mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
|
||||
import { browseSshDirectory } from './ssh-remote-directory-browse'
|
||||
|
||||
// Runs the generated remote command in a real local `sh`, standing in for the SSH exec channel.
|
||||
function shellBackedManager(): Parameters<typeof browseSshDirectory>[0] {
|
||||
const exec = async (command: string): Promise<EventEmitter & { stderr: EventEmitter }> => {
|
||||
const child = spawn('/bin/sh', ['-c', command], { stdio: ['ignore', 'pipe', 'pipe'] })
|
||||
const channel = Object.assign(new EventEmitter(), { stderr: new EventEmitter() })
|
||||
child.stdout.on('data', (data: Buffer) => channel.emit('data', data))
|
||||
child.stderr.on('data', (data: Buffer) => channel.stderr.emit('data', data))
|
||||
child.on('close', (code) => {
|
||||
channel.emit('exit', code)
|
||||
channel.emit('close', code)
|
||||
})
|
||||
return channel
|
||||
}
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: browseSshDirectory only calls getConnection(id).exec(command).
|
||||
return { getConnection: () => ({ exec }) } as never
|
||||
}
|
||||
|
||||
describe.skipIf(process.platform === 'win32')('browseSshDirectory over a POSIX shell', () => {
|
||||
let root: string
|
||||
|
||||
beforeEach(async () => {
|
||||
root = await mkdtemp(join(tmpdir(), 'orca-ssh-browse-symlink-'))
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
await rm(root, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
it('marks a symlinked directory as a directory and keeps broken links and loops as files', async () => {
|
||||
await mkdir(join(root, 'real-dir'))
|
||||
await writeFile(join(root, 'real-file.txt'), 'x\n')
|
||||
await writeFile(join(root, '.hidden'), 'x\n')
|
||||
await writeFile(join(root, 'name with space'), 'x\n')
|
||||
await symlink(join(root, 'real-dir'), join(root, 'linked-dir'))
|
||||
await symlink(join(root, 'real-file.txt'), join(root, 'linked-file'))
|
||||
await symlink(join(root, 'missing-target'), join(root, 'broken-link'))
|
||||
await symlink(join(root, 'loop-b'), join(root, 'loop-a'))
|
||||
await symlink(join(root, 'loop-a'), join(root, 'loop-b'))
|
||||
|
||||
const result = await browseSshDirectory(shellBackedManager(), 'ssh-1', root)
|
||||
|
||||
expect(result.pathFlavor).toBe('posix')
|
||||
expect(result.entries).toEqual([
|
||||
{ name: 'linked-dir', isDirectory: true },
|
||||
{ name: 'real-dir', isDirectory: true },
|
||||
{ name: '.hidden', isDirectory: false },
|
||||
{ name: 'broken-link', isDirectory: false },
|
||||
{ name: 'linked-file', isDirectory: false },
|
||||
{ name: 'loop-a', isDirectory: false },
|
||||
{ name: 'loop-b', isDirectory: false },
|
||||
{ name: 'name with space', isDirectory: false },
|
||||
{ name: 'real-file.txt', isDirectory: false }
|
||||
])
|
||||
})
|
||||
|
||||
it('navigates into a symlinked directory and lists an empty one as empty', async () => {
|
||||
await mkdir(join(root, 'real-dir', 'project'), { recursive: true })
|
||||
await mkdir(join(root, 'empty-real'))
|
||||
await symlink(join(root, 'real-dir'), join(root, 'linked-dir'))
|
||||
await symlink(join(root, 'empty-real'), join(root, 'linked-empty'))
|
||||
|
||||
const linked = await browseSshDirectory(shellBackedManager(), 'ssh-1', join(root, 'linked-dir'))
|
||||
const empty = await browseSshDirectory(
|
||||
shellBackedManager(),
|
||||
'ssh-1',
|
||||
join(root, 'linked-empty')
|
||||
)
|
||||
|
||||
expect(linked.entries).toEqual([{ name: 'project', isDirectory: true }])
|
||||
expect(empty.entries).toEqual([])
|
||||
})
|
||||
|
||||
it('still fails instead of looking empty when the directory cannot be listed', async () => {
|
||||
await expect(
|
||||
browseSshDirectory(shellBackedManager(), 'ssh-1', join(root, 'missing'))
|
||||
).rejects.toThrow()
|
||||
// Execute-only: `cd` and `pwd` succeed but `ls` cannot read it.
|
||||
const unreadable = join(root, 'unreadable')
|
||||
await mkdir(unreadable, { mode: 0o100 })
|
||||
try {
|
||||
if (process.getuid?.() !== 0) {
|
||||
await expect(
|
||||
browseSshDirectory(shellBackedManager(), 'ssh-1', unreadable)
|
||||
).rejects.toThrow()
|
||||
}
|
||||
} finally {
|
||||
await chmod(unreadable, 0o700)
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -62,14 +62,23 @@ export async function browseSshDirectory(
|
||||
}
|
||||
}
|
||||
|
||||
const POSIX_LIST_WITH_DIR_MARKERS =
|
||||
'entries=$(command ls -1A) && printf \'%s\\n\' "$entries" | while IFS= read -r f; do ' +
|
||||
'if [ -d "$f" ]; then printf \'%s/\\n\' "$f"; else printf \'%s\\n\' "$f"; fi; done'
|
||||
|
||||
type SshBrowseConnection = NonNullable<ReturnType<SshConnectionManager['getConnection']>>
|
||||
|
||||
function browseWithPosixShell(
|
||||
conn: SshBrowseConnection,
|
||||
dirPath: string
|
||||
): Promise<RemoteBrowseResult> {
|
||||
// Why: `command ls` skips aliases; `&&` makes a failing ls exit non-zero (not look empty); -1Ap = one-per-line + trailing / on dirs.
|
||||
return runBrowseCommand(conn, `cd ${shellEscape(dirPath)} && pwd && command ls -1Ap`, 'posix')
|
||||
// Why: `command ls` skips aliases; capturing it keeps a failing ls exiting non-zero (not looking empty).
|
||||
// The trailing / comes from `[ -d ]`, which follows symlinks (ls -p doesn't); broken links and loops test false.
|
||||
return runBrowseCommand(
|
||||
conn,
|
||||
`cd ${shellEscape(dirPath)} && pwd && ${POSIX_LIST_WITH_DIR_MARKERS}`,
|
||||
'posix'
|
||||
)
|
||||
}
|
||||
|
||||
function browseWithWindowsPowerShell(
|
||||
|
||||
Reference in New Issue
Block a user