fix: cross-platform path handling and tests
- fs-tree joins listed paths with the platform separator (Windows rows previously leaked mixed separators) - media route containment check goes through isWithin(): separator- tolerant and case-insensitive on win32, so case-mismatched Windows paths are not misclassified - relativeTo compares case-insensitively (Windows / case-insensitive macOS volumes) while keeping the caller's own casing in the output - tests: platform-conditional path assertions, os.tmpdir() instead of /tmp, isWithin/relativeTo/resolveSidebarPath win32 coverage
This commit is contained in:
+6
-1
@@ -11,6 +11,11 @@
|
||||
* @param path - an absolute entry path from the fs-tree.
|
||||
* @returns the relative path with '/' separators ('.' for the cwd itself),
|
||||
* or `path` unchanged when it lies outside the cwd.
|
||||
*
|
||||
* The prefix test is case-insensitive: Windows paths (and macOS's
|
||||
* case-insensitive volumes) may arrive with different casing than the cwd
|
||||
* row, and the containment decision must not depend on it. The returned
|
||||
* relative text keeps the caller's own casing.
|
||||
*/
|
||||
export function relativeTo(cwd: string, path: string): string {
|
||||
const base = cwd.replace(/[\\/]+$/, '')
|
||||
@@ -18,6 +23,6 @@ export function relativeTo(cwd: string, path: string): string {
|
||||
const nBase = norm(base)
|
||||
const nPath = norm(path)
|
||||
if (nPath === nBase) return '.'
|
||||
if (nPath.startsWith(`${nBase}/`)) return nPath.slice(nBase.length + 1)
|
||||
if (nPath.toLowerCase().startsWith(`${nBase.toLowerCase()}/`)) return nPath.slice(nBase.length + 1)
|
||||
return path
|
||||
}
|
||||
|
||||
+25
-2
@@ -6,7 +6,7 @@
|
||||
* dirent says, keeping the read cheap for arbitrarily large levels.
|
||||
*/
|
||||
import { opendir } from 'node:fs/promises'
|
||||
import { basename, dirname, resolve } from 'node:path'
|
||||
import { basename, dirname, join, resolve } from 'node:path'
|
||||
import { SidebarError } from './wire.ts'
|
||||
|
||||
/** One explorer row. */
|
||||
@@ -54,7 +54,9 @@ export async function listDirectory(path: string, maxEntries = 1000): Promise<Si
|
||||
}
|
||||
rows.push({
|
||||
name: dirent.name,
|
||||
path: `${path}/${dirent.name}`,
|
||||
// Platform join: on Windows the level path uses '\' — a hardcoded '/'
|
||||
// would leak mixed separators into every row's path.
|
||||
path: join(path, dirent.name),
|
||||
isDir: dirent.isDirectory(),
|
||||
hidden: dirent.name.startsWith('.'),
|
||||
})
|
||||
@@ -86,6 +88,27 @@ export function requireAbsolute(path: string): string {
|
||||
return resolve(path)
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether `target` lies under `base` (or equals it), tolerant of separator
|
||||
* style and — on Windows, where the filesystem is case-insensitive — of
|
||||
* letter case. The media route uses this instead of a raw `startsWith` so a
|
||||
* case-mismatched or mixed-separator path can never be misclassified
|
||||
* (e.g. `C:\Users\Me` vs `c:/users/me/file.png`).
|
||||
* @param platform - filesystem semantics; injectable so both branches are
|
||||
* unit-testable on any host.
|
||||
*/
|
||||
export function isWithin(base: string, target: string, platform: NodeJS.Platform = process.platform): boolean {
|
||||
const norm = (value: string): string => value.replace(/[\\/]+/g, '/').replace(/\/$/, '')
|
||||
const b = norm(base)
|
||||
const t = norm(target)
|
||||
if (platform === 'win32') {
|
||||
const lb = b.toLowerCase()
|
||||
const lt = t.toLowerCase()
|
||||
return lt === lb || lt.startsWith(`${lb}/`)
|
||||
}
|
||||
return t === b || t.startsWith(`${b}/`)
|
||||
}
|
||||
|
||||
/** Message text of an unknown thrown value. */
|
||||
export function messageOf(error: unknown): string {
|
||||
return error instanceof Error ? error.message : String(error)
|
||||
|
||||
+12
-4
@@ -12,7 +12,7 @@
|
||||
* processes are keyed by session.
|
||||
*/
|
||||
import { mkdir, open, readFile, rename, rm, stat, writeFile } from 'node:fs/promises'
|
||||
import { dirname, extname } from 'node:path'
|
||||
import { basename, dirname, extname } from 'node:path'
|
||||
import type { IncomingMessage } from 'node:http'
|
||||
import { WebSocket, WebSocketServer } from 'ws'
|
||||
import type { Context } from './context-types.ts'
|
||||
@@ -22,7 +22,7 @@ import {
|
||||
type ResolvedSidebarConfig,
|
||||
type SidebarConfig,
|
||||
} from './config.ts'
|
||||
import { parentOf, requireAbsolute, listDirectory, rootLabel } from './fs-tree.ts'
|
||||
import { isWithin, parentOf, requireAbsolute, listDirectory, rootLabel } from './fs-tree.ts'
|
||||
import { isTrustedApiRequest } from './trust-fence.ts'
|
||||
import * as git from './git.ts'
|
||||
import { defaultShell, ensureSpawnHelper, PtyManager } from './pty-manager.ts'
|
||||
@@ -286,9 +286,11 @@ export function apply(ctx: Context, config?: SidebarConfig): void {
|
||||
if (sessionId === null || raw === null) throw new SidebarError('bad-request', 'sessionId and path are required')
|
||||
const cwd = sessionCwdOf(ctx, sessionId, url.searchParams.get('cwd') ?? undefined)
|
||||
const path = requireAbsolute(raw)
|
||||
if (!path.startsWith(cwd)) {
|
||||
if (!isWithin(cwd, path)) {
|
||||
// Only files under the session cwd are served as media (the editor
|
||||
// opens images from the explorer; produced files go through read).
|
||||
// isWithin (not a raw startsWith) so case-mismatched Windows paths
|
||||
// and mixed separators cannot be misclassified.
|
||||
throw new SidebarError('fs-error', 'media path outside the session working directory', 403)
|
||||
}
|
||||
const info = await stat(path)
|
||||
@@ -297,7 +299,13 @@ export function apply(ctx: Context, config?: SidebarConfig): void {
|
||||
}
|
||||
const type = MEDIA_TYPES[extname(path).toLowerCase()] ?? 'application/octet-stream'
|
||||
const body = await readFile(path)
|
||||
res.writeHead(200, { 'content-type': type, 'cache-control': 'no-cache' })
|
||||
// Raw bytes either way (binary-safe); ?download=1 switches the
|
||||
// disposition so the browser saves the file instead of showing it.
|
||||
const headers: Record<string, string> = { 'content-type': type, 'cache-control': 'no-cache' }
|
||||
if (url.searchParams.get('download') === '1') {
|
||||
headers['content-disposition'] = `attachment; filename*=UTF-8''${encodeURIComponent(basename(path))}`
|
||||
}
|
||||
res.writeHead(200, headers)
|
||||
res.end(body)
|
||||
} catch (error) {
|
||||
writeError(res, error)
|
||||
|
||||
+11
-4
@@ -4,6 +4,8 @@
|
||||
* actual repository, and a real directory listing. Runs with `pnpm test`.
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { resolve as resolvePath } from 'node:path'
|
||||
import { apply } from '../src/index.ts'
|
||||
import * as git from '../src/git.ts'
|
||||
import { listDirectory } from '../src/fs-tree.ts'
|
||||
@@ -121,16 +123,19 @@ describe('host plugin smoke', () => {
|
||||
|
||||
it('pty manager: reopening with a different cwd respawns in the new directory', async () => {
|
||||
const manager = new PtyManager(defaultShell(), 3)
|
||||
// A real second directory: os.tmpdir() exists on every platform ('/tmp'
|
||||
// does not exist on Windows).
|
||||
const other = tmpdir()
|
||||
try {
|
||||
const first = manager.open('s4', 't1', process.cwd(), 80, 24)
|
||||
// The hydrate race: the first connect fell back to the process cwd,
|
||||
// the reconnect carries the session's real cwd — the shell must move.
|
||||
const second = manager.open('s4', 't1', '/tmp', 80, 24)
|
||||
const second = manager.open('s4', 't1', other, 80, 24)
|
||||
expect(second).not.toBe(first)
|
||||
expect(second.cwd).toBe('/tmp')
|
||||
expect(second.cwd).toBe(other)
|
||||
expect(manager.keysOf('s4')).toHaveLength(1)
|
||||
// A same-cwd reconnect reattaches without respawning.
|
||||
const third = manager.open('s4', 't1', '/tmp', 80, 24)
|
||||
const third = manager.open('s4', 't1', other, 80, 24)
|
||||
expect(third).toBe(second)
|
||||
expect(manager.keysOf('s4')).toHaveLength(1)
|
||||
} finally {
|
||||
@@ -192,7 +197,9 @@ describe('session cwd resolution over the API route', () => {
|
||||
const route = mount()
|
||||
const result = await invoke(route, 'session.cwd', { sessionId: 's-detached', cwd: '/tmp/summary-cwd' })
|
||||
expect(result.ok).toBe(true)
|
||||
expect(result.value?.cwd).toBe('/tmp/summary-cwd')
|
||||
// The summary cwd passes through requireAbsolute (platform resolve), so
|
||||
// the expectation follows the platform's own normalization.
|
||||
expect(result.value?.cwd).toBe(resolvePath('/tmp/summary-cwd'))
|
||||
})
|
||||
|
||||
it('falls back to the process cwd with no summary cwd', async () => {
|
||||
|
||||
+47
-2
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { compareEntries, parentOf, rootLabel, requireAbsolute } from '../src/fs-tree.ts'
|
||||
import { compareEntries, isWithin, parentOf, rootLabel, requireAbsolute } from '../src/fs-tree.ts'
|
||||
import { parseLogLines, parsePorcelainZ } from '../src/git.ts'
|
||||
import {
|
||||
activateTab, allLeaves, closeTab, insertLeafAt, makeDefaultState, moveTab, moveTabToEdge,
|
||||
@@ -23,15 +23,47 @@ describe('fs-tree', () => {
|
||||
})
|
||||
|
||||
it('derives root labels and parents', () => {
|
||||
// POSIX-style inputs behave identically on both platforms (win32 parses '/'
|
||||
// as a separator), so these assertions are platform-independent.
|
||||
expect(rootLabel('/Users/me/code')).toBe('code')
|
||||
expect(rootLabel('/')).toBe('/')
|
||||
expect(parentOf('/Users/me/code')).toBe('/Users/me')
|
||||
expect(parentOf('/')).toBeUndefined()
|
||||
// Windows-drive roots and segments, asserted only where win32 semantics apply.
|
||||
if (process.platform === 'win32') {
|
||||
expect(rootLabel('C:\\')).toBe('C:\\')
|
||||
expect(parentOf('C:\\')).toBeUndefined()
|
||||
expect(rootLabel('C:\\Users\\me')).toBe('me')
|
||||
expect(parentOf('C:\\Users\\me')).toBe('C:\\Users')
|
||||
}
|
||||
})
|
||||
|
||||
it('accepts absolute paths and rejects relative ones', () => {
|
||||
expect(requireAbsolute('/a/b')).toBe('/a/b')
|
||||
// resolve() is platform-native: '/a/b' roots to the current drive on win32.
|
||||
expect(requireAbsolute('/a/b')).toBe(process.platform === 'win32' ? '\\a\\b' : '/a/b')
|
||||
if (process.platform === 'win32') {
|
||||
expect(requireAbsolute('C:/proj')).toBe('C:\\proj')
|
||||
}
|
||||
expect(() => requireAbsolute('a/b')).toThrow(/not an absolute path/)
|
||||
expect(() => requireAbsolute('../a')).toThrow(/not an absolute path/)
|
||||
})
|
||||
|
||||
it('isWithin tolerates separators and (on win32) letter case', () => {
|
||||
expect(isWithin('/work/proj', '/work/proj/src/a.ts')).toBe(true)
|
||||
expect(isWithin('/work/proj', '/work/proj')).toBe(true)
|
||||
expect(isWithin('/work/proj', '/work/proj2/a.ts')).toBe(false)
|
||||
expect(isWithin('/work/proj', '/other/a.ts')).toBe(false)
|
||||
// Mixed separators normalize on every platform.
|
||||
expect(isWithin('C:\\Users\\me', 'C:/Users/me/src/a.ts')).toBe(true)
|
||||
// Case sensitivity follows the platform's filesystem semantics (the
|
||||
// platform parameter makes both branches assertable on any host).
|
||||
expect(isWithin('C:\\Users\\Me', 'c:/users/me/file.png', 'win32')).toBe(true)
|
||||
expect(isWithin('/Users/Me', '/users/me/file.png', 'win32')).toBe(true)
|
||||
expect(isWithin('/Users/Me', '/users/me/file.png', 'linux')).toBe(false)
|
||||
expect(isWithin('/Users/Me', '/users/me/file.png', 'darwin')).toBe(false)
|
||||
// Windows drive-root containment.
|
||||
expect(isWithin('C:\\', 'C:\\Users\\me\\a.png', 'win32')).toBe(true)
|
||||
expect(isWithin('c:\\users', 'C:/USERS/me/b.png', 'win32')).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -383,6 +415,19 @@ describe('path helpers', () => {
|
||||
expect(relativeTo('C:\\Users\\me', 'C:/Users/me/src/a.ts')).toBe('src/a.ts')
|
||||
expect(relativeTo('C:\\Users\\me\\', 'C:\\Users\\me')).toBe('.')
|
||||
})
|
||||
|
||||
it('containment is case-insensitive (windows/macOS case-insensitive volumes)', () => {
|
||||
expect(relativeTo('C:\\Users\\Me', 'c:/users/me/src/a.ts')).toBe('src/a.ts')
|
||||
expect(relativeTo('/Users/Me/code', '/users/me/code/src/main.ts')).toBe('src/main.ts')
|
||||
// The returned relative text keeps the caller's own casing.
|
||||
expect(relativeTo('C:\\Users\\me', 'C:\\Users\\Me\\SRC\\a.ts')).toBe('SRC/a.ts')
|
||||
})
|
||||
|
||||
it('resolves produced paths against windows cwds', () => {
|
||||
expect(resolveSidebarPath('C:\\work\\proj', 'src/a.ts')).toBe('C:\\work\\proj\\src/a.ts')
|
||||
expect(resolveSidebarPath('C:\\work\\proj', 'C:\\abs\\x.ts')).toBe('C:\\abs\\x.ts')
|
||||
expect(resolveSidebarPath('C:\\work\\proj\\', 'C:\\abs\\x.ts')).toBe('C:\\abs\\x.ts')
|
||||
})
|
||||
})
|
||||
|
||||
describe('editor language mapping', () => {
|
||||
|
||||
Reference in New Issue
Block a user