Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions packages/core/src/exec.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { mkdtemp, rm, writeFile } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { delimiter, join } from 'node:path';
import { afterEach, describe, expect, it } from 'vitest';
import { ensureCli, exec } from './exec.js';

const tempDirs: string[] = [];
const oldPath = process.env.PATH;

afterEach(async () => {
process.env.PATH = oldPath;
await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true })));
});

describe('exec', () => {
it('preserves percent-wrapped arguments on Windows shell execution', async () => {
const binDir = await mkdtemp(join(tmpdir(), 'sh1pt-exec-bin-'));
tempDirs.push(binDir);
await installEchoArgsCli(binDir, 'sh1pt-echo-args');
process.env.PATH = `${binDir}${delimiter}${oldPath ?? ''}`;

const result = await exec('sh1pt-echo-args', ['%SH1PT_EXEC_LITERAL%', 'C:\\tmp\\path\\'], {
log: () => {},
});

expect(result.exitCode).toBe(0);
expect(JSON.parse(result.stdout.trim())).toEqual(['%SH1PT_EXEC_LITERAL%', 'C:\\tmp\\path\\']);
});
});

describe('ensureCli', () => {
it('throws when a command exits non-zero instead of reporting it as installed', async () => {
const binDir = await mkdtemp(join(tmpdir(), 'sh1pt-exec-bin-'));
tempDirs.push(binDir);
await installFailingCli(binDir, 'sh1pt-missing-version');
process.env.PATH = `${binDir}${delimiter}${oldPath ?? ''}`;

await expect(ensureCli('sh1pt-missing-version', 'install it', () => {}))
.rejects.toThrow('sh1pt-missing-version not installed. install it');
});
});

async function installFailingCli(binDir: string, name: string): Promise<void> {
if (process.platform === 'win32') {
await writeFile(join(binDir, `${name}.cmd`), '@echo off\r\nexit /b 9009\r\n', 'utf-8');
return;
}

const script = join(binDir, name);
await writeFile(script, '#!/usr/bin/env sh\nexit 127\n', { encoding: 'utf-8', mode: 0o755 });
}

async function installEchoArgsCli(binDir: string, name: string): Promise<void> {
const helper = join(binDir, 'echo-args.js');
await writeFile(helper, 'console.log(JSON.stringify(process.argv.slice(2)));\n', 'utf-8');

if (process.platform === 'win32') {
await writeFile(
join(binDir, `${name}.cmd`),
`@echo off\r\n"${process.execPath}" "%~dp0echo-args.js" %*\r\n`,
'utf-8',
);
return;
}

const script = join(binDir, name);
await writeFile(script, `#!/usr/bin/env sh\n"${process.execPath}" "${helper}" "$@"\n`, {
encoding: 'utf-8',
mode: 0o755,
});
}
22 changes: 17 additions & 5 deletions packages/core/src/exec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { spawn } from 'node:child_process';
import type { SpawnOptions } from 'node:child_process';
import type { BuildContext } from './target.js';

type LogFn = BuildContext['log'];
Expand Down Expand Up @@ -28,22 +29,25 @@ export async function exec(cmd: string, args: string[], opts: ExecOptions): Prom
}

return new Promise<ExecResult>((resolve, reject) => {
const child = spawn(cmd, args, {
const spawnOptions: SpawnOptions = {
cwd: opts.cwd,
env: { ...process.env, ...extraEnv },
stdio: ['ignore', 'pipe', 'pipe'],
});
};
const child = shouldUseWindowsCmd(cmd)
? spawn('cmd.exe', ['/d', '/s', '/c', cmd, ...args], spawnOptions)
: spawn(cmd, args, spawnOptions);

let stdout = '';
let stderr = '';

child.stdout.on('data', (chunk: Buffer) => {
child.stdout?.on('data', (chunk: Buffer) => {
const text = chunk.toString();
stdout += text;
for (const line of text.split('\n')) if (line) opts.log(line);
});

child.stderr.on('data', (chunk: Buffer) => {
child.stderr?.on('data', (chunk: Buffer) => {
const text = chunk.toString();
stderr += text;
for (const line of text.split('\n')) if (line) opts.log(line, 'warn');
Expand All @@ -69,9 +73,17 @@ export async function exec(cmd: string, args: string[], opts: ExecOptions): Prom
});
}

function shouldUseWindowsCmd(cmd: string): boolean {
return process.platform === 'win32'
&& !cmd.includes('/')
&& !cmd.includes('\\')
&& !/\.(?:exe|com)$/i.test(cmd);
}

export async function ensureCli(cmd: string, installHint: string, log: LogFn): Promise<void> {
try {
await exec(cmd, ['--version'], { log: () => {}, throwOnNonZero: false });
const result = await exec(cmd, ['--version'], { log: () => {}, throwOnNonZero: false });
if (result.exitCode !== 0) throw new Error(`command not found: ${cmd}`);
Comment on lines +85 to +86
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 ensureCli now rejects installed CLIs that exit non-zero for --version

On Linux (where spawn still directly executes the binary), any CLI that returns a non-zero exit code from --version — even though it is installed and working — will now be thrown as command not found: <cmd> and ultimately reported as "not installed". Before this PR, only ENOENT triggered that branch on Linux; a non-zero exit was silently ignored and treated as "present". The broader exitCode !== 0 guard is necessary on Windows (where cmd.exe swallows ENOENT and returns 9009), but applying it unconditionally on Linux changes the contract for every caller. Consider limiting the exitCode check to process.platform === 'win32' so the Linux path retains its original ENOENT-only semantics.

} catch (err) {
if (err instanceof Error && err.message.startsWith('command not found')) {
log(`${cmd} not found on PATH`, 'error');
Expand Down
20 changes: 18 additions & 2 deletions packages/docs/pandoc/src/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { contractTestDocs } from '@profullstack/sh1pt-core/testing';
import { chmod, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { delimiter, join } from 'node:path';
import { afterEach, describe, expect, it } from 'vitest';
import docs from './index.js';

Expand Down Expand Up @@ -48,7 +48,7 @@ describe('docs-pandoc generation', () => {
const binDir = await mkdtemp(join(tmpdir(), 'sh1pt-pandoc-bin-'));
tempDirs.push(outDir, binDir);
await installFakePandoc(binDir);
process.env.PATH = `${binDir}:${oldPath ?? ''}`;
process.env.PATH = `${binDir}${delimiter}${oldPath ?? ''}`;

const result = await docs.generate({ secret: () => undefined, log: () => {}, dryRun: false }, {
kind: 'whitepaper',
Expand Down Expand Up @@ -90,6 +90,22 @@ describe('docs-pandoc generation', () => {
});

async function installFakePandoc(binDir: string): Promise<void> {
if (process.platform === 'win32') {
const helper = join(binDir, 'pandoc.js');
await writeFile(helper, [
'const { writeFileSync } = require("node:fs");',
'const { dirname, join } = require("node:path");',
'const args = process.argv.slice(2);',
'const outIndex = args.indexOf("-o");',
'const out = outIndex >= 0 ? args[outIndex + 1] : "";',
'if (!out) throw new Error("missing -o");',
'writeFileSync(join(dirname(out), "pandoc-args.json"), JSON.stringify(args));',
'writeFileSync(out, "fake pandoc output\\n");',
].join('\n'), 'utf-8');
await writeFile(join(binDir, 'pandoc.cmd'), `@echo off\r\n"${process.execPath}" "%~dp0\\pandoc.js" %*\r\n`, 'utf-8');
return;
}

const script = join(binDir, 'pandoc');
await writeFile(script, [
'#!/usr/bin/env bash',
Expand Down