Skip to content

fix(tools): prevent git argument injection and path traversal in checkpoint manager#7944

Merged
teknium1 merged 1 commit into
mainfrom
hermes/hermes-80a283e0
Apr 11, 2026
Merged

fix(tools): prevent git argument injection and path traversal in checkpoint manager#7944
teknium1 merged 1 commit into
mainfrom
hermes/hermes-80a283e0

Conversation

@teknium1

Copy link
Copy Markdown
Contributor

Summary

Salvage of #7919 by @Dusk1e — cherry-picked onto current main with their authorship preserved.

Adds input validation to CheckpointManager.restore() and diff() to prevent:

  1. Git argument injection — crafted commit hashes starting with - (e.g. --patch, --exec) get interpreted as git flags when passed to git cat-file, git diff, git checkout before the -- separator
  2. Path traversalfile_path in restore() allowed absolute paths (/etc/passwd) and relative escapes (../../../etc/passwd)

Changes

  • _validate_commit_hash() — enforces 4-64 hex chars, rejects leading -
  • _validate_file_path() — rejects absolute paths, uses Path.resolve() + relative_to() for containment check
  • Validation applied at entry points of both restore() and diff()
  • TestSecurity test suite covering argument injection, invalid hex, path traversal, and valid path acceptance

Test results

41 passed in 1.13s

…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.
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