Skip to content

Commit 288d797

Browse files
lapc506claude
lapc506
and
claude
committed
fix(hooks): close inline-DB-mutation regex bypasses + walk up for sentinel
Greptile PR #25 review (Confidence 2/5) flagged 5 P1 findings against the v1.17.0 inline-db-mutation family. This commit closes all of them with TDD-style coverage (failing tests added first against the original patterns, then the patterns / lookup logic updated to make them pass). Regex bypasses fixed: - mysql: spacing between -e/--execute and the SQL keyword is now [[:space:]]* (was [[:space:]]+), so the short form `mysql -e"DROP ..."` and the long form `mysql --execute="..."` are no longer bypasses. - psql: same fix for -c/--command — `psql -c"UPDATE ..."` now blocks. - mongo: --eval now matches the mutation method anywhere up to a pipe, not just inside the first non-space token after --eval. Real-world `mongo --eval "db.x.update({_id: 1}, ...)"` (whitespace inside quoted JS) now blocks. The --eval= equals shape is also covered. - gcloud sql: tolerates zero-or-more `--flag[=value]` tokens between `gcloud` and `sql`. `gcloud --project=PROD sql export sql ...` — exactly the shape that targets production — now blocks. Sentinel walk-up: - hooks/lib/eval-rule.sh now walks up from cwd looking for a `.git` marker (file or directory; worktrees use a file) and resolves `disable_if_repo_file` relative to that root. Falls back to cwd if no git root is found within 32 levels (preserves the previous behavior for tarball deploys). - New hooks/test-sentinel-walkup.sh exercises 5 cases: root cwd with sentinel, subdir cwd with sentinel at root, subdir cwd without sentinel, no-git fallback with sentinel at cwd, no-git fallback with sentinel only at parent. CI now runs both test suites. Verification: - Existing hook smoke tests: 274 / 274 passing (was 262 baseline, +12 new tests for the bypasses Greptile flagged). - Sentinel walk-up tests: 5 / 5 passing. - TDD red→green confirmed: with the regex / lookup changes reverted, the 7 new bypass tests and the subdir-with-root-sentinel test all fail; with the fixes applied, they all pass. - shellcheck clean on hooks/lib/eval-rule.sh + new test script. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 83e695a commit 288d797

6 files changed

Lines changed: 386 additions & 9 deletions

File tree

‎.github/workflows/test-hooks.yml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,9 @@ jobs:
3737
3838
- name: Run hook smoke tests
3939
run: bash hooks/test-hooks.sh
40+
41+
- name: Run sentinel walk-up tests
42+
# Exercises the `disable_if_repo_file` lookup from subdirectories
43+
# (Greptile #25 P1 regression — sentinel at repo root must be honored
44+
# even when the hook fires from any nested cwd within the repo).
45+
run: bash hooks/test-sentinel-walkup.sh

‎CHANGELOG.md‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1818
1919
## [Unreleased]
2020

21+
### Fixed
22+
- **Inline-DB-mutation regex bypasses** (Greptile PR #25, P1 + Security):
23+
- `mysql -e"..."` / `mysql --execute="..."` short/long-option-no-space
24+
shapes now block (spacing between `-e`/`--execute` and the SQL keyword
25+
is `[[:space:]]*` instead of `[[:space:]]+`).
26+
- `psql -c"..."` / `psql --command="..."` short/long-option-no-space
27+
shapes now block.
28+
- `mongo --eval "db.x.update(...)"` with whitespace inside the quoted JS
29+
expression now blocks (regex no longer requires the mutation method to
30+
live inside a single non-space token after `--eval`). `--eval=` shape
31+
also covered.
32+
- `gcloud --project=PROD sql export ...` and other variants with global
33+
flags between `gcloud` and `sql` now block (regex tolerates
34+
zero-or-more `--flag[=value]` tokens before the `sql` command group).
35+
- **`disable_if_repo_file` sentinel walks up to repo root** (Greptile PR
36+
#25, P1). Previously the lookup only checked `./<sentinel>` in the
37+
process cwd, so a sentinel placed at the documented location (repo
38+
root) was ignored whenever the hook fired from any subdirectory. The
39+
lookup now walks upward looking for a `.git` marker (file or
40+
directory — worktrees use a file) and resolves the sentinel relative
41+
to that root, with a cwd fallback for non-git deployments. Added
42+
`hooks/test-sentinel-walkup.sh` to lock the behavior in CI.
43+
2144
## [1.17.0] - 2026-05-21
2245

2346
### Added

‎hooks/lib/eval-rule.sh‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,22 @@ if [ -n "$BYPASS_MARKER" ]; then
8585
fi
8686

8787
# Per-repo escape hatch: if the rule declares `disable_if_repo_file: <name>`
88-
# and that file exists at the current working directory, the rule is a no-op.
88+
# and that file exists at the repo root, the rule is a no-op.
8989
# Use case: a repo explicitly opts out of a class of enforcement because its
9090
# workflow legitimately requires the otherwise-blocked operation (e.g., a
9191
# data-migration repo that runs inline DB mutations as part of its job).
9292
# The filename is validated to be a flat, safe name (no slashes / dots /
9393
# wildcards) so the lookup can't escape the cwd or pull in unintended files.
94+
#
95+
# Lookup strategy (Greptile #25 P1): walk upward from cwd looking for a
96+
# repo-root marker (`.git` file or dir — `.git` is a file in worktrees) and
97+
# check the sentinel at that root. If no repo root is found within a
98+
# bounded ascent, fall back to checking cwd itself (preserves the previous
99+
# behavior for repos without `.git`, e.g. tarball deploys).
100+
#
101+
# The ascent is bounded (32 levels) so an unrooted absolute path can't loop
102+
# forever — `/` has no parent, so the loop terminates naturally, but the
103+
# bound is defensive against pathological symlink trees.
94104
DISABLE_IF_REPO_FILE="$(printf '%s' "$RULE_JSON" | jq -r '.disable_if_repo_file // empty')"
95105
if [ -n "$DISABLE_IF_REPO_FILE" ]; then
96106
# Validate filename: only [a-zA-Z0-9._-], and reject "." / ".." sentinels.
@@ -101,6 +111,30 @@ if [ -n "$DISABLE_IF_REPO_FILE" ]; then
101111
echo "make-no-mistakes: rule ${RULE_ID} has invalid disable_if_repo_file (must be a flat filename of [a-zA-Z0-9._-]); ignoring escape hatch." >&2
102112
;;
103113
*)
114+
# Walk up looking for a `.git` marker (worktrees use a `.git` *file*,
115+
# standard checkouts use a `.git` *directory* — both satisfy `-e`).
116+
SENTINEL_DIR=""
117+
SEARCH_DIR="$PWD"
118+
DEPTH=0
119+
while [ "$DEPTH" -lt 32 ]; do
120+
if [ -e "${SEARCH_DIR}/.git" ]; then
121+
SENTINEL_DIR="$SEARCH_DIR"
122+
break
123+
fi
124+
PARENT="$(dirname "$SEARCH_DIR")"
125+
# `dirname /` returns `/` — bail when we stop moving up.
126+
if [ "$PARENT" = "$SEARCH_DIR" ]; then
127+
break
128+
fi
129+
SEARCH_DIR="$PARENT"
130+
DEPTH=$((DEPTH + 1))
131+
done
132+
133+
# If we found a repo root, check the sentinel there. Otherwise fall
134+
# back to cwd so non-git deployments still have an opt-out path.
135+
if [ -n "$SENTINEL_DIR" ] && [ -f "${SENTINEL_DIR}/${DISABLE_IF_REPO_FILE}" ]; then
136+
exit 0
137+
fi
104138
if [ -f "./${DISABLE_IF_REPO_FILE}" ]; then
105139
exit 0
106140
fi

‎hooks/rules/rules.json‎

Lines changed: 112 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2795,7 +2795,7 @@
27952795
"match": [
27962796
{
27972797
"field": "command",
2798-
"pattern": "(\\bmysql[[:space:]][^|<]*(-e|--execute)([[:space:]]+|=)['\"]?[[:space:]]*(UPDATE|DELETE|INSERT|REPLACE|CREATE|ALTER|DROP|GRANT|REVOKE|TRUNCATE|RENAME)\\b)|(\\bmysql[[:space:]][^|]*<[[:space:]]*[^[:space:]]+\\.sql\\b)|(\\bmysqldump\\b[^|]*\\|[[:space:]]*mysql\\b)",
2798+
"pattern": "(\\bmysql[[:space:]][^|<]*(-e|--execute)([[:space:]]*|=)['\"]?[[:space:]]*(UPDATE|DELETE|INSERT|REPLACE|CREATE|ALTER|DROP|GRANT|REVOKE|TRUNCATE|RENAME)\\b)|(\\bmysql[[:space:]][^|]*<[[:space:]]*[^[:space:]]+\\.sql\\b)|(\\bmysqldump\\b[^|]*\\|[[:space:]]*mysql\\b)",
27992799
"flags": "i"
28002800
},
28012801
{
@@ -2836,6 +2836,33 @@
28362836
},
28372837
"expected_exit": 2
28382838
},
2839+
{
2840+
"name": "blocks-mysql-long-execute-space",
2841+
"input": {
2842+
"tool_input": {
2843+
"command": "mysql --execute \"UPDATE users SET active=1\""
2844+
}
2845+
},
2846+
"expected_exit": 2
2847+
},
2848+
{
2849+
"name": "blocks-mysql-long-execute-equals",
2850+
"input": {
2851+
"tool_input": {
2852+
"command": "mysql --execute=\"DROP TABLE legacy\""
2853+
}
2854+
},
2855+
"expected_exit": 2
2856+
},
2857+
{
2858+
"name": "blocks-mysql-short-e-no-space-quoted",
2859+
"input": {
2860+
"tool_input": {
2861+
"command": "mysql -uroot -e\"DROP TABLE legacy\""
2862+
}
2863+
},
2864+
"expected_exit": 2
2865+
},
28392866
{
28402867
"name": "blocks-mysql-stdin-redirect",
28412868
"input": {
@@ -2910,7 +2937,7 @@
29102937
"match": [
29112938
{
29122939
"field": "command",
2913-
"pattern": "(\\bpsql\\b[^|]*(-c|--command)([[:space:]]+|=)['\"]?[[:space:]]*(UPDATE|INSERT|REPLACE|CREATE|ALTER|GRANT|REVOKE|RENAME)\\b)|(\\bpg_restore\\b)",
2940+
"pattern": "(\\bpsql\\b[^|]*(-c|--command)([[:space:]]*|=)['\"]?[[:space:]]*(UPDATE|INSERT|REPLACE|CREATE|ALTER|GRANT|REVOKE|RENAME)\\b)|(\\bpg_restore\\b)",
29142941
"flags": "i"
29152942
},
29162943
{
@@ -2960,6 +2987,24 @@
29602987
},
29612988
"expected_exit": 2
29622989
},
2990+
{
2991+
"name": "blocks-psql-c-no-space-quoted",
2992+
"input": {
2993+
"tool_input": {
2994+
"command": "psql -d mydb -c\"UPDATE users SET active=true\""
2995+
}
2996+
},
2997+
"expected_exit": 2
2998+
},
2999+
{
3000+
"name": "blocks-psql-long-command-equals",
3001+
"input": {
3002+
"tool_input": {
3003+
"command": "psql -d mydb --command=\"UPDATE sessions SET expired=true\""
3004+
}
3005+
},
3006+
"expected_exit": 2
3007+
},
29633008
{
29643009
"name": "blocks-pg-restore",
29653010
"input": {
@@ -3104,7 +3149,7 @@
31043149
"match": [
31053150
{
31063151
"field": "command",
3107-
"pattern": "(\\b(mongo|mongosh)\\b[^|]*--eval[[:space:]]+[^[:space:]]*\\.(update|insert|delete|drop|createCollection|dropDatabase|replaceOne|save|findAndModify|deleteOne|deleteMany|updateOne|updateMany|insertOne|insertMany)\\b)|(\\bmongorestore\\b)"
3152+
"pattern": "(\\b(mongo|mongosh)\\b[^|]*--eval[[:space:]=][^|]*\\.(update|insert|delete|drop|createCollection|dropDatabase|replaceOne|save|findAndModify|deleteOne|deleteMany|updateOne|updateMany|insertOne|insertMany)\\b)|(\\bmongorestore\\b)"
31083153
},
31093154
{
31103155
"field": "command",
@@ -3126,6 +3171,33 @@
31263171
},
31273172
"expected_exit": 2
31283173
},
3174+
{
3175+
"name": "blocks-mongo-eval-update-with-spaces-inside-arg",
3176+
"input": {
3177+
"tool_input": {
3178+
"command": "mongo --eval \"db.users.update({_id: 42}, {$set: {active: true}})\""
3179+
}
3180+
},
3181+
"expected_exit": 2
3182+
},
3183+
{
3184+
"name": "blocks-mongosh-eval-insertOne-spaces",
3185+
"input": {
3186+
"tool_input": {
3187+
"command": "mongosh --eval \"db.audit.insertOne({ event: \\\"oneshot\\\" })\""
3188+
}
3189+
},
3190+
"expected_exit": 2
3191+
},
3192+
{
3193+
"name": "blocks-mongo-eval-equals",
3194+
"input": {
3195+
"tool_input": {
3196+
"command": "mongo --eval=\"db.users.deleteMany({active:false})\""
3197+
}
3198+
},
3199+
"expected_exit": 2
3200+
},
31293201
{
31303202
"name": "blocks-mongo-eval-deleteMany",
31313203
"input": {
@@ -3324,7 +3396,7 @@
33243396
"match": [
33253397
{
33263398
"field": "command",
3327-
"pattern": "\\bgcloud[[:space:]]+sql[[:space:]]+(import|export)[[:space:]]+(sql|csv|bak)\\b",
3399+
"pattern": "\\bgcloud[[:space:]]+(--?[a-zA-Z0-9_-]+(=[^[:space:]]*)?[[:space:]]+)*sql[[:space:]]+(import|export)[[:space:]]+(sql|csv|bak)\\b",
33283400
"flags": "i"
33293401
},
33303402
{
@@ -3377,6 +3449,42 @@
33773449
},
33783450
"expected_exit": 2
33793451
},
3452+
{
3453+
"name": "blocks-gcloud-global-project-sql-export",
3454+
"input": {
3455+
"tool_input": {
3456+
"command": "gcloud --project=my-prod sql export sql my-instance gs://backups/dump.sql --database=mydb"
3457+
}
3458+
},
3459+
"expected_exit": 2
3460+
},
3461+
{
3462+
"name": "blocks-gcloud-global-quiet-sql-import",
3463+
"input": {
3464+
"tool_input": {
3465+
"command": "gcloud --quiet sql import sql my-instance gs://backups/dump.sql --database=mydb"
3466+
}
3467+
},
3468+
"expected_exit": 2
3469+
},
3470+
{
3471+
"name": "blocks-gcloud-global-account-sql-export-csv",
3472+
"input": {
3473+
"tool_input": {
3474+
"command": "gcloud --account=svc@example.iam.gserviceaccount.com sql export csv my-instance gs://backups/users.csv --database=mydb --query=\"SELECT * FROM users\""
3475+
}
3476+
},
3477+
"expected_exit": 2
3478+
},
3479+
{
3480+
"name": "blocks-gcloud-multiple-global-flags-sql-export",
3481+
"input": {
3482+
"tool_input": {
3483+
"command": "gcloud --project=my-prod --quiet sql export sql my-instance gs://backups/dump.sql --database=mydb"
3484+
}
3485+
},
3486+
"expected_exit": 2
3487+
},
33803488
{
33813489
"name": "allows-gcloud-sql-instances-list",
33823490
"input": {

0 commit comments

Comments
 (0)