Harden Argus MCP path resolution and propagate path errors

[Meru143]

Motivation

• Prevent tools from accessing paths outside the configured repository by canonicalizing and validating requested paths against self.repo_path to avoid directory traversal or absolute path escapes.
• Make path resolution fallible with clear MCP errors so tool handlers can propagate and report path issues consistently.

Description

• Changed ArgusServer::resolve_path signature to return Result<PathBuf, McpError> and updated the implementation to canonicalize the configured repo root and the requested path, handle relative vs absolute inputs, and reject any resolved path that does not start with the canonical repo path (error via McpError).
• Updated tool handlers search_codebase, get_repo_map, get_hotspots, and get_history to call resolve_path(&params.path)? and thus propagate MCP errors cleanly.
• Added unit tests exercising path resolution for the four required scenarios (relative in-repo accepted, absolute in-repo accepted, ../ escape rejected, absolute out-of-repo rejected) inside crates/argus-mcp/src/tools.rs and added tempfile to crates/argus-mcp dev-dependencies to support the tests.

Testing

• Ran cargo fmt --all successfully.
• Ran cargo test -p argus-mcp which ran the new unit tests (4 passed), integration tests (8 passed), and doc-tests (3 passed), with all tests succeeding.

---

Codex Task

[coderabbitai]

Warning

Rate limit exceeded

@Meru143 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 10 minutes and 20 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between c74b03e and 9f38932.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (2)

• crates/argus-mcp/Cargo.toml
• crates/argus-mcp/src/tools.rs

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch codex/update-argusserver-path-resolution-and-handlers

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This PR hardens the Argus MCP server's path resolution to prevent directory traversal attacks and improves error handling by making path resolution fallible with clear MCP errors.

Changes:

• Modified resolve_path to return Result<PathBuf, McpError> with canonicalization-based validation
• Updated all four tools that accept path parameters to propagate path resolution errors
• Added comprehensive unit tests for path security scenarios

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File	Description
crates/argus-mcp/src/tools.rs	Implemented secure path resolution with canonicalization and boundary checking; updated tool handlers to propagate errors; added unit tests
crates/argus-mcp/Cargo.toml	Added tempfile dev-dependency for testing
Cargo.lock	Updated lock file with tempfile dependency

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The "safe" directory created on this line is never used in the test. The parent directory traversal test (using "../") will work without creating this directory because the parent of the temporary directory (typically /tmp) already exists. Consider removing this line to simplify the test.

Suggested change

fs::create_dir_all(repo.path().join("safe")).unwrap();