feat(rules): add 4 migration-discipline PreToolUse rules#15
Conversation
Adds a new rule family — "Database / migration discipline" — that prevents
the class of drift seen when a teammate's PR shipped supabase/migrations
files that did not auto-run on staging, and the manual SQL Editor
workaround introduced silent skew between the migrations directory and
other devs' local databases.
Layered defense (4 new rules, 21 new tests):
1. schema-sql-outside-migrations (BLOCK on Edit/Write/MultiEdit)
Blocks ALTER/CREATE/DROP/GRANT/REVOKE/CREATE POLICY in any .sql
file outside supabase/migrations/, supabase/tests/, supabase/seed.
2. warn-psql-against-supabase-remote (WARN on Bash)
Nudges away from psql/SQL Editor execution against any *.supabase.co
host — that workflow is the primary source of drift vs versioned
migrations.
3. pr-create-with-migrations-needs-deploy-note (WARN on Bash)
Reminds the author to describe migration deployment in the PR body
when calling gh pr create. Pure heuristic — does not inspect the
diff; bypass marker available for migration-free PRs.
4. block-supabase-db-push-prod (BLOCK on Bash)
Hard stop on supabase db push aimed at the PRODUCTION project ref
(ukwovawzehnebuoowcec). Only the create-release SOP may apply
migrations to prod.
Test count: 59 → 80 (21 new tests, all 80 pass)
Version: 1.6.0 → 1.7.0 (minor bump per semver — feature add)
Files:
- hooks/rules/rules.yaml — 4 new rule entries with full test arrays
- hooks/rules/rules.json — regenerated via npm run build-rules
- hooks/rules/README.md — new "Database / migration discipline" family
- package.json, .claude-plugin/{plugin,marketplace}.json — version bump
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@greptile review |
Greptile SummaryThis PR introduces a "Database / migration discipline" rule family (4 new
Confidence Score: 5/5Safe to merge — the new rules are additive enforcement hooks with documented bypass markers; no runtime code path is altered and the substitution mechanism degrades gracefully to an inert rule when unconfigured. All four rules carry full positive/negative/bypass test coverage and the only findings are minor pattern-coverage gaps (CREATE INDEX omission, loosely-anchored seed not_pattern) and a documentation clarification on regex safety for substitution values — none affect correctness of the existing test suite or runtime behavior. hooks/rules/rules.yaml — the schema-sql-outside-migrations content pattern and the seed not_pattern anchor are worth tightening before the next rule iteration.
|
| Filename | Overview |
|---|---|
| hooks/rules/rules.yaml | Adds 4 new migration-discipline rules with good test coverage; minor gaps in the schema-sql-outside-migrations pattern (missing CREATE/DROP INDEX, loosely-anchored seed not_pattern). |
| hooks/rules/rules.json | Generated artifact matching rules.yaml; placeholder tokens remain verbatim, which is the correct documented inert-state behavior for marketplace consumers. |
| scripts/build-rules.mjs | Adds a well-structured opt-in substitution layer; token validation is solid, but the 'literal string' comment slightly overstates safety since values are later consumed as ERE regex patterns. |
| hooks/rules/README.md | Documentation for the substitution mechanism and new rule family is clear and accurate. |
| .claude-plugin/marketplace.json | Version bumped from 1.6.0 to 1.7.0 consistently in both top-level and nested plugin entry. |
| .claude-plugin/plugin.json | Version bump only, no functional changes. |
| package.json | Version bump only, no functional changes. |
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
hooks/rules/rules.yaml:714
The `not_pattern` for the seed directory is not path-anchored. Because `supabase/seed` is a prefix match with no trailing delimiter, it would also exempt any file whose path contains that substring — for example, `supabase/seedlings/setup.sql` or `supabase/seed_historical/fix.sql`. A more precise pattern would use a trailing `/` or `\.sql` to pin the match to the actual seed file or directory.
```suggestion
not_pattern: '(supabase/migrations/|supabase/tests/|supabase/seed[./])'
```
### Issue 2 of 3
hooks/rules/rules.yaml:716
`CREATE INDEX` and `DROP INDEX` are DDL statements that modify the database schema and should live in versioned migration files, just like `ALTER TABLE` or `CREATE TABLE`. They are not currently in the content pattern, so a developer who writes `CREATE INDEX` in a script outside `supabase/migrations/` would bypass this rule entirely.
```suggestion
pattern: '\b(ALTER TABLE|CREATE TABLE|DROP TABLE|CREATE INDEX|DROP INDEX|CREATE POLICY|DROP POLICY|GRANT |REVOKE |CREATE FUNCTION|DROP FUNCTION|CREATE TYPE|ALTER TYPE)\b'
```
### Issue 3 of 3
scripts/build-rules.mjs:848-851
**Substitution values land verbatim inside regex patterns**
The comment "no risk of $-backref interpolation in the replacement" is accurate for the JS string substitution step, but the substituted value is then interpreted as a POSIX ERE regex by the hook engine at runtime. If a consumer sets `PROD_SUPABASE_REF` to a value containing regex metacharacters (`.`, `+`, `(`, `|`, etc.), those characters will be treated as operators in the compiled pattern, not as literals. Supabase project refs are typically alphanumeric so this is unlikely in practice, but the comment and the README could note that token values must be regex-safe (alphanumeric and hyphens only for project refs).
Reviews (2): Last reviewed commit: "fix(rules): address Greptile review on m..." | Re-trigger Greptile
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>
|
Round 2: addressed all 4 Greptile findings. See commit e930eba.
Test count: 80 -> 86 (all pass locally). @greptile review |
…ipts_not_db.md) (#16) * feat(rules): add warn-curl-mutating-supabase-rest (close feedback_scripts_not_db.md) Adds a single warn rule to the migration-discipline family that catches the last documented escape hatch from the "no ad-hoc DB writes" memory: direct curl mutations (POST / PATCH / PUT / DELETE) against the Supabase PostgREST endpoint. The neighbor warn-psql-against-supabase-remote already covers the psql / pg_dump / pg_restore vector; warn-curl-mutating-supabase-rest closes the final gap. Match is anchored on the explicit `-X <METHOD>` form and narrowed to `.supabase.co/rest/v1/` so Auth, Storage, and Edge Function endpoints are not flagged. Test fixtures use sanitized hosts (example.supabase.co) — same convention the migration-discipline rules adopted in PR #15 round-2. - 19 rules (was 18) - 93 / 93 tests pass (was 86, +7) - 1.7.0 -> 1.8.0 in package.json, plugin.json, marketplace.json (top-level + nested plugin entry) - README family entry extended with the new rule * fix(rules): close 3 Greptile findings on warn-curl-mutating-supabase-rest 1. Cover the no-space `-XPOST` shorthand: relax `[[:space:]]+` to `[[:space:]]*`. Adds `warns-curl-xpost-no-space` test. 2. Cover URL-before-flag ordering: extend the pattern to a two-clause alternation that matches whether `-X METHOD` precedes or follows the `.supabase.co/rest/v1/` segment. Adds `warns-curl-url-before-flag` test. 3. Rename the four `blocks-*` test names to `warns-*` — the rule action is `warn` (expected_exit 0), so `blocks-*` was misleading. Tests: 93 -> 95 passing (+2 coverage tests). * fix(rules): close round-2 Greptile findings on warn-curl-mutating-supabase-rest 1. **Implicit POST via -d/--data is now caught** — extend the pattern alternation to fire when `-d`, `--data`, `--data-raw`, or `--data-binary` are present without `-X`. Curl auto-promotes to POST in that case, so a one-liner like curl -d '{"id":1}' https://<project>.supabase.co/rest/v1/profiles would otherwise pass silently. Three new tests cover -d before URL, --data-raw before URL, and URL before -d. 2. **design.md test list refreshed** — replace the stale `blocks-*` names with the actual `warns-*` names and list all 12 tests after rounds 1-2. Drop the old "blocks- naming for symmetry" rationale that was made obsolete by round 1. 3. **Pattern comment refactored** — document the three vectors the alternation covers (explicit -X, URL ordering, implicit POST via data flags). Tests: 95 -> 98 passing. * fix(rules): close round-3 Greptile findings on warn-curl-mutating-supabase-rest 1. **--data-urlencode now caught** — extend the `--data(...)` alternation to include `-urlencode[[:space:]]` and `-urlencode=`. Curl auto-promotes to POST for `--data-urlencode` the same way it does for `-d`/`--data`/`--data-raw`/`--data-binary`, so a one-liner like curl --data-urlencode 'name=Alice' https://<project>.supabase.co/rest/v1/profiles would otherwise pass silently. Adds `warns-curl-implicit-post-via-data-urlencode` test. 2. **design.md Pattern section refreshed** — replace the original single-clause regex with the actual 4-clause alternation, documenting each vector (explicit -X, URL ordering, implicit POST via data flags, implicit POST via --data-urlencode). The accepted false-negatives subsection now also mentions `-G` (which overrides auto-promotion). Tests: 98 -> 99 passing. Greptile already at 5/5 on round 3 commit ac5b931 — these are belt-and-suspenders fixes for the two nits Greptile flagged in its 5/5 review.
Summary
Adds a new rule family — Database / migration discipline — that prevents the class of drift exposed when a teammate's PR shipped
supabase/migrations/files that did not auto-run on staging. The manual SQL Editor workaround introduced silent skew between the migrations directory and other devs' local databases. Future devs had no record that the migrations had ever been applied outside of timestamped files.Pain pattern
supabase/migrations/.*.supabase.co.supabase db reset --localmay hit conflicts, or worse, ship a follow-up migration that re-applies the same DDL.The 4 new rules
schema-sql-outside-migrationsEdit,Write,MultiEditALTER/CREATE/DROP/GRANT/REVOKE/CREATE POLICYin any.sqlfile outsidesupabase/migrations/,supabase/tests/, orsupabase/seed.warn-psql-against-supabase-remoteBashpsql/ SQL-Editor-style direct execution against any*.supabase.cohost.pr-create-with-migrations-needs-deploy-noteBashgh pr create --body. Heuristic only — does not inspect the diff. Bypass marker for migration-free PRs.block-supabase-db-push-prodBashsupabase db pushaimed at the PRODUCTION project ref (ukwovawzehnebuoowcec). Only thecreate-releaseSOP may apply migrations to prod.Each rule ships with a kebab-case
bypass_markerfor documented exceptions and a populatedtestsarray (full coverage of positive matches, negative matches, and bypass behavior).Test count
Version bump
1.6.0 → 1.7.0Per semver, adding new enforcement rules is a feature add → minor bump. Bumped in:
package.json.claude-plugin/plugin.json.claude-plugin/marketplace.json(top-level + nested plugin entry)Test plan
npm run build-rules— regeneratesrules.json(14 → 18 rules)npm run test-hooks— 80 / 80 passfeat/migration-discipline-hooks(Blacksmith runner) —npm run test-hooks+git diff hooks/rules/rules.jsoncleanNotes for reviewers
[[:space:]]+not\s) consistent with the existing manifest.block-supabase-db-push-prodis layered on top of the existingdestructive-db-opsrule. Both fire for a proddb push; the new rule has a narrower bypass marker (prod-db-push-approved) to make the prod-specific approval explicit.pr-create-with-migrations-needs-deploy-noteis intentionally a heuristic — it cannot read the PR diff. Bypass marker is the escape hatch for migration-free PRs.🤖 Generated with Claude Code