From e9ce58afea76e67df2dbd86c5153126d9dd40668 Mon Sep 17 00:00:00 2001
From: kelvinwww <1970138194@qq.com>
Date: Sun, 23 Aug 2026 10:28:05 +0800
Subject: [PATCH] =?UTF-8?q?fix:=20=E9=99=90=E5=88=B6=E6=96=87=E4=BB=B6=20A?=
 =?UTF-8?q?PI=20=E8=AE=BF=E9=97=AE=E4=BC=9A=E8=AF=9D=E5=B7=A5=E4=BD=9C?=
 =?UTF-8?q?=E5=8C=BA?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 src/fs-operations.ts        |  32 +++---
 src/html-route.ts           |   4 +-
 src/index.ts                |  34 +++----
 src/path-security.ts        |  75 +++++++++++++++
 tests/fs-operations.spec.ts |  19 +++-
 tests/smoke.spec.ts         | 187 +++++++++++++++++++++++++++++++++++-
 6 files changed, 307 insertions(+), 44 deletions(-)
 create mode 100644 src/path-security.ts

diff --git a/src/fs-operations.ts b/src/fs-operations.ts
index a48447d..c867a09 100644
--- a/src/fs-operations.ts
+++ b/src/fs-operations.ts
@@ -1,13 +1,12 @@
 /**
  * Workspace-safe file mutations for the sidebar (the upload route today).
  *
- * Every write is lexically confined to the session workspace: the upload
- * directory is resolved absolute and must sit inside the session cwd, the
- * relative path is sanitized (absolute paths, '.', '..' and empty segments
- * are refused), and the final target must stay inside both. Containment is
- * lexical (no symlink resolution) — a symlinked directory inside the cwd can
- * redirect writes outside, matching the trust model of the other /sidebar/*
- * routes. Bytes stream from the request body to a uniquely named temp sibling
+ * Every write is confined to the real session workspace: the upload
+ * directory is resolved absolute and its target is checked through existing
+ * filesystem ancestors, the relative path is sanitized (absolute paths, '.',
+ * '..' and empty segments are refused), and the final target must stay inside
+ * the workspace after symlink resolution. Bytes stream from the request body
+ * to a uniquely named temp sibling
  * and are renamed into place, so a failed, aborted, or oversized upload never
  * leaves a partial file at the target path.
  */
@@ -17,7 +16,8 @@ import { createWriteStream } from 'node:fs'
 import { mkdir, rename, rm, stat } from 'node:fs/promises'
 import { basename, dirname, join } from 'node:path'
 import type { SidebarHttpRequest } from './context-types.ts'
-import { isWithin, requireAbsolute } from './fs-tree.ts'
+import { requireAbsolute } from './fs-tree.ts'
+import { ensureWorkspacePath, ensureWorkspaceWritePath } from './path-security.ts'
 import { SidebarError } from './wire.ts'
 
 /** Inputs of one upload: the session scope plus the request body stream. */
@@ -48,9 +48,7 @@ export interface WorkspaceUploadInput {
 export async function writeWorkspaceUpload(input: WorkspaceUploadInput): Promise<{ path: string; size: number }> {
   const { cwd, dir, relativePath, chunks, limit } = input
   const base = requireAbsolute(dir)
-  if (!isWithin(cwd, base)) {
-    throw new SidebarError('forbidden', 'upload directory escapes the session workspace', 403)
-  }
+  await ensureWorkspacePath(cwd, base)
   if (relativePath === '' || relativePath.startsWith('/') || relativePath.startsWith('\\')) {
     throw new SidebarError('bad-request', 'relativePath must stay below the upload directory', 400)
   }
@@ -59,11 +57,9 @@ export async function writeWorkspaceUpload(input: WorkspaceUploadInput): Promise
     throw new SidebarError('bad-request', 'relativePath must stay below the upload directory', 400)
   }
   const target = join(base, ...segments)
-  if (!isWithin(cwd, target) || !isWithin(base, target)) {
-    throw new SidebarError('forbidden', 'target escapes the session workspace', 403)
-  }
-  const tmp = join(dirname(target), `.${basename(target)}.dsh-upload-${randomUUID()}.tmp`)
-  await mkdir(dirname(target), { recursive: true })
+  const safeTarget = await ensureWorkspaceWritePath(cwd, target)
+  const tmp = join(dirname(safeTarget), `.${basename(safeTarget)}.dsh-upload-${randomUUID()}.tmp`)
+  await mkdir(dirname(safeTarget), { recursive: true })
   const stream = createWriteStream(tmp, { flags: 'wx' })
   // Resolves once the stream fully closes; created up front so a stream that
   // already closed (successful end, later failure) cannot leave the wait hanging.
@@ -85,8 +81,8 @@ export async function writeWorkspaceUpload(input: WorkspaceUploadInput): Promise
       stream.end((error?: Error | null) => (error === undefined || error === null ? resolve() : reject(error)))
     })
     if (streamError !== undefined) throw streamError
-    await rename(tmp, target)
-    const info = await stat(target)
+    await rename(tmp, safeTarget)
+    const info = await stat(safeTarget)
     return { path: target, size: info.size }
   } catch (error) {
     // Wait for the stream to fully close before unlinking (Windows locks open
diff --git a/src/html-route.ts b/src/html-route.ts
index 1dfac34..cc74261 100644
--- a/src/html-route.ts
+++ b/src/html-route.ts
@@ -57,7 +57,7 @@ export function encodeHtmlUrl(sessionId: string, path: string): string {
  * Decode a route pathname into the session + absolute file path. Rejects
  * a wrong prefix (404), an empty path, malformed percent encoding, and a
  * missing sessionId or file path (400). The caller still must bound the
- * decoded path with requireAbsolute + isWithin(cwd) — a decoded `..`
+ * decoded path with the workspace real-path guard — a decoded `..`
  * segment resolves outside the cwd and is refused there.
  */
 export function decodeHtmlUrl(pathname: string): HtmlDecodeResult {
@@ -96,7 +96,7 @@ export function decodeHtmlUrl(pathname: string): HtmlDecodeResult {
     // A Windows drive segment ('D:') is the FIRST path segment of an encoded
     // drive path. Rejoining it with a leading slash would yield '/D:/work/...'
     // which node's path.resolve() mangles into 'D:\D:\work\...' on Windows —
-    // the html route's isWithin(cwd) fence would then reject every drive path.
+    // the html route's workspace fence would then reject every drive path.
     // Keep the drive form slash-free so requireAbsolute() resolves it verbatim.
     path = tail.join('/')
   } else {
diff --git a/src/index.ts b/src/index.ts
index 067add6..ca6e7e1 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -28,8 +28,9 @@ import {
   type SidebarConfig,
   type SidebarPrefs,
 } from './config.ts'
-import { isWithin, parentOf, requireAbsolute, listDirectory, rootLabel } from './fs-tree.ts'
+import { parentOf, requireAbsolute, listDirectory, rootLabel } from './fs-tree.ts'
 import { writeWorkspaceUpload } from './fs-operations.ts'
+import { ensureWorkspacePath, ensureWorkspaceWritePath } from './path-security.ts'
 import { searchFiles } from './fs-search.ts'
 import { decodeHtmlUrl } from './html-route.ts'
 import { extractFrameAncestors } from './browser-probe.ts'
@@ -128,6 +129,12 @@ function sessionCwdOf(ctx: Context, sessionId: string, clientCwd?: string): stri
  */
 async function resolveGitPath(cwd: string, raw: string): Promise<string> {
   if (isAbsolute(raw)) return requireAbsolute(raw)
+  // Prefer the session-relative interpretation when it names an existing
+  // path. Git status reports repository-root-relative names, but the sidebar
+  // security boundary is the session workspace; this preference keeps files
+  // inside a nested session readable without reopening the repository root.
+  const sessionPath = requireAbsolute(join(cwd, raw))
+  if (await stat(sessionPath).then(() => true).catch(() => false)) return sessionPath
   const root = await git.repoRoot(cwd).catch(() => cwd)
   return requireAbsolute(join(root, raw))
 }
@@ -248,7 +255,7 @@ function buildApi(
     'fs.tree': async (payload) => {
       const { cwd } = cwdOf(payload)
       const record = payload as { path?: unknown }
-      const target = record.path === undefined ? cwd : requireAbsolute(requireString(payload, 'path'))
+      const target = record.path === undefined ? cwd : await ensureWorkspacePath(cwd, requireString(payload, 'path'))
       return listDirectory(target, resolved.listLimit)
     },
     'fs.search': async (payload) => {
@@ -263,14 +270,14 @@ function buildApi(
       const { cwd } = cwdOf(payload)
       // Relative paths are git-derived (status/diff report repo-root-relative
       // names; the untracked diff view reads the file through this route).
-      const path = await resolveGitPath(cwd, requireString(payload, 'path'))
+      const path = await ensureWorkspacePath(cwd, await resolveGitPath(cwd, requireString(payload, 'path')))
       const { content, truncated, binary, size, head } = await readText(path, resolved.readLimit)
       if (binary) return { kind: 'binary', size, truncated, head }
       return { kind: 'text', content, truncated }
     },
     'fs.write': async (payload) => {
       const { cwd } = cwdOf(payload)
-      const path = requireAbsolute(requireString(payload, 'path'))
+      const path = await ensureWorkspaceWritePath(cwd, requireString(payload, 'path'))
       const content = requireString(payload, 'content')
       const tmp = `${path}.dsh-sidebar-tmp-${process.pid}`
       try {
@@ -722,14 +729,7 @@ export function apply(ctx: Context, config?: SidebarConfig): void {
         const raw = url.searchParams.get('path')
         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 (!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 path = await ensureWorkspacePath(cwd, raw)
         const info = await stat(path)
         if (!info.isFile() || info.size > resolved.mediaLimit) {
           throw new SidebarError('fs-error', 'not a file or too large', 400)
@@ -784,13 +784,11 @@ export function apply(ctx: Context, config?: SidebarConfig): void {
         const { sessionId, path } = decoded.ref
         // The session's authoritative cwd (client cwd cannot ride in the URL
         // — the path encoding has no query; a detached first request falls
-        // back to the process cwd and is normally refused by isWithin, same
-        // semantics as the media route's fallback).
+        // back to the process cwd and is normally refused by the workspace
+        // real-path guard, with the same semantics as the media route's
+        // fallback.
         const cwd = sessionCwdOf(ctx, sessionId)
-        const absolute = requireAbsolute(path)
-        if (!isWithin(cwd, absolute)) {
-          throw new SidebarError('fs-error', 'html path outside the session working directory', 403)
-        }
+        const absolute = await ensureWorkspacePath(cwd, path)
         const info = await stat(absolute)
         if (!info.isFile() || info.size > resolved.mediaLimit) {
           throw new SidebarError('fs-error', 'not a file or too large', 400)
diff --git a/src/path-security.ts b/src/path-security.ts
new file mode 100644
index 0000000..0edbcef
--- /dev/null
+++ b/src/path-security.ts
@@ -0,0 +1,75 @@
+/** Filesystem path guards shared by sidebar APIs that access a session workspace. */
+import { realpath } from 'node:fs/promises'
+import { basename, dirname, join } from 'node:path'
+import { isWithin, requireAbsolute } from './fs-tree.ts'
+import { SidebarError } from './wire.ts'
+
+/** Resolve a path and convert filesystem resolution failures to an API error. */
+async function resolveRealPath(path: string, label: string): Promise<string> {
+  try {
+    return await realpath(path)
+  } catch (error) {
+    throw new SidebarError('fs-error', `cannot resolve ${label} "${path}": ${error instanceof Error ? error.message : String(error)}`, 400)
+  }
+}
+
+/** Reject a resolved path whose real filesystem target escapes the workspace. */
+function assertWithinWorkspace(workspace: string, target: string): void {
+  if (!isWithin(workspace, target)) {
+    throw new SidebarError('forbidden', `path "${target}" is outside workspace`, 403)
+  }
+}
+
+/**
+ * Resolve an existing workspace path through symlinks and enforce containment.
+ *
+ * @param cwd - Session workspace directory.
+ * @param target - Client-supplied absolute path.
+ * @returns The canonical absolute path used for the filesystem operation.
+ */
+export async function ensureWorkspacePath(cwd: string, target: string): Promise<string> {
+  const absolute = requireAbsolute(target)
+  const [realCwd, realTarget] = await Promise.all([
+    resolveRealPath(cwd, 'workspace'),
+    resolveRealPath(absolute, 'target'),
+  ])
+  assertWithinWorkspace(realCwd, realTarget)
+  return realTarget
+}
+
+/**
+ * Validate a write destination, including destinations that do not exist yet.
+ * Existing targets are resolved to catch symlinks; missing targets are checked
+ * against the nearest existing ancestor before the caller creates or renames.
+ * The returned path is rebuilt from that canonical ancestor, so an existing
+ * symlink is never left in the path passed to the write operation.
+ *
+ * @param cwd - Session workspace directory.
+ * @param target - Client-supplied absolute destination path.
+ * @returns A canonical path for an existing target or its nearest existing ancestor.
+ */
+export async function ensureWorkspaceWritePath(cwd: string, target: string): Promise<string> {
+  const absolute = requireAbsolute(target)
+  const realCwd = await resolveRealPath(cwd, 'workspace')
+  let existingPath = absolute
+  const missingSegments: string[] = []
+
+  for (;;) {
+    try {
+      const realTarget = await realpath(existingPath)
+      assertWithinWorkspace(realCwd, realTarget)
+      return missingSegments.reduce((path, segment) => join(path, segment), realTarget)
+    } catch (error) {
+      if ((error as NodeJS.ErrnoException).code !== 'ENOENT') {
+        if (error instanceof SidebarError) throw error
+        throw new SidebarError('fs-error', `cannot resolve target "${existingPath}": ${error instanceof Error ? error.message : String(error)}`, 400)
+      }
+      const parent = dirname(existingPath)
+      if (parent === existingPath) {
+        throw new SidebarError('fs-error', `cannot resolve target "${absolute}"`, 400)
+      }
+      missingSegments.unshift(basename(existingPath))
+      existingPath = parent
+    }
+  }
+}
diff --git a/tests/fs-operations.spec.ts b/tests/fs-operations.spec.ts
index 63c3896..e0ebefb 100644
--- a/tests/fs-operations.spec.ts
+++ b/tests/fs-operations.spec.ts
@@ -1,5 +1,5 @@
 import { afterAll, describe, expect, it } from 'vitest'
-import { existsSync, mkdtempSync, readdirSync, readFileSync, rmSync } from 'node:fs'
+import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, symlinkSync } from 'node:fs'
 import { join } from 'node:path'
 import { tmpdir } from 'node:os'
 import { writeWorkspaceUpload } from '../src/fs-operations.ts'
@@ -103,6 +103,23 @@ describe('writeWorkspaceUpload', () => {
     }
   })
 
+  it('refuses upload directories and targets that resolve outside the workspace', async () => {
+    const outside = mkdtempSync(join(tmpdir(), 'dsh-sidebar-upload-symlink-outside-'))
+    const link = join(root, 'upload-link')
+    try {
+      symlinkSync(outside, link)
+      await expect(writeWorkspaceUpload({
+        cwd: root, dir: link, relativePath: 'x.txt', chunks: chunksOf('x'), limit: 1024,
+      })).rejects.toMatchObject({ code: 'forbidden' })
+      await expect(writeWorkspaceUpload({
+        cwd: root, dir: root, relativePath: 'upload-link/x.txt', chunks: chunksOf('x'), limit: 1024,
+      })).rejects.toMatchObject({ code: 'forbidden' })
+    } finally {
+      rmSync(link, { force: true })
+      rmSync(outside, { recursive: true, force: true })
+    }
+  })
+
   it('refuses oversized uploads without leaving a target or temp file', async () => {
     const target = join(root, 'big.bin')
     await expect(writeWorkspaceUpload({
diff --git a/tests/smoke.spec.ts b/tests/smoke.spec.ts
index ba6ffc7..4b57753 100644
--- a/tests/smoke.spec.ts
+++ b/tests/smoke.spec.ts
@@ -5,7 +5,7 @@
  */
 import { describe, expect, it, vi } from 'vitest'
 import { spawnSync } from 'node:child_process'
-import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
+import { mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join, resolve as resolvePath } from 'node:path'
 import { SettingsConflictError, settingsNamespace } from '@deepseek-ai/dsh-settings'
@@ -15,6 +15,20 @@ import { listDirectory } from '../src/fs-tree.ts'
 import { defaultShell, PtyManager, type SidebarPty } from '../src/pty-manager.ts'
 import type { SidebarWebRoute, SidebarWebUpgradeRoute } from '../src/context-types.ts'
 
+/** Symlink creation may require elevated privileges on Windows. */
+const canCreateSymlink = (() => {
+  const dir = mkdtempSync(join(tmpdir(), 'dsh-sidebar-security-probe-'))
+  try {
+    mkdirSync(join(dir, 'target'))
+    symlinkSync(join(dir, 'target'), join(dir, 'link'))
+    return true
+  } catch {
+    return false
+  } finally {
+    rmSync(dir, { recursive: true, force: true })
+  }
+})()
+
 interface FakeContext {
   webRuntime: { trustedHosts: readonly string[] }
   webServer: {
@@ -420,7 +434,7 @@ describe('session cwd resolution over the API route', () => {
     sessions?: { get: (id: string) => { header: { cwd?: string } } | undefined }
   }
 
-  const mount = (overrides: CtxOverrides = {}): SidebarWebRoute => {
+  const mountAll = (overrides: CtxOverrides = {}): SidebarWebRoute[] => {
     const routes: SidebarWebRoute[] = []
     const ctx = {
       webRuntime: { trustedHosts: [] },
@@ -438,14 +452,16 @@ describe('session cwd resolution over the API route', () => {
       get: () => undefined,
     }
     apply(ctx as never)
-    return routes.find(route => route.path === '/sidebar/api')!
+    return routes
   }
 
+  const mount = (overrides: CtxOverrides = {}): SidebarWebRoute => mountAll(overrides).find(route => route.path === '/sidebar/api')!
+
   const invoke = async (
     route: SidebarWebRoute,
     method: string,
     payload: unknown,
-  ): Promise<{ ok: boolean; value?: { cwd: string }; error?: { message: string } }> => {
+  ): Promise<{ ok: boolean; status: number; value?: { cwd: string }; error?: { code?: string; message: string } }> => {
     const body = Buffer.from(JSON.stringify(payload))
     const req = {
       method: 'POST',
@@ -459,7 +475,18 @@ describe('session cwd resolution over the API route', () => {
       end: (chunk: unknown) => { out.body += String(chunk ?? '') },
     } as never
     await route.handler(req, res)
-    return JSON.parse(out.body) as { ok: boolean; value?: { cwd: string }; error?: { message: string } }
+    return { ...JSON.parse(out.body) as { ok: boolean; value?: { cwd: string }; error?: { code?: string; message: string } }, status: out.status }
+  }
+
+  const invokeGet = async (route: SidebarWebRoute, url: string): Promise<{ status: number; body: string }> => {
+    const out: { status: number; body: string } = { status: 200, body: '' }
+    const req = { method: 'GET', url, headers: { host: '127.0.0.1:3080' } } as never
+    const res = {
+      writeHead: (status: number) => { out.status = status },
+      end: (chunk: unknown) => { out.body += String(chunk ?? '') },
+    } as never
+    await route.handler(req, res)
+    return out
   }
 
   it('uses the client summary cwd while the session is detached', async () => {
@@ -532,6 +559,156 @@ describe('session cwd resolution over the API route', () => {
     expect(value.value?.kind).toBe('text')
     expect(value.value?.content).toContain('runGit')
   })
+
+  it('rejects repo-root-relative fs.read paths outside a nested session workspace', async () => {
+    const route = mount({
+      sessions: {
+        get: () => ({ header: { cwd: join(process.cwd(), 'src') } }),
+      },
+    })
+    const result = await invoke(route, 'fs.read', { sessionId: 's-sub', path: 'package.json' })
+    expect(result).toMatchObject({ ok: false, status: 403, error: { code: 'forbidden' } })
+  })
+
+  it('rejects fs.tree paths outside the session workspace', async () => {
+    const root = mkdtempSync(join(tmpdir(), 'dsh-sidebar-fs-security-'))
+    const workspace = join(root, 'workspace')
+    const outside = join(root, 'outside')
+    mkdirSync(workspace)
+    mkdirSync(outside)
+    const outsideFile = join(outside, 'secret.txt')
+    writeFileSync(outsideFile, 'secret')
+    try {
+      const route = mount({ sessions: { get: () => ({ header: { cwd: workspace } }) } })
+      const tree = await invoke(route, 'fs.tree', { sessionId: 'security', path: outside })
+      expect(tree).toMatchObject({ ok: false, status: 403, error: { code: 'forbidden' } })
+    } finally {
+      rmSync(root, { recursive: true, force: true })
+    }
+  })
+
+  it('rejects fs.read paths outside the session workspace', async () => {
+    const root = mkdtempSync(join(tmpdir(), 'dsh-sidebar-fs-security-'))
+    const workspace = join(root, 'workspace')
+    const outside = join(root, 'outside')
+    mkdirSync(workspace)
+    mkdirSync(outside)
+    const outsideFile = join(outside, 'secret.txt')
+    writeFileSync(outsideFile, 'secret')
+    try {
+      const route = mount({ sessions: { get: () => ({ header: { cwd: workspace } }) } })
+      const read = await invoke(route, 'fs.read', { sessionId: 'security', path: outsideFile })
+      expect(read).toMatchObject({ ok: false, status: 403, error: { code: 'forbidden' } })
+    } finally {
+      rmSync(root, { recursive: true, force: true })
+    }
+  })
+
+  it('rejects fs.write paths outside the session workspace', async () => {
+    const root = mkdtempSync(join(tmpdir(), 'dsh-sidebar-fs-security-'))
+    const workspace = join(root, 'workspace')
+    const outside = join(root, 'outside')
+    mkdirSync(workspace)
+    mkdirSync(outside)
+    try {
+      const route = mount({ sessions: { get: () => ({ header: { cwd: workspace } }) } })
+      const write = await invoke(route, 'fs.write', { sessionId: 'security', path: join(outside, 'written.txt'), content: 'hack' })
+      expect(write).toMatchObject({ ok: false, status: 403, error: { code: 'forbidden' } })
+    } finally {
+      rmSync(root, { recursive: true, force: true })
+    }
+  })
+
+  it('rejects media and HTML reads through a workspace symlink', async () => {
+    if (!canCreateSymlink) return
+    const root = mkdtempSync(join(tmpdir(), 'dsh-sidebar-route-symlink-security-'))
+    const workspace = join(root, 'workspace')
+    const outside = join(root, 'outside')
+    mkdirSync(workspace)
+    mkdirSync(outside)
+    const mediaPath = join(outside, 'secret.png')
+    const htmlPath = join(outside, 'secret.html')
+    writeFileSync(mediaPath, 'not an image')
+    writeFileSync(htmlPath, '<p>secret</p>')
+    try {
+      symlinkSync(outside, join(workspace, 'link'))
+      const routes = mountAll({ sessions: { get: () => ({ header: { cwd: workspace } }) } })
+      const media = routes.find(route => route.path === '/sidebar/file')!
+      const html = routes.find(route => route.path === '/sidebar/html')!
+      const mediaResult = await invokeGet(media, `/sidebar/file?sessionId=security&path=${encodeURIComponent(join(workspace, 'link', 'secret.png'))}`)
+      const htmlPathname = join(workspace, 'link', 'secret.html').replace(/\\/g, '/').split('/').map(encodeURIComponent).join('/')
+      const htmlResult = await invokeGet(html, `/sidebar/html/security${htmlPathname}`)
+      expect(mediaResult).toMatchObject({ status: 403 })
+      expect(JSON.parse(mediaResult.body)).toMatchObject({ ok: false, error: { code: 'forbidden' } })
+      expect(htmlResult).toMatchObject({ status: 403 })
+      expect(JSON.parse(htmlResult.body)).toMatchObject({ ok: false, error: { code: 'forbidden' } })
+    } finally {
+      rmSync(root, { recursive: true, force: true })
+    }
+  })
+
+  it('keeps fs.tree missing-path failures as fs errors', async () => {
+    const root = mkdtempSync(join(tmpdir(), 'dsh-sidebar-fs-security-'))
+    const workspace = join(root, 'workspace')
+    mkdirSync(workspace)
+    try {
+      const route = mount({ sessions: { get: () => ({ header: { cwd: workspace } }) } })
+      const tree = await invoke(route, 'fs.tree', { sessionId: 'security', path: join(workspace, 'missing') })
+      expect(tree).toMatchObject({ ok: false, status: 400, error: { code: 'fs-error' } })
+    } finally {
+      rmSync(root, { recursive: true, force: true })
+    }
+  })
+
+  it.skipIf(!canCreateSymlink)('rejects workspace symlinks that resolve outside the workspace', async () => {
+    const root = mkdtempSync(join(tmpdir(), 'dsh-sidebar-fs-symlink-security-'))
+    const workspace = join(root, 'workspace')
+    const outside = join(root, 'outside')
+    mkdirSync(workspace)
+    mkdirSync(outside)
+    writeFileSync(join(outside, 'secret.txt'), 'secret')
+    try {
+      symlinkSync(outside, join(workspace, 'link'))
+      const route = mount({ sessions: { get: () => ({ header: { cwd: workspace } }) } })
+      const tree = await invoke(route, 'fs.tree', { sessionId: 'security', path: join(workspace, 'link') })
+      expect(tree).toMatchObject({ ok: false, status: 403, error: { code: 'forbidden' } })
+    } finally {
+      rmSync(root, { recursive: true, force: true })
+    }
+  })
+
+  it.skipIf(!canCreateSymlink)('rejects fs.read through a workspace symlink', async () => {
+    const root = mkdtempSync(join(tmpdir(), 'dsh-sidebar-fs-symlink-security-'))
+    const workspace = join(root, 'workspace')
+    const outside = join(root, 'outside')
+    mkdirSync(workspace)
+    mkdirSync(outside)
+    writeFileSync(join(outside, 'secret.txt'), 'secret')
+    try {
+      symlinkSync(outside, join(workspace, 'link'))
+      const route = mount({ sessions: { get: () => ({ header: { cwd: workspace } }) } })
+      const read = await invoke(route, 'fs.read', { sessionId: 'security', path: join(workspace, 'link', 'secret.txt') })
+      expect(read).toMatchObject({ ok: false, status: 403, error: { code: 'forbidden' } })
+    } finally {
+      rmSync(root, { recursive: true, force: true })
+    }
+  })
+
+  it.skipIf(!canCreateSymlink)('rejects fs.write through a workspace symlink', async () => {
+    const root = mkdtempSync(join(tmpdir(), 'dsh-sidebar-fs-symlink-security-'))
+    const workspace = join(root, 'workspace')
+    const outside = join(root, 'outside')
+    mkdirSync(workspace)
+    mkdirSync(outside)
+    try {
+      symlinkSync(outside, join(workspace, 'link'))
+      const route = mount({ sessions: { get: () => ({ header: { cwd: workspace } }) } })
+      const write = await invoke(route, 'fs.write', { sessionId: 'security', path: join(workspace, 'link', 'new.txt'), content: 'hack' })
+      expect(write).toMatchObject({ ok: false, status: 403, error: { code: 'forbidden' } })
+    } finally {
+      rmSync(root, { recursive: true, force: true })
+    }
+  })
 })
 
 describe('side card settings routes', () => {