Repository navigation
fix: governance controls for memory and git attribution - #1806
Conversation
Default persistent memory writes to explicit approval unless memory.requireApprovalBeforeWrite is disabled. Skip automatic memory extraction/dream writes while approval is required, avoid auto-creating memory directories in that mode, and surface approval prompts for auto-memory file writes. Make generated commit and PR attribution opt-in through git.addAICoAuthor and git.addGeneratedWithFooter while preserving explicit custom attribution. Add forbidden commit-message patterns and surface them in commit prompts. Enforce commit-message governance in Bash and PowerShell permission checks, including global git options, file-backed messages, shell wrappers, env split strings, chained commands, and PowerShell call/operator forms. Add regression coverage for the policy paths.
jatmn
changed the title
📝 WalkthroughWalkthroughAdds settings-driven governance checks for memory writes, git attribution, and forbidden commit messages, then wires them into prompts, permissions, and Bash/PowerShell command handling. Tests also isolate settings and config state between runs. ChangesGovernance Policy Enforcement
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/tools/BashTool/bashSecurity.ts`:
- Around line 792-825: The commit-message parsing in extractGitCommitMessages
and gitCommitUsesFileMessage only handles a narrow set of inspectable forms,
which can let unsupported git commit message sources bypass policy. Update the
commit governance flow so any commit command whose message source cannot be
fully parsed is treated as uninspectable and returns ask whenever commit-message
policy is enabled, rather than null/allow; use the existing helpers
normalizeGitCommitCommand, extractGitCommitMessages, and
gitCommitUsesFileMessage to locate the change, and ensure the logic in the
commit permission decision path fails closed for forms like -C, --reuse-message,
-c/--reedit-message, and other unparsed message syntaxes.
In `@src/tools/PowerShellTool/PowerShellTool.tsx`:
- Around line 251-279: The PowerShell commit-message parser in
extractPowerShellGitCommitMessages and powerShellGitCommitUsesFile only
recognizes a few literal message sources, so unsupported commit forms can slip
through as if they were safe. Update the permission check path that uses these
helpers to fail closed: if a git commit command uses any message source you do
not fully parse, return ask instead of null/allow, similar to the existing
--file/-F handling. Make sure checkPermissions treats uninspectable forms like
-C/--reuse-message, -c/--reedit-message, and here-string variants as requiring
user approval.
In `@src/utils/attribution.test.ts`:
- Around line 20-88: The cleanup mock is overriding real memory-governance state
by hardcoding isMemoryWriteApprovalRequired() to true and restoring that helper
after each test, which can leak an unrealistic governancePolicy into later
Bun-worker tests. Update restoredGovernancePolicyModule() to derive memory-write
approval from actualSettings state the same way the other git helpers do, and
adjust the test cleanup/afterEach restoration so it resets to the real
settings-backed implementation rather than a permanent true stub.
In `@src/utils/governancePolicy.test.ts`:
- Around line 37-58: The governancePolicy test setup only isolates the working
directory, so config-home-backed sources like user settings can still affect
isMemoryWriteApprovalRequired() and make the suite environment-dependent. Update
the beforeEach/afterEach setup in governancePolicy.test.ts to create and point
to a temp config home for the duration of the suite, then restore and remove it
afterward, alongside the existing originalCwd and settings cache resets.
In `@src/utils/permissions/filesystem.ts`:
- Around line 1662-1672: The memory-write approval branch in
permissions/filesystem.ts is only enforced when !hasAutoMemPathOverride(), which
lets overridden memdir paths bypass the explicit approval gate through normal
allow/mode flows. Update the path check around isMemoryWriteApprovalRequired()
so isAutoMemPath(normalizedPath) is evaluated first and always triggers the
approval prompt for overridden memory paths, then preserve the existing silent
auto-allow behavior only for the non-override case in the filesystem permission
logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: cb665ffb-7101-4291-bb92-5f1ed568fe4f
📒 Files selected for processing (22)
src/commands/commit-push-pr.tssrc/commands/commit.tssrc/components/ProviderManager.test.tsxsrc/memdir/memdir.tssrc/memdir/paths.test.tssrc/memdir/teamMemPrompts.tssrc/services/autoDream/autoDream.tssrc/services/extractMemories/extractMemories.tssrc/tools/BashTool/bashPermissions.test.tssrc/tools/BashTool/bashPermissions.tssrc/tools/BashTool/bashSecurity.test.tssrc/tools/BashTool/bashSecurity.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/PowerShellTool/powershellPermissions.test.tssrc/utils/attribution.test.tssrc/utils/attribution.tssrc/utils/conversationArc.perf.test.tssrc/utils/governancePolicy.test.tssrc/utils/governancePolicy.tssrc/utils/permissions/filesystem.test.tssrc/utils/permissions/filesystem.tssrc/utils/settings/types.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (15)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/memdir/paths.test.tssrc/memdir/teamMemPrompts.tssrc/services/extractMemories/extractMemories.tssrc/tools/BashTool/bashSecurity.test.tssrc/utils/conversationArc.perf.test.tssrc/commands/commit.tssrc/components/ProviderManager.test.tsxsrc/services/autoDream/autoDream.tssrc/commands/commit-push-pr.tssrc/utils/permissions/filesystem.tssrc/utils/attribution.tssrc/utils/governancePolicy.tssrc/tools/BashTool/bashPermissions.test.tssrc/memdir/memdir.tssrc/utils/settings/types.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/BashTool/bashPermissions.tssrc/tools/PowerShellTool/powershellPermissions.test.tssrc/utils/governancePolicy.test.tssrc/tools/BashTool/bashSecurity.tssrc/utils/permissions/filesystem.test.tssrc/utils/attribution.test.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/memdir/paths.test.tssrc/memdir/teamMemPrompts.tssrc/services/extractMemories/extractMemories.tssrc/tools/BashTool/bashSecurity.test.tssrc/utils/conversationArc.perf.test.tssrc/commands/commit.tssrc/components/ProviderManager.test.tsxsrc/services/autoDream/autoDream.tssrc/commands/commit-push-pr.tssrc/utils/permissions/filesystem.tssrc/utils/attribution.tssrc/utils/governancePolicy.tssrc/tools/BashTool/bashPermissions.test.tssrc/memdir/memdir.tssrc/utils/settings/types.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/BashTool/bashPermissions.tssrc/tools/PowerShellTool/powershellPermissions.test.tssrc/utils/governancePolicy.test.tssrc/tools/BashTool/bashSecurity.tssrc/utils/permissions/filesystem.test.tssrc/utils/attribution.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/memdir/paths.test.tssrc/tools/BashTool/bashSecurity.test.tssrc/utils/conversationArc.perf.test.tssrc/components/ProviderManager.test.tsxsrc/tools/BashTool/bashPermissions.test.tssrc/tools/PowerShellTool/powershellPermissions.test.tssrc/utils/governancePolicy.test.tssrc/utils/permissions/filesystem.test.tssrc/utils/attribution.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/memdir/paths.test.tssrc/tools/BashTool/bashSecurity.test.tssrc/utils/conversationArc.perf.test.tssrc/components/ProviderManager.test.tsxsrc/tools/BashTool/bashPermissions.test.tssrc/tools/PowerShellTool/powershellPermissions.test.tssrc/utils/governancePolicy.test.tssrc/utils/permissions/filesystem.test.tssrc/utils/attribution.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/memdir/paths.test.tssrc/memdir/teamMemPrompts.tssrc/services/extractMemories/extractMemories.tssrc/tools/BashTool/bashSecurity.test.tssrc/utils/conversationArc.perf.test.tssrc/commands/commit.tssrc/components/ProviderManager.test.tsxsrc/services/autoDream/autoDream.tssrc/commands/commit-push-pr.tssrc/utils/permissions/filesystem.tssrc/utils/attribution.tssrc/utils/governancePolicy.tssrc/tools/BashTool/bashPermissions.test.tssrc/memdir/memdir.tssrc/utils/settings/types.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/BashTool/bashPermissions.tssrc/tools/PowerShellTool/powershellPermissions.test.tssrc/utils/governancePolicy.test.tssrc/tools/BashTool/bashSecurity.tssrc/utils/permissions/filesystem.test.tssrc/utils/attribution.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/memdir/paths.test.tssrc/memdir/teamMemPrompts.tssrc/services/extractMemories/extractMemories.tssrc/tools/BashTool/bashSecurity.test.tssrc/utils/conversationArc.perf.test.tssrc/commands/commit.tssrc/components/ProviderManager.test.tsxsrc/services/autoDream/autoDream.tssrc/commands/commit-push-pr.tssrc/utils/permissions/filesystem.tssrc/utils/attribution.tssrc/utils/governancePolicy.tssrc/tools/BashTool/bashPermissions.test.tssrc/memdir/memdir.tssrc/utils/settings/types.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/BashTool/bashPermissions.tssrc/tools/PowerShellTool/powershellPermissions.test.tssrc/utils/governancePolicy.test.tssrc/tools/BashTool/bashSecurity.tssrc/utils/permissions/filesystem.test.tssrc/utils/attribution.test.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/memdir/paths.test.tssrc/memdir/teamMemPrompts.tssrc/services/extractMemories/extractMemories.tssrc/tools/BashTool/bashSecurity.test.tssrc/utils/conversationArc.perf.test.tssrc/commands/commit.tssrc/components/ProviderManager.test.tsxsrc/services/autoDream/autoDream.tssrc/commands/commit-push-pr.tssrc/utils/permissions/filesystem.tssrc/utils/attribution.tssrc/utils/governancePolicy.tssrc/tools/BashTool/bashPermissions.test.tssrc/memdir/memdir.tssrc/utils/settings/types.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/BashTool/bashPermissions.tssrc/tools/PowerShellTool/powershellPermissions.test.tssrc/utils/governancePolicy.test.tssrc/tools/BashTool/bashSecurity.tssrc/utils/permissions/filesystem.test.tssrc/utils/attribution.test.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/memdir/paths.test.tssrc/memdir/teamMemPrompts.tssrc/services/extractMemories/extractMemories.tssrc/tools/BashTool/bashSecurity.test.tssrc/utils/conversationArc.perf.test.tssrc/commands/commit.tssrc/components/ProviderManager.test.tsxsrc/services/autoDream/autoDream.tssrc/commands/commit-push-pr.tssrc/utils/permissions/filesystem.tssrc/utils/attribution.tssrc/utils/governancePolicy.tssrc/tools/BashTool/bashPermissions.test.tssrc/memdir/memdir.tssrc/utils/settings/types.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/BashTool/bashPermissions.tssrc/tools/PowerShellTool/powershellPermissions.test.tssrc/utils/governancePolicy.test.tssrc/tools/BashTool/bashSecurity.tssrc/utils/permissions/filesystem.test.tssrc/utils/attribution.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/memdir/paths.test.tssrc/tools/BashTool/bashSecurity.test.tssrc/utils/conversationArc.perf.test.tssrc/components/ProviderManager.test.tsxsrc/tools/BashTool/bashPermissions.test.tssrc/tools/PowerShellTool/powershellPermissions.test.tssrc/utils/governancePolicy.test.tssrc/utils/permissions/filesystem.test.tssrc/utils/attribution.test.ts
{src/commands/**/*.ts,src/services/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
chalkfor terminal color in CLI code
Files:
src/services/extractMemories/extractMemories.tssrc/commands/commit.tssrc/services/autoDream/autoDream.tssrc/commands/commit-push-pr.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/services/extractMemories/extractMemories.tssrc/utils/conversationArc.perf.test.tssrc/services/autoDream/autoDream.tssrc/utils/permissions/filesystem.tssrc/utils/attribution.tssrc/utils/governancePolicy.tssrc/utils/settings/types.tssrc/utils/governancePolicy.test.tssrc/utils/permissions/filesystem.test.tssrc/utils/attribution.test.ts
{src/integrations/**/*.ts,src/services/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Test the exact provider/model path you changed when possible for provider modifications
Files:
src/services/extractMemories/extractMemories.tssrc/services/autoDream/autoDream.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/tools/BashTool/bashSecurity.test.tssrc/utils/permissions/filesystem.tssrc/tools/BashTool/bashPermissions.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/BashTool/bashPermissions.tssrc/tools/PowerShellTool/powershellPermissions.test.tssrc/tools/BashTool/bashSecurity.tssrc/utils/permissions/filesystem.test.ts
{src/commands/**/*.ts,src/entrypoints/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
commanderfor CLI argument parsing
Files:
src/commands/commit.tssrc/commands/commit-push-pr.ts
src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + Ink for terminal UI implementations
Files:
src/components/ProviderManager.test.tsx
🪛 GitHub Actions: PR Checks / 1_smoke-and-tests.txt
src/utils/governancePolicy.test.ts
[error] 81-81: Jest test failed in governancePolicy.test.ts. expect(received).toBe(expected) failed. Expected: false, Received: true.
🪛 GitHub Actions: PR Checks / smoke-and-tests
src/utils/governancePolicy.test.ts
[error] 81-81: Test failed in Jest: expect(received).toBe(expected). Expected false but received true for isMemoryWriteApprovalRequired().
🪛 GitHub Check: smoke-and-tests
src/utils/governancePolicy.test.ts
[failure] 81-81: error: expect(received).toBe(expected)
Expected: false
Received: true
at <anonymous> (/home/runner/work/openclaude/openclaude/src/utils/governancePolicy.test.ts:81:43)
🔇 Additional comments (4)
src/components/ProviderManager.test.tsx (1)
17-19: 🎯 Functional CorrectnessNo change needed —
.ts?...cache-busting imports are supported here and match existing tests.> Likely an incorrect or invalid review comment.src/utils/attribution.ts (1)
25-29: LGTM!Also applies to: 145-153, 173-176
src/utils/attribution.test.ts (1)
217-237: LGTM!Also applies to: 372-422
src/memdir/memdir.ts (1)
470-494: 🩺 Stability & AvailabilityNo blocker:
logMemoryDirCounts()already falls back whenreaddirfails, so skippingensureMemoryDirExists()here won’t break first-run sessions.> Likely an incorrect or invalid review comment.
904bcdf to
c4e113f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/governancePolicy.ts`:
- Around line 16-19: The fallback in getGovernanceSettingsForSource is treating
a test override that returns null as if no override exists, so it still calls
getSettingsForSource and leaks real config into tests. Update
getGovernanceSettingsForSource to return the installed
getSettingsForSourceForTesting result as-is whenever that hook is present, and
only call getSettingsForSource when no test getter is set.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d4035538-b4c6-44ce-be10-4640d52c4272
📒 Files selected for processing (2)
src/utils/governancePolicy.test.tssrc/utils/governancePolicy.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (10)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/utils/governancePolicy.test.tssrc/utils/governancePolicy.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/utils/governancePolicy.test.tssrc/utils/governancePolicy.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/utils/governancePolicy.test.tssrc/utils/governancePolicy.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/utils/governancePolicy.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/utils/governancePolicy.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/utils/governancePolicy.test.tssrc/utils/governancePolicy.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/utils/governancePolicy.test.tssrc/utils/governancePolicy.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/utils/governancePolicy.test.tssrc/utils/governancePolicy.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/utils/governancePolicy.test.tssrc/utils/governancePolicy.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/utils/governancePolicy.test.ts
c4e113f to
f7e4d41
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/utils/governancePolicy.ts (1)
16-19: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHonor
nullfrom the test override.Once
setGovernancePolicySettingsForSourceForTesting()is installed, anullresult means “no settings for this source”. The??fallback still readsgetSettingsForSource(source), sosrc/utils/governancePolicy.test.tsleaks real config for unseeded sources and stays environment-dependent.Proposed fix
function getGovernanceSettingsForSource( source: SettingSource, ): SettingsJson | null { - return getSettingsForSourceForTesting?.(source) ?? getSettingsForSource(source) + if (getSettingsForSourceForTesting) { + return getSettingsForSourceForTesting(source) + } + return getSettingsForSource(source) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/governancePolicy.ts` around lines 16 - 19, getGovernanceSettingsForSource is incorrectly falling back to getSettingsForSource(source) when the test override returns null, which causes real config to leak into tests. Update the logic in getGovernanceSettingsForSource so that once getSettingsForSourceForTesting is installed, its return value is honored exactly, including null, and only call getSettingsForSource(source) when no test override exists. Use the existing setGovernancePolicySettingsForSourceForTesting/getSettingsForSourceForTesting path to distinguish “override absent” from “override returned no settings”.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/utils/governancePolicy.ts`:
- Around line 16-19: getGovernanceSettingsForSource is incorrectly falling back
to getSettingsForSource(source) when the test override returns null, which
causes real config to leak into tests. Update the logic in
getGovernanceSettingsForSource so that once getSettingsForSourceForTesting is
installed, its return value is honored exactly, including null, and only call
getSettingsForSource(source) when no test override exists. Use the existing
setGovernancePolicySettingsForSourceForTesting/getSettingsForSourceForTesting
path to distinguish “override absent” from “override returned no settings”.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 01f9a299-b956-44ca-9d53-03c259999ac6
📒 Files selected for processing (2)
src/utils/governancePolicy.test.tssrc/utils/governancePolicy.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (10)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/utils/governancePolicy.tssrc/utils/governancePolicy.test.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/utils/governancePolicy.tssrc/utils/governancePolicy.test.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/utils/governancePolicy.tssrc/utils/governancePolicy.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/utils/governancePolicy.tssrc/utils/governancePolicy.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/utils/governancePolicy.tssrc/utils/governancePolicy.test.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/utils/governancePolicy.tssrc/utils/governancePolicy.test.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/utils/governancePolicy.tssrc/utils/governancePolicy.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/utils/governancePolicy.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/utils/governancePolicy.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/utils/governancePolicy.test.ts
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/utils/attribution.test.ts (1)
86-88: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMirror the per-source memory policy here, not the merged initial value.
Line 86 still diverges from
src/utils/governancePolicy.ts: the real helper scans settings sources and keeps approval required unless the sources explicitly disable it. Reading onlygetInitialSettings().memory?.requireApprovalBeforeWritecan restorefalsefor later tests when one source disables the flag but another source omits it, so this cleanup mock can still leak environment-dependent behavior across Bun workers.Suggested fix
- isMemoryWriteApprovalRequired: () => - actualSettings.getInitialSettings().memory?.requireApprovalBeforeWrite !== - false, + isMemoryWriteApprovalRequired: () => { + let allExplicitlyDisable = true + for (const source of [ + 'userSettings', + 'projectSettings', + 'localSettings', + 'policySettings', + 'flagSettings', + ]) { + const requireApproval = + actualSettings.getSettingsForSource(source as never)?.memory + ?.requireApprovalBeforeWrite + if (requireApproval === true) return true + if (requireApproval !== false) { + allExplicitlyDisable = false + } + } + return !allExplicitlyDisable + },As per path instructions, "Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/attribution.test.ts` around lines 86 - 88, The cleanup mock in attribution.test.ts is still using the merged initial memory setting, which can leak environment-dependent approval behavior across tests. Update the isMemoryWriteApprovalRequired mock to mirror the per-source logic from governancePolicy.ts by checking the individual settings sources rather than only actualSettings.getInitialSettings().memory?.requireApprovalBeforeWrite. Keep approval required unless a source explicitly disables it, and make sure the mock stays isolated so Bun worker state does not bleed between tests.Source: Path instructions
src/tools/BashTool/bashSecurity.ts (1)
797-813: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winFail closed on expandable quoted commit messages.
The quoted-message regex treats double-quoted shell expansions as inspectable. For example,
git commit -m "$MSG"orgit commit -m "$(cat .git/OPENCLAUDE_COMMIT_MSG)"yields a non-empty extracted message like$MSG, so Lines 860-865 do not ask even though the actual commit text can contain forbidden attribution. Only literal single-quoted strings and explicitly supported literal heredocs should be considered inspectable; other$..., backtick, or unquoted-heredoc expansion forms should ask when policy is active. As per path instructions,src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: "Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses..."Suggested direction
-function extractGitCommitMessages(command: string): string[] { +function extractGitCommitMessages(command: string): { + messages: string[] + hasUninspectableSource: boolean +} { const normalizedCommand = normalizeGitCommitCommand(command) - if (!normalizedCommand) return [] + 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)) { - messages.push(match[2] ?? match[4] ?? '') + const quote = match[1] ?? match[3] + const message = match[2] ?? match[4] ?? '' + if (quote === '"' && /(^|[^\\])(?:\$[\w{(]|\`)/.test(message)) { + hasUninspectableSource = true + continue + } + messages.push(message) } ... - return messages + return { messages, hasUninspectableSource } } ... - const messages = extractGitCommitMessages(command) - if (messages.length === 0 && hasPolicyRestrictions) { + const { messages, hasUninspectableSource } = extractGitCommitMessages(command) + if ((hasUninspectableSource || messages.length === 0) && hasPolicyRestrictions) { return commitPolicyAsk( 'Git commit message source cannot be checked against commit-message policy', )Also applies to: 860-865
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tools/BashTool/bashSecurity.ts` around lines 797 - 813, The message extraction in bashSecurity.ts is too permissive because the quoted-message handling treats double-quoted shell expansions as inspectable, which can bypass the permission prompt. Tighten the logic around simpleMessagePattern, unquotedMessagePattern, and heredocMessagePattern so only literal single-quoted commit messages and explicitly supported literal heredocs are considered safe to inspect; any expandable double-quoted, $(), backtick, or unquoted heredoc forms should be treated as non-inspectable and force a prompt when policy is active.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/tools/BashTool/bashSecurity.ts`:
- Around line 797-813: The message extraction in bashSecurity.ts is too
permissive because the quoted-message handling treats double-quoted shell
expansions as inspectable, which can bypass the permission prompt. Tighten the
logic around simpleMessagePattern, unquotedMessagePattern, and
heredocMessagePattern so only literal single-quoted commit messages and
explicitly supported literal heredocs are considered safe to inspect; any
expandable double-quoted, $(), backtick, or unquoted heredoc forms should be
treated as non-inspectable and force a prompt when policy is active.
In `@src/utils/attribution.test.ts`:
- Around line 86-88: The cleanup mock in attribution.test.ts is still using the
merged initial memory setting, which can leak environment-dependent approval
behavior across tests. Update the isMemoryWriteApprovalRequired mock to mirror
the per-source logic from governancePolicy.ts by checking the individual
settings sources rather than only
actualSettings.getInitialSettings().memory?.requireApprovalBeforeWrite. Keep
approval required unless a source explicitly disables it, and make sure the mock
stays isolated so Bun worker state does not bleed between tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e88e03b6-d641-4dfd-968a-0f4904b661fe
📒 Files selected for processing (6)
src/tools/BashTool/bashSecurity.test.tssrc/tools/BashTool/bashSecurity.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/PowerShellTool/powershellPermissions.test.tssrc/utils/attribution.test.tssrc/utils/governancePolicy.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (11)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/utils/attribution.test.tssrc/utils/governancePolicy.tssrc/tools/BashTool/bashSecurity.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/utils/attribution.test.tssrc/utils/governancePolicy.tssrc/tools/BashTool/bashSecurity.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/tools/BashTool/bashSecurity.test.tssrc/utils/attribution.test.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/tools/BashTool/bashSecurity.test.tssrc/utils/attribution.test.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/utils/attribution.test.tssrc/utils/governancePolicy.tssrc/tools/BashTool/bashSecurity.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/utils/attribution.test.tssrc/utils/governancePolicy.tssrc/tools/BashTool/bashSecurity.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/utils/attribution.test.tssrc/utils/governancePolicy.tssrc/tools/BashTool/bashSecurity.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/utils/attribution.test.tssrc/utils/governancePolicy.tssrc/tools/BashTool/bashSecurity.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/BashTool/bashSecurity.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/tools/BashTool/bashSecurity.test.tssrc/utils/attribution.test.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
{src/services/**/*.ts,src/utils/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use
execafor child processes
Files:
src/utils/attribution.test.tssrc/utils/governancePolicy.ts
🔇 Additional comments (4)
src/tools/BashTool/bashSecurity.ts (1)
4-8: LGTM!Also applies to: 818-825, 846-859, 867-897
src/tools/BashTool/bashSecurity.test.ts (1)
2-53: LGTM!Also applies to: 280-302
src/tools/PowerShellTool/PowerShellTool.tsx (1)
344-377: LGTM!Also applies to: 554-557
src/tools/PowerShellTool/powershellPermissions.test.ts (1)
98-146: LGTM!Also applies to: 259-286
Treat Bash and PowerShell expandable commit-message sources as uninspectable when commit-message governance policy is active. This prevents runtime-expanded variables, command substitutions, and expandable here-strings from bypassing forbidden attribution checks while preserving literal quoted heredocs/here-strings. Validation: bun test src/tools/BashTool/bashSecurity.test.ts src/tools/BashTool/bashPermissions.test.ts; bun test src/tools/PowerShellTool/powershellPermissions.test.ts; bun test src/tools/BashTool/bashSecurity.test.ts src/tools/BashTool/bashPermissions.test.ts src/tools/BashTool/readOnlyValidation.test.ts src/tools/BashTool/utils.test.ts src/tools/PowerShellTool/powershellPermissions.test.ts src/utils/permissions/filesystem.test.ts src/utils/permissions/permissions.test.ts src/memdir/paths.test.ts; bun run typecheck; bun run smoke; bun run security:pr-scan -- --base origin/main --head HEAD
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/tools/PowerShellTool/PowerShellTool.tsx (1)
264-278: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUnquoted
-mpath never checks expandability — policy bypass.Lines 264-272 flag expandable double-quoted values, but the unquoted matcher on Line 275 (
[^"'\s;|&()]+) permits$and backtick, and the loop on Lines 276-277 pushes the token verbatim without callinghasExpandablePowerShellText. Sogit commit -m $msg(or-m fix$msg) is treated as an inspectable literal and skips the fail-closedask, letting a variable-expanded forbidden trailer / attribution through. Unlike the bash sibling, this leaks even for a bare$msg. Mirror the double-quoted/here-string handling and flag uninspectable.As per path instructions:
src/{...,tools,...}/**— review shell/PowerShell execution paths as security-sensitive and "Block on bypasses."🔒 Proposed fix
const unquotedPattern = /(?:^|\s)(?:-m|--message)\s+([^"'\s;|&()]+)|(?:^|\s)--message=([^"'\s;|&()]+)/g; for (const match of normalizedCommand.matchAll(unquotedPattern)) { - messages.push(match[1] ?? match[2] ?? ''); + const message = match[1] ?? match[2] ?? ''; + if (hasExpandablePowerShellText(message)) { + hasUninspectableSource = true; + continue; + } + messages.push(message); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tools/PowerShellTool/PowerShellTool.tsx` around lines 264 - 278, The unquoted `-m`/`--message` parsing in `PowerShellTool.tsx` is treating variable-containing values as safe literals and bypassing the fail-closed inspection path. Update the unquoted message handling in the `quotedPattern`/`unquotedPattern` flow inside the command inspection logic to run `hasExpandablePowerShellText` on unquoted captures as well, and mark `hasUninspectableSource` when expansion is possible instead of always pushing the token into `messages`. Make the behavior consistent with the quoted double-quoted branch so `git commit -m $msg` cannot skip policy checks.Source: Path instructions
src/tools/BashTool/bashSecurity.ts (1)
807-827: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winTreat unquoted expandable
-mvalues as uninspectable
git commit -m fix$VARis parsed asfix, so the policy check only sees the literal prefix and misses the expanded tail. Fail closed on unquoted-m/--messagevalues containing$or backticks instead of partially capturing them.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tools/BashTool/bashSecurity.ts` around lines 807 - 827, The `bashSecurity.ts` message parsing around `simpleMessagePattern` and `unquotedMessagePattern` is too permissive for unquoted `-m/--message` values with shell expansion characters. Update the parsing logic in the `matchAll` loops so any unquoted message containing `$` or backticks is treated as uninspectable and sets `hasUninspectableSource` instead of being partially captured into `messages`. Keep the quoted-message handling in place, but make the unquoted branch fail closed for expandable content.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/tools/BashTool/bashSecurity.test.ts`:
- Around line 303-337: Add a regression test in bashSecurity.test.ts for the
unquoted expandable commit-message form so the fail-closed behavior is covered
once bashSecurity.ts is fixed. Extend the existing commit-message policy tests
around bashCommandIsSafe_DEPRECATED and expectAskMessage with a case like git
commit -m fix$VAR, asserting it is treated as cannot be checked. Keep it
alongside the current expandable message and heredoc coverage so the new gap
stays pinned.
In `@src/tools/PowerShellTool/powershellPermissions.test.ts`:
- Around line 288-311: Add a regression test in powershellPermissions.test.ts
covering the unquoted-variable commit message form in the commit-message policy
path. Use withProjectSettings and the existing
checkPowerShellCommitMessagePolicy helper alongside the current cases in the
test that exercises quoted and here-string forms, and assert that git commit -m
$msg is treated as cannot be checked / asks for confirmation rather than
returning null. This will lock in the fail-closed behavior once
PowerShellTool.tsx handles the unquoted-variable gap.
---
Outside diff comments:
In `@src/tools/BashTool/bashSecurity.ts`:
- Around line 807-827: The `bashSecurity.ts` message parsing around
`simpleMessagePattern` and `unquotedMessagePattern` is too permissive for
unquoted `-m/--message` values with shell expansion characters. Update the
parsing logic in the `matchAll` loops so any unquoted message containing `$` or
backticks is treated as uninspectable and sets `hasUninspectableSource` instead
of being partially captured into `messages`. Keep the quoted-message handling in
place, but make the unquoted branch fail closed for expandable content.
In `@src/tools/PowerShellTool/PowerShellTool.tsx`:
- Around line 264-278: The unquoted `-m`/`--message` parsing in
`PowerShellTool.tsx` is treating variable-containing values as safe literals and
bypassing the fail-closed inspection path. Update the unquoted message handling
in the `quotedPattern`/`unquotedPattern` flow inside the command inspection
logic to run `hasExpandablePowerShellText` on unquoted captures as well, and
mark `hasUninspectableSource` when expansion is possible instead of always
pushing the token into `messages`. Make the behavior consistent with the quoted
double-quoted branch so `git commit -m $msg` cannot skip policy checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fa96c050-e2a0-4280-84ca-5c290cca4fe4
📒 Files selected for processing (4)
src/tools/BashTool/bashSecurity.test.tssrc/tools/BashTool/bashSecurity.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/PowerShellTool/powershellPermissions.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: typecheck
🧰 Additional context used
📓 Path-based instructions (10)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/PowerShellTool/powershellPermissions.test.tssrc/tools/BashTool/bashSecurity.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/PowerShellTool/powershellPermissions.test.tssrc/tools/BashTool/bashSecurity.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/PowerShellTool/powershellPermissions.test.tssrc/tools/BashTool/bashSecurity.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/PowerShellTool/powershellPermissions.test.tssrc/tools/BashTool/bashSecurity.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/PowerShellTool/powershellPermissions.test.tssrc/tools/BashTool/bashSecurity.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/PowerShellTool/powershellPermissions.test.tssrc/tools/BashTool/bashSecurity.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/PowerShellTool/powershellPermissions.test.tssrc/tools/BashTool/bashSecurity.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
🔇 Additional comments (2)
src/tools/BashTool/bashSecurity.ts (1)
872-914: LGTM!src/tools/PowerShellTool/PowerShellTool.tsx (1)
368-401: LGTM!
Extend Bash and PowerShell commit-message governance to treat unquoted expandable message tokens as uninspectable under active policy. This covers Bash forms like `git commit -m fix` and PowerShell forms like `git commit -m `, alongside the quoted and heredoc cases already covered. Validation: bun test src/tools/BashTool/bashSecurity.test.ts src/tools/BashTool/bashPermissions.test.ts; bun test src/tools/PowerShellTool/powershellPermissions.test.ts; bun test src/tools/BashTool/bashSecurity.test.ts src/tools/BashTool/bashPermissions.test.ts src/tools/BashTool/readOnlyValidation.test.ts src/tools/BashTool/utils.test.ts src/tools/PowerShellTool/powershellPermissions.test.ts src/utils/permissions/filesystem.test.ts src/utils/permissions/permissions.test.ts src/memdir/paths.test.ts; bun run typecheck; bun run smoke; bun run security:pr-scan -- --base origin/main --head HEAD
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/tools/BashTool/bashSecurity.ts`:
- Around line 823-834: The `extractCommitMessages` parsing in `bashSecurity.ts`
is treating a bare `$(cat` as if it were safe, which leaves
`hasUninspectableSource` false and allows the policy check to proceed
incorrectly. Update the `unquotedMessagePattern` loop so that when `message` is
`$(cat` in a plain `-m/--message` argument it is marked uninspectable instead of
skipped, and only preserve any special handling for the heredoc form when it is
matched later. Ensure `messages`, `hasUninspectableSource`, and the existing
`hasExpandableShellText`/`uninspectable` logic stay consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 93ad828b-d035-405a-ab1e-169dcdbbe13b
📒 Files selected for processing (4)
src/tools/BashTool/bashSecurity.test.tssrc/tools/BashTool/bashSecurity.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/PowerShellTool/powershellPermissions.test.ts
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: PR Checks / 0_smoke-and-tests.txt: fix: governance controls for memory and git attribution
Conclusion: failure
ss) does not use BNKR_API_KEY for non-Bankr OpenAI-compatible routes [2.00ms]
(pass) preserves Gemini tool call extra_content from streaming chunks [2.00ms]
(pass) preserves Gemini thought signature from streaming delta extra_content [1.00ms]
(pass) preserves Gemini thought signature from non-streaming message extra_content [1.00ms]
(pass) converts Gemini raw tool-call text into streaming tool_use blocks [2.00ms]
(pass) converts Gemini raw tool-call text into non-streaming tool_use blocks [2.00ms]
(pass) normalizes plain string Bash tool arguments from OpenAI-compatible responses [1.00ms]
(pass) normalizes Bash tool arguments that are valid JSON strings [1.00ms]
(pass) preserves malformed Bash JSON literals as parsed values in non-streaming responses: false [1.00ms]
(pass) preserves malformed Bash JSON literals as parsed values in non-streaming responses: null [1.00ms]
(pass) preserves malformed Bash JSON literals as parsed values in non-streaming responses: [] [1.00ms]
(pass) keeps terminal empty Bash tool arguments invalid in non-streaming responses [1.00ms]
(pass) normalizes plain string Bash tool arguments in streaming responses [2.00ms]
(pass) normalizes plain string Bash tool arguments when streaming starts with an empty chunk [2.00ms]
(pass) normalizes plain string Bash tool arguments when streaming starts with whitespace [2.00ms]
(pass) keeps terminal whitespace-only Bash arguments invalid in streaming responses [1.00ms]
(pass) normalizes streaming Bash arguments that begin with bracket syntax [2.00ms]
(pass) normalizes streaming Bash arguments when the first chunk is only an opening brace [2.00ms]
(pass) repairs truncated structured Bash JSON in streaming responses [2.00ms]
(pass) does not normalize incomplete streamed Bash commands when finish_reason is length [2.00ms]
(pass) repairs truncated JSON objects even without command field [2.00ms]
(pass) preserves raw input for unknown plain string tool arguments [1.00ms]
(pass) preserve...
GitHub Actions: PR Checks / smoke-and-tests: fix: governance controls for memory and git attribution
Conclusion: failure
up]
##[group]src/services/compact/microCompact.test.ts:
(pass) microCompact MCP tool compaction > module exports load correctly [1.00ms]
(pass) microCompact MCP tool compaction > estimateMessageTokens counts MCP tool_use blocks
(pass) microCompact MCP tool compaction > microcompactMessages processes MCP tools without error
(pass) microCompact MCP tool compaction > microcompactMessages processes mixed built-in and MCP tools [1.00ms]
(pass) microCompact MCP tool compaction > time-based microcompact clears native toolUseResult for old compacted results
##[endgroup]
##[group]src/services/compact/snipProjection.test.ts:
(pass) isSnipBoundaryMessage > returns true for message with snipMetadata [4.00ms]
(pass) isSnipBoundaryMessage > returns false for compact_boundary without snipMetadata
(pass) isSnipBoundaryMessage > returns false for regular message
(pass) isSnipBoundaryMessage > returns false for null/undefined
(pass) projectSnippedView > returns original array when no snip boundaries present
(pass) projectSnippedView > removes messages whose UUIDs appear in snipMetadata.removedUuids
(pass) projectSnippedView > accumulates removedUuids from multiple snip boundaries
(pass) projectSnippedView > handles boundaries with no removedUuids gracefully
(pass) projectSnippedView > prunes earlier messages when the boundary is appended last
##[endgroup]
##[group]src/services/compact/autoCompact.test.ts:
(pass) getEffectiveContextWindowSize > returns positive value for known models with large context windows [2.00ms]
(pass) getEffectiveContextWindowSize > never returns negative even for unknown 3P models (issue `#635`) [4.00ms]
(pass) getEffectiveContextWindowSize > uses MiniMax M2 context and output metadata for compact budget [6.00ms]
(pass) getAutoCompactThreshold > returns positive threshold for known models [1.00ms]
(pass) getAutoCompactThreshold > never returns negative threshold even for unknown 3P models (issue `#635`) [4.00ms]
(pass) getAutoCompactFa...
🧰 Additional context used
📓 Path-based instructions (10)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/BashTool/bashSecurity.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/BashTool/bashSecurity.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/BashTool/bashSecurity.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/BashTool/bashSecurity.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/BashTool/bashSecurity.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/BashTool/bashSecurity.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/BashTool/bashSecurity.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
🔇 Additional comments (3)
src/tools/BashTool/bashSecurity.test.ts (1)
314-321: LGTM!src/tools/PowerShellTool/PowerShellTool.tsx (1)
277-282: LGTM!Also applies to: 583-586
src/tools/PowerShellTool/powershellPermissions.test.ts (1)
301-311: LGTM!Also applies to: 317-327
Treat plain unquoted command substitutions like git commit -m $(cat .git/OPENCLAUDE_COMMIT_MSG) as uninspectable while still letting the heredoc-specific parser handle literal cat heredoc commit messages. Validation: bun test src/tools/BashTool/bashSecurity.test.ts src/tools/BashTool/bashPermissions.test.ts; bun test src/tools/BashTool/bashSecurity.test.ts src/tools/BashTool/bashPermissions.test.ts src/tools/BashTool/readOnlyValidation.test.ts src/tools/BashTool/utils.test.ts src/tools/PowerShellTool/powershellPermissions.test.ts src/utils/permissions/filesystem.test.ts src/utils/permissions/permissions.test.ts src/memdir/paths.test.ts; bun run typecheck; bun run security:pr-scan -- --base origin/main --head HEAD; bun run smoke
Make getEnhancedPRAttribution use the same generated PR attribution opt-in and block semantics as getAttributionTexts, so git.addGeneratedWithFooter enables the enhanced PR footer path and false blocks legacy generated PR attribution. Validation: bun test src/utils/attribution.test.ts; bun test src/utils/governancePolicy.test.ts src/utils/attribution.test.ts src/commands/commit-push-pr.test.ts (commit-push-pr test path absent, 32 tests ran); bun run typecheck; bun run security:pr-scan -- --base origin/main --head HEAD; bun run smoke; git diff --check
Recognize bare git commit as an uninspectable commit-message source in both Bash and PowerShell governance checks, so active forbidden-pattern policy prompts instead of passing through the editor/template path. Validation: bun test --feature=UNATTENDED_RETRY src/tools/BashTool/bashSecurity.test.ts src/tools/BashTool/bashPermissions.test.ts src/tools/PowerShellTool/powershellPermissions.test.ts; bun run typecheck; bun run smoke; bun run security:pr-scan -- --base origin/main --head HEAD; git diff --check
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/tools/BashTool/bashSecurity.ts (2)
885-927: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFail-closed decision flow looks correct.
Bare
git commit,-F/--file, and uninspectable/expandable sources all route toaskunder active policy, which matches the governance intent and the earlier fail-closed feedback. No blocker here.One non-blocking note: the AI-attribution marker regex
/Co-Authored-By:|Generated with/i(Line 918) is duplicated verbatim insrc/tools/PowerShellTool/PowerShellTool.tsx(Line 393). Since this is security-sensitive policy, consider hoisting the marker predicate intogovernancePolicyso the Bash and PowerShell paths can't silently diverge if the marker set changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tools/BashTool/bashSecurity.ts` around lines 885 - 927, The AI-attribution marker check is duplicated in checkBashCommitMessagePolicy and the PowerShell policy path, which risks the two implementations diverging over time. Hoist the marker predicate into the shared governancePolicy layer (or another common helper) and have checkBashCommitMessagePolicy use that shared source of truth so both Bash and PowerShell enforcement stay consistent.
929-938: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueMinor: redundant re-normalization.
normalizeGitCommitCommand(context.originalCommand)runs here again aftercheckBashCommitMessagePolicyalready normalized internally, purely to pick a passthrough message string. Both branches returnpassthrough, so the second normalize call is wasted regex work on every non-violating command. Not blocking — collapse to a single passthrough if you touch this again.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tools/BashTool/bashSecurity.ts` around lines 929 - 938, The issue is redundant re-normalization in validateGitCommitMessagePolicy, where normalizeGitCommitCommand(context.originalCommand) is called again even though checkBashCommitMessagePolicy already did the normalization work. Update validateGitCommitMessagePolicy to avoid the second normalize call and return a single passthrough result for non-violations, using the existing checkBashCommitMessagePolicy flow as the source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/tools/BashTool/bashSecurity.ts`:
- Around line 885-927: The AI-attribution marker check is duplicated in
checkBashCommitMessagePolicy and the PowerShell policy path, which risks the two
implementations diverging over time. Hoist the marker predicate into the shared
governancePolicy layer (or another common helper) and have
checkBashCommitMessagePolicy use that shared source of truth so both Bash and
PowerShell enforcement stay consistent.
- Around line 929-938: The issue is redundant re-normalization in
validateGitCommitMessagePolicy, where
normalizeGitCommitCommand(context.originalCommand) is called again even though
checkBashCommitMessagePolicy already did the normalization work. Update
validateGitCommitMessagePolicy to avoid the second normalize call and return a
single passthrough result for non-violations, using the existing
checkBashCommitMessagePolicy flow as the source of truth.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 40a28c16-0177-44ab-98d5-a78d2ecc4bee
📒 Files selected for processing (4)
src/tools/BashTool/bashSecurity.test.tssrc/tools/BashTool/bashSecurity.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/PowerShellTool/powershellPermissions.test.ts
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: PR Checks / smoke-and-tests: fix: governance controls for memory and git attribution
Conclusion: failure
ervices/compact/microCompact.test.ts:
(pass) microCompact MCP tool compaction > module exports load correctly [1.00ms]
(pass) microCompact MCP tool compaction > estimateMessageTokens counts MCP tool_use blocks
(pass) microCompact MCP tool compaction > microcompactMessages processes MCP tools without error
(pass) microCompact MCP tool compaction > microcompactMessages processes mixed built-in and MCP tools
(pass) microCompact MCP tool compaction > time-based microcompact clears native toolUseResult for old compacted results [1.00ms]
##[endgroup]
##[group]src/services/compact/snipProjection.test.ts:
(pass) isSnipBoundaryMessage > returns true for message with snipMetadata
(pass) isSnipBoundaryMessage > returns false for compact_boundary without snipMetadata
(pass) isSnipBoundaryMessage > returns false for regular message
(pass) isSnipBoundaryMessage > returns false for null/undefined
(pass) projectSnippedView > returns original array when no snip boundaries present
(pass) projectSnippedView > removes messages whose UUIDs appear in snipMetadata.removedUuids
(pass) projectSnippedView > accumulates removedUuids from multiple snip boundaries
(pass) projectSnippedView > handles boundaries with no removedUuids gracefully
(pass) projectSnippedView > prunes earlier messages when the boundary is appended last
##[endgroup]
##[group]src/services/compact/autoCompact.test.ts:
(pass) getEffectiveContextWindowSize > returns positive value for known models with large context windows [2.00ms]
(pass) getEffectiveContextWindowSize > never returns negative even for unknown 3P models (issue `#635`) [5.00ms]
(pass) getEffectiveContextWindowSize > uses MiniMax M2 context and output metadata for compact budget [1.00ms]
(pass) getAutoCompactThreshold > returns positive threshold for known models [1.00ms]
(pass) getAutoCompactThreshold > never returns negative threshold even for unknown 3P models (issue `#635`) [4.00ms]
(pass) getAutoCompactFailureCooldownMs > uses valid...
GitHub Actions: PR Checks / 2_smoke-and-tests.txt: fix: governance controls for memory and git attribution
Conclusion: failure
NAI_API_KEYS permanently evicts 403 auth failures [6.00ms]
(pass) does not use BNKR_API_KEY for non-Bankr OpenAI-compatible routes [2.00ms]
(pass) preserves Gemini tool call extra_content from streaming chunks [2.00ms]
(pass) preserves Gemini thought signature from streaming delta extra_content [1.00ms]
(pass) preserves Gemini thought signature from non-streaming message extra_content [2.00ms]
(pass) converts Gemini raw tool-call text into streaming tool_use blocks [1.00ms]
(pass) converts Gemini raw tool-call text into non-streaming tool_use blocks [2.00ms]
(pass) normalizes plain string Bash tool arguments from OpenAI-compatible responses [1.00ms]
(pass) normalizes Bash tool arguments that are valid JSON strings [1.00ms]
(pass) preserves malformed Bash JSON literals as parsed values in non-streaming responses: false [1.00ms]
(pass) preserves malformed Bash JSON literals as parsed values in non-streaming responses: null [1.00ms]
(pass) preserves malformed Bash JSON literals as parsed values in non-streaming responses: []
(pass) keeps terminal empty Bash tool arguments invalid in non-streaming responses [1.00ms]
(pass) normalizes plain string Bash tool arguments in streaming responses [2.00ms]
(pass) normalizes plain string Bash tool arguments when streaming starts with an empty chunk [2.00ms]
(pass) normalizes plain string Bash tool arguments when streaming starts with whitespace [2.00ms]
(pass) keeps terminal whitespace-only Bash arguments invalid in streaming responses [2.00ms]
(pass) normalizes streaming Bash arguments that begin with bracket syntax [2.00ms]
(pass) normalizes streaming Bash arguments when the first chunk is only an opening brace [2.00ms]
(pass) repairs truncated structured Bash JSON in streaming responses [2.00ms]
(pass) does not normalize incomplete streamed Bash commands when finish_reason is length [1.00ms]
(pass) repairs truncated JSON objects even without command field [2.00ms]
(pass) preserves raw input for unknown...
🧰 Additional context used
📓 Path-based instructions (10)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with strict mode and ESM imports
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/PowerShellTool/powershellPermissions.test.tssrc/tools/BashTool/bashSecurity.ts
**/*.{ts,tsx,js,jsx,py,json,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow the existing code style in the touched files
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/PowerShellTool/powershellPermissions.test.tssrc/tools/BashTool/bashSecurity.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add or update tests when the change affects behavior
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
**/*.test.{ts,tsx,js}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Test the exact provider/model path you changed when possible
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
**/*.{ts,tsx,js,jsx,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Keep comments useful and concise
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/PowerShellTool/powershellPermissions.test.tssrc/tools/BashTool/bashSecurity.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Follow TypeScript strict mode and type safety practices by running typecheck before submitting
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/PowerShellTool/powershellPermissions.test.tssrc/tools/BashTool/bashSecurity.ts
**
⚙️ CodeRabbit configuration file
**: # AGENTS.md - AI Agent Coding GuideThis guide is for AI coding agents working in the OpenClaude repository. Read it before changing code, and also follow CONTRIBUTING.md for contributor policy, PR expectations, review follow-up, and project scope.
Project Snapshot
OpenClaude is a coding-agent CLI for cloud and local model providers. It supports OpenAI-compatible APIs, Anthropic, Gemini, DeepSeek, Ollama, MCP, local backends, slash commands, tools, agents, and a React/Ink terminal UI.
The installed CLI runs on Node.js
>=22.0.0. Bun is used for source builds, scripts, dependency management, and tests.Work Style
- Keep changes focused on one problem.
- Prefer existing patterns in the file or nearby module.
- Avoid unrelated formatting, renames, dependency changes, or broad rewrites.
- Add or update tests when behavior changes.
- Update docs when setup, commands, provider behavior, or user-facing behavior changes.
- For new features, larger refactors, dependencies, or runtime changes, follow the issue-first guidance in CONTRIBUTING.md.
Stack And Conventions
- TypeScript with strict mode and ESM imports.
- React + Ink for terminal UI.
- Bun lockfile and Bun scripts for development workflows.
- Node runtime for the built CLI.
- Python exists for legacy/local-provider helper code. Do not add new Python code or expand Python-based features unless a maintainer explicitly approves that direction.
Common libraries and patterns:
chalkfor terminal color.commanderfor CLI argument parsing.execafor child processes.- Existing service, provider, settings, permission, and UI patterns over new abstractions.
Repository Map
src/commands/- slash and CLI command implementations.src/components/- React/Ink UI components.src/services/- API, MCP, OAuth, wiki, voice, and other service integrations.src/tools/- tool implementations.src/utils/- shared utilities.- `src/integration...
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/PowerShellTool/powershellPermissions.test.tssrc/tools/BashTool/bashSecurity.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/PowerShellTool/powershellPermissions.test.tssrc/tools/BashTool/bashSecurity.ts
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**
⚙️ CodeRabbit configuration file
src/{components/permissions,utils/permissions,hooks/toolPermission,tools,entrypoints/sdk}/**: Review permission prompts, auto-allow logic, sandbox behavior, SDK permission schemas, shell/PowerShell execution, and background execution paths as security-sensitive. Block on bypasses, unclear trust boundaries, unsafe path handling, missing user visibility, or changes that broaden allowed behavior without an explicit maintainer decision.
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/PowerShellTool.tsxsrc/tools/PowerShellTool/powershellPermissions.test.tssrc/tools/BashTool/bashSecurity.ts
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}
⚙️ CodeRabbit configuration file
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.
Files:
src/tools/BashTool/bashSecurity.test.tssrc/tools/PowerShellTool/powershellPermissions.test.ts
🔇 Additional comments (6)
src/tools/PowerShellTool/PowerShellTool.tsx (3)
373-406: Fail-closed logic mirrors the Bash path correctly. The duplicated attribution marker regex at Line 393 is called out at its root cause inbashSecurity.ts; no separate action needed here.
213-249: LGTM!
583-587: LGTM!src/tools/BashTool/bashSecurity.ts (1)
754-790: LGTM!src/tools/BashTool/bashSecurity.test.ts (1)
284-295: LGTM!src/tools/PowerShellTool/powershellPermissions.test.ts (1)
263-280: LGTM!
|
@kevincodex1 LGTM |
* fix: add governance controls for memory and git attribution Default persistent memory writes to explicit approval unless memory.requireApprovalBeforeWrite is disabled. Skip automatic memory extraction/dream writes while approval is required, avoid auto-creating memory directories in that mode, and surface approval prompts for auto-memory file writes. Make generated commit and PR attribution opt-in through git.addAICoAuthor and git.addGeneratedWithFooter while preserving explicit custom attribution. Add forbidden commit-message patterns and surface them in commit prompts. Enforce commit-message governance in Bash and PowerShell permission checks, including global git options, file-backed messages, shell wrappers, env split strings, chained commands, and PowerShell call/operator forms. Add regression coverage for the policy paths. * test: isolate governance policy settings state * fix: address governance policy review findings * fix: require approval for overridden memory writes * fix: fail closed on expandable commit messages Treat Bash and PowerShell expandable commit-message sources as uninspectable when commit-message governance policy is active. This prevents runtime-expanded variables, command substitutions, and expandable here-strings from bypassing forbidden attribution checks while preserving literal quoted heredocs/here-strings. Validation: bun test src/tools/BashTool/bashSecurity.test.ts src/tools/BashTool/bashPermissions.test.ts; bun test src/tools/PowerShellTool/powershellPermissions.test.ts; bun test src/tools/BashTool/bashSecurity.test.ts src/tools/BashTool/bashPermissions.test.ts src/tools/BashTool/readOnlyValidation.test.ts src/tools/BashTool/utils.test.ts src/tools/PowerShellTool/powershellPermissions.test.ts src/utils/permissions/filesystem.test.ts src/utils/permissions/permissions.test.ts src/memdir/paths.test.ts; bun run typecheck; bun run smoke; bun run security:pr-scan -- --base origin/main --head HEAD * fix: fail closed on unquoted expandable commit messages Extend Bash and PowerShell commit-message governance to treat unquoted expandable message tokens as uninspectable under active policy. This covers Bash forms like `git commit -m fix` and PowerShell forms like `git commit -m `, alongside the quoted and heredoc cases already covered. Validation: bun test src/tools/BashTool/bashSecurity.test.ts src/tools/BashTool/bashPermissions.test.ts; bun test src/tools/PowerShellTool/powershellPermissions.test.ts; bun test src/tools/BashTool/bashSecurity.test.ts src/tools/BashTool/bashPermissions.test.ts src/tools/BashTool/readOnlyValidation.test.ts src/tools/BashTool/utils.test.ts src/tools/PowerShellTool/powershellPermissions.test.ts src/utils/permissions/filesystem.test.ts src/utils/permissions/permissions.test.ts src/memdir/paths.test.ts; bun run typecheck; bun run smoke; bun run security:pr-scan -- --base origin/main --head HEAD * fix: fail closed on unquoted cat substitutions Treat plain unquoted command substitutions like git commit -m $(cat .git/OPENCLAUDE_COMMIT_MSG) as uninspectable while still letting the heredoc-specific parser handle literal cat heredoc commit messages. Validation: bun test src/tools/BashTool/bashSecurity.test.ts src/tools/BashTool/bashPermissions.test.ts; bun test src/tools/BashTool/bashSecurity.test.ts src/tools/BashTool/bashPermissions.test.ts src/tools/BashTool/readOnlyValidation.test.ts src/tools/BashTool/utils.test.ts src/tools/PowerShellTool/powershellPermissions.test.ts src/utils/permissions/filesystem.test.ts src/utils/permissions/permissions.test.ts src/memdir/paths.test.ts; bun run typecheck; bun run security:pr-scan -- --base origin/main --head HEAD; bun run smoke * fix: honor PR footer governance opt-in Make getEnhancedPRAttribution use the same generated PR attribution opt-in and block semantics as getAttributionTexts, so git.addGeneratedWithFooter enables the enhanced PR footer path and false blocks legacy generated PR attribution. Validation: bun test src/utils/attribution.test.ts; bun test src/utils/governancePolicy.test.ts src/utils/attribution.test.ts src/commands/commit-push-pr.test.ts (commit-push-pr test path absent, 32 tests ran); bun run typecheck; bun run security:pr-scan -- --base origin/main --head HEAD; bun run smoke; git diff --check * fix: fail closed on bare git commit governance Recognize bare git commit as an uninspectable commit-message source in both Bash and PowerShell governance checks, so active forbidden-pattern policy prompts instead of passing through the editor/template path. Validation: bun test --feature=UNATTENDED_RETRY src/tools/BashTool/bashSecurity.test.ts src/tools/BashTool/bashPermissions.test.ts src/tools/PowerShellTool/powershellPermissions.test.ts; bun run typecheck; bun run smoke; bun run security:pr-scan -- --base origin/main --head HEAD; git diff --check --------- Co-authored-by: jatmn <jatmn@users.noreply.github.com> (cherry picked from commit 00b8c15)
…ne#1806 followup) Fixes 5 attribution.test.ts runtime-divergence fails from Twigpine#1806 port: - adds codename fallback for unknown first-party models - sanitizes unknown internal co-author names - avoids duplicate Claude prefix - skips Claude-internal formatting for non-Claude providers OpenCC rebrand preserved (opencc not openclaude). (cherry picked from commit 7979cadd3b18d7c3cfce26c4b2d437914239e3a6)
…ne#1806 followup) Fixes 4 paths.test.ts runtime-divergence fails from Twigpine#1806 port: - honor new memory.autoWrite: false opt-out - preserve legacy autoMemoryEnabled: false back-compat - lower-priority false survives higher-priority true (explicit opt-out wins) - parent autoMemoryEnabled: false not re-enabled by child memory.autoWrite: true (cherry picked from commit 5e86243f2fd352f0b77bec954d13f68d063bb11d)
…ine#1806 followup) Fixes 22 runtime-divergence test fails: - bashSecurity.test.ts: 11 pass / 0 fail (was 11 fail) - bashPermissions.test.ts: 1 pass / 0 fail - powershellPermissions.test.ts: 10 pass / 0 fail Root cause: cherry-pick cfcfd48 kept the upstream .openclaude/ fixture directory while OpenCC's settings loader still reads from .claude/ (per src/utils/settings/settings.ts getRelativeSettingsFilePathForSource). Tests wrote policy settings to a directory the runtime never opened, so forbiddenCommitMessagePatterns + addAICoAuthor never reached the runtime. Wiring in bashSecurity.ts, bashPermissions.ts and PowerShellTool.tsx was already present; no helper rewrites needed. (cherry picked from commit a1d4065658acfe5b214bb22bba824fa09c94c31c)
…cherry-pick) The toolExecution.test.ts that was cherry-picked in cfcfd48 (Twigpine#1806 governance) referenced replay symbols (ReplayIndexBuilder, getReplayIndexBuilder, resetAllReplayIndexBuilders) but the producer code was never ported to OpenCC. The test file at HEAD was @ts-nocheck-masked and ReferenceError-broken at module load. Port the minimum producer surface from upstream main (203f055): the ReplayIndexBuilder class + factory/reset wiring needed by the test file. Test file imports were also missing several symbols that the same upstream test file imports — added afterEach, QueryLifecycleOperationTracker, createToolFixture, CanUseToolFn, getDefaultAppState, SkillTool, runToolUse, MessageUpdateLazy, plus getReplayIndexBuilder / ReplayIndexBuilder. src/utils/replayIndexBuilder.ts: - 248-line class ported verbatim from upstream main, repathed to '../types/logs.js' (was 'src/types/logs.js') and './stableStringify.js' (was 'src/utils/stableStringify.js'). - ReplayUserStep import was unused — removed. src/types/logs.ts: - Added 7 Replay* types (ReplayStep, ReplayToolStep, ReplayUserStep, ReplayRetryStep, ReplayErrorStep, ReplaySummary, ReplayIndex) ported from upstream logs.ts:378-478. src/bootstrap/state.ts: - Added ReplayIndexBuilderEntry type. - Added replayIndexBuilders: Map<SessionId, ReplayIndexBuilderEntry> to State type and initialized it in getInitialState(). - Added 3 exports (getReplayIndexBuilder, resetReplayIndexBuilder, resetAllReplayIndexBuilders) ported from upstream state.ts:1683-1726. src/services/tools/toolExecution.test.ts: - Added all missing imports (afterEach, queryLifecycle types, replay types, test fixtures, getDefaultAppState, SkillTool, runToolUse, MessageUpdateLazy). Test delta: before: 4354 pass / 147 skip / 55 fail / 1 error after: 4394 pass / 147 skip / 66 fail / 0 error +47 pass (lifecycle tests now run; some pass on the replay-only path). +11 fail (real production gaps in runToolUse — see followup). -1 error (afterEach + QueryLifecycleOperationTracker + 3 replay helpers now resolve cleanly). NOT YET PORTED (followup work): - runToolUse() in src/services/tools/toolExecution.ts does not call queryLifecycle.startToolUse / endToolUse / updateToolUse. 7 "query lifecycle tool-use cleanup" tests fail because of this. Needs ~50 lines ported from upstream toolExecution.ts:871-911 (trackLifecycleToolUse + endLifecycleToolUse closures, plus createLifecycleToolUse helper). - runToolUse() does not call getReplayIndexBuilder().trackToolStart / trackToolEnd at the 4 integration points (upstream toolExecution.ts:1213, 1424, 1766, 1785). 3 "replay tool lifecycle records" tests fail because of this. - 2 getReplayModifiedFiles tests fail; likely small test-setup mismatch (need investigation). Typecheck clean. No new lint warnings.
2 participants
Summary
Fixes #1326.
This adds explicit governance controls for OpenClaude memory writes and generated git attribution, then wires those controls through the places that can write memory or create commit/PR attribution.
What changed
memory.requireApprovalBeforeWrite,git.addAICoAuthor,git.addGeneratedWithFooter, andgit.forbiddenCommitMessagePatterns.User / developer impact
Governed repositories can now block unattended memory writes and prevent generated attribution from being added unless explicitly allowed. Attempts to commit messages containing configured forbidden patterns are routed through bypass-resistant permission prompts.
Checks run
bun installPATH=/tmp/openclaude-test-tools/node_modules/.bin:$PATH bun run buildPATH=/tmp/openclaude-test-tools/node_modules/.bin:$PATH bun run smokePATH=/tmp/openclaude-test-tools/node_modules/.bin:$PATH GIT_AUTHOR_NAME=OpenClaude GIT_AUTHOR_EMAIL=openclaude@example.com GIT_COMMITTER_NAME=OpenClaude GIT_COMMITTER_EMAIL=openclaude@example.com bun run check5290 pass, 0 fail127.0.0.1.PATH=/tmp/openclaude-test-tools/node_modules/.bin:$PATH bun run typecheckScreenshots
Not applicable; no UI or terminal presentation changes.
Provider path tested
Not applicable; no provider behavior changed.
Summary by CodeRabbit
memory.requireApprovalBeforeWriteand git governance fields for attribution and forbidden patterns.