fix(docker): clean up exec output handling and remove app:list workaround

lib/docker.ts

@@ -251,9 +251,8 @@ export const configureNextcloud = async function(apps = ['viewer'], vendoredBran
 	console.log('│  └─ OK !')
 
 	// Build app list
-	const json = await runOcc(['app:list', '--output', 'json'], { container })
-	// fix dockerode bug returning invalid leading characters
-	const applist = JSON.parse(json.substring(json.indexOf('{')))
+	const { stdout: json } = await runOcc(['app:list', '--output', 'json'], { container, verbose: true })
+	const applist = JSON.parse(json)
 
 	// Enable apps and give status
 	for (const app of apps) {
@@ -263,7 +262,8 @@ export const configureNextcloud = async function(apps = ['viewer'], vendoredBran
 			// built in or mounted already as the app under development
 			await runOcc(['app:enable', '--force', app], { container, verbose: true })
 		} else {
-			const { shippedApps } = JSON.parse(await runExec(['cat', 'core/shipped.json'], { container }))
+			const { stdout: jsonOutput } = await runExec(['cat', 'core/shipped.json'], { container })
+			const { shippedApps } = JSON.parse(jsonOutput)
 			if (shippedApps.includes(app)) {
 				const branchOption = ['main', 'master'].includes(vendoredBranch) ? [] : [`--branch=${vendoredBranch}`]
 				// apps that are vendored but still missing (i.e. not build in or mounted already)
@@ -408,13 +408,19 @@ interface RunExecOptions {
 	verbose: boolean
 }
 
+type RunExecResult = {
+	stdout: string
+	stderr: string
+	exitCode: number
+}
+
 /**
- * Execute a command in the container
+ * Execute a command in the container and return stdout/stderr separately.
  */
-export const runExec = async function(
+export async function runExec(
 	command: string | string[],
 	{ container, user='www-data', verbose=false, env=[], failOnError=true }: Partial<RunExecOptions> = {},
-) {
+): Promise<RunExecResult> {
 	container = container || getContainer()
 	const exec = await container.exec({
 		Cmd: typeof command === 'string' ? [command] : command,
@@ -424,48 +430,109 @@ export const runExec = async function(
 		Env: env,
 	})
 
-	return new Promise<string>((resolve, reject) => {
-		const dataStream = new PassThrough()
+	return new Promise<RunExecResult>((resolve, reject) => {
+		const stdoutStream = new PassThrough()
+		const stderrStream = new PassThrough()
 
-		exec.start({}, (err, stream) => {
-			if (stream) {
-				// Pass stdout and stderr to dataStream
-				exec.modem.demuxStream(stream, dataStream, dataStream)
-				stream.on('end', () => dataStream.end())
-			} else {
-				reject(err)
+		const stdout: string[] = []
+		const stderr: string[] = []
+
+		let settled = false
+		let finishedStreams = 0
+
+		const cleanup = () => {
+			stdoutStream.removeAllListeners()
+			stderrStream.removeAllListeners()
+		}
+
+		const settleResolve = (result: RunExecResult) => {
+			if (settled) {
+				return
+			}
+			settled = true
+			cleanup()
+			resolve(result)
+		}
+
+		const settleReject = (err: unknown) => {
+			if (settled) {
+				return
+			}
+			settled = true
+			cleanup()
+			reject(err)
+		}
+
+		const maybeResolve = async () => {
+			finishedStreams++
+			if (finishedStreams === 2) {
+				const inspectionResult = await exec.inspect()
+				const result = {
+					stdout: stdout.join(''),
+					stderr: stderr.join(''),
+					exitCode: inspectionResult.ExitCode ?? 0,
+				}
+
+				if (result.exitCode && failOnError) {
+					settleReject(new Error('command exited with non-zero exit code', { cause: result }))
+					return
+				}
+				settleResolve(result)
+			}
+		}
+
+		stdoutStream.on('data', (chunk) => {
+			const text = chunk.toString('utf8')
+			stdout.push(text)
+			if (verbose && text.trim()) {
+				console.log(`├─ stdout: ${text.trim().replace(/\n/gi, '\n├─ stdout: ')}`)
 			}
 		})
 
-		const data: string[] = []
-		dataStream.on('data', (chunk) => {
-			data.push(chunk.toString('utf8'))
-			const printable = data.at(-1)?.trim()
-			if (verbose && printable) {
-				console.log(`├─ ${printable.replace(/\n/gi, '\n├─ ')}`)
+		stderrStream.on('data', (chunk) => {
+			const text = chunk.toString('utf8')
+			stderr.push(text)
+			if (verbose && text.trim()) {
+				console.log(`├─ stderr: ${text.trim().replace(/\n/gi, '\n├─ stderr: ')}`)
 			}
 		})
-		dataStream.on('error', (err) => reject(err))
-		dataStream.on('end', async () => {
-			const result = await exec.inspect()
-			if (result.ExitCode && failOnError) {
-				reject(new Error(data.join(''), { cause: result.ExitCode }))
+
+		stdoutStream.on('error', settleReject)
+		stderrStream.on('error', settleReject)
+
+		stdoutStream.on('end', maybeResolve)
+		stderrStream.on('end', maybeResolve)
+
+		exec.start({}, (err, stream) => {
+			if (err) {
+				settleReject(err)
+				return
+			}
+			if (!stream) {
+				settleReject(new Error('No exec stream returned'))
 				return
 			}
-			resolve(data.join(''))
+
+			stream.on('error', settleReject)
+			stream.on('end', () => {
+				stdoutStream.end()
+				stderrStream.end()
+			})
+
+			exec.modem.demuxStream(stream, stdoutStream, stderrStream)
 		})
 	})
 }
 
 /**
  * Execute an occ command in the container
  */
-export const runOcc = function(
+export async function runOcc(
 	command: string | string[],
-	{ container, env=[], verbose=false }: Partial<Omit<RunExecOptions, 'user'>> = {},
+	{ container, env=[], verbose=false, ...rest }: Partial<Omit<RunExecOptions, 'user'>> = {},
 ) {
 	const cmdArray = typeof command === 'string' ? [command] : command
-	return runExec(['php', 'occ', ...cmdArray], { container, verbose, env })
+	return runExec(['php', 'occ', ...cmdArray], { ...rest, container, verbose, env })
 }
 
 /**
@@ -482,11 +549,12 @@ export const setSystemConfig = function(
 /**
  * Get a Nextcloud system config value from the container.
  */
-export const getSystemConfig = function(
+export async function getSystemConfig(
 	key: string,
 	{ container }: { container?: Docker.Container } = {},
 ) {
-	return runOcc(['config:system:get', key], { container })
+	const { stdout } = await runOcc(['config:system:get', key], { container })
+	return stdout.trim()
 }
 
 

tests/runExec.spec.ts

@@ -0,0 +1,98 @@
+/*
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+import type { Container } from 'dockerode'
+
+import assert from 'node:assert/strict'
+import { after, before, describe, test } from 'node:test'
+
+import { getContainer, runExec, startNextcloud, stopNextcloud, waitOnNextcloud } from '../lib/docker.ts'
+
+describe('Docker: runExec', async () => {
+	let container: Container
+
+	before(async () => {
+		const ip = await startNextcloud('master', false)
+		await waitOnNextcloud(ip)
+		container = getContainer()
+	})
+
+	after(async () => await stopNextcloud())
+
+	await test('captures stdout of a command', async () => {
+		const { stdout, stderr, exitCode } = await runExec(['echo', 'hello world'], { container })
+		assert.equal(stdout.trim(), 'hello world')
+		assert.equal(stderr, '')
+		assert.equal(exitCode, 0)
+	})
+
+	await test('accepts a plain string command', async () => {
+		const { stdout, exitCode } = await runExec('hostname', { container })
+		assert.ok(stdout.trim().length > 0)
+		assert.equal(exitCode, 0)
+	})
+
+	await test('captures stderr separately from stdout', async () => {
+		const { stdout, stderr, exitCode } = await runExec(
+			['sh', '-c', 'echo out; echo err >&2'],
+			{ container },
+		)
+		assert.equal(stdout.trim(), 'out')
+		assert.equal(stderr.trim(), 'err')
+		assert.equal(exitCode, 0)
+	})
+
+	await test('rejects on a non-zero exit code by default', async () => {
+		await assert.rejects(
+			runExec(['sh', '-c', 'echo boom >&2; exit 3'], { container }),
+			(err: Error & { cause?: { stderr: string, exitCode: number } }) => {
+				assert.match(err.message, /non-zero exit code/)
+				// the collected result is attached as the error cause
+				assert.equal(err.cause?.exitCode, 3)
+				assert.equal(err.cause?.stderr.trim(), 'boom')
+				return true
+			},
+		)
+	})
+
+	await test('does not reject on a non-zero exit code when failOnError is false', async () => {
+		const { stdout, stderr, exitCode } = await runExec(
+			['sh', '-c', 'echo out; echo err >&2; exit 5'],
+			{ container, failOnError: false },
+		)
+		assert.equal(exitCode, 5)
+		assert.equal(stdout.trim(), 'out')
+		assert.equal(stderr.trim(), 'err')
+	})
+
+	await test('runs as the www-data user by default', async () => {
+		const { stdout } = await runExec('whoami', { container })
+		assert.equal(stdout.trim(), 'www-data')
+	})
+
+	await test('runs as the requested user', async () => {
+		const { stdout } = await runExec('whoami', { container, user: 'root' })
+		assert.equal(stdout.trim(), 'root')
+	})
+
+	await test('forwards environment variables to the command', async () => {
+		const { stdout } = await runExec(
+			['sh', '-c', 'echo "$MY_TEST_VAR"'],
+			{ container, env: ['MY_TEST_VAR=from-env'] },
+		)
+		assert.equal(stdout.trim(), 'from-env')
+	})
+
+	await test('handles large multi-chunk output without truncation', async () => {
+		// seq produces 100000 lines, forcing the stream to be delivered in
+		// multiple chunks that runExec must concatenate in order
+		const { stdout, exitCode } = await runExec(['seq', '1', '100000'], { container })
+		const lines = stdout.trim().split('\n')
+		assert.equal(exitCode, 0)
+		assert.equal(lines.length, 100000)
+		assert.equal(lines[0], '1')
+		assert.equal(lines.at(-1), '100000')
+	})
+})