Skip to content

Commit 529c6de

Browse files
authored
fix(devframe): contain remote asset materialization (#328)
1 parent 740fa95 commit 529c6de

3 files changed

Lines changed: 67 additions & 6 deletions

File tree

packages/devframe/src/utils/remote-assets.test.ts

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import type { AddressInfo } from 'node:net'
22
import type { MockInstance } from 'vitest'
3-
import type { RemoteAssets, RemoteAssetsErrorMessage, RemoteAssetsStore } from '../types/remote-assets'
3+
import type { RemoteAssets, RemoteAssetsErrorMessage, RemoteAssetsProviderCustom, RemoteAssetsStore } from '../types/remote-assets'
44
import { existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, writeFileSync } from 'node:fs'
55
import { createServer } from 'node:http'
66
import { tmpdir } from 'node:os'
7-
import { join } from 'node:path'
7+
import { dirname, join } from 'node:path'
88
import { pathToFileURL } from 'node:url'
99
import { H3, toNodeHandler } from 'h3'
1010
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
@@ -188,6 +188,43 @@ describe('resolveStaticAssetsSource (remote store)', () => {
188188
expect(existsSync(join(target, 'package.json'))).toBe(false)
189189
})
190190

191+
it('rejects unsafe provider-listed paths before fetching or writing them', async () => {
192+
const calls: string[] = []
193+
const fetchImpl: typeof globalThis.fetch = async (input) => {
194+
const url = String(input)
195+
calls.push(url)
196+
return url.endsWith('/dist/assets/app.js') ? new Response('console.log("app")') : new Response('should never be served')
197+
}
198+
const provider: RemoteAssetsProviderCustom = {
199+
fileUrl: (pkg, version, filePath) => `https://mirror.example.com/${pkg}@${version}/${filePath}`,
200+
// A compromised (or merely buggy) custom provider — every entry below is
201+
// unsafe or out of scope except the one normal nested asset.
202+
listFiles: async () => [
203+
'package.json', // ordinary file outside the selected prefix — stays ignored
204+
'dist/assets/app.js', // a normal nested asset — still materializes
205+
'dist/../evil-traversal.txt', // prefixed traversal entry
206+
'/outside/evil-absolute.txt', // absolute path entry
207+
'dist/evil\\..\\..\\evil-backslash.txt', // backslash traversal entry — rejected on every platform
208+
'dist-confusable/evil-prefix.txt', // prefix-confusion entry — outside the selected prefix
209+
],
210+
}
211+
const store = storeFor({ fetch: fetchImpl }, makeTmp(), { provider })
212+
const target = makeTmp()
213+
214+
await store.materialize(target)
215+
216+
// The one normal nested asset still materializes.
217+
expect(readFileSync(join(target, 'assets/app.js'), 'utf8')).toBe('console.log("app")')
218+
// Nothing else was fetched...
219+
expect(calls).toEqual([expect.stringContaining('/dist/assets/app.js')])
220+
// ...or written, inside or outside the target directory.
221+
expect(existsSync(join(target, 'package.json'))).toBe(false)
222+
expect(existsSync(join(target, 'evil-traversal.txt'))).toBe(false)
223+
expect(existsSync(join(dirname(target), 'evil-traversal.txt'))).toBe(false)
224+
expect(existsSync(join(target, 'evil-backslash.txt'))).toBe(false)
225+
expect(existsSync(join(dirname(target), 'evil-prefix.txt'))).toBe(false)
226+
})
227+
191228
it('supports the unpkg provider URL scheme', async () => {
192229
const calls: string[] = []
193230
const fetchImpl: typeof globalThis.fetch = async (input) => {

packages/devframe/src/utils/remote-assets.ts

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { createRequire } from 'node:module'
1111
import { Readable } from 'node:stream'
1212
import { lookup } from 'mrmime'
1313
import { createDebug } from 'obug'
14-
import { dirname, extname, join, normalize, sep } from 'pathe'
14+
import { dirname, extname, isAbsolute, join, normalize, resolve, sep } from 'pathe'
1515
import { diagnostics } from '../node/diagnostics'
1616

1717
const debugFetch = createDebug('devframe:remote-assets:fetch')
@@ -189,6 +189,27 @@ function candidatePaths(prefix: string, cleaned: string): string[] {
189189
return candidates
190190
}
191191

192+
/**
193+
* Resolve the safe destination for a provider-listed `filePath` beneath
194+
* `prefix`, materializing into the already-resolved `root`, or `null` when
195+
* the entry is unsafe or lies outside the selected `prefix`. A compromised
196+
* provider is an untrusted boundary even though the built-in jsDelivr/unpkg
197+
* listings never emit any of this — reject an absolute path, a backslash
198+
* (never normalized into a separator; Windows-style traversal stays rejected
199+
* on every platform), and any suffix whose resolved destination would land
200+
* outside `root`. `target === root` is deliberately unsafe too: it names the
201+
* directory itself, never a writable file.
202+
*/
203+
function materializeTarget(filePath: string, prefix: string, root: string): string | null {
204+
if (filePath.includes('\\') || isAbsolute(filePath) || !filePath.startsWith(prefix))
205+
return null
206+
const suffix = filePath.slice(prefix.length)
207+
if (!suffix || isAbsolute(suffix))
208+
return null
209+
const target = resolve(root, suffix)
210+
return target === root || !target.startsWith(root + sep) ? null : target
211+
}
212+
192213
function createStore(assets: RemoteAssets, cacheDir: string): RemoteAssetsStore {
193214
const normalized = { ...assets, path: assets.path ?? 'dist' }
194215
const { provider, name: providerName } = resolveProvider(assets)
@@ -340,8 +361,11 @@ function createStore(assets: RemoteAssets, cacheDir: string): RemoteAssetsStore
340361
catch (error) {
341362
return fail(errText(error), error)
342363
}
343-
for (const filePath of files.filter(f => f.startsWith(prefix))) {
344-
const target = join(targetDir, filePath.slice(prefix.length))
364+
const root = resolve(targetDir)
365+
for (const filePath of files) {
366+
const target = materializeTarget(filePath, prefix, root)
367+
if (target == null)
368+
continue
345369
const url = provider.fileUrl(normalized.package, normalized.version, filePath)
346370
let res: Response
347371
try {

plans/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ Generated by the improve skill on 2026-09-01 at commit `2d978f84`. Execute in th
99
| 001 | Pin privileged GitHub Actions dependencies | P1 | S | - | TODO |
1010
| 002 | Require authentication on route-based MCP | P1 | M | 001 | TODO |
1111
| 003 | Enforce shared-state exposure policy on direct MCP reads | P1 | S | 002 | TODO |
12-
| 004 | Contain remote asset materialization | P1 | S | - | TODO |
12+
| 004 | Contain remote asset materialization | P1 | S | - | DONE |
1313
| 005 | Block Data Inspector prototype-chain writes | P1 | S | - | DONE |
1414
| 006 | Validate request-derived authentication-link origins | P1 | M | - | TODO |
1515
| 007 | Reject pre-existing symlink escapes from filesystem roots | P2 | M | - | TODO |

0 commit comments

Comments
 (0)