Skip to content

fix(tools): prevent command argument injection and path traversal in checkpoint manager#7919

Closed
Dusk1e wants to merge 1 commit into
NousResearch:mainfrom
Dusk1e:fix/checkpoint-manager-input-validation
Closed

fix(tools): prevent command argument injection and path traversal in checkpoint manager#7919
Dusk1e wants to merge 1 commit into
NousResearch:mainfrom
Dusk1e:fix/checkpoint-manager-input-validation

Conversation

@Dusk1e

@Dusk1e Dusk1e commented Apr 11, 2026

Copy link
Copy Markdown
Contributor

What

Fixed a critical security vulnerability in tools/checkpoint_manager.py where unsanitized user inputs were passed directly to git subprocess commands, enabling Git argument injection and path traversal.

  • Added _validate_commit_hash(str) to enforce strict hexadecimal patterns and reject inputs starting with - (e.g., --patch).
  • Added _validate_file_path(str, str) to reject absolute paths and prevent directory escapes (e.g., ../outside_file) using pathlib.Path.resolve().
  • Implemented these validations seamlessly inside CheckpointManager.restore() and diff().
  • Authored comprehensive regression tests under the new TestSecurity suite in test_checkpoint_manager.py.

Why

The vulnerability was exposed in two main workflows:

  1. The /rollback <hash> command passed from the CLI via cli.py parsing logic.
  2. The /rollback slash command dispatched by the messaging gateway (gateway/run.py).

Because user input flowed into git checkout <hash> -- <file_path> directly, malicious payloads passed as the hash could trigger unintended git flags (argument injection). Furthermore, malicious sequences like ../ within file_path allowed attackers to restore/overwrite arbitrary files outside the intended project boundary, leading to an environment escape.

How to test

  1. Observe the regression tests passing with full coverage:
pytest tests/tools/test_checkpoint_manager.py

…checkpoint manager

This commit addresses a security vulnerability where unsanitized user inputs for commit_hash and file_path were passed directly to git commands in CheckpointManager.restore() and diff(). It validates commit hashes to be strictly hexadecimal characters without leading dashes (preventing flag injection like '--patch') and enforces file paths to stay within the working directory via root resolution. Regression tests test_restore_rejects_argument_injection, test_restore_rejects_invalid_hex_chars, and test_restore_rejects_path_traversal were added.
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #7944 — your commit was cherry-picked onto current main with your authorship preserved in git log. Thanks for the contribution, @Dusk1e!

One note for posterity: the test case "abc; rm -rf /" validates the hex regex correctly, but it's worth clarifying that subprocess.run() is called with a list (not shell=True), so ;" and & characters would never actually trigger shell command injection here — they'd just be literal string arguments to git that fail. The *real* security value of this fix is preventing **git-level argument injection** (--patch, --exec, --output=file) where crafted inputs starting with -get interpreted as git flags before the--` separator. The hex regex catches both cases, which is the right defense-in-depth approach.

@teknium1 teknium1 closed this Apr 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants