Skip to content

feat: add git add and commit repo tools #18

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Chris-godz merged 2 commits into from
Mar 23, 2026
Merged

feat: add git add and commit repo tools #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f55ffc4
refactor: split tool approval policy into two risk dimensions
LeeLaugh-jason Mar 23, 2026
ecebc5e
feat: add hardened git add and commit repo tools
LeeLaugh-jason Mar 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions include/repo_tools.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,13 @@ nlohmann::json git_show(const std::string& workspace_abs,
size_t context_lines,
size_t output_limit = 0);

nlohmann::json git_add(const std::string& workspace_abs,
const std::vector<std::string>& pathspecs,
size_t output_limit = 0);

nlohmann::json git_commit(const std::string& workspace_abs,
const std::string& message,
size_t output_limit = 0);

void set_rg_binary_for_testing(const std::string& path);
void clear_rg_binary_for_testing();
2 changes: 2 additions & 0 deletions include/tool_registry.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ struct ToolDescriptor {
std::string name;
std::string description;
ToolCategory category = ToolCategory::ReadOnly;
bool mutates_repository_state = false;
bool can_execute_repo_controlled_code = false;
bool requires_approval = false;
nlohmann::json json_schema = nlohmann::json::object();
size_t max_output_bytes = 0;
Expand Down
71 changes: 71 additions & 0 deletions src/agent_tools.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,14 @@ bool optional_bool_arg(const ToolCall& cmd, const char* key, bool default_value
return value.get<bool>();
}

std::string require_non_empty_string_arg(const ToolCall& cmd, const char* key, const char* tool_name) {
const std::string value = require_string_arg(cmd, key, tool_name);
if (value.empty()) {
throw std::runtime_error("Argument '" + std::string(key) + "' for " + tool_name + " must not be empty.");
}
return value;
}
Copy link
Copy Markdown

@cursor cursor bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New helper function is defined but never called

Low Severity

require_non_empty_string_arg is added in this PR but never called anywhere in the codebase. The git_commit executor at line 544 uses require_string_arg instead, relying on the git_commit() function in repo_tools.cpp to perform its own empty-message check. This is dead code that adds maintenance burden without providing value.

Additional Locations (1)
Fix in Cursor Fix in Web


size_t parse_bounded_output_bytes_arg(const ToolCall& cmd, const char* key, size_t default_value) {
const size_t parsed = optional_size_arg(cmd, key, default_value);
if (parsed == 0 || parsed > kMaxBuildToolOutputBytes) {
Expand Down Expand Up @@ -228,6 +236,8 @@ ToolRegistry build_default_tool_registry() {
.name = "read_file_safe",
.description = "Reads the complete contents of a workspace file.",
.category = ToolCategory::ReadOnly,
.mutates_repository_state = false,
.can_execute_repo_controlled_code = false,
.requires_approval = false,
.json_schema = make_parameters_schema({
{"path", {
Expand All @@ -252,6 +262,8 @@ ToolRegistry build_default_tool_registry() {
.name = "write_file_safe",
.description = "Writes string content to a workspace file, overwriting existing content.",
.category = ToolCategory::Mutating,
.mutates_repository_state = true,
.can_execute_repo_controlled_code = false,
.requires_approval = true,
.json_schema = make_parameters_schema({
{"path", {
Expand Down Expand Up @@ -279,6 +291,8 @@ ToolRegistry build_default_tool_registry() {
.name = "bash_execute_safe",
.description = "Executes a bounded bash command inside the workspace.",
.category = ToolCategory::Execution,
.mutates_repository_state = true,
.can_execute_repo_controlled_code = true,
.requires_approval = true,
.json_schema = make_parameters_schema({
{"command", {
Expand Down Expand Up @@ -311,6 +325,8 @@ ToolRegistry build_default_tool_registry() {
.name = "build_project_safe",
.description = "Runs ./build.sh in debug or release mode with bounded retained output.",
.category = ToolCategory::Execution,
.mutates_repository_state = true,
.can_execute_repo_controlled_code = true,
.requires_approval = true,
.json_schema = make_parameters_schema({
{"build_mode", {
Expand Down Expand Up @@ -360,6 +376,8 @@ ToolRegistry build_default_tool_registry() {
.name = "test_project_safe",
.description = "Runs ./build.sh test with bounded retained output and a small ctest summary.",
.category = ToolCategory::Execution,
.mutates_repository_state = true,
.can_execute_repo_controlled_code = true,
.requires_approval = true,
.json_schema = make_parameters_schema({
{"timeout_ms", {
Expand Down Expand Up @@ -407,6 +425,8 @@ ToolRegistry build_default_tool_registry() {
.name = "list_files_bounded",
.description = "Lists workspace files with optional directory and extension filters.",
.category = ToolCategory::ReadOnly,
.mutates_repository_state = false,
.can_execute_repo_controlled_code = false,
.requires_approval = false,
.json_schema = make_parameters_schema({
{"directory", {
Expand Down Expand Up @@ -438,6 +458,8 @@ ToolRegistry build_default_tool_registry() {
.name = "rg_search",
.description = "Searches the workspace with ripgrep and returns structured matches.",
.category = ToolCategory::ReadOnly,
.mutates_repository_state = false,
.can_execute_repo_controlled_code = false,
.requires_approval = false,
.json_schema = make_parameters_schema({
{"query", {
Expand Down Expand Up @@ -473,6 +495,8 @@ ToolRegistry build_default_tool_registry() {
.name = "git_status",
.description = "Returns the current git working tree status for the workspace.",
.category = ToolCategory::ReadOnly,
.mutates_repository_state = false,
.can_execute_repo_controlled_code = false,
.requires_approval = false,
.json_schema = make_parameters_schema(nlohmann::json::object()),
.max_output_bytes = kMaxRepoOutputBytes,
Expand All @@ -481,13 +505,56 @@ ToolRegistry build_default_tool_registry() {
}
});

register_or_throw(&registry, ToolDescriptor{
.name = "git_add",
.description = "Stages explicitly listed git pathspecs into the index.",
.category = ToolCategory::Mutating,
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The git_add tool is categorized as ToolCategory::Mutating, while git_commit is ToolCategory::Execution. Both tools execute an external git command and modify the repository state. For consistency with git_commit, bash_execute_safe, and other tools that run external processes, git_add should also be categorized as ToolCategory::Execution. This ensures that its use is governed by the allow_execution_tools configuration, providing a consistent security and approval model for all tools that execute external commands.

Suggested change
.category = ToolCategory::Mutating,
.category = ToolCategory::Execution,

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Require execution approval for git_add

Registering git_add as ToolCategory::Mutating means src/tool_registry.cpp:53-59 will allow it whenever allow_mutating_tools is true, even if execution tools are blocked. That is an execution-policy bypass: git_add() shells out to git add, and the repo already exercises repo-defined clean filters in tests/test_repo_tools.cpp:658-680; the official gitattributes docs also say clean/process filters run on check-in (https://git-scm.com/docs/gitattributes.html). In an untrusted repo, staging a file can therefore run arbitrary commands despite allow_execution_tools=false.

Useful? React with 👍 / 👎.

.mutates_repository_state = true,
.can_execute_repo_controlled_code = true,
.requires_approval = true,
.json_schema = make_parameters_schema({
{"pathspecs", {
{"type", "array"},
{"items", {{"type", "string"}}},
{"description", "Explicit git pathspecs to stage. Must contain at least one entry."}
}}
}, {"pathspecs"}),
.max_output_bytes = kMaxRepoOutputBytes,
.execute = [](const ToolCall& cmd, const AgentConfig& config, size_t output_limit) {
const std::vector<std::string> pathspecs = optional_string_array_arg_strict(cmd, "pathspecs");
return git_add(config.workspace_abs, pathspecs, output_limit);
}
});

register_or_throw(&registry, ToolDescriptor{
.name = "git_commit",
.description = "Creates a git commit from the currently staged index changes.",
.category = ToolCategory::Mutating,
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Require execution approval for git_commit

Marking git_commit as mutating-only bypasses the separate execution gate for repo-controlled hooks and signing helpers. src/tool_registry.cpp:53-59 checks only allow_mutating_tools for this category, tests/test_schema_and_args_tolerance.cpp:474-505 even asserts git_commit succeeds with allow_execution_tools=false, and git commit is documented to run commit hooks (https://git-scm.com/docs/git-commit.html#_hooks); tests/test_repo_tools.cpp:851-863 also shows a repo-configured gpg.program is invoked. A malicious repository can therefore execute commands via pre-commit/commit-msg/signing while the policy is supposedly forbidding execution.

Useful? React with 👍 / 👎.

.mutates_repository_state = true,
.can_execute_repo_controlled_code = true,
.requires_approval = true,
Comment on lines +530 to +535
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Treat git_commit as an execution-capable tool

git_commit is only approval-gated as a mutating tool here, but src/repo_tools.cpp invokes a raw git commit, which executes repository hooks. I verified locally that a .git/hooks/pre-commit script runs through this path. In any repo that carries hooks (or sets core.hooksPath), a caller with allow_mutating_tools=true but allow_execution_tools=false can still run arbitrary shell code, which bypasses the current execution approval boundary.

Useful? React with 👍 / 👎.

.json_schema = make_parameters_schema({
{"message", {
{"type", "string"},
{"description", "Commit message for the staged index changes."}
}}
}, {"message"}),
.max_output_bytes = kMaxRepoOutputBytes,
.execute = [](const ToolCall& cmd, const AgentConfig& config, size_t output_limit) {
const std::string message = require_string_arg(cmd, "message", "git_commit");
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency and to fail earlier, you should use the require_non_empty_string_arg helper function that was introduced in this same pull request. This ensures that the validation for a non-empty commit message happens at the argument parsing layer, which is a better practice than relying on the downstream git_commit function to perform this check.

Suggested change
const std::string message = require_string_arg(cmd, "message", "git_commit");
const std::string message = require_non_empty_string_arg(cmd, "message", "git_commit");

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency and to perform argument validation at the earliest opportunity, consider using the require_non_empty_string_arg helper function you've added in this PR. While git_commit also checks for an empty message, using the specific helper here makes the argument requirements clearer at the tool registration level.

Suggested change
const std::string message = require_string_arg(cmd, "message", "git_commit");
const std::string message = require_non_empty_string_arg(cmd, "message", "git_commit");

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency and to fail earlier, please use the newly introduced require_non_empty_string_arg helper function here to validate the message argument. This ensures that an empty commit message is rejected at the argument parsing stage.

Suggested change
const std::string message = require_string_arg(cmd, "message", "git_commit");
const std::string message = require_non_empty_string_arg(cmd, "message", "git_commit");

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency and to centralize argument validation, consider using the require_non_empty_string_arg helper function here, which you've just introduced. The git_commit implementation currently performs its own check for an empty message, but using the helper would make the intent clearer at the argument parsing stage and avoid redundant checks.

Suggested change
const std::string message = require_string_arg(cmd, "message", "git_commit");
const std::string message = require_non_empty_string_arg(cmd, "message", "git_commit");

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency and to fail early, consider using the new require_non_empty_string_arg helper function here. This function was added in this PR and seems perfect for ensuring the commit message is not empty at the argument parsing stage, rather than inside the git_commit implementation.

Suggested change
const std::string message = require_string_arg(cmd, "message", "git_commit");
const std::string message = require_non_empty_string_arg(cmd, "message", "git_commit");

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency with the newly introduced require_non_empty_string_arg function and to enforce argument validation at the earliest point, consider using it here instead of require_string_arg. The git_commit implementation in repo_tools.cpp also performs this check, but centralizing this validation in the argument parsing layer improves maintainability.

Suggested change
const std::string message = require_string_arg(cmd, "message", "git_commit");
const std::string message = require_non_empty_string_arg(cmd, "message", "git_commit");

return git_commit(config.workspace_abs, message, output_limit);
}
});

register_or_throw(&registry, ToolDescriptor{
.name = "apply_patch",
.description = "Applies an exact-text replacement patch to a workspace file. "
"Supports single mode (old_text + new_text) and batch mode (patches array). "
"old_text must appear exactly once in the file. "
"An explicit empty new_text is valid and deletes the matched text.",
.category = ToolCategory::Mutating,
.mutates_repository_state = true,
.can_execute_repo_controlled_code = false,
.requires_approval = true,
.json_schema = nlohmann::json{
{"type", "object"},
Expand Down Expand Up @@ -678,6 +745,8 @@ ToolRegistry build_default_tool_registry() {
"By default shows unstaged changes; set cached=true for staged changes. "
"Optionally filter by pathspecs and control hunk context with context_lines (default 3).",
.category = ToolCategory::ReadOnly,
.mutates_repository_state = false,
.can_execute_repo_controlled_code = false,
.requires_approval = false,
.json_schema = make_parameters_schema({
{"cached", {
Expand Down Expand Up @@ -716,6 +785,8 @@ ToolRegistry build_default_tool_registry() {
"rev is required. Optionally disable patch or stat output, filter by pathspecs, "
"and control diff context with context_lines (default 3).",
.category = ToolCategory::ReadOnly,
.mutates_repository_state = false,
.can_execute_repo_controlled_code = false,
.requires_approval = false,
.json_schema = make_parameters_schema({
{"rev", {
Expand Down
Loading