Skip to content

Commit 7bbc587

Browse files
committed
fix: enforce symlink-aware filesystem roots
1 parent 1c9f789 commit 7bbc587

16 files changed

Lines changed: 393 additions & 32 deletions

File tree

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

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import type { AddressInfo } from 'node:net'
22
import type { ServeStaticOptions } from './serve-static'
3-
import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs'
3+
import { mkdirSync, mkdtempSync, symlinkSync, writeFileSync } from 'node:fs'
44
import { createServer } from 'node:http'
55
import { tmpdir } from 'node:os'
66
import { join } from 'node:path'
7+
import process from 'node:process'
78
import { H3, toNodeHandler } from 'h3'
89
import { afterEach, describe, expect, it } from 'vitest'
910
import { mountStaticHandler, serveStaticHandler, serveStaticNodeMiddleware } from './serve-static'
@@ -216,6 +217,66 @@ describe('mountStaticHandler', () => {
216217
})
217218
})
218219

220+
// Symlinks require privileges on Windows that hosted CI runners lack, so
221+
// gate the symlink-containment suite off that platform.
222+
describe.skipIf(process.platform === 'win32')('serveStaticHandler symlink containment', () => {
223+
let fx: Fixture | undefined
224+
225+
afterEach(async () => {
226+
await fx?.close()
227+
fx = undefined
228+
})
229+
230+
it('returns 404 for a file symlink escaping the served root', async () => {
231+
const dir = makeTmp('devframe-serve-link-')
232+
const outside = makeTmp('devframe-serve-outside-')
233+
writeFileSync(join(outside, 'secret.txt'), 'top secret', 'utf-8')
234+
symlinkSync(join(outside, 'secret.txt'), join(dir, 'leak.txt'))
235+
writeFileSync(join(dir, 'ok.txt'), 'in root', 'utf-8')
236+
fx = await startH3(dir, { single: false })
237+
238+
const leak = await fetch(`${fx.baseUrl}/leak.txt`)
239+
expect(leak.status).toBe(404)
240+
// Ordinary in-root files still serve.
241+
const ok = await fetch(`${fx.baseUrl}/ok.txt`)
242+
expect(ok.status).toBe(200)
243+
expect(await ok.text()).toBe('in root')
244+
})
245+
246+
it('returns 404 for a file reached through an escaping directory symlink', async () => {
247+
const dir = makeTmp('devframe-serve-link-')
248+
const outside = makeTmp('devframe-serve-outside-')
249+
writeFileSync(join(outside, 'secret.txt'), 'top secret', 'utf-8')
250+
symlinkSync(outside, join(dir, 'escape'))
251+
fx = await startH3(dir, { single: false })
252+
253+
const res = await fetch(`${fx.baseUrl}/escape/secret.txt`)
254+
expect(res.status).toBe(404)
255+
})
256+
257+
it('serves a symlink whose canonical target stays inside the served root', async () => {
258+
const dir = makeTmp('devframe-serve-link-')
259+
writeFileSync(join(dir, 'real.txt'), 'contained', 'utf-8')
260+
symlinkSync(join(dir, 'real.txt'), join(dir, 'alias.txt'))
261+
fx = await startH3(dir, { single: false })
262+
263+
const res = await fetch(`${fx.baseUrl}/alias.txt`)
264+
expect(res.status).toBe(200)
265+
expect(await res.text()).toBe('contained')
266+
})
267+
268+
it('returns 404 through the Node middleware for an escaping symlink', async () => {
269+
const dir = makeTmp('devframe-serve-link-')
270+
const outside = makeTmp('devframe-serve-outside-')
271+
writeFileSync(join(outside, 'secret.txt'), 'top secret', 'utf-8')
272+
symlinkSync(join(outside, 'secret.txt'), join(dir, 'leak.txt'))
273+
fx = await startMw(dir, { single: false })
274+
275+
const res = await fetch(`${fx.baseUrl}/leak.txt`)
276+
expect(res.status).toBe(404)
277+
})
278+
})
279+
219280
describe('serveStaticNodeMiddleware', () => {
220281
let fx: Fixture | undefined
221282

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

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { IncomingMessage, ServerResponse } from 'node:http'
33
import type { ReadableStream as NodeWebReadableStream } from 'node:stream/web'
44
import type { RemoteAssetsErrorMessage, RemoteAssetsStore } from '../types/remote-assets'
55
import { createReadStream } from 'node:fs'
6-
import { stat } from 'node:fs/promises'
6+
import { realpath, stat } from 'node:fs/promises'
77
import { Readable } from 'node:stream'
88
import { defineHandler, H3 } from 'h3'
99
import { lookup } from 'mrmime'
@@ -31,11 +31,39 @@ interface ResolvedFile {
3131

3232
const HTML_EXTENSIONS = ['.html', '.htm']
3333

34-
async function statFile(abs: string): Promise<ResolvedFile | null> {
34+
/** `child === root` or a path nested beneath it, using pathe's `/` separator. */
35+
function isWithin(child: string, root: string): boolean {
36+
return child === root || child.startsWith(root + sep)
37+
}
38+
39+
/**
40+
* The canonical (symlink-resolved) served root. Falls back to the lexical
41+
* path when the directory does not exist yet so an empty deployment simply
42+
* serves nothing rather than throwing.
43+
*/
44+
async function canonicalRoot(absDir: string): Promise<string> {
45+
try {
46+
return normalize(await realpath(absDir))
47+
}
48+
catch {
49+
return absDir
50+
}
51+
}
52+
53+
/**
54+
* Stat a candidate file and confirm its canonical target stays inside the
55+
* canonical served root, so a symlink inside the root can only resolve to a
56+
* file that is still within the root. A symlink escaping the root reads as a
57+
* miss (`null`), not a leak.
58+
*/
59+
async function statFile(abs: string, realRoot: string): Promise<ResolvedFile | null> {
3560
try {
3661
const s = await stat(abs)
3762
if (!s.isFile())
3863
return null
64+
const real = normalize(await realpath(abs))
65+
if (!isWithin(real, realRoot))
66+
return null
3967
return { abs, size: s.size, mtime: s.mtime }
4068
}
4169
catch {
@@ -45,6 +73,7 @@ async function statFile(abs: string): Promise<ResolvedFile | null> {
4573

4674
async function resolveTarget(
4775
absDir: string,
76+
realRoot: string,
4877
urlPath: string,
4978
indexNames: string[],
5079
single: boolean,
@@ -67,15 +96,15 @@ async function resolveTarget(
6796
if (abs !== absDir && !abs.startsWith(absDir + sep))
6897
return null
6998

70-
const direct = await statFile(abs)
99+
const direct = await statFile(abs, realRoot)
71100
if (direct)
72101
return direct
73102

74103
try {
75104
const s = await stat(abs)
76105
if (s.isDirectory()) {
77106
for (const name of indexNames) {
78-
const candidate = await statFile(join(abs, name))
107+
const candidate = await statFile(join(abs, name), realRoot)
79108
if (candidate)
80109
return candidate
81110
}
@@ -90,15 +119,15 @@ async function resolveTarget(
90119
// fallback so pretty-URL deployments resolve to the right page.
91120
if (!extname(cleaned)) {
92121
for (const ext of HTML_EXTENSIONS) {
93-
const candidate = await statFile(abs + ext)
122+
const candidate = await statFile(abs + ext, realRoot)
94123
if (candidate)
95124
return candidate
96125
}
97126
}
98127

99128
const fallbackIndex = indexNames[0]
100129
if (single && fallbackIndex && !/\.[a-z0-9]+$/i.test(cleaned)) {
101-
const indexFile = await statFile(join(absDir, fallbackIndex))
130+
const indexFile = await statFile(join(absDir, fallbackIndex), realRoot)
102131
if (indexFile)
103132
return indexFile
104133
}
@@ -199,14 +228,18 @@ export function serveStaticHandler(
199228
return serveRemoteAssetsHandler(source)
200229
const absDir = resolve(source)
201230
const opts = normalizeOptions(options)
231+
// Canonicalize the served root once and reuse it — the containment check
232+
// compares every candidate's canonical path against this.
233+
let realRootPromise: Promise<string> | undefined
234+
const getRealRoot = (): Promise<string> => (realRootPromise ??= canonicalRoot(absDir))
202235
return defineHandler(async (event) => {
203236
const method = event.req.method
204237
if (method !== 'GET' && method !== 'HEAD') {
205238
event.res.status = 405
206239
event.res.headers.set('Allow', 'GET, HEAD')
207240
return ''
208241
}
209-
const file = await resolveTarget(absDir, event.url.pathname, opts.indexNames, opts.single)
242+
const file = await resolveTarget(absDir, await getRealRoot(), event.url.pathname, opts.indexNames, opts.single)
210243
if (!file) {
211244
event.res.status = 404
212245
return ''
@@ -250,6 +283,8 @@ export function serveStaticNodeMiddleware(
250283
): (req: IncomingMessage, res: ServerResponse, next?: (err?: Error) => void) => void {
251284
const absDir = typeof source === 'string' ? resolve(source) : undefined
252285
const opts = normalizeOptions(options)
286+
let realRootPromise: Promise<string> | undefined
287+
const getRealRoot = (dir: string): Promise<string> => (realRootPromise ??= canonicalRoot(dir))
253288
return (req, res, next) => {
254289
void (async () => {
255290
const method = req.method
@@ -282,7 +317,7 @@ export function serveStaticNodeMiddleware(
282317
return
283318
}
284319

285-
const file = await resolveTarget(absDir, url, opts.indexNames, opts.single)
320+
const file = await resolveTarget(absDir, await getRealRoot(absDir), url, opts.indexNames, opts.single)
286321
if (!file) {
287322
if (next) {
288323
next()

plans/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ Generated by the improve skill on 2026-09-01 at commit `2d978f84`. Execute in th
1212
| 004 | Contain remote asset materialization | P1 | S | - | TODO |
1313
| 005 | Block Data Inspector prototype-chain writes | P1 | S | - | TODO |
1414
| 006 | Validate request-derived authentication-link origins | P1 | M | - | TODO |
15-
| 007 | Reject pre-existing symlink escapes from filesystem roots | P2 | M | - | TODO |
15+
| 007 | Reject pre-existing symlink escapes from filesystem roots | P2 | M | - | DONE |
1616

1717
Status values: TODO | IN PROGRESS | DONE | BLOCKED (with reason) | REJECTED (with rationale)
1818

plugins/assets/src/node/context.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { DevframeNodeContext, RpcStreamingChannel } from 'devframe'
2-
import { resolveAssetPath } from './paths'
2+
import { assertAssetMutationPath, resolveAssetPath, resolveAssetReadPath } from './paths'
33

44
export interface AssetsConfig {
55
/** Directory this devframe manages. */
@@ -19,8 +19,19 @@ export interface AssetsConfig {
1919
}
2020

2121
export interface AssetsContext extends AssetsConfig {
22-
/** Resolve a root-relative path to an absolute one, rejecting escapes. */
22+
/** Resolve a root-relative path to an absolute one, rejecting lexical escapes. */
2323
resolvePath: (relativePath: string) => string
24+
/**
25+
* Resolve a path for a read, allowing an in-root symlink only when its
26+
* canonical target stays inside the managed root.
27+
*/
28+
resolveReadPath: (relativePath: string) => Promise<string>
29+
/**
30+
* Resolve a path for a mutation, rejecting every pre-existing symlink
31+
* component. Call again after creating directories and right before the
32+
* mutating I/O.
33+
*/
34+
assertMutationPath: (relativePath: string) => Promise<string>
2435
}
2536

2637
const configs = new WeakMap<DevframeNodeContext, AssetsConfig>()
@@ -54,6 +65,8 @@ export function getAssetsContext(ctx: DevframeNodeContext): AssetsContext {
5465
baseURL: config?.baseURL ?? '/',
5566
uploadChannel: config?.uploadChannel,
5667
resolvePath: (relativePath: string) => resolveAssetPath(dir, relativePath),
68+
resolveReadPath: (relativePath: string) => resolveAssetReadPath(dir, relativePath),
69+
assertMutationPath: (relativePath: string) => assertAssetMutationPath(dir, relativePath),
5770
}
5871
contexts.set(ctx, built)
5972
return built

plugins/assets/src/node/paths.ts

Lines changed: 100 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
1-
import { resolve } from 'pathe'
1+
import fsp from 'node:fs/promises'
2+
import { dirname, normalize, resolve } from 'pathe'
23
import { diagnostics } from '../diagnostics'
34

45
/**
56
* Resolve a client-supplied, root-relative path against the managed
67
* directory, rejecting anything that would escape it (`..` traversal, a
7-
* rogue absolute path, etc.). Every RPC handler that touches the
8-
* filesystem goes through this — never trust a path from the wire.
8+
* rogue absolute path, etc.). This is the lexical guard every RPC handler
9+
* that touches the filesystem goes through first — never trust a path from
10+
* the wire.
11+
*
12+
* Lexical checks alone cannot see symlinks: use {@link resolveAssetReadPath}
13+
* (reads) or {@link assertAssetMutationPath} (mutations) to also close
14+
* pre-existing symlink escapes.
915
*/
1016
export function resolveAssetPath(root: string, relativePath: string): string {
1117
const cleaned = relativePath.replace(/^[/\\]+/, '')
@@ -15,3 +21,94 @@ export function resolveAssetPath(root: string, relativePath: string): string {
1521
throw diagnostics.DP_ASSETS_0001({ path: relativePath })
1622
return absolute
1723
}
24+
25+
/** `child === root` or a path nested beneath it, using pathe's `/` separator. */
26+
function isWithin(child: string, root: string): boolean {
27+
return child === root || child.startsWith(`${root}/`)
28+
}
29+
30+
/**
31+
* The canonical (symlink-resolved) managed root. Falls back to the lexical
32+
* path when the directory does not exist yet.
33+
*/
34+
async function canonicalRoot(root: string): Promise<string> {
35+
const normalizedRoot = resolve(root)
36+
try {
37+
return normalize(await fsp.realpath(normalizedRoot))
38+
}
39+
catch {
40+
return normalizedRoot
41+
}
42+
}
43+
44+
/**
45+
* Canonical path of the nearest existing ancestor of `absolute` (the target
46+
* itself when it exists), with every symlink along the way resolved.
47+
*/
48+
async function nearestExistingCanonical(absolute: string): Promise<string> {
49+
let current = absolute
50+
for (;;) {
51+
try {
52+
return normalize(await fsp.realpath(current))
53+
}
54+
catch {
55+
const parent = dirname(current)
56+
if (parent === current)
57+
return current
58+
current = parent
59+
}
60+
}
61+
}
62+
63+
/**
64+
* Resolve a path for a **read**, allowing a symlink only when its canonical
65+
* target stays inside the canonical managed root. Lexical escapes and
66+
* symlinks whose canonical target leaves the root both throw
67+
* `DP_ASSETS_0001`.
68+
*/
69+
export async function resolveAssetReadPath(root: string, relativePath: string): Promise<string> {
70+
const absolute = resolveAssetPath(root, relativePath)
71+
const canonRoot = await canonicalRoot(root)
72+
const nearest = await nearestExistingCanonical(absolute)
73+
if (!isWithin(nearest, canonRoot))
74+
throw diagnostics.DP_ASSETS_0001({ path: relativePath })
75+
return absolute
76+
}
77+
78+
/**
79+
* Resolve a path for a **mutation**, rejecting every pre-existing symlink
80+
* among the path components from the managed root down to the target —
81+
* including in-root symlinks — so a mutation can never follow a symlink out
82+
* of (or around) the root. Walks only components that already exist, so it
83+
* is safe for not-yet-created upload/mkdir targets; call it again after
84+
* creating directories and immediately before the mutating I/O to re-check
85+
* the freshly materialized components.
86+
*
87+
* This closes deterministic, pre-existing symlink escapes; it does not
88+
* defeat a concurrent local process swapping a component between this check
89+
* and the I/O.
90+
*/
91+
export async function assertAssetMutationPath(root: string, relativePath: string): Promise<string> {
92+
const absolute = resolveAssetPath(root, relativePath)
93+
const canonRoot = await canonicalRoot(root)
94+
const lexicalRoot = resolve(root)
95+
const rel = absolute === lexicalRoot ? '' : absolute.slice(lexicalRoot.length + 1)
96+
const segments = rel ? rel.split('/') : []
97+
98+
let current = canonRoot
99+
for (const segment of segments) {
100+
current = `${current}/${segment}`
101+
let stat
102+
try {
103+
stat = await fsp.lstat(current)
104+
}
105+
catch {
106+
// This component does not exist yet — nothing deeper can either, so
107+
// there is no pre-existing symlink left to reject.
108+
break
109+
}
110+
if (stat.isSymbolicLink())
111+
throw diagnostics.DP_ASSETS_0001({ path: relativePath })
112+
}
113+
return absolute
114+
}

plugins/assets/src/node/scanner.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,17 @@ export function statToAssetInfo(dir: string, baseURL: string, relPath: string, s
5050

5151
/** Recursively lists every file under `dir`, sorted alphabetically by path. */
5252
export async function scanAssets(dir: string, baseURL: string, includeFsPath = false): Promise<AssetInfo[]> {
53-
const files = await glob(['**/*'], { cwd: dir, onlyFiles: true, dot: false })
53+
// Never traverse into or across symlinks — a symlink inside the managed
54+
// directory must not expose files (or whole trees) that live outside it.
55+
const files = await glob(['**/*'], { cwd: dir, onlyFiles: true, dot: false, followSymbolicLinks: false })
5456

5557
const infos = await Promise.all(files.map(async (relPath): Promise<AssetInfo | undefined> => {
5658
try {
5759
const stat = await fsp.lstat(join(dir, relPath))
60+
// `lstat` describes the link itself; drop any symlink entry so the
61+
// listing only ever names real files contained in the root.
62+
if (stat.isSymbolicLink())
63+
return undefined
5864
return statToAssetInfo(dir, baseURL, relPath, stat, includeFsPath)
5965
}
6066
catch {

0 commit comments

Comments
 (0)