feat: implement built-in git tools with security hardening#150
feat: implement built-in git tools with security hardening#150
Conversation
Add 6 workspace-scoped git tools for version control operations: GitStatusTool, GitLogTool, GitDiffTool, GitBranchTool, GitCommitTool, and GitCloneTool. All tools enforce workspace boundary security via path validation and use async subprocess execution with GIT_TERMINAL_PROMPT=0 to prevent interactive prompts. - Add _BaseGitTool shared base class with _run_git() and _validate_path() - Add GitToolError to tools/errors.py - Add git event constants to observability/events/git.py - Add 68 unit tests covering all tools, workspace validation, and edge cases - Re-export all git tools from tools/__init__.py Closes #132 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Security hardening: - Add _check_ref() to reject flag injection via ref/branch/name params - Add _is_allowed_clone_url() to validate clone URL schemes - Set GIT_CONFIG_NOSYSTEM=1 to prevent config-based attacks - Widen subprocess error catch from FileNotFoundError to OSError Code quality: - Change GIT_COMMAND_FAILED log level from INFO to WARNING - Remove unused GitToolError (dead code) - Eliminate type: ignore suppressions in GitBranchTool - Add explicit action guard (no unguarded delete fallthrough) - Extract _list_branches/_create_branch helpers for readability Tests: - Add TestFlagInjectionPrevention (6 tests) - Add TestCloneUrlValidation (6 tests) - Tighten test_empty_repo assertion - Add allow_local_clone fixture for functional clone tests - Deduplicate parametrize lists via _ALL_GIT_TOOL_CLASSES Docs: - DESIGN_SPEC: add events/git.py, remove (M3) tag from git_tools.py - DESIGN_SPEC: add implementation note about built-in git tools - CLAUDE.md: update tools/ package description Pre-reviewed by 9 agents, 16 findings addressed
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a suite of built-in Git tools designed to empower agents with essential version control capabilities directly within their workspace. The primary focus has been on ensuring these tools are not only functional but also highly secure, mitigating common vulnerabilities like command injection and path traversal. This enhancement significantly expands the agents' operational scope while maintaining a strong security posture. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughIntroduces six built-in git tools (status, log, diff, branch, commit, clone) with workspace boundary enforcement, validation helpers, subprocess execution, and event logging. Includes event constant definitions, documentation updates, test fixtures, and comprehensive unit tests exercising all operations and edge cases. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Tool
participant Validation
participant Subprocess
participant Logging
participant Result
Client->>Tool: execute(arguments)
Tool->>Validation: validate paths/refs/workspace
alt Validation Fails
Validation->>Result: ToolExecutionResult(error)
Result->>Client: error response
else Validation Succeeds
Logging->>Logging: log GIT_COMMAND_START
Tool->>Subprocess: run git command (async)
Subprocess-->>Tool: stdout/stderr/exit code
alt Command Succeeds
Logging->>Logging: log GIT_COMMAND_SUCCESS
Tool->>Result: ToolExecutionResult(output)
else Command Timeout
Logging->>Logging: log GIT_COMMAND_TIMEOUT
Tool->>Result: ToolExecutionResult(timeout error)
else Command Fails
Logging->>Logging: log GIT_COMMAND_FAILED
Tool->>Result: ToolExecutionResult(git error)
end
Result->>Client: result with status
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a suite of six built-in git tools with significant security hardening measures, featuring a well-structured implementation and comprehensive test coverage. However, a critical vulnerability exists in the GitCloneTool where it lacks protections for url, directory, and branch parameters, potentially allowing flag injection and Remote Code Execution (RCE) via options like ProxyCommand. Additionally, there's a missing security environment variable, an opportunity to enhance clone URL validation, and a minor inconsistency in the test environment setup.
| args.append(url) | ||
|
|
||
| if directory := arguments.get("directory"): | ||
| if err := self._check_paths([directory]): | ||
| return err | ||
| args.append(directory) |
There was a problem hiding this comment.
The GitCloneTool is vulnerable to flag injection. The url and directory parameters are taken from user input and passed to the git clone command as positional arguments without the use of the -- separator. An attacker can provide a malicious URL or directory name starting with - to inject arbitrary git flags. For example, a URL like -oProxyCommand=touch /tmp/pwned @host:path would pass the _is_allowed_clone_url check but would be interpreted as a flag by git clone, potentially leading to Remote Code Execution (RCE).
| args.append(url) | |
| if directory := arguments.get("directory"): | |
| if err := self._check_paths([directory]): | |
| return err | |
| args.append(directory) | |
| args.append("--") | |
| args.append(url) | |
| if directory := arguments.get("directory"): | |
| if err := self._check_paths([directory]): | |
| return err | |
| args.append(directory) |
src/ai_company/tools/git_tools.py
Outdated
| env = { | ||
| **os.environ, | ||
| "GIT_TERMINAL_PROMPT": "0", | ||
| "GIT_CONFIG_NOSYSTEM": "1", | ||
| } |
There was a problem hiding this comment.
The DESIGN_SPEC.md and the PR description mention using GIT_PROTOCOL_FROM_USER=0 as a security hardening measure to restrict protocol attack surfaces. This environment variable is missing from the env dictionary in _run_git. Please add it to align with the specified security measures.
| env = { | |
| **os.environ, | |
| "GIT_TERMINAL_PROMPT": "0", | |
| "GIT_CONFIG_NOSYSTEM": "1", | |
| } | |
| env = { | |
| **os.environ, | |
| "GIT_TERMINAL_PROMPT": "0", | |
| "GIT_CONFIG_NOSYSTEM": "1", | |
| "GIT_PROTOCOL_FROM_USER": "0", | |
| } |
| if branch := arguments.get("branch"): | ||
| args.extend(["--branch", branch]) |
There was a problem hiding this comment.
The GitCloneTool's branch parameter is not validated against flag injection using the _check_ref method, which is inconsistent with other git tools in this PR. This omission could allow a malicious value (e.g., starting with -) to be passed, potentially leading to flag injection or command injection. It is recommended to use the _check_ref helper for validation and add a corresponding test case in TestFlagInjectionPrevention.
| if branch := arguments.get("branch"): | |
| args.extend(["--branch", branch]) | |
| if branch := arguments.get("branch"): | |
| if err := self._check_ref(branch, param="branch"): | |
| return err | |
| args.extend(["--branch", branch]) |
src/ai_company/tools/git_tools.py
Outdated
| # SCP-like: user@host:path (e.g. git@github.com:user/repo.git) | ||
| return "@" in url and ":" in url and "::" not in url and "://" not in url |
There was a problem hiding this comment.
The current check for SCP-like URLs is a bit too permissive and could allow some invalid formats, such as those with multiple colons (e.g., user@host:path:other) or Windows-style paths. A more robust validation would improve security by being more specific about the expected format.
| # SCP-like: user@host:path (e.g. git@github.com:user/repo.git) | |
| return "@" in url and ":" in url and "::" not in url and "://" not in url | |
| # SCP-like: user@host:path (e.g. git@github.com:user/repo.git) | |
| if "://" in url or "::" in url: | |
| return False | |
| parts = url.split(":") | |
| return len(parts) == 2 and "@" in parts[0] |
| _GIT_ENV = { | ||
| **os.environ, | ||
| "GIT_AUTHOR_NAME": "Test", | ||
| "GIT_AUTHOR_EMAIL": "test@test.local", | ||
| "GIT_COMMITTER_NAME": "Test", | ||
| "GIT_COMMITTER_EMAIL": "test@test.local", | ||
| "GIT_TERMINAL_PROMPT": "0", | ||
| "GIT_CONFIG_NOSYSTEM": "1", | ||
| } |
There was a problem hiding this comment.
The _GIT_ENV fixture is missing the GIT_PROTOCOL_FROM_USER=0 environment variable, which is part of the security hardening for the git tools. To ensure tests run in an environment that is as close to production as possible, please add this variable to the fixture.
| _GIT_ENV = { | |
| **os.environ, | |
| "GIT_AUTHOR_NAME": "Test", | |
| "GIT_AUTHOR_EMAIL": "test@test.local", | |
| "GIT_COMMITTER_NAME": "Test", | |
| "GIT_COMMITTER_EMAIL": "test@test.local", | |
| "GIT_TERMINAL_PROMPT": "0", | |
| "GIT_CONFIG_NOSYSTEM": "1", | |
| } | |
| _GIT_ENV = { | |
| **os.environ, | |
| "GIT_AUTHOR_NAME": "Test", | |
| "GIT_AUTHOR_EMAIL": "test@test.local", | |
| "GIT_COMMITTER_NAME": "Test", | |
| "GIT_COMMITTER_EMAIL": "test@test.local", | |
| "GIT_TERMINAL_PROMPT": "0", | |
| "GIT_CONFIG_NOSYSTEM": "1", | |
| "GIT_PROTOCOL_FROM_USER": "0", | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ai_company/tools/git_tools.py`:
- Around line 182-186: The environment dict that sets git-related env vars (the
dict that currently spreads os.environ and sets "GIT_TERMINAL_PROMPT" and
"GIT_CONFIG_NOSYSTEM") is missing "GIT_PROTOCOL_FROM_USER":"0"; update that env
mapping to include "GIT_PROTOCOL_FROM_USER": "0" alongside the existing keys to
restrict dangerous remote protocols (i.e., modify the env dict in the same scope
where those keys are set).
- Around line 34-49: The SCP-like URL check in _is_allowed_clone_url is subtle;
add a concise inline comment above the return that explains the SCP pattern
being matched (e.g. "SCP-like syntax: user@host:path, e.g.
git@github.com:user/repo.git") and note why checks for "::" and "://" are
required to reject ext:: and normal URL schemes respectively; keep the logic
unchanged but document the intent so future maintainers understand the
security-relevant heuristic.
- Around line 806-807: The branch value is inserted into the git CLI args
without validation; before extending args with ["--branch", branch] validate
branch using the existing _check_ref helper (or equivalent) to reject values
that start with "-" or contain unsafe characters, and only then append it;
update the block that reads `if branch := arguments.get("branch"):` to call
`_check_ref(branch)` (or raise/return an error if invalid) prior to
`args.extend(["--branch", branch])` so flag-injection like "--upload-pack=evil"
is prevented.
In `@tests/unit/tools/git/conftest.py`:
- Around line 47-55: The git_repo fixture needs to configure a local git user
identity so commits succeed in CI: inside the git_repo fixture (which calls
_run_git and returns tmp_path) run _run_git to set user.name and user.email in
the repo (use the same _run_git helper and _GIT_ENV pattern used elsewhere) so
the repo has a persistent local identity and subsequent calls from
GitCommitTool.execute (which uses os.environ) can create commits; update the
git_repo setup to call _run_git(["config", "user.name", "Test User"], tmp_path)
and _run_git(["config", "user.email", "test@example.com"], tmp_path) (or
equivalent values) before making commits.
In `@tests/unit/tools/git/test_git_tools.py`:
- Around line 374-389: Tests fail because commits lack author identity; update
the tests to configure a repo-local git identity (rather than changing
GitCommitTool._run_git). In the git_repo_with_changes fixture or before calling
GitCommitTool.execute in the failing tests, run the same setup used in conftest
(apply the _GIT_ENV helper or run git config user.name and git config user.email
inside the test repo) so the repository has user.name and user.email set; this
ensures GitCommitTool._run_git (which currently only sets GIT_TERMINAL_PROMPT
and GIT_CONFIG_NOSYSTEM) can successfully create commits without modifying the
tool itself.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d6b92191-1182-43d4-970c-caa804a06615
📒 Files selected for processing (9)
CLAUDE.mdDESIGN_SPEC.mdsrc/ai_company/observability/events/git.pysrc/ai_company/tools/__init__.pysrc/ai_company/tools/git_tools.pytests/unit/observability/test_events.pytests/unit/tools/git/__init__.pytests/unit/tools/git/conftest.pytests/unit/tools/git/test_git_tools.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Agent
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use Python 3.14+ with PEP 649 native lazy annotations
Do NOT usefrom __future__ import annotations—Python 3.14 has PEP 649
Useexcept A, B:syntax (no parentheses) for exception handling on Python 3.14—ruff enforces this
Add type hints to all public functions in Python; mypy strict mode is enforced
Use Google-style docstrings on all public classes and functions—ruff D rules enforce this
Create new objects instead of mutating existing ones; usecopy.deepcopy()at construction for non-Pydantic internal collections andMappingProxyTypewrapping for read-only enforcement
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Use Pydantic v2 withBaseModel,model_validator,computed_field, andConfigDict
Use@computed_fieldfor derived values instead of storing + validating redundant fields (e.g.TokenUsage.total_tokens)
UseNotBlankStr(fromcore.types) for all identifier/name fields—including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants—instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over barecreate_task
Enforce line length of 88 characters (ruff enforces this)
Functions should be less than 50 lines, files less than 800 lines
Handle errors explicitly; never silently swallow errors in Python code
Validate at system boundaries (user input, external APIs, config files)
Files:
src/ai_company/observability/events/git.pytests/unit/observability/test_events.pytests/unit/tools/git/conftest.pytests/unit/tools/git/test_git_tools.pysrc/ai_company/tools/git_tools.pysrc/ai_company/tools/__init__.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Every module with business logic MUST importfrom ai_company.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging,logging.getLogger(), orprint()in application code
Always useloggeras the variable name for loggers (not_logger, notlog)
Use event name constants from domain-specific modules underai_company.observability.events(e.g.PROVIDER_CALL_STARTfromevents.provider,BUDGET_RECORD_ADDEDfromevents.budget). Import directly:from ai_company.observability.events.<domain> import EVENT_CONSTANT
Always use structured logging withlogger.info(EVENT, key=value)format—neverlogger.info('msg %s', val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO level
DEBUG level logging should be used for object creation, internal flow, entry/exit of key functions
Pure data models, enums, and re-exports do NOT need logging
Files:
src/ai_company/observability/events/git.pysrc/ai_company/tools/git_tools.pysrc/ai_company/tools/__init__.py
{src/**/*.py,tests/**/*.py,src/**/*.yaml,src/**/*.yml,tests/**/*.yaml,tests/**/*.yml,examples/**/*.yaml,examples/**/*.yml}
📄 CodeRabbit inference engine (CLAUDE.md)
NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:
example-provider,example-large-001,example-medium-001,example-small-001,large/medium/smallas aliases. Vendor names may only appear in: (1) DESIGN_SPEC.md provider list, (2).claude/skill/agent files, (3) third-party import paths/module names (e.g.litellm.types.llms.openai). Tests must usetest-provider,test-small-001, etc.
Files:
src/ai_company/observability/events/git.pytests/unit/observability/test_events.pytests/unit/tools/git/conftest.pytests/unit/tools/git/test_git_tools.pysrc/ai_company/tools/git_tools.pysrc/ai_company/tools/__init__.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Mark unit tests with@pytest.mark.unit, integration tests with@pytest.mark.integration, e2e tests with@pytest.mark.e2e, and slow tests with@pytest.mark.slow
Useasyncio_mode = 'auto'for pytest async tests—no manual@pytest.mark.asyncioneeded
Set a 30-second timeout per test
Files:
tests/unit/observability/test_events.pytests/unit/tools/git/conftest.pytests/unit/tools/git/test_git_tools.py
🧠 Learnings (1)
📚 Learning: 2026-03-06T21:51:55.175Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T21:51:55.175Z
Learning: Applies to src/**/*.py : Use event name constants from domain-specific modules under `ai_company.observability.events` (e.g. `PROVIDER_CALL_START` from `events.provider`, `BUDGET_RECORD_ADDED` from `events.budget`). Import directly: `from ai_company.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/ai_company/observability/events/git.pytests/unit/observability/test_events.py
🧬 Code graph analysis (3)
tests/unit/tools/git/conftest.py (1)
src/ai_company/tools/git_tools.py (2)
GitCloneTool(733-819)_run_git(163-253)
tests/unit/tools/git/test_git_tools.py (2)
src/ai_company/core/enums.py (1)
ToolCategory(213-227)src/ai_company/tools/base.py (3)
parameters_schema(120-128)description(115-117)to_definition(130-140)
src/ai_company/tools/git_tools.py (3)
src/ai_company/core/enums.py (1)
ToolCategory(213-227)src/ai_company/observability/_logger.py (1)
get_logger(8-28)src/ai_company/tools/base.py (5)
BaseTool(56-161)ToolExecutionResult(24-53)description(115-117)parameters_schema(120-128)category(110-112)
🪛 GitHub Actions: CI
tests/unit/tools/git/test_git_tools.py
[error] 382-382: Git commit failed. Author identity unknown: please configure user.name and user.email. Command: 'git commit -m add new file'.
[error] 389-389: Git commit failed. Author identity unknown: please configure user.name and user.email. Command: 'git commit -m commit all'.
🔇 Additional comments (8)
tests/unit/observability/test_events.py (1)
73-87: LGTM!The expected domain modules set correctly includes "git" to match the newly added
src/ai_company/observability/events/git.pymodule. The test will now validate that the git event constants module is properly discovered.CLAUDE.md (1)
57-57: LGTM!Documentation accurately updated to reflect the new built-in git tools in the package structure description.
src/ai_company/tools/__init__.py (1)
12-32: LGTM!The six git tool classes are properly imported and exported, maintaining alphabetical ordering in
__all__. This aligns with the existing pattern for tool exports.DESIGN_SPEC.md (1)
1652-1653: LGTM!The M3 implementation note comprehensively documents the six workspace-scoped git tools and their security hardening measures including:
- Workspace boundary enforcement via
resolve()+relative_to()- Non-shell subprocess execution
- Environment flags (
GIT_TERMINAL_PROMPT=0,GIT_CONFIG_NOSYSTEM=1,GIT_PROTOCOL_FROM_USER=0)- Flag injection prevention
- URL scheme validation for clone
This provides excellent security documentation for the implementation.
src/ai_company/observability/events/git.py (1)
1-9: LGTM!Event constants follow the established
domain.subject.qualifierpattern and cover the key git operation lifecycle events:
- Command start/success/failed/timeout
- Workspace boundary violations for security auditing
This aligns with the event naming conventions used in other domain modules. Based on learnings: "Use event name constants from domain-specific modules under
ai_company.observability.events".tests/unit/tools/git/test_git_tools.py (1)
1-29: LGTM! Comprehensive test coverage.The test module provides excellent coverage including:
- Workspace boundary enforcement with symlink escape tests
- Tool properties validation across all 6 tools
- Individual tool behavior tests
- Security-focused flag injection prevention tests
- Clone URL validation tests
The test organization with dedicated test classes per concern is clean and maintainable.
tests/unit/tools/git/conftest.py (1)
1-119: Well-structured test fixtures.The fixtures provide good separation of concerns:
workspacefor bare directoriesgit_repo/empty_git_repo/git_repo_with_changesfor various repo states- Tool factory fixtures for each git tool
allow_local_clonefor controlled test scenariosThe
_GIT_ENVsetup for secure git execution in tests mirrors the production security settings.src/ai_company/tools/git_tools.py (1)
1-28: LGTM! Well-structured module with comprehensive security measures.The module implements:
- Workspace boundary enforcement via
_validate_pathusingresolve()+relative_to()- Non-shell subprocess execution with
asyncio.create_subprocess_exec- Structured logging with domain-specific event constants
- Flag injection prevention via
_check_ref- URL scheme validation for clone operations
- Proper timeout handling with process kill
The code follows project conventions for logging, type hints, and docstrings.
| def _is_allowed_clone_url(url: str) -> bool: | ||
| """Check if a clone URL uses an allowed remote scheme. | ||
|
|
||
| Allows standard remote schemes and SCP-like syntax. Rejects | ||
| ``file://``, ``ext::``, and bare local paths. | ||
|
|
||
| Args: | ||
| url: Repository URL string to validate. | ||
|
|
||
| Returns: | ||
| ``True`` if the URL scheme is allowed. | ||
| """ | ||
| if any(url.startswith(scheme) for scheme in _ALLOWED_CLONE_SCHEMES): | ||
| return True | ||
| # SCP-like: user@host:path (e.g. git@github.com:user/repo.git) | ||
| return "@" in url and ":" in url and "::" not in url and "://" not in url |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
URL validation logic is reasonable but could be more explicit.
The SCP-like URL validation at line 49 uses heuristics ("@" in url and ":" in url and "::" not in url and "://" not in url). This correctly identifies patterns like git@github.com:user/repo.git while rejecting ext:: protocols.
Consider adding a brief inline comment explaining the SCP pattern for future maintainers, as this is subtle security-relevant logic.
📝 Suggested documentation improvement
if any(url.startswith(scheme) for scheme in _ALLOWED_CLONE_SCHEMES):
return True
- # SCP-like: user@host:path (e.g. git@github.com:user/repo.git)
+ # SCP-like syntax: user@host:path (e.g. git@github.com:user/repo.git)
+ # Must have @ and : but NOT :: (ext protocol) or :// (explicit scheme)
return "@" in url and ":" in url and "::" not in url and "://" not in url🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/tools/git_tools.py` around lines 34 - 49, The SCP-like URL
check in _is_allowed_clone_url is subtle; add a concise inline comment above the
return that explains the SCP pattern being matched (e.g. "SCP-like syntax:
user@host:path, e.g. git@github.com:user/repo.git") and note why checks for "::"
and "://" are required to reject ext:: and normal URL schemes respectively; keep
the logic unchanged but document the intent so future maintainers understand the
security-relevant heuristic.
src/ai_company/tools/git_tools.py
Outdated
| env = { | ||
| **os.environ, | ||
| "GIT_TERMINAL_PROMPT": "0", | ||
| "GIT_CONFIG_NOSYSTEM": "1", | ||
| } |
There was a problem hiding this comment.
Missing GIT_PROTOCOL_FROM_USER=0 environment variable.
The DESIGN_SPEC.md (line 1652) documents that GIT_PROTOCOL_FROM_USER=0 is set to "restrict config/protocol attack surfaces", but it's not included in the environment dictionary here.
This setting prevents the use of ext:: and other potentially dangerous remote protocols that could be specified in git config.
🛡️ Proposed fix
env = {
**os.environ,
"GIT_TERMINAL_PROMPT": "0",
"GIT_CONFIG_NOSYSTEM": "1",
+ "GIT_PROTOCOL_FROM_USER": "0",
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| env = { | |
| **os.environ, | |
| "GIT_TERMINAL_PROMPT": "0", | |
| "GIT_CONFIG_NOSYSTEM": "1", | |
| } | |
| env = { | |
| **os.environ, | |
| "GIT_TERMINAL_PROMPT": "0", | |
| "GIT_CONFIG_NOSYSTEM": "1", | |
| "GIT_PROTOCOL_FROM_USER": "0", | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/tools/git_tools.py` around lines 182 - 186, The environment
dict that sets git-related env vars (the dict that currently spreads os.environ
and sets "GIT_TERMINAL_PROMPT" and "GIT_CONFIG_NOSYSTEM") is missing
"GIT_PROTOCOL_FROM_USER":"0"; update that env mapping to include
"GIT_PROTOCOL_FROM_USER": "0" alongside the existing keys to restrict dangerous
remote protocols (i.e., modify the env dict in the same scope where those keys
are set).
| if branch := arguments.get("branch"): | ||
| args.extend(["--branch", branch]) |
There was a problem hiding this comment.
Validate branch parameter to prevent flag injection.
The branch parameter is passed directly to git without flag injection validation. If an attacker provides branch="--upload-pack=evil", the command becomes git clone --branch --upload-pack=evil ..., which git would interpret as separate flags.
Apply the same _check_ref validation used elsewhere:
🔒 Proposed fix
if branch := arguments.get("branch"):
+ if err := self._check_ref(branch, param="branch"):
+ return err
args.extend(["--branch", branch])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if branch := arguments.get("branch"): | |
| args.extend(["--branch", branch]) | |
| if branch := arguments.get("branch"): | |
| if err := self._check_ref(branch, param="branch"): | |
| return err | |
| args.extend(["--branch", branch]) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/tools/git_tools.py` around lines 806 - 807, The branch value
is inserted into the git CLI args without validation; before extending args with
["--branch", branch] validate branch using the existing _check_ref helper (or
equivalent) to reject values that start with "-" or contain unsafe characters,
and only then append it; update the block that reads `if branch :=
arguments.get("branch"):` to call `_check_ref(branch)` (or raise/return an error
if invalid) prior to `args.extend(["--branch", branch])` so flag-injection like
"--upload-pack=evil" is prevented.
| @pytest.fixture | ||
| def git_repo(tmp_path: Path) -> Path: | ||
| """Initialized git repo with one commit.""" | ||
| _run_git(["init"], tmp_path) | ||
| readme = tmp_path / "README.md" | ||
| readme.write_text("# Test\n") | ||
| _run_git(["add", "."], tmp_path) | ||
| _run_git(["commit", "-m", "initial commit"], tmp_path) | ||
| return tmp_path |
There was a problem hiding this comment.
Configure git user identity in test repository to fix CI failures.
The git_repo fixture uses _run_git with _GIT_ENV for the initial commit (which works), but when tests invoke GitCommitTool.execute(), the tool's internal _run_git uses os.environ without author identity. This causes commits to fail in CI environments without global git config.
Configure the identity within the repository so it persists for tool invocations:
🐛 Proposed fix
`@pytest.fixture`
def git_repo(tmp_path: Path) -> Path:
"""Initialized git repo with one commit."""
_run_git(["init"], tmp_path)
+ _run_git(["config", "user.name", "Test"], tmp_path)
+ _run_git(["config", "user.email", "test@test.local"], tmp_path)
readme = tmp_path / "README.md"
readme.write_text("# Test\n")
_run_git(["add", "."], tmp_path)
_run_git(["commit", "-m", "initial commit"], tmp_path)
return tmp_path🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/tools/git/conftest.py` around lines 47 - 55, The git_repo fixture
needs to configure a local git user identity so commits succeed in CI: inside
the git_repo fixture (which calls _run_git and returns tmp_path) run _run_git to
set user.name and user.email in the repo (use the same _run_git helper and
_GIT_ENV pattern used elsewhere) so the repo has a persistent local identity and
subsequent calls from GitCommitTool.execute (which uses os.environ) can create
commits; update the git_repo setup to call _run_git(["config", "user.name",
"Test User"], tmp_path) and _run_git(["config", "user.email",
"test@example.com"], tmp_path) (or equivalent values) before making commits.
| async def test_commit_with_paths(self, git_repo_with_changes: Path) -> None: | ||
| tool = GitCommitTool(workspace=git_repo_with_changes) | ||
| result = await tool.execute( | ||
| arguments={ | ||
| "message": "add new file", | ||
| "paths": ["new_file.txt"], | ||
| }, | ||
| ) | ||
| assert not result.is_error | ||
|
|
||
| async def test_commit_all(self, git_repo_with_changes: Path) -> None: | ||
| tool = GitCommitTool(workspace=git_repo_with_changes) | ||
| result = await tool.execute( | ||
| arguments={"message": "commit all", "all": True}, | ||
| ) | ||
| assert not result.is_error |
There was a problem hiding this comment.
Pipeline failure: Git commits fail due to missing author identity.
The CI failures at lines 382 and 389 occur because GitCommitTool._run_git() in git_tools.py doesn't set GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, GIT_COMMITTER_NAME, or GIT_COMMITTER_EMAIL environment variables. While the test helper _run_git in conftest.py sets these via _GIT_ENV, the actual tool invocation uses git_tools.py's _run_git which only sets GIT_TERMINAL_PROMPT=0 and GIT_CONFIG_NOSYSTEM=1.
In CI environments without a globally configured git identity, commits fail. Options to fix:
- Recommended: Configure git identity in the test repo itself using the conftest helper:
`@pytest.fixture`
def git_repo(tmp_path: Path) -> Path:
"""Initialized git repo with one commit."""
_run_git(["init"], tmp_path)
+ _run_git(["config", "user.name", "Test"], tmp_path)
+ _run_git(["config", "user.email", "test@test.local"], tmp_path)
readme = tmp_path / "README.md"- Alternative: Have
git_tools.pyset default author/committer env vars for commits.
🧰 Tools
🪛 GitHub Actions: CI
[error] 382-382: Git commit failed. Author identity unknown: please configure user.name and user.email. Command: 'git commit -m add new file'.
[error] 389-389: Git commit failed. Author identity unknown: please configure user.name and user.email. Command: 'git commit -m commit all'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/tools/git/test_git_tools.py` around lines 374 - 389, Tests fail
because commits lack author identity; update the tests to configure a repo-local
git identity (rather than changing GitCommitTool._run_git). In the
git_repo_with_changes fixture or before calling GitCommitTool.execute in the
failing tests, run the same setup used in conftest (apply the _GIT_ENV helper or
run git config user.name and git config user.email inside the test repo) so the
repository has user.name and user.email set; this ensures GitCommitTool._run_git
(which currently only sets GIT_TERMINAL_PROMPT and GIT_CONFIG_NOSYSTEM) can
successfully create commits without modifying the tool itself.
Greptile SummaryThis PR adds six workspace-scoped built-in git tools (
Confidence Score: 3/5
Important Files Changed
Class Diagram%%{init: {'theme': 'neutral'}}%%
classDiagram
class BaseTool {
+name: str
+description: str
+category: ToolCategory
+parameters_schema: dict
+execute(arguments) ToolExecutionResult
+to_definition() ToolDefinition
}
class _BaseGitTool {
-_workspace: Path
+workspace: Path
+_validate_path(relative) Path
+_check_paths(paths) ToolExecutionResult|None
+_check_ref(value, param) ToolExecutionResult|None
+_run_git(args, cwd, deadline) ToolExecutionResult
}
class GitStatusTool {
+execute(arguments) ToolExecutionResult
}
class GitLogTool {
-_MAX_COUNT_LIMIT: int
+execute(arguments) ToolExecutionResult
}
class GitDiffTool {
+execute(arguments) ToolExecutionResult
}
class GitBranchTool {
-_ACTIONS_REQUIRING_NAME: frozenset
-_list_branches() ToolExecutionResult
-_create_branch(name, arguments) ToolExecutionResult
+execute(arguments) ToolExecutionResult
}
class GitCommitTool {
+execute(arguments) ToolExecutionResult
}
class GitCloneTool {
+execute(arguments) ToolExecutionResult
}
BaseTool <|-- _BaseGitTool
_BaseGitTool <|-- GitStatusTool
_BaseGitTool <|-- GitLogTool
_BaseGitTool <|-- GitDiffTool
_BaseGitTool <|-- GitBranchTool
_BaseGitTool <|-- GitCommitTool
_BaseGitTool <|-- GitCloneTool
note for _BaseGitTool "GIT_CONFIG_GLOBAL=/dev/null\nGIT_TERMINAL_PROMPT=0\nGIT_CONFIG_NOSYSTEM=1\nshell=False always"
Last reviewed commit: 9d5dcc5 |
There was a problem hiding this comment.
Pull request overview
This PR implements six built-in git tools (GitStatusTool, GitLogTool, GitDiffTool, GitBranchTool, GitCommitTool, GitCloneTool) that agents use to interact with git repositories, with a shared _BaseGitTool base class providing workspace boundary enforcement and security hardening. It closes issue #132.
Changes:
- Added
src/ai_company/tools/git_tools.pywith 6 workspace-scoped git tools sharing a_BaseGitToolbase class, including subprocess execution viaasyncio.create_subprocess_exec, path traversal prevention, flag injection checks, and clone URL scheme validation - Added comprehensive test suite (tests/unit/tools/git/) with fixtures, workspace validation, tool properties, operation tests, and security tests
- Updated documentation (DESIGN_SPEC.md, CLAUDE.md) and observability events (events/git.py) to reflect the new built-in tools
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/ai_company/tools/git_tools.py |
Core implementation: _BaseGitTool base class and 6 concrete git tool classes with security hardening |
src/ai_company/tools/__init__.py |
Re-exports the 6 git tool classes from the tools package |
src/ai_company/observability/events/git.py |
New event constants for git command lifecycle logging |
tests/unit/tools/git/__init__.py |
Package marker for git tool tests |
tests/unit/tools/git/conftest.py |
Test fixtures: git repo setup, tool factories, allow_local_clone monkeypatch |
tests/unit/tools/git/test_git_tools.py |
~55+ tests covering workspace validation, tool properties, operations, flag injection, and URL validation |
tests/unit/observability/test_events.py |
Adds "git" to expected domain modules list |
DESIGN_SPEC.md |
M3 implementation note for git tools and updated file tree |
CLAUDE.md |
Updated tools/ directory description |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if any(url.startswith(scheme) for scheme in _ALLOWED_CLONE_SCHEMES): | ||
| return True | ||
| # SCP-like: user@host:path (e.g. git@github.com:user/repo.git) | ||
| return "@" in url and ":" in url and "::" not in url and "://" not in url |
There was a problem hiding this comment.
The SCP-like URL validation can be bypassed with a URL like -cfoo=bar@host:path, which passes all checks (@ present, : present, no ::, no ://) but would be interpreted by git as -c foo=bar@host:path — a config injection attack. Git's option parser supports the short form -cKEY=VALUE without a space separator. The SCP-like check should additionally reject URLs starting with -, e.g. by adding not url.startswith("-") to the condition.
| return "@" in url and ":" in url and "::" not in url and "://" not in url | |
| return ( | |
| not url.startswith("-") | |
| and "@" in url | |
| and ":" in url | |
| and "::" not in url | |
| and "://" not in url | |
| ) |
DESIGN_SPEC.md
Outdated
|
|
||
| The `ToolPermissionChecker` resolves permissions using a priority-based system: denied list (highest) → allowed list → access-level categories → deny (default). `AgentEngine._make_tool_invoker()` creates a permission-aware invoker from the agent's `ToolPermissions` at the start of each `run()` call. Note: M3 implements category-level gating only; the granular sub-constraints described in §11.2 (workspace scope, network mode) are planned for when sandboxing is implemented. | ||
|
|
||
| > **M3 implementation note — Built-in git tools:** Six workspace-scoped git tools are implemented in `tools/git_tools.py`: `GitStatusTool`, `GitLogTool`, `GitDiffTool`, `GitBranchTool`, `GitCommitTool`, and `GitCloneTool`. All share a `_BaseGitTool` base class that enforces workspace boundary security (path traversal prevention via `resolve()` + `relative_to()`) and provides a common `_run_git()` helper using `asyncio.create_subprocess_exec` (never `shell=True`). Security hardening includes: `GIT_TERMINAL_PROMPT=0` to prevent credential prompts, `GIT_CONFIG_NOSYSTEM=1` and `GIT_PROTOCOL_FROM_USER=0` to restrict config/protocol attack surfaces, rejection of flag-like ref/branch values (starting with `-`), and URL scheme validation on clone (only `https://`, `http://`, `ssh://`, `git://`, and SCP-like syntax). All tools return `ToolExecutionResult` for errors rather than raising exceptions. |
There was a problem hiding this comment.
The DESIGN_SPEC claims GIT_PROTOCOL_FROM_USER=0 is set to restrict the protocol attack surface, but this environment variable is not actually set in the _run_git() env dict in git_tools.py (lines 182-186). Either the code or the documentation should be updated to be consistent.
|
|
||
|
|
There was a problem hiding this comment.
There's no flag injection test for the GitCloneTool's branch parameter. The TestFlagInjectionPrevention class covers GitLogTool.ref, GitDiffTool.ref1/ref2, GitBranchTool.name/start_point, but omits GitCloneTool.branch. This corresponds to the missing _check_ref() validation in GitCloneTool.execute() (line 806 in git_tools.py).
| async def test_clone_branch_flag_blocked( | |
| self, | |
| clone_tool: GitCloneTool, | |
| ) -> None: | |
| result = await clone_tool.execute( | |
| arguments={ | |
| "url": "https://example.com/repo.git", | |
| "branch": "--upload-pack=evil", | |
| }, | |
| ) | |
| assert result.is_error |
| async def test_relative_path_blocked(self, clone_tool: GitCloneTool) -> None: | ||
| result = await clone_tool.execute( | ||
| arguments={"url": "../outside-repo"}, | ||
| ) | ||
| assert result.is_error |
There was a problem hiding this comment.
Missing test for URLs that start with - but match the SCP-like pattern (e.g., -cfoo=bar@host:path). This URL passes the current _is_allowed_clone_url validation because it contains @ and :, no ::, and no ://, but would be interpreted by git as a -c config flag injection. Add a test asserting that such URLs are rejected.
src/ai_company/tools/git_tools.py
Outdated
| env = { | ||
| **os.environ, | ||
| "GIT_TERMINAL_PROMPT": "0", | ||
| "GIT_CONFIG_NOSYSTEM": "1", | ||
| } |
There was a problem hiding this comment.
The DESIGN_SPEC documents GIT_PROTOCOL_FROM_USER=0 as a security hardening measure, but this environment variable is not actually set in _run_git(). Either add "GIT_PROTOCOL_FROM_USER": "0" to the env dict here, or remove the mention from the DESIGN_SPEC. The GIT_PROTOCOL_FROM_USER=0 variable prevents git from using user-specified protocol helpers, which is relevant for mitigating ext:: protocol attacks — worth including for defense in depth.
|
|
||
| args = ["clone"] | ||
|
|
||
| if branch := arguments.get("branch"): |
There was a problem hiding this comment.
The branch parameter is passed to git clone --branch without _check_ref() validation, unlike all other ref/branch parameters in the codebase (ref in GitLogTool, ref1/ref2 in GitDiffTool, name/start_point in GitBranchTool). While --branch as a separate argument provides some protection, for consistency with the defense-in-depth approach used elsewhere, branch should be validated with _check_ref(branch, param="branch") before being passed to _run_git().
| if branch := arguments.get("branch"): | |
| if branch := arguments.get("branch"): | |
| if err := self._check_ref(branch, param="branch"): | |
| return err |
DESIGN_SPEC.md
Outdated
|
|
||
| The `ToolPermissionChecker` resolves permissions using a priority-based system: denied list (highest) → allowed list → access-level categories → deny (default). `AgentEngine._make_tool_invoker()` creates a permission-aware invoker from the agent's `ToolPermissions` at the start of each `run()` call. Note: M3 implements category-level gating only; the granular sub-constraints described in §11.2 (workspace scope, network mode) are planned for when sandboxing is implemented. | ||
|
|
||
| > **M3 implementation note — Built-in git tools:** Six workspace-scoped git tools are implemented in `tools/git_tools.py`: `GitStatusTool`, `GitLogTool`, `GitDiffTool`, `GitBranchTool`, `GitCommitTool`, and `GitCloneTool`. All share a `_BaseGitTool` base class that enforces workspace boundary security (path traversal prevention via `resolve()` + `relative_to()`) and provides a common `_run_git()` helper using `asyncio.create_subprocess_exec` (never `shell=True`). Security hardening includes: `GIT_TERMINAL_PROMPT=0` to prevent credential prompts, `GIT_CONFIG_NOSYSTEM=1` and `GIT_PROTOCOL_FROM_USER=0` to restrict config/protocol attack surfaces, rejection of flag-like ref/branch values (starting with `-`), and URL scheme validation on clone (only `https://`, `http://`, `ssh://`, `git://`, and SCP-like syntax). All tools return `ToolExecutionResult` for errors rather than raising exceptions. |
There was a problem hiding this comment.
The documented behavior for GitCloneTool (allowing clone URLs with any host over https://, http://, ssh://, git://, and SCP-like syntax) enables server-side request forgery (SSRF) when the URL is controlled by the tool caller. Even with scheme validation and file:///ext:: blocked, an attacker can pass URLs such as http://127.0.0.1:8080/... or http://169.254.169.254/... to make the agent issue HTTP/SSH requests to internal services from its network context. The clone tool should restrict remote hosts to a configured allowlist (or explicitly block loopback, link-local, and private network ranges) to prevent abuse for internal network probing or metadata endpoint access.
| > **M3 implementation note — Built-in git tools:** Six workspace-scoped git tools are implemented in `tools/git_tools.py`: `GitStatusTool`, `GitLogTool`, `GitDiffTool`, `GitBranchTool`, `GitCommitTool`, and `GitCloneTool`. All share a `_BaseGitTool` base class that enforces workspace boundary security (path traversal prevention via `resolve()` + `relative_to()`) and provides a common `_run_git()` helper using `asyncio.create_subprocess_exec` (never `shell=True`). Security hardening includes: `GIT_TERMINAL_PROMPT=0` to prevent credential prompts, `GIT_CONFIG_NOSYSTEM=1` and `GIT_PROTOCOL_FROM_USER=0` to restrict config/protocol attack surfaces, rejection of flag-like ref/branch values (starting with `-`), and URL scheme validation on clone (only `https://`, `http://`, `ssh://`, `git://`, and SCP-like syntax). All tools return `ToolExecutionResult` for errors rather than raising exceptions. | |
| > **M3 implementation note — Built-in git tools:** Six workspace-scoped git tools are implemented in `tools/git_tools.py`: `GitStatusTool`, `GitLogTool`, `GitDiffTool`, `GitBranchTool`, `GitCommitTool`, and `GitCloneTool`. All share a `_BaseGitTool` base class that enforces workspace boundary security (path traversal prevention via `resolve()` + `relative_to()`) and provides a common `_run_git()` helper using `asyncio.create_subprocess_exec` (never `shell=True`). Security hardening includes: `GIT_TERMINAL_PROMPT=0` to prevent credential prompts, `GIT_CONFIG_NOSYSTEM=1` and `GIT_PROTOCOL_FROM_USER=0` to restrict config/protocol attack surfaces, rejection of flag-like ref/branch values (starting with `-`), URL scheme validation on clone (only `https://`, `http://`, `ssh://`, `git://`, and SCP-like syntax), and host/IP validation for clone targets (enforcing a configurable allowlist and/or explicitly blocking loopback, link-local, and private network ranges such as `127.0.0.0/8`, `::1`, `10.0.0.0/8`, `172.16.0.0/12`, `192.168.0.0/16`, and `169.254.169.254`) to prevent SSRF and internal network probing. All tools return `ToolExecutionResult` for errors rather than raising exceptions. |
- Extract _BaseGitTool to _git_base.py (keep files under 800 lines) - Add GIT_CONFIG_GLOBAL=/dev/null and GIT_PROTOCOL_FROM_USER=0 to env - Add _check_ref on clone branch param, -- separator before URL - Reject URLs starting with - in _is_allowed_clone_url - Add workspace absolute path validation, OSError handling in _validate_path - Add security logging for ref injection and clone URL rejection - Add post-kill communicate() timeout (5s) in _run_git - Log both stderr and stdout on failure - Add git event constants: GIT_CLONE_URL_REJECTED, GIT_REF_INJECTION_BLOCKED - Add tests: detached HEAD, merge conflict, timeout, OSError, flag injection - Update DESIGN_SPEC.md project structure and M3 implementation notes - Note SSRF host/IP allowlist as future consideration in DESIGN_SPEC Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| if action == "switch": | ||
| return await self._run_git(["switch", branch_name]) | ||
|
|
||
| # action == "delete" (per schema enum constraint) | ||
| flag = "-D" if arguments.get("force") else "-d" | ||
| return await self._run_git(["branch", flag, branch_name]) |
There was a problem hiding this comment.
Unknown action silently falls through to delete
The final if/elif chain has no explicit else guard for the delete branch — it relies entirely on the JSON Schema enum constraint to reject unknown actions. If schema validation is bypassed (e.g. a test monkeypatch, a future refactor that loosens validation, or direct instantiation in code), any unrecognised action string would silently execute git branch -d/-D <name>, potentially deleting a branch.
Adding a defensive guard makes the invariant explicit and self-documenting:
| if action == "switch": | |
| return await self._run_git(["switch", branch_name]) | |
| # action == "delete" (per schema enum constraint) | |
| flag = "-D" if arguments.get("force") else "-d" | |
| return await self._run_git(["branch", flag, branch_name]) | |
| if action == "switch": | |
| return await self._run_git(["switch", branch_name]) | |
| if action != "delete": # defensive; schema enum should prevent this | |
| return ToolExecutionResult( | |
| content=f"Unknown branch action: '{action}'", | |
| is_error=True, | |
| ) | |
| flag = "-D" if arguments.get("force") else "-d" | |
| return await self._run_git(["branch", flag, branch_name]) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ai_company/tools/git_tools.py
Line: 449-454
Comment:
**Unknown `action` silently falls through to `delete`**
The final `if/elif` chain has no explicit `else` guard for the `delete` branch — it relies entirely on the JSON Schema `enum` constraint to reject unknown actions. If schema validation is bypassed (e.g. a test monkeypatch, a future refactor that loosens validation, or direct instantiation in code), any unrecognised action string would silently execute `git branch -d/-D <name>`, potentially deleting a branch.
Adding a defensive guard makes the invariant explicit and self-documenting:
```suggestion
if action == "switch":
return await self._run_git(["switch", branch_name])
if action != "delete": # defensive; schema enum should prevent this
return ToolExecutionResult(
content=f"Unknown branch action: '{action}'",
is_error=True,
)
flag = "-D" if arguments.get("force") else "-d"
return await self._run_git(["branch", flag, branch_name])
```
How can I resolve this? If you propose a fix, please make it concise.| if ref1 := arguments.get("ref1"): | ||
| if err := self._check_ref(ref1, param="ref1"): | ||
| return err | ||
| args.append(ref1) | ||
| if ref2 := arguments.get("ref2"): | ||
| if err := self._check_ref(ref2, param="ref2"): | ||
| return err | ||
| args.append(ref2) |
There was a problem hiding this comment.
ref2 is accepted without ref1, producing a misleading diff
When only ref2 is supplied (no ref1), the args list becomes ["diff", ..., ref2] — which git interprets as git diff <ref2> (i.e. diff that commit against the working tree), not the two-ref comparison the parameter name implies. An LLM instructed to "compare branch A with branch B" would get silently wrong output if it only sets ref2.
Consider returning an error when ref2 is provided without ref1:
| if ref1 := arguments.get("ref1"): | |
| if err := self._check_ref(ref1, param="ref1"): | |
| return err | |
| args.append(ref1) | |
| if ref2 := arguments.get("ref2"): | |
| if err := self._check_ref(ref2, param="ref2"): | |
| return err | |
| args.append(ref2) | |
| if ref1 := arguments.get("ref1"): | |
| if err := self._check_ref(ref1, param="ref1"): | |
| return err | |
| args.append(ref1) | |
| if ref2 := arguments.get("ref2"): | |
| if not arguments.get("ref1"): | |
| return ToolExecutionResult( | |
| content="'ref2' requires 'ref1' to be set for a ref-to-ref diff", | |
| is_error=True, | |
| ) | |
| if err := self._check_ref(ref2, param="ref2"): | |
| return err | |
| args.append(ref2) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ai_company/tools/git_tools.py
Line: 310-317
Comment:
**`ref2` is accepted without `ref1`, producing a misleading diff**
When only `ref2` is supplied (no `ref1`), the args list becomes `["diff", ..., ref2]` — which git interprets as `git diff <ref2>` (i.e. diff that commit against the working tree), not the two-ref comparison the parameter name implies. An LLM instructed to "compare branch A with branch B" would get silently wrong output if it only sets `ref2`.
Consider returning an error when `ref2` is provided without `ref1`:
```suggestion
if ref1 := arguments.get("ref1"):
if err := self._check_ref(ref1, param="ref1"):
return err
args.append(ref1)
if ref2 := arguments.get("ref2"):
if not arguments.get("ref1"):
return ToolExecutionResult(
content="'ref2' requires 'ref1' to be set for a ref-to-ref diff",
is_error=True,
)
if err := self._check_ref(ref2, param="ref2"):
return err
args.append(ref2)
```
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| logger.debug( | ||
| GIT_COMMAND_START, | ||
| command=["git", *args], | ||
| cwd=str(work_dir), | ||
| ) |
There was a problem hiding this comment.
Clone URL credentials logged in plaintext
The full args list is logged at GIT_COMMAND_START (debug) and again inside GIT_COMMAND_FAILED (warning) on every invocation. For GitCloneTool, any clone URL that embeds credentials (e.g. using the user:password@host or token@host format that many CI systems use) is written verbatim into structured log output — including warning-level logs that are commonly shipped to log aggregation backends.
The URL should be sanitised before being handed to the logger. Only the subprocess call should receive the original args:
import re
def _redact_args(args: list[str]) -> list[str]:
return [re.sub(r"(https?://)([^@/\s]+)@", r"\1<redacted>@", a) for a in args]Then log with command=["git", *_redact_args(args)] while still passing the unmodified args to create_subprocess_exec.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ai_company/tools/_git_base.py
Line: 192-197
Comment:
**Clone URL credentials logged in plaintext**
The full `args` list is logged at `GIT_COMMAND_START` (debug) and again inside `GIT_COMMAND_FAILED` (warning) on every invocation. For `GitCloneTool`, any clone URL that embeds credentials (e.g. using the `user:password@host` or `token@host` format that many CI systems use) is written verbatim into structured log output — including warning-level logs that are commonly shipped to log aggregation backends.
The URL should be sanitised before being handed to the logger. Only the subprocess call should receive the original args:
```python
import re
def _redact_args(args: list[str]) -> list[str]:
return [re.sub(r"(https?://)([^@/\s]+)@", r"\1<redacted>@", a) for a in args]
```
Then log with `command=["git", *_redact_args(args)]` while still passing the unmodified `args` to `create_subprocess_exec`.
How can I resolve this? If you propose a fix, please make it concise.🤖 I have created a release *beep* *boop* --- ## [0.1.1](ai-company-v0.1.0...ai-company-v0.1.1) (2026-03-10) ### Features * add autonomy levels and approval timeout policies ([#42](#42), [#126](#126)) ([#197](#197)) ([eecc25a](eecc25a)) * add CFO cost optimization service with anomaly detection, reports, and approval decisions ([#186](#186)) ([a7fa00b](a7fa00b)) * add code quality toolchain (ruff, mypy, pre-commit, dependabot) ([#63](#63)) ([36681a8](36681a8)) * add configurable cost tiers and subscription/quota-aware tracking ([#67](#67)) ([#185](#185)) ([9baedfa](9baedfa)) * add container packaging, Docker Compose, and CI pipeline ([#269](#269)) ([435bdfe](435bdfe)), closes [#267](#267) * add coordination error taxonomy classification pipeline ([#146](#146)) ([#181](#181)) ([70c7480](70c7480)) * add cost-optimized, hierarchical, and auction assignment strategies ([#175](#175)) ([ce924fa](ce924fa)), closes [#173](#173) * add design specification, license, and project setup ([8669a09](8669a09)) * add env var substitution and config file auto-discovery ([#77](#77)) ([7f53832](7f53832)) * add FastestStrategy routing + vendor-agnostic cleanup ([#140](#140)) ([09619cb](09619cb)), closes [#139](#139) * add HR engine and performance tracking ([#45](#45), [#47](#47)) ([#193](#193)) ([2d091ea](2d091ea)) * add issue auto-search and resolution verification to PR review skill ([#119](#119)) ([deecc39](deecc39)) * add memory retrieval, ranking, and context injection pipeline ([#41](#41)) ([873b0aa](873b0aa)) * add pluggable MemoryBackend protocol with models, config, and events ([#180](#180)) ([46cfdd4](46cfdd4)) * add pluggable MemoryBackend protocol with models, config, and events ([#32](#32)) ([46cfdd4](46cfdd4)) * add pluggable PersistenceBackend protocol with SQLite implementation ([#36](#36)) ([f753779](f753779)) * add progressive trust and promotion/demotion subsystems ([#43](#43), [#49](#49)) ([3a87c08](3a87c08)) * add retry handler, rate limiter, and provider resilience ([#100](#100)) ([b890545](b890545)) * add SecOps security agent with rule engine, audit log, and ToolInvoker integration ([#40](#40)) ([83b7b6c](83b7b6c)) * add shared org memory and memory consolidation/archival ([#125](#125), [#48](#48)) ([4a0832b](4a0832b)) * design unified provider interface ([#86](#86)) ([3e23d64](3e23d64)) * expand template presets, rosters, and add inheritance ([#80](#80), [#81](#81), [#84](#84)) ([15a9134](15a9134)) * implement agent runtime state vs immutable config split ([#115](#115)) ([4cb1ca5](4cb1ca5)) * implement AgentEngine core orchestrator ([#11](#11)) ([#143](#143)) ([f2eb73a](f2eb73a)) * implement basic tool system (registry, invocation, results) ([#15](#15)) ([c51068b](c51068b)) * implement built-in file system tools ([#18](#18)) ([325ef98](325ef98)) * implement communication foundation — message bus, dispatcher, and messenger ([#157](#157)) ([8e71bfd](8e71bfd)) * implement company template system with 7 built-in presets ([#85](#85)) ([cbf1496](cbf1496)) * implement conflict resolution protocol ([#122](#122)) ([#166](#166)) ([e03f9f2](e03f9f2)) * implement core entity and role system models ([#69](#69)) ([acf9801](acf9801)) * implement crash recovery with fail-and-reassign strategy ([#149](#149)) ([e6e91ed](e6e91ed)) * implement engine extensions — Plan-and-Execute loop and call categorization ([#134](#134), [#135](#135)) ([#159](#159)) ([9b2699f](9b2699f)) * implement enterprise logging system with structlog ([#73](#73)) ([2f787e5](2f787e5)) * implement graceful shutdown with cooperative timeout strategy ([#130](#130)) ([6592515](6592515)) * implement hierarchical delegation and loop prevention ([#12](#12), [#17](#17)) ([6be60b6](6be60b6)) * implement LiteLLM driver and provider registry ([#88](#88)) ([ae3f18b](ae3f18b)), closes [#4](#4) * implement LLM decomposition strategy and workspace isolation ([#174](#174)) ([aa0eefe](aa0eefe)) * implement meeting protocol system ([#123](#123)) ([ee7caca](ee7caca)) * implement message and communication domain models ([#74](#74)) ([560a5d2](560a5d2)) * implement model routing engine ([#99](#99)) ([d3c250b](d3c250b)) * implement parallel agent execution ([#22](#22)) ([#161](#161)) ([65940b3](65940b3)) * implement per-call cost tracking service ([#7](#7)) ([#102](#102)) ([c4f1f1c](c4f1f1c)) * implement personality injection and system prompt construction ([#105](#105)) ([934dd85](934dd85)) * implement single-task execution lifecycle ([#21](#21)) ([#144](#144)) ([c7e64e4](c7e64e4)) * implement subprocess sandbox for tool execution isolation ([#131](#131)) ([#153](#153)) ([3c8394e](3c8394e)) * implement task assignment subsystem with pluggable strategies ([#172](#172)) ([c7f1b26](c7f1b26)), closes [#26](#26) [#30](#30) * implement task decomposition and routing engine ([#14](#14)) ([9c7fb52](9c7fb52)) * implement Task, Project, Artifact, Budget, and Cost domain models ([#71](#71)) ([81eabf1](81eabf1)) * implement tool permission checking ([#16](#16)) ([833c190](833c190)) * implement YAML config loader with Pydantic validation ([#59](#59)) ([ff3a2ba](ff3a2ba)) * implement YAML config loader with Pydantic validation ([#75](#75)) ([ff3a2ba](ff3a2ba)) * initialize project with uv, hatchling, and src layout ([39005f9](39005f9)) * initialize project with uv, hatchling, and src layout ([#62](#62)) ([39005f9](39005f9)) * Litestar REST API, WebSocket feed, and approval queue (M6) ([#189](#189)) ([29fcd08](29fcd08)) * make TokenUsage.total_tokens a computed field ([#118](#118)) ([c0bab18](c0bab18)), closes [#109](#109) * parallel tool execution in ToolInvoker.invoke_all ([#137](#137)) ([58517ee](58517ee)) * testing framework, CI pipeline, and M0 gap fixes ([#64](#64)) ([f581749](f581749)) * wire all modules into observability system ([#97](#97)) ([f7a0617](f7a0617)) ### Bug Fixes * address Greptile post-merge review findings from PRs [#170](https://github.com/Aureliolo/ai-company/issues/170)-[#175](https://github.com/Aureliolo/ai-company/issues/175) ([#176](#176)) ([c5ca929](c5ca929)) * address post-merge review feedback from PRs [#164](https://github.com/Aureliolo/ai-company/issues/164)-[#167](https://github.com/Aureliolo/ai-company/issues/167) ([#170](#170)) ([3bf897a](3bf897a)), closes [#169](#169) * enforce strict mypy on test files ([#89](#89)) ([aeeff8c](aeeff8c)) * harden Docker sandbox, MCP bridge, and code runner ([#50](#50), [#53](#53)) ([d5e1b6e](d5e1b6e)) * harden git tools security + code quality improvements ([#150](#150)) ([000a325](000a325)) * harden subprocess cleanup, env filtering, and shutdown resilience ([#155](#155)) ([d1fe1fb](d1fe1fb)) * incorporate post-merge feedback + pre-PR review fixes ([#164](#164)) ([c02832a](c02832a)) * pre-PR review fixes for post-merge findings ([#183](#183)) ([26b3108](26b3108)) * strengthen immutability for BaseTool schema and ToolInvoker boundaries ([#117](#117)) ([7e5e861](7e5e861)) ### Performance * harden non-inferable principle implementation ([#195](#195)) ([02b5f4e](02b5f4e)), closes [#188](#188) ### Refactoring * adopt NotBlankStr across all models ([#108](#108)) ([#120](#120)) ([ef89b90](ef89b90)) * extract _SpendingTotals base class from spending summary models ([#111](#111)) ([2f39c1b](2f39c1b)) * harden BudgetEnforcer with error handling, validation extraction, and review fixes ([#182](#182)) ([c107bf9](c107bf9)) * harden personality profiles, department validation, and template rendering ([#158](#158)) ([10b2299](10b2299)) * pre-PR review improvements for ExecutionLoop + ReAct loop ([#124](#124)) ([8dfb3c0](8dfb3c0)) * split events.py into per-domain event modules ([#136](#136)) ([e9cba89](e9cba89)) ### Documentation * add ADR-001 memory layer evaluation and selection ([#178](#178)) ([db3026f](db3026f)), closes [#39](#39) * add agent scaling research findings to DESIGN_SPEC ([#145](#145)) ([57e487b](57e487b)) * add CLAUDE.md, contributing guide, and dev documentation ([#65](#65)) ([55c1025](55c1025)), closes [#54](#54) * add crash recovery, sandboxing, analytics, and testing decisions ([#127](#127)) ([5c11595](5c11595)) * address external review feedback with MVP scope and new protocols ([#128](#128)) ([3b30b9a](3b30b9a)) * expand design spec with pluggable strategy protocols ([#121](#121)) ([6832db6](6832db6)) * finalize 23 design decisions (ADR-002) ([#190](#190)) ([8c39742](8c39742)) * update project docs for M2.5 conventions and add docs-consistency review agent ([#114](#114)) ([99766ee](99766ee)) ### Tests * add e2e single agent integration tests ([#24](#24)) ([#156](#156)) ([f566fb4](f566fb4)) * add provider adapter integration tests ([#90](#90)) ([40a61f4](40a61f4)) ### CI/CD * add Release Please for automated versioning and GitHub Releases ([#278](#278)) ([a488758](a488758)) * bump actions/checkout from 4 to 6 ([#95](#95)) ([1897247](1897247)) * bump actions/upload-artifact from 4 to 7 ([#94](#94)) ([27b1517](27b1517)) * harden CI/CD pipeline ([#92](#92)) ([ce4693c](ce4693c)) * split vulnerability scans into critical-fail and high-warn tiers ([#277](#277)) ([aba48af](aba48af)) ### Maintenance * add /worktree skill for parallel worktree management ([#171](#171)) ([951e337](951e337)) * add design spec context loading to research-link skill ([8ef9685](8ef9685)) * add post-merge-cleanup skill ([#70](#70)) ([f913705](f913705)) * add pre-pr-review skill and update CLAUDE.md ([#103](#103)) ([92e9023](92e9023)) * add research-link skill and rename skill files to SKILL.md ([#101](#101)) ([651c577](651c577)) * bump aiosqlite from 0.21.0 to 0.22.1 ([#191](#191)) ([3274a86](3274a86)) * bump pyyaml from 6.0.2 to 6.0.3 in the minor-and-patch group ([#96](#96)) ([0338d0c](0338d0c)) * bump ruff from 0.15.4 to 0.15.5 ([a49ee46](a49ee46)) * fix M0 audit items ([#66](#66)) ([c7724b5](c7724b5)) * pin setup-uv action to full SHA ([#281](#281)) ([4448002](4448002)) * post-audit cleanup — PEP 758, loggers, bug fixes, refactoring, tests, hookify rules ([#148](#148)) ([c57a6a9](c57a6a9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- ## [0.1.0](v0.0.0...v0.1.0) (2026-03-11) ### Features * add autonomy levels and approval timeout policies ([#42](#42), [#126](#126)) ([#197](#197)) ([eecc25a](eecc25a)) * add CFO cost optimization service with anomaly detection, reports, and approval decisions ([#186](#186)) ([a7fa00b](a7fa00b)) * add code quality toolchain (ruff, mypy, pre-commit, dependabot) ([#63](#63)) ([36681a8](36681a8)) * add configurable cost tiers and subscription/quota-aware tracking ([#67](#67)) ([#185](#185)) ([9baedfa](9baedfa)) * add container packaging, Docker Compose, and CI pipeline ([#269](#269)) ([435bdfe](435bdfe)), closes [#267](#267) * add coordination error taxonomy classification pipeline ([#146](#146)) ([#181](#181)) ([70c7480](70c7480)) * add cost-optimized, hierarchical, and auction assignment strategies ([#175](#175)) ([ce924fa](ce924fa)), closes [#173](#173) * add design specification, license, and project setup ([8669a09](8669a09)) * add env var substitution and config file auto-discovery ([#77](#77)) ([7f53832](7f53832)) * add FastestStrategy routing + vendor-agnostic cleanup ([#140](#140)) ([09619cb](09619cb)), closes [#139](#139) * add HR engine and performance tracking ([#45](#45), [#47](#47)) ([#193](#193)) ([2d091ea](2d091ea)) * add issue auto-search and resolution verification to PR review skill ([#119](#119)) ([deecc39](deecc39)) * add mandatory JWT + API key authentication ([#256](#256)) ([c279cfe](c279cfe)) * add memory retrieval, ranking, and context injection pipeline ([#41](#41)) ([873b0aa](873b0aa)) * add pluggable MemoryBackend protocol with models, config, and events ([#180](#180)) ([46cfdd4](46cfdd4)) * add pluggable MemoryBackend protocol with models, config, and events ([#32](#32)) ([46cfdd4](46cfdd4)) * add pluggable output scan response policies ([#263](#263)) ([b9907e8](b9907e8)) * add pluggable PersistenceBackend protocol with SQLite implementation ([#36](#36)) ([f753779](f753779)) * add progressive trust and promotion/demotion subsystems ([#43](#43), [#49](#49)) ([3a87c08](3a87c08)) * add retry handler, rate limiter, and provider resilience ([#100](#100)) ([b890545](b890545)) * add SecOps security agent with rule engine, audit log, and ToolInvoker integration ([#40](#40)) ([83b7b6c](83b7b6c)) * add shared org memory and memory consolidation/archival ([#125](#125), [#48](#48)) ([4a0832b](4a0832b)) * design unified provider interface ([#86](#86)) ([3e23d64](3e23d64)) * expand template presets, rosters, and add inheritance ([#80](#80), [#81](#81), [#84](#84)) ([15a9134](15a9134)) * implement agent runtime state vs immutable config split ([#115](#115)) ([4cb1ca5](4cb1ca5)) * implement AgentEngine core orchestrator ([#11](#11)) ([#143](#143)) ([f2eb73a](f2eb73a)) * implement AuditRepository for security audit log persistence ([#279](#279)) ([94bc29f](94bc29f)) * implement basic tool system (registry, invocation, results) ([#15](#15)) ([c51068b](c51068b)) * implement built-in file system tools ([#18](#18)) ([325ef98](325ef98)) * implement communication foundation — message bus, dispatcher, and messenger ([#157](#157)) ([8e71bfd](8e71bfd)) * implement company template system with 7 built-in presets ([#85](#85)) ([cbf1496](cbf1496)) * implement conflict resolution protocol ([#122](#122)) ([#166](#166)) ([e03f9f2](e03f9f2)) * implement core entity and role system models ([#69](#69)) ([acf9801](acf9801)) * implement crash recovery with fail-and-reassign strategy ([#149](#149)) ([e6e91ed](e6e91ed)) * implement engine extensions — Plan-and-Execute loop and call categorization ([#134](#134), [#135](#135)) ([#159](#159)) ([9b2699f](9b2699f)) * implement enterprise logging system with structlog ([#73](#73)) ([2f787e5](2f787e5)) * implement graceful shutdown with cooperative timeout strategy ([#130](#130)) ([6592515](6592515)) * implement hierarchical delegation and loop prevention ([#12](#12), [#17](#17)) ([6be60b6](6be60b6)) * implement LiteLLM driver and provider registry ([#88](#88)) ([ae3f18b](ae3f18b)), closes [#4](#4) * implement LLM decomposition strategy and workspace isolation ([#174](#174)) ([aa0eefe](aa0eefe)) * implement meeting protocol system ([#123](#123)) ([ee7caca](ee7caca)) * implement message and communication domain models ([#74](#74)) ([560a5d2](560a5d2)) * implement model routing engine ([#99](#99)) ([d3c250b](d3c250b)) * implement parallel agent execution ([#22](#22)) ([#161](#161)) ([65940b3](65940b3)) * implement per-call cost tracking service ([#7](#7)) ([#102](#102)) ([c4f1f1c](c4f1f1c)) * implement personality injection and system prompt construction ([#105](#105)) ([934dd85](934dd85)) * implement single-task execution lifecycle ([#21](#21)) ([#144](#144)) ([c7e64e4](c7e64e4)) * implement subprocess sandbox for tool execution isolation ([#131](#131)) ([#153](#153)) ([3c8394e](3c8394e)) * implement task assignment subsystem with pluggable strategies ([#172](#172)) ([c7f1b26](c7f1b26)), closes [#26](#26) [#30](#30) * implement task decomposition and routing engine ([#14](#14)) ([9c7fb52](9c7fb52)) * implement Task, Project, Artifact, Budget, and Cost domain models ([#71](#71)) ([81eabf1](81eabf1)) * implement tool permission checking ([#16](#16)) ([833c190](833c190)) * implement YAML config loader with Pydantic validation ([#59](#59)) ([ff3a2ba](ff3a2ba)) * implement YAML config loader with Pydantic validation ([#75](#75)) ([ff3a2ba](ff3a2ba)) * initialize project with uv, hatchling, and src layout ([39005f9](39005f9)) * initialize project with uv, hatchling, and src layout ([#62](#62)) ([39005f9](39005f9)) * Litestar REST API, WebSocket feed, and approval queue (M6) ([#189](#189)) ([29fcd08](29fcd08)) * make TokenUsage.total_tokens a computed field ([#118](#118)) ([c0bab18](c0bab18)), closes [#109](#109) * parallel tool execution in ToolInvoker.invoke_all ([#137](#137)) ([58517ee](58517ee)) * testing framework, CI pipeline, and M0 gap fixes ([#64](#64)) ([f581749](f581749)) * wire all modules into observability system ([#97](#97)) ([f7a0617](f7a0617)) ### Bug Fixes * address Greptile post-merge review findings from PRs [#170](https://github.com/Aureliolo/ai-company/issues/170)-[#175](https://github.com/Aureliolo/ai-company/issues/175) ([#176](#176)) ([c5ca929](c5ca929)) * address post-merge review feedback from PRs [#164](https://github.com/Aureliolo/ai-company/issues/164)-[#167](https://github.com/Aureliolo/ai-company/issues/167) ([#170](#170)) ([3bf897a](3bf897a)), closes [#169](#169) * enforce strict mypy on test files ([#89](#89)) ([aeeff8c](aeeff8c)) * harden Docker sandbox, MCP bridge, and code runner ([#50](#50), [#53](#53)) ([d5e1b6e](d5e1b6e)) * harden git tools security + code quality improvements ([#150](#150)) ([000a325](000a325)) * harden subprocess cleanup, env filtering, and shutdown resilience ([#155](#155)) ([d1fe1fb](d1fe1fb)) * incorporate post-merge feedback + pre-PR review fixes ([#164](#164)) ([c02832a](c02832a)) * pre-PR review fixes for post-merge findings ([#183](#183)) ([26b3108](26b3108)) * resolve circular imports, bump litellm, fix release tag format ([#286](#286)) ([a6659b5](a6659b5)) * strengthen immutability for BaseTool schema and ToolInvoker boundaries ([#117](#117)) ([7e5e861](7e5e861)) ### Performance * harden non-inferable principle implementation ([#195](#195)) ([02b5f4e](02b5f4e)), closes [#188](#188) ### Refactoring * adopt NotBlankStr across all models ([#108](#108)) ([#120](#120)) ([ef89b90](ef89b90)) * extract _SpendingTotals base class from spending summary models ([#111](#111)) ([2f39c1b](2f39c1b)) * harden BudgetEnforcer with error handling, validation extraction, and review fixes ([#182](#182)) ([c107bf9](c107bf9)) * harden personality profiles, department validation, and template rendering ([#158](#158)) ([10b2299](10b2299)) * pre-PR review improvements for ExecutionLoop + ReAct loop ([#124](#124)) ([8dfb3c0](8dfb3c0)) * split events.py into per-domain event modules ([#136](#136)) ([e9cba89](e9cba89)) ### Documentation * add ADR-001 memory layer evaluation and selection ([#178](#178)) ([db3026f](db3026f)), closes [#39](#39) * add agent scaling research findings to DESIGN_SPEC ([#145](#145)) ([57e487b](57e487b)) * add CLAUDE.md, contributing guide, and dev documentation ([#65](#65)) ([55c1025](55c1025)), closes [#54](#54) * add crash recovery, sandboxing, analytics, and testing decisions ([#127](#127)) ([5c11595](5c11595)) * address external review feedback with MVP scope and new protocols ([#128](#128)) ([3b30b9a](3b30b9a)) * expand design spec with pluggable strategy protocols ([#121](#121)) ([6832db6](6832db6)) * finalize 23 design decisions (ADR-002) ([#190](#190)) ([8c39742](8c39742)) * update project docs for M2.5 conventions and add docs-consistency review agent ([#114](#114)) ([99766ee](99766ee)) ### Tests * add e2e single agent integration tests ([#24](#24)) ([#156](#156)) ([f566fb4](f566fb4)) * add provider adapter integration tests ([#90](#90)) ([40a61f4](40a61f4)) ### CI/CD * add Release Please for automated versioning and GitHub Releases ([#278](#278)) ([a488758](a488758)) * bump actions/checkout from 4 to 6 ([#95](#95)) ([1897247](1897247)) * bump actions/upload-artifact from 4 to 7 ([#94](#94)) ([27b1517](27b1517)) * bump anchore/scan-action from 6.5.1 to 7.3.2 ([#271](#271)) ([80a1c15](80a1c15)) * bump docker/build-push-action from 6.19.2 to 7.0.0 ([#273](#273)) ([dd0219e](dd0219e)) * bump docker/login-action from 3.7.0 to 4.0.0 ([#272](#272)) ([33d6238](33d6238)) * bump docker/metadata-action from 5.10.0 to 6.0.0 ([#270](#270)) ([baee04e](baee04e)) * bump docker/setup-buildx-action from 3.12.0 to 4.0.0 ([#274](#274)) ([5fc06f7](5fc06f7)) * bump sigstore/cosign-installer from 3.9.1 to 4.1.0 ([#275](#275)) ([29dd16c](29dd16c)) * harden CI/CD pipeline ([#92](#92)) ([ce4693c](ce4693c)) * split vulnerability scans into critical-fail and high-warn tiers ([#277](#277)) ([aba48af](aba48af)) ### Maintenance * add /worktree skill for parallel worktree management ([#171](#171)) ([951e337](951e337)) * add design spec context loading to research-link skill ([8ef9685](8ef9685)) * add post-merge-cleanup skill ([#70](#70)) ([f913705](f913705)) * add pre-pr-review skill and update CLAUDE.md ([#103](#103)) ([92e9023](92e9023)) * add research-link skill and rename skill files to SKILL.md ([#101](#101)) ([651c577](651c577)) * bump aiosqlite from 0.21.0 to 0.22.1 ([#191](#191)) ([3274a86](3274a86)) * bump pyyaml from 6.0.2 to 6.0.3 in the minor-and-patch group ([#96](#96)) ([0338d0c](0338d0c)) * bump ruff from 0.15.4 to 0.15.5 ([a49ee46](a49ee46)) * fix M0 audit items ([#66](#66)) ([c7724b5](c7724b5)) * **main:** release ai-company 0.1.1 ([#282](#282)) ([2f4703d](2f4703d)) * pin setup-uv action to full SHA ([#281](#281)) ([4448002](4448002)) * post-audit cleanup — PEP 758, loggers, bug fixes, refactoring, tests, hookify rules ([#148](#148)) ([c57a6a9](c57a6a9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Signed-off-by: Aurelio <19254254+Aureliolo@users.noreply.github.com>
Summary
GitStatusTool,GitLogTool,GitDiffTool,GitBranchTool,GitCommitTool,GitCloneTool_BaseGitToolbase class with workspace boundary enforcement and common_run_git()subprocess helper_check_ref), clone URL scheme validation (_is_allowed_clone_url),GIT_CONFIG_NOSYSTEM=1,GIT_TERMINAL_PROMPT=0,shell=Falsesubprocess execution, path traversal preventionGitToolError(dead code — tools useToolExecutionResultfor error reporting)Security measures
_check_ref()rejects values starting with-_is_allowed_clone_url()allowlistshttps://,http://,ssh://,git://, SCP-likeasyncio.create_subprocess_exec(nevershell=True)_validate_path()withresolve()+relative_to()GIT_CONFIG_NOSYSTEM=1blocks system git configGIT_TERMINAL_PROMPT=0Test plan
Review coverage
9 agents ran in parallel: code-reviewer, python-reviewer, pr-test-analyzer, silent-failure-hunter, type-design-analyzer, logging-audit, resilience-audit, security-reviewer, docs-consistency. 16 findings triaged and implemented (3 Critical security, 5 Major code, 5 Major docs, 3 Medium tests).
Closes #132