Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 62 additions & 1 deletion packages/devframe/src/utils/serve-static.test.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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

Expand Down
38 changes: 29 additions & 9 deletions packages/devframe/src/utils/serve-static.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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 }
}
Expand All @@ -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,
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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 ''
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion plans/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | - | TODO |
| 005 | Block Data Inspector prototype-chain writes | P1 | S | - | TODO |
| 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)

Expand Down
7 changes: 6 additions & 1 deletion plugins/assets/src/node/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
62 changes: 56 additions & 6 deletions plugins/assets/src/node/paths.ts
Original file line number Diff line number Diff line change
@@ -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
}
8 changes: 7 additions & 1 deletion plugins/assets/src/node/scanner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 3 additions & 1 deletion plugins/assets/src/rpc/functions/delete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>()

Expand All @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion plugins/assets/src/rpc/functions/mkdir.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>()

Expand All @@ -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,
}
},
Expand Down
3 changes: 2 additions & 1 deletion plugins/assets/src/rpc/functions/read-image-meta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>()

Expand All @@ -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 }
}
Expand Down
3 changes: 2 additions & 1 deletion plugins/assets/src/rpc/functions/read-text.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>()

Expand All @@ -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 {
Expand Down
Loading
Loading