diff --git a/docs/plans/state.json b/docs/plans/state.json index 40e3e99..bbff0a2 100644 --- a/docs/plans/state.json +++ b/docs/plans/state.json @@ -1503,6 +1503,80 @@ "src/cli/skills.test.ts" ], "test_status": "pnpm typecheck + pnpm test:run src/cli/skills.test.ts + pnpm test:run + pnpm lint (warnings only, 0 errors) + pnpm build passing" + }, + "installer_execution_global_policy_gate": { + "status": "completed", + "description": "Added a global skills execution policy gate (`skills.installation_execution`) with default disabled behavior, wiring install/execute CLI flows to return deterministic `execution_policy_disabled` receipts unless config explicitly enables execution", + "files_modified": [ + "src/config/schema.ts", + "src/config/schema.test.ts", + "src/cli/skills.ts", + "src/cli/skills.test.ts", + "config/default.yaml" + ], + "test_status": "pnpm typecheck + pnpm test:run src/config/schema.test.ts src/cli/skills.test.ts + pnpm test:run + pnpm lint (warnings only, 0 errors) + pnpm build passing" + }, + "installer_execution_policy_help_text": { + "status": "completed", + "description": "Improved user-facing execution guidance by documenting config gate requirements in CLI option help text and adding explicit render output when policy blocks installer execution", + "files_modified": [ + "src/cli/skills.ts", + "src/cli/skills.test.ts" + ], + "test_status": "pnpm typecheck + pnpm test:run src/cli/skills.test.ts + pnpm test:run + pnpm lint (warnings only, 0 errors) + pnpm build passing" + }, + "shell_runner_rollout_controls": { + "status": "completed", + "description": "Added explicit config-gated rollout control for shell runner usage (`skills.allow_shell_runner` default false), with deterministic CLI guard errors for install/execute when `--runner shell` is requested without policy enablement", + "files_modified": [ + "src/config/schema.ts", + "src/config/schema.test.ts", + "src/cli/skills.ts", + "src/cli/skills.test.ts", + "config/default.yaml" + ], + "test_status": "pnpm typecheck + pnpm test:run src/config/schema.test.ts src/cli/skills.test.ts + pnpm test:run + pnpm lint (warnings only, 0 errors) + pnpm build passing" + }, + "shell_runner_command_allowlist": { + "status": "completed", + "description": "Added command-level shell runner allowlisting via `skills.shell_runner_allowlist` patterns, wiring install/execute shell runs to block non-matching commands with deterministic `allowlist_blocked` receipt reasons", + "files_modified": [ + "src/config/schema.ts", + "src/config/schema.test.ts", + "src/cli/skills.ts", + "src/cli/skills.test.ts", + "config/default.yaml" + ], + "test_status": "pnpm typecheck + pnpm test:run src/config/schema.test.ts src/cli/skills.test.ts + pnpm test:run + pnpm lint (warnings only, 0 errors) + pnpm build passing" + }, + "shell_runner_audit_telemetry": { + "status": "completed", + "description": "Added deterministic shell-runner audit telemetry hooks using existing audit tool events, emitting policy denials and per-command outcomes for install/execute shell flows via `emitShellRunnerAuditEvents(...)`", + "files_modified": [ + "src/cli/skills.ts", + "src/cli/skills.test.ts" + ], + "test_status": "pnpm typecheck + pnpm test:run src/config/schema.test.ts src/cli/skills.test.ts + pnpm test:run + pnpm lint (warnings only, 0 errors) + pnpm build passing" + }, + "skills_installer_dedicated_audit_events": { + "status": "completed", + "description": "Added dedicated audit event types (`skills.installer.execution_blocked`, `skills.installer.command_result`) and migrated shell-runner telemetry emission from generic tool events to richer skills installer event contracts", + "files_modified": [ + "src/audit/types.ts", + "src/audit/logger.ts", + "src/cli/skills.ts", + "src/cli/skills.test.ts" + ], + "test_status": "pnpm typecheck + pnpm test:run src/cli/skills.test.ts + pnpm test:run + pnpm lint (warnings only, 0 errors) + pnpm build passing" + }, + "skills_installer_audit_command_redaction_hashing": { + "status": "completed", + "description": "Hardened skills installer audit payload safety by hashing command strings and sanitizing potentially sensitive spawn-error reason details before emitting shell-runner command-result telemetry", + "files_modified": [ + "src/cli/skills.ts", + "src/cli/skills.test.ts" + ], + "test_status": "pnpm typecheck + pnpm test:run src/cli/skills.test.ts + pnpm test:run + pnpm lint + pnpm build passing" } } } @@ -1531,7 +1605,7 @@ }, "overall_progress": { - "total_test_count": 1560, + "total_test_count": 1575, "all_tests_passing": true, "p0_completion": "3/3 (100%)", "p1_completion": "4/4 (100%)", @@ -1551,7 +1625,7 @@ "gmail_auth_cli": "flynn gmail-auth command implemented with OAuth2 flow, doctor check, config routed to Telegram", "native_audio_support": "completed — smart routing for native audio (Gemini/OpenAI/GitHub) vs Whisper transcription fallback", "remaining_phases_completion": "Phase 1: 3/3 (100%) — context levels, command registry, memory structure. Phase 2: 2/2 (100%) — component registry, confidence routing. Phase 3: 2/2 (100%) — adaptive memory/compaction, truthfulness/autonomy hardening", - "next_up": "Skills infrastructure follow-up: define policy/autonomy model for safely enabling real installer execution beyond the current execution-disabled default" + "next_up": "Skills infrastructure follow-up: define phased enablement criteria for shell runner (allowlist governance, telemetry review, and rollout guardrails) now that audit command strings are hashed" }, "soul_md_and_cron_create": { "date": "2026-02-11", diff --git a/src/cli/skills.test.ts b/src/cli/skills.test.ts index c890d13..6d53d61 100644 --- a/src/cli/skills.test.ts +++ b/src/cli/skills.test.ts @@ -25,6 +25,10 @@ import { runInstallerCommandsWithPolicy, noOpSkillInstallerCommandRunner, createShellSkillInstallerCommandRunner, + checkCommandAgainstAllowlist, + emitShellRunnerAuditEvents, + hashSkillInstallerAuditCommand, + sanitizeSkillInstallerAuditReason, resolveSkillInstallerCommandRunner, runSkillExecuteAction, runSkillInstallAction, @@ -48,7 +52,18 @@ function buildSkill(overrides: Partial): Skill { }; } -function writeSkillsCliConfig(configPath: string, opts: { managedDir: string; bundledDir: string; workspaceDir: string }): void { +function writeSkillsCliConfig( + configPath: string, + opts: { + managedDir: string; + bundledDir: string; + workspaceDir: string; + installationExecution?: 'disabled' | 'enabled'; + allowShellRunner?: boolean; + shellRunnerAllowlist?: string[]; + }, +): void { + const allowlist = opts.shellRunnerAllowlist ?? []; writeFileSync( configPath, [ @@ -60,6 +75,9 @@ function writeSkillsCliConfig(configPath: string, opts: { managedDir: string; bu ` managed_dir: ${opts.managedDir}`, ` bundled_dir: ${opts.bundledDir}`, ` workspace_dir: ${opts.workspaceDir}`, + ` installation_execution: ${opts.installationExecution ?? 'disabled'}`, + ` allow_shell_runner: ${opts.allowShellRunner ?? false}`, + ` shell_runner_allowlist: [${allowlist.map((item) => `'${item}'`).join(', ')}]`, ].join('\n'), 'utf-8', ); @@ -319,6 +337,25 @@ describe('skills CLI helpers', () => { expect(output).toContain('[brew] succeeded brew install jq (runner_reported_success)'); }); + it('renders policy guidance when execution is blocked by config policy', () => { + const output = renderSkillInstallerExecutionStub({ + skill: { name: 'exec-stub', tier: 'bundled', version: '1.0.0' }, + execution: 'stub', + mode: 'install', + confirmed: true, + execution_enabled: false, + executed: [], + reason: 'execution_policy_disabled', + attempted: [{ installer_type: 'brew', command: 'brew install jq' }], + results: [{ installer_type: 'brew', command: 'brew install jq', status: 'skipped', reason: 'execution_policy_disabled' }], + wouldRun: ['brew install jq'], + skipped: [], + }); + + expect(output).toContain('Execution policy blocked installer commands.'); + expect(output).toContain('skills.installation_execution: enabled'); + }); + it('derives execution stub view from preflight data', () => { const preflight = { sourcePath: '/tmp/source-skill', @@ -396,6 +433,147 @@ describe('skills CLI helpers', () => { expect(policy.reason).toBe('execution_enabled'); }); + it('keeps execution disabled when config policy is disabled', () => { + const policy = evaluateInstallerExecutionPolicy({ + mode: 'install', + confirmed: true, + executionRequested: true, + configPolicyEnabled: false, + }); + + expect(policy.confirmed).toBe(true); + expect(policy.execution_enabled).toBe(false); + expect(policy.reason).toBe('execution_policy_disabled'); + }); + + it('matches shell command allowlist patterns with wildcard support', () => { + expect(checkCommandAgainstAllowlist('npm install -g zx', ['npm install*'])).toBe(true); + expect(checkCommandAgainstAllowlist('node -e "process.exit(0)"', ['node -e*'])).toBe(true); + expect(checkCommandAgainstAllowlist('rm -rf /tmp/demo', ['npm install*'])).toBe(false); + expect(checkCommandAgainstAllowlist('echo hi', [])).toBe(false); + }); + + it('emits audit denied events for allowlist-blocked shell commands', () => { + const logger = { + skillsInstallerExecutionBlocked: vi.fn(), + skillsInstallerCommandResult: vi.fn(), + }; + + emitShellRunnerAuditEvents({ + skillName: 'audit-skill', + phase: 'install', + executionRequested: true, + executionEnabled: true, + reason: 'execution_enabled', + results: [ + { + installer_type: 'download', + command: 'download https://example.com/pkg.tgz -> ', + status: 'failed', + reason: 'allowlist_blocked', + }, + ], + logger, + }); + + expect(logger.skillsInstallerCommandResult).toHaveBeenCalledWith( + expect.objectContaining({ + skill_name: 'audit-skill', + phase: 'install', + status: 'failed', + reason: 'allowlist_blocked', + }), + ); + expect(logger.skillsInstallerCommandResult).toHaveBeenCalledWith( + expect.objectContaining({ + command: hashSkillInstallerAuditCommand('download https://example.com/pkg.tgz -> '), + }), + ); + expect(logger.skillsInstallerExecutionBlocked).not.toHaveBeenCalled(); + }); + + it('hashes audit command values deterministically for non-sensitive commands', () => { + const command = 'download https://example.com/tool.tgz -> '; + expect(hashSkillInstallerAuditCommand(command)).toBe(hashSkillInstallerAuditCommand(command)); + expect(hashSkillInstallerAuditCommand(command)).toMatch(/^sha256:[a-f0-9]{64}$/); + }); + + it('sanitizes spawn_error reason values for audit events', () => { + expect(sanitizeSkillInstallerAuditReason('spawn_error:ENOENT some sensitive detail')).toBe('spawn_error'); + expect(sanitizeSkillInstallerAuditReason('allowlist_blocked')).toBe('allowlist_blocked'); + }); + + it('emits hashed command values for both successful and failed audit command results', () => { + const logger = { + skillsInstallerExecutionBlocked: vi.fn(), + skillsInstallerCommandResult: vi.fn(), + }; + + emitShellRunnerAuditEvents({ + skillName: 'audit-skill', + phase: 'execute', + executionRequested: true, + executionEnabled: true, + reason: 'execution_enabled', + results: [ + { + installer_type: 'node', + command: 'node -e "process.exit(0)"', + status: 'succeeded', + reason: 'runner_reported_success', + }, + { + installer_type: 'node', + command: 'node -e "process.exit(7)"', + status: 'failed', + reason: 'exit_code_7', + }, + ], + logger, + }); + + expect(logger.skillsInstallerCommandResult).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + command: hashSkillInstallerAuditCommand('node -e "process.exit(0)"'), + status: 'succeeded', + }), + ); + expect(logger.skillsInstallerCommandResult).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + command: hashSkillInstallerAuditCommand('node -e "process.exit(7)"'), + status: 'failed', + }), + ); + }); + + it('emits audit denied event when shell execution is policy-blocked', () => { + const logger = { + skillsInstallerExecutionBlocked: vi.fn(), + skillsInstallerCommandResult: vi.fn(), + }; + + emitShellRunnerAuditEvents({ + skillName: 'audit-skill', + phase: 'execute', + executionRequested: true, + executionEnabled: false, + reason: 'execution_policy_disabled', + results: [], + logger, + }); + + expect(logger.skillsInstallerExecutionBlocked).toHaveBeenCalledWith( + expect.objectContaining({ + skill_name: 'audit-skill', + phase: 'execute', + reason: 'execution_policy_disabled', + }), + ); + expect(logger.skillsInstallerCommandResult).not.toHaveBeenCalled(); + }); + it('resolves installer runner mode and validates invalid values', () => { const noop = resolveSkillInstallerCommandRunner(); expect('error' in noop).toBe(false); @@ -472,6 +650,20 @@ describe('skills CLI helpers', () => { ]); }); + it('shell command runner blocks commands outside allowlist', () => { + const runner = createShellSkillInstallerCommandRunner(['npm install*']); + + const results = runner.run(['node -e "process.exit(0)"']); + + expect(results).toEqual([ + { + command: 'node -e "process.exit(0)"', + status: 'failed', + reason: 'allowlist_blocked', + }, + ]); + }); + it('maps runner command results into structured per-step statuses', () => { const attempted = [ { installer_type: 'brew', command: 'brew install jq' }, @@ -958,7 +1150,7 @@ describe('skills CLI helpers', () => { mkdirSync(managedDir, { recursive: true }); mkdirSync(bundledDir, { recursive: true }); mkdirSync(workspaceDir, { recursive: true }); - writeSkillsCliConfig(configPath, { managedDir, bundledDir, workspaceDir }); + writeSkillsCliConfig(configPath, { managedDir, bundledDir, workspaceDir, installationExecution: 'enabled' }); const program = new Command(); registerSkillsCommand(program); @@ -976,6 +1168,94 @@ describe('skills CLI helpers', () => { rmSync(root, { recursive: true, force: true }); }); + it('skills install rejects shell runner when config disallows it', async () => { + const root = mkdtempSync(join(tmpdir(), 'flynn-skills-cli-')); + const configPath = join(root, 'config.yaml'); + const managedDir = join(root, 'managed'); + const bundledDir = join(root, 'bundled'); + const workspaceDir = join(root, 'workspace'); + mkdirSync(managedDir, { recursive: true }); + mkdirSync(bundledDir, { recursive: true }); + mkdirSync(workspaceDir, { recursive: true }); + writeSkillsCliConfig(configPath, { managedDir, bundledDir, workspaceDir, allowShellRunner: false }); + + const program = new Command(); + registerSkillsCommand(program); + + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined); + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined); + process.exitCode = undefined; + + await program.parseAsync(['skills', 'install', '/tmp/any-skill', '--runner', 'shell', '-c', configPath], { from: 'user' }); + + expect(errorSpy).toHaveBeenCalledWith("Runner 'shell' is disabled by config. Set skills.allow_shell_runner: true to enable it."); + expect(logSpy).not.toHaveBeenCalled(); + expect(process.exitCode).toBe(1); + + errorSpy.mockRestore(); + logSpy.mockRestore(); + process.exitCode = undefined; + rmSync(root, { recursive: true, force: true }); + }); + + it('skills install marks shell execution as allowlist_blocked when command is not allowed', async () => { + const root = mkdtempSync(join(tmpdir(), 'flynn-skills-cli-')); + const configPath = join(root, 'config.yaml'); + const sourceDir = join(root, 'source-skill'); + const managedDir = join(root, 'managed'); + const bundledDir = join(root, 'bundled'); + const workspaceDir = join(root, 'workspace'); + mkdirSync(sourceDir, { recursive: true }); + mkdirSync(managedDir, { recursive: true }); + mkdirSync(bundledDir, { recursive: true }); + mkdirSync(workspaceDir, { recursive: true }); + writeSkillsCliConfig(configPath, { + managedDir, + bundledDir, + workspaceDir, + installationExecution: 'enabled', + allowShellRunner: true, + shellRunnerAllowlist: ['echo*'], + }); + writeFileSync(join(sourceDir, 'SKILL.md'), '# Install Skill\nInstructions'); + writeFileSync( + join(sourceDir, 'manifest.json'), + JSON.stringify({ + name: 'cli-install-allowlist-blocked', + description: 'CLI install allowlist blocked', + version: '1.0.0', + installers: [{ type: 'download', url: 'https://example.com/cli-install-allowlist-blocked.tgz' }], + }), + 'utf-8', + ); + + const program = new Command(); + registerSkillsCommand(program); + + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined); + process.exitCode = undefined; + + await program.parseAsync( + ['skills', 'install', sourceDir, '--json', '--execute', '--confirm', '--runner', 'shell', '-c', configPath], + { from: 'user' }, + ); + + const payload = JSON.parse(String(logSpy.mock.calls[logSpy.mock.calls.length - 1]?.[0])); + expect(payload.execution.execution_enabled).toBe(true); + expect(payload.execution.results).toEqual([ + { + installer_type: 'download', + command: 'download https://example.com/cli-install-allowlist-blocked.tgz -> ', + status: 'failed', + reason: 'allowlist_blocked', + }, + ]); + + logSpy.mockRestore(); + process.exitCode = undefined; + rmSync(root, { recursive: true, force: true }); + }); + it('skills install parses execute flags and emits execution-enabled JSON receipt', async () => { const root = mkdtempSync(join(tmpdir(), 'flynn-skills-cli-')); const configPath = join(root, 'config.yaml'); @@ -987,7 +1267,7 @@ describe('skills CLI helpers', () => { mkdirSync(managedDir, { recursive: true }); mkdirSync(bundledDir, { recursive: true }); mkdirSync(workspaceDir, { recursive: true }); - writeSkillsCliConfig(configPath, { managedDir, bundledDir, workspaceDir }); + writeSkillsCliConfig(configPath, { managedDir, bundledDir, workspaceDir, installationExecution: 'enabled' }); writeFileSync(join(sourceDir, 'SKILL.md'), '# Install Skill\nInstructions'); writeFileSync( join(sourceDir, 'manifest.json'), @@ -1158,7 +1438,7 @@ describe('skills CLI helpers', () => { mkdirSync(skillDir, { recursive: true }); mkdirSync(bundledDir, { recursive: true }); mkdirSync(workspaceDir, { recursive: true }); - writeSkillsCliConfig(configPath, { managedDir, bundledDir, workspaceDir }); + writeSkillsCliConfig(configPath, { managedDir, bundledDir, workspaceDir, installationExecution: 'enabled' }); writeFileSync(join(skillDir, 'SKILL.md'), '# Execute Skill\nInstructions'); writeFileSync( join(skillDir, 'manifest.json'), @@ -1191,6 +1471,68 @@ describe('skills CLI helpers', () => { rmSync(root, { recursive: true, force: true }); }); + it('skills execute keeps execution policy-disabled even with --execute --confirm', async () => { + const root = mkdtempSync(join(tmpdir(), 'flynn-skills-cli-')); + const configPath = join(root, 'config.yaml'); + const managedDir = join(root, 'managed'); + const bundledDir = join(root, 'bundled'); + const workspaceDir = join(root, 'workspace'); + const skillDir = join(managedDir, 'cli-exec-policy-disabled'); + mkdirSync(skillDir, { recursive: true }); + mkdirSync(bundledDir, { recursive: true }); + mkdirSync(workspaceDir, { recursive: true }); + writeSkillsCliConfig(configPath, { managedDir, bundledDir, workspaceDir, installationExecution: 'disabled' }); + writeFileSync(join(skillDir, 'SKILL.md'), '# Execute Skill\nInstructions'); + writeFileSync( + join(skillDir, 'manifest.json'), + JSON.stringify({ + name: 'cli-exec-policy-disabled', + description: 'CLI execute policy disabled', + version: '1.0.0', + installers: [{ type: 'download', url: 'https://example.com/cli-exec-policy-disabled.tgz' }], + }), + 'utf-8', + ); + + const program = new Command(); + registerSkillsCommand(program); + + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined); + process.exitCode = undefined; + + await program.parseAsync( + [ + 'skills', + 'execute', + 'cli-exec-policy-disabled', + '--json', + '--execute', + '--confirm', + '--runner', + 'noop', + '-c', + configPath, + ], + { from: 'user' }, + ); + + const payload = JSON.parse(String(logSpy.mock.calls[0]?.[0])); + expect(payload.execution_enabled).toBe(false); + expect(payload.reason).toBe('execution_policy_disabled'); + expect(payload.results).toEqual([ + { + installer_type: 'download', + command: 'download https://example.com/cli-exec-policy-disabled.tgz -> ', + status: 'skipped', + reason: 'execution_policy_disabled', + }, + ]); + + logSpy.mockRestore(); + process.exitCode = undefined; + rmSync(root, { recursive: true, force: true }); + }); + it('skills execute JSON uses execution_disabled fallback when --execute is omitted', async () => { const root = mkdtempSync(join(tmpdir(), 'flynn-skills-cli-')); const configPath = join(root, 'config.yaml'); @@ -1281,4 +1623,113 @@ describe('skills CLI helpers', () => { process.exitCode = undefined; rmSync(root, { recursive: true, force: true }); }); + + it('skills execute rejects shell runner when config disallows it', async () => { + const root = mkdtempSync(join(tmpdir(), 'flynn-skills-cli-')); + const configPath = join(root, 'config.yaml'); + const managedDir = join(root, 'managed'); + const bundledDir = join(root, 'bundled'); + const workspaceDir = join(root, 'workspace'); + const skillDir = join(managedDir, 'cli-exec-skill'); + mkdirSync(skillDir, { recursive: true }); + mkdirSync(bundledDir, { recursive: true }); + mkdirSync(workspaceDir, { recursive: true }); + writeSkillsCliConfig(configPath, { managedDir, bundledDir, workspaceDir, allowShellRunner: false }); + writeFileSync(join(skillDir, 'SKILL.md'), '# Execute Skill\nInstructions'); + writeFileSync( + join(skillDir, 'manifest.json'), + JSON.stringify({ + name: 'cli-exec-skill', + description: 'CLI execute parse', + version: '1.0.0', + }), + 'utf-8', + ); + + const program = new Command(); + registerSkillsCommand(program); + + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined); + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined); + process.exitCode = undefined; + + await program.parseAsync(['skills', 'execute', 'cli-exec-skill', '--runner', 'shell', '-c', configPath], { from: 'user' }); + + expect(errorSpy).toHaveBeenCalledWith("Runner 'shell' is disabled by config. Set skills.allow_shell_runner: true to enable it."); + expect(logSpy).not.toHaveBeenCalled(); + expect(process.exitCode).toBe(1); + + errorSpy.mockRestore(); + logSpy.mockRestore(); + process.exitCode = undefined; + rmSync(root, { recursive: true, force: true }); + }); + + it('skills execute marks shell execution as allowlist_blocked when command is not allowed', async () => { + const root = mkdtempSync(join(tmpdir(), 'flynn-skills-cli-')); + const configPath = join(root, 'config.yaml'); + const managedDir = join(root, 'managed'); + const bundledDir = join(root, 'bundled'); + const workspaceDir = join(root, 'workspace'); + const skillDir = join(managedDir, 'cli-exec-allowlist-blocked'); + mkdirSync(skillDir, { recursive: true }); + mkdirSync(bundledDir, { recursive: true }); + mkdirSync(workspaceDir, { recursive: true }); + writeSkillsCliConfig(configPath, { + managedDir, + bundledDir, + workspaceDir, + installationExecution: 'enabled', + allowShellRunner: true, + shellRunnerAllowlist: ['echo*'], + }); + writeFileSync(join(skillDir, 'SKILL.md'), '# Execute Skill\nInstructions'); + writeFileSync( + join(skillDir, 'manifest.json'), + JSON.stringify({ + name: 'cli-exec-allowlist-blocked', + description: 'CLI execute allowlist blocked', + version: '1.0.0', + installers: [{ type: 'download', url: 'https://example.com/cli-exec-allowlist-blocked.tgz' }], + }), + 'utf-8', + ); + + const program = new Command(); + registerSkillsCommand(program); + + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => undefined); + process.exitCode = undefined; + + await program.parseAsync( + [ + 'skills', + 'execute', + 'cli-exec-allowlist-blocked', + '--json', + '--execute', + '--confirm', + '--runner', + 'shell', + '-c', + configPath, + ], + { from: 'user' }, + ); + + const payload = JSON.parse(String(logSpy.mock.calls[0]?.[0])); + expect(payload.execution_enabled).toBe(true); + expect(payload.results).toEqual([ + { + installer_type: 'download', + command: 'download https://example.com/cli-exec-allowlist-blocked.tgz -> ', + status: 'failed', + reason: 'allowlist_blocked', + }, + ]); + + logSpy.mockRestore(); + process.exitCode = undefined; + rmSync(root, { recursive: true, force: true }); + }); }); diff --git a/src/cli/skills.ts b/src/cli/skills.ts index 53a5df3..609f1d1 100644 --- a/src/cli/skills.ts +++ b/src/cli/skills.ts @@ -2,6 +2,8 @@ import type { Command } from 'commander'; import { resolve } from 'path'; import { homedir } from 'os'; import { spawnSync } from 'child_process'; +import { createHash } from 'crypto'; +import { auditLogger } from '../audit/index.js'; import type { Skill } from '../skills/index.js'; import { loadAllSkills, SkillInstaller, buildInstallerPlan, loadSkill } from '../skills/index.js'; import { loadConfigSafe } from './shared.js'; @@ -54,7 +56,11 @@ export interface SkillInstallerExecutionStubView { } export type SkillInstallActionMode = 'plan-only' | 'stub' | 'install'; -export type SkillInstallerExecutionReason = 'execution_disabled' | 'confirmation_required' | 'execution_enabled'; +export type SkillInstallerExecutionReason = + | 'execution_disabled' + | 'confirmation_required' + | 'execution_enabled' + | 'execution_policy_disabled'; export type SkillInstallerStepStatus = 'blocked' | 'skipped' | 'succeeded' | 'failed'; export type SkillInstallerRunnerMode = 'noop' | 'shell'; @@ -74,16 +80,111 @@ export interface SkillInstallerCommandRunResult { reason?: string; } +export function hashSkillInstallerAuditCommand(command: string): string { + const digest = createHash('sha256').update(command).digest('hex'); + return `sha256:${digest}`; +} + +export function sanitizeSkillInstallerAuditReason(reason: string): string { + if (reason.startsWith('spawn_error:')) { + return 'spawn_error'; + } + return reason; +} + +interface SkillShellRunnerAuditLogger { + skillsInstallerExecutionBlocked(event: { + skill_name: string; + phase: 'install' | 'execute'; + execution_requested: boolean; + execution_enabled: boolean; + reason: string; + attempted_command_count: number; + }): void; + skillsInstallerCommandResult(event: { + skill_name: string; + phase: 'install' | 'execute'; + installer_type: string; + command: string; + status: 'blocked' | 'skipped' | 'succeeded' | 'failed'; + reason: string; + }): void; +} + +export function emitShellRunnerAuditEvents(args: { + skillName: string; + phase: 'install' | 'execute'; + executionRequested: boolean; + executionEnabled: boolean; + reason: SkillInstallerExecutionReason; + results: SkillInstallerExecutionStubView['results']; + logger?: SkillShellRunnerAuditLogger | null; +}): void { + const logger = args.logger ?? (auditLogger as unknown as SkillShellRunnerAuditLogger | null); + if (!logger) { + return; + } + + if (args.executionRequested && !args.executionEnabled) { + logger.skillsInstallerExecutionBlocked({ + skill_name: args.skillName, + phase: args.phase, + execution_requested: args.executionRequested, + execution_enabled: args.executionEnabled, + reason: sanitizeSkillInstallerAuditReason(args.reason), + attempted_command_count: args.results.length, + }); + return; + } + + for (const result of args.results) { + logger.skillsInstallerCommandResult({ + skill_name: args.skillName, + phase: args.phase, + installer_type: result.installer_type, + command: hashSkillInstallerAuditCommand(result.command), + status: result.status, + reason: sanitizeSkillInstallerAuditReason(result.reason), + }); + } +} + +export function checkCommandAgainstAllowlist(command: string, allowlist?: string[]): boolean { + if (!allowlist) { + return true; + } + + if (allowlist.length === 0) { + return false; + } + + return allowlist.some((pattern) => { + const regexSource = pattern + .split('*') + .map((segment) => segment.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')) + .join('.*'); + return new RegExp(`^${regexSource}$`).test(command); + }); +} + export const noOpSkillInstallerCommandRunner: SkillInstallerCommandRunner = { run(_commands: string[]): SkillInstallerCommandRunResult[] { return []; }, }; -export function createShellSkillInstallerCommandRunner(): SkillInstallerCommandRunner { +export function createShellSkillInstallerCommandRunner(allowlist?: string[]): SkillInstallerCommandRunner { return { run(commands: string[]): SkillInstallerCommandRunResult[] { return commands.map((command) => { + if (!checkCommandAgainstAllowlist(command, allowlist)) { + return { + command, + status: 'failed', + reason: 'allowlist_blocked', + }; + } + const result = spawnSync(command, { shell: true, stdio: 'pipe', @@ -133,13 +234,14 @@ export function createShellSkillInstallerCommandRunner(): SkillInstallerCommandR export function resolveSkillInstallerCommandRunner( mode?: string, + allowlist?: string[], ): { mode: SkillInstallerRunnerMode; runner: SkillInstallerCommandRunner } | { error: string } { if (!mode || mode === 'noop') { return { mode: 'noop', runner: noOpSkillInstallerCommandRunner }; } if (mode === 'shell') { - return { mode: 'shell', runner: createShellSkillInstallerCommandRunner() }; + return { mode: 'shell', runner: createShellSkillInstallerCommandRunner(allowlist) }; } return { error: `Invalid runner '${mode}'. Allowed values: noop, shell.` }; @@ -386,13 +488,19 @@ export function toSkillInstallerExecutionStubView(skill: Skill): SkillInstallerE export function toSkillInstallerExecutionStubFromPreflight( preflight: SkillInstallPreflightView, - options?: { mode?: SkillInstallActionMode; confirmed?: boolean; executionRequested?: boolean }, + options?: { + mode?: SkillInstallActionMode; + confirmed?: boolean; + executionRequested?: boolean; + configPolicyEnabled?: boolean; + }, ): SkillInstallerExecutionStubView { const mode = options?.mode ?? 'stub'; const policy = evaluateInstallerExecutionPolicy({ mode, confirmed: options?.confirmed ?? false, executionRequested: options?.executionRequested ?? false, + configPolicyEnabled: options?.configPolicyEnabled, }); const stepEnvelopes = toInstallerExecutionStepEnvelopes(preflight.steps, policy); return { @@ -414,6 +522,7 @@ export function evaluateInstallerExecutionPolicy(opts: { mode: SkillInstallActionMode; confirmed: boolean; executionRequested?: boolean; + configPolicyEnabled?: boolean; }): SkillInstallerExecutionPolicy { if (opts.mode === 'install' && !opts.confirmed) { return { @@ -423,6 +532,14 @@ export function evaluateInstallerExecutionPolicy(opts: { }; } + if (opts.mode === 'install' && opts.executionRequested && opts.configPolicyEnabled === false) { + return { + confirmed: opts.confirmed, + execution_enabled: false, + reason: 'execution_policy_disabled', + }; + } + if (opts.mode === 'install' && opts.confirmed && opts.executionRequested) { return { confirmed: true, @@ -456,6 +573,11 @@ export function renderSkillInstallerExecutionStub(view: SkillInstallerExecutionS view.executed.length > 0 ? 'Installer commands were executed.' : 'No installer commands were executed.', ]; + if (view.reason === 'execution_policy_disabled') { + lines.push('Execution policy blocked installer commands.'); + lines.push('Set skills.installation_execution: enabled in config and re-run with --execute --confirm to allow execution.'); + } + if (view.wouldRun.length === 0) { lines.push('Would run: none'); } else { @@ -489,6 +611,8 @@ export function runSkillExecuteAction( confirmed: boolean; executionRequested?: boolean; commandRunner?: SkillInstallerCommandRunner; + configPolicyEnabled?: boolean; + runnerMode?: SkillInstallerRunnerMode; }, ): { ok: true; execution: SkillInstallerExecutionStubView } { const execution = toSkillInstallerExecutionStubView(skill); @@ -496,6 +620,7 @@ export function runSkillExecuteAction( mode: 'install', confirmed: opts.confirmed, executionRequested: opts.executionRequested ?? false, + configPolicyEnabled: opts.configPolicyEnabled, }); execution.confirmed = policy.confirmed; @@ -510,6 +635,17 @@ export function runSkillExecuteAction( execution.executed = commandResults.map((result) => result.command); execution.results = mergeInstallerExecutionResults(execution.attempted, policy, commandResults); + if (opts.runnerMode === 'shell') { + emitShellRunnerAuditEvents({ + skillName: execution.skill.name, + phase: 'execute', + executionRequested: opts.executionRequested ?? false, + executionEnabled: policy.execution_enabled, + reason: policy.reason, + results: execution.results, + }); + } + if (opts.asJson) { console.log(JSON.stringify(execution, null, 2)); } else { @@ -588,6 +724,8 @@ export function runSkillInstallAction( confirmed: boolean; executionRequested?: boolean; commandRunner?: SkillInstallerCommandRunner; + configPolicyEnabled?: boolean; + runnerMode?: SkillInstallerRunnerMode; }, ): { ok: true } | { ok: false; error: string } { const preflight = toSkillInstallPreflightView(sourcePath); @@ -600,6 +738,7 @@ export function runSkillInstallAction( const execution = toSkillInstallerExecutionStubFromPreflight(preflight, { mode: 'plan-only', confirmed: opts.confirmed, + configPolicyEnabled: opts.configPolicyEnabled, }); console.log(JSON.stringify({ preflight, execution }, null, 2)); } else { @@ -615,6 +754,7 @@ export function runSkillInstallAction( const stub = toSkillInstallerExecutionStubFromPreflight(preflight, { mode: 'stub', confirmed: opts.confirmed, + configPolicyEnabled: opts.configPolicyEnabled, }); if (opts.asJson) { console.log(JSON.stringify({ execution: stub }, null, 2)); @@ -633,11 +773,16 @@ export function runSkillInstallAction( return { ok: false, error: result.error ?? `Failed to install skill from '${sourcePath}'.` }; } - const installPolicy = evaluateInstallerExecutionPolicy({ mode: 'install', confirmed: opts.confirmed }); + const installPolicy = evaluateInstallerExecutionPolicy({ + mode: 'install', + confirmed: opts.confirmed, + configPolicyEnabled: opts.configPolicyEnabled, + }); const requestedInstallPolicy = evaluateInstallerExecutionPolicy({ mode: 'install', confirmed: opts.confirmed, executionRequested: opts.executionRequested ?? false, + configPolicyEnabled: opts.configPolicyEnabled, }); const execution = @@ -646,6 +791,7 @@ export function runSkillInstallAction( mode: 'install', confirmed: opts.confirmed, executionRequested: opts.executionRequested ?? false, + configPolicyEnabled: opts.configPolicyEnabled, }) : { skill: { @@ -677,6 +823,17 @@ export function runSkillInstallAction( commandResults, ); + if (opts.runnerMode === 'shell') { + emitShellRunnerAuditEvents({ + skillName: execution.skill.name, + phase: 'install', + executionRequested: opts.executionRequested ?? false, + executionEnabled: requestedInstallPolicy.execution_enabled, + reason: requestedInstallPolicy.reason, + results: execution.results, + }); + } + if (opts.asJson) { console.log( JSON.stringify( @@ -788,8 +945,8 @@ export function registerSkillsCommand(program: Command): void { .option('--preflight-only', 'Show installer preflight without performing install') .option('--stub', 'Show installer execution stub without performing install') .option('--confirm', 'Mark installer execution intent as confirmed (required with --execute)') - .option('--execute', 'Enable installer command execution (requires --confirm)') - .option('--runner ', 'Installer runner: noop (default) or shell') + .option('--execute', 'Enable installer command execution (requires --confirm and skills.installation_execution=enabled)') + .option('--runner ', 'Installer runner: noop (default) or shell (requires skills.allow_shell_runner=true and allowlist)') .option('-c, --config ', 'Config file path') .action( (pathArg: string, opts: { @@ -808,7 +965,10 @@ export function registerSkillsCommand(program: Command): void { return; } - const runnerResolution = resolveSkillInstallerCommandRunner(opts.runner); + const runnerResolution = resolveSkillInstallerCommandRunner( + opts.runner, + loaded.config.skills.shell_runner_allowlist, + ); if ('error' in runnerResolution) { console.error(runnerResolution.error); process.exitCode = 1; @@ -821,16 +981,25 @@ export function registerSkillsCommand(program: Command): void { return; } + if ((opts.runner ?? 'noop') === 'shell' && !loaded.config.skills.allow_shell_runner) { + console.error("Runner 'shell' is disabled by config. Set skills.allow_shell_runner: true to enable it."); + process.exitCode = 1; + return; + } + const defaultManagedDir = resolve(homedir(), '.flynn/workspace/skills'); const installer = new SkillInstaller(loaded.config.skills.managed_dir ?? defaultManagedDir); const mode: SkillInstallActionMode = opts.preflightOnly ? 'plan-only' : opts.stub ? 'stub' : 'install'; + const configPolicyEnabled = loaded.config.skills.installation_execution === 'enabled'; const result = runSkillInstallAction(installer, pathArg, { mode, asJson: opts.json ?? false, confirmed: opts.confirm ?? false, executionRequested: opts.execute ?? false, commandRunner: runnerResolution.runner, + configPolicyEnabled, + runnerMode: runnerResolution.mode, }); if (!result.ok) { @@ -933,8 +1102,8 @@ export function registerSkillsCommand(program: Command): void { .description('Preview or execute installer steps for an installed skill') .option('--json', 'Output as JSON') .option('--confirm', 'Mark installer execution intent as confirmed (required with --execute)') - .option('--execute', 'Enable installer command execution (requires --confirm)') - .option('--runner ', 'Installer runner: noop (default) or shell') + .option('--execute', 'Enable installer command execution (requires --confirm and skills.installation_execution=enabled)') + .option('--runner ', 'Installer runner: noop (default) or shell (requires skills.allow_shell_runner=true and allowlist)') .option('-c, --config ', 'Config file path') .action((name: string, opts: { json?: boolean; confirm?: boolean; execute?: boolean; runner?: string; config?: string }) => { const loaded = loadSkillsFromConfig(opts.config); @@ -944,6 +1113,13 @@ export function registerSkillsCommand(program: Command): void { return; } + const configLoaded = loadConfigSafe(opts.config); + if (configLoaded.error || !configLoaded.config) { + console.error(configLoaded.error ?? 'Failed to load config'); + process.exitCode = 1; + return; + } + const skill = loaded.skills.find((item) => item.manifest.name === name); if (!skill) { console.error(`Skill '${name}' not found.`); @@ -951,7 +1127,10 @@ export function registerSkillsCommand(program: Command): void { return; } - const runnerResolution = resolveSkillInstallerCommandRunner(opts.runner); + const runnerResolution = resolveSkillInstallerCommandRunner( + opts.runner, + configLoaded.config.skills.shell_runner_allowlist, + ); if ('error' in runnerResolution) { console.error(runnerResolution.error); process.exitCode = 1; @@ -964,11 +1143,19 @@ export function registerSkillsCommand(program: Command): void { return; } + if ((opts.runner ?? 'noop') === 'shell' && !configLoaded.config.skills.allow_shell_runner) { + console.error("Runner 'shell' is disabled by config. Set skills.allow_shell_runner: true to enable it."); + process.exitCode = 1; + return; + } + runSkillExecuteAction(skill, { asJson: opts.json ?? false, confirmed: opts.confirm ?? false, executionRequested: opts.execute ?? false, commandRunner: runnerResolution.runner, + configPolicyEnabled: configLoaded.config.skills.installation_execution === 'enabled', + runnerMode: runnerResolution.mode, }); }); }