Skip to content

Commit a1ba2f2

Browse files
authored
fix(miner): make the -f force-push deny-rule token-aware (#2886)
Substring matching on "-f" false-positively blocked git push --follow-tags (the flag contains "-f" as a substring). Add inputTokenPattern, a rule constraint that matches a whole whitespace-separated token instead of a substring, and use it for the short-flag force-push guard.
1 parent c0b6bee commit a1ba2f2

4 files changed

Lines changed: 48 additions & 9 deletions

File tree

packages/gittensory-miner/lib/deny-hooks.d.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ export type DenyRule = {
55
pathPattern?: string;
66
/** Optional substrings that must ALL appear in one string-shaped input field (e.g. a shell command). */
77
inputIncludesAll?: string[];
8+
/** Optional pattern that must match a whole whitespace-separated token (quotes stripped) of one
9+
* string-shaped input field — for flag-shaped needles where a substring test would false-positive
10+
* on an unrelated longer flag (e.g. `-f` vs. `--follow-tags`). */
11+
inputTokenPattern?: RegExp;
812
/** Human-readable reason surfaced when this rule blocks a call. */
913
reason: string;
1014
};

packages/gittensory-miner/lib/deny-hooks.js

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,11 @@
66
//
77
// A rule fires when its tool-name `matcher` matches AND every constraint it declares also matches:
88
// - `pathPattern` (a glob) must match some path-shaped string in the tool-call input, and/or
9-
// - `inputIncludesAll` (substrings) must ALL appear in a single string-shaped input field (e.g. a command).
10-
// A rule with neither constraint fires on the matcher alone. The built-in DEFAULT_DENY_RULES mirror the
9+
// - `inputIncludesAll` (substrings) must ALL appear in a single string-shaped input field (e.g. a command), and/or
10+
// - `inputTokenPattern` (a RegExp) must match a whole whitespace-separated token (quotes stripped) of a single
11+
// string-shaped input field — for flag-shaped needles like `-f`, where a substring test would also fire on
12+
// `--follow-tags`.
13+
// A rule with none of these constraints fires on the matcher alone. The built-in DEFAULT_DENY_RULES mirror the
1114
// forbidden-path patterns enforced in `scripts/check-mcp-package.mjs` plus a conservative git force-push guard.
1215

1316
/**
@@ -62,6 +65,15 @@ function collectInputStrings(input, seen = new WeakSet()) {
6265
return strings;
6366
}
6467

68+
/** Split a string-shaped input field into whitespace-separated tokens with surrounding quotes stripped —
69+
* shared by path-candidate expansion and flag-token matching below. */
70+
function splitTokens(value) {
71+
return value
72+
.split(/\s+/)
73+
.map((token) => token.replace(/^["']+|["']+$/g, ""))
74+
.filter(Boolean);
75+
}
76+
6577
/**
6678
* The candidate strings a path glob is tested against for one input value: the whole value AND each
6779
* whitespace-separated token (surrounding quotes stripped). A protected path is frequently embedded as one
@@ -71,12 +83,9 @@ function collectInputStrings(input, seen = new WeakSet()) {
7183
*/
7284
function pathCandidates(value) {
7385
const candidates = new Set([value, normalizePathCandidate(value)]);
74-
for (const token of value.split(/\s+/)) {
75-
const trimmed = token.replace(/^["']+|["']+$/g, "");
76-
if (trimmed) {
77-
candidates.add(trimmed);
78-
candidates.add(normalizePathCandidate(trimmed));
79-
}
86+
for (const trimmed of splitTokens(value)) {
87+
candidates.add(trimmed);
88+
candidates.add(normalizePathCandidate(trimmed));
8089
}
8190
return [...candidates].filter(Boolean);
8291
}
@@ -99,13 +108,18 @@ function ruleMatches(rule, toolName, inputStrings) {
99108
const needles = rule.inputIncludesAll.filter((needle) => typeof needle === "string");
100109
if (!inputStrings.some((value) => needles.every((needle) => value.includes(needle)))) return false;
101110
}
111+
if (rule.inputTokenPattern instanceof RegExp) {
112+
if (!inputStrings.some((value) => splitTokens(value).some((token) => rule.inputTokenPattern.test(token)))) {
113+
return false;
114+
}
115+
}
102116
return true;
103117
}
104118

105119
/**
106120
* The built-in house-rule deny set — a non-empty starting example a later phase can extend or replace. Mirrors the
107121
* forbidden-path regex in `scripts/check-mcp-package.mjs` (CI workflows, env files, secret-bearing paths, private
108-
* key material) and adds a conservative git force-push guard (a command carrying both `push` and `--force`).
122+
* key material) and adds conservative git force-push guards (a command carrying `push` plus a force flag).
109123
*/
110124
export const DEFAULT_DENY_RULES = [
111125
{ matcher: "*", pathPattern: "**/.github/workflows/**", reason: "Never modify CI workflows (.github/workflows/**)." },
@@ -120,6 +134,10 @@ export const DEFAULT_DENY_RULES = [
120134
{ matcher: "*", pathPattern: "**/*private*key*", reason: "Never touch private key material (**/*private*key*)." },
121135
{ matcher: "*", pathPattern: "**/*.pem", reason: "Never touch PEM key material (*.pem)." },
122136
{ matcher: "*", inputIncludesAll: ["push", "--force"], reason: "Never force-push (git push --force)." },
137+
// Token-matched rather than substring-matched: a substring test for "-f" would also fire on an
138+
// unrelated long flag like --follow-tags. Matches a whole short-option token (bundled or not)
139+
// whose letters include "f", e.g. -f, -uf, -fu, but not a "--"-prefixed long flag.
140+
{ matcher: "*", inputIncludesAll: ["push"], inputTokenPattern: /^-[a-z]*f[a-z]*$/i, reason: "Never force-push (git push -f)." },
123141
];
124142

125143
/**

test/fixtures/deny-hooks/cases.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,16 @@ export const denyHookFixtures: DenyHookFixture[] = [
8989
expected: { allowed: false, blockedByIncludes: "force-push" },
9090
},
9191
{ name: "allows a normal push", toolCall: { name: "Bash", input: { command: "git push origin main" } }, expected: { allowed: true } },
92+
{
93+
name: "blocks the short -f force-push flag",
94+
toolCall: { name: "Bash", input: { command: "git push -f origin main" } },
95+
expected: { allowed: false, blockedByIncludes: "force-push" },
96+
},
97+
{
98+
name: "GLOB EDGE: --follow-tags is NOT a force flag (token-matched, not a `-f` substring match)",
99+
toolCall: { name: "Bash", input: { command: "git push --follow-tags origin main" } },
100+
expected: { allowed: true },
101+
},
92102
{ name: "allows running the test suite", toolCall: { name: "Bash", input: { command: "npm test" } }, expected: { allowed: true } },
93103

94104
// ── multi-path edits + custom-rule cases ──────────────────────────────────────────────────────

test/unit/miner-deny-hooks.test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,13 @@ describe("evaluateDenyHooks — built-in DEFAULT_DENY_RULES", () => {
6161
expect(evaluateDenyHooks({ name: "Bash", input: { command: "git push origin main" } }).allowed).toBe(true);
6262
expect(evaluateDenyHooks({ name: "Bash", input: { command: "git commit --amend" } }).allowed).toBe(true);
6363
});
64+
65+
it("blocks the short -f force-push flag (bare or bundled) without false-flagging --follow-tags", () => {
66+
expect(evaluateDenyHooks({ name: "Bash", input: { command: "git push -f origin main" } }).allowed).toBe(false);
67+
expect(evaluateDenyHooks({ name: "Bash", input: { command: "git push -uf origin main" } }).allowed).toBe(false);
68+
// --follow-tags contains "-f" as a substring but is not a force flag — must not be blocked.
69+
expect(evaluateDenyHooks({ name: "Bash", input: { command: "git push --follow-tags origin main" } }).allowed).toBe(true);
70+
});
6471
});
6572

6673
describe("evaluateDenyHooks — rule composition and allow paths", () => {

0 commit comments

Comments
 (0)