Closes DOCS-AUDIT §4.2 (point 3, the last one open) and §1.3.
`executeCall` joined `DeployCall.command` and ran it through
`execa({shell: true})`, which made every value on that command line shell
syntax. The finding framed it as a quoting problem "in the password"; it was
wider than that. `--data` values were interpolated inside double quotes, so a
backtick or a `$(…)` in *any* variable value was command substitution and a `;`
ended the command and began another.
`buildDeployCall` now emits a true argv — one element per argument, nothing
pre-quoted — and the executor spawns `execa(command[0], command.slice(1))` with
no `shell` option at all. pyinfra is still found on PATH and stdio stays
inherited, so live output is unchanged.
`maskCommand()` walks the argv by position instead of pattern-matching a joined
string, which closes a leak of its own: it used to bound a secret's value on the
closing `"` the builder had written two modules away, so a value containing a
`"` leaked its own tail. It is now the only thing that turns the command back
into a string, for display, and it shell-quotes as it goes so `--print-only`
output stays pasteable.
Also in `buildDeployCall`: `logConfigToFlags()` finally has a caller (§1.3). It
was exported and unit-tested with nothing consuming it, so `log.verbosity` and
`log.debug` in `.nopyrc.json` did nothing at all. The flags are prefixed onto
the argv right after `-y`. Consequence worth knowing rather than discovering:
`packages/nopy/.nopyrc.json` has always asked for `"verbosity": "trace",
"debug": true`, so a run from that directory now really does get `-vvv --debug`.
The tests move with it — the mock is `execa(file, args, opts)` with no factory
to unwrap, and the new cases are the ones that would have caught this: an argv
element holding `$(id); rm -rf /` stays one element, a secret whose value
contains a quote is masked whole, and `execa` is asserted never to be asked for
a shell.
What remains is not fixable here: the value still reaches pyinfra on its command
line, so it is visible in `ps`. That is pyinfra's `--data` interface.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DCzYTAm9QUhvLNr2EpdagJ
262 lines
7.7 KiB
TypeScript
262 lines
7.7 KiB
TypeScript
/**
|
|
* Tests for nopy.executor module
|
|
*/
|
|
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
|
import {
|
|
type DeployCall,
|
|
type ExecutionResult,
|
|
maskCommand,
|
|
outputExecutionPlan,
|
|
summarizeResults,
|
|
} from '../src/nopy.executor.js';
|
|
|
|
/**
|
|
* Helper to create a test deploy call
|
|
*/
|
|
function createTestCall(cube: string, host: string, deps: string[] = []): DeployCall {
|
|
return {
|
|
cube,
|
|
host,
|
|
cwd: `/test/${cube}`,
|
|
command: ['pyinfra', host, '-y', `${cube}.deploy.py`],
|
|
env: { VAR: 'value' },
|
|
dependencies: deps,
|
|
};
|
|
}
|
|
|
|
/**
|
|
* Helper to create a test execution result
|
|
*/
|
|
function createTestResult(
|
|
cube: string,
|
|
host: string,
|
|
success: boolean,
|
|
duration = 1000
|
|
): ExecutionResult {
|
|
return {
|
|
cube,
|
|
host,
|
|
success,
|
|
duration,
|
|
...(success ? {} : { error: new Error('Test error') }),
|
|
};
|
|
}
|
|
|
|
describe('summarizeResults', () => {
|
|
it('summarizes successful results', () => {
|
|
const results: ExecutionResult[] = [
|
|
createTestResult('cube-a', 'host1', true, 1000),
|
|
createTestResult('cube-b', 'host1', true, 2000),
|
|
];
|
|
|
|
const summary = summarizeResults(results);
|
|
|
|
expect(summary.total).toBe(2);
|
|
expect(summary.successful).toBe(2);
|
|
expect(summary.failed).toBe(0);
|
|
expect(summary.totalDuration).toBe(3000);
|
|
expect(summary.failures).toHaveLength(0);
|
|
});
|
|
|
|
it('summarizes failed results', () => {
|
|
const results: ExecutionResult[] = [
|
|
createTestResult('cube-a', 'host1', true, 1000),
|
|
createTestResult('cube-b', 'host1', false, 500),
|
|
];
|
|
|
|
const summary = summarizeResults(results);
|
|
|
|
expect(summary.total).toBe(2);
|
|
expect(summary.successful).toBe(1);
|
|
expect(summary.failed).toBe(1);
|
|
expect(summary.totalDuration).toBe(1500);
|
|
expect(summary.failures).toHaveLength(1);
|
|
expect(summary.failures[0].cube).toBe('cube-b');
|
|
});
|
|
|
|
it('handles empty results', () => {
|
|
const summary = summarizeResults([]);
|
|
|
|
expect(summary.total).toBe(0);
|
|
expect(summary.successful).toBe(0);
|
|
expect(summary.failed).toBe(0);
|
|
expect(summary.totalDuration).toBe(0);
|
|
});
|
|
|
|
it('handles all failed results', () => {
|
|
const results: ExecutionResult[] = [
|
|
createTestResult('cube-a', 'host1', false, 100),
|
|
createTestResult('cube-b', 'host1', false, 200),
|
|
];
|
|
|
|
const summary = summarizeResults(results);
|
|
|
|
expect(summary.successful).toBe(0);
|
|
expect(summary.failed).toBe(2);
|
|
expect(summary.failures).toHaveLength(2);
|
|
});
|
|
});
|
|
|
|
describe('outputExecutionPlan', () => {
|
|
let consoleLogSpy: ReturnType<typeof vi.spyOn>;
|
|
|
|
beforeEach(() => {
|
|
consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
|
|
});
|
|
|
|
// vitest reuses an existing spy rather than re-wrapping, so recorded calls
|
|
// would otherwise leak from one test into the next.
|
|
afterEach(() => {
|
|
vi.restoreAllMocks();
|
|
});
|
|
|
|
it('outputs text format by default', () => {
|
|
const calls = [createTestCall('cube-a', 'host1')];
|
|
|
|
outputExecutionPlan(calls);
|
|
|
|
expect(consoleLogSpy).toHaveBeenCalled();
|
|
const output = consoleLogSpy.mock.calls.map((c) => c[0]).join('\n');
|
|
expect(output).toContain('Execution Plan');
|
|
expect(output).toContain('cube-a');
|
|
expect(output).toContain('host1');
|
|
});
|
|
|
|
it('masks variables the manifest declared secret', () => {
|
|
const call: DeployCall = {
|
|
...createTestCall('cube-a', 'host1'),
|
|
command: ['pyinfra', 'host1', '-y', '--data', 'PASSWORD=hunter2', '--data', 'OTHER=visible'],
|
|
env: { PASSWORD: 'hunter2', OTHER: 'visible' },
|
|
secrets: ['PASSWORD'],
|
|
};
|
|
|
|
outputExecutionPlan([call]);
|
|
|
|
const output = consoleLogSpy.mock.calls.map((c) => c[0]).join('\n');
|
|
expect(output).toContain('********');
|
|
// Both the variable list and the command line above it — the command used
|
|
// to be printed unmasked, which defeated the masking entirely.
|
|
expect(output).not.toContain('hunter2');
|
|
expect(output).toContain('visible');
|
|
});
|
|
|
|
it('leaves a password-looking variable alone when the manifest says nothing', () => {
|
|
const call: DeployCall = {
|
|
...createTestCall('cube-a', 'host1'),
|
|
env: { PASSWORD: 'visible' },
|
|
};
|
|
|
|
outputExecutionPlan([call]);
|
|
|
|
const output = consoleLogSpy.mock.calls.map((c) => c[0]).join('\n');
|
|
expect(output).toContain('visible');
|
|
});
|
|
|
|
it('shows step numbers', () => {
|
|
const calls = [createTestCall('cube-a', 'host1'), createTestCall('cube-b', 'host1')];
|
|
|
|
outputExecutionPlan(calls);
|
|
|
|
const output = consoleLogSpy.mock.calls.map((c) => c[0]).join('\n');
|
|
expect(output).toContain('Step 1');
|
|
expect(output).toContain('Step 2');
|
|
});
|
|
|
|
it('shows total count', () => {
|
|
const calls = [
|
|
createTestCall('cube-a', 'host1'),
|
|
createTestCall('cube-b', 'host1'),
|
|
createTestCall('cube-c', 'host1'),
|
|
];
|
|
|
|
outputExecutionPlan(calls);
|
|
|
|
const output = consoleLogSpy.mock.calls.map((c) => c[0]).join('\n');
|
|
expect(output).toContain('Total: 3');
|
|
});
|
|
});
|
|
|
|
describe('maskCommand', () => {
|
|
const call = (command: string[], secrets?: string[]): DeployCall => ({
|
|
...createTestCall('cube-a', 'host1'),
|
|
command,
|
|
secrets,
|
|
});
|
|
|
|
it('replaces the value of a declared secret', () => {
|
|
const masked = maskCommand(
|
|
call(['pyinfra', 'host1', '--data', 'PASSWORD=hunter2'], ['PASSWORD'])
|
|
);
|
|
|
|
expect(masked).toBe('pyinfra host1 --data PASSWORD=********');
|
|
});
|
|
|
|
it('leaves other data alone', () => {
|
|
const masked = maskCommand(
|
|
call(['--data', 'SSID=home', '--data', 'PASSWORD=hunter2'], ['PASSWORD'])
|
|
);
|
|
|
|
expect(masked).toBe('--data SSID=home --data PASSWORD=********');
|
|
});
|
|
|
|
it('masks a value containing spaces', () => {
|
|
const masked = maskCommand(
|
|
call(['--data', 'PASSWORD=two words', '--chdir', '/x'], ['PASSWORD'])
|
|
);
|
|
|
|
expect(masked).toBe('--data PASSWORD=******** --chdir /x');
|
|
});
|
|
|
|
it('masks an empty secret value', () => {
|
|
expect(maskCommand(call(['--data', 'PASSWORD='], ['PASSWORD']))).toBe(
|
|
'--data PASSWORD=********'
|
|
);
|
|
});
|
|
|
|
it('masks a secret whose value contains a quote', () => {
|
|
// The old implementation bounded the value on the closing `"` the builder
|
|
// had written, so a value containing one leaked the rest of itself.
|
|
const masked = maskCommand(call(['--data', 'PASSWORD=he said "hi"'], ['PASSWORD']));
|
|
|
|
expect(masked).toBe('--data PASSWORD=********');
|
|
expect(masked).not.toContain('hi');
|
|
});
|
|
|
|
it('masks the ssh password whether or not the cube declares secrets', () => {
|
|
const masked = maskCommand(
|
|
call(['pyinfra', 'host1', '--user', 'bob', '--password', 's3cr3t', '-y'])
|
|
);
|
|
|
|
expect(masked).toBe('pyinfra host1 --user bob --password ******** -y');
|
|
});
|
|
|
|
it('returns the command untouched when there is nothing to hide', () => {
|
|
expect(maskCommand(call(['pyinfra', 'host1', '-y']))).toBe('pyinfra host1 -y');
|
|
});
|
|
|
|
it('quotes an argument a shell would otherwise re-parse', () => {
|
|
// Display only — nothing runs through a shell — but `--print-only` output is
|
|
// meant to be pasteable, so it has to survive the round trip.
|
|
const masked = maskCommand(call(['--data', 'MOTD=$(id); rm -rf /']));
|
|
|
|
expect(masked).toBe(`--data 'MOTD=$(id); rm -rf /'`);
|
|
});
|
|
|
|
it('escapes an embedded single quote', () => {
|
|
expect(maskCommand(call(['--data', "NAME=o'brien"]))).toBe(`--data 'NAME=o'\\''brien'`);
|
|
});
|
|
|
|
it('quotes an empty argument rather than dropping it', () => {
|
|
expect(maskCommand(call(['pyinfra', '']))).toBe("pyinfra ''");
|
|
});
|
|
|
|
it('leaves a trailing --password with no value alone', () => {
|
|
expect(maskCommand(call(['pyinfra', '--password']))).toBe('pyinfra --password');
|
|
});
|
|
|
|
it('handles a --data argument with no equals sign', () => {
|
|
expect(maskCommand(call(['--data', 'BARE'], ['BARE']))).toBe('--data BARE=********');
|
|
});
|
|
});
|