From adce2eb3467c7e3ec571edd945370e9b1b7f2c82 Mon Sep 17 00:00:00 2001 From: yulia-ivashko Date: Thu, 6 Aug 2026 16:11:39 +0300 Subject: [PATCH] fix(snapshot): stop handing tar a path it may read as a remote host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Creating a workspace on Windows failed partway through and rolled back, taking roughly twenty-five seconds to do it — long enough that the pods appeared, started, and were then killed, which reads as a timeout rather than a failure. The cause was the source snapshot: `tar -cf C:\...\source.tar` is a request to GNU tar to connect to a host named "C", and it answers "Cannot connect to C: resolve failed". Which tar answers is decided by PATH order. Windows ships bsdtar in System32, which takes the path literally and works; Git for Windows ships GNU tar, which does not, and Git for Windows usually comes first. The same command therefore succeeded or failed depending on what else was installed — and every provider snapshots the same way, so this was never specific to Kubernetes. `--force-local` would fix GNU tar and break bsdtar, which rejects the option, so the archive is now written through stdout and no path is passed to tar at all. Nothing has to detect a flavour, and nothing has to be revisited for the next tool that takes a path. Verified on Windows against a live cluster: creation with GNU tar first on PATH failed after 25 seconds before this change and completes in 78 seconds after it. Two long-standing Windows test failures in the snapshot suite were the same bug and now pass. --- src/process.js | 62 +++++++++++++++++++++++++++++++++++++++++++++++++ src/snapshot.js | 7 ++++-- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/src/process.js b/src/process.js index d91740c..25ea45c 100644 --- a/src/process.js +++ b/src/process.js @@ -1,4 +1,5 @@ import { spawn, spawnSync } from 'node:child_process'; +import { createWriteStream } from 'node:fs'; import { canonicalWorkspaceLabelID } from './label-id.js'; import { ProcessError } from './errors.js'; @@ -94,6 +95,67 @@ export function run(binary, args, options = {}) { }); } +/** + * Runs a command and writes its stdout to a file, so the destination never has to be + * passed to the command as an argument. + * + * That matters on Windows: GNU tar reads `C:\path` as a remote `host:path` and fails + * with "Cannot connect to C", while the bsdtar shipped in System32 accepts it. Which one + * runs is decided by PATH order, so the same command works or fails depending on whether + * Git for Windows appears first — and Git for Windows very often does. Handing the path + * over stdout removes the question instead of detecting the flavour, which would have to + * be re-detected for every tool that takes a path. + */ +export function runToFile(binary, args, destinationPath, options = {}) { + if (typeof binary !== 'string' || !binary || !Array.isArray(args) || args.some((arg) => typeof arg !== 'string')) { + return Promise.reject(new TypeError('Process runner requires an executable and string argument array')); + } + const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; + const display = formatCommand(binary, args, options.sensitiveArgs ?? [], []); + return new Promise((resolve, reject) => { + const output = createWriteStream(destinationPath); + const child = spawn(binary, args, { + cwd: options.cwd, + env: options.env ? { ...process.env, ...options.env } : process.env, + stdio: ['ignore', 'pipe', 'pipe'], + windowsHide: true, + }); + let stderr = ''; + let settled = false; + const fail = (kind, message, details = {}) => { + if (settled) return; + settled = true; + clearTimeout(timer); + output.destroy(); + reject(new ProcessError(`${display} ${message}${stderr ? `: ${stderr.trim()}` : ''}`.trim(), { kind, stderr, ...details })); + }; + const timer = setTimeout(() => { + if (process.platform === 'win32' && child.pid) spawn('taskkill', ['/pid', String(child.pid), '/T', '/F'], { stdio: 'ignore', windowsHide: true }); + else child.kill('SIGKILL'); + fail('timeout', `timed out after ${timeoutMs}ms`); + }, timeoutMs); + + child.stderr.on('data', (chunk) => { if (stderr.length < 64 * 1024) stderr += String(chunk); }); + child.on('error', (error) => fail('spawn', `could not start: ${error.message}`)); + output.on('error', (error) => fail('exit', `could not write output: ${error.message}`)); + child.stdout.pipe(output); + child.on('close', (code, signal) => { + if (settled) return; + if (code !== 0) { + fail('exit', `failed with ${code ?? signal}`, { exitCode: code, signal }); + return; + } + // Resolve only once the file is closed, so a caller may read it immediately. + output.end(() => { + if (settled) return; + settled = true; + clearTimeout(timer); + resolve({ stderr }); + }); + }); + }); +} + export function spawnBackground(binary, args, options = {}) { return spawn(binary, args, { cwd: options.cwd, diff --git a/src/snapshot.js b/src/snapshot.js index 483df52..5150db4 100644 --- a/src/snapshot.js +++ b/src/snapshot.js @@ -3,7 +3,7 @@ import { createReadStream } from 'node:fs'; import { chmod, lstat, mkdtemp, readFile, readdir, readlink, realpath, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { dirname, isAbsolute, join, relative, resolve, sep } from 'node:path'; -import { run } from './process.js'; +import { run, runToFile } from './process.js'; const DEFAULT_LIMITS = Object.freeze({ maxEntries: 100_000, maxBytes: 2 * 1024 ** 3, maxFileBytes: 256 * 1024 ** 2 }); const RESERVED_ROOTS = new Set(['.openchamber', '.openchamber-runtime']); @@ -37,7 +37,10 @@ export async function createSourceSnapshot(sourceDirectory, options = {}) { await chmod(temporaryDirectory, 0o700); const archivePath = join(temporaryDirectory, 'source.tar'); try { - await run('tar', ['-cf', archivePath, '--exclude', './.git', '.'], { cwd: root, env: { COPYFILE_DISABLE: '1' }, timeoutMs: options.timeoutMs ?? 300_000, maxOutputBytes: 64 * 1024 }); + // Written through stdout rather than by naming the file to tar: a Windows path is a + // remote host to GNU tar and an ordinary path to bsdtar, and PATH order decides which + // one answers. + await runToFile('tar', ['-cf', '-', '--exclude', './.git', '.'], archivePath, { cwd: root, env: { COPYFILE_DISABLE: '1' }, timeoutMs: options.timeoutMs ?? 300_000 }); const after = await scanSourceTree(root, limits); if (JSON.stringify(before.entries) !== JSON.stringify(after.entries)) throw new Error('Workspace source changed while the immutable snapshot was being created'); const generation = createHash('sha256').update(JSON.stringify(before.entries.map(({ mtimeNs, ...entry }) => entry))).digest('hex');