Fix literal path handling in volume copies

resources/processes/volume-cleanup.mjs

@@ -6,11 +6,22 @@ import path from 'node:path'
  * the original wildcard cleanup, top-level hidden entries remain untouched.
  */
 export function deleteFilesFromVolumeFolder(volumeName, targetFolder) {
-	const targetFolderPath = path.posix.join('/volume/', targetFolder, '/*')
+	const targetFolderPath = path.posix.join('/volume/', targetFolder)
 	const cleanup =
 		targetFolder === '/'
 			? "find /volume/ -mindepth 1 -maxdepth 1 ! -name '.*' ! -name 'main-statistics' -exec rm -rf {} +"
-			: `rm -rf ${targetFolderPath}`
+			: 'rm -rf "$1"/*'
 
-	return `docker run --rm -v ${volumeName}:/volume alpine sh -c "${cleanup}"`
+	return [
+		'run',
+		'--rm',
+		'-v',
+		`${volumeName}:/volume`,
+		'alpine',
+		'sh',
+		'-c',
+		cleanup,
+		'sh',
+		...(targetFolder === '/' ? [] : [targetFolderPath])
+	]
 }

resources/processes/volume-copy.mjs

@@ -1,5 +1,41 @@
 import path from 'node:path'
 
+// Docker receives argument data directly. Only the fixed script is shell syntax;
+// the paths following its $0 placeholder remain quoted positional parameters.
+function copyCommand(sourceFolder, volumeName, script, ...parameters) {
+	return [
+		'run',
+		'--rm',
+		'-v',
+		`${sourceFolder}:/host`,
+		'-v',
+		`${volumeName}:/volume`,
+		'-w',
+		'/host',
+		'alpine',
+		'sh',
+		'-c',
+		script,
+		'sh',
+		...parameters
+	]
+}
+
+/** Copy a host input into the requested folder in the volume. */
+export function copyFileToVolumeCommand(sourceFolder, fileName, volumeName, destFolder) {
+	const innerFilePath = path.posix.join('/host/', fileName)
+	const newDestFolder = path.posix.join('/volume/', destFolder)
+	const destFile = path.posix.join(newDestFolder, fileName)
+	return copyCommand(
+		sourceFolder,
+		volumeName,
+		'mkdir -p "$1" && cp -r "$2" "$3"',
+		newDestFolder,
+		innerFilePath,
+		destFile
+	)
+}
+
 /** Copy a volume output to the host, assigning private reports to the host user. */
 export function copyFileToHostCommand(
 	sourceFolder,
@@ -17,12 +53,13 @@ export function copyFileToHostCommand(
 		typeof process.getgid === 'function'
 			? `${process.getuid()}:${process.getgid()}`
 			: undefined
-	const ownership = hostOwner === undefined ? '' : ` && chown ${hostOwner} '${innerFilePath}'`
-
-	return `
-        docker run --rm -v "${sourceFolder}":/host -v ${volumeName}:/volume -w /host alpine
-        sh -c "rm -rf '${innerFilePath}' && cp -r '${destFile}' '${innerFilePath}'${ownership}"
-    `
-		.replace(/\s+/g, ' ')
-		.trim()
+	const script = 'rm -rf "$1" && cp -r "$2" "$1"'
+	return copyCommand(
+		sourceFolder,
+		volumeName,
+		hostOwner === undefined ? script : `${script} && chown "$3" "$1"`,
+		innerFilePath,
+		destFile,
+		...(hostOwner === undefined ? [] : [hostOwner])
+	)
 }

resources/processes/volume-server-script.mjs

@@ -1,22 +1,22 @@
 import express from 'express'
 import cors from 'cors'
 import path, { basename, dirname } from 'path'
-import { exec } from 'child_process'
+import { execFile } from 'child_process'
 import bodyParser from 'body-parser'
 import { deleteFilesFromVolumeFolder } from './volume-cleanup.mjs'
-import { copyFileToHostCommand } from './volume-copy.mjs'
+import { copyFileToHostCommand, copyFileToVolumeCommand } from './volume-copy.mjs'
 
 const app = express()
 app.use(cors())
 app.use(bodyParser.json())
 
 const PORT = process.argv[2] || 3001
 
-function runCommand(command) {
+function runCommand(args) {
 	return new Promise((resolve, reject) => {
-		exec(command, (error, stdout, stderr) => {
+		execFile('docker', args, (error, stdout, stderr) => {
 			if (error) {
-				reject(stderr)
+				reject(stderr || error.message)
 			} else {
 				resolve(stdout)
 			}
@@ -90,27 +90,3 @@ app.post('/clear-volume', async (req, res) => {
 })
 
 app.listen(PORT)
-
-/**
- * Creates a command to copy a file from the host file system into a volume
- * @param {string} sourceFolder The folder of the file on the host
- * @param {string} fileName The name of the file
- * @param {string} volumeName The volume to copy into
- * @param {string} destFolder The target folder in the volume
- * @returns An executable command
- */
-function copyFileToVolumeCommand(sourceFolder, fileName, volumeName, destFolder) {
-	const innerFilePath = path.posix.join('/host/', fileName)
-	const newDestFolder = path.posix.join('/volume/', destFolder)
-	const destFile = path.posix.join(newDestFolder, fileName)
-
-	// Construct the Docker command
-	const command = `
-    docker run --rm -v "${sourceFolder}":/host -v ${volumeName}:/volume -w /host alpine sh -c "
-    mkdir -p '${newDestFolder}' && cp -r '${innerFilePath}' '${destFile}'"
-`
-		.replace(/\s+/g, ' ')
-		.trim()
-
-	return command
-}

scripts/tests/volume-cleanup.test.mjs

@@ -24,10 +24,17 @@ async function cleanupFixture(runTest) {
 		`#!${process.execPath}
 const assert = require('node:assert/strict')
 const { execFile } = require('node:child_process')
-assert.deepEqual(process.argv.slice(2, -1), ['run', '--rm', '-v', 'fixture-volume:/volume', 'alpine', 'sh', '-c'])
-const script = process.argv.at(-1).replaceAll('/volume/', process.env.FIXTURE_VOLUME + '/')
+const args = process.argv.slice(2)
+assert.deepEqual(args.slice(0, 7), ['run', '--rm', '-v', 'fixture-volume:/volume', 'alpine', 'sh', '-c'])
+// The whole-volume find script has one fixed mount root. Plugin paths travel
+// exclusively in positional data, so their shell metacharacters stay literal.
+const script = args[7].replaceAll('/volume/', process.env.FIXTURE_VOLUME + '/')
+const parameters = args.slice(9).map(value => {
+  assert.ok(value.startsWith('/volume/'))
+  return process.env.FIXTURE_VOLUME + value.slice('/volume'.length)
+})
 assert.ok(!script.includes('/volume/'), 'Never execute against a real volume')
-execFile('/bin/sh', ['-c', script], { env: process.env, encoding: 'utf8' }, (error, stdout, stderr) => {
+execFile('/bin/sh', ['-c', script, args[8], ...parameters], { env: process.env, encoding: 'utf8' }, (error, stdout, stderr) => {
   process.stdout.write(stdout)
   process.stderr.write(stderr)
   if (error) process.stderr.write(String(error))
@@ -47,7 +54,7 @@ execFile('/bin/sh', ['-c', script], { env: process.env, encoding: 'utf8' }, (err
 			// Other Unix test hosts use their standard shell utilities.
 		}
 		const clear = (targetFolder) =>
-			run('/bin/sh', ['-c', deleteFilesFromVolumeFolder('fixture-volume', targetFolder)], {
+			run('docker', deleteFilesFromVolumeFolder('fixture-volume', targetFolder), {
 				env: { ...process.env, PATH: `${bin}:${process.env.PATH}`, FIXTURE_VOLUME: volume }
 			})
 		await runTest(volume, clear)
@@ -96,14 +103,15 @@ test(
 	},
 	async () => {
 		await cleanupFixture(async (volume, clear) => {
-			await mkdir(join(volume, 'main'))
-			await writeFile(join(volume, 'main', 'output.tif'), 'scratch')
-			await writeFile(join(volume, 'main', '.hidden'), 'preserved')
+			const plugin = 'plugin \' " $HOME `printf EXPANDED` $(printf EXPANDED)  folder'
+			await mkdir(join(volume, plugin))
+			await writeFile(join(volume, plugin, 'output.tif'), 'scratch')
+			await writeFile(join(volume, plugin, '.hidden'), 'preserved')
 			await mkdir(join(volume, 'main-statistics'))
 			await writeFile(join(volume, 'main-statistics', 'report.json'), '{}')
-			await clear('main')
-			await assert.rejects(access(join(volume, 'main', 'output.tif')))
-			assert.equal(await readFile(join(volume, 'main', '.hidden'), 'utf8'), 'preserved')
+			await clear(plugin)
+			await assert.rejects(access(join(volume, plugin, 'output.tif')))
+			assert.equal(await readFile(join(volume, plugin, '.hidden'), 'utf8'), 'preserved')
 			assert.equal(
 				await readFile(join(volume, 'main-statistics', 'report.json'), 'utf8'),
 				'{}'

scripts/tests/volume-copy.test.mjs

@@ -5,13 +5,16 @@ import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { promisify } from 'node:util'
 import test from 'node:test'
-import { copyFileToHostCommand } from '../../resources/processes/volume-copy.mjs'
+import {
+	copyFileToHostCommand,
+	copyFileToVolumeCommand
+} from '../../resources/processes/volume-copy.mjs'
 
 const run = promisify(execFile)
 
 async function copyFixture(plugin, runTest, failOwnership = false) {
 	const root = await mkdtemp(join(tmpdir(), 'ouroboros-volume-copy-'))
-	const host = join(root, 'host')
+	const host = join(root, 'host \' " $HOME `printf EXPANDED` $(printf EXPANDED)  spaces')
 	const volume = join(root, 'volume')
 	const bin = join(root, 'bin')
 	const ownershipLog = join(root, 'ownership')
@@ -27,9 +30,13 @@ async function copyFixture(plugin, runTest, failOwnership = false) {
 			`#!${process.execPath}
 const assert = require('node:assert/strict')
 const { execFile } = require('node:child_process')
-assert.deepEqual(process.argv.slice(2, -1), ['run', '--rm', '-v', process.env.FIXTURE_HOST + ':/host', '-v', 'fixture-volume:/volume', '-w', '/host', 'alpine', 'sh', '-c'])
-const script = process.argv.at(-1).replaceAll('/volume/', process.env.FIXTURE_VOLUME + '/').replaceAll('/host/', process.env.FIXTURE_HOST + '/')
-execFile('/bin/sh', ['-c', script], { env: process.env }, (error, stdout, stderr) => {
+const args = process.argv.slice(2)
+assert.deepEqual(args.slice(0, 11), ['run', '--rm', '-v', process.env.FIXTURE_HOST + ':/host', '-v', 'fixture-volume:/volume', '-w', '/host', 'alpine', 'sh', '-c'])
+// Substitute mount roots only in positional data, never in the shell script.
+const parameters = args.slice(13).map(value => value.startsWith('/volume/')
+  ? process.env.FIXTURE_VOLUME + value.slice('/volume'.length)
+  : value.startsWith('/host/') ? process.env.FIXTURE_HOST + value.slice('/host'.length) : value)
+execFile('/bin/sh', ['-c', args[11], args[12], ...parameters], { env: process.env }, (error, stdout, stderr) => {
   process.stdout.write(stdout)
   process.stderr.write(stderr)
   process.exit(error ? 1 : 0)
@@ -51,27 +58,28 @@ process.exit(result.status ?? 1)
 		)
 		await chmod(join(bin, 'docker'), 0o755)
 		await chmod(join(bin, 'chown'), 0o755)
-		const copy = () =>
-			run(
-				'/bin/sh',
-				[
-					'-c',
-					copyFileToHostCommand(host, 'report.json', 'fixture-volume', plugin, plugin)
-				],
-				{
-					env: {
-						...process.env,
-						PATH: `${bin}:${process.env.PATH}`,
-						FIXTURE_HOST: host,
-						FIXTURE_VOLUME: volume,
-						FIXTURE_OWNERSHIP_LOG: ownershipLog,
-						FIXTURE_REAL_CHOWN: realChown,
-						FIXTURE_FAIL_OWNERSHIP: failOwnership ? '1' : '0'
-					}
+		const execute = (args) =>
+			run('docker', args, {
+				env: {
+					...process.env,
+					PATH: `${bin}:${process.env.PATH}`,
+					FIXTURE_HOST: host,
+					FIXTURE_VOLUME: volume,
+					FIXTURE_OWNERSHIP_LOG: ownershipLog,
+					FIXTURE_REAL_CHOWN: realChown,
+					FIXTURE_FAIL_OWNERSHIP: failOwnership ? '1' : '0'
 				}
-			)
+			})
+		const copy = (fileName = 'report.json') =>
+			execute(copyFileToHostCommand(host, fileName, 'fixture-volume', plugin, plugin))
+		const copyToVolume = (fileName) =>
+			execute(copyFileToVolumeCommand(host, fileName, 'fixture-volume', plugin))
+
 		await runTest({
 			copy,
+			copyToVolume,
+			host,
+			volume,
 			delivered: join(host, 'report.json'),
 			source: join(volume, plugin, 'report.json'),
 			ownershipLog
@@ -145,3 +153,35 @@ console.log(copyFileToHostCommand('output', 'report.json', 'fixture-volume', 'ma
 	assert.match(stdout, /cp -r/)
 	assert.doesNotMatch(stdout, /chown/)
 })
+
+for (const directory of [false, true]) {
+	test(
+		`both copy directions preserve literal ${directory ? 'directory' : 'file'} paths`,
+		{ skip: process.platform === 'win32' },
+		async () => {
+			const plugin = 'plugin \' " $HOME `printf EXPANDED` $(printf EXPANDED)  folder'
+			const name = 'Bob\'s "stack" $HOME `printf EXPANDED` $(printf EXPANDED)  01.tif'
+			await copyFixture(plugin, async ({ copy, copyToVolume, host, volume }) => {
+				const hostPath = join(host, name)
+				const payload = directory ? join(hostPath, 'slice  01.tif') : hostPath
+				if (directory) await mkdir(hostPath)
+				await writeFile(payload, 'literal path payload')
+				await copyToVolume(name)
+				assert.equal(
+					await readFile(
+						join(volume, plugin, name, ...(directory ? ['slice  01.tif'] : [])),
+						'utf8'
+					),
+					'literal path payload'
+				)
+				await writeFile(payload, 'stale output')
+				if (directory)
+					await writeFile(join(hostPath, 'obsolete'), 'replace the old directory')
+				await copy(name)
+				assert.equal(await readFile(payload, 'utf8'), 'literal path payload')
+				if (directory)
+					await assert.rejects(stat(join(hostPath, 'obsolete')), { code: 'ENOENT' })
+			})
+		}
+	)
+}