fix: align Python check with runtime truth (python3) and harden edge cases

Review fixes for the installer Python check:

- Probe python3 first on all platforms: every runtime call site invokes a
  literal python3, so only that command vouches for BMAD features. Python
  found via py/python now gets an explicit mismatch warning instead of a
  false "all BMAD features supported".
- Treat closed/piped stdin as non-interactive (in addition to --yes) so
  scripted installs no longer silently exit 0 via clack's cancel path.
- Retry probes with shell:true on win32 EINVAL (CVE-2024-27980 hardening
  rejects .bat/.cmd shims like pyenv-win's without a shell).
- Add Suite 46 branch tests for checkPythonEnvironment with stubbed
  detection, prompts, and process.exit.
This commit is contained in:
Brian Madison 2026-06-11 21:03:34 -05:00
parent a8439bf196
commit b4f29bd2d1
3 changed files with 152 additions and 28 deletions

View File

@ -3353,9 +3353,101 @@ async function runTests() {
// result must be well-formed. (CI machines may or may not have Python.) // result must be well-formed. (CI machines may or may not have Python.)
const detected = detectPython(); const detected = detectPython();
assert( assert(
detected === null || (typeof detected.command === 'string' && typeof detected.version.raw === 'string'), detected === null ||
(typeof detected.command === 'string' &&
typeof detected.version.raw === 'string' &&
typeof detected.isRuntimeCommand === 'boolean'),
'detectPython returns null or a well-formed result', 'detectPython returns null or a well-formed result',
); );
// checkPythonEnvironment branch coverage — stub detection, prompts, and
// process.exit so the assertions are deterministic regardless of the
// machine's Python. python-check resolves detectPython via module.exports
// and prompts via the shared module object, so swapping properties works.
const pythonCheck = require('../tools/installer/core/python-check');
const promptsModule = require('../tools/installer/prompts');
const real = {
detectPython: pythonCheck.detectPython,
log: promptsModule.log,
note: promptsModule.note,
select: promptsModule.select,
cancel: promptsModule.cancel,
exit: process.exit,
};
const stub = (detectResult, selectAnswer) => {
const seen = { success: [], warn: [], info: [], note: [], select: [], cancel: [], exit: [] };
pythonCheck.detectPython = () => detectResult;
promptsModule.log = {
success: async (m) => void seen.success.push(m),
warn: async (m) => void seen.warn.push(m),
info: async (m) => void seen.info.push(m),
error: async () => {},
};
promptsModule.note = async (m, t) => void seen.note.push(t || m);
promptsModule.select = async (opts) => {
seen.select.push(opts.message);
return selectAnswer;
};
promptsModule.cancel = async (m) => void seen.cancel.push(m);
process.exit = (code) => {
seen.exit.push(code);
throw new Error('__stub_exit__');
};
return seen;
};
try {
const v = (major, minor, patch) => ({ major, minor, patch, raw: `${major}.${minor}.${patch}` });
// Branch: full support via the runtime command — success, no prompt.
let seen = stub({ command: 'python3', version: v(3, 12, 1), isRuntimeCommand: true }, 'continue');
let result = await pythonCheck.checkPythonEnvironment();
assert(result.status === 'full' && seen.success.length === 1, 'full support via python3 logs success');
assert(seen.select.length === 0 && seen.warn.length === 0, 'full support via python3 skips warning and ack prompt');
// Branch: modern Python found, but not as `python3` — runtime mismatch.
seen = stub({ command: 'py -3', version: v(3, 12, 0), isRuntimeCommand: false }, 'continue');
result = await pythonCheck.checkPythonEnvironment();
assert(seen.success.length === 0, 'python3-mismatch never reports full support');
assert(
seen.warn.length === 1 && seen.warn[0].includes('python3') && seen.warn[0].includes('py -3'),
'python3-mismatch warns that scripts invoke python3',
);
assert(seen.select.length === 1 && result.status === 'full', 'python3-mismatch still requires the ack prompt');
// Branch: partial support (3.83.10) — warn + ack, continue returns.
seen = stub({ command: 'python3', version: v(3, 9, 5), isRuntimeCommand: true }, 'continue');
result = await pythonCheck.checkPythonEnvironment();
assert(
result.status === 'partial' && seen.warn.length === 1 && seen.warn[0].includes('3.11+'),
'partial support warns about tomllib floor',
);
assert(seen.select.length === 1 && seen.exit.length === 0, 'partial support prompts and continue proceeds');
// Branch: no Python, non-interactive — warn + info, never prompts.
seen = stub(null, 'continue');
result = await pythonCheck.checkPythonEnvironment({ nonInteractive: true });
assert(result.status === 'none' && seen.warn[0].includes('No Python found'), 'non-interactive with no Python warns');
assert(seen.select.length === 0 && seen.info.length === 1, 'non-interactive skips the ack prompt and logs continuation');
// Branch: no Python, interactive, user quits — cancel message + exit 0.
seen = stub(null, 'quit');
let threw = false;
try {
await pythonCheck.checkPythonEnvironment();
} catch (error) {
threw = error.message === '__stub_exit__';
}
assert(threw && seen.exit.length === 1 && seen.exit[0] === 0, 'quit choice exits 0 (user-cancel convention)');
assert(seen.cancel.length === 1, 'quit choice shows the cancel guidance');
} finally {
pythonCheck.detectPython = real.detectPython;
promptsModule.log = real.log;
promptsModule.note = real.note;
promptsModule.select = real.select;
promptsModule.cancel = real.cancel;
process.exit = real.exit;
}
} catch (error) { } catch (error) {
console.log(`${colors.red}Test Suite 46 setup failed: ${error.message}${colors.reset}`); console.log(`${colors.red}Test Suite 46 setup failed: ${error.message}${colors.reset}`);
console.log(error.stack); console.log(error.stack);

View File

@ -7,14 +7,16 @@ const prompts = require('../prompts');
const PYTHON_FULL_SUPPORT = { major: 3, minor: 11 }; const PYTHON_FULL_SUPPORT = { major: 3, minor: 11 };
const PYTHON_PARTIAL_SUPPORT = { major: 3, minor: 8 }; const PYTHON_PARTIAL_SUPPORT = { major: 3, minor: 8 };
// Probe order matters: on Windows the `py` launcher is the most reliable way // Every runtime call site (skill steps, on_complete hooks) invokes a literal
// to find Python 3 (a bare `python` is often the Microsoft Store alias that // `python3`, so only that command's version vouches for BMAD features. The
// exits without printing a version). On POSIX, `python3` is canonical. // fallback probes exist to tell the user "Python is installed, but not under
// the name BMAD uses" instead of a misleading "No Python found".
const RUNTIME_COMMAND = 'python3';
const PROBE_CANDIDATES = const PROBE_CANDIDATES =
process.platform === 'win32' process.platform === 'win32'
? [ ? [
{ command: 'py', args: ['-3', '--version'] },
{ command: 'python3', args: ['--version'] }, { command: 'python3', args: ['--version'] },
{ command: 'py', args: ['-3', '--version'] },
{ command: 'python', args: ['--version'] }, { command: 'python', args: ['--version'] },
] ]
: [ : [
@ -57,24 +59,45 @@ function classifyPython(version) {
return 'unsupported'; return 'unsupported';
} }
/**
* Run one probe candidate and return its parsed version, or null.
* @param {{command: string, args: string[]}} candidate
* @returns {{major: number, minor: number, patch: number, raw: string}|null}
*/
function probeVersion(candidate) {
const run = (extra = {}) =>
spawnSync(candidate.command, candidate.args, {
encoding: 'utf8',
timeout: 5000,
windowsHide: true,
...extra,
});
let result = run();
// Node >=18.20/20.12 refuses to spawn .bat/.cmd without a shell
// (CVE-2024-27980 hardening) and reports EINVAL — pyenv-win ships its
// python shims as .bat. Args here are static literals, so a shell retry
// is injection-safe.
if (result.error && result.error.code === 'EINVAL' && process.platform === 'win32') {
result = run({ shell: true });
}
if (result.error) return null;
return parsePythonVersion(`${result.stdout || ''}\n${result.stderr || ''}`);
}
/** /**
* Probe the local environment for a Python interpreter. * Probe the local environment for a Python interpreter.
* Tries each candidate command and returns the first that reports a version. * Tries each candidate command and returns the first that reports a version.
* @returns {{command: string, version: {major: number, minor: number, patch: number, raw: string}}|null} * `isRuntimeCommand` is true only when the match is `python3` the command
* BMAD scripts actually invoke.
* @returns {{command: string, version: {major: number, minor: number, patch: number, raw: string}, isRuntimeCommand: boolean}|null}
*/ */
function detectPython() { function detectPython() {
for (const candidate of PROBE_CANDIDATES) { for (const candidate of PROBE_CANDIDATES) {
try { try {
const result = spawnSync(candidate.command, candidate.args, { const version = probeVersion(candidate);
encoding: 'utf8',
timeout: 5000,
windowsHide: true,
});
if (result.error) continue;
const version = parsePythonVersion(`${result.stdout || ''}\n${result.stderr || ''}`);
if (version) { if (version) {
const display = candidate.args.length > 1 ? `${candidate.command} ${candidate.args.slice(0, -1).join(' ')}` : candidate.command; const display = candidate.args.length > 1 ? `${candidate.command} ${candidate.args.slice(0, -1).join(' ')}` : candidate.command;
return { command: display, version }; return { command: display, version, isRuntimeCommand: candidate.command === RUNTIME_COMMAND };
} }
} catch { } catch {
// Candidate not runnable — try the next one. // Candidate not runnable — try the next one.
@ -85,9 +108,9 @@ function detectPython() {
function upgradeHints() { function upgradeHints() {
return [ return [
'How to get Python 3.11+:', 'How to get Python 3.11+ (as `python3`):',
' macOS: brew install python3', ' macOS: brew install python3',
' Windows: winget install Python.Python.3.12', ' Windows: winget install Python.Python.3.12 (then ensure `python3` resolves, e.g. enable the python3 alias)',
' Linux/WSL: sudo apt install python3 (Ubuntu 24.04+ ships 3.12; older distros: use pyenv or deadsnakes)', ' Linux/WSL: sudo apt install python3 (Ubuntu 24.04+ ships 3.12; older distros: use pyenv or deadsnakes)',
' Docker: add python3 to your image (e.g. apk add python3 / apt-get install -y python3)', ' Docker: add python3 to your image (e.g. apk add python3 / apt-get install -y python3)',
].join('\n'); ].join('\n');
@ -98,23 +121,30 @@ function upgradeHints() {
* *
* Warn-don't-block: most of BMAD works without Python, so the install always * Warn-don't-block: most of BMAD works without Python, so the install always
* may proceed but the user must explicitly acknowledge the warning so it * may proceed but the user must explicitly acknowledge the warning so it
* can't scroll past unseen. In non-interactive mode (--yes) the warning is * can't scroll past unseen. In non-interactive runs (--yes, or stdin is not
* logged and the install continues without a prompt. * a TTY) the warning is logged and the install continues without a prompt.
* *
* @param {Object} [options] * @param {Object} [options]
* @param {boolean} [options.nonInteractive=false] - Skip the ack prompt (--yes mode) * @param {boolean} [options.nonInteractive=false] - Skip the ack prompt (--yes, or no TTY)
* @returns {Promise<{status: string, detected: Object|null}>} * @returns {Promise<{status: string, detected: Object|null}>}
*/ */
async function checkPythonEnvironment({ nonInteractive = false } = {}) { async function checkPythonEnvironment({ nonInteractive = false } = {}) {
const detected = detectPython(); // Called via module.exports so tests can stub detection.
const detected = module.exports.detectPython();
const status = classifyPython(detected ? detected.version : null); const status = classifyPython(detected ? detected.version : null);
if (status === 'full') { if (status === 'full' && detected.isRuntimeCommand) {
await prompts.log.success(`Python ${detected.version.raw} detected (${detected.command}) — all BMAD features supported.`); await prompts.log.success(`Python ${detected.version.raw} detected (${detected.command}) — all BMAD features supported.`);
return { status, detected }; return { status, detected };
} }
if (status === 'partial') { if (detected && !detected.isRuntimeCommand) {
await prompts.log.warn(
`Python ${detected.version.raw} found via \`${detected.command}\`, but BMAD scripts invoke \`python3\`, which is not on PATH.\n` +
`Python-powered features (memlog session memory, TOML config resolution) won't run until \`python3\` resolves —\n` +
`add a python3 alias/shim, or reinstall Python with the python3 launcher enabled.`,
);
} else if (status === 'partial') {
await prompts.log.warn( await prompts.log.warn(
`Python ${detected.version.raw} detected (${detected.command}) — BMAD's TOML config tools need Python 3.11+ (stdlib tomllib).\n` + `Python ${detected.version.raw} detected (${detected.command}) — BMAD's TOML config tools need Python 3.11+ (stdlib tomllib).\n` +
`Works: memlog session memory. Won't work: config/customization resolution scripts.`, `Works: memlog session memory. Won't work: config/customization resolution scripts.`,
@ -130,29 +160,29 @@ async function checkPythonEnvironment({ nonInteractive = false } = {}) {
await prompts.note(upgradeHints(), 'Python 3.11+ recommended'); await prompts.note(upgradeHints(), 'Python 3.11+ recommended');
if (nonInteractive) { if (nonInteractive) {
await prompts.log.info('Continuing without Python 3.11+ (--yes mode). You can install Python later — no reinstall needed.'); await prompts.log.info('Continuing anyway (non-interactive run). You can fix Python later — no reinstall needed.');
return { status, detected }; return { status, detected };
} }
const choice = await prompts.select({ const choice = await prompts.select({
message: 'Python 3.11+ was not found. How do you want to proceed?', message: "BMAD's Python-powered features won't work yet. How do you want to proceed?",
choices: [ choices: [
{ {
name: 'Continue install', name: 'Continue install',
value: 'continue', value: 'continue',
hint: 'BMAD works without Python — you can add Python 3.11+ later, no reinstall needed', hint: 'BMAD works without Python — you can fix Python later, no reinstall needed',
}, },
{ {
name: 'Quit and fix Python first', name: 'Quit and fix Python first',
value: 'quit', value: 'quit',
hint: 'install Python 3.11+, then re-run the installer', hint: 'make Python 3.11+ available as python3, then re-run the installer',
}, },
], ],
default: 'continue', default: 'continue',
}); });
if (choice === 'quit') { if (choice === 'quit') {
await prompts.cancel('Install Python 3.11+ (see hints above), then re-run the installer.'); await prompts.cancel('Make Python 3.11+ available as `python3` (see hints above), then re-run the installer.');
process.exit(0); process.exit(0);
} }

View File

@ -166,8 +166,10 @@ class UI {
// runtime. Warn-don't-block, but require an explicit ack so the warning // runtime. Warn-don't-block, but require an explicit ack so the warning
// can't scroll past unseen. The installer runs in the destination // can't scroll past unseen. The installer runs in the destination
// environment, so probing PATH here tests the right machine. // environment, so probing PATH here tests the right machine.
// Skip the ack when stdin isn't a TTY (CI/Docker/piped): clack's select
// on closed stdin resolves to cancel, which would silently exit 0.
const { checkPythonEnvironment } = require('./core/python-check'); const { checkPythonEnvironment } = require('./core/python-check');
await checkPythonEnvironment({ nonInteractive: !!options.yes }); await checkPythonEnvironment({ nonInteractive: !!options.yes || !process.stdin.isTTY });
// Parse channel flags (--channel/--all-*/--next=/--pin) once. Warnings // Parse channel flags (--channel/--all-*/--next=/--pin) once. Warnings
// are surfaced immediately so the user sees them before any git ops run. // are surfaced immediately so the user sees them before any git ops run.