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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 39 additions & 2 deletions packages/devframe/src/utils/remote-assets.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import type { AddressInfo } from 'node:net'
import type { MockInstance } from 'vitest'
import type { RemoteAssets, RemoteAssetsErrorMessage, RemoteAssetsStore } from '../types/remote-assets'
import type { RemoteAssets, RemoteAssetsErrorMessage, RemoteAssetsProviderCustom, RemoteAssetsStore } from '../types/remote-assets'
import { existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, writeFileSync } from 'node:fs'
import { createServer } from 'node:http'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { dirname, join } from 'node:path'
import { pathToFileURL } from 'node:url'
import { H3, toNodeHandler } from 'h3'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
Expand Down Expand Up @@ -188,6 +188,43 @@ describe('resolveStaticAssetsSource (remote store)', () => {
expect(existsSync(join(target, 'package.json'))).toBe(false)
})

it('rejects unsafe provider-listed paths before fetching or writing them', async () => {
const calls: string[] = []
const fetchImpl: typeof globalThis.fetch = async (input) => {
const url = String(input)
calls.push(url)
return url.endsWith('/dist/assets/app.js') ? new Response('console.log("app")') : new Response('should never be served')
}
const provider: RemoteAssetsProviderCustom = {
fileUrl: (pkg, version, filePath) => `https://mirror.example.com/${pkg}@${version}/${filePath}`,
// A compromised (or merely buggy) custom provider — every entry below is
// unsafe or out of scope except the one normal nested asset.
listFiles: async () => [
'package.json', // ordinary file outside the selected prefix — stays ignored
'dist/assets/app.js', // a normal nested asset — still materializes
'dist/../evil-traversal.txt', // prefixed traversal entry
'/outside/evil-absolute.txt', // absolute path entry
'dist/evil\\..\\..\\evil-backslash.txt', // backslash traversal entry — rejected on every platform
'dist-confusable/evil-prefix.txt', // prefix-confusion entry — outside the selected prefix
],
}
const store = storeFor({ fetch: fetchImpl }, makeTmp(), { provider })
const target = makeTmp()

await store.materialize(target)

// The one normal nested asset still materializes.
expect(readFileSync(join(target, 'assets/app.js'), 'utf8')).toBe('console.log("app")')
// Nothing else was fetched...
expect(calls).toEqual([expect.stringContaining('/dist/assets/app.js')])
// ...or written, inside or outside the target directory.
expect(existsSync(join(target, 'package.json'))).toBe(false)
expect(existsSync(join(target, 'evil-traversal.txt'))).toBe(false)
expect(existsSync(join(dirname(target), 'evil-traversal.txt'))).toBe(false)
expect(existsSync(join(target, 'evil-backslash.txt'))).toBe(false)
expect(existsSync(join(dirname(target), 'evil-prefix.txt'))).toBe(false)
})

it('supports the unpkg provider URL scheme', async () => {
const calls: string[] = []
const fetchImpl: typeof globalThis.fetch = async (input) => {
Expand Down
30 changes: 27 additions & 3 deletions packages/devframe/src/utils/remote-assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { createRequire } from 'node:module'
import { Readable } from 'node:stream'
import { lookup } from 'mrmime'
import { createDebug } from 'obug'
import { dirname, extname, join, normalize, sep } from 'pathe'
import { dirname, extname, isAbsolute, join, normalize, resolve, sep } from 'pathe'
import { diagnostics } from '../node/diagnostics'

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

/**
* Resolve the safe destination for a provider-listed `filePath` beneath
* `prefix`, materializing into the already-resolved `root`, or `null` when
* the entry is unsafe or lies outside the selected `prefix`. A compromised
* provider is an untrusted boundary even though the built-in jsDelivr/unpkg
* listings never emit any of this — reject an absolute path, a backslash
* (never normalized into a separator; Windows-style traversal stays rejected
* on every platform), and any suffix whose resolved destination would land
* outside `root`. `target === root` is deliberately unsafe too: it names the
* directory itself, never a writable file.
*/
function materializeTarget(filePath: string, prefix: string, root: string): string | null {
if (filePath.includes('\\') || isAbsolute(filePath) || !filePath.startsWith(prefix))
return null
const suffix = filePath.slice(prefix.length)
if (!suffix || isAbsolute(suffix))
return null
const target = resolve(root, suffix)
return target === root || !target.startsWith(root + sep) ? null : target
}

function createStore(assets: RemoteAssets, cacheDir: string): RemoteAssetsStore {
const normalized = { ...assets, path: assets.path ?? 'dist' }
const { provider, name: providerName } = resolveProvider(assets)
Expand Down Expand Up @@ -340,8 +361,11 @@ function createStore(assets: RemoteAssets, cacheDir: string): RemoteAssetsStore
catch (error) {
return fail(errText(error), error)
}
for (const filePath of files.filter(f => f.startsWith(prefix))) {
const target = join(targetDir, filePath.slice(prefix.length))
const root = resolve(targetDir)
for (const filePath of files) {
const target = materializeTarget(filePath, prefix, root)
if (target == null)
continue
const url = provider.fileUrl(normalized.package, normalized.version, filePath)
let res: Response
try {
Expand Down
2 changes: 1 addition & 1 deletion plans/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Generated by the improve skill on 2026-09-01 at commit `2d978f84`. Execute in th
| 001 | Pin privileged GitHub Actions dependencies | P1 | S | - | TODO |
| 002 | Require authentication on route-based MCP | P1 | M | 001 | TODO |
| 003 | Enforce shared-state exposure policy on direct MCP reads | P1 | S | 002 | TODO |
| 004 | Contain remote asset materialization | P1 | S | - | TODO |
| 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 |
Expand Down
Loading