diff --git a/packages/agent-core/src/bash-subprocess-env.ts b/packages/agent-core/src/bash-subprocess-env.ts index 4cf39ca..c52cb0d 100644 --- a/packages/agent-core/src/bash-subprocess-env.ts +++ b/packages/agent-core/src/bash-subprocess-env.ts @@ -11,8 +11,8 @@ * parent session against the runtime API, or a nested `mavis` CLI inside * bash would silently bind to the parent runtime's profile. * - * Layer B — third-party secret scrub. GATED, mirroring the reference CLI's - * `subprocessEnv()` design (src/utils/subprocessEnv.ts): + * Layer B — provider and integration secret scrub. GATED, mirroring the + * reference CLI's `subprocessEnv()` design (src/utils/subprocessEnv.ts): * - `off` (interactive default): do not strip user secrets. CC parity — * interactive users rely on env credentials (gh, npm, ...). * - `scrub` (auto in CI / non-interactive): precise blocklist of @@ -74,6 +74,7 @@ export interface BashEnvSanitizeResult { */ export const BASH_SUBPROCESS_SCRUB: readonly string[] = [ // Provider / LLM auth — the runtime re-reads these per-request itself + 'MCODE_PROVIDER_API_KEY', 'ANTHROPIC_API_KEY', 'ANTHROPIC_AUTH_TOKEN', 'CLAUDE_CODE_OAUTH_TOKEN', @@ -197,7 +198,7 @@ export function sanitizeBashSubprocessEnv( // Layer A — MCode boundary strip. Always on; allowlist deliberately ignored. removed.push(...stripRuntimeBoundaryKeysFrom(out, 'agent-runtime')); - // Layer B — third-party secret scrub, gated by mode. + // Layer B — provider and integration secret scrub, gated by mode. if (policy.mode === 'scrub') { for (const key of BASH_SUBPROCESS_SCRUB) { for (const name of [key, `INPUT_${key}`]) { diff --git a/packages/agent-core/test/unit/bash-subprocess-env.test.ts b/packages/agent-core/test/unit/bash-subprocess-env.test.ts new file mode 100644 index 0000000..6158a2f --- /dev/null +++ b/packages/agent-core/test/unit/bash-subprocess-env.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from 'vitest'; + +import { + createBashEnvSpawnHook, + resolveBashEnvPolicy, + sanitizeBashSubprocessEnv, +} from '../../src/bash-subprocess-env.js'; + +const providerEnv = { + MCODE_PROVIDER_API_KEY: 'synthetic-provider-value', + INPUT_MCODE_PROVIDER_API_KEY: 'synthetic-input-value', + MCODE_PROVIDER_BASE_URL: 'https://provider.example.invalid', + MCODE_PROVIDER_MODEL: 'synthetic-model', + CUSTOM_PROVIDER_API_KEY: 'synthetic-custom-value', + GH_TOKEN: 'synthetic-gh-value', + GITHUB_TOKEN: 'synthetic-github-value', + NPM_TOKEN: 'synthetic-npm-value', +}; + +describe('bash subprocess default provider credentials', () => { + it.each([{ CI: 'true' }, { GITHUB_ACTIONS: 'true' }, { MAVIS_BASH_ENV_SANITIZE: 'scrub' }])( + 'scrubs both provider key names using policy %j', + (markers) => { + const original = { ...providerEnv, ...markers }; + const policy = resolveBashEnvPolicy(undefined, original); + expect(policy.mode).toBe('scrub'); + const { env, removed } = sanitizeBashSubprocessEnv(original, policy); + const { MCODE_PROVIDER_API_KEY, INPUT_MCODE_PROVIDER_API_KEY, ...preserved } = original; + expect(env).toEqual(preserved); + expect(removed).toEqual(['INPUT_MCODE_PROVIDER_API_KEY', 'MCODE_PROVIDER_API_KEY']); + expect(original.MCODE_PROVIDER_API_KEY).toBe(MCODE_PROVIDER_API_KEY); + expect(original.INPUT_MCODE_PROVIDER_API_KEY).toBe(INPUT_MCODE_PROVIDER_API_KEY); + }, + ); + + it('applies scrub to the actual spawn-hook environment without mutating its input', () => { + const original = { command: 'echo synthetic', cwd: '.', env: { ...providerEnv } }; + const result = createBashEnvSpawnHook({ mode: 'scrub' })(original); + expect(result.env).not.toHaveProperty('MCODE_PROVIDER_API_KEY'); + expect(result.env).not.toHaveProperty('INPUT_MCODE_PROVIDER_API_KEY'); + expect(result.command).toBe(original.command); + expect(original.env).toEqual(providerEnv); + }); + + it('preserves credentials when off is explicitly selected, including in CI', () => { + const policy = resolveBashEnvPolicy({ mode: 'off' }, { CI: 'true' }); + expect(sanitizeBashSubprocessEnv(providerEnv, policy)).toEqual({ + env: providerEnv, + removed: [], + }); + }); + + it('continues to strip both provider names in strict mode', () => { + const { env, removed } = sanitizeBashSubprocessEnv(providerEnv, { mode: 'strict' }); + expect(env).not.toHaveProperty('MCODE_PROVIDER_API_KEY'); + expect(env).not.toHaveProperty('INPUT_MCODE_PROVIDER_API_KEY'); + expect(removed).toContain('MCODE_PROVIDER_API_KEY'); + expect(removed).toContain('INPUT_MCODE_PROVIDER_API_KEY'); + expect(env.MCODE_PROVIDER_MODEL).toBe(providerEnv.MCODE_PROVIDER_MODEL); + expect(env.MCODE_PROVIDER_BASE_URL).toBe(providerEnv.MCODE_PROVIDER_BASE_URL); + }); + + it('retains the explicit strict allowlist escape hatch', () => { + const { env } = sanitizeBashSubprocessEnv(providerEnv, { + mode: 'strict', + allowlist: ['MCODE_PROVIDER_API_KEY', 'INPUT_MCODE_PROVIDER_API_KEY'], + }); + expect(env.MCODE_PROVIDER_API_KEY).toBe(providerEnv.MCODE_PROVIDER_API_KEY); + expect(env.INPUT_MCODE_PROVIDER_API_KEY).toBe(providerEnv.INPUT_MCODE_PROVIDER_API_KEY); + }); +}); diff --git a/packages/agent-modules/permission/src/context.ts b/packages/agent-modules/permission/src/context.ts index 0e90d42..d04a06c 100644 --- a/packages/agent-modules/permission/src/context.ts +++ b/packages/agent-modules/permission/src/context.ts @@ -224,6 +224,7 @@ function cloneContextWith( trustedExactWritePaths: ctx.trustedExactWritePaths, dataDir: ctx.dataDir, homeDir: ctx.homeDir, + isFile: ctx.isFile, agentName: ctx.agentName, sessionId: ctx.sessionId, allowManagedPermissionRulesOnly: ctx.allowManagedPermissionRulesOnly, diff --git a/packages/agent-modules/permission/src/tools/bash-permission.ts b/packages/agent-modules/permission/src/tools/bash-permission.ts index 6fa897d..5c3274b 100644 --- a/packages/agent-modules/permission/src/tools/bash-permission.ts +++ b/packages/agent-modules/permission/src/tools/bash-permission.ts @@ -1133,13 +1133,21 @@ function evaluateSingleSubcommand( // -------- COMMON LAYER -------- + // Every rule behavior sees the same raw and transparent-wrapper-stripped + // forms. Otherwise a wrapped command could skip deny/ask and match allow. + const wrapperStrippedCommand = stripTransparentWrappersForRuleMatch(command); + const matchesUserRule = (parsedRule: CompiledShellPermissionRule | undefined): boolean => + !!parsedRule && + (matchShellRule(command, parsedRule) || + (!!wrapperStrippedCommand && matchShellRule(wrapperStrippedCommand, parsedRule))); + // Step 1: user deny rule for (const { rule, parsedRule } of bashRules) { if (rule.ruleBehavior !== 'deny') continue; if (!rule.ruleValue.ruleContent) { return logLayer('user-deny', { verdict: 'deny', reason: { type: 'rule', rule } }); } - if (parsedRule && matchShellRule(command, parsedRule)) { + if (matchesUserRule(parsedRule)) { return logLayer('user-deny', { verdict: 'deny', reason: { type: 'rule', rule } }); } } @@ -1150,7 +1158,7 @@ function evaluateSingleSubcommand( if (!rule.ruleValue.ruleContent) { return logLayer('user-ask', { verdict: 'ask', reason: { type: 'rule', rule } }); } - if (parsedRule && matchShellRule(command, parsedRule)) { + if (matchesUserRule(parsedRule)) { return logLayer('user-ask', { verdict: 'ask', reason: { type: 'rule', rule } }); } } @@ -1301,20 +1309,13 @@ function evaluateSingleSubcommand( }); } - // Step 5: user allow rule. Match against the original command first; - // if a transparent wrapper prefixes the line (nohup/setsid/timeout/xargs/...), - // also match the stripped form so `pnpm:*` covers `nohup pnpm install`. - // Privilege wrappers (sudo / bash -c / env K=V) are not stripped. - const wrapperStrippedCommand = stripTransparentWrappersForRuleMatch(command); + // Step 5: user allow rule, using the same command forms as deny/ask above. for (const { rule, parsedRule } of bashRules) { if (rule.ruleBehavior !== 'allow') continue; if (!rule.ruleValue.ruleContent) { return logLayer('user-allow', { verdict: 'allow', reason: { type: 'rule', rule } }); } - if ( - (parsedRule && matchShellRule(command, parsedRule)) || - (wrapperStrippedCommand && parsedRule && matchShellRule(wrapperStrippedCommand, parsedRule)) - ) { + if (matchesUserRule(parsedRule)) { return logLayer('user-allow', { verdict: 'allow', reason: { type: 'rule', rule } }); } } diff --git a/packages/agent-modules/permission/test/unit/permission/bash-policy-regressions.test.ts b/packages/agent-modules/permission/test/unit/permission/bash-policy-regressions.test.ts new file mode 100644 index 0000000..a1ca0fe --- /dev/null +++ b/packages/agent-modules/permission/test/unit/permission/bash-policy-regressions.test.ts @@ -0,0 +1,112 @@ +import path from 'node:path'; +import { describe, expect, it, vi } from 'vitest'; + +import { applyPermissionUpdate, createToolPermissionContext } from '../../../src/context.js'; +import { BashToolPermissionChecker } from '../../../src/tools/bash-checker.js'; +import { evaluateBashStatic } from '../../../src/tools/bash-permission.js'; +import type { PermissionBehavior, PermissionRule } from '../../../src/types.js'; + +function bashRule(behavior: PermissionBehavior, content?: string): PermissionRule { + return { + source: 'session', + ruleBehavior: behavior, + ruleValue: { toolName: 'bash', ruleContent: content }, + }; +} + +describe.each(['default', 'auto', 'bypass'] as const)('bash rule precedence in %s mode', (mode) => { + const commands = [ + 'npm publish', + 'nohup npm publish', + 'timeout 30 npm publish', + 'nice -n 5 npm publish', + 'setsid npm publish', + 'xargs npm publish', + 'nohup timeout 30 npm publish', + 'CI=1 npm publish', + 'env CI=1 npm publish', + 'nohup /usr/bin/npm publish', + 'echo ready && nohup npm publish', + ]; + + describe.each(['deny', 'ask'] as const)('%s before allow', (behavior) => { + it.each(commands)('enforces the restricted prefix for %s', (command) => { + const rules = [bashRule('allow', 'npm:*'), bashRule(behavior, 'npm publish:*')]; + expect(evaluateBashStatic(command, rules, undefined, mode).verdict).toBe(behavior); + }); + + it.each(['npm publish', 'npm pub*'])('matches stripped exact/wildcard rule %s', (content) => { + const rules = [bashRule('allow', 'npm:*'), bashRule(behavior, content)]; + expect(evaluateBashStatic('nohup npm publish', rules, undefined, mode).verdict).toBe( + behavior, + ); + }); + + it('still matches raw wrapper rules', () => { + const rules = [bashRule('allow', 'npm:*'), bashRule(behavior, 'nohup npm:*')]; + expect(evaluateBashStatic('nohup npm publish', rules, undefined, mode).verdict).toBe( + behavior, + ); + }); + + it('still matches whole-tool rules', () => { + const rules = [bashRule('allow', 'npm:*'), bashRule(behavior)]; + expect(evaluateBashStatic('nohup npm publish', rules, undefined, mode).verdict).toBe( + behavior, + ); + }); + }); + + it('allows an unrestricted wrapped command in the same prefix family', () => { + const rules = [bashRule('allow', 'npm:*'), bashRule('deny', 'npm publish:*')]; + expect(evaluateBashStatic('nohup npm install', rules, undefined, mode).verdict).toBe('allow'); + }); + + it('keeps deny stronger than ask regardless of rule order', () => { + const rules = [ + bashRule('ask', 'npm:*'), + bashRule('allow', 'npm:*'), + bashRule('deny', 'npm publish:*'), + ]; + expect(evaluateBashStatic('nohup npm publish', rules, undefined, mode).verdict).toBe('deny'); + }); +}); + +describe('permission context rule updates', () => { + it.each(['addRules', 'replaceRules', 'removeRules'] as const)( + '%s preserves local-script classification and the host file probe', + (type) => { + const workingDirectory = path.resolve('synthetic-workspace'); + const isFile = vi.fn((file: string) => file === path.join(workingDirectory, 'build.js')); + const ctx = createToolPermissionContext({ + workingDirectory, + dataDir: path.join(workingDirectory, 'data'), + homeDir: path.join(workingDirectory, 'home'), + platform: 'linux', + shellFamily: 'posix', + isFile, + rules: [{ source: 'session', ruleBehavior: 'allow', ruleValue: { toolName: 'read' } }], + }); + const checker = new BashToolPermissionChecker(); + const input = { command: 'echo ready && node ./build.js' }; + const before = checker.checkPermissions('bash', input, ctx); + expect(isFile).toHaveBeenCalledWith(path.join(workingDirectory, 'build.js')); + isFile.mockClear(); + + const updated = applyPermissionUpdate(ctx, { + type, + source: 'session', + destination: 'session', + behavior: 'allow', + rules: [{ toolName: 'read' }], + }); + expect(checker.checkPermissions('bash', input, updated)).toEqual(before); + expect(isFile).toHaveBeenCalledWith(path.join(workingDirectory, 'build.js')); + expect(updated.isFile).toBe(isFile); + expect(updated).not.toBe(ctx); + expect(Object.isFrozen(updated)).toBe(true); + expect(Object.isFrozen(updated.rules)).toBe(true); + expect(ctx.rules).toHaveLength(1); + }, + ); +}); diff --git a/release/public-source.json b/release/public-source.json index f3981de..f1820b3 100644 --- a/release/public-source.json +++ b/release/public-source.json @@ -104,6 +104,7 @@ "packages/agent-core/src/tools/index.ts", "packages/agent-core/src/tools/input-validation.ts", "packages/agent-core/src/tools/types.ts", + "packages/agent-core/test/unit/bash-subprocess-env.test.ts", "packages/agent-core/test/unit/event-bridge/converters.test.ts", "packages/agent-core/test/unit/image-dimensions.test.ts", "packages/agent-core/test/unit/image-fixtures.ts", @@ -238,6 +239,7 @@ "packages/agent-modules/permission/src/types.ts", "packages/agent-modules/permission/src/windows-trash-execution.ts", "packages/agent-modules/permission/src/written-files-registry.ts", + "packages/agent-modules/permission/test/unit/permission/bash-policy-regressions.test.ts", "packages/agent-modules/permission/test/unit/permission/cloud-classify-client.test.ts", "packages/agent-modules/plugin-hooks/package.json", "packages/agent-modules/plugin-hooks/src/command-invocation.ts", diff --git a/test/vitest-suites.json b/test/vitest-suites.json index d8f10b1..0be223a 100644 --- a/test/vitest-suites.json +++ b/test/vitest-suites.json @@ -2,6 +2,8 @@ "comment": "Vitest suites for the standalone distribution, grouped by the gate that runs them. `vitest.oss.config.mjs` includes every group; `scripts/run-vitest-suite.mjs ` runs one. Add a test file here rather than in package.json or the Vitest config.", "suites": { "capability": [ + "packages/agent-core/test/unit/bash-subprocess-env.test.ts", + "packages/agent-modules/permission/test/unit/permission/bash-policy-regressions.test.ts", "packages/agent-tools/src/shared/replace-all-edit.test.ts", "packages/local-runtime-v2/test/unit/agent/agent-import.test.ts", "packages/tui/test/unit/auth-application.test.ts",