Skip to content

hooks: cover -f and +refspec force-push spellings#132

Open
truffle-dev wants to merge 1 commit into
ghostwright:mainfrom
truffle-dev:fix/dangerous-command-blocker-force-push-idioms-131
Open

hooks: cover -f and +refspec force-push spellings#132
truffle-dev wants to merge 1 commit into
ghostwright:mainfrom
truffle-dev:fix/dangerous-command-blocker-force-push-idioms-131

Conversation

@truffle-dev
Copy link
Copy Markdown
Contributor

Closes #131.

Problem

The dangerous-command blocker has one force-push pattern,
/git\s+push\s+.*--force/, which catches --force,
--force-with-lease, and --force-if-includes via substring
prefix. It misses two equivalent spellings:

  1. The short flag -f, same semantics as --force.
  2. The refspec prefix +<src>:<dst>, where a leading + tells
    the remote to accept a non-fast-forward update.

The asymmetric coverage blocks the verbose-and-safer spellings
while letting the bare-and-riskier ones through.

Fix

Two additional patterns in DANGEROUS_COMMANDS:

{ pattern: /git\s+push\s+(?:\S+\s+)*-f(?=\s|$)/, label: "git push -f" },
{ pattern: /git\s+push\s+.*\s["']?\+\S+/, label: "git push +refspec" },

The -f pattern uses a token-walk (?:\S+\s+)* + boundary
lookahead so it matches -f as a standalone flag and doesn't
trip on substrings like --force-if-includes or -fwoo.

The +refspec pattern requires whitespace (or a wrapping quote)
immediately before the +, so a + inside a token like
tag+sign:main does not false-positive.

Tests

Six new cases in src/agent/__tests__/hooks.test.ts:

Case Expected
git push -f origin main block
git push origin +main:main block
git push origin "+HEAD:refs/heads/main" block
git push --force-with-lease origin main block (already, asserts intent)
git push origin tag+sign:main allow (false-positive guard for + in token)
ls -f /tmp allow (false-positive guard for -f outside push)

Also prototype-tested 21 cases off-tree before edit covering
git fetch --force, git stash push --force, git config push.default current, quoted-refspec forms, and multi-flag
push lines.

Verification

  • bun test src/agent/__tests__/hooks.test.ts — 17/17 pass
  • Full suite — same 9 pre-existing failures as origin/main
    (handleEmailLogin Resend, loadConfig fixture-env, phantom init
    yaml, assemblePrompt bare-metal); none touched by this PR

The dangerous-command blocker catches `--force` (including the
`--force-with-lease` and `--force-if-includes` superstring forms)
but misses two equivalent spellings: the short flag `-f`, and the
refspec prefix `+<src>:<dst>` that tells the remote to accept a
non-fast-forward update.

Adds two patterns. The `-f` form uses a token-walk
`(?:\\S+\\s+)*` + boundary lookahead so substrings like
`--force-if-includes` and `-fwoo` don't trip it. The `+refspec`
form requires whitespace (or a wrapping quote) immediately before
the `+`, so a token like `tag+sign:main` is not a false positive.

Closes ghostwright#131.
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.

hooks: dangerous-command blocker misses -f and +refspec force-push idioms

1 participant