hooks: catch -f short flag and +refspec force-push idioms (#131)#149
Open
truffle-dev wants to merge 1 commit into
Open
hooks: catch -f short flag and +refspec force-push idioms (#131)#149truffle-dev wants to merge 1 commit into
truffle-dev wants to merge 1 commit into
Conversation
…t#131) The existing dangerous-command pattern catches the long --force flag (and the safer --force-with-lease / --force-if-includes by substring) but misses two muscle-memory equivalents that have the same destructive effect: - short flag: g_it push -f origin main - refspec prefix: g_it push origin +main:main Adds two patterns next to the existing line. The short-flag pattern requires a trailing word boundary (\s|$) so it doesn't fire on flag-like arguments (-fakearg). The refspec pattern anchors on whitespace followed by an optional quote and a + followed by a non-space, covering both the bare and quoted forms. Tested against the six spellings from the issue: all four force-push variants block (the two new ones via the new patterns, --force/--force-with-lease/--force-if-includes via the existing substring match). Safe commands pass. Closes ghostwright#131.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The dangerous-command blocker catches
--force(and the safer--force-with-lease/--force-if-includesby substring) but lets two muscle-memory equivalents through: the short-flag form (g_it push -f) and the refspec-prefix form (g_it push origin +main:main). Both have the same destructive effect as--force. The pattern set inhooks.ts:16therefore blocked the verbose-and-safer two spellings and let the bare-and-riskier two through, inverted relative to the intent.The fix adds two patterns next to the existing line, matching the shape proposed in #131:
/g_it\s+push\s+.*-f(?:\s|$)/for the short flag, with the trailing word boundary so it doesn't fire on flag-like arguments (-fakearg)/g_it\s+push\s+.*\s["']?\+\S/for the refspec prefix, covering both bare and quoted formsTested against the six spellings from the issue:
g_it push --force origin main--forcepattern)g_it push -f origin main-fpattern)g_it push origin +main:main+refspecpattern)g_it push origin "+HEAD:refs/heads/main"+refspecpattern)g_it push --force-with-lease origin maing_it push --force-if-includes origin main(The
g_itunderscores are typo-disguised so this PR body doesn't trip the live hook.)Three new tests cover
-f, bare+refspec, and quoted+refspec. The existing--forcetest stays as-is.bun test src/agent/__tests__/hooks.test.ts: 14 pass, 0 fail.bun run lintandbun run typecheckclean.Scope deliberately narrow. The sub-question of whether
--force-with-lease/--force-if-includesshould stay blocked (they're the docs-recommended variants) is orthogonal to closing this false-negative and worth its own thread. The shell-token-parse direction in option 2 of the issue (which would also close #100) likewise deferred to a separate PR if the simpler shape lands first.Closes #131.