Skip to content

fix(hooks): make integrity messages match what was checked (fixes #5039) - #5058

Open
IG-ABHINAV wants to merge 1 commit into
rtk-ai:developfrom
IG-ABHINAV:fix-5039
Open

IG-ABHINAV wants to merge 1 commit into
rtk-ai:developfrom
IG-ABHINAV:fix-5039

Conversation

@IG-ABHINAV

Copy link
Copy Markdown

Fixes #5039.

What / why

Three integrity messages in src/hooks/integrity.rs do not match what actually happened:

  1. The tampered-hook error always prints ~/.claude/hooks/... even when CLAUDE_CONFIG_DIR points somewhere else — so with a custom config dir the message names a file that was never checked.
  2. A bad baseline (Invalid hash format ...) stops the command with the error alone; there is no hint that rtk init -g --auto-patch is what rewrites it, even though the docs promise one.
  3. runtime_check returns early whenever the hook file is missing, so the documented OrphanedHash warning ("hash file exists but hook is missing") can never print — an orphaned baseline is indistinguishable from nothing installed.

Approach

  • Print hook_path.display() in the tampered message instead of the hardcoded ~/.claude prefix.
  • Wrap verify_hook_at failures with \n To restore: rtk init -g --auto-patch (the original first line is preserved, so existing assertions still match).
  • Drop the early !hook_path.exists() return so the orphaned-hash warning is emitted; NotInstalled stays silent.

Tests

  • cargo fmt --all -- --check → PASS
  • cargo clippy --workspace --all-targets → PASS (0 lints; changed file confirmed linted)
  • cargo test --all --no-fail-fast → 5155 passed / 6 failed / 8 ignored
  • cargo test --lib hooks::integrity → 20 passed, 0 failed (includes the 2 new unit tests)
  • bash scripts/check-test-presence.sh origin/develop → OK

The 6 failures are proven pre-existing: the identical command set on a clean origin/develop (08a261a) fails the same 5 targets with the same 6 tests (find_cmd exit-status, read_lines_lossy UTF-8, help_passthrough_fuzz_test, plus 3 flaky targets — git_ls_tree_test, go_test_pipe_test, rewrite_shell_wrapper_test fail on a different individual test each run on both base and branch).

Manual end-to-end on Windows (target/debug/rtk.exe, CLAUDE_CONFIG_DIR=<tmp>/.claude):

  • tampered hook → exit 1, message shows the real path, no ~/.claude anywhere;
  • .rtk-hook.sha256 = not a hash → Invalid hash format ... followed by To restore: rtk init -g --auto-patch;
  • valid baseline with the hook deleted → rtk: warning: hash file exists but hook is missing + Run \rtk init -g` to reinstall.`, command still exits 0.

Judgment call: I kept the restore hint on the error path only (not on warnings) so routine output stays quiet — say the word if you would rather have it on the orphaned-hash warning too.

The tamper message named ~/.claude even when CLAUDE_CONFIG_DIR pointed elsewhere; print the path actually verified. A damaged baseline stopped the command with the error alone, so name the command that rewrites it. runtime_check returned before verify_hook_at whenever the hook was missing, so the documented OrphanedHash warning could never print.
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

Hook integrity messages: ~/.claude named under CLAUDE_CONFIG_DIR, no restore hint on a bad baseline, orphaned-baseline warning never prints

2 participants