Skip to content

Harden hook-kit guard precision: overwrite-guard tool/field coverage and push-detail generic-word matching #295

Description

@daegunjhy

Problem

Two hook-kit guard scripts pass a permissive check where a stricter one is needed, letting the exact case they exist to catch slip through. Both were flagged by CodeRabbit's post-merge review of #292 and verified still present on current local HEAD.

1. block-write-file-overwrite.sh — missing Write/file_path coverage

block-write-file-overwrite.sh only recognizes the Claude tool names write_to_file / default_api:write_to_file, and only reads .tool_input.TargetFile / .tool_input.target_file for the target path. It never checks tool_name == "Write" nor .tool_input.file_path (Claude Code's actual Write tool uses file_path, not TargetFile).

edit-guard.sh's own check_write_file_overwrite does recognize Write|write_to_file, but it exempts */task.md and */scratch/* paths outright — relying on block-write-file-overwrite.sh to cover the task.md case specifically. Since block-write-file-overwrite.sh never fires for a plain Write tool call using file_path, a Write call targeting an existing task.md with Overwrite: true can pass both guards unblocked.

2. ask-guard.sh — generic-word push-detail bypass

In check_push_without_details(), has_remote is set to 1 whenever the option text matches generic words like remote|branch|local|main|master — not an actual remote/branch pair (e.g. origin/local). Similarly has_commit_info is set to 1 for the bare word commit — not an actual SHA or a labeled commit subject.

An option such as "Push the commit to main" satisfies both checks (main → has_remote, commit → has_commit_info) without identifying any concrete remote, branch, SHA, or commit subject — defeating the intent of the git-push-detail HARD STOP the guard exists to enforce.

Scope

  1. skills/hook-kit/resources/block-write-file-overwrite.sh:
    • Recognize tool_name == "Write" in addition to write_to_file / default_api:write_to_file for the Claude branch.
    • Read .tool_input.file_path as a fallback target-path field alongside .tool_input.TargetFile / .tool_input.target_file.
    • Coordinate with edit-guard.sh's task.md/scratch exemption so the two guards don't leave a combined gap — either this guard now fully covers what edit-guard.sh exempts, or the exemption is narrowed to match this guard's actual coverage.
  2. skills/hook-kit/resources/ask-guard.sh check_push_without_details():
    • Require an actual remote-and-branch pair (e.g. origin/<branch> shape, or an explicit branch name token) for has_remote, not bare remote|branch|local|main|master.
    • Require a real commit SHA ([0-9a-f]{7,40}) or an explicitly labeled commit subject for has_commit_info, not the bare word commit.
  3. Add or extend self-tests for both scripts covering the previously-passing bypass cases (a Write call with file_path targeting an existing task.md; an ask option like "Push the commit to main").

Verification

Feature Procedure Expected Result
Write-tool overwrite blocked Pipe a Claude Write tool_name payload with file_path pointing at an existing task.md and Overwrite: true into block-write-file-overwrite.sh Guard denies (exit 2 / stderr DENIED)
Combined guard coverage Confirm edit-guard.sh's task.md/scratch exemption and block-write-file-overwrite.sh's coverage together leave no gap for registered Write/write_to_file tool names No case passes both guards unblocked
Push-detail generic bypass blocked Run ask-guard.sh against an ask option text "Push the commit to main" Guard denies (previously passed)
Push-detail concrete case still passes Run ask-guard.sh against an ask option text naming an explicit remote/branch (e.g. origin/local) and a commit SHA or labeled subject Guard allows
Existing self-tests Run each script's existing self-test/fixture suite All still pass

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions