fix: enforce symlink-aware filesystem roots (#326)

Author: antfubot

packages/devframe/src/utils/serve-static.test.ts

@@ -1,9 +1,10 @@
 import type { AddressInfo } from 'node:net'
 import type { ServeStaticOptions } from './serve-static'
-import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs'
+import { mkdirSync, mkdtempSync, symlinkSync, writeFileSync } from 'node:fs'
 import { createServer } from 'node:http'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
+import process from 'node:process'
 import { H3, toNodeHandler } from 'h3'
 import { afterEach, describe, expect, it } from 'vitest'
 import { mountStaticHandler, serveStaticHandler, serveStaticNodeMiddleware } from './serve-static'
@@ -216,6 +217,66 @@ describe('mountStaticHandler', () => {
   })
 })
 
+// Symlinks require privileges on Windows that hosted CI runners lack, so
+// gate the symlink-containment suite off that platform.
+describe.skipIf(process.platform === 'win32')('serveStaticHandler symlink containment', () => {
+  let fx: Fixture | undefined
+
+  afterEach(async () => {
+    await fx?.close()
+    fx = undefined
+  })
+
+  it('returns 404 for a file symlink escaping the served root', async () => {
+    const dir = makeTmp('devframe-serve-link-')
+    const outside = makeTmp('devframe-serve-outside-')
+    writeFileSync(join(outside, 'secret.txt'), 'top secret', 'utf-8')
+    symlinkSync(join(outside, 'secret.txt'), join(dir, 'leak.txt'))
+    writeFileSync(join(dir, 'ok.txt'), 'in root', 'utf-8')
+    fx = await startH3(dir, { single: false })
+
+    const leak = await fetch(`${fx.baseUrl}/leak.txt`)
+    expect(leak.status).toBe(404)
+    // Ordinary in-root files still serve.
+    const ok = await fetch(`${fx.baseUrl}/ok.txt`)
+    expect(ok.status).toBe(200)
+    expect(await ok.text()).toBe('in root')
+  })
+
+  it('returns 404 for a file reached through an escaping directory symlink', async () => {
+    const dir = makeTmp('devframe-serve-link-')
+    const outside = makeTmp('devframe-serve-outside-')
+    writeFileSync(join(outside, 'secret.txt'), 'top secret', 'utf-8')
+    symlinkSync(outside, join(dir, 'escape'))
+    fx = await startH3(dir, { single: false })
+
+    const res = await fetch(`${fx.baseUrl}/escape/secret.txt`)
+    expect(res.status).toBe(404)
+  })
+
+  it('serves a symlink whose canonical target stays inside the served root', async () => {
+    const dir = makeTmp('devframe-serve-link-')
+    writeFileSync(join(dir, 'real.txt'), 'contained', 'utf-8')
+    symlinkSync(join(dir, 'real.txt'), join(dir, 'alias.txt'))
+    fx = await startH3(dir, { single: false })
+
+    const res = await fetch(`${fx.baseUrl}/alias.txt`)
+    expect(res.status).toBe(200)
+    expect(await res.text()).toBe('contained')
+  })
+
+  it('returns 404 through the Node middleware for an escaping symlink', async () => {
+    const dir = makeTmp('devframe-serve-link-')
+    const outside = makeTmp('devframe-serve-outside-')
+    writeFileSync(join(outside, 'secret.txt'), 'top secret', 'utf-8')
+    symlinkSync(join(outside, 'secret.txt'), join(dir, 'leak.txt'))
+    fx = await startMw(dir, { single: false })
+
+    const res = await fetch(`${fx.baseUrl}/leak.txt`)
+    expect(res.status).toBe(404)
+  })
+})
+
 describe('serveStaticNodeMiddleware', () => {
   let fx: Fixture | undefined
 

packages/devframe/src/utils/serve-static.ts

@@ -3,7 +3,7 @@ import type { IncomingMessage, ServerResponse } from 'node:http'
 import type { ReadableStream as NodeWebReadableStream } from 'node:stream/web'
 import type { RemoteAssetsErrorMessage, RemoteAssetsStore } from '../types/remote-assets'
 import { createReadStream } from 'node:fs'
-import { stat } from 'node:fs/promises'
+import { realpath, stat } from 'node:fs/promises'
 import { Readable } from 'node:stream'
 import { defineHandler, H3 } from 'h3'
 import { lookup } from 'mrmime'
@@ -31,10 +31,25 @@ interface ResolvedFile {
 
 const HTML_EXTENSIONS = ['.html', '.htm']
 
-async function statFile(abs: string): Promise<ResolvedFile | null> {
+/**
+ * The canonical (symlink-resolved) served root, falling back to the lexical
+ * path when the directory doesn't exist yet (an empty deployment then serves
+ * nothing rather than throwing).
+ */
+async function canonicalRoot(absDir: string): Promise<string> {
+  return realpath(absDir).then(normalize, () => absDir)
+}
+
+/**
+ * Stat a candidate file, confirming its canonical target stays inside the
+ * canonical served root — a symlink inside the root can only resolve to a
+ * file still within it; one escaping the root reads as a miss, not a leak.
+ */
+async function statFile(abs: string, realRoot: string): Promise<ResolvedFile | null> {
   try {
     const s = await stat(abs)
-    if (!s.isFile())
+    const real = normalize(await realpath(abs))
+    if (!s.isFile() || (real !== realRoot && !real.startsWith(realRoot + sep)))
       return null
     return { abs, size: s.size, mtime: s.mtime }
   }
@@ -45,6 +60,7 @@ async function statFile(abs: string): Promise<ResolvedFile | null> {
 
 async function resolveTarget(
   absDir: string,
+  realRoot: string,
   urlPath: string,
   indexNames: string[],
   single: boolean,
@@ -67,15 +83,15 @@ async function resolveTarget(
   if (abs !== absDir && !abs.startsWith(absDir + sep))
     return null
 
-  const direct = await statFile(abs)
+  const direct = await statFile(abs, realRoot)
   if (direct)
     return direct
 
   try {
     const s = await stat(abs)
     if (s.isDirectory()) {
       for (const name of indexNames) {
-        const candidate = await statFile(join(abs, name))
+        const candidate = await statFile(join(abs, name), realRoot)
         if (candidate)
           return candidate
       }
@@ -90,15 +106,15 @@ async function resolveTarget(
   // fallback so pretty-URL deployments resolve to the right page.
   if (!extname(cleaned)) {
     for (const ext of HTML_EXTENSIONS) {
-      const candidate = await statFile(abs + ext)
+      const candidate = await statFile(abs + ext, realRoot)
       if (candidate)
         return candidate
     }
   }
 
   const fallbackIndex = indexNames[0]
   if (single && fallbackIndex && !/\.[a-z0-9]+$/i.test(cleaned)) {
-    const indexFile = await statFile(join(absDir, fallbackIndex))
+    const indexFile = await statFile(join(absDir, fallbackIndex), realRoot)
     if (indexFile)
       return indexFile
   }
@@ -199,14 +215,17 @@ export function serveStaticHandler(
     return serveRemoteAssetsHandler(source)
   const absDir = resolve(source)
   const opts = normalizeOptions(options)
+  // Canonicalize the served root once; the containment check compares every
+  // candidate's canonical path against it.
+  const realRoot = canonicalRoot(absDir)
   return defineHandler(async (event) => {
     const method = event.req.method
     if (method !== 'GET' && method !== 'HEAD') {
       event.res.status = 405
       event.res.headers.set('Allow', 'GET, HEAD')
       return ''
     }
-    const file = await resolveTarget(absDir, event.url.pathname, opts.indexNames, opts.single)
+    const file = await resolveTarget(absDir, await realRoot, event.url.pathname, opts.indexNames, opts.single)
     if (!file) {
       event.res.status = 404
       return ''
@@ -250,6 +269,7 @@ export function serveStaticNodeMiddleware(
 ): (req: IncomingMessage, res: ServerResponse, next?: (err?: Error) => void) => void {
   const absDir = typeof source === 'string' ? resolve(source) : undefined
   const opts = normalizeOptions(options)
+  const realRoot = absDir === undefined ? undefined : canonicalRoot(absDir)
   return (req, res, next) => {
     void (async () => {
       const method = req.method
@@ -282,7 +302,7 @@ export function serveStaticNodeMiddleware(
         return
       }
 
-      const file = await resolveTarget(absDir, url, opts.indexNames, opts.single)
+      const file = await resolveTarget(absDir, await realRoot!, url, opts.indexNames, opts.single)
       if (!file) {
         if (next) {
           next()

plans/README.md

@@ -12,7 +12,7 @@ Generated by the improve skill on 2026-09-01 at commit `2d978f84`. Execute in th
 | 004 | Contain remote asset materialization | P1 | S | - | DONE |
 | 005 | Block Data Inspector prototype-chain writes | P1 | S | - | DONE |
 | 006 | Validate request-derived authentication-link origins | P1 | M | - | TODO |
-| 007 | Reject pre-existing symlink escapes from filesystem roots | P2 | M | - | TODO |
+| 007 | Reject pre-existing symlink escapes from filesystem roots | P2 | M | - | DONE |
 
 Status values: TODO | IN PROGRESS | DONE | BLOCKED (with reason) | REJECTED (with rationale)
 

plugins/assets/src/node/context.ts

@@ -19,7 +19,12 @@ export interface AssetsConfig {
 }
 
 export interface AssetsContext extends AssetsConfig {
-  /** Resolve a root-relative path to an absolute one, rejecting escapes. */
+  /**
+   * Resolve a root-relative path to an absolute one, rejecting lexical
+   * escapes. Symlink-aware containment for reads and mutations lives in
+   * `node/paths` (`resolveAssetReadPath` / `assertAssetMutationPath`), which
+   * the RPC handlers call directly with {@link AssetsContext.dir}.
+   */
   resolvePath: (relativePath: string) => string
 }
 

plugins/assets/src/node/paths.ts

@@ -1,17 +1,67 @@
-import { resolve } from 'pathe'
+import fsp from 'node:fs/promises'
+import { normalize, resolve } from 'pathe'
 import { diagnostics } from '../diagnostics'
 
+/** realpath, pathe-normalized, or `null` when the path doesn't exist. */
+async function realpath(path: string): Promise<string | null> {
+  try {
+    return normalize(await fsp.realpath(path))
+  }
+  catch {
+    return null
+  }
+}
+
 /**
  * Resolve a client-supplied, root-relative path against the managed
- * directory, rejecting anything that would escape it (`..` traversal, a
- * rogue absolute path, etc.). Every RPC handler that touches the
- * filesystem goes through this — never trust a path from the wire.
+ * directory, rejecting anything that would escape it lexically (`..`
+ * traversal, a rogue absolute path). The first guard every RPC handler runs;
+ * symlink-aware containment is layered on by {@link resolveAssetReadPath}
+ * (reads) and {@link assertAssetMutationPath} (mutations).
  */
 export function resolveAssetPath(root: string, relativePath: string): string {
-  const cleaned = relativePath.replace(/^[/\\]+/, '')
   const normalizedRoot = resolve(root)
-  const absolute = resolve(normalizedRoot, cleaned)
+  const absolute = resolve(normalizedRoot, relativePath.replace(/^[/\\]+/, ''))
   if (absolute !== normalizedRoot && !absolute.startsWith(`${normalizedRoot}/`))
     throw diagnostics.DP_ASSETS_0001({ path: relativePath })
   return absolute
 }
+
+/**
+ * Resolve a path for a **read**, allowing a symlink only when its canonical
+ * target stays inside the canonical managed root. A target resolving outside
+ * throws `DP_ASSETS_0001`; a missing target is left for the caller's own read
+ * to fail.
+ */
+export async function resolveAssetReadPath(root: string, relativePath: string): Promise<string> {
+  const absolute = resolveAssetPath(root, relativePath)
+  const real = await realpath(absolute)
+  const canonRoot = (await realpath(root)) ?? resolve(root)
+  if (real && real !== canonRoot && !real.startsWith(`${canonRoot}/`))
+    throw diagnostics.DP_ASSETS_0001({ path: relativePath })
+  return absolute
+}
+
+/**
+ * Resolve a path for a **mutation**, rejecting every pre-existing symlink
+ * among the path components from the managed root down to the target
+ * (including in-root symlinks) so a mutation can never follow a symlink out
+ * of, or around, the root. Only existing components are inspected, so it is
+ * safe for not-yet-created upload/mkdir targets — call it again after
+ * creating directories and right before the I/O. This closes deterministic,
+ * pre-existing symlink escapes, not concurrent component-swap races.
+ */
+export async function assertAssetMutationPath(root: string, relativePath: string): Promise<string> {
+  const lexRoot = resolve(root)
+  const absolute = resolveAssetPath(root, relativePath)
+  let current = (await realpath(root)) ?? lexRoot
+  for (const segment of absolute.slice(lexRoot.length).split('/').filter(Boolean)) {
+    current += `/${segment}`
+    const stat = await fsp.lstat(current).catch(() => null)
+    if (!stat)
+      break
+    if (stat.isSymbolicLink())
+      throw diagnostics.DP_ASSETS_0001({ path: relativePath })
+  }
+  return absolute
+}

plugins/assets/src/node/scanner.ts

@@ -50,11 +50,17 @@ export function statToAssetInfo(dir: string, baseURL: string, relPath: string, s
 
 /** Recursively lists every file under `dir`, sorted alphabetically by path. */
 export async function scanAssets(dir: string, baseURL: string, includeFsPath = false): Promise<AssetInfo[]> {
-  const files = await glob(['**/*'], { cwd: dir, onlyFiles: true, dot: false })
+  // Never traverse into or across symlinks — a symlink inside the managed
+  // directory must not expose files (or whole trees) that live outside it.
+  const files = await glob(['**/*'], { cwd: dir, onlyFiles: true, dot: false, followSymbolicLinks: false })
 
   const infos = await Promise.all(files.map(async (relPath): Promise<AssetInfo | undefined> => {
     try {
       const stat = await fsp.lstat(join(dir, relPath))
+      // `lstat` describes the link itself; drop any symlink entry so the
+      // listing only ever names real files contained in the root.
+      if (stat.isSymbolicLink())
+        return undefined
       return statToAssetInfo(dir, baseURL, relPath, stat, includeFsPath)
     }
     catch {

plugins/assets/src/rpc/functions/delete.ts

@@ -3,6 +3,7 @@ import fsp from 'node:fs/promises'
 import { createDefineWrapperWithContext } from 'devframe/rpc'
 import { s } from 'devframe/utils/simple-schema'
 import { getAssetsContext } from '../../node/context'
+import { assertAssetMutationPath } from '../../node/paths'
 
 const defineAssetsRpc = createDefineWrapperWithContext<DevframeNodeContext>()
 
@@ -26,7 +27,8 @@ export const deleteAssets = defineAssetsRpc({
       handler: (async ({ paths }: { paths: string[] }): Promise<{ deleted: string[] }> => {
         const deleted: string[] = []
         for (const path of paths) {
-          const absolute = assets.resolvePath(path)
+          // Reject any pre-existing symlink component right before unlinking.
+          const absolute = await assertAssetMutationPath(assets.dir, path)
           try {
             await fsp.unlink(absolute)
             deleted.push(path)

plugins/assets/src/rpc/functions/mkdir.ts

@@ -4,6 +4,7 @@ import { createDefineWrapperWithContext } from 'devframe/rpc'
 import { s } from 'devframe/utils/simple-schema'
 import { diagnostics } from '../../diagnostics'
 import { getAssetsContext } from '../../node/context'
+import { assertAssetMutationPath } from '../../node/paths'
 
 const defineAssetsRpc = createDefineWrapperWithContext<DevframeNodeContext>()
 
@@ -24,11 +25,14 @@ export const mkdir = defineAssetsRpc({
     return {
       // See `list.ts` for why the async handler is cast.
       handler: (async ({ path }: { path: string }): Promise<void> => {
-        const absolute = assets.resolvePath(path)
+        const absolute = await assertAssetMutationPath(assets.dir, path)
         const stat = await fsp.stat(absolute).catch(() => undefined)
         if (stat && !stat.isDirectory())
           throw diagnostics.DP_ASSETS_0005({ path })
         await fsp.mkdir(absolute, { recursive: true })
+        // Re-check after creation: reject any symlink component that
+        // materialized under the root before anything follows this path.
+        await assertAssetMutationPath(assets.dir, path)
       }) as any,
     }
   },

plugins/assets/src/rpc/functions/read-image-meta.ts

@@ -5,6 +5,7 @@ import { createDefineWrapperWithContext } from 'devframe/rpc'
 import { s } from 'devframe/utils/simple-schema'
 import { imageMeta } from 'image-meta'
 import { getAssetsContext } from '../../node/context'
+import { resolveAssetReadPath } from '../../node/paths'
 
 const defineAssetsRpc = createDefineWrapperWithContext<DevframeNodeContext>()
 
@@ -30,7 +31,7 @@ export const readImageMeta = defineAssetsRpc({
       // See `list.ts` for why the async handler is cast.
       handler: (async (path: string): Promise<AssetImageMeta | null> => {
         try {
-          const buffer = await fsp.readFile(assets.resolvePath(path))
+          const buffer = await fsp.readFile(await resolveAssetReadPath(assets.dir, path))
           const meta = imageMeta(buffer)
           return { width: meta.width, height: meta.height, orientation: meta.orientation }
         }

plugins/assets/src/rpc/functions/read-text.ts

@@ -3,6 +3,7 @@ import fsp from 'node:fs/promises'
 import { createDefineWrapperWithContext } from 'devframe/rpc'
 import { s } from 'devframe/utils/simple-schema'
 import { getAssetsContext } from '../../node/context'
+import { resolveAssetReadPath } from '../../node/paths'
 
 const defineAssetsRpc = createDefineWrapperWithContext<DevframeNodeContext>()
 
@@ -26,7 +27,7 @@ export const readText = defineAssetsRpc({
       // See `list.ts` for why the async handler is cast.
       handler: (async (path: string, limit: number = DEFAULT_LIMIT): Promise<string | null> => {
         try {
-          const content = await fsp.readFile(assets.resolvePath(path), 'utf-8')
+          const content = await fsp.readFile(await resolveAssetReadPath(assets.dir, path), 'utf-8')
           return content.slice(0, limit)
         }
         catch {