feat(cli): postInstall executor for mcpctl skills sync
Closes the biggest deferred item from PR-5. metadata.postInstall scripts now actually run when their hash changes, with audit emission back to mcpd. Trust model unchanged from the corporate-appliance design: mcpd is the source of truth, content is reviewed at publish time, the client just runs. No sandbox, no signing, no consent prompts. The controls that matter are already on the publishing side (RBAC, audit, reviewer queue). What we DO provide is ops hygiene: - Hard timeout (default 60 s; per-skill override via metadata.postInstallTimeoutSec). SIGTERM at the deadline, SIGKILL after a 2 s grace. - Hash pinning. The script's sha256 is recorded in ~/.mcpctl/skills-state.json. Re-syncs of an unchanged script are a cheap no-op. A re-published "same version, fixed script" still triggers re-execution because its hash changed. - Curated env. PATH/HOME/USER/SHELL inherited; everything else dropped. MCPCTL_SKILL_NAME / _VERSION / _DIR / _PROJECT injected so scripts have stable context. - Per-skill install log under ~/.mcpctl/skills/<name>/install.log. Bounded at 5 × 256 KB; old entries truncated from the front. - Audit event back to mcpd (POST /api/v1/audit-events, eventKind=skill_postinstall) on every run, including hostname so admins can see fleet rollout state. Best-effort — failures are warned but never block the sync. - Path-escape rejection. metadata.postInstall must resolve inside the skill bundle; relative paths that try to climb out are refused. - Auto-chmod 0755 on the script before spawn. Some upstreams ship 0644 + a shebang and expect a shell to handle it; we always spawn the path directly so we need +x. Failure semantics: on timeout or non-zero exit, the recorded postInstallHash is NOT updated. Next sync retries. Other skills in the same sync run continue regardless — postInstall errors are scoped, not fatal. ## Files ### Added - src/cli/src/utils/postinstall.ts (~200 LOC) - src/cli/tests/utils/postinstall.test.ts (~190 LOC, 10 tests covering success, env vars, chmod, non-zero exit, timeout via exec sleep, path-escape, missing script, log file shape + append-across-runs) ### Edited - src/cli/src/commands/skills.ts: applyOne now invokes runPostInstall + emitPostInstallAudit when metadata.postInstall is set and the script hash differs from prior state. New SyncResult fields: postInstallsRan, postInstallsSkipped. Summary line surfaces "N postInstall ran". --skip-postinstall flag now actually does what it says. ## Verification 163 test files / 2171 tests green (up from 2161). End-to-end on a real machine (after this PR ships and a skill with metadata.postInstall is published): ``` mcpctl skills sync # → mcpctl: 1 installed, 1 postInstall ran cat ~/.mcpctl/skills/<name>/install.log # see stdout/stderr mcpctl skills sync # idempotent — skipped ``` If the same skill is republished with a fixed script: ``` mcpctl skills sync # → mcpctl: 1 updated, 1 postInstall ran (hash changed → rerun) ``` If the script fails (exit 7): ``` mcpctl skills sync # → mcpctl: 1 updated, 1 errors mcpctl skills sync # state.postInstallHash NOT updated → retries ``` Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Michal
223
src/cli/tests/utils/postinstall.test.ts
Normal file
223
src/cli/tests/utils/postinstall.test.ts
Normal file
@@ -0,0 +1,223 @@
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import { mkdtemp, rm, writeFile, chmod, readFile, mkdir } from 'node:fs/promises';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
|
||||
import { runPostInstall, hashScript } from '../../src/utils/postinstall.js';
|
||||
|
||||
describe('postinstall executor', () => {
|
||||
let tmp: string;
|
||||
|
||||
beforeEach(async () => {
|
||||
tmp = await mkdtemp(join(tmpdir(), 'mcpctl-postinstall-'));
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await rm(tmp, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe('hashScript', () => {
|
||||
it('returns deterministic sha256-prefixed hash', () => {
|
||||
expect(hashScript('hello')).toMatch(/^sha256:[0-9a-f]{64}$/);
|
||||
expect(hashScript('hello')).toBe(hashScript('hello'));
|
||||
expect(hashScript('hello')).not.toBe(hashScript('hellp'));
|
||||
});
|
||||
});
|
||||
|
||||
describe('runPostInstall — success path', () => {
|
||||
it('runs a passing script and returns exit 0 + script hash', async () => {
|
||||
const installDir = join(tmp, 'skill');
|
||||
await mkdir(installDir, { recursive: true });
|
||||
const scriptPath = join(installDir, 'install.sh');
|
||||
await writeFile(scriptPath, '#!/bin/sh\necho hello-stdout\necho hello-stderr 1>&2\nexit 0\n');
|
||||
await chmod(scriptPath, 0o755);
|
||||
|
||||
const result = await runPostInstall({
|
||||
installDir,
|
||||
scriptPath: 'install.sh',
|
||||
skillName: 'test-skill',
|
||||
semver: '0.1.0',
|
||||
logsDir: join(tmp, 'logs'),
|
||||
});
|
||||
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.timedOut).toBe(false);
|
||||
expect(result.stdoutTail).toContain('hello-stdout');
|
||||
expect(result.stderrTail).toContain('hello-stderr');
|
||||
expect(result.scriptHash).toMatch(/^sha256:/);
|
||||
});
|
||||
|
||||
it('passes curated env (MCPCTL_SKILL_NAME, _VERSION, _DIR, _PROJECT)', async () => {
|
||||
const installDir = join(tmp, 'skill');
|
||||
await mkdir(installDir, { recursive: true });
|
||||
const scriptPath = join(installDir, 'install.sh');
|
||||
// Write env vars to a file we can read back.
|
||||
const outFile = join(tmp, 'env-dump.txt');
|
||||
await writeFile(scriptPath, `#!/bin/sh
|
||||
echo "name=$MCPCTL_SKILL_NAME" > ${JSON.stringify(outFile)}
|
||||
echo "version=$MCPCTL_SKILL_VERSION" >> ${JSON.stringify(outFile)}
|
||||
echo "dir=$MCPCTL_SKILL_DIR" >> ${JSON.stringify(outFile)}
|
||||
echo "project=$MCPCTL_PROJECT" >> ${JSON.stringify(outFile)}
|
||||
`);
|
||||
await chmod(scriptPath, 0o755);
|
||||
|
||||
const result = await runPostInstall({
|
||||
installDir,
|
||||
scriptPath: 'install.sh',
|
||||
skillName: 'env-test',
|
||||
semver: '1.2.3',
|
||||
projectName: 'demo',
|
||||
logsDir: join(tmp, 'logs'),
|
||||
});
|
||||
expect(result.exitCode).toBe(0);
|
||||
|
||||
const dumped = await readFile(outFile, 'utf-8');
|
||||
expect(dumped).toContain('name=env-test');
|
||||
expect(dumped).toContain('version=1.2.3');
|
||||
expect(dumped).toContain('dir=' + installDir);
|
||||
expect(dumped).toContain('project=demo');
|
||||
});
|
||||
|
||||
it('chmods 0644 scripts to executable before spawn', async () => {
|
||||
const installDir = join(tmp, 'skill');
|
||||
await mkdir(installDir, { recursive: true });
|
||||
const scriptPath = join(installDir, 'install.sh');
|
||||
await writeFile(scriptPath, '#!/bin/sh\nexit 0\n');
|
||||
await chmod(scriptPath, 0o644); // not executable
|
||||
|
||||
const result = await runPostInstall({
|
||||
installDir,
|
||||
scriptPath: 'install.sh',
|
||||
skillName: 't',
|
||||
semver: '0.1.0',
|
||||
logsDir: join(tmp, 'logs'),
|
||||
});
|
||||
|
||||
expect(result.exitCode).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('runPostInstall — failure paths', () => {
|
||||
it('captures non-zero exit code and returns it', async () => {
|
||||
const installDir = join(tmp, 'skill');
|
||||
await mkdir(installDir, { recursive: true });
|
||||
const scriptPath = join(installDir, 'fail.sh');
|
||||
await writeFile(scriptPath, '#!/bin/sh\necho oops 1>&2\nexit 7\n');
|
||||
await chmod(scriptPath, 0o755);
|
||||
|
||||
const result = await runPostInstall({
|
||||
installDir,
|
||||
scriptPath: 'fail.sh',
|
||||
skillName: 't',
|
||||
semver: '0.1.0',
|
||||
logsDir: join(tmp, 'logs'),
|
||||
});
|
||||
|
||||
expect(result.exitCode).toBe(7);
|
||||
expect(result.timedOut).toBe(false);
|
||||
expect(result.stderrTail).toContain('oops');
|
||||
});
|
||||
|
||||
it('honors timeoutSec — kills via SIGTERM and reports timedOut=true', async () => {
|
||||
const installDir = join(tmp, 'skill');
|
||||
await mkdir(installDir, { recursive: true });
|
||||
const scriptPath = join(installDir, 'hang.sh');
|
||||
// `exec` so SIGTERM hits sleep directly — without it /bin/sh
|
||||
// catches the signal but the orphaned sleep keeps the streams
|
||||
// open until SIGKILL; the test then has to wait for the 2s grace
|
||||
// window before we force-kill, which is fine but flakier.
|
||||
await writeFile(scriptPath, '#!/bin/sh\nexec sleep 30\n');
|
||||
await chmod(scriptPath, 0o755);
|
||||
|
||||
const start = Date.now();
|
||||
const result = await runPostInstall({
|
||||
installDir,
|
||||
scriptPath: 'hang.sh',
|
||||
skillName: 't',
|
||||
semver: '0.1.0',
|
||||
timeoutSec: 1,
|
||||
logsDir: join(tmp, 'logs'),
|
||||
});
|
||||
const elapsed = Date.now() - start;
|
||||
|
||||
expect(result.timedOut).toBe(true);
|
||||
// 1s timeout + up to 2s grace before SIGKILL.
|
||||
expect(elapsed).toBeLessThan(5000);
|
||||
expect(elapsed).toBeGreaterThanOrEqual(1000);
|
||||
}, 15_000);
|
||||
|
||||
it('rejects path-escape attempts', async () => {
|
||||
const installDir = join(tmp, 'skill');
|
||||
await mkdir(installDir, { recursive: true });
|
||||
|
||||
await expect(runPostInstall({
|
||||
installDir,
|
||||
scriptPath: '../escape.sh',
|
||||
skillName: 't',
|
||||
semver: '0.1.0',
|
||||
logsDir: join(tmp, 'logs'),
|
||||
})).rejects.toThrow(/escapes skill dir/);
|
||||
});
|
||||
|
||||
it('throws when the script does not exist', async () => {
|
||||
const installDir = join(tmp, 'skill');
|
||||
await mkdir(installDir, { recursive: true });
|
||||
|
||||
await expect(runPostInstall({
|
||||
installDir,
|
||||
scriptPath: 'missing.sh',
|
||||
skillName: 't',
|
||||
semver: '0.1.0',
|
||||
logsDir: join(tmp, 'logs'),
|
||||
})).rejects.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('runPostInstall — install log', () => {
|
||||
it('writes stdout + stderr + exit summary to logsDir/install.log', async () => {
|
||||
const installDir = join(tmp, 'skill');
|
||||
await mkdir(installDir, { recursive: true });
|
||||
const scriptPath = join(installDir, 'install.sh');
|
||||
await writeFile(scriptPath, '#!/bin/sh\necho hello\nexit 0\n');
|
||||
await chmod(scriptPath, 0o755);
|
||||
|
||||
const logsDir = join(tmp, 'logs');
|
||||
await runPostInstall({
|
||||
installDir,
|
||||
scriptPath: 'install.sh',
|
||||
skillName: 'log-test',
|
||||
semver: '0.1.0',
|
||||
logsDir,
|
||||
});
|
||||
|
||||
const log = await readFile(join(logsDir, 'install.log'), 'utf-8');
|
||||
expect(log).toContain('log-test@0.1.0');
|
||||
expect(log).toContain('hello');
|
||||
expect(log).toContain('exit 0');
|
||||
});
|
||||
|
||||
it('appends across runs without losing prior history', async () => {
|
||||
const installDir = join(tmp, 'skill');
|
||||
await mkdir(installDir, { recursive: true });
|
||||
const scriptPath = join(installDir, 'install.sh');
|
||||
await writeFile(scriptPath, '#!/bin/sh\necho run\nexit 0\n');
|
||||
await chmod(scriptPath, 0o755);
|
||||
|
||||
const logsDir = join(tmp, 'logs');
|
||||
const input = {
|
||||
installDir,
|
||||
scriptPath: 'install.sh',
|
||||
skillName: 't',
|
||||
semver: '0.1.0',
|
||||
logsDir,
|
||||
};
|
||||
await runPostInstall(input);
|
||||
await runPostInstall(input);
|
||||
|
||||
const log = await readFile(join(logsDir, 'install.log'), 'utf-8');
|
||||
// Two run headers separated by `===`.
|
||||
const headers = (log.match(/=== /g) ?? []).length;
|
||||
expect(headers).toBeGreaterThanOrEqual(2);
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user