Repository navigation
Let review profiles carry each reviewer's own agent config - #2711
Open
peyton-alt wants to merge 7 commits into
Open
peyton-alt wants to merge 7 commits into
peyton-alt wants to merge 7 commits into
Conversation
A reviewer in a review profile can now have its own agent config, set with `entire review --edit` or `--configure --set-config reviewer=<file|isolated|none>`. It replaces the reviewed checkout's hooks, MCP servers and extensions for that reviewer: - Claude Code: --setting-sources user and --strict-mcp-config drop the checkout's settings and MCP servers; the profile's settings (with Entire's tracking hooks added) and MCP servers are passed as 0600 files. The checkout's skills and commands load as the "project" plugin, copied from the committed tree. - Codex: the checkout and the main repo are marked untrusted for the run; profile MCP servers go through -c. Codex hooks and literal MCP env values are refused. - Pi: --no-extensions, plus Entire's extension and the profile's. Configs are honored only from developer-owned layers and saved there; a local file that can't be verified fails the review. Every command a config names must be absolute and outside the checkouts, or a bare tool name. Reviewing someone else's code still needs approval, since the branch's skills load; the warning and --show-config say which reviewers use their own config and list only what still loads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 26dfb3c. Configure here.
| return err | ||
| } | ||
| fmt.Fprintf(out, "Saved reviewer agent config to %s (agent config is never written to the shared settings file).\n", where) | ||
| } |
There was a problem hiding this comment.
Edit drops isolated reviewer config
High Severity
--edit writes the profile without config and only calls saveReviewAgentConfigs when the user changes that field. Keeping the current config therefore never rewrites the developer-owned layer. Saving to local strips isolation entirely; saving to project leaves the stale clone-local snapshot in place, so later skill and model edits never take effect.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 26dfb3c. Configure here.
| if strings.Contains(arg, "CLAUDE_PROJECT_DIR") || (filepath.IsAbs(arg) && under(arg, forbiddenRoots)) { | ||
| return fmt.Errorf("MCP server %q: argument %q points into a checkout", name, arg) | ||
| } | ||
| } |
There was a problem hiding this comment.
MCP args allow checkout paths
High Severity
validateMCPServer only rejects MCP arguments that contain CLAUDE_PROJECT_DIR or are absolute paths under a forbidden root. Relative arguments such as ./tools/server.js or ../bin/mcp pass, so an allowed binary can still execute code from the reviewed checkout.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 26dfb3c. Configure here.
| } | ||
| for _, root := range roots { | ||
| cfg.ExtraArgs = append(cfg.ExtraArgs, "-c", untrustedProjectOverride(root)) | ||
| } |
There was a problem hiding this comment.
Codex trust overrides replace each other
High Severity
Linked worktrees emit two separate -c projects={...} overrides, one for the worktree and one for the main repo. Each assignment replaces the projects table, so only the last root stays untrusted. The process then starts in the worktree, which can still load that checkout’s project config, hooks, and rules.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 26dfb3c. Configure here.
| return true | ||
| } | ||
| } | ||
| return false |
There was a problem hiding this comment.
Checkout path checks ignore Windows
Medium Severity
under compares paths with a case-sensitive prefix, and validateCommand only treats ./ and ../ as relative arguments. On Windows, C:\Repo versus c:\repo does not match, and .\hook.ps1 is accepted, so a profile command can still run from the reviewed checkout.
Reviewed by Cursor Bugbot for commit 26dfb3c. Configure here.
Validation looked only at the first word of a hook command, split on whitespace without quoting, so "/usr/bin/true; node_modules/.bin/x" or a quoted path with spaces inside the checkout got through, and MCP arguments were checked only when absolute. Commands are now split like a shell would (quotes, escapes, ; && | as separators) and every word is checked: paths must be absolute and outside the checkouts, and $VAR paths, command substitution and project launchers are refused. MCP commands are checked as one program path, and their arguments word by word. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M4E8H349JGBMPP75KSV3X25A
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Command-path validation can still execute checkout-controlled code, and clone-local profile copies can shadow later profile edits.
4 open findings
What changed in this PR
Adds developer-owned, per-reviewer agent configuration to review profiles while isolating review runs from checkout-provided hooks, MCP servers, and extensions.
Changes:
- Adds trusted profile storage, editing, validation, and trust-gate reporting.
- Applies isolated configurations for Claude Code, Codex, and Pi.
- Adds agent-specific and trust-inventory tests and documentation.
| File | Description |
|---|---|
docs/architecture/review-command.md |
Documents reviewer configuration and isolation. |
cmd/entire/cli/settings/settings.go |
Adds reviewer agent configuration schema. |
cmd/entire/cli/settings/agent_prompt_trust.go |
Enforces developer-owned configuration provenance. |
cmd/entire/cli/settings/agent_config_trust_test.go |
Tests configuration trust rules. |
cmd/entire/cli/review/types/template.go |
Adds pre-launch preparation and cleanup. |
cmd/entire/cli/review/types/reviewer.go |
Extends per-run configuration types. |
cmd/entire/cli/review/trust.go |
Reports isolated reviewers in trust output. |
cmd/entire/cli/review/trust_run.go |
Tracks isolation when inspecting profiles. |
cmd/entire/cli/review/trust_cmd_test.go |
Tests clone-local configuration saving. |
cmd/entire/cli/review/target.go |
Uses isolation-aware target inspection. |
cmd/entire/cli/review/profile.go |
Strips private config from project settings. |
cmd/entire/cli/review/picker.go |
Adds interactive agent-config selection. |
cmd/entire/cli/review/cmd.go |
Adds --set-config and runtime propagation. |
cmd/entire/cli/review/agent_config.go |
Validates agent configuration and commands. |
cmd/entire/cli/review/agent_config_test.go |
Tests validation and trust behavior. |
cmd/entire/cli/review/agent_config_save.go |
Loads and saves developer-owned configurations. |
cmd/entire/cli/review/agent_config_run.go |
Manages isolated per-run files. |
cmd/entire/cli/review_trust_inventory.go |
Filters inventory for isolated agents. |
cmd/entire/cli/review_trust_inventory_test.go |
Tests isolated inventory reporting. |
cmd/entire/cli/agent/pi/reviewer.go |
Wires Pi configuration preparation. |
cmd/entire/cli/agent/pi/review_config.go |
Applies isolated Pi extensions. |
cmd/entire/cli/agent/pi/review_config_test.go |
Tests Pi extension isolation. |
cmd/entire/cli/agent/codex/reviewer.go |
Wires Codex preparation and arguments. |
cmd/entire/cli/agent/codex/review_config.go |
Applies Codex trust and MCP overrides. |
cmd/entire/cli/agent/codex/review_config_test.go |
Tests Codex configuration handling. |
cmd/entire/cli/agent/claudecode/reviewer.go |
Wires Claude configuration preparation. |
cmd/entire/cli/agent/claudecode/review_config.go |
Applies Claude settings, MCP, and plugin isolation. |
cmd/entire/cli/agent/claudecode/review_config_test.go |
Tests Claude isolation and validation. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+79
to
+86
| for _, ext := range cfg.Extensions { | ||
| if !filepath.IsAbs(ext) { | ||
| return fmt.Errorf("extension %q: use an absolute path", ext) | ||
| } | ||
| if under(ext, forbiddenRoots) { | ||
| return fmt.Errorf("extension %q is inside a checkout; keep reviewer extensions outside the repository", ext) | ||
| } | ||
| } |
Comment on lines
+91
to
+95
| var server struct { | ||
| Command string `json:"command"` | ||
| Args []string `json:"args"` | ||
| URL string `json:"url"` | ||
| } |
Comment on lines
+123
to
+128
| err = settings.ModifyClonePreferences(ctx, func(p *settings.ClonePreferences) error { | ||
| if p.ReviewProfiles == nil { | ||
| p.ReviewProfiles = map[string]settings.ReviewProfileConfig{} | ||
| } | ||
| p.ReviewProfiles[profileName] = profile | ||
| return nil |
| if what == trustWhatNothing { | ||
| switch { | ||
| case isolated && what != trustWhatNothing: | ||
| fmt.Fprintf(errOut, "The code is by someone else; the review agent uses the profile's config but would still load the branch's skills and commands (%s --show-config lists them).\n", command) |
Parsing shell to decide whether a command is safe added complexity to a security check. Commands are now refused outright if they use shell syntax (; & | < > ( ) $ ` quotes), so splitting on whitespace is exact and every word is checked; hooks that need a shell go in a script at an absolute path. The shell-word parser is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M4EA872H4EW8BVS8TRD1ZGY7
A project launcher given by absolute path (/usr/local/bin/npx) was accepted, though it still resolves tools from the reviewed project; launchers now match by program name, including Windows extensions. A --flag=value word was checked whole, so --config=/opt/x.json read as a relative path; only the value is checked now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M4EB2DN44AAJS65D8680PKYA
A launcher such as npx is refused even by absolute path, so the old advice led to a second refusal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M4EBCRMS9W6QPT24XB0QRE41
- Codex: mark the checkout and main repo untrusted in one -c override; a second projects= assignment replaced the first. - Clone-local preferences keep only the reviewer configs, by profile and reviewer, and overlay them at load, so later profile edits still apply. - --edit keeps reviewer configs when re-saving a local profile; only the shared file drops them. - env values in MCP servers and Claude settings may not point into a checkout, and *PATH variables list only absolute directories. - Checkout containment follows symlinks and ignores case on Windows. - The no-terminal refusal says Pi's settings still load for an isolated Pi. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M4HQEWEBDX2ZT1ZRTA2FZ88J
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Entire-Checkpoint: 01M4HR0XXR145GNZ0V01YWR5HZ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://entire.io/gh/entireio/cli/trails/1516
A reviewer in a review profile can carry its own agent config, set with
entire review --editorentire review --configure <profile> --set-config <reviewer>=<config.json|isolated|none>. It replaces the reviewed checkout's hooks, MCP servers and extensions for that reviewer. This is the review-profile form of the--agent-configproposal discussed on trail 1493 (#2671).--setting-sources user,--strict-mcp-configsettings(with Entire's tracking hooks added) andmcp_servers, passed as 0600 files.claude/skillsand.claude/commandsas theprojectplugin, copied from the committed tree (/reviewbecomes/project:review)mcp_serversvia-c; hooks and literal MCPenvvalues are refused--no-extensionsextensions, plus Entire's own.pi/settings.json.entire/settings.local.json); a config in the committed file, the judge or the legacyreviewmap is dropped with a note, and a local file that can't be verified fails the review.;,&,|, redirects,$, backticks, quotes) is refused, so every word is checked; relative paths,$CLAUDE_PROJECT_DIRand project launchers (npx,uvx, …) are refused, at save time and before each run. Hooks that need a shell go in a script at an absolute path.--show-config(isolated_agentsin--json) name the reviewers using their own config and list only what still loads. Each run prints which reviewers use their own config.+1768/-57 in 28 files: about 1,190 lines of code, 530 of tests, 47 of docs.
Testing
mise run checkpasses.$CLAUDE_PROJECT_DIR, per-agent fields), per-agent argv and files (Claude plugin copy, symlinked skill refused, Codex linked-worktree trust overrides, Pi extensions), gate inventory for profile-config agents,--set-configsaving to clone-local preferences.--show-configand the agent refusal checked on a branch by someone else.Follow-ups
🤖 Generated with Claude Code
Note
High Risk
Changes how review agents load hooks, MCP, and extensions from untrusted branches, with command validation and fail-closed behavior, but misconfiguration or gaps could still affect security-sensitive review runs.
Overview
Adds per-reviewer agent config on review profiles so a reviewer can run with your hooks/MCP/extensions instead of the reviewed checkout’s, while branch skills can still load where supported.
Configuration & storage:
ReviewConfiggains optionalconfig(settings,mcp_servers,extensions). Set viaentire review --editor--configure --set-config reviewer=<file|isolated|none>. Config is never written to shared.entire/settings.json; it lives in clone-local preferences or untracked.entire/settings.local.json, with the same trust rules as reviewer prompts. Unverifiable local config blocks the review rather than falling back to checkout config.Runtime:
ReviewerTemplateadds aPreparestep that validates commands (absolute paths outside checkouts, nonpx/relative/$CLAUDE_PROJECT_DIR, per-agent field support) and writes ephemeral 0600 files under the user cache. Claude Code uses user-only settings plus profile MCP, copies committed skills/commands as theprojectplugin, and rewrites/skill→/project:skill. Codex marks checkout and main repo untrusted and applies profile MCP via-c. Pi uses--no-extensionsplus Entire’s extension and profile extensions.Trust gate: Inspectors take
TrustAgentwith anIsolatedflag;--show-configexposesisolated_agentsand narrower approval copy when only branch skills still execute.Reviewed by Cursor Bugbot for commit 26dfb3c. Configure here.