diff --git a/src/commands/commit-push-pr.ts b/src/commands/commit-push-pr.ts index 12e3ecf422..d7dda3a41d 100644 --- a/src/commands/commit-push-pr.ts +++ b/src/commands/commit-push-pr.ts @@ -4,6 +4,7 @@ import { getEnhancedPRAttribution, } from '../utils/attribution.js' import { getDefaultBranch } from '../utils/git.js' +import { getForbiddenCommitMessagePatterns } from '../utils/governancePolicy.js' import { executeShellCommandsInPrompt } from '../utils/promptShellExecution.js' import { getUndercoverInstructions, isUndercover } from '../utils/undercover.js' @@ -29,6 +30,11 @@ function getPromptContent( ): string { const { commit: commitAttribution, pr: defaultPrAttribution } = getAttributionTexts() + const forbiddenPatterns = getForbiddenCommitMessagePatterns() + const forbiddenPatternText = + forbiddenPatterns.length > 0 + ? `\n- The commit message must not contain these forbidden patterns: ${forbiddenPatterns.map(p => `\`${p}\``).join(', ')}` + : '' // Use provided PR attribution or fall back to default const effectivePrAttribution = prAttribution ?? defaultPrAttribution const safeUser = process.env.SAFEUSER || '' @@ -71,7 +77,7 @@ function getPromptContent( - NEVER skip hooks (--no-verify, --no-gpg-sign, etc) unless the user explicitly requests it - NEVER run force push to main/master, warn the user if they request it - Do not commit files that likely contain secrets (.env, credentials.json, etc) -- Never use git commands with the -i flag (like git rebase -i or git add -i) since they require interactive input which is not supported +- Never use git commands with the -i flag (like git rebase -i or git add -i) since they require interactive input which is not supported${forbiddenPatternText} ## Your task diff --git a/src/commands/commit.ts b/src/commands/commit.ts index 2c9b555566..cdfb82c9d3 100644 --- a/src/commands/commit.ts +++ b/src/commands/commit.ts @@ -1,5 +1,6 @@ import type { Command } from '../commands.js' import { getAttributionTexts } from '../utils/attribution.js' +import { getForbiddenCommitMessagePatterns } from '../utils/governancePolicy.js' import { executeShellCommandsInPrompt } from '../utils/promptShellExecution.js' import { getUndercoverInstructions, isUndercover } from '../utils/undercover.js' @@ -11,6 +12,11 @@ const ALLOWED_TOOLS = [ function getPromptContent(): string { const { commit: commitAttribution } = getAttributionTexts() + const forbiddenPatterns = getForbiddenCommitMessagePatterns() + const forbiddenPatternText = + forbiddenPatterns.length > 0 + ? `\n- The commit message must not contain these forbidden patterns: ${forbiddenPatterns.map(p => `\`${p}\``).join(', ')}` + : '' let prefix = '' if (process.env.USER_TYPE === 'ant' && isUndercover()) { @@ -31,7 +37,7 @@ function getPromptContent(): string { - CRITICAL: ALWAYS create NEW commits. NEVER use git commit --amend, unless the user explicitly requests it - Do not commit files that likely contain secrets (.env, credentials.json, etc). Warn the user if they specifically request to commit those files - If there are no changes to commit (i.e., no untracked files and no modifications), do not create an empty commit -- Never use git commands with the -i flag (like git rebase -i or git add -i) since they require interactive input which is not supported +- Never use git commands with the -i flag (like git rebase -i or git add -i) since they require interactive input which is not supported${forbiddenPatternText} ## Your task diff --git a/src/components/ProviderManager.test.tsx b/src/components/ProviderManager.test.tsx index c182abf3f9..04527b2c86 100644 --- a/src/components/ProviderManager.test.tsx +++ b/src/components/ProviderManager.test.tsx @@ -12,6 +12,12 @@ import { releaseSharedMutationLock, } from '../test/sharedMutationLock.js' +type SettingsModule = typeof import('../utils/settings/settings.js') + +const actualSettingsModule = (await import( + `../utils/settings/settings.ts?providerManagerSettingsActual=${Date.now()}-${Math.random()}` +)) as SettingsModule + const SYNC_START = '\x1B[?2026h' const SYNC_END = '\x1B[?2026l' @@ -411,6 +417,7 @@ function mockProviderManagerDependencies( })) mock.module('../utils/settings/settings.js', () => ({ + ...actualSettingsModule, updateSettingsForSource: () => ({ error: null }), })) @@ -524,6 +531,7 @@ beforeEach(async () => { afterEach(() => { try { mock.restore() + mock.module('../utils/settings/settings.js', () => actualSettingsModule) for (const [key, value] of Object.entries(ORIGINAL_ENV)) { if (value === undefined) { diff --git a/src/memdir/memdir.ts b/src/memdir/memdir.ts index 1e7e68b559..6e174ad04f 100644 --- a/src/memdir/memdir.ts +++ b/src/memdir/memdir.ts @@ -21,6 +21,7 @@ import { logForDebugging } from '../utils/debug.js' import { hasEmbeddedSearchTools } from '../utils/embeddedTools.js' import { isEnvTruthy } from '../utils/envUtils.js' import { formatFileSize } from '../utils/format.js' +import { isMemoryWriteApprovalRequired } from '../utils/governancePolicy.js' import { getProjectDir } from '../utils/sessionStorage.js' import { getInitialSettings } from '../utils/settings/settings.js' import { @@ -202,6 +203,10 @@ export function buildMemoryLines( extraGuidelines?: string[], skipIndex = false, ): string[] { + const requiresApproval = isMemoryWriteApprovalRequired() + const dirGuidance = requiresApproval + ? 'Do not create or update files there until the user explicitly approves the specific memory write.' + : DIR_EXISTS_GUIDANCE const howToSave = skipIndex ? [ '## How to save memories', @@ -236,10 +241,16 @@ export function buildMemoryLines( const lines: string[] = [ `# ${displayName}`, '', - `You have a persistent, file-based memory system at \`${memoryDir}\`. ${DIR_EXISTS_GUIDANCE}`, + `You have a persistent, file-based memory system at \`${memoryDir}\`. ${dirGuidance}`, '', "You should build up this memory system over time so that future conversations can have a complete picture of who the user is, how they'd like to collaborate with you, what behaviors to avoid or repeat, and the context behind the work the user gives you.", '', + ...(requiresApproval + ? [ + 'Before creating, updating, or deleting persistent memory files, explicitly ask the user for approval and wait for confirmation.', + '', + ] + : []), 'If the user explicitly asks you to remember something, save it immediately as whichever type fits best. If they ask you to forget something, find and remove the relevant entry.', '', ...TYPES_SECTION_INDIVIDUAL, @@ -456,7 +467,9 @@ export async function loadMemoryPrompt(): Promise { // creates the auto dir as a side effect. If the team dir ever moves // out from under the auto dir, add a second ensureMemoryDirExists call // for autoDir here. - await ensureMemoryDirExists(teamDir) + if (!isMemoryWriteApprovalRequired()) { + await ensureMemoryDirExists(teamDir) + } logMemoryDirCounts(autoDir, { memory_type: 'auto' as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, @@ -476,7 +489,9 @@ export async function loadMemoryPrompt(): Promise { const autoDir = getAutoMemPath() // Harness guarantees the directory exists so the model can write without // checking. The prompt text reflects this ("already exists"). - await ensureMemoryDirExists(autoDir) + if (!isMemoryWriteApprovalRequired()) { + await ensureMemoryDirExists(autoDir) + } logMemoryDirCounts(autoDir, { memory_type: 'auto' as AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, diff --git a/src/memdir/paths.test.ts b/src/memdir/paths.test.ts index 025d20c648..0adee9bdc2 100644 --- a/src/memdir/paths.test.ts +++ b/src/memdir/paths.test.ts @@ -1,9 +1,12 @@ import { afterEach, beforeEach, expect, test, mock } from 'bun:test' import { setAllowedSettingSources } from '../bootstrap/state.js' import { SETTING_SOURCES } from '../utils/settings/constants.js' -import * as realSettings from '../utils/settings/settings.js' import { isAutoMemoryEnabled } from './paths.ts' +const realSettings = (await import( + `../utils/settings/settings.js?pathsTestReal=${Date.now()}-${Math.random()}` +)) as typeof import('../utils/settings/settings.js') + // Pin issue #1326: `memory.autoWrite` is a discoverable alias for the legacy // `autoMemoryEnabled` setting, and either key opts out for governance / // regulated / client-sensitive repos. The opt-out is evaluated across the raw diff --git a/src/memdir/teamMemPrompts.ts b/src/memdir/teamMemPrompts.ts index de5ea8465a..9b7181aa56 100644 --- a/src/memdir/teamMemPrompts.ts +++ b/src/memdir/teamMemPrompts.ts @@ -13,6 +13,7 @@ import { } from './memoryTypes.js' import { getAutoMemPath } from './paths.js' import { getTeamMemPath } from './teamMemPaths.js' +import { isMemoryWriteApprovalRequired } from '../utils/governancePolicy.js' /** * Build the combined prompt when both auto memory and team memory are enabled. @@ -25,6 +26,10 @@ export function buildCombinedMemoryPrompt( ): string { const autoDir = getAutoMemPath() const teamDir = getTeamMemPath() + const requiresApproval = isMemoryWriteApprovalRequired() + const dirGuidance = requiresApproval + ? 'Do not create or update files there until the user explicitly approves the specific memory write.' + : DIRS_EXIST_GUIDANCE const howToSave = skipIndex ? [ @@ -60,10 +65,16 @@ export function buildCombinedMemoryPrompt( const lines = [ '# Memory', '', - `You have a persistent, file-based memory system with two directories: a private directory at \`${autoDir}\` and a shared team directory at \`${teamDir}\`. ${DIRS_EXIST_GUIDANCE}`, + `You have a persistent, file-based memory system with two directories: a private directory at \`${autoDir}\` and a shared team directory at \`${teamDir}\`. ${dirGuidance}`, '', "You should build up this memory system over time so that future conversations can have a complete picture of who the user is, how they'd like to collaborate with you, what behaviors to avoid or repeat, and the context behind the work the user gives you.", '', + ...(requiresApproval + ? [ + 'Before creating, updating, or deleting persistent memory files, explicitly ask the user for approval and wait for confirmation.', + '', + ] + : []), 'If the user explicitly asks you to remember something, save it immediately as whichever type fits best. If they ask you to forget something, find and remove the relevant entry.', '', '## Memory scope', diff --git a/src/services/autoDream/autoDream.ts b/src/services/autoDream/autoDream.ts index b9c09082c2..0cc27b0ce0 100644 --- a/src/services/autoDream/autoDream.ts +++ b/src/services/autoDream/autoDream.ts @@ -25,6 +25,7 @@ import type { ToolUseContext } from '../../Tool.js' import { logEvent } from '../analytics/index.js' import { getFeatureValue_CACHED_MAY_BE_STALE } from '../analytics/growthbook.js' import { isAutoMemoryEnabled, getAutoMemPath } from '../../memdir/paths.js' +import { isMemoryWriteApprovalRequired } from '../../utils/governancePolicy.js' import { isAutoDreamEnabled } from './config.js' import { getProjectDir } from '../../utils/sessionStorage.js' import { @@ -96,6 +97,7 @@ function isGateOpen(): boolean { if (getKairosActive()) return false // KAIROS mode uses disk-skill dream if (getIsRemoteMode()) return false if (!isAutoMemoryEnabled()) return false + if (isMemoryWriteApprovalRequired()) return false return isAutoDreamEnabled() } diff --git a/src/services/extractMemories/extractMemories.ts b/src/services/extractMemories/extractMemories.ts index 708d47d71c..b878d32780 100644 --- a/src/services/extractMemories/extractMemories.ts +++ b/src/services/extractMemories/extractMemories.ts @@ -43,6 +43,7 @@ import type { } from '../../types/message.js' import { createAbortController } from '../../utils/abortController.js' import { count, uniq } from '../../utils/array.js' +import { isMemoryWriteApprovalRequired } from '../../utils/governancePolicy.js' import { logForDebugging } from '../../utils/debug.js' import { createCacheSafeParams, @@ -546,6 +547,10 @@ export function initExtractMemories(): void { return } + if (isMemoryWriteApprovalRequired()) { + return + } + // Skip in remote mode if (getIsRemoteMode()) { return diff --git a/src/tools/BashTool/bashPermissions.test.ts b/src/tools/BashTool/bashPermissions.test.ts index d649a8b728..7203b1f44f 100644 --- a/src/tools/BashTool/bashPermissions.test.ts +++ b/src/tools/BashTool/bashPermissions.test.ts @@ -1,10 +1,20 @@ import { afterEach, beforeEach, describe, expect, test } from 'bun:test' +import { mkdir, mkdtemp, rm, writeFile } from 'fs/promises' +import { tmpdir } from 'os' +import { join } from 'path' import { getEmptyToolPermissionContext } from '../../Tool.js' +import { + getOriginalCwd, + setAllowedSettingSources, + setOriginalCwd, +} from '../../bootstrap/state.js' import { acquireSharedMutationLock, releaseSharedMutationLock, } from '../../test/sharedMutationLock.js' +import { resetSettingsCache } from '../../utils/settings/settingsCache.js' +import { SETTING_SOURCES } from '../../utils/settings/constants.js' import { SandboxManager } from '../../utils/sandbox/sandbox-adapter.js' import { _test, @@ -183,6 +193,127 @@ test('checkSandboxAutoAllow caps fanout when astSubcommands is null', () => { }) }) +test('git commit governance policy runs through the production Bash permission path', async () => { + const originalCwd = getOriginalCwd() + const projectDir = await mkdtemp(join(tmpdir(), 'openclaude-git-policy-')) + + try { + setOriginalCwd(projectDir) + setAllowedSettingSources([...SETTING_SOURCES]) + await mkdir(join(projectDir, '.openclaude'), { recursive: true }) + await writeFile( + join(projectDir, '.openclaude', 'settings.local.json'), + JSON.stringify({ + git: { forbiddenCommitMessagePatterns: ['Generated with'] }, + }), + ) + resetSettingsCache() + + const result = await bashToolHasPermission( + { command: 'git commit -m "fix: policy\n\nGenerated with OpenClaude"' }, + makeToolUseContext(), + ) + const compoundResult = await bashToolHasPermission( + { + command: + 'cd repo && git commit -m "fix: policy\n\nGenerated with OpenClaude"', + }, + makeToolUseContext(), + ) + const safeCommitThenEchoResult = await bashToolHasPermission( + { + command: + 'git commit -m "safe" && echo -m "Generated with OpenClaude"', + }, + makeToolUseContext(), + ) + const commandWrappedResult = await bashToolHasPermission( + { + command: + 'command git commit -m "fix: policy\n\nGenerated with OpenClaude"', + }, + makeToolUseContext(), + ) + const commandPathWrappedResult = await bashToolHasPermission( + { + command: + 'command -p git commit -m "fix: policy\n\nGenerated with OpenClaude"', + }, + makeToolUseContext(), + ) + const envWrappedResult = await bashToolHasPermission( + { + command: + 'env git commit -m "fix: policy\n\nGenerated with OpenClaude"', + }, + makeToolUseContext(), + ) + const envSplitStringWrappedResult = await bashToolHasPermission( + { + command: + 'env -S \'git commit -m "fix: policy\n\nGenerated with OpenClaude"\'', + }, + makeToolUseContext(), + ) + const envSplitStringAssignmentWrappedResult = await bashToolHasPermission( + { + command: + 'env -S \'GIT_AUTHOR_NAME=bot git commit -m "fix: policy\n\nGenerated with OpenClaude"\'', + }, + makeToolUseContext(), + ) + + expect(result.behavior).toBe('ask') + expect(compoundResult.behavior).toBe('ask') + expect(safeCommitThenEchoResult.behavior).not.toBe('ask') + expect(commandWrappedResult.behavior).toBe('ask') + expect(commandPathWrappedResult.behavior).toBe('ask') + expect(envWrappedResult.behavior).toBe('ask') + expect(envSplitStringWrappedResult.behavior).toBe('ask') + expect(envSplitStringAssignmentWrappedResult.behavior).toBe('ask') + expect(result.decisionReason).toMatchObject({ + type: 'safetyCheck', + reason: 'Git commit message contains forbidden pattern: Generated with', + classifierApprovable: false, + }) + expect(compoundResult.decisionReason).toMatchObject({ + type: 'safetyCheck', + reason: 'Git commit message contains forbidden pattern: Generated with', + classifierApprovable: false, + }) + expect(commandWrappedResult.decisionReason).toMatchObject({ + type: 'safetyCheck', + reason: 'Git commit message contains forbidden pattern: Generated with', + classifierApprovable: false, + }) + expect(commandPathWrappedResult.decisionReason).toMatchObject({ + type: 'safetyCheck', + reason: 'Git commit message contains forbidden pattern: Generated with', + classifierApprovable: false, + }) + expect(envWrappedResult.decisionReason).toMatchObject({ + type: 'safetyCheck', + reason: 'Git commit message contains forbidden pattern: Generated with', + classifierApprovable: false, + }) + expect(envSplitStringWrappedResult.decisionReason).toMatchObject({ + type: 'safetyCheck', + reason: 'Git commit message contains forbidden pattern: Generated with', + classifierApprovable: false, + }) + expect(envSplitStringAssignmentWrappedResult.decisionReason).toMatchObject({ + type: 'safetyCheck', + reason: 'Git commit message contains forbidden pattern: Generated with', + classifierApprovable: false, + }) + } finally { + setOriginalCwd(originalCwd) + setAllowedSettingSources([...SETTING_SOURCES]) + resetSettingsCache() + await rm(projectDir, { recursive: true, force: true }) + } +}) + // SEC-02 regression: array subscript with command substitution must NOT be stripped. // Bash executes FOO[$(cmd)]=val as a side effect; if the pattern matched the // subscript, the env-var prefix would be stripped while $(cmd) silently ran. diff --git a/src/tools/BashTool/bashPermissions.ts b/src/tools/BashTool/bashPermissions.ts index e88b8d50b8..ac90feaa13 100644 --- a/src/tools/BashTool/bashPermissions.ts +++ b/src/tools/BashTool/bashPermissions.ts @@ -71,6 +71,7 @@ import { BashTool } from './BashTool.js' import { checkCommandOperatorPermissions } from './bashCommandHelpers.js' import { bashCommandIsSafeAsync_DEPRECATED, + checkBashCommitMessagePolicy, stripSafeHeredocSubstitutions, } from './bashSecurity.js' import { checkPermissionMode } from './modeValidation.js' @@ -1708,6 +1709,26 @@ export async function bashToolHasPermission( ): Promise { let appState = context.getAppState() + const commitPolicyResult = checkBashCommitMessagePolicy(input.command) + if (commitPolicyResult) { + if ( + commitPolicyResult.behavior !== 'ask' && + commitPolicyResult.behavior !== 'deny' + ) { + return commitPolicyResult + } + return { + ...commitPolicyResult, + message: createPermissionRequestMessage( + BashTool.name, + commitPolicyResult.decisionReason ?? { + type: 'other', + reason: 'Git commit policy violation', + }, + ), + } + } + // 0. AST-based security parse. This replaces both tryParseShellCommand // (the shell-quote pre-check) and the bashCommandIsSafe misparsing gate. // tree-sitter produces either a clean SimpleCommand[] (quotes resolved, @@ -2201,6 +2222,29 @@ export async function bashToolHasPermission( cwdMingw, ) + for (const subcommand of subcommands) { + const subcommandCommitPolicyResult = + checkBashCommitMessagePolicy(subcommand) + if (subcommandCommitPolicyResult) { + if ( + subcommandCommitPolicyResult.behavior !== 'ask' && + subcommandCommitPolicyResult.behavior !== 'deny' + ) { + return subcommandCommitPolicyResult + } + return { + ...subcommandCommitPolicyResult, + message: createPermissionRequestMessage( + BashTool.name, + subcommandCommitPolicyResult.decisionReason ?? { + type: 'other', + reason: 'Git commit policy violation', + }, + ), + } + } + } + // CC-643: Cap subcommand fanout. Only the legacy splitCommand path can // explode — the AST path returns a bounded list (astSubcommands !== null) // or short-circuits to 'too-complex' for structures it can't represent. diff --git a/src/tools/BashTool/bashSecurity.test.ts b/src/tools/BashTool/bashSecurity.test.ts index 37bd5fecb4..8f156bd14c 100644 --- a/src/tools/BashTool/bashSecurity.test.ts +++ b/src/tools/BashTool/bashSecurity.test.ts @@ -1,10 +1,56 @@ import { describe, expect, test } from 'bun:test' +import { mkdir, mkdtemp, rm, writeFile } from 'fs/promises' +import { tmpdir } from 'os' +import { join } from 'path' +import { + getOriginalCwd, + setAllowedSettingSources, + setOriginalCwd, +} from '../../bootstrap/state.js' +import { SETTING_SOURCES } from '../../utils/settings/constants.js' +import { resetSettingsCache } from '../../utils/settings/settingsCache.js' import { bashCommandIsSafe_DEPRECATED, stripSafeHeredocSubstitutions, } from './bashSecurity.js' +async function withProjectSettings( + settings: Record, + fn: () => void, +): Promise { + const originalCwd = getOriginalCwd() + const projectDir = await mkdtemp(join(tmpdir(), 'openclaude-bash-security-')) + + try { + setOriginalCwd(projectDir) + setAllowedSettingSources([...SETTING_SOURCES]) + await mkdir(join(projectDir, '.openclaude'), { recursive: true }) + await writeFile( + join(projectDir, '.openclaude', 'settings.local.json'), + JSON.stringify(settings), + ) + resetSettingsCache() + fn() + } finally { + setOriginalCwd(originalCwd) + setAllowedSettingSources([...SETTING_SOURCES]) + resetSettingsCache() + await rm(projectDir, { recursive: true, force: true }) + } +} + +function expectAskMessage( + result: ReturnType, + text: string, +): void { + expect(result.behavior).toBe('ask') + if (result.behavior !== 'ask') { + throw new Error(`Expected ask result, got ${result.behavior}`) + } + expect(result.message).toContain(text) +} + describe('stripSafeHeredocSubstitutions', () => { test('strips a single safe heredoc substitution', () => { const cmd = "git commit -m $(cat <<'EOF'\nfix: whatever\nEOF\n)" @@ -73,3 +119,243 @@ describe('validateZshDangerousCommands: fc -e detection (#1051 BUG-01)', () => { expect(result.behavior).not.toBe('ask') }) }) + +describe('git commit governance policy (#1326)', () => { + test('asks when a simple commit message contains a forbidden pattern', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Co-Authored-By:'] } }, + () => { + const result = bashCommandIsSafe_DEPRECATED( + 'git commit -m "fix: policy\n\nCo-Authored-By: OpenClaude "', + ) + + expectAskMessage(result, 'Co-Authored-By:') + }, + ) + }) + + test('asks when a heredoc commit message contains disabled AI attribution', async () => { + await withProjectSettings( + { git: { addAICoAuthor: false } }, + () => { + const result = bashCommandIsSafe_DEPRECATED( + "git commit -m \"$(cat <<'EOF'\nfix: policy\n\nGenerated with OpenClaude\nEOF\n)\"", + ) + + expectAskMessage(result, 'AI attribution') + }, + ) + }) + + test('checks commit messages when git global options precede commit', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Generated with'] } }, + () => { + const result = bashCommandIsSafe_DEPRECATED( + 'git -C ./repo -c user.name=bot commit -m "fix: policy\n\nGenerated with OpenClaude"', + ) + + expectAskMessage(result, 'Generated with') + }, + ) + }) + + test('checks commit messages when safe env assignments precede git', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Generated with'] } }, + () => { + const result = bashCommandIsSafe_DEPRECATED( + 'GIT_AUTHOR_NAME=bot git commit -m "fix: policy\n\nGenerated with OpenClaude"', + ) + + expectAskMessage(result, 'Generated with') + }, + ) + }) + + test('checks commit messages when git executable is quoted, git.exe, or wrapped with command', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Generated with'] } }, + () => { + const doubleQuoted = bashCommandIsSafe_DEPRECATED( + '"git" commit -m "fix: policy\n\nGenerated with OpenClaude"', + ) + const singleQuoted = bashCommandIsSafe_DEPRECATED( + "'git' commit -m \"fix: policy\n\nGenerated with OpenClaude\"", + ) + const gitExe = bashCommandIsSafe_DEPRECATED( + 'git.exe commit -m "fix: policy\n\nGenerated with OpenClaude"', + ) + const commandWrapped = bashCommandIsSafe_DEPRECATED( + 'command git commit -m "fix: policy\n\nGenerated with OpenClaude"', + ) + const commandPathWrapped = bashCommandIsSafe_DEPRECATED( + 'command -p git commit -m "fix: policy\n\nGenerated with OpenClaude"', + ) + const commandEndOfOptionsWrapped = bashCommandIsSafe_DEPRECATED( + 'command -- git commit -m "fix: policy\n\nGenerated with OpenClaude"', + ) + + expect(doubleQuoted.behavior).toBe('ask') + expect(singleQuoted.behavior).toBe('ask') + expect(gitExe.behavior).toBe('ask') + expect(commandWrapped.behavior).toBe('ask') + expect(commandPathWrapped.behavior).toBe('ask') + expect(commandEndOfOptionsWrapped.behavior).toBe('ask') + }, + ) + }) + + test('checks commit messages when env wraps git', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Generated with'] } }, + () => { + const envWrapped = bashCommandIsSafe_DEPRECATED( + 'env git commit -m "fix: policy\n\nGenerated with OpenClaude"', + ) + const envAssignmentWrapped = bashCommandIsSafe_DEPRECATED( + 'env GIT_AUTHOR_NAME=bot git commit -m "fix: policy\n\nGenerated with OpenClaude"', + ) + const envOptionWrapped = bashCommandIsSafe_DEPRECATED( + 'env -i -u GIT_CONFIG_GLOBAL git commit -m "fix: policy\n\nGenerated with OpenClaude"', + ) + const envSplitStringWrapped = bashCommandIsSafe_DEPRECATED( + 'env -S \'git commit -m "fix: policy\n\nGenerated with OpenClaude"\'', + ) + const envInlineSplitStringWrapped = bashCommandIsSafe_DEPRECATED( + 'env --split-string="git commit -m \\"fix: policy\n\nGenerated with OpenClaude\\""', + ) + const envSplitStringAssignmentWrapped = bashCommandIsSafe_DEPRECATED( + 'env -S \'GIT_AUTHOR_NAME=bot git commit -m "fix: policy\n\nGenerated with OpenClaude"\'', + ) + const envSplitStringGlobalOptionWrapped = bashCommandIsSafe_DEPRECATED( + 'env -S \'git -c user.name=bot commit -m "fix: policy\n\nGenerated with OpenClaude"\'', + ) + + expect(envWrapped.behavior).toBe('ask') + expect(envAssignmentWrapped.behavior).toBe('ask') + expect(envOptionWrapped.behavior).toBe('ask') + expect(envSplitStringWrapped.behavior).toBe('ask') + expect(envInlineSplitStringWrapped.behavior).toBe('ask') + expect(envSplitStringAssignmentWrapped.behavior).toBe('ask') + expect(envSplitStringGlobalOptionWrapped.behavior).toBe('ask') + }, + ) + }) + + test('checks commit messages passed with long message options', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Generated'] } }, + () => { + const spaced = bashCommandIsSafe_DEPRECATED( + 'git commit --message "fix: policy\n\nGenerated with OpenClaude"', + ) + const equals = bashCommandIsSafe_DEPRECATED( + 'git commit --message="fix: policy\n\nGenerated with OpenClaude"', + ) + const unquoted = bashCommandIsSafe_DEPRECATED( + 'git commit --message=Generated', + ) + + expect(spaced.behavior).toBe('ask') + expect(equals.behavior).toBe('ask') + expect(unquoted.behavior).toBe('ask') + }, + ) + }) + + test('asks for file-backed commit messages when commit-message policy is active', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Generated with'] } }, + () => { + const result = bashCommandIsSafe_DEPRECATED( + 'git commit -F .git/OPENCLAUDE_COMMIT_MSG', + ) + + expectAskMessage(result, 'loaded from a file') + }, + ) + }) + + test('asks for uninspectable commit message sources when commit-message policy is active', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Generated with'] } }, + () => { + const editorDefault = bashCommandIsSafe_DEPRECATED('git commit') + const reuseShort = bashCommandIsSafe_DEPRECATED('git commit -C HEAD') + const reuseLong = bashCommandIsSafe_DEPRECATED( + 'git commit --reuse-message=HEAD', + ) + const reeditShort = bashCommandIsSafe_DEPRECATED('git commit -c HEAD') + const reeditLong = bashCommandIsSafe_DEPRECATED( + 'git commit --reedit-message HEAD', + ) + const editor = bashCommandIsSafe_DEPRECATED('git commit --amend') + + expectAskMessage(editorDefault, 'cannot be checked') + expectAskMessage(reuseShort, 'cannot be checked') + expectAskMessage(reuseLong, 'cannot be checked') + expectAskMessage(reeditShort, 'cannot be checked') + expectAskMessage(reeditLong, 'cannot be checked') + expectAskMessage(editor, 'cannot be checked') + }, + ) + }) + + test('asks for expandable commit messages when commit-message policy is active', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Generated with'] } }, + () => { + const envVar = bashCommandIsSafe_DEPRECATED('git commit -m "$MSG"') + const commandSubstitution = bashCommandIsSafe_DEPRECATED( + 'git commit --message="$(cat .git/OPENCLAUDE_COMMIT_MSG)"', + ) + const backtick = bashCommandIsSafe_DEPRECATED( + 'git commit -m "`cat .git/OPENCLAUDE_COMMIT_MSG`"', + ) + const unquoted = bashCommandIsSafe_DEPRECATED( + 'git commit -m fix$VAR', + ) + const unquotedCommandSubstitution = bashCommandIsSafe_DEPRECATED( + 'git commit -m $(cat .git/OPENCLAUDE_COMMIT_MSG)', + ) + + expectAskMessage(envVar, 'cannot be checked') + expectAskMessage(commandSubstitution, 'cannot be checked') + expectAskMessage(backtick, 'cannot be checked') + expectAskMessage(unquoted, 'cannot be checked') + expectAskMessage(unquotedCommandSubstitution, 'cannot be checked') + }, + ) + }) + + test('asks for expandable heredoc commit messages when commit-message policy is active', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Generated with'] } }, + () => { + const unquotedHeredoc = bashCommandIsSafe_DEPRECATED( + 'git commit -m $(cat < { + await withProjectSettings( + { git: { addGeneratedWithFooter: false } }, + () => { + const result = bashCommandIsSafe_DEPRECATED( + 'git commit -m "fix: policy\n\nCo-Authored-By: OpenClaude "', + ) + + expect(result.behavior).toBe('passthrough') + }, + ) + }) +}) diff --git a/src/tools/BashTool/bashSecurity.ts b/src/tools/BashTool/bashSecurity.ts index fd368dda05..0875f9518d 100644 --- a/src/tools/BashTool/bashSecurity.ts +++ b/src/tools/BashTool/bashSecurity.ts @@ -1,6 +1,11 @@ import { logEvent } from 'src/services/analytics/index.js' import { extractHeredocs } from '../../utils/bash/heredoc.js' import { ParsedCommand } from '../../utils/bash/ParsedCommand.js' +import { + findForbiddenCommitMessagePattern, + getForbiddenCommitMessagePatterns, + isGeneratedCommitAttributionBlocked, +} from '../../utils/governancePolicy.js' import { hasMalformedTokens, hasShellQuoteSingleQuoteBug, @@ -625,6 +630,313 @@ function validateSafeCommandSubstitution( } } +function takeShellToken(input: string): string | null { + const match = input.match(/^(?:"[^"\n\r]*"|'[^'\n\r]*'|[^ \t\n\r;&|`$<>()]+)/) + return match?.[0] ?? null +} + +function stripLeadingSafeEnvAssignments(command: string): string { + let rest = command.trimStart() + for (;;) { + const match = rest.match( + /^[A-Za-z_][A-Za-z0-9_]*(?:\+)?=(?:"[^"$`\n\r]*"|'[^'\n\r]*'|[^ \t\n\r;&|`$<>()]*)[ \t]+/, + ) + if (!match) return rest + rest = rest.slice(match[0].length) + } +} + +function unquoteEnvSplitString(value: string): string { + if ( + (value.startsWith('"') && value.endsWith('"')) || + (value.startsWith("'") && value.endsWith("'")) + ) { + const inner = value.slice(1, -1) + return value[0] === '"' + ? inner.replace(/\\(["\\$`])/g, '$1') + : inner + } + return value +} + +function stripLeadingGitExecutionWrappers(command: string, depth = 0): string { + let rest = stripLeadingSafeEnvAssignments(command) + + const commandMatch = rest.match(/^command[ \t]+/) + if (commandMatch) { + rest = rest.slice(commandMatch[0].length) + for (;;) { + const option = takeShellToken(rest) + if (option === '-p' || option === '--') { + rest = rest.slice(option.length).replace(/^[ \t]+/, '') + continue + } + break + } + return rest + } + + const envMatch = rest.match(/^env[ \t]+/) + if (envMatch) { + rest = rest.slice(envMatch[0].length) + for (;;) { + const splitStringMatch = rest.match( + /^(?:-S|--split-string)[ \t]+((?:"(?:\\.|[^"\\])*"|'[^']*'|[^ \t]+))/, + ) + const inlineSplitStringMatch = rest.match( + /^--split-string=((?:"(?:\\.|[^"\\])*"|'[^']*'|[^ \t]+))/, + ) + if (splitStringMatch ?? inlineSplitStringMatch) { + const splitCommand = unquoteEnvSplitString( + (splitStringMatch ?? inlineSplitStringMatch)![1]!, + ).trimStart() + return depth < 2 + ? stripLeadingGitExecutionWrappers(splitCommand, depth + 1) + : splitCommand + } + + const token = takeShellToken(rest) + if (!token) return rest + if (/^[A-Za-z_][A-Za-z0-9_]*(?:\+)?=/.test(token)) { + rest = rest.slice(token.length).replace(/^[ \t]+/, '') + continue + } + if ( + token === '-i' || + token === '--ignore-environment' || + token === '--' + ) { + rest = rest.slice(token.length).replace(/^[ \t]+/, '') + continue + } + if (token === '-u' || token === '--unset' || token === '-C' || token === '--chdir') { + rest = rest.slice(token.length).replace(/^[ \t]+/, '') + const value = takeShellToken(rest) + if (!value) return rest + rest = rest.slice(value.length).replace(/^[ \t]+/, '') + continue + } + if ( + token.startsWith('-u') || + token.startsWith('--unset=') || + token.startsWith('-C') || + token.startsWith('--chdir=') + ) { + rest = rest.slice(token.length).replace(/^[ \t]+/, '') + continue + } + break + } + } + + return rest +} + +function hasShellOperatorOutsideQuotes(command: string): boolean { + let quote: '"' | "'" | null = null + for (let i = 0; i < command.length; i++) { + const char = command[i] + if (quote) { + if (char === quote) quote = null + continue + } + if (char === '"' || char === "'") { + quote = char + continue + } + if (char === ';' || char === '|' || char === '&') { + return true + } + } + return false +} + +function normalizeGitCommitCommand(command: string): string | null { + if (hasShellOperatorOutsideQuotes(command)) return null + const normalizedInput = stripLeadingGitExecutionWrappers(command) + const gitMatch = normalizedInput.match(/^(?:"git(?:\.exe)?"|'git(?:\.exe)?'|git(?:\.exe)?)[ \t]+/) + if (!gitMatch) return null + + let rest = normalizedInput.slice(gitMatch[0].length) + for (;;) { + const commitMatch = rest.match(/^commit(?=$|[ \t])/) + if (commitMatch) { + return `git commit${rest.slice(commitMatch[0].length)}` + } + + const flag = takeShellToken(rest) + if (!flag?.startsWith('-')) return null + rest = rest.slice(flag.length).replace(/^[ \t]+/, '') + + const hasInlineValue = flag.includes('=') + const needsValue = + flag === '-C' || + flag === '-c' || + [ + '--git-dir', + '--work-tree', + '--namespace', + '--super-prefix', + '--config-env', + '--exec-path', + ].includes(flag) + + if (needsValue && !hasInlineValue) { + const value = takeShellToken(rest) + if (!value) return null + rest = rest.slice(value.length).replace(/^[ \t]+/, '') + } + } +} + +function hasExpandableShellText(message: string): boolean { + return /(^|[^\\])(?:\$[\w{(]|`)/.test(message) +} + +function extractGitCommitMessages(command: string): { + messages: string[] + hasUninspectableSource: boolean +} { + const normalizedCommand = normalizeGitCommitCommand(command) + if (!normalizedCommand) { + return { messages: [], hasUninspectableSource: false } + } + + const messages: string[] = [] + let hasUninspectableSource = false + const simpleMessagePattern = + /(?:^|[ \t])(?:-m|--message)[ \t]+(["'])([\s\S]*?)\1|(?:^|[ \t])--message=(["'])([\s\S]*?)\3/g + for (const match of normalizedCommand.matchAll(simpleMessagePattern)) { + const quote = match[1] ?? match[3] + const message = match[2] ?? match[4] ?? '' + if ( + quote === '"' && + hasExpandableShellText(message) && + !/^\$\(cat[ \t]+<()]/.test(message)) { + hasUninspectableSource = true + continue + } + messages.push(message) + } + + const heredocMessagePattern = + /(?:^|[ \t])(?:-m|--message)[ \t]+(["']?)\$\(cat[ \t]+<<(['\\]?)([A-Za-z_][A-Za-z0-9_-]*)\2\n([\s\S]*?)\n\3\n?\)\1|(?:^|[ \t])--message=(["']?)\$\(cat[ \t]+<<(['\\]?)([A-Za-z_][A-Za-z0-9_-]*)\6\n([\s\S]*?)\n\7\n?\)\5/g + for (const match of normalizedCommand.matchAll(heredocMessagePattern)) { + const delimiterQuote = match[2] ?? match[6] ?? '' + const message = match[4] ?? match[8] ?? '' + if (delimiterQuote !== "'" && delimiterQuote !== '\\') { + hasUninspectableSource = true + continue + } + messages.push(message) + } + + return { messages, hasUninspectableSource } +} + +function gitCommitUsesFileMessage(command: string): boolean { + const normalizedCommand = normalizeGitCommitCommand(command) + if (!normalizedCommand) return false + + return /(?:^|[ \t])(?:-F|--file)[ \t]+(?:"[^"\n\r]*"|'[^'\n\r]*'|[^ \t\n\r;&|`$<>()]+)|(?:^|[ \t])--file=(?:"[^"\n\r]*"|'[^'\n\r]*'|[^ \t\n\r;&|`$<>()]+)/.test( + normalizedCommand, + ) +} + +function hasCommitMessagePolicyRestrictions(): boolean { + return ( + getForbiddenCommitMessagePatterns().length > 0 || + isGeneratedCommitAttributionBlocked() + ) +} + +function commitPolicyAsk(message: string): PermissionResult { + return { + behavior: 'ask', + message, + decisionReason: { + type: 'safetyCheck', + reason: message, + classifierApprovable: false, + }, + } +} + +export function checkBashCommitMessagePolicy( + command: string, +): PermissionResult | null { + const normalizedCommand = normalizeGitCommitCommand(command) + if (!normalizedCommand) return null + + const hasPolicyRestrictions = hasCommitMessagePolicyRestrictions() + + if (gitCommitUsesFileMessage(command) && hasPolicyRestrictions) { + return commitPolicyAsk( + 'Git commit message is loaded from a file and cannot be checked against commit-message policy', + ) + } + + const { messages, hasUninspectableSource } = extractGitCommitMessages(command) + if ( + (hasUninspectableSource || messages.length === 0) && + hasPolicyRestrictions + ) { + return commitPolicyAsk( + 'Git commit message source cannot be checked against commit-message policy', + ) + } + + for (const message of messages) { + const forbiddenPattern = findForbiddenCommitMessagePattern(message) + if (forbiddenPattern) { + return commitPolicyAsk( + `Git commit message contains forbidden pattern: ${forbiddenPattern}`, + ) + } + if ( + isGeneratedCommitAttributionBlocked() && + /Co-Authored-By:|Generated with/i.test(message) + ) { + return commitPolicyAsk( + 'Git commit message contains AI attribution that is disabled by policy', + ) + } + } + + return null +} + +function validateGitCommitMessagePolicy( + context: ValidationContext, +): PermissionResult { + const result = checkBashCommitMessagePolicy(context.originalCommand) + if (result) return result + if (!normalizeGitCommitCommand(context.originalCommand)) { + return { behavior: 'passthrough', message: 'Not a git commit' } + } + return { behavior: 'passthrough', message: 'No commit policy violation' } +} + function validateGitCommit(context: ValidationContext): PermissionResult { const { originalCommand, baseCommand } = context @@ -2328,6 +2640,7 @@ export function bashCommandIsSafe_DEPRECATED( const earlyValidators = [ validateEmpty, validateIncompleteCommands, + validateGitCommitMessagePolicy, validateSafeCommandSubstitution, validateGitCommit, ] @@ -2538,6 +2851,7 @@ export async function bashCommandIsSafeAsync_DEPRECATED( const earlyValidators = [ validateEmpty, validateIncompleteCommands, + validateGitCommitMessagePolicy, validateSafeCommandSubstitution, validateGitCommit, ] diff --git a/src/tools/PowerShellTool/PowerShellTool.tsx b/src/tools/PowerShellTool/PowerShellTool.tsx index 55cdd033fc..71ea2a537f 100644 --- a/src/tools/PowerShellTool/PowerShellTool.tsx +++ b/src/tools/PowerShellTool/PowerShellTool.tsx @@ -17,6 +17,7 @@ import { extractClaudeCodeHints } from '../../utils/claudeCodeHints.js'; import { isEnvTruthy } from '../../utils/envUtils.js'; import { errorMessage as getErrorMessage, ShellError } from '../../utils/errors.js'; import { truncate } from '../../utils/format.js'; +import { findForbiddenCommitMessagePattern, getForbiddenCommitMessagePatterns, isGeneratedCommitAttributionBlocked } from '../../utils/governancePolicy.js'; import { lazySchema } from '../../utils/lazySchema.js'; import { logError } from '../../utils/log.js'; import type { PermissionResult } from '../../utils/permissions/PermissionResult.js'; @@ -204,6 +205,206 @@ export function detectBlockedSleepPattern(command: string): string | null { return rest ? `Start-Sleep ${secs} followed by: ${rest}` : `standalone Start-Sleep ${secs}`; } +function takePowerShellToken(input: string): string | null { + const match = input.match(/^(?:"[^"\r\n]*"|'[^'\r\n]*'|[^\s;|&()]+)/); + return match?.[0] ?? null; +} + +function normalizePowerShellGitCommitCommand(command: string): string | null { + const normalizedInput = command + .trimStart() + .replace(/^&\s*(?=(?:"git(?:\.exe)?"|'git(?:\.exe)?'|git(?:\.exe)?\s+))/i, ''); + const gitMatch = normalizedInput.match(/^(?:"git(?:\.exe)?"|'git(?:\.exe)?'|git(?:\.exe)?)\s+/i); + if (!gitMatch) return null; + + let rest = normalizedInput.slice(gitMatch[0].length); + for (;;) { + const commitMatch = rest.match(/^commit(?=$|\s)/i); + if (commitMatch) { + return `git commit${rest.slice(commitMatch[0].length)}`; + } + + const flag = takePowerShellToken(rest); + if (!flag?.startsWith('-')) return null; + rest = rest.slice(flag.length).replace(/^\s+/, ''); + + const hasInlineValue = flag.includes('='); + const needsValue = + flag.toLowerCase() === '-c' || + [ + '--git-dir', + '--work-tree', + '--namespace', + '--super-prefix', + '--config-env', + '--exec-path' + ].includes(flag.toLowerCase()); + + if (needsValue && !hasInlineValue) { + const value = takePowerShellToken(rest); + if (!value) return null; + rest = rest.slice(value.length).replace(/^\s+/, ''); + } + } +} + +function hasExpandablePowerShellText(message: string): boolean { + return /(^|[^`])\$\w|(^|[^`])\$\{[^}]+\}|(^|[^`])\$\(/.test(message); +} + +function extractPowerShellGitCommitMessages(command: string): { + messages: string[]; + hasUninspectableSource: boolean; +} { + const normalizedCommand = normalizePowerShellGitCommitCommand(command); + if (!normalizedCommand) return { messages: [], hasUninspectableSource: false }; + + const messages: string[] = []; + let hasUninspectableSource = false; + const quotedPattern = /(?:^|\s)(?:-m|--message)\s+(["'])([\s\S]*?)\1|(?:^|\s)--message=(["'])([\s\S]*?)\3/g; + for (const match of normalizedCommand.matchAll(quotedPattern)) { + const quote = match[1] ?? match[3]; + const message = match[2] ?? match[4] ?? ''; + if (quote === '"' && hasExpandablePowerShellText(message)) { + hasUninspectableSource = true; + continue; + } + messages.push(message); + } + + const unquotedPattern = /(?:^|\s)(?:-m|--message)\s+([^"'\s;|&()]+)|(?:^|\s)--message=([^"'\s;|&()]+)/g; + for (const match of normalizedCommand.matchAll(unquotedPattern)) { + const message = match[1] ?? match[2] ?? ''; + if (hasExpandablePowerShellText(message)) { + hasUninspectableSource = true; + continue; + } + messages.push(message); + } + + const hereStringPattern = /(?:^|\s)(?:-m|--message)\s+@'\r?\n([\s\S]*?)\r?\n'@|(?:^|\s)--message=@'\r?\n([\s\S]*?)\r?\n'@/g; + for (const match of normalizedCommand.matchAll(hereStringPattern)) { + messages.push(match[1] ?? match[2] ?? ''); + } + + const expandableHereStringPattern = /(?:^|\s)(?:-m|--message)\s+@"\r?\n([\s\S]*?)\r?\n"@|(?:^|\s)--message=@"\r?\n([\s\S]*?)\r?\n"@/g; + for (const match of normalizedCommand.matchAll(expandableHereStringPattern)) { + const message = match[1] ?? match[2] ?? ''; + if (hasExpandablePowerShellText(message)) { + hasUninspectableSource = true; + continue; + } + messages.push(message); + } + + return { messages, hasUninspectableSource }; +} + +function powerShellGitCommitUsesFileMessage(command: string): boolean { + const normalizedCommand = normalizePowerShellGitCommitCommand(command); + if (!normalizedCommand) return false; + + return /(?:^|\s)(?:-F|--file)\s+(?:"[^"\r\n]*"|'[^'\r\n]*'|[^\s;|&()]+)|(?:^|\s)--file=(?:"[^"\r\n]*"|'[^'\r\n]*'|[^\s;|&()]+)/i.test(normalizedCommand); +} + +function hasCommitMessagePolicyRestrictions(): boolean { + return getForbiddenCommitMessagePatterns().length > 0 || isGeneratedCommitAttributionBlocked(); +} + +function commitPolicyAsk(message: string): PermissionResult { + return { + behavior: 'ask', + message, + decisionReason: { + type: 'safetyCheck', + reason: message, + classifierApprovable: false + } + }; +} + +function splitPowerShellStatements(command: string): string[] { + const statements: string[] = []; + let start = 0; + let quote: '"' | "'" | null = null; + let hereStringTerminator: '"@' | "'@" | null = null; + + for (let i = 0; i < command.length; i++) { + if (hereStringTerminator) { + if ( + command.startsWith(hereStringTerminator, i) && + (i === 0 || command[i - 1] === '\n' || command[i - 1] === '\r') + ) { + i += hereStringTerminator.length - 1; + hereStringTerminator = null; + } + continue; + } + + const char = command[i]; + const next = command[i + 1]; + if (!quote && char === '@' && (next === "'" || next === '"')) { + hereStringTerminator = next === "'" ? "'@" : '"@'; + i++; + continue; + } + if (quote) { + if (char === quote) quote = null; + continue; + } + if (char === '"' || char === "'") { + quote = char; + continue; + } + if (char === ';' || char === '|' || char === '\n' || char === '\r' || (char === '&' && next === '&')) { + const statement = command.slice(start, i).trim(); + if (statement) statements.push(statement); + if (char === '&') i++; + if (char === '\r' && next === '\n') i++; + start = i + 1; + } + } + + const statement = command.slice(start).trim(); + if (statement) statements.push(statement); + return statements.length > 0 ? statements : [command]; +} + +function checkPowerShellCommitMessagePolicyForStatement(command: string): PermissionResult | null { + const normalizedCommand = normalizePowerShellGitCommitCommand(command); + if (!normalizedCommand) return null; + + const hasPolicyRestrictions = hasCommitMessagePolicyRestrictions(); + + if (powerShellGitCommitUsesFileMessage(command) && hasPolicyRestrictions) { + return commitPolicyAsk('Git commit message is loaded from a file and cannot be checked against commit-message policy'); + } + + const { messages, hasUninspectableSource } = extractPowerShellGitCommitMessages(command); + if ((hasUninspectableSource || messages.length === 0) && hasPolicyRestrictions) { + return commitPolicyAsk('Git commit message source cannot be checked against commit-message policy'); + } + + for (const message of messages) { + const forbiddenPattern = findForbiddenCommitMessagePattern(message); + if (forbiddenPattern) { + return commitPolicyAsk(`Git commit message contains forbidden pattern: ${forbiddenPattern}`); + } + if (isGeneratedCommitAttributionBlocked() && /Co-Authored-By:|Generated with/i.test(message)) { + return commitPolicyAsk('Git commit message contains AI attribution that is disabled by policy'); + } + } + return null; +} + +export function checkPowerShellCommitMessagePolicy(command: string): PermissionResult | null { + for (const statement of splitPowerShellStatements(command)) { + const result = checkPowerShellCommitMessagePolicyForStatement(statement); + if (result) return result; + } + return null; +} + /** * On Windows native, sandbox is unavailable (bwrap/sandbox-exec are * POSIX-only). If enterprise policy has sandbox.enabled AND forbids @@ -380,6 +581,8 @@ export const PowerShellTool = buildTool({ }; }, async checkPermissions(input: PowerShellToolInput, context: Parameters[1]): Promise { + const policyResult = checkPowerShellCommitMessagePolicy(input.command); + if (policyResult) return policyResult; return await powershellToolHasPermission(input, context); }, renderToolUseMessage, diff --git a/src/tools/PowerShellTool/powershellPermissions.test.ts b/src/tools/PowerShellTool/powershellPermissions.test.ts index f133c74fae..7c2e7e1fcc 100644 --- a/src/tools/PowerShellTool/powershellPermissions.test.ts +++ b/src/tools/PowerShellTool/powershellPermissions.test.ts @@ -1,15 +1,34 @@ import { afterEach, beforeEach, describe, expect, test } from 'bun:test' -import { mkdtemp, mkdir, rm } from 'fs/promises' +import { mkdtemp, mkdir, rm, writeFile } from 'fs/promises' import { tmpdir } from 'os' import { join } from 'path' import { getOriginalCwd, + setAllowedSettingSources, setCwdState, setOriginalCwd, } from '../../bootstrap/state.js' import type { ToolPermissionContext } from '../../types/permissions.js' +import { SETTING_SOURCES } from '../../utils/settings/constants.js' +import { resetSettingsCache } from '../../utils/settings/settingsCache.js' import { isUnsafeDotGitWritePathForPowerShell } from './powershellPermissions.js' +type CheckPowerShellCommitMessagePolicy = + typeof import('./PowerShellTool.js')['checkPowerShellCommitMessagePolicy'] +type PowerShellCommitPolicyResult = + ReturnType + +function expectPowerShellAskMessage( + result: PowerShellCommitPolicyResult, + text: string, +): void { + expect(result?.behavior).toBe('ask') + if (!result || result.behavior !== 'ask') { + throw new Error(`Expected ask result, got ${result?.behavior ?? 'null'}`) + } + expect(result.message).toContain(text) +} + function permissionContext(mode: ToolPermissionContext['mode']) { return { mode, @@ -76,3 +95,237 @@ describe('PowerShell .git write safety', () => { ).toBe(true) }) }) + +describe('PowerShell git commit governance policy', () => { + let originalCwd: string + let projectDir: string + + async function withProjectSettings( + settings: Record, + fn: (checkPowerShellCommitMessagePolicy: CheckPowerShellCommitMessagePolicy) => void, + ): Promise { + originalCwd = getOriginalCwd() + projectDir = await mkdtemp(join(tmpdir(), 'openclaude-ps-policy-')) + try { + setOriginalCwd(projectDir) + setAllowedSettingSources([...SETTING_SOURCES]) + await mkdir(join(projectDir, '.openclaude'), { recursive: true }) + await writeFile( + join(projectDir, '.openclaude', 'settings.local.json'), + JSON.stringify(settings), + ) + resetSettingsCache() + const { checkPowerShellCommitMessagePolicy } = await import( + `./PowerShellTool.js?psPolicyTest=${Date.now()}-${Math.random()}` + ) + fn(checkPowerShellCommitMessagePolicy) + } finally { + setOriginalCwd(originalCwd) + setAllowedSettingSources([...SETTING_SOURCES]) + resetSettingsCache() + await rm(projectDir, { recursive: true, force: true }) + } + } + + test('returns a bypass-immune safety check for forbidden commit messages', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Generated with'] } }, + checkPowerShellCommitMessagePolicy => { + const result = checkPowerShellCommitMessagePolicy( + 'git -C ./repo commit -m "fix: policy\n\nGenerated with OpenClaude"', + ) + + expect(result?.behavior).toBe('ask') + expect(result?.decisionReason).toMatchObject({ + type: 'safetyCheck', + reason: + 'Git commit message contains forbidden pattern: Generated with', + }) + }, + ) + }) + + test('checks call-operator git commits', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Generated with'] } }, + checkPowerShellCommitMessagePolicy => { + const result = checkPowerShellCommitMessagePolicy( + '& git commit -m "fix: policy\n\nGenerated with OpenClaude"', + ) + + expectPowerShellAskMessage(result, 'Generated with') + }, + ) + }) + + test('checks quoted and exe PowerShell git invocations', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Generated with'] } }, + checkPowerShellCommitMessagePolicy => { + const quoted = checkPowerShellCommitMessagePolicy( + '& "git" commit -m "fix: policy\n\nGenerated with OpenClaude"', + ) + const exe = checkPowerShellCommitMessagePolicy( + 'git.exe commit -m "fix: policy\n\nGenerated with OpenClaude"', + ) + const quotedExe = checkPowerShellCommitMessagePolicy( + '& "git.exe" commit -m "fix: policy\n\nGenerated with OpenClaude"', + ) + const singleQuotedExe = checkPowerShellCommitMessagePolicy( + "& 'git.exe' commit -m \"fix: policy\n\nGenerated with OpenClaude\"", + ) + + expect(quoted?.behavior).toBe('ask') + expect(exe?.behavior).toBe('ask') + expect(quotedExe?.behavior).toBe('ask') + expect(singleQuotedExe?.behavior).toBe('ask') + }, + ) + }) + + test('checks git commits after earlier statements', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Generated with'] } }, + checkPowerShellCommitMessagePolicy => { + const result = checkPowerShellCommitMessagePolicy( + 'Set-Location repo; git commit -m "fix: policy\n\nGenerated with OpenClaude"', + ) + + expectPowerShellAskMessage(result, 'Generated with') + }, + ) + }) + + test('checks git commits after PowerShell chain operators', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Generated with'] } }, + checkPowerShellCommitMessagePolicy => { + const result = checkPowerShellCommitMessagePolicy( + 'Write-Output ok && git commit -m "fix: policy\n\nGenerated with OpenClaude"', + ) + + expectPowerShellAskMessage(result, 'Generated with') + }, + ) + }) + + test('checks git commits after pipeline segments', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Generated with'] } }, + checkPowerShellCommitMessagePolicy => { + const result = checkPowerShellCommitMessagePolicy( + 'Get-Content .git/OPENCLAUDE_COMMIT_MSG | git commit --file=-', + ) + + expectPowerShellAskMessage(result, 'loaded from a file') + }, + ) + }) + + test('checks commit messages passed with long message options', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Generated'] } }, + checkPowerShellCommitMessagePolicy => { + const spaced = checkPowerShellCommitMessagePolicy( + 'git commit --message "fix: policy\n\nGenerated with OpenClaude"', + ) + const equals = checkPowerShellCommitMessagePolicy( + 'git commit --message="fix: policy\n\nGenerated with OpenClaude"', + ) + const unquoted = checkPowerShellCommitMessagePolicy( + 'git commit --message=Generated', + ) + + expect(spaced?.behavior).toBe('ask') + expect(equals?.behavior).toBe('ask') + expect(unquoted?.behavior).toBe('ask') + }, + ) + }) + + test('asks for file-backed commit messages when commit-message policy is active', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Generated with'] } }, + checkPowerShellCommitMessagePolicy => { + const result = checkPowerShellCommitMessagePolicy( + 'git commit --file=.git/OPENCLAUDE_COMMIT_MSG', + ) + + expectPowerShellAskMessage(result, 'loaded from a file') + }, + ) + }) + + test('asks for uninspectable commit message sources when commit-message policy is active', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Generated with'] } }, + checkPowerShellCommitMessagePolicy => { + const editorDefault = checkPowerShellCommitMessagePolicy('git commit') + const reuseShort = checkPowerShellCommitMessagePolicy( + 'git commit -C HEAD', + ) + const reuseLong = checkPowerShellCommitMessagePolicy( + 'git commit --reuse-message=HEAD', + ) + const reeditShort = checkPowerShellCommitMessagePolicy( + 'git commit -c HEAD', + ) + const reeditLong = checkPowerShellCommitMessagePolicy( + 'git commit --reedit-message HEAD', + ) + const editor = checkPowerShellCommitMessagePolicy( + 'git commit --amend', + ) + + expectPowerShellAskMessage(editorDefault, 'cannot be checked') + expectPowerShellAskMessage(reuseShort, 'cannot be checked') + expectPowerShellAskMessage(reuseLong, 'cannot be checked') + expectPowerShellAskMessage(reeditShort, 'cannot be checked') + expectPowerShellAskMessage(reeditLong, 'cannot be checked') + expectPowerShellAskMessage(editor, 'cannot be checked') + }, + ) + }) + + test('asks for expandable commit messages when commit-message policy is active', async () => { + await withProjectSettings( + { git: { forbiddenCommitMessagePatterns: ['Generated with'] } }, + checkPowerShellCommitMessagePolicy => { + const variable = checkPowerShellCommitMessagePolicy( + 'git commit -m "$msg"', + ) + const subexpression = checkPowerShellCommitMessagePolicy( + 'git commit --message="$(Get-Content .git/OPENCLAUDE_COMMIT_MSG)"', + ) + const expandableHereString = checkPowerShellCommitMessagePolicy( + 'git commit -m @"\n$msg\n"@', + ) + const unquotedVariable = checkPowerShellCommitMessagePolicy( + 'git commit -m $msg', + ) + const literalHereString = checkPowerShellCommitMessagePolicy( + "git commit -m @'\nsafe literal\n'@", + ) + + expectPowerShellAskMessage(variable, 'cannot be checked') + expectPowerShellAskMessage(subexpression, 'cannot be checked') + expectPowerShellAskMessage(expandableHereString, 'cannot be checked') + expectPowerShellAskMessage(unquotedVariable, 'cannot be checked') + expect(literalHereString).toBeNull() + }, + ) + }) + + test('uses the commit-specific attribution blocker', async () => { + await withProjectSettings( + { git: { addGeneratedWithFooter: false } }, + checkPowerShellCommitMessagePolicy => { + const result = checkPowerShellCommitMessagePolicy( + 'git commit -m "fix: policy\n\nCo-Authored-By: OpenClaude "', + ) + + expect(result).toBeNull() + }, + ) + }) +}) diff --git a/src/utils/attribution.test.ts b/src/utils/attribution.test.ts index f8ebd89dee..e7eefb8607 100644 --- a/src/utils/attribution.test.ts +++ b/src/utils/attribution.test.ts @@ -12,9 +12,83 @@ import { resetSettingsCache, setSessionSettingsCache, } from './settings/settingsCache.js' -import * as actualSettings from './settings/settings.js' +import * as realSettings from './settings/settings.js' import type { SettingsJson } from './settings/types.js' +const actualSettings = { ...realSettings } + +function restoredGovernancePolicyModule() { + const getForbiddenCommitMessagePatterns = (): string[] => { + const patterns: string[] = [] + const sourceNames = [ + 'userSettings', + 'projectSettings', + 'localSettings', + 'policySettings', + 'flagSettings', + ] + for (const source of sourceNames) { + const sourcePatterns = + actualSettings.getSettingsForSource(source as never)?.git + ?.forbiddenCommitMessagePatterns ?? [] + for (const pattern of sourcePatterns) { + if (!patterns.includes(pattern)) { + patterns.push(pattern) + } + } + } + return patterns + } + + const findForbiddenCommitMessagePattern = (message: string): string | null => { + const normalizedMessage = message.toLocaleLowerCase() + for (const pattern of getForbiddenCommitMessagePatterns()) { + if (pattern && normalizedMessage.includes(pattern.toLocaleLowerCase())) { + return pattern + } + } + return null + } + + const sourceHasGitFlag = ( + key: 'addAICoAuthor' | 'addGeneratedWithFooter', + value: boolean, + ): boolean => + [ + 'userSettings', + 'projectSettings', + 'localSettings', + 'policySettings', + 'flagSettings', + ].some( + source => + actualSettings.getSettingsForSource(source as never)?.git?.[key] === + value, + ) + + return { + getGitAttributionOptIns: () => { + const git = actualSettings.getInitialSettings().git + return { + addAICoAuthor: git?.addAICoAuthor === true, + addGeneratedWithFooter: git?.addGeneratedWithFooter === true, + } + }, + isGeneratedCommitAttributionBlocked: () => + sourceHasGitFlag('addAICoAuthor', false), + isGeneratedPrAttributionBlocked: () => + sourceHasGitFlag('addGeneratedWithFooter', false), + isGitAttributionBlocked: () => + sourceHasGitFlag('addAICoAuthor', false) || + sourceHasGitFlag('addGeneratedWithFooter', false), + getForbiddenCommitMessagePatterns, + findForbiddenCommitMessagePattern, + isMemoryWriteApprovalRequired: () => + actualSettings.getInitialSettings().memory?.requireApprovalBeforeWrite !== + false, + } +} + let getAttributionTexts: (typeof import('./attribution.js'))['getAttributionTexts'] let getDefaultCommitCoAuthorEmail: (typeof import('./attribution.js'))[ 'getDefaultCommitCoAuthorEmail' @@ -142,8 +216,27 @@ beforeEach(async () => { ...actualSettings, getInitialSettings: () => testSettings, getSettings_DEPRECATED: () => testSettings, + getSettingsForSource: () => testSettings, + })) + mock.module('./governancePolicy.js', () => ({ + getGitAttributionOptIns: () => ({ + addAICoAuthor: testSettings.git?.addAICoAuthor === true, + addGeneratedWithFooter: + testSettings.git?.addGeneratedWithFooter === true, + }), + isGeneratedCommitAttributionBlocked: () => + testSettings.git?.addAICoAuthor === false, + isGeneratedPrAttributionBlocked: () => + testSettings.git?.addGeneratedWithFooter === false, + isGitAttributionBlocked: () => + testSettings.git?.addAICoAuthor === false || + testSettings.git?.addGeneratedWithFooter === false, + getForbiddenCommitMessagePatterns: () => + testSettings.git?.forbiddenCommitMessagePatterns ?? [], + findForbiddenCommitMessagePattern: () => null, + isMemoryWriteApprovalRequired: () => + testSettings.memory?.requireApprovalBeforeWrite !== false, })) - const attribution = await import( `./attribution.ts?attributionTest=${Date.now()}-${Math.random()}` ) @@ -160,9 +253,8 @@ afterEach(() => { testSettings = {} setClientType(originalClientType) setMainLoopModelOverride(originalMainLoopModelOverride) - mock.module('./model/model.js', () => actualModel) - mock.module('./model/providers.js', () => actualProviders) mock.module('./settings/settings.js', () => actualSettings) + mock.module('./governancePolicy.js', restoredGovernancePolicyModule) restoreEnv() }) @@ -279,6 +371,57 @@ describe('getAttributionTexts', () => { expect(attribution.pr).toBe(defaultPrAttribution) }) + it('uses git.addAICoAuthor as an explicit generated commit opt-in', () => { + useSettings({ git: { addAICoAuthor: true } }) + + const attribution = getAttributionTexts() + expect(attribution.commit).toStartWith('Co-Authored-By: ') + expect(attribution.pr).toBe('') + }) + + it('uses git.addGeneratedWithFooter as an explicit generated PR opt-in', () => { + useSettings({ git: { addGeneratedWithFooter: true } }) + + const attribution = getAttributionTexts() + expect(attribution.commit).toBe('') + expect(attribution.pr).toBe(defaultPrAttribution) + }) + + it('lets git.addAICoAuthor false block legacy generated commit attribution', () => { + useSettings({ includeCoAuthoredBy: true, git: { addAICoAuthor: false } }) + + expect(getAttributionTexts()).toEqual({ + commit: '', + pr: defaultPrAttribution, + }) + }) + + it('lets git.addGeneratedWithFooter false block legacy generated PR attribution', () => { + useSettings({ + includeCoAuthoredBy: true, + git: { addGeneratedWithFooter: false }, + }) + + const attribution = getAttributionTexts() + expect(attribution.commit).toStartWith('Co-Authored-By: ') + expect(attribution.pr).toBe('') + }) + + it('does not block explicit custom attribution when generated attribution is disabled', () => { + useSettings({ + attribution: { + commit: 'Signed-off-by: Human ', + pr: 'Reviewed by release engineering.', + }, + git: { addAICoAuthor: false, addGeneratedWithFooter: false }, + }) + + expect(getAttributionTexts()).toEqual({ + commit: 'Signed-off-by: Human ', + pr: 'Reviewed by release engineering.', + }) + }) + it('keeps attribution off when includeCoAuthoredBy is false', () => { useSettings({ includeCoAuthoredBy: false }) @@ -357,4 +500,25 @@ describe('getEnhancedPRAttribution', () => { defaultPrAttribution, ) }) + + it('uses git.addGeneratedWithFooter as an explicit opt-in to generated PR attribution', async () => { + useSettings({ git: { addGeneratedWithFooter: true } }) + + await expect(getEnhancedPRAttribution(() => ({} as never))).resolves.toBe( + defaultPrAttribution, + ) + }) + + it('lets git.addGeneratedWithFooter false block legacy generated PR attribution', async () => { + useSettings({ + includeCoAuthoredBy: true, + git: { addGeneratedWithFooter: false }, + }) + + await expect( + getEnhancedPRAttribution(() => { + throw new Error('app state should not be read when PR attribution is blocked') + }), + ).resolves.toBe('') + }) }) diff --git a/src/utils/attribution.ts b/src/utils/attribution.ts index 492c3fac64..e7c3ed928f 100644 --- a/src/utils/attribution.ts +++ b/src/utils/attribution.ts @@ -22,6 +22,11 @@ import { logForDebugging } from './debug.js' import { parseJSONL } from './json.js' import { logError } from './log.js' import { getAPIProvider } from './model/providers.js' +import { + getGitAttributionOptIns, + isGeneratedCommitAttributionBlocked, + isGeneratedPrAttributionBlocked, +} from './governancePolicy.js' import { getCanonicalName, getMainLoopModel, @@ -137,7 +142,15 @@ export function getAttributionTexts(): AttributionTexts { } } - if (settings.includeCoAuthoredBy !== true) { + const optIns = getGitAttributionOptIns() + const includeGeneratedCommit = + (settings.includeCoAuthoredBy === true || optIns.addAICoAuthor) && + !isGeneratedCommitAttributionBlocked() + const includeGeneratedPr = + (settings.includeCoAuthoredBy === true || optIns.addGeneratedWithFooter) && + !isGeneratedPrAttributionBlocked() + + if (!includeGeneratedCommit && !includeGeneratedPr) { return { commit: '', pr: '' } } @@ -157,7 +170,10 @@ export function getAttributionTexts(): AttributionTexts { ? '' : `Co-Authored-By: ${modelName} <${coAuthorEmail}>` - return { commit: defaultCommit, pr: DEFAULT_PR_ATTRIBUTION } + return { + commit: includeGeneratedCommit ? defaultCommit : '', + pr: includeGeneratedPr ? DEFAULT_PR_ATTRIBUTION : '', + } } /** @@ -387,9 +403,14 @@ export async function getEnhancedPRAttribution( return '' } - // Backward compatibility: deprecated includeCoAuthoredBy setting is now an - // explicit opt-in for the old generated attribution behavior. - if (settings.includeCoAuthoredBy !== true) { + const optIns = getGitAttributionOptIns() + const includeGeneratedPr = + (settings.includeCoAuthoredBy === true || optIns.addGeneratedWithFooter) && + !isGeneratedPrAttributionBlocked() + + // Backward compatibility: deprecated includeCoAuthoredBy remains an explicit + // opt-in, and git.addGeneratedWithFooter is the new generated-PR opt-in. + if (!includeGeneratedPr) { return '' } diff --git a/src/utils/conversationArc.perf.test.ts b/src/utils/conversationArc.perf.test.ts index 01af5aee57..8cf7b42ec7 100644 --- a/src/utils/conversationArc.perf.test.ts +++ b/src/utils/conversationArc.perf.test.ts @@ -1,10 +1,14 @@ import { describe, expect, it, beforeEach, afterEach } from 'bun:test' +import { mkdtempSync, rmSync } from 'fs' +import { tmpdir } from 'os' +import { join } from 'path' import { initializeArc, updateArcPhase, getArcSummary, resetArc } from './conversationArc.js' +import { setClaudeConfigHomeDirForTesting } from './envUtils.js' import { getGlobalGraph, clearMemoryOnly, resetGlobalGraph } from './knowledgeGraph.js' import { acquireSharedMutationLock, @@ -19,8 +23,12 @@ function createMessage(content: string): any { } describe('Conversation Arc Scale and Stability', () => { + let configDir: string + beforeEach(async () => { await acquireSharedMutationLock('conversationArc.perf') + configDir = mkdtempSync(join(tmpdir(), 'openclaude-arc-perf-')) + setClaudeConfigHomeDirForTesting(configDir) resetGlobalGraph() clearMemoryOnly() resetArc() @@ -32,6 +40,8 @@ describe('Conversation Arc Scale and Stability', () => { resetGlobalGraph() clearMemoryOnly() resetArc() + setClaudeConfigHomeDirForTesting(undefined) + rmSync(configDir, { recursive: true, force: true }) } finally { releaseSharedMutationLock() } diff --git a/src/utils/governancePolicy.test.ts b/src/utils/governancePolicy.test.ts new file mode 100644 index 0000000000..2bddf81997 --- /dev/null +++ b/src/utils/governancePolicy.test.ts @@ -0,0 +1,88 @@ +import { afterEach, beforeEach, expect, test } from 'bun:test' +import { + getAllowedSettingSources, + setAllowedSettingSources, +} from '../bootstrap/state.js' +import { + SETTING_SOURCES, + type SettingSource, +} from './settings/constants.js' +import { resetSettingsCache } from './settings/settingsCache.js' +import type { SettingsJson } from './settings/types.js' +type GovernancePolicyModule = typeof import('./governancePolicy.js') + +let originalAllowedSettingSources: ReturnType +let settingsBySource = new Map() +let governancePolicy: GovernancePolicyModule + +function setSourceSettings( + source: SettingSource, + settings: SettingsJson, +): void { + settingsBySource.set(source, settings) +} + +beforeEach(async () => { + originalAllowedSettingSources = [...getAllowedSettingSources()] + settingsBySource = new Map() + setAllowedSettingSources([...SETTING_SOURCES]) + governancePolicy = (await import( + `./governancePolicy.ts?governancePolicyTest=${Date.now()}-${Math.random()}` + )) as GovernancePolicyModule + governancePolicy.setGovernancePolicySettingsForSourceForTesting( + source => settingsBySource.get(source) ?? null, + ) + resetSettingsCache() +}) + +afterEach(() => { + governancePolicy.setGovernancePolicySettingsForSourceForTesting(null) + setAllowedSettingSources(originalAllowedSettingSources) + settingsBySource = new Map() + resetSettingsCache() +}) + +test('memory approval is required when any settings source opts in', () => { + setSourceSettings('projectSettings', { + memory: { requireApprovalBeforeWrite: true }, + }) + + expect(governancePolicy.isMemoryWriteApprovalRequired()).toBe(true) +}) + +test('memory approval is required by default', () => { + expect(governancePolicy.isMemoryWriteApprovalRequired()).toBe(true) +}) + +test('memory approval can be explicitly disabled when no source requires it', () => { + setAllowedSettingSources(['projectSettings', 'localSettings']) + setSourceSettings('projectSettings', { + memory: { requireApprovalBeforeWrite: false }, + }) + + expect(governancePolicy.isMemoryWriteApprovalRequired()).toBe(false) +}) + +test('generated attribution block settings are evaluated independently', () => { + setSourceSettings('projectSettings', { + git: { addAICoAuthor: false }, + }) + + expect(governancePolicy.isGeneratedCommitAttributionBlocked()).toBe(true) + expect(governancePolicy.isGeneratedPrAttributionBlocked()).toBe(false) +}) + +test('forbidden commit message patterns are combined across settings sources', () => { + setSourceSettings('projectSettings', { + git: { forbiddenCommitMessagePatterns: ['Generated with'] }, + }) + setSourceSettings('localSettings', { + git: { forbiddenCommitMessagePatterns: ['Co-Authored-By:'] }, + }) + + expect( + governancePolicy.findForbiddenCommitMessagePattern( + 'fix: policy\n\nco-authored-by: OpenClaude ', + ), + ).toBe('Co-Authored-By:') +}) diff --git a/src/utils/governancePolicy.ts b/src/utils/governancePolicy.ts new file mode 100644 index 0000000000..aaaa499e6e --- /dev/null +++ b/src/utils/governancePolicy.ts @@ -0,0 +1,103 @@ +import { getEnabledSettingSources } from './settings/constants.js' +import { getInitialSettings, getSettingsForSource } from './settings/settings.js' +import type { SettingSource } from './settings/constants.js' +import type { SettingsJson } from './settings/types.js' + +let getSettingsForSourceForTesting: + | ((source: SettingSource) => SettingsJson | null) + | null = null + +export function setGovernancePolicySettingsForSourceForTesting( + getter: ((source: SettingSource) => SettingsJson | null) | null, +): void { + getSettingsForSourceForTesting = getter +} + +function getGovernanceSettingsForSource( + source: SettingSource, +): SettingsJson | null { + if (getSettingsForSourceForTesting) { + return getSettingsForSourceForTesting(source) + } + return getSettingsForSource(source) +} + +export function isMemoryWriteApprovalRequired(): boolean { + let explicitlyDisabled = false + for (const source of getEnabledSettingSources()) { + const value = + getGovernanceSettingsForSource(source)?.memory?.requireApprovalBeforeWrite + if (value === true) { + return true + } + if (value === false) { + explicitlyDisabled = true + } + } + return !explicitlyDisabled +} + +export function isGitAttributionBlocked(): boolean { + return ( + isGeneratedCommitAttributionBlocked() || isGeneratedPrAttributionBlocked() + ) +} + +export function isGeneratedCommitAttributionBlocked(): boolean { + for (const source of getEnabledSettingSources()) { + const git = getGovernanceSettingsForSource(source)?.git + if (git?.addAICoAuthor === false) { + return true + } + } + return false +} + +export function isGeneratedPrAttributionBlocked(): boolean { + for (const source of getEnabledSettingSources()) { + const git = getGovernanceSettingsForSource(source)?.git + if (git?.addGeneratedWithFooter === false) { + return true + } + } + return false +} + +export function getGitAttributionOptIns(): { + addAICoAuthor: boolean + addGeneratedWithFooter: boolean +} { + const git = getInitialSettings().git + return { + addAICoAuthor: git?.addAICoAuthor === true, + addGeneratedWithFooter: git?.addGeneratedWithFooter === true, + } +} + +export function getForbiddenCommitMessagePatterns(): string[] { + const patterns: string[] = [] + for (const source of getEnabledSettingSources()) { + const sourcePatterns = + getGovernanceSettingsForSource(source)?.git + ?.forbiddenCommitMessagePatterns ?? [] + for (const pattern of sourcePatterns) { + if (!patterns.includes(pattern)) { + patterns.push(pattern) + } + } + } + return patterns +} + +export function findForbiddenCommitMessagePattern( + message: string, +): string | null { + const normalizedMessage = message.toLocaleLowerCase() + for (const pattern of getForbiddenCommitMessagePatterns()) { + if (!pattern) continue + if (normalizedMessage.includes(pattern.toLocaleLowerCase())) { + return pattern + } + } + return null +} diff --git a/src/utils/permissions/filesystem.test.ts b/src/utils/permissions/filesystem.test.ts index 4fa590209a..9447b9be6a 100644 --- a/src/utils/permissions/filesystem.test.ts +++ b/src/utils/permissions/filesystem.test.ts @@ -4,7 +4,13 @@ import { tmpdir } from 'os' import { join } from 'path' import { z } from 'zod/v4' import type { ToolPermissionContext } from '../../types/permissions.js' -import { getOriginalCwd, setOriginalCwd } from '../../bootstrap/state.js' +import { + getOriginalCwd, + getProjectRoot, + setOriginalCwd, + setProjectRoot, +} from '../../bootstrap/state.js' +import { getAutoMemPath } from '../../memdir/paths.js' import { createToolFixture } from '../../test/toolFixtures.js' import { checkWritePermissionForTool } from './filesystem.js' @@ -12,6 +18,69 @@ const writeInputSchema = z.object({ file_path: z.string(), }) +describe('auto-memory write permissions', () => { + let originalProjectRoot: string + let originalMemoryPathOverride: string | undefined + let projectDir: string + + beforeEach(async () => { + originalProjectRoot = getProjectRoot() + originalMemoryPathOverride = process.env.CLAUDE_COWORK_MEMORY_PATH_OVERRIDE + delete process.env.CLAUDE_COWORK_MEMORY_PATH_OVERRIDE + getAutoMemPath.cache.clear?.() + projectDir = await mkdtemp(join(tmpdir(), 'openclaude-memory-perms-')) + setProjectRoot(projectDir) + }) + + afterEach(async () => { + setProjectRoot(originalProjectRoot) + if (originalMemoryPathOverride === undefined) { + delete process.env.CLAUDE_COWORK_MEMORY_PATH_OVERRIDE + } else { + process.env.CLAUDE_COWORK_MEMORY_PATH_OVERRIDE = + originalMemoryPathOverride + } + getAutoMemPath.cache.clear?.() + await rm(projectDir, { recursive: true, force: true }) + }) + + test('requires approval for default auto-memory writes', () => { + const result = checkWritePermissionForTool( + writeTool, + { file_path: join(getAutoMemPath(), 'user_role.md') }, + permissionContext('bypassPermissions'), + ) + + expect(result.behavior).toBe('ask') + expect(result.decisionReason).toMatchObject({ + type: 'safetyCheck', + reason: 'Persistent memory writes require explicit approval', + }) + }) + + test('requires approval for overridden auto-memory writes', async () => { + const overrideDir = await mkdtemp(join(tmpdir(), 'openclaude-memory-')) + process.env.CLAUDE_COWORK_MEMORY_PATH_OVERRIDE = overrideDir + getAutoMemPath.cache.clear?.() + + try { + const result = checkWritePermissionForTool( + writeTool, + { file_path: join(getAutoMemPath(), 'user_role.md') }, + permissionContext('bypassPermissions'), + ) + + expect(result.behavior).toBe('ask') + expect(result.decisionReason).toMatchObject({ + type: 'safetyCheck', + reason: 'Persistent memory writes require explicit approval', + }) + } finally { + await rm(overrideDir, { recursive: true, force: true }) + } + }) +}) + const writeTool = createToolFixture(writeInputSchema, { name: 'Write', getPath(input: { file_path: string }) { diff --git a/src/utils/permissions/filesystem.ts b/src/utils/permissions/filesystem.ts index bb4ca962ec..b22d5c8a6a 100644 --- a/src/utils/permissions/filesystem.ts +++ b/src/utils/permissions/filesystem.ts @@ -22,6 +22,7 @@ import { getCwd } from '../cwd.js' import { logForDebugging } from '../debug.js' import { getClaudeConfigHomeDir } from '../envUtils.js' import { isFsInaccessible } from '../errors.js' +import { isMemoryWriteApprovalRequired } from '../governancePolicy.js' import { getFsImplementation, getPathsForPermissionCheck, @@ -1650,13 +1651,23 @@ export function checkEditableInternalPath( } } - // Memdir directory (persistent memory for cross-session learning) - // This pre-safety-check carve-out exists because the default path is under - // ~/.claude/, which is in DANGEROUS_DIRECTORIES. The CLAUDE_COWORK_MEMORY_PATH_OVERRIDE - // override is an arbitrary caller-designated directory with no such conflict, - // so it gets NO special permission treatment here — writes go through normal - // permission flow (step 5 → ask). SDK callers who want silent memory should - // pass an allow rule for the override path. + // Memdir directory (persistent memory for cross-session learning). + // Explicit memory-write approval applies even when the env override points + // memory at a caller-designated directory. The silent pre-safety-check + // carve-out below exists only for the default/settings-backed path because + // it can live under ~/.claude/, which is in DANGEROUS_DIRECTORIES. + if (isAutoMemPath(normalizedPath) && isMemoryWriteApprovalRequired()) { + return { + behavior: 'ask', + message: `${PRODUCT_DISPLAY_NAME} wants to save persistent memory. Approve this memory write?`, + decisionReason: { + type: 'safetyCheck', + reason: 'Persistent memory writes require explicit approval', + classifierApprovable: false, + }, + } + } + if (!hasAutoMemPathOverride() && isAutoMemPath(normalizedPath)) { return { behavior: 'allow', diff --git a/src/utils/settings/types.ts b/src/utils/settings/types.ts index 564cdc2337..d405829201 100644 --- a/src/utils/settings/types.ts +++ b/src/utils/settings/types.ts @@ -1025,10 +1025,41 @@ export const SettingsSchema = lazySchema(() => .describe( 'When false, disables auto-memory reads and writes for this project. Discoverable alias for `autoMemoryEnabled`; the two are equivalent and either one can be used to opt out for governance / regulated / client-sensitive repos (issue #1326). When both are set, the more restrictive (false) value wins so a parent-scope opt-out cannot be silently re-enabled by a narrower scope.', ), + requireApprovalBeforeWrite: z + .boolean() + .optional() + .describe( + 'Persistent auto-memory writes require explicit permission approval by default. Set to false to restore automatic memory writes when auto-memory is enabled.', + ), + }) + .optional() + .describe( + 'Memory governance settings. `autoWrite` controls whether auto-memory is active; `requireApprovalBeforeWrite` forces explicit approval for persistent memory writes.', + ), + git: z + .object({ + addAICoAuthor: z + .boolean() + .optional() + .describe( + 'When true, opt in to the generated Co-Authored-By trailer for local commits. When false in any settings source, generated commit attribution is blocked.', + ), + addGeneratedWithFooter: z + .boolean() + .optional() + .describe( + 'When true, opt in to the generated OpenClaude footer for PR descriptions. When false in any settings source, generated PR attribution is blocked.', + ), + forbiddenCommitMessagePatterns: z + .array(z.string()) + .optional() + .describe( + 'Literal text patterns that must not appear in git commit messages, such as "Co-Authored-By:" or "Generated with".', + ), }) .optional() .describe( - 'Memory governance settings. Currently exposes `autoWrite` as the discoverable shape requested in #1326; further opt-in fields (e.g. approval gates) may be added under this namespace without taking a new top-level key each time.', + 'Git governance settings for AI attribution and commit-message policy.', ), autoMemoryDirectory: z .string()