fix(cache): keep download writes inside the downloads directory

POST /_web/cache/save-to-downloads joined the caller-supplied `filename`
straight into the downloads directory:

    const filePath = path.join(downloadsDir, filename)

`path.join` collapses `..`, so `{"filename":"../escaped.txt"}` resolved
one level above the downloads directory and the route wrote
attacker-chosen bytes there, returning `{ success: true }`. Any client
that can reach the route could therefore write outside the downloads
directory as the app user.

The route is registered by the shared web routes on both CLI Web and the
Electron internal API, so the fix covers both. Its own siblings already
constrain this input — merge.ts and the CLI upload route both run the
name through `path.basename` — so the missing guard here was an
oversight rather than a documented allowance.

Reject anything that is not a plain file name, and join only the
validated basename. Names the app itself sends stay unchanged: every
caller (conversation export, SQL export, screen capture) already passes a
flat file name.
This commit is contained in:
ttsdj authored and digua committed 2026-09-10 16:17:10 +08:00
1 parent 0e8339bc4c
commit 89974f34e3
2 files changed
+56 -1

No files matched your search

@@ -297,3 +297,50 @@ describe('registerCacheRoutes data directory routes', () => {
await app.close()
})
})
describe('registerCacheRoutes save-to-downloads', () => {
it('writes a plain file name inside the downloads directory', async () => {
const app = Fastify()
registerCacheRoutes(app, { pathProvider: createPathProvider() })
await app.ready()
const response = await app.inject({
method: 'POST',
url: '/_web/cache/save-to-downloads',
payload: {
filename: 'result.csv',
dataUrl: `data:text/csv;charset=utf-8,${encodeURIComponent('a,b\n1,2')}`,
},
})
assert.equal(response.statusCode, 200)
assert.deepEqual(response.json(), {
success: true,
filePath: path.join(testSystemDir, 'downloads', 'result.csv'),
})
await app.close()
})
it('refuses a file name that would escape the downloads directory', async () => {
const app = Fastify()
registerCacheRoutes(app, { pathProvider: createPathProvider() })
await app.ready()
const escapedPath = path.join(testSystemDir, 'escaped.txt')
const response = await app.inject({
method: 'POST',
url: '/_web/cache/save-to-downloads',
payload: {
filename: '../escaped.txt',
dataUrl: `data:text/plain;charset=utf-8,${encodeURIComponent('attacker controlled')}`,
},
})
assert.equal(response.statusCode, 200)
assert.deepEqual(response.json(), { success: false, error: 'Invalid filename' })
assert.equal(fs.existsSync(escapedPath), false)
await app.close()
})
})
+9 -1
View File
@@ -267,7 +267,15 @@ export function registerCacheRoutes(server: FastifyInstance, ctx: CacheRouteCont
fs.mkdirSync(downloadsDir, { recursive: true })
}
const filePath = path.join(downloadsDir, filename)
// Confine the write to the downloads directory: browser callers pick this
// name, and `path.join` collapses `..`, so anything but a plain file name
// would otherwise resolve outside `downloadsDir`.
const safeName = path.basename(filename)
if (safeName !== filename || safeName === '.' || safeName === '..') {
return { success: false, error: 'Invalid filename' }
}
const filePath = path.join(downloadsDir, safeName)
fs.writeFileSync(filePath, buffer)
return { success: true, filePath }