core+cli: never write a server-named download outside --out-dir or over a file

DOWNLOAD_FORM names come from the server. safeDownloadName keeps the last path
segment only (either separator), strips control characters and refuses empty,
. and ..; qqq process run writes with flag wx and takes name-1.ext, name-2.ext
when the name is taken. Tested with traversal names.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
james.maesandClaude Fable 5.1 committed 2026-10-31 17:50:20 -05:00
1 parent 0ef9bfb364
commit ffedcbb2ff
3 files changed
+46 -4

No files matched your search

+17 -3
View File
@@ -7,7 +7,7 @@ import { mkdirSync, readFileSync, writeFileSync } from 'node:fs'
import { basename, join } from 'node:path'
import type { Command } from 'commander'
import { UiApiError } from '@qrun-io/qqq-ui-api-client'
import { BACK, FormInputError, FormSession, ProcessCancelledError, ProcessError, blocksToText, checkInputRecords, downloadProcessFile, formatTable, helpText, isMissingValue, processHelpText, runProcess, screenToText, summaryLines, type FieldMeta, type ProcessMeta, type RunOutcome, type ScreenBlock } from '@qrun-io/qqq-core'
import { BACK, alternativeNames, safeDownloadName, FormInputError, FormSession, ProcessCancelledError, ProcessError, blocksToText, checkInputRecords, downloadProcessFile, formatTable, helpText, isMissingValue, processHelpText, runProcess, screenToText, summaryLines, type FieldMeta, type ProcessMeta, type RunOutcome, type ScreenBlock } from '@qrun-io/qqq-core'
import { UsageError, type Context } from '../context.js'
import { ask, promptForm } from '../prompt.js'
import { authenticatedFetch } from '../session.js'
@@ -94,8 +94,8 @@ export function registerProcess(program: Command, ctx: () => Context): void {
if (block.kind !== 'download') continue
const file = await downloadProcessFile(c.server, fetchImpl, block)
mkdirSync(options.outDir, { recursive: true })
const path = join(options.outDir, basename(block.fileName))
writeFileSync(path, file.bytes)
// the name comes from the server: last segment only, and never overwrite an existing file
const path = writeNew(options.outDir, safeDownloadName(block.fileName), file.bytes)
saved.push(path)
c.note(`saved ${path} (${file.bytes.length} bytes${file.contentType ? `, ${file.contentType}` : ''})`)
}
@@ -199,6 +199,20 @@ export function registerProcess(program: Command, ctx: () => Context): void {
interface RunOptions { values?: string[]; valuesJson?: string; records?: string; filterJson?: string; file?: string[]; interactive?: boolean; prompt: boolean; timeout: string; skipValidation?: boolean; validate?: boolean; outDir: string; recordLimit: string; quiet?: boolean }
/** Writes bytes to a new file in `dir` (flag wx), taking `name-1.ext`, `name-2.ext`, … when the name is taken. */
function writeNew(dir: string, name: string, bytes: Uint8Array): string {
for (const candidate of alternativeNames(name)) {
const path = join(dir, candidate)
try {
writeFileSync(path, bytes, { flag: 'wx' })
return path
} catch (error) {
if ((error as NodeJS.ErrnoException).code !== 'EEXIST') throw error
}
}
throw new UsageError(`no free file name for ${name} in ${dir}`)
}
function outcomeStepLabel(meta: ProcessMeta, stepName: string): string {
return meta.frontendSteps?.find((s) => s.name === stepName)?.label ?? stepName
}
+19
View File
@@ -140,6 +140,25 @@ export function downloadReference(values: FieldValues): { fileName: string; quer
return undefined
}
/**
* A server-supplied file name made safe to write in a chosen directory: the last path segment
* only (either separator), without control characters; empty, `.` and `..` are refused (throws).
*/
export function safeDownloadName(name: string): string {
const base = name.split(/[\\/]/).pop()!.replace(/[\u0000-\u001f\u007f]/g, '').trim()
if (!base || base === '.' || base === '..') throw new Error(`unsafe file name from the server: ${JSON.stringify(name)}`)
return base
}
/** Candidate names when one exists already: `a.csv`, `a-1.csv`, `a-2.csv`, … */
export function* alternativeNames(name: string): Generator<string> {
yield name
const dot = name.lastIndexOf('.')
const stem = dot > 0 ? name.slice(0, dot) : name
const ext = dot > 0 ? name.slice(dot) : ''
for (let i = 1; i < 1000; i++) yield `${stem}-${i}${ext}`
}
/** Downloads a DOWNLOAD_FORM file (GET /download/{file}); returns its bytes and content type. */
export async function downloadProcessFile(baseUrl: string, fetchImpl: typeof fetch, reference: { fileName: string; query: Record<string, string> }): Promise<{ bytes: Uint8Array; contentType: string }> {
const result = await callOperation(baseUrl, 'download', { path: { file: reference.fileName }, query: reference.query }, fetchImpl)
+10 -1
View File
@@ -6,7 +6,7 @@
import assert from 'node:assert/strict'
import { test } from 'node:test'
import { BACK, ProcessCancelledError, ProcessError, checkInputRecords, hasInputs, inputFields, runProcess, type ProcessMeta, type ProcessStep } from '../process/runner.js'
import { buildScreen, downloadReference, screenToText, summaryLines, type ScreenBlock } from '../process/screen.js'
import { alternativeNames, buildScreen, downloadReference, safeDownloadName, screenToText, summaryLines, type ScreenBlock } from '../process/screen.js'
type Handler = (request: Request, url: URL) => Promise<unknown> | unknown
@@ -238,3 +238,12 @@ test('screens: help content, HELP_TEXT preview, WIDGET named and ad hoc, Google
assert.equal(downloadReference({ downloadFileName: 'a.csv' }), undefined)
assert.deepEqual(summaryLines([{ status: 'OK', tableName: 'person', recordId: 4, linkPreText: 'Created ', linkText: 'Avery' }]), [{ status: 'OK', count: undefined, message: 'Created Avery', bullets: undefined, link: 'person 4' }])
})
test('download names from the server are reduced to a safe base name', () => {
assert.equal(safeDownloadName('../../etc/passwd'), 'passwd')
assert.equal(safeDownloadName('..\\..\\Windows\\win.ini'), 'win.ini')
assert.equal(safeDownloadName('report\u0007.csv'), 'report.csv')
for (const bad of ['', '.', '..', '../', 'a/..', '\u0001']) assert.throws(() => safeDownloadName(bad), /unsafe file name/)
const names = alternativeNames('out.csv')
assert.deepEqual([names.next().value, names.next().value, names.next().value], ['out.csv', 'out-1.csv', 'out-2.csv'])
})