Skip to content

Commit e930eba

Browse files
lapc506claude
andcommitted
fix(rules): address Greptile review on migration-discipline rules
Greptile flagged 4 issues on the initial PR (3/5 confidence). All four are now resolved without weakening the rules' intent: 1. Marketplace-safe production project ref (was: hardcoded literal) - Replace ukwovawzehnebuoowcec / xepaexmpawtpqtilhcpw in rules.yaml with __PROD_SUPABASE_REF__ / __STAGING_SUPABASE_REF__ placeholders. - Extend scripts/build-rules.mjs with an opt-in substitution layer that reads .private/substitutions.json (UPPER_SNAKE token map) and swaps placeholders before YAML parsing. Mirrors the existing gitignored .private/forbidden-names.txt IP-leak guard. - When the file is absent, placeholders remain in rules.json and the rule is documented inert (only fires for commands containing the literal placeholder text). Preferred over a silent broken protection for marketplace consumers. - hooks/rules/README.md documents the substitution workflow and lists currently-consumed tokens. 2. Cover supabase db push --linked, which silently bypassed the block when the linked project was prod. Pattern now matches __PROD_SUPABASE_REF__ OR --linked\b. Two new tests: blocks-supabase-db-push-linked and blocks-supabase-db-push-linked-with-extra-flags. Bypass marker prod-db-push-approved continues to apply. 3. Stop false-positive on gh pr create --body-file. Positive pattern tightened to require --body followed by space, "=", or end-of-string; added not_pattern --body-file as defense in depth. New test allows-pr-create-with-body-file. 4. Extend warn-psql-against-supabase-remote to cover pg_dump, pg_restore, pg_dumpall — equivalent vectors for ad-hoc Postgres CLI work against the live Supabase host. Two new tests: warns-on-pg-dump-supabase-remote and warns-on-pg-restore-supabase-remote. Test fixtures use generic example.supabase.co hosts (no team-specific refs). Test count: 80 -> 86 (6 new tests, all 86 pass). No version bump (still 1.7.0 — pre-merge fixes). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e3161a0 commit e930eba

4 files changed

Lines changed: 275 additions & 59 deletions

File tree

hooks/rules/README.md

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,43 @@ client-acme
8484
client-beta
8585
```
8686

87+
## Per-install substitutions (opt-in, gitignored)
88+
89+
Some rules need values that are specific to your team or environment —
90+
for example, the `block-supabase-db-push-prod` rule guards a Supabase
91+
production project ref that varies per organization. Hard-coding such
92+
values in the public rules.yaml would either leak the value upstream or
93+
leave consumers of this toolkit with a rule that silently never fires.
94+
95+
The build script supports literal-string substitution from a gitignored
96+
`.private/substitutions.json` file. The file is a flat JSON object where
97+
keys are UPPER_SNAKE token names and values are the literal replacement
98+
strings. For each pair, every occurrence of `__TOKEN__` in `rules.yaml` is
99+
replaced before the YAML is parsed, so both the rule patterns and the
100+
test fixtures see the substituted value.
101+
102+
Example `.private/substitutions.json`:
103+
104+
```json
105+
{
106+
"PROD_SUPABASE_REF": "abcdefghij1234567890",
107+
"STAGING_SUPABASE_REF": "klmnopqrst0987654321"
108+
}
109+
```
110+
111+
When the file is absent, `__TOKEN__` placeholders remain in `rules.json`
112+
verbatim. The rule still parses and runs; it simply does not fire for any
113+
real-world command (only for commands that happen to contain the literal
114+
placeholder text). This is a deliberately documented inert state — it is
115+
preferable to a silently-broken protection.
116+
117+
Tokens currently consumed by the published rule set:
118+
119+
- `PROD_SUPABASE_REF` — production Supabase project ref, used by the
120+
`block-supabase-db-push-prod` rule and its test fixtures.
121+
- `STAGING_SUPABASE_REF` — staging Supabase project ref, used as the
122+
negative-match example in the same rule's test array.
123+
87124
## Rule families
88125

89126
The manifest groups rules into informal families by prefix / domain.
@@ -110,12 +147,15 @@ Adding a new family is fine — just keep ids unique and follow the schema.
110147
`pr-create-with-migrations-needs-deploy-note`,
111148
`block-supabase-db-push-prod`) — keep schema mutations inside
112149
versioned `supabase/migrations/` files, nudge developers away from
113-
SQL-Editor-style direct execution against `*.supabase.co` hosts,
114-
remind PR authors to document migration deployment, and hard-block
115-
`supabase db push` aimed at the production project ref. Added after
116-
a Slack discussion surfaced drift between manually-applied SQL and
117-
the migrations directory when migrations failed to auto-run on
118-
staging after a teammate's PR merged.
150+
direct `psql` / `pg_dump` / `pg_restore` execution against
151+
`*.supabase.co` hosts, remind PR authors to document migration
152+
deployment, and hard-block `supabase db push` aimed at the production
153+
project ref or `--linked` (which transparently resolves to whichever
154+
project was last linked, possibly production). The production project
155+
ref is configured per install via the substitutions mechanism described
156+
above (`PROD_SUPABASE_REF` token). Added after a discussion surfaced
157+
drift between manually-applied SQL and the migrations directory when
158+
migrations failed to auto-run after a teammate's PR merged.
119159

120160
## Tier 2 — decomposing non-deterministic memories
121161

hooks/rules/rules.json

Lines changed: 75 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -982,27 +982,27 @@
982982
},
983983
{
984984
"id": "warn-psql-against-supabase-remote",
985-
"description": "Warn when psql/SQL is executed directly against a *.supabase.co host (drift risk vs versioned migrations)",
985+
"description": "Warn when psql / pg_dump / pg_restore is executed directly against a *.supabase.co host (drift risk vs versioned migrations)",
986986
"applies_to": [
987987
"Bash"
988988
],
989989
"match": [
990990
{
991991
"field": "command",
992-
"pattern": "psql.*\\.supabase\\.co",
992+
"pattern": "\\b(psql|pg_dump|pg_restore|pg_dumpall)\\b.*\\.supabase\\.co",
993993
"flags": "i"
994994
}
995995
],
996996
"action": "warn",
997997
"bypass_marker": "psql-supabase-remote",
998998
"memory_ref": "feedback_scripts_not_db.md",
999-
"message": "WARNING: direct psql execution against a Supabase remote host.\n\nSQL Editor / psql-against-remote workflows create drift between the\nversioned migrations directory and other devs' local databases.\n\nCorrect workflow:\n 1. Add a migration in supabase/migrations/<timestamp>_<slug>.sql\n 2. Test locally: supabase db reset --local\n 3. Push to develop — CI applies the migration to staging automatically\n 4. For production: William runs the create-release SOP\n\nUse the bypass marker only for legitimate hotfixes where the PR cycle\nis too slow.\n",
999+
"message": "WARNING: direct psql / pg_dump / pg_restore execution against a Supabase\nremote host.\n\nSQL Editor / direct-Postgres-CLI workflows create drift between the\nversioned migrations directory and other devs' local databases.\n\nCorrect workflow:\n 1. Add a migration in supabase/migrations/<timestamp>_<slug>.sql\n 2. Test locally: supabase db reset --local\n 3. Push to develop — CI applies the migration to staging automatically\n 4. For production: follow the create-release SOP\n\nUse the bypass marker only for legitimate hotfixes where the PR cycle\nis too slow.\n",
10001000
"tests": [
10011001
{
10021002
"name": "warns-on-psql-supabase-remote",
10031003
"input": {
10041004
"tool_input": {
1005-
"command": "psql postgresql://user:pass@db.xepaexmpawtpqtilhcpw.supabase.co:5432/postgres -c 'SELECT 1'"
1005+
"command": "psql postgresql://user:pass@db.example1.supabase.co:5432/postgres -c 'SELECT 1'"
10061006
}
10071007
},
10081008
"expected_exit": 0,
@@ -1012,7 +1012,27 @@
10121012
"name": "warns-on-psql-different-supabase-host",
10131013
"input": {
10141014
"tool_input": {
1015-
"command": "psql -h db.ukwovawzehnebuoowcec.supabase.co -d postgres -c 'CREATE INDEX foo_idx ON foo (id)'"
1015+
"command": "psql -h db.example2.supabase.co -d postgres -c 'CREATE INDEX foo_idx ON foo (id)'"
1016+
}
1017+
},
1018+
"expected_exit": 0,
1019+
"expected_stderr_contains": "warn-psql-against-supabase-remote"
1020+
},
1021+
{
1022+
"name": "warns-on-pg-dump-supabase-remote",
1023+
"input": {
1024+
"tool_input": {
1025+
"command": "pg_dump -h db.example1.supabase.co -d postgres -t public.foo > foo.sql"
1026+
}
1027+
},
1028+
"expected_exit": 0,
1029+
"expected_stderr_contains": "warn-psql-against-supabase-remote"
1030+
},
1031+
{
1032+
"name": "warns-on-pg-restore-supabase-remote",
1033+
"input": {
1034+
"tool_input": {
1035+
"command": "pg_restore -h db.example1.supabase.co -d postgres backup.dump"
10161036
}
10171037
},
10181038
"expected_exit": 0,
@@ -1031,7 +1051,7 @@
10311051
"name": "allows-non-psql-supabase-references",
10321052
"input": {
10331053
"tool_input": {
1034-
"command": "curl https://xepaexmpawtpqtilhcpw.supabase.co/rest/v1/foo"
1054+
"command": "curl https://example1.supabase.co/rest/v1/foo"
10351055
}
10361056
},
10371057
"expected_exit": 0
@@ -1040,7 +1060,7 @@
10401060
"name": "allows-bypass-marker",
10411061
"input": {
10421062
"tool_input": {
1043-
"command": "psql -h db.xepaexmpawtpqtilhcpw.supabase.co -c 'SELECT 1' # hook-bypass: psql-supabase-remote"
1063+
"command": "psql -h db.example1.supabase.co -c 'SELECT 1' # hook-bypass: psql-supabase-remote"
10441064
}
10451065
},
10461066
"expected_exit": 0
@@ -1049,14 +1069,15 @@
10491069
},
10501070
{
10511071
"id": "pr-create-with-migrations-needs-deploy-note",
1052-
"description": "Warn when gh pr create body does not mention migrations (caller should verify the PR has no migration files)",
1072+
"description": "Warn when gh pr create body (inline --body) does not mention migrations (caller should verify the PR has no migration files)",
10531073
"applies_to": [
10541074
"Bash"
10551075
],
10561076
"match": [
10571077
{
10581078
"field": "command",
1059-
"pattern": "gh[[:space:]]+pr[[:space:]]+create.*--body",
1079+
"pattern": "gh[[:space:]]+pr[[:space:]]+create.*--body([[:space:]]|=|$)",
1080+
"not_pattern": "--body-file",
10601081
"flags": "i"
10611082
},
10621083
{
@@ -1068,7 +1089,7 @@
10681089
"action": "warn",
10691090
"bypass_marker": "pr-create-no-migration-note",
10701091
"memory_ref": "feedback_scripts_not_db.md",
1071-
"message": "WARNING: gh pr create body does not mention migrations.\n\nIf your PR modifies files under supabase/migrations/, the body MUST\ninclude a \"Migration deployment\" section that lists:\n 1. Which envs need supabase db push (staging auto via CI; prod via release)\n 2. Backfill order if the migrations depend on existing data\n 3. Post-merge verification steps\n\nThis rule does NOT inspect the diff — it only reads the command. Use\nthe bypass marker if your PR has no migrations.\n",
1092+
"message": "WARNING: gh pr create body does not mention migrations.\n\nIf your PR modifies files under supabase/migrations/, the body MUST\ninclude a \"Migration deployment\" section that lists:\n 1. Which envs need supabase db push (staging auto via CI; prod via release)\n 2. Backfill order if the migrations depend on existing data\n 3. Post-merge verification steps\n\nThis rule does NOT inspect the diff — it only reads the command, and it\nintentionally skips --body-file invocations because it cannot read the\nreferenced file. Use the bypass marker if your PR has no migrations.\n",
10721093
"tests": [
10731094
{
10741095
"name": "warns-on-pr-create-body-without-migration-mention",
@@ -1098,6 +1119,15 @@
10981119
},
10991120
"expected_exit": 0
11001121
},
1122+
{
1123+
"name": "allows-pr-create-with-body-file",
1124+
"input": {
1125+
"tool_input": {
1126+
"command": "gh pr create --base develop --title \"feat: foo\" --body-file ./pr-body.md"
1127+
}
1128+
},
1129+
"expected_exit": 0
1130+
},
11011131
{
11021132
"name": "allows-bypass-marker",
11031133
"input": {
@@ -1111,26 +1141,26 @@
11111141
},
11121142
{
11131143
"id": "block-supabase-db-push-prod",
1114-
"description": "Block supabase db push targeting the PRODUCTION project ref without an explicit bypass marker",
1144+
"description": "Block supabase db push that targets the PRODUCTION project ref or uses --linked (the linked project may transparently be production)",
11151145
"applies_to": [
11161146
"Bash"
11171147
],
11181148
"match": [
11191149
{
11201150
"field": "command",
1121-
"pattern": "supabase[[:space:]]+db[[:space:]]+push.*ukwovawzehnebuoowcec"
1151+
"pattern": "supabase[[:space:]]+db[[:space:]]+push.*(__PROD_SUPABASE_REF__|--linked\\b)"
11221152
}
11231153
],
11241154
"action": "block",
11251155
"bypass_marker": "prod-db-push-approved",
11261156
"memory_ref": "feedback_never_touch_prod_without_approval.md",
1127-
"message": "BLOCKED: supabase db push targeting the PRODUCTION project ref\n(ukwovawzehnebuoowcec).\n\nNEVER run against production without explicit user approval — see\nfeedback_never_touch_prod_without_approval.md.\n\nCorrect workflow:\n 1. Merge to main\n 2. William runs the release per the SOP at\n https://github.com/DojoCodingLabs/dojo-os/blob/develop/.claude/commands/create-release.md\n\nUse the bypass marker ONLY when the user has verbally approved the\noperation.\n",
1157+
"message": "BLOCKED: supabase db push targeting either the PRODUCTION project ref\nor the currently-linked project (which may be production).\n\nNEVER run against production without explicit user approval — see\nfeedback_never_touch_prod_without_approval.md.\n\nCorrect workflow:\n 1. Merge to main\n 2. The release owner runs the create-release SOP that ships\n migrations to production through CI, not from a developer machine.\n\nUse the bypass marker ONLY when the user has verbally approved the\noperation. For --linked, double-check the active link first:\n supabase status --output json | jq -r '.project_ref'\n\nMarketplace consumers: the production project ref is configured per\ninstall via .private/substitutions.json (PROD_SUPABASE_REF token). See\nhooks/rules/README.md for the substitution workflow.\n",
11281158
"tests": [
11291159
{
11301160
"name": "blocks-supabase-db-push-prod",
11311161
"input": {
11321162
"tool_input": {
1133-
"command": "supabase db push --db-url postgresql://postgres:pwd@db.ukwovawzehnebuoowcec.supabase.co:5432/postgres"
1163+
"command": "supabase db push --db-url postgresql://postgres:pwd@db.__PROD_SUPABASE_REF__.supabase.co:5432/postgres"
11341164
}
11351165
},
11361166
"expected_exit": 2
@@ -1139,7 +1169,25 @@
11391169
"name": "blocks-supabase-db-push-prod-with-project-ref-flag",
11401170
"input": {
11411171
"tool_input": {
1142-
"command": "supabase db push --project-ref ukwovawzehnebuoowcec"
1172+
"command": "supabase db push --project-ref __PROD_SUPABASE_REF__"
1173+
}
1174+
},
1175+
"expected_exit": 2
1176+
},
1177+
{
1178+
"name": "blocks-supabase-db-push-linked",
1179+
"input": {
1180+
"tool_input": {
1181+
"command": "supabase db push --linked"
1182+
}
1183+
},
1184+
"expected_exit": 2
1185+
},
1186+
{
1187+
"name": "blocks-supabase-db-push-linked-with-extra-flags",
1188+
"input": {
1189+
"tool_input": {
1190+
"command": "supabase db push --linked --include-all"
11431191
}
11441192
},
11451193
"expected_exit": 2
@@ -1148,7 +1196,7 @@
11481196
"name": "allows-supabase-db-push-staging",
11491197
"input": {
11501198
"tool_input": {
1151-
"command": "supabase db push --project-ref xepaexmpawtpqtilhcpw"
1199+
"command": "supabase db push --project-ref __STAGING_SUPABASE_REF__"
11521200
}
11531201
},
11541202
"expected_exit": 0
@@ -1163,10 +1211,19 @@
11631211
"expected_exit": 0
11641212
},
11651213
{
1166-
"name": "allows-bypass-marker",
1214+
"name": "allows-bypass-marker-prod-ref",
1215+
"input": {
1216+
"tool_input": {
1217+
"command": "supabase db push --project-ref __PROD_SUPABASE_REF__ # hook-bypass: prod-db-push-approved"
1218+
}
1219+
},
1220+
"expected_exit": 0
1221+
},
1222+
{
1223+
"name": "allows-bypass-marker-linked",
11671224
"input": {
11681225
"tool_input": {
1169-
"command": "supabase db push --project-ref ukwovawzehnebuoowcec # hook-bypass: prod-db-push-approved"
1226+
"command": "supabase db push --linked # hook-bypass: prod-db-push-approved"
11701227
}
11711228
},
11721229
"expected_exit": 0

0 commit comments

Comments
 (0)