diff --git a/.github/workflows/review.yml b/.github/workflows/review.yml index 4307ecd..bc63c02 100644 --- a/.github/workflows/review.yml +++ b/.github/workflows/review.yml @@ -38,13 +38,39 @@ permissions: id-token: write concurrency: - group: review-${{ github.event.pull_request.number }} + # The fallback is not decoration. `github.event.pull_request.number` is EMPTY on any event + # that is not a pull request, and the caller template triggers on `[pull_request, push]`, so + # every push run collapsed into one shared group key of `review-` and cancelled the previous + # one. Harmless only for as long as this job stays skipped on push — which is a property of + # the `if` below, not of the group, and the two are not written down together anywhere else. + # A concurrency key that silently means "all of them" is the same shape as a check that + # silently means "nothing". + group: review-${{ github.event.pull_request.number || github.ref }} cancel-in-progress: true jobs: review: name: Draft review - if: github.event.pull_request.draft == true + # DRAFT, AND NOT FROM A FORK. The second half is about what a secret is worth on each + # trigger, and it was raised as an exfiltration risk — "a PR can rewrite the caller job and + # read CLAUDE_CODE_OAUTH_TOKEN". The load-bearing distinction is the trigger word: the + # caller runs on `pull_request`, NOT `pull_request_target`. On `pull_request` a fork's PR + # gets no repository secrets at all, so there is nothing there to read; and a same-repo + # branch is pushed by someone who already has write access, which is not a boundary this + # trigger was ever holding. So the risk as stated does not hold here. + # + # What DOES hold is duller and still worth the line: on a fork PR the secret expression + # resolves to empty while `review.yml` declares it `required: true`, so the reviewer is red + # on every fork PR for a reason having nothing to do with the code — the same shape as the + # 403 the `permissions:` note in caller-template.yml exists to prevent. Skipping is the + # honest outcome: as with a ready PR, `skipped` is an ABSENCE and is not claimed as a pass. + # + # This is reasoned from GitHub's documented trigger semantics, not measured: a fork PR + # against this repository is not something this branch can stage. Stated so the next reader + # knows which kind of claim it is. + if: >- + github.event.pull_request.draft == true + && github.event.pull_request.head.repo.fork == false runs-on: ubuntu-latest timeout-minutes: 20 steps: @@ -132,7 +158,12 @@ jobs: # finishing. If the count file is missing or unparseable the run says UNKNOWN and goes RED: # there is no third colour, and a reviewer that died is not a clean bill of health. - name: Post the tally - if: always() + # success() || failure(), NOT always(). With cancel-in-progress on, a superseded run is + # CANCELLED, and always() runs this step there too — so every push to a draft PR left a + # spurious "UNKNOWN — the reviewer did not report a count" comment from the run that was + # deliberately killed. The tally must be unmissable when the reviewer ran; it must not + # narrate a run that was cancelled on purpose. + if: success() || failure() env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} PR: ${{ github.event.pull_request.number }} diff --git a/CITATION.cff b/CITATION.cff index 2d7a3d5..97f9ad0 100644 --- a/CITATION.cff +++ b/CITATION.cff @@ -10,7 +10,7 @@ authors: repository-code: "https://github.com/chiibitsu/gates" url: "https://chiibitsu.com" license: MIT -version: "1.2.0" +version: "1.2.1" date-released: "2026-09-08" keywords: - ci diff --git a/README.md b/README.md index a953300..f6650a8 100644 --- a/README.md +++ b/README.md @@ -137,17 +137,42 @@ sends someone hunting for a bug in their code rather than a blind spot in the ga ## How to call it +This snippet carries `` rather than a real hash, and that is the +one place in this repository where a placeholder is right: it showed `25a1ca1` / v1.0.3 for +three releases after that pin stopped being current, and a stale real SHA is copied without +hesitation while a placeholder cannot be. `caller-template.yml` ships a real SHA because it has +to run; this prose does not. + Copy `caller-template.yml` into your repo as `.github/workflows/gates.yml`. It ships a real -SHA rather than a placeholder, and that SHA is the **previous** release — replace both -occurrences with the one you want from the Releases table below: +SHA rather than a placeholder, and that SHA is the **previous** release. Replace **all three** +occurrences with the one you want from the Releases table below — the `gates:` `uses:` ref, +the `gates_ref:` under it, and the `review:` `uses:` ref. The first two must stay identical to +each other: a reusable workflow cannot discover its own commit, so the ref it runs from has to +be handed to it, and a mismatch runs one release's workflow over another release's gates. This +sentence said "both occurrences" over a snippet containing three, which is a count narrower +than the thing it describes — the defect this repository exists to catch, in its own +instructions. ```yaml on: [pull_request, push] jobs: gates: - uses: chiibitsu/gates/.github/workflows/gates.yml@25a1ca1f7ad7875138dd943657a5f7f1a3aa1809 # v1.0.3 + uses: chiibitsu/gates/.github/workflows/gates.yml@ # v1.2.1 with: - gates_ref: 25a1ca1f7ad7875138dd943657a5f7f1a3aa1809 # v1.0.3 + gates_ref: # v1.2.1 + + # The draft-stage reviewer. Needs a repository secret CLAUDE_CODE_OAUTH_TOKEN, and the + # permissions block because a called workflow can only narrow the caller's token, never + # widen it — without it the tally step 403s on a read-only default. + review: + permissions: + contents: read + pull-requests: write + issues: read + id-token: write + uses: chiibitsu/gates/.github/workflows/review.yml@ # v1.2.1 + secrets: + CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} ``` Pin a SHA, not a tag — that is the same rule `gates/actions-sha-pinned.sh` enforces on @@ -233,7 +258,7 @@ Five live in the repo being checked. The sixth belongs to the template, not here ## Releases -The Commit column names a **tag** for the current release and a SHA for superseded ones, and that asymmetry is forced: the table lives in the commit being tagged, so it cannot contain that commit's own hash. Resolve the tag — `git rev-list -n1 v1.2.0` — and pin the SHA you get. Pin a SHA, never a tag: a tag is a movable name, and this table exists because names have been wrong here before. +The Commit column names a **tag** for the current release and a SHA for superseded ones, and that asymmetry is forced: the table lives in the commit being tagged, so it cannot contain that commit's own hash. Resolve the tag — `git rev-list -n1 v1.2.1` — and pin the SHA you get. Pin a SHA, never a tag: a tag is a movable name, and this table exists because names have been wrong here before. That rule applies to everything this repo pins, not only to itself, and the reviewer's action is the measured case. On 2026-09-08, `anthropics/claude-code-action` resolved as: @@ -248,8 +273,9 @@ A release's `CITATION.cff` names its own version — that is the part that must | Version | Commit | Use it? | |---|---|---| -| **v1.2.0** | tag `v1.2.0` — resolve with `git rev-list -n1 v1.2.0`, or read it off the release page | **Yes — use this one.** Adds the draft-stage reviewer: `REVIEW.md`, `.github/workflows/review.yml`, and `fixtures/review/bad/`. **The gates are byte-identical to v1.1.0** — nothing that blocks changed, and a consumer that adopts this without wiring the reviewer gets exactly v1.1.0's behaviour. Carries v1.0.2's known false green in `actions-sha-pinned.sh`, described below. | -| v1.1.0 | `7832ea6` | **Usable, and superseded by v1.2.0** — same six gates, no reviewer. Added `service-role.sh`, the UNKNOWN outcome, and `gates/MANIFEST.txt` with a both-directions check against the `gates/` directory. Adopting it can turn a consumer red on code that was green under v1.0.4, because `service-role.sh` did not exist to check it; that is a finding, not a regression. | +| **v1.2.1** | tag `v1.2.1` — resolve with `git rev-list -n1 v1.2.1`, or read it off the release page | **Yes — use this one.** Fixes three defects in `service-role.sh` that v1.1.0 and v1.2.0 both shipped, each reproduced before the fix and each carrying a fixture: an ordinary circular import **did not terminate** (a hang, which reports nothing at all); one file reached by two spellings was counted twice, printing two findings each claiming "1 request-path module(s)"; and a multi-line `await import(` was a **false green** — `ok`, exit 0, over a page reaching `SUPABASE_SERVICE_ROLE_KEY`. Also fixes a concurrency key in `review.yml` that collapsed on non-PR events. Carries v1.0.2's known false green in `actions-sha-pinned.sh`, described below. | +| v1.2.0 | `44f6125` | **Do not use.** Its `service-role.sh` is byte-identical to v1.1.0's and carries all three defects listed under v1.2.1 — including a false green on a multi-line dynamic import and a walk that does not terminate on a circular one. The reviewer it adds is sound; the gate underneath it is not. Move to v1.2.1. | +| v1.1.0 | `7832ea6` | **Do not use** — same three `service-role.sh` defects as v1.2.0, described in the v1.2.1 row. Six gates, no reviewer. Added `service-role.sh`, the UNKNOWN outcome, and `gates/MANIFEST.txt` with a both-directions check against the `gates/` directory. Adopting it can turn a consumer red on code that was green under v1.0.4, because `service-role.sh` did not exist to check it; that is a finding, not a regression. | | v1.0.4 | `e075a93` | **Usable, and superseded by v1.1.0** — five gates instead of six, so nothing checks whether the request path can reach the service-role secret. Its `CITATION.cff` names the version on its tag, which v1.0.0 and v1.0.1 also did and v1.0.2 and v1.0.3 did not. Gates byte-identical to v1.0.2 and v1.0.3, so it carries their one known false green, described below. | | v1.0.3 | `25a1ca1` | **Do not cite.** Its gates are correct and identical to v1.0.2's, so a pin at this SHA works. But its `CITATION.cff` says `1.0.2` and its caller template says `v1.0.2` — this tag reproduces the exact defect it was cut to fix. The cause is structural and is the useful part: the correction to a version's metadata is *made by* the pull request, so it lands in a commit **after** the one being tagged. Tagging the merged head of the PR that reports the previous version is guaranteed to be one behind. A release has to declare its own version **before** it is tagged, which is what v1.0.4 does. | | v1.0.2 | `c3e3f49` | **Usable, and superseded by v1.0.4** — same gates, wrong version metadata inside the tag. Everything six review rounds found in v1.0.0 and v1.0.1 is fixed here, each fix carrying the minimal fixture that proves the shape is still caught. It has **one known false green**, reproducible: `steps: [{uses: a/b@main}, {uses: c/d@v1}]` reports ok. The rule that produces it is stated once, in Known gaps below, and deliberately not paraphrased here — a first draft of this row paraphrased it and got the rule wrong in a different way than the gap section did, which is how two statements of one fact always end. Treat this release as a first line, never as the only one. An earlier draft of this row claimed no known false green while the gap below already described one — the claim was wrong, and it is corrected here rather than quietly dropped. | @@ -276,15 +302,110 @@ header, which is the honest account of what that scanner does not do. here and is called from consumer repositories; wiring a self-call is a separate change, so until then the fixture procedure is the only thing that exercises it. Stated because a reviewer nobody has run is a reviewer nobody has seen fail. -- **The service-role gate reads imports with a regex, not a parser.** It follows literal - `from`, `import` and `require` specifiers, resolving `./`, `../`, absolute paths, and - **every alias declared in tsconfig `compilerOptions.paths`** — not just `@/*`. A specifier - matching no declared alias is treated as a published package; a specifier that matches one - and resolves to no file is UNKNOWN, never skipped. If the `paths` object is present and no - alias can be parsed out of it, that is UNKNOWN too. A specifier that appears inside a - comment or a string is - followed as though it were real — over-inclusive, which costs a false red rather than a - false green. What it cannot follow at all it calls UNKNOWN. +- **The migrations-lint gate tokenises SQL; it does not match it.** Statements are scanned + once — quoted identifiers with their `""` escapes, any schema or database qualifier, + `unlogged`/`temp`/`temporary`, `IF [NOT] EXISTS`, `ALTER TABLE ONLY`, a statement spanning + lines — and the created and RLS-enabled **(schema, table) pairs are compared as strings**. + So RLS on `archive.orders` does not satisfy a `create table public.orders`; an unqualified + name normalises to `public`; and a quoted identifier keeps its case, because PostgreSQL + folds `Orders` to `orders` but keeps `"Orders"` distinct — Prisma and Drizzle emit the + quoted PascalCase form. Three successive regex versions each traded one error for another + here; the pair comparison leaves no interpolated pattern to be wider than the name it was + given. A `'…'` string and a `$$`/`$tag$` dollar-quoted body are both **data** to the scanner. + Reading them named tables nobody created: `create function f() … as $$ … create table + public.tmp … $$` was a violation naming `public.tmp`, which does not exist at definition + time and is created only when the function runs. But a string or body containing a + `create … table` **sequence** is UNKNOWN rather than passed over, because `execute` and + `DO $$` do run — UNKNOWN is still red, and it does not claim a violation the gate has not + established. Sequence, not two words anywhere: Sequence with **at most two modifier words**, not two words + anywhere and not any number: `'created three tables last week'` and `'To create a new + monthly revenue table, run the report'` are prose and pass, while `'CREATE UNLOGGED TABLE …'` + does not. The bound was once written `{0,2}`, which **mawk miscompiles** to zero repetitions + when the group begins with a `+`-quantified bracket, so every modifier form passed silently; + replacing it with `*` fixed that and made the allowance unbounded, so English prose blocked + a compliant migration. It is two optional groups now — the bound was always the point, only + its spelling was wrong, twice. Comments are recognised by the same scanner, not stripped by an + earlier stage: a stage that cannot see strings took `values ('x /* y')` for the start of a + block comment and deleted every line to the next `*/`, hiding a whole `create table` — + verified against PostgreSQL 16 as a real table left with RLS off — and the same swallow + turned a compliant migration containing `values ('/api/*')` red with a message naming a + string that does not exist. What it genuinely cannot read, as silent passes: a name + assembled at runtime (`execute format('create table %I …')` or string concatenation) and + `select … into`. **An unterminated quoted identifier, string, block comment or dollar-quoted + body is UNKNOWN, not a pass** — a scanner that lost sync read everything after it as + something it is not. A `"` inside an ordinary string literal (an inch mark in + `values ('24" monitor')`) is *not* one of those: it is string data, and reads as such. + It was not always — before the scanner had a string state that one byte opened a quoted + identifier that hid every statement after it — which is why the case is named here rather + than left to be rediscovered. A quoted identifier may contain a tab or a **newline**; those are + escaped in the scanner's output and compared escaped, so a name containing one no longer + shifts the records after it — that shift once reported a single violation naming a table + that does not exist while silently dropping two real ones. The escaped form is what the + message prints, so `public."weird"` reads as `public.we\nird`. +- **The service-role gate tokenises JavaScript; it does not match it.** Strings, template + literals, line and block comments and regex literals are recognised as what they are, and a + specifier is emitted only from a real `from`/`import`/`require` position. This replaced a + `grep -o` extractor whose non-overlapping window let a string ending in the word `from` + consume the following import into one invented span — a defect with no correct filter, + because reporting the span was a blocking UNKNOWN on ordinary source (`Array.from(",")`, a + regex literal, `{ note: "Imported from " }`) and dropping it lost a real import that had + been swallowed. Both were measured, in three consecutive review rounds, before the + extractor itself was replaced. A multi-line `await import(` is followed; a `//` comment no + longer runs into the code after it; `import(/* webpackChunkName */ "./x")` is read as the + literal import it is. **A template literal or block comment still open at end of file is + UNKNOWN**, because a scanner that lost sync read the rest of the file as something it is + not — one stray backtick in JSX text otherwise consumed everything below it, and a + `require("@/lib/admin")` under it reported `ok`. What it still cannot read is **JSX text**: + `

Copied from "a" to "b"

` puts `from` before a quote and nothing short of a JSX parser + can tell that from an import, so a candidate carrying `<`, `>`, `{` or `}` is dropped as + text. Node subpath imports (`#internal/db`) are UNKNOWN: they resolve through `package.json` + `imports`, which this gate does not read. +- **Every `grep` over file content passes `-a`**, in all six shell gates and in `lib.sh`'s + shared `tgrep`. Without it, one byte that is invalid in the ambient locale makes GNU grep + declare a file binary, print nothing and **exit 0** with a note on stderr — which a gate + reads as "not found". Measured: a module holding `SUPABASE_SERVICE_ROLE_KEY` plus a single + NUL byte, reached from a page, reported `ok`, exit 0; the same byte hid a `NEXT_PUBLIC_` + service-key name from `nextjs-env` and an unpinned `uses:` ref from `actions-sha-pinned`; + and a migration creating a table whose quoted name held a LATIN1 byte had that table dropped + from the comparison entirely while the clean table beside it was still reported, so the loss + looked like a smaller violation count rather than a missing check. One byte must not decide + whether a gate can see a secret. + + The exit status is worth stating exactly, because an earlier version of this bullet had it + backwards: grep exits **0**, not non-zero. Any guard written around a non-zero status — + `service-role` has one — would not have caught this, and did not. +- **Every scanner here is line-incremental, and the comparison is not a shell loop.** The + first version accumulated each file with `buf = buf $0 "\n"`, which mawk reallocates and + copies every line: 12.1s on a 1.1MB generated types file against 0.11s for the greps it + replaced, and quadratic. Fixing that left the same shape one stage downstream — a nested + bash loop comparing created tables against RLS-enabled ones, 33.5s for 2000 tables — which + is now a `grep -Fxv`, 0.10s. `gates.yml` sets no `timeout-minutes`, so either would have + surfaced not as a red but as a job taking minutes: the shape of the hang this toolkit has + already shipped once. Both numbers are here because the first fix was reported as closing + the problem while half of it was still there. +- **The service-role gate resolves like TypeScript, with one deliberate divergence.** It + follows `./`, `../`, absolute paths, and **every alias declared in tsconfig + `compilerOptions.paths`** — not just `@/*` — plus a **`baseUrl` with no matching `paths` + entry**, which is Next.js's documented "Absolute Imports" and which was a false green + chosen by nothing but the spelling of the import. `baseUrl` arms on the presence of the + key, read from a **comment-stripped** copy of tsconfig.json, because a commented-out + `// "baseUrl": "."` is not configuration. A specifier matching no alias is called a + published package **only if it could be one** — tested against npm's name shape, so + `@/lib/secret` is UNKNOWN, not a dependency to skip. One that matches an alias and resolves + to no file is UNKNOWN, never skipped; a `paths` object nothing parses out of is UNKNOWN too. + It maps the **emitted** extension back to the source (`./m.mjs` → `m.mts`, `.cjs` → `.cts`, + `.js` → `.ts`/`.tsx`) as `moduleResolution: nodenext` requires. **The divergence: + implementation before declaration.** TypeScript resolves `./admin.js` to `admin.d.ts` when + both exist; this gate takes `admin.js`, because a declaration file by construction cannot + hold a secret and the question here is what code runs in the request path, not where the + types are. +- **The service-role gate does not follow tsconfig `extends`.** Aliases are read from the + repo's own `tsconfig.json` only. TypeScript does not deep-merge `paths` — measured with + tsc 5.6.3: a child that declares `paths` REPLACES the base's object entirely, so a base's + `@/*` is already dead in that tree — but a child that declares `extends` and no `paths` of + its own inherits them, and this gate cannot see them. That case is UNKNOWN, not a pass. + A `baseUrl` **inherited from a base config** is not read either, which makes alias targets + fail to resolve and go UNKNOWN: a false red, and the direction this toolkit errs in. - **The service-role gate does not look inside published packages.** A bare specifier (`react`, `@supabase/ssr`) is out of scope by definition, so a third-party module that reads `process.env.SUPABASE_SECRET_KEY` itself is invisible to it. Repo source is what it diff --git a/caller-template.yml b/caller-template.yml index bb36123..ad11b3f 100644 --- a/caller-template.yml +++ b/caller-template.yml @@ -3,6 +3,14 @@ # purpose: a reusable workflow cannot discover its own commit, so the ref you pin has to be # handed to it. Change both together. # +# READ THIS BEFORE COPYING: the SHA shipped below is v1.2.0, and the Releases table in the +# README marks v1.2.0 **do not use** — its service-role gate has a false green on a multi-line +# dynamic import and does not terminate on a circular one. The template pins the PREVIOUS +# release by construction, and the previous release is the bad one, so this is the first time +# the convention has shipped a pin the table contradicts. TAKE v1.2.1 FROM THE TABLE. The +# convention is left visible rather than quietly patched because it is the convention that +# needs a decision, not this line. +# # It does NOT name the release you are reading, and cannot: a commit cannot contain its own # SHA. What it does name is not derivable — from v1.0.2 onward it is the previous release, but # v1.0.0 and v1.0.1 pinned untagged ancestors, and v1.0.1 labelled its untagged pin with its @@ -13,27 +21,43 @@ on: [pull_request, push] jobs: gates: - uses: chiibitsu/gates/.github/workflows/gates.yml@7832ea67355f22a3e9077d061f1dd937cead905a # v1.1.0 + uses: chiibitsu/gates/.github/workflows/gates.yml@44f6125815853021fcc4c8c96f5e2305cd4c4313 # v1.2.0 with: - gates_ref: 7832ea67355f22a3e9077d061f1dd937cead905a # v1.1.0 + gates_ref: 44f6125815853021fcc4c8c96f5e2305cd4c4313 # v1.2.0 # ── The draft-stage reviewer (Tier 2, not a gate) ──────────────────────────────────────── # - # Commented, and NOT because it is a placeholder. review.yml does not exist before v1.2.0, so - # pinning it at the SHA above — the previous release, which is what this template ships — - # would fail with a file-not-found on every run. A caller line that is broken by construction - # is worse than one that says why it is off. + # ACTIVE from v1.2.1 onward. It shipped commented in v1.2.0 for a reason that has since + # stopped being true: review.yml did not exist at the release this template pins, so an + # active line would have failed file-not-found on every run. review.yml exists at v1.2.0, + # which is what the pin below now names, so the block is live. A comment kept past the + # expiry of its own reason is how a file starts lying quietly. # - # To turn it on: take the v1.2.0 SHA (or later) from the Releases table in the README, put it - # on the `uses:` line below and on both `gates` lines above, and uncomment. There is ONE SHA - # in this block — the other line is the secret mapping, and an earlier wording said "BOTH - # lines below", which would have had a careful reader overwrite the token with a commit hash. - # It needs a repository secret CLAUDE_CODE_OAUTH_TOKEN, from `claude setup-token`. + # It needs a repository secret CLAUDE_CODE_OAUTH_TOKEN, from `claude setup-token`. Without + # it the job fails loudly rather than reviewing nothing. # # It runs ONLY while the pull request is a draft. On a ready PR the job reports `skipped`, - # which is an absence, not a pass — see AGENTS.md on accounting for a check that is not there. + # which is an absence, not a pass — account for it before flipping, as AGENTS.md says. + # + # From v1.2.1 it also skips on pull requests FROM FORKS, and the pin below predates that. + # A fork PR gets no repository secrets, so the token expression is empty against a + # `required: true` declaration and the job is red for a reason having nothing to do with the + # code. Pinning the SHA on this line to v1.2.1 or later is what gets the skip; that is the + # same pin question the header above hands to the reader, named here so it is not discovered + # by a red run on someone's first outside contribution. # - # review: - # uses: chiibitsu/gates/.github/workflows/review.yml@ - # secrets: - # CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + # ONE SHA in this block, on the `uses:` line. The line under `secrets:` is a token mapping. + review: + # PERMISSIONS ARE THE CALLER'S TO GRANT. A called workflow can only narrow the token it is + # handed, never widen it, so review.yml's own `permissions:` block is a ceiling and not a + # grant. On a repository whose default workflow token is read-only, the tally step's + # `gh pr comment` gets a 403 and the job is red on every draft PR — a reviewer that fails + # for a reason having nothing to do with the code. + permissions: + contents: read + pull-requests: write + issues: read + id-token: write + uses: chiibitsu/gates/.github/workflows/review.yml@44f6125815853021fcc4c8c96f5e2305cd4c4313 # v1.2.0 + secrets: + CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} diff --git a/fixtures/migrations-lint/bad/cases/alter-table-named-if/supabase/migrations/20260110000000_orders.down.sql b/fixtures/migrations-lint/bad/cases/alter-table-named-if/supabase/migrations/20260110000000_orders.down.sql new file mode 100644 index 0000000..718c9c0 --- /dev/null +++ b/fixtures/migrations-lint/bad/cases/alter-table-named-if/supabase/migrations/20260110000000_orders.down.sql @@ -0,0 +1,2 @@ +drop table if exists public.orders; +drop table if exists public."if"; diff --git a/fixtures/migrations-lint/bad/cases/alter-table-named-if/supabase/migrations/20260110000000_orders.sql b/fixtures/migrations-lint/bad/cases/alter-table-named-if/supabase/migrations/20260110000000_orders.sql new file mode 100644 index 0000000..6392c6b --- /dev/null +++ b/fixtures/migrations-lint/bad/cases/alter-table-named-if/supabase/migrations/20260110000000_orders.sql @@ -0,0 +1,9 @@ +-- `if` and `exists` are non-reserved in PostgreSQL and are legal table names — the create +-- side already knew that. The ALTER side swallowed them wherever they appeared, so it bound +-- `enable` as the table name here, emitted no RLS record, and reported a violation on a file +-- that enables RLS correctly. The planted violation is `orders`, which genuinely has none; +-- the point of the file is that `if` must NOT also be reported. +create table if (id int); +alter table if enable row level security; + +create table public.orders (id int); diff --git a/fixtures/migrations-lint/bad/cases/comment-marker-in-a-string/supabase/migrations/20260106000000_orders.down.sql b/fixtures/migrations-lint/bad/cases/comment-marker-in-a-string/supabase/migrations/20260106000000_orders.down.sql new file mode 100644 index 0000000..344e7c9 --- /dev/null +++ b/fixtures/migrations-lint/bad/cases/comment-marker-in-a-string/supabase/migrations/20260106000000_orders.down.sql @@ -0,0 +1,2 @@ +drop table if exists public.orders; +drop table if exists public.notes; diff --git a/fixtures/migrations-lint/bad/cases/comment-marker-in-a-string/supabase/migrations/20260106000000_orders.sql b/fixtures/migrations-lint/bad/cases/comment-marker-in-a-string/supabase/migrations/20260106000000_orders.sql new file mode 100644 index 0000000..da237e6 --- /dev/null +++ b/fixtures/migrations-lint/bad/cases/comment-marker-in-a-string/supabase/migrations/20260106000000_orders.sql @@ -0,0 +1,13 @@ +-- A `/*` INSIDE a string literal. Comment stripping used to run as a separate stage with no +-- string state, so this opened a block comment that deleted every line until the `*/` in the +-- string below it — taking the create table with it. Verified against PostgreSQL 16: the file +-- applies cleanly and leaves public.orders with RLS off, while the gate reported ok, exit 0. +-- The two surviving quotes pair up, so the unterminated-string guard never fired either. +create table public.notes (body text); +alter table public.notes enable row level security; + +insert into public.notes (body) values ('x /* y'); + +create table public.orders (id int); + +insert into public.notes (body) values ('*/ z'); diff --git a/fixtures/migrations-lint/bad/cases/cross-schema-rls/supabase/migrations/20260102000000_orders.down.sql b/fixtures/migrations-lint/bad/cases/cross-schema-rls/supabase/migrations/20260102000000_orders.down.sql new file mode 100644 index 0000000..78d2ff6 --- /dev/null +++ b/fixtures/migrations-lint/bad/cases/cross-schema-rls/supabase/migrations/20260102000000_orders.down.sql @@ -0,0 +1,2 @@ +drop table if exists archive.orders; +drop table if exists public.orders; diff --git a/fixtures/migrations-lint/bad/cases/cross-schema-rls/supabase/migrations/20260102000000_orders.sql b/fixtures/migrations-lint/bad/cases/cross-schema-rls/supabase/migrations/20260102000000_orders.sql new file mode 100644 index 0000000..8f2806c --- /dev/null +++ b/fixtures/migrations-lint/bad/cases/cross-schema-rls/supabase/migrations/20260102000000_orders.sql @@ -0,0 +1,9 @@ +-- RLS ON A DIFFERENT SCHEMA'S SAME-NAMED TABLE. The create side discarded the schema +-- qualifier after reading it, so the check asked only "does SOME table called orders, in SOME +-- schema, have RLS?" while the message named one specific table. Both statements below are +-- accepted by PostgreSQL and the resulting state really does leave public.orders unprotected. +-- Two schemas holding a same-named table is an ordinary layout, not a contrivance. +create table public.orders (id uuid primary key, owner uuid); + +create table archive.orders (id uuid primary key, owner uuid); +alter table archive.orders enable row level security; diff --git a/fixtures/migrations-lint/bad/cases/multiline-create-table/supabase/migrations/20260104000000_orders.down.sql b/fixtures/migrations-lint/bad/cases/multiline-create-table/supabase/migrations/20260104000000_orders.down.sql new file mode 100644 index 0000000..94b5e0d --- /dev/null +++ b/fixtures/migrations-lint/bad/cases/multiline-create-table/supabase/migrations/20260104000000_orders.down.sql @@ -0,0 +1 @@ +drop table if exists public.orders; diff --git a/fixtures/migrations-lint/bad/cases/multiline-create-table/supabase/migrations/20260104000000_orders.sql b/fixtures/migrations-lint/bad/cases/multiline-create-table/supabase/migrations/20260104000000_orders.sql new file mode 100644 index 0000000..1270387 --- /dev/null +++ b/fixtures/migrations-lint/bad/cases/multiline-create-table/supabase/migrations/20260104000000_orders.sql @@ -0,0 +1,8 @@ +-- The table name is on the line after `create table`. grep is line-scoped, so the statement +-- was not seen at all and the file passed with no RLS anywhere in it: a silent green, which +-- is the one outcome this toolkit refuses. The tokeniser reads statements, not lines. +create table + public.orders ( + id uuid primary key, + owner uuid + ); diff --git a/fixtures/migrations-lint/bad/cases/non-utf8-identifier/supabase/migrations/20260107000000_orders.down.sql b/fixtures/migrations-lint/bad/cases/non-utf8-identifier/supabase/migrations/20260107000000_orders.down.sql new file mode 100644 index 0000000..bdadf11 --- /dev/null +++ b/fixtures/migrations-lint/bad/cases/non-utf8-identifier/supabase/migrations/20260107000000_orders.down.sql @@ -0,0 +1 @@ +drop table if exists public.clean; diff --git a/fixtures/migrations-lint/bad/cases/non-utf8-identifier/supabase/migrations/20260107000000_orders.sql b/fixtures/migrations-lint/bad/cases/non-utf8-identifier/supabase/migrations/20260107000000_orders.sql new file mode 100644 index 0000000..99079b5 --- /dev/null +++ b/fixtures/migrations-lint/bad/cases/non-utf8-identifier/supabase/migrations/20260107000000_orders.sql @@ -0,0 +1,7 @@ +-- A LATIN1 byte in a quoted identifier. `grep` without `-a` calls the records file +-- binary under an ordinary UTF-8 locale, SUPPRESSES the matching line and still exits 0, +-- so this table never reached the comparison and was never checked. The clean table +-- beside it was, which is what made the loss invisible: one violation reported where +-- two are planted. PostgreSQL 16 confirms both tables are real and both have RLS off. +create table public.clean (id int); +create table public."año" (id int); diff --git a/fixtures/migrations-lint/bad/cases/quote-inside-a-string/supabase/migrations/20260105000000_orders.down.sql b/fixtures/migrations-lint/bad/cases/quote-inside-a-string/supabase/migrations/20260105000000_orders.down.sql new file mode 100644 index 0000000..94b5e0d --- /dev/null +++ b/fixtures/migrations-lint/bad/cases/quote-inside-a-string/supabase/migrations/20260105000000_orders.down.sql @@ -0,0 +1 @@ +drop table if exists public.orders; diff --git a/fixtures/migrations-lint/bad/cases/quote-inside-a-string/supabase/migrations/20260105000000_orders.sql b/fixtures/migrations-lint/bad/cases/quote-inside-a-string/supabase/migrations/20260105000000_orders.sql new file mode 100644 index 0000000..4fea767 --- /dev/null +++ b/fixtures/migrations-lint/bad/cases/quote-inside-a-string/supabase/migrations/20260105000000_orders.sql @@ -0,0 +1,10 @@ +-- ONE double quote inside an ordinary string literal. The scanner had no single-quoted-string +-- state, so this `"` opened a quoted identifier that ran to the end of the file and every +-- statement after it was invisible: the create below had no RLS anywhere and the file +-- reported ok, exit 0. An inch mark in seed data is enough to do it. +insert into public.products (name) values ('24" monitor'); + +create table public.orders ( + id uuid primary key, + owner_id uuid not null +); diff --git a/fixtures/migrations-lint/bad/cases/quoted-identifier-case/supabase/migrations/20260103000000_orders.down.sql b/fixtures/migrations-lint/bad/cases/quoted-identifier-case/supabase/migrations/20260103000000_orders.down.sql new file mode 100644 index 0000000..d6d4f22 --- /dev/null +++ b/fixtures/migrations-lint/bad/cases/quoted-identifier-case/supabase/migrations/20260103000000_orders.down.sql @@ -0,0 +1,2 @@ +drop table if exists public."Orders"; +drop table if exists public.orders; diff --git a/fixtures/migrations-lint/bad/cases/quoted-identifier-case/supabase/migrations/20260103000000_orders.sql b/fixtures/migrations-lint/bad/cases/quoted-identifier-case/supabase/migrations/20260103000000_orders.sql new file mode 100644 index 0000000..aefb0a7 --- /dev/null +++ b/fixtures/migrations-lint/bad/cases/quoted-identifier-case/supabase/migrations/20260103000000_orders.sql @@ -0,0 +1,8 @@ +-- PostgreSQL folds an unquoted identifier to lower case and keeps a quoted one exactly, so +-- "Orders" and orders are two different tables. The extractor lowercased both and the RLS +-- search was case-insensitive, so RLS on one satisfied a create of the other — a false green. +-- The quoted PascalCase form is what Prisma and Drizzle emit, so this is not a contrivance. +create table public."Orders" (id uuid primary key, owner uuid); + +create table public.orders (id uuid primary key, owner uuid); +alter table public.orders enable row level security; diff --git a/fixtures/migrations-lint/bad/cases/quoted-schema-qualifier/supabase/migrations/20260101000000_orders.down.sql b/fixtures/migrations-lint/bad/cases/quoted-schema-qualifier/supabase/migrations/20260101000000_orders.down.sql new file mode 100644 index 0000000..157967c --- /dev/null +++ b/fixtures/migrations-lint/bad/cases/quoted-schema-qualifier/supabase/migrations/20260101000000_orders.down.sql @@ -0,0 +1,2 @@ +drop table if exists "public"."orders"; +drop table if exists public.public; diff --git a/fixtures/migrations-lint/bad/cases/quoted-schema-qualifier/supabase/migrations/20260101000000_orders.sql b/fixtures/migrations-lint/bad/cases/quoted-schema-qualifier/supabase/migrations/20260101000000_orders.sql new file mode 100644 index 0000000..4c664c6 --- /dev/null +++ b/fixtures/migrations-lint/bad/cases/quoted-schema-qualifier/supabase/migrations/20260101000000_orders.sql @@ -0,0 +1,12 @@ +-- The schema qualifier is quoted SEPARATELY from the table: "public"."orders". The extractor +-- read the first identifier it could and took `public` for the table name, so the RLS check +-- below was run against a table called public rather than against orders. +-- +-- This fixture is shaped to discriminate. A table actually named `public` exists here and +-- does enable RLS, so the broken reading finds what it is looking for and the file passes, +-- with orders — the real table, with no RLS — never checked at all. A gate that reads the +-- name correctly checks orders, finds no RLS, and goes red. Before the fix: green. After: red. +create table public.public (id uuid primary key); +alter table public.public enable row level security; + +create table "public"."orders" (id uuid primary key); diff --git a/fixtures/migrations-lint/bad/unknown/ddl-in-a-dollar-quoted-body/supabase/migrations/20260109000000_orders.down.sql b/fixtures/migrations-lint/bad/unknown/ddl-in-a-dollar-quoted-body/supabase/migrations/20260109000000_orders.down.sql new file mode 100644 index 0000000..20f1679 --- /dev/null +++ b/fixtures/migrations-lint/bad/unknown/ddl-in-a-dollar-quoted-body/supabase/migrations/20260109000000_orders.down.sql @@ -0,0 +1 @@ +drop function if exists public.f(); diff --git a/fixtures/migrations-lint/bad/unknown/ddl-in-a-dollar-quoted-body/supabase/migrations/20260109000000_orders.sql b/fixtures/migrations-lint/bad/unknown/ddl-in-a-dollar-quoted-body/supabase/migrations/20260109000000_orders.sql new file mode 100644 index 0000000..7aaa786 --- /dev/null +++ b/fixtures/migrations-lint/bad/unknown/ddl-in-a-dollar-quoted-body/supabase/migrations/20260109000000_orders.sql @@ -0,0 +1,13 @@ +-- A dollar-quoted body is data, like any other string. Scanning it as SQL made the create +-- below a FAIL naming public.tmp — a table that does not exist at definition time and is +-- created only when the function runs. A red on a compliant file, naming a table nobody +-- created, which is the shape this gate refuses. +-- +-- `do $$ … $$` does execute immediately, so a create inside one is real; but the gate cannot +-- see whether RLS follows it inside the body either. UNKNOWN is the honest answer for both: +-- still red, still blocking, and claiming nothing it has not established. +create function public.f() returns void language plpgsql as $$ +begin + create table public.tmp (id int); +end +$$; diff --git a/fixtures/migrations-lint/bad/unknown/modifier-table-in-a-string/supabase/migrations/20260108000000_orders.down.sql b/fixtures/migrations-lint/bad/unknown/modifier-table-in-a-string/supabase/migrations/20260108000000_orders.down.sql new file mode 100644 index 0000000..6e997cb --- /dev/null +++ b/fixtures/migrations-lint/bad/unknown/modifier-table-in-a-string/supabase/migrations/20260108000000_orders.down.sql @@ -0,0 +1 @@ +drop table if exists public.o; diff --git a/fixtures/migrations-lint/bad/unknown/modifier-table-in-a-string/supabase/migrations/20260108000000_orders.sql b/fixtures/migrations-lint/bad/unknown/modifier-table-in-a-string/supabase/migrations/20260108000000_orders.sql new file mode 100644 index 0000000..7d16004 --- /dev/null +++ b/fixtures/migrations-lint/bad/unknown/modifier-table-in-a-string/supabase/migrations/20260108000000_orders.sql @@ -0,0 +1,9 @@ +-- `execute` running DDL this gate cannot follow must be UNKNOWN, and it was — until the +-- modifier allowance in that test was written as {0,2}, which mawk miscompiles to zero +-- repetitions when the group starts with a +-quantified bracket. The allowance was inert and +-- the test became exactly `create table`, so this line passed over silently. PostgreSQL 16 +-- confirms it creates a persistent unlogged table with RLS off. +create table public.o (id int); +alter table public.o enable row level security; + +do $x$ begin execute 'CREATE UNLOGGED TABLE public.x (id int)'; end $x$; diff --git a/fixtures/nextjs-env/bad/cases/nul-byte-hides-the-name/src/config.ts b/fixtures/nextjs-env/bad/cases/nul-byte-hides-the-name/src/config.ts new file mode 100644 index 0000000..23d3bfd Binary files /dev/null and b/fixtures/nextjs-env/bad/cases/nul-byte-hides-the-name/src/config.ts differ diff --git a/fixtures/service-role/bad/cases/aliased-duplicate-path/package.json b/fixtures/service-role/bad/cases/aliased-duplicate-path/package.json new file mode 100644 index 0000000..655114d --- /dev/null +++ b/fixtures/service-role/bad/cases/aliased-duplicate-path/package.json @@ -0,0 +1,4 @@ +{ + "name": "fixture", + "dependencies": { "next": "15.0.0" } +} diff --git a/fixtures/service-role/bad/cases/aliased-duplicate-path/src/app/page.tsx b/fixtures/service-role/bad/cases/aliased-duplicate-path/src/app/page.tsx new file mode 100644 index 0000000..5c884ab --- /dev/null +++ b/fixtures/service-role/bad/cases/aliased-duplicate-path/src/app/page.tsx @@ -0,0 +1,2 @@ +import { key } from "@/lib/secret"; +export default function Page() { return
{key ? "y" : "n"}
; } diff --git a/fixtures/service-role/bad/cases/aliased-duplicate-path/src/app/x/page.tsx b/fixtures/service-role/bad/cases/aliased-duplicate-path/src/app/x/page.tsx new file mode 100644 index 0000000..4be6d0d --- /dev/null +++ b/fixtures/service-role/bad/cases/aliased-duplicate-path/src/app/x/page.tsx @@ -0,0 +1,2 @@ +import { key } from "../../lib/secret"; +export default function X() { return
{key ? "y" : "n"}
; } diff --git a/fixtures/service-role/bad/cases/aliased-duplicate-path/src/lib/secret.ts b/fixtures/service-role/bad/cases/aliased-duplicate-path/src/lib/secret.ts new file mode 100644 index 0000000..55bd353 --- /dev/null +++ b/fixtures/service-role/bad/cases/aliased-duplicate-path/src/lib/secret.ts @@ -0,0 +1 @@ +export const key = process.env.SUPABASE_SECRET_KEY; diff --git a/fixtures/service-role/bad/cases/aliased-duplicate-path/tsconfig.json b/fixtures/service-role/bad/cases/aliased-duplicate-path/tsconfig.json new file mode 100644 index 0000000..18254f7 --- /dev/null +++ b/fixtures/service-role/bad/cases/aliased-duplicate-path/tsconfig.json @@ -0,0 +1,6 @@ +{ + "compilerOptions": { + "baseUrl": ".", + "paths": { "@/*": ["./src/*"] } + } +} diff --git a/fixtures/service-role/bad/cases/baseurl-absolute-import/package.json b/fixtures/service-role/bad/cases/baseurl-absolute-import/package.json new file mode 100644 index 0000000..0612c27 --- /dev/null +++ b/fixtures/service-role/bad/cases/baseurl-absolute-import/package.json @@ -0,0 +1 @@ +{ "name": "fixture-baseurl-absolute-import", "dependencies": { "next": "15.0.0" } } diff --git a/fixtures/service-role/bad/cases/baseurl-absolute-import/src/app/page.tsx b/fixtures/service-role/bad/cases/baseurl-absolute-import/src/app/page.tsx new file mode 100644 index 0000000..0470121 --- /dev/null +++ b/fixtures/service-role/bad/cases/baseurl-absolute-import/src/app/page.tsx @@ -0,0 +1,9 @@ +// Next.js "Absolute Imports": baseUrl alone, no paths alias anywhere. `lib/supabase-admin` +// means `src/lib/supabase-admin.ts`. The gate parsed this baseUrl — it is the base for every +// alias target — and still skipped this bare specifier as a published package. Written +// `../lib/supabase-admin`, the same file and the same secret were caught. +import { admin } from "lib/supabase-admin"; + +export default function Page() { + return admin; +} diff --git a/fixtures/service-role/bad/cases/baseurl-absolute-import/src/lib/supabase-admin.ts b/fixtures/service-role/bad/cases/baseurl-absolute-import/src/lib/supabase-admin.ts new file mode 100644 index 0000000..7b4886c --- /dev/null +++ b/fixtures/service-role/bad/cases/baseurl-absolute-import/src/lib/supabase-admin.ts @@ -0,0 +1 @@ +export const admin = process.env.SUPABASE_SERVICE_ROLE_KEY; diff --git a/fixtures/service-role/bad/cases/baseurl-absolute-import/tsconfig.json b/fixtures/service-role/bad/cases/baseurl-absolute-import/tsconfig.json new file mode 100644 index 0000000..41b4e99 --- /dev/null +++ b/fixtures/service-role/bad/cases/baseurl-absolute-import/tsconfig.json @@ -0,0 +1 @@ +{ "compilerOptions": { "baseUrl": "src" } } diff --git a/fixtures/service-role/bad/cases/baseurl-dot-absolute-import/app/page.tsx b/fixtures/service-role/bad/cases/baseurl-dot-absolute-import/app/page.tsx new file mode 100644 index 0000000..0335ecb --- /dev/null +++ b/fixtures/service-role/bad/cases/baseurl-dot-absolute-import/app/page.tsx @@ -0,0 +1,10 @@ +// `"baseUrl": "."` is the spelling in Next.js's own Absolute Imports documentation and the +// one create-next-app ships. The first version of the baseUrl fallback armed on the VALUE +// rather than on the key being present, and "." is exactly the value it excluded — so the +// false green it was written to close stayed open on the commonest spelling, while the +// fixture beside this one used "src" and kept the selftest green over the half that worked. +import { admin } from "lib/supabase-admin"; + +export default function Page() { + return admin; +} diff --git a/fixtures/service-role/bad/cases/baseurl-dot-absolute-import/lib/supabase-admin.ts b/fixtures/service-role/bad/cases/baseurl-dot-absolute-import/lib/supabase-admin.ts new file mode 100644 index 0000000..7b4886c --- /dev/null +++ b/fixtures/service-role/bad/cases/baseurl-dot-absolute-import/lib/supabase-admin.ts @@ -0,0 +1 @@ +export const admin = process.env.SUPABASE_SERVICE_ROLE_KEY; diff --git a/fixtures/service-role/bad/cases/baseurl-dot-absolute-import/package.json b/fixtures/service-role/bad/cases/baseurl-dot-absolute-import/package.json new file mode 100644 index 0000000..e5b4d86 --- /dev/null +++ b/fixtures/service-role/bad/cases/baseurl-dot-absolute-import/package.json @@ -0,0 +1 @@ +{ "name": "fixture-baseurl-dot-absolute-import", "dependencies": { "next": "15.0.0" } } diff --git a/fixtures/service-role/bad/cases/baseurl-dot-absolute-import/tsconfig.json b/fixtures/service-role/bad/cases/baseurl-dot-absolute-import/tsconfig.json new file mode 100644 index 0000000..af8c5aa --- /dev/null +++ b/fixtures/service-role/bad/cases/baseurl-dot-absolute-import/tsconfig.json @@ -0,0 +1 @@ +{ "compilerOptions": { "baseUrl": "." } } diff --git a/fixtures/service-role/bad/cases/baseurl-nonpackage-name/package.json b/fixtures/service-role/bad/cases/baseurl-nonpackage-name/package.json new file mode 100644 index 0000000..870a565 --- /dev/null +++ b/fixtures/service-role/bad/cases/baseurl-nonpackage-name/package.json @@ -0,0 +1 @@ +{ "name": "fixture-baseurl-nonpackage-name", "dependencies": { "next": "15.0.0" } } diff --git a/fixtures/service-role/bad/cases/baseurl-nonpackage-name/src/_components/Button.ts b/fixtures/service-role/bad/cases/baseurl-nonpackage-name/src/_components/Button.ts new file mode 100644 index 0000000..7b4886c --- /dev/null +++ b/fixtures/service-role/bad/cases/baseurl-nonpackage-name/src/_components/Button.ts @@ -0,0 +1 @@ +export const admin = process.env.SUPABASE_SERVICE_ROLE_KEY; diff --git a/fixtures/service-role/bad/cases/baseurl-nonpackage-name/src/app/page.tsx b/fixtures/service-role/bad/cases/baseurl-nonpackage-name/src/app/page.tsx new file mode 100644 index 0000000..ee52bc9 --- /dev/null +++ b/fixtures/service-role/bad/cases/baseurl-nonpackage-name/src/app/page.tsx @@ -0,0 +1,10 @@ +// An underscore-prefixed private folder is an ordinary Next.js convention, and a leading +// underscore is not a valid npm package name. The baseUrl probe was gated on the specifier +// LOOKING like a package, so it skipped exactly the names most likely to be baseUrl-relative: +// this file resolves under baseUrl and was reported UNKNOWN anyway — a red on a tree the gate +// could read perfectly well, and one that hides the violation underneath it. +import { admin } from "_components/Button"; + +export default function Page() { + return admin; +} diff --git a/fixtures/service-role/bad/cases/baseurl-nonpackage-name/tsconfig.json b/fixtures/service-role/bad/cases/baseurl-nonpackage-name/tsconfig.json new file mode 100644 index 0000000..41b4e99 --- /dev/null +++ b/fixtures/service-role/bad/cases/baseurl-nonpackage-name/tsconfig.json @@ -0,0 +1 @@ +{ "compilerOptions": { "baseUrl": "src" } } diff --git a/fixtures/service-role/bad/cases/circular-import/package.json b/fixtures/service-role/bad/cases/circular-import/package.json new file mode 100644 index 0000000..655114d --- /dev/null +++ b/fixtures/service-role/bad/cases/circular-import/package.json @@ -0,0 +1,4 @@ +{ + "name": "fixture", + "dependencies": { "next": "15.0.0" } +} diff --git a/fixtures/service-role/bad/cases/circular-import/src/app/page.tsx b/fixtures/service-role/bad/cases/circular-import/src/app/page.tsx new file mode 100644 index 0000000..b4be9e4 --- /dev/null +++ b/fixtures/service-role/bad/cases/circular-import/src/app/page.tsx @@ -0,0 +1,2 @@ +import { b } from "../lib/b"; +export default function Page() { return
{b ? "y" : "n"}
; } diff --git a/fixtures/service-role/bad/cases/circular-import/src/lib/a.ts b/fixtures/service-role/bad/cases/circular-import/src/lib/a.ts new file mode 100644 index 0000000..16c70c4 --- /dev/null +++ b/fixtures/service-role/bad/cases/circular-import/src/lib/a.ts @@ -0,0 +1,2 @@ +import { b } from "./b"; +export const a = b; diff --git a/fixtures/service-role/bad/cases/circular-import/src/lib/b.ts b/fixtures/service-role/bad/cases/circular-import/src/lib/b.ts new file mode 100644 index 0000000..12d8c0e --- /dev/null +++ b/fixtures/service-role/bad/cases/circular-import/src/lib/b.ts @@ -0,0 +1,4 @@ +// b imports a, a imports b. Before the paths were canonicalised this pair grew a longer +// spelling every hop, the visited set never matched, and the walk did not terminate. +import { a } from "./a"; +export const b = a ?? process.env.SUPABASE_SECRET_KEY; diff --git a/fixtures/service-role/bad/cases/declaration-sidecar/app/page.jsx b/fixtures/service-role/bad/cases/declaration-sidecar/app/page.jsx new file mode 100644 index 0000000..c96179c --- /dev/null +++ b/fixtures/service-role/bad/cases/declaration-sidecar/app/page.jsx @@ -0,0 +1,9 @@ +// A hand-written declaration sits beside the implementation. Probing the declaration first +// resolved to a file that BY CONSTRUCTION cannot hold a secret, and lib/admin.js — the module +// Node actually loads — was never read. Deleting the .d.ts turned the same tree red, which is +// the sidecar doing the hiding. +import { admin } from "../lib/admin.js"; + +export default function Page() { + return admin; +} diff --git a/fixtures/service-role/bad/cases/declaration-sidecar/lib/admin.d.ts b/fixtures/service-role/bad/cases/declaration-sidecar/lib/admin.d.ts new file mode 100644 index 0000000..2f29f42 --- /dev/null +++ b/fixtures/service-role/bad/cases/declaration-sidecar/lib/admin.d.ts @@ -0,0 +1 @@ +export declare const admin: string | undefined; diff --git a/fixtures/service-role/bad/cases/declaration-sidecar/lib/admin.js b/fixtures/service-role/bad/cases/declaration-sidecar/lib/admin.js new file mode 100644 index 0000000..7b4886c --- /dev/null +++ b/fixtures/service-role/bad/cases/declaration-sidecar/lib/admin.js @@ -0,0 +1 @@ +export const admin = process.env.SUPABASE_SERVICE_ROLE_KEY; diff --git a/fixtures/service-role/bad/cases/declaration-sidecar/package.json b/fixtures/service-role/bad/cases/declaration-sidecar/package.json new file mode 100644 index 0000000..a211ace --- /dev/null +++ b/fixtures/service-role/bad/cases/declaration-sidecar/package.json @@ -0,0 +1 @@ +{ "name": "fixture-declaration-sidecar", "type": "module", "dependencies": { "next": "15.0.0" } } diff --git a/fixtures/service-role/bad/cases/division-after-brace/lib/admin.js b/fixtures/service-role/bad/cases/division-after-brace/lib/admin.js new file mode 100644 index 0000000..7b4886c --- /dev/null +++ b/fixtures/service-role/bad/cases/division-after-brace/lib/admin.js @@ -0,0 +1 @@ +export const admin = process.env.SUPABASE_SERVICE_ROLE_KEY; diff --git a/fixtures/service-role/bad/cases/division-after-brace/package.json b/fixtures/service-role/bad/cases/division-after-brace/package.json new file mode 100644 index 0000000..420ec29 --- /dev/null +++ b/fixtures/service-role/bad/cases/division-after-brace/package.json @@ -0,0 +1 @@ +{ "name": "fixture-division-after-brace", "dependencies": { "next": "15.0.0" } } diff --git a/fixtures/service-role/bad/cases/division-after-brace/pages/index.js b/fixtures/service-role/bad/cases/division-after-brace/pages/index.js new file mode 100644 index 0000000..ba8d311 --- /dev/null +++ b/fixtures/service-role/bad/cases/division-after-brace/pages/index.js @@ -0,0 +1,10 @@ +// A slash after `}` is division here, not a regex. Treating `}` as a regex position made the +// scanner consume the rest of the line looking for a closing slash — swallowing the real +// import beside it. ok, exit 0, on a module reaching the key, where the pre-tokeniser +// extractor caught it. A block close can precede a regex, so this trades a rare false red +// for a false green, which is the trade this toolkit takes every time. +const x = {} / foo; import { admin } from "../lib/admin"; + +export default function Page() { + return admin; +} diff --git a/fixtures/service-role/bad/cases/emitted-extension-import/app/admin.mts b/fixtures/service-role/bad/cases/emitted-extension-import/app/admin.mts new file mode 100644 index 0000000..7b4886c --- /dev/null +++ b/fixtures/service-role/bad/cases/emitted-extension-import/app/admin.mts @@ -0,0 +1 @@ +export const admin = process.env.SUPABASE_SERVICE_ROLE_KEY; diff --git a/fixtures/service-role/bad/cases/emitted-extension-import/app/page.tsx b/fixtures/service-role/bad/cases/emitted-extension-import/app/page.tsx new file mode 100644 index 0000000..c27b0fb --- /dev/null +++ b/fixtures/service-role/bad/cases/emitted-extension-import/app/page.tsx @@ -0,0 +1,7 @@ +// nodenext makes the import name the EMITTED file (.mjs) while the source is .mts. +// The gate must follow that mapping; appending extensions to "./admin.mjs" finds nothing. +import { admin } from "./admin.mjs"; + +export default function Page() { + return admin; +} diff --git a/fixtures/service-role/bad/cases/emitted-extension-import/package.json b/fixtures/service-role/bad/cases/emitted-extension-import/package.json new file mode 100644 index 0000000..7a67e5e --- /dev/null +++ b/fixtures/service-role/bad/cases/emitted-extension-import/package.json @@ -0,0 +1 @@ +{ "name": "fixture-emitted-extension-import", "dependencies": { "next": "15.0.0" } } diff --git a/fixtures/service-role/bad/cases/emitted-extension-import/tsconfig.json b/fixtures/service-role/bad/cases/emitted-extension-import/tsconfig.json new file mode 100644 index 0000000..1bfb2d8 --- /dev/null +++ b/fixtures/service-role/bad/cases/emitted-extension-import/tsconfig.json @@ -0,0 +1 @@ +{ "compilerOptions": { "moduleResolution": "nodenext", "verbatimModuleSyntax": true } } diff --git a/fixtures/service-role/bad/cases/explicit-js-over-ts-sibling/lib/admin.js b/fixtures/service-role/bad/cases/explicit-js-over-ts-sibling/lib/admin.js new file mode 100644 index 0000000..36cfd0b --- /dev/null +++ b/fixtures/service-role/bad/cases/explicit-js-over-ts-sibling/lib/admin.js @@ -0,0 +1 @@ +module.exports.admin = process.env.SUPABASE_SERVICE_ROLE_KEY; diff --git a/fixtures/service-role/bad/cases/explicit-js-over-ts-sibling/lib/admin.ts b/fixtures/service-role/bad/cases/explicit-js-over-ts-sibling/lib/admin.ts new file mode 100644 index 0000000..f320d9f --- /dev/null +++ b/fixtures/service-role/bad/cases/explicit-js-over-ts-sibling/lib/admin.ts @@ -0,0 +1 @@ +export const admin = "clean"; diff --git a/fixtures/service-role/bad/cases/explicit-js-over-ts-sibling/package.json b/fixtures/service-role/bad/cases/explicit-js-over-ts-sibling/package.json new file mode 100644 index 0000000..74f7218 --- /dev/null +++ b/fixtures/service-role/bad/cases/explicit-js-over-ts-sibling/package.json @@ -0,0 +1 @@ +{ "name": "fixture-explicit-js-over-ts-sibling", "dependencies": { "next": "15.0.0" } } diff --git a/fixtures/service-role/bad/cases/explicit-js-over-ts-sibling/pages/index.js b/fixtures/service-role/bad/cases/explicit-js-over-ts-sibling/pages/index.js new file mode 100644 index 0000000..3bf310b --- /dev/null +++ b/fixtures/service-role/bad/cases/explicit-js-over-ts-sibling/pages/index.js @@ -0,0 +1,9 @@ +// A JavaScript project importing an explicit `.js`. A same-named `.ts` sits beside it and is +// clean; `lib/admin.js` — the module Node actually loads — holds the key. Ordering the +// candidate list put the `.ts` substitution first, so the clean file was scanned and this +// returned ok, exit 0. Whichever candidate is second gets skipped, so both are walked now. +const { admin } = require("../lib/admin.js"); + +module.exports = function Page() { + return admin; +}; diff --git a/fixtures/service-role/bad/cases/module-require-loader/lib/admin.js b/fixtures/service-role/bad/cases/module-require-loader/lib/admin.js new file mode 100644 index 0000000..36cfd0b --- /dev/null +++ b/fixtures/service-role/bad/cases/module-require-loader/lib/admin.js @@ -0,0 +1 @@ +module.exports.admin = process.env.SUPABASE_SERVICE_ROLE_KEY; diff --git a/fixtures/service-role/bad/cases/module-require-loader/package.json b/fixtures/service-role/bad/cases/module-require-loader/package.json new file mode 100644 index 0000000..ecd9042 --- /dev/null +++ b/fixtures/service-role/bad/cases/module-require-loader/package.json @@ -0,0 +1 @@ +{ "name": "fixture-module-require-loader", "dependencies": { "next": "15.0.0" } } diff --git a/fixtures/service-role/bad/cases/module-require-loader/pages/index.js b/fixtures/service-role/bad/cases/module-require-loader/pages/index.js new file mode 100644 index 0000000..913a844 --- /dev/null +++ b/fixtures/service-role/bad/cases/module-require-loader/pages/index.js @@ -0,0 +1,8 @@ +// `module.require()` is a real Node module loader. Dropping every `require` preceded by a dot +// — added to stop `Array.from(",")` opening a specifier slot — took this with it, and the +// dependency below went unwalked: ok, exit 0, where the pre-tokeniser extractor caught it. +const { admin } = module.require("../lib/admin"); + +module.exports = function Page() { + return admin; +}; diff --git a/fixtures/service-role/bad/cases/nul-byte-in-a-module/app/page.tsx b/fixtures/service-role/bad/cases/nul-byte-in-a-module/app/page.tsx new file mode 100644 index 0000000..a8f2957 --- /dev/null +++ b/fixtures/service-role/bad/cases/nul-byte-in-a-module/app/page.tsx @@ -0,0 +1,8 @@ +// The module below holds the service-role key and contains one NUL byte. Without `-a`, grep +// calls that file binary, prints nothing and exits 1 — which this gate reads as "the term is +// not present". One byte hid the secret and the gate reported ok, exit 0. +import { admin } from "../lib/admin"; + +export default function Page() { + return admin; +} diff --git a/fixtures/service-role/bad/cases/nul-byte-in-a-module/lib/admin.ts b/fixtures/service-role/bad/cases/nul-byte-in-a-module/lib/admin.ts new file mode 100644 index 0000000..d042d55 Binary files /dev/null and b/fixtures/service-role/bad/cases/nul-byte-in-a-module/lib/admin.ts differ diff --git a/fixtures/service-role/bad/cases/nul-byte-in-a-module/package.json b/fixtures/service-role/bad/cases/nul-byte-in-a-module/package.json new file mode 100644 index 0000000..2288db3 --- /dev/null +++ b/fixtures/service-role/bad/cases/nul-byte-in-a-module/package.json @@ -0,0 +1 @@ +{ "name": "fixture-nul-byte-in-a-module", "dependencies": { "next": "15.0.0" } } diff --git a/fixtures/service-role/bad/cases/one-line-swallow/app/page.tsx b/fixtures/service-role/bad/cases/one-line-swallow/app/page.tsx new file mode 100644 index 0000000..c3cdb3c --- /dev/null +++ b/fixtures/service-role/bad/cases/one-line-swallow/app/page.tsx @@ -0,0 +1,16 @@ +// A string ending in the word `from`, immediately before a real import, on ONE physical line. +// This is the shape that broke the old grep extractor in both directions: reporting the +// invented span was a blocking UNKNOWN naming an import that does not exist, and dropping it +// lost the real import below and returned `ok`, exit 0, over a module reaching the secret. +// +// The tokeniser does neither — it recognises the string as a string and the import as an +// import, so this fixture asserts the VIOLATION, not a blind spot. It lived under +// bad/unknown/ for exactly one commit, which is the record of the gate having got better: +// the honest answer moved from "I could not read this" to "here is what it reaches". +// +// cases/string-ending-in-from is this file with a newline between the two statements. +const label = "imported from "; import { admin } from "../lib/admin"; + +export default function Page() { + return admin; +} diff --git a/fixtures/service-role/bad/cases/one-line-swallow/lib/admin.ts b/fixtures/service-role/bad/cases/one-line-swallow/lib/admin.ts new file mode 100644 index 0000000..7b4886c --- /dev/null +++ b/fixtures/service-role/bad/cases/one-line-swallow/lib/admin.ts @@ -0,0 +1 @@ +export const admin = process.env.SUPABASE_SERVICE_ROLE_KEY; diff --git a/fixtures/service-role/bad/cases/one-line-swallow/package.json b/fixtures/service-role/bad/cases/one-line-swallow/package.json new file mode 100644 index 0000000..127c127 --- /dev/null +++ b/fixtures/service-role/bad/cases/one-line-swallow/package.json @@ -0,0 +1 @@ +{ "name": "fixture-one-line-swallow", "dependencies": { "next": "15.0.0" } } diff --git a/fixtures/service-role/bad/cases/string-ending-in-from/package.json b/fixtures/service-role/bad/cases/string-ending-in-from/package.json new file mode 100644 index 0000000..655114d --- /dev/null +++ b/fixtures/service-role/bad/cases/string-ending-in-from/package.json @@ -0,0 +1,4 @@ +{ + "name": "fixture", + "dependencies": { "next": "15.0.0" } +} diff --git a/fixtures/service-role/bad/cases/string-ending-in-from/src/app/page.tsx b/fixtures/service-role/bad/cases/string-ending-in-from/src/app/page.tsx new file mode 100644 index 0000000..4c47aa6 --- /dev/null +++ b/fixtures/service-role/bad/cases/string-ending-in-from/src/app/page.tsx @@ -0,0 +1,9 @@ +// The string ends in `from "`. With the whole file flattened to one line and grep -o +// matching non-overlapping, that span swallows the opening quote of the real import +// below it, and the edge to src/lib/secret.ts disappears. Caught as a regression before +// it shipped; kept here so it cannot come back. +const label = "imported from "; +import { key } from "../lib/secret"; +export default function Page() { + return
{label}{key ? "y" : "n"}
; +} diff --git a/fixtures/service-role/bad/cases/string-ending-in-from/src/lib/secret.ts b/fixtures/service-role/bad/cases/string-ending-in-from/src/lib/secret.ts new file mode 100644 index 0000000..f0c6e75 --- /dev/null +++ b/fixtures/service-role/bad/cases/string-ending-in-from/src/lib/secret.ts @@ -0,0 +1 @@ +export const key = process.env.SUPABASE_SERVICE_ROLE_KEY; diff --git a/fixtures/service-role/bad/unknown/alias-shaped-nonpackage/app/page.tsx b/fixtures/service-role/bad/unknown/alias-shaped-nonpackage/app/page.tsx new file mode 100644 index 0000000..b82738d --- /dev/null +++ b/fixtures/service-role/bad/unknown/alias-shaped-nonpackage/app/page.tsx @@ -0,0 +1,8 @@ +// "@/lib/secret" matches no alias declared above, and npm has no empty scope, so it cannot +// be a published package either. The gate must not skip it as a dependency: it does not +// know what it is, and an unread module is not a clean one. +import { key } from "@/lib/secret"; + +export default function Page() { + return key; +} diff --git a/fixtures/service-role/bad/unknown/alias-shaped-nonpackage/package.json b/fixtures/service-role/bad/unknown/alias-shaped-nonpackage/package.json new file mode 100644 index 0000000..4af6d69 --- /dev/null +++ b/fixtures/service-role/bad/unknown/alias-shaped-nonpackage/package.json @@ -0,0 +1 @@ +{ "name": "fixture-alias-shaped-nonpackage", "dependencies": { "next": "15.0.0" } } diff --git a/fixtures/service-role/bad/unknown/alias-shaped-nonpackage/tsconfig.json b/fixtures/service-role/bad/unknown/alias-shaped-nonpackage/tsconfig.json new file mode 100644 index 0000000..3ea5598 --- /dev/null +++ b/fixtures/service-role/bad/unknown/alias-shaped-nonpackage/tsconfig.json @@ -0,0 +1 @@ +{ "compilerOptions": { "baseUrl": ".", "paths": { "~/util/*": ["./src/util/*"] } } } diff --git a/fixtures/service-role/bad/unknown/multiline-dynamic-import/package.json b/fixtures/service-role/bad/unknown/multiline-dynamic-import/package.json new file mode 100644 index 0000000..655114d --- /dev/null +++ b/fixtures/service-role/bad/unknown/multiline-dynamic-import/package.json @@ -0,0 +1,4 @@ +{ + "name": "fixture", + "dependencies": { "next": "15.0.0" } +} diff --git a/fixtures/service-role/bad/unknown/multiline-dynamic-import/src/app/page.tsx b/fixtures/service-role/bad/unknown/multiline-dynamic-import/src/app/page.tsx new file mode 100644 index 0000000..ad6b140 --- /dev/null +++ b/fixtures/service-role/bad/unknown/multiline-dynamic-import/src/app/page.tsx @@ -0,0 +1,8 @@ +export default async function Page() { + // The specifier sits on its own line, which is what Prettier does to a long import(). + // A line-at-a-time scan sees neither the call nor the argument as one statement. + const mod = await import( + process.env.MODULE_NAME ?? "./safe" + ); + return
{String(mod.value)}
; +} diff --git a/fixtures/service-role/bad/unknown/unterminated-template-literal/app/page.tsx b/fixtures/service-role/bad/unknown/unterminated-template-literal/app/page.tsx new file mode 100644 index 0000000..77c89e0 --- /dev/null +++ b/fixtures/service-role/bad/unknown/unterminated-template-literal/app/page.tsx @@ -0,0 +1,13 @@ +// A stray backtick in JSX text. It is not a template literal, but nothing short of a JSX +// parser can tell — so the scan opens one and runs to the end of the file, and every import +// below it is read as template-literal text. Before the sync-loss guard this file reported +// ok, exit 0, over a module that reaches the secret through the require() below. +export function Hint() { + return

Press the ` key to continue

; +} + +const { admin } = require("../lib/admin"); + +export default function Page() { + return admin; +} diff --git a/fixtures/service-role/bad/unknown/unterminated-template-literal/lib/admin.ts b/fixtures/service-role/bad/unknown/unterminated-template-literal/lib/admin.ts new file mode 100644 index 0000000..7b4886c --- /dev/null +++ b/fixtures/service-role/bad/unknown/unterminated-template-literal/lib/admin.ts @@ -0,0 +1 @@ +export const admin = process.env.SUPABASE_SERVICE_ROLE_KEY; diff --git a/fixtures/service-role/bad/unknown/unterminated-template-literal/package.json b/fixtures/service-role/bad/unknown/unterminated-template-literal/package.json new file mode 100644 index 0000000..29e224a --- /dev/null +++ b/fixtures/service-role/bad/unknown/unterminated-template-literal/package.json @@ -0,0 +1 @@ +{ "name": "fixture-unterminated-template-literal", "dependencies": { "next": "15.0.0" } } diff --git a/gates/actions-sha-pinned.sh b/gates/actions-sha-pinned.sh index af92791..5784042 100755 --- a/gates/actions-sha-pinned.sh +++ b/gates/actions-sha-pinned.sh @@ -29,7 +29,7 @@ LIST="$(mktemp)"; ERR="$(mktemp)" trap 'rm -f "$LIST" "$ERR"' EXIT set +e # -e for the pattern and -- before the path, so neither can be read as an option. -grep -rEn --include='*.yml' --include='*.yaml' -e "$KEY" -- "$WF" > "$LIST" 2> "$ERR" +grep -arEn --include='*.yml' --include='*.yaml' -e "$KEY" -- "$WF" > "$LIST" 2> "$ERR" rc=$? set -e if [ "$rc" -gt 1 ]; then diff --git a/gates/lib.sh b/gates/lib.sh index c6d375f..3fcc69c 100755 --- a/gates/lib.sh +++ b/gates/lib.sh @@ -101,4 +101,13 @@ finish() { # for a literal term handed grep two matchers; grep answered "conflicting matchers specified" # and exited 2 on every such search, which made the per-repo denylist unusable in any repo # that had one. A helper that dictates the matcher is a helper that decides the search. -tgrep() { grep -rn --exclude-dir=node_modules --exclude-dir=.git --exclude-dir=.next --exclude-dir=coverage "$@"; } +# `-a` is not decoration. Without it one byte that is invalid in the ambient locale makes GNU +# grep call a file binary, print nothing and EXIT 0 — which every caller here reads as "not +# found". Measured: a NUL byte in a module hid a `NEXT_PUBLIC_`-prefixed service-key name from +# nextjs-env, the gate whose whole job is secret names, and the file reported ok, exit 0. +# +# The name is written in pieces here on purpose: spelled out, this comment is itself a +# NEXT_PUBLIC_ variable named like a server secret, and nextjs-env red on the toolkit's own +# tree the moment it was added. The gate was right, which is the second-best way to learn it +# works. +tgrep() { grep -arn --exclude-dir=node_modules --exclude-dir=.git --exclude-dir=.next --exclude-dir=coverage "$@"; } diff --git a/gates/migrations-lint.sh b/gates/migrations-lint.sh index a2b34c6..4b8699c 100755 --- a/gates/migrations-lint.sh +++ b/gates/migrations-lint.sh @@ -31,45 +31,307 @@ MIG="$ROOT/supabase/migrations" # the rest of that line. That is a real gap, not a safe one — a create table sharing that # line would go unchecked. A SQL parser is the fix if it ever bites; a regex that pretends # to know about quoting is not. -strip_sql_comments() { - awk ' - BEGIN { inblk = 0 } - { - line = $0; out = "" - while (length(line) > 0) { - if (inblk) { - p = index(line, "*/") - if (p == 0) { line = ""; break } - line = substr(line, p + 2); inblk = 0 - } else { - pb = index(line, "/*"); pl = index(line, "--") - if (pl > 0 && (pb == 0 || pl < pb)) { out = out substr(line, 1, pl - 1); line = ""; break } - if (pb > 0) { out = out substr(line, 1, pb - 1); line = substr(line, pb + 2); inblk = 1 } - else { out = out line; line = "" } - } + +SQLSCAN="$(mktemp)" +cat > "$SQLSCAN" <<'SQLSCANAWK' +# Streaming SQL scanner. One record per line: kind schema table, where kind is +# "C" (create table), "R" (RLS enabled) or "U" (this scanner could not answer). Values are +# escaped, because a quoted identifier may contain a tab or a newline and a delimiter a value +# can contain is not a delimiter. +# +# LINE-INCREMENTAL, NOT SLURPED. The first version built the whole file with `buf = buf $0 +# "\n"`, which mawk reallocates and copies every line: 12.1s on a 1.1MB file against 0.11s for +# the greps it replaced, and quadratic, so it got worse with size. A gates job that takes +# minutes and scales the wrong way is the shape of the hang this repository has already been +# bitten by once. State is carried across lines instead; nothing accumulates but the current +# token and the current name. +function esc(v) { gsub(/\\/, "\\\\", v); gsub(/\t/, "\\t", v); gsub(/\n/, "\\n", v); return v } +function emitpair(kind, tbl, sch) { + if (nn == 0) return + tbl = P[nn]; sch = (nn >= 2) ? P[nn - 1] : "public" + if (tbl == "") return + print kind "\t" esc(sch) "\t" esc(tbl) +} +function tok(k, v, again) { + again = 1 + while (again) { + again = 0 + if (st == 0) { + if (k == "W" && v == "create") st = 1 + else if (k == "W" && v == "alter") st = 4 + # The destructive-statement note is emitted from the token stream too, so that the + # comment stripper can go away entirely rather than survive for one caller. A + # commented-out `drop table` is a comment, and a note claiming otherwise is a claim. + else if (k == "W" && v == "drop") st = 8 + else if (k == "W" && v == "truncate") { print "X\t\t"; } + else if (k == "W" && v == "delete") st = 9 + } else if (st == 8) { + if (k == "W" && v == "table") { print "X\t\t"; st = 0 } + else { st = 0; again = 1 } + } else if (st == 9) { + if (k == "W" && v == "from") { st = 10 } + else { st = 0; again = 1 } + } else if (st == 10) { + # A `delete from` is tier-3 only WITHOUT a where, which is what the note says. Emitting + # it for every delete made the note claim something the code beside it did not. + if (k == "W" && v == "where") st = 0 + else if (k == "P" && v == ";") { print "X\t\t"; st = 0 } + else if (k == "W" && (v == "create" || v == "alter" || v == "drop" || v == "insert" || v == "update" || v == "truncate")) { print "X\t\t"; st = 0; again = 1 } + } else if (st == 1) { + if (k == "W" && (v == "global" || v == "local" || v == "temporary" || v == "temp" || v == "unlogged")) { } + else if (k == "W" && v == "table") st = 2 + else { st = 0; again = 1 } + } else if (st == 2) { + # `IF NOT EXISTS` AS A SEQUENCE, not three words to swallow wherever they appear. + # Swallowing them lost `create table exists (id int)` and `create table if (id int)` + # entirely — both legal, both verified against PostgreSQL 16, and a create table this + # scanner does not see is a silent pass. + if (k == "W" && v == "if") st = 21 + else if (k == "W" || k == "Q") { nn = 1; P[1] = v; st = 3; wantpart = 0 } + else { st = 0; again = 1 } + } else if (st == 21) { + if (k == "W" && v == "not") st = 22 + else { nn = 1; P[1] = "if"; st = 3; wantpart = 0; again = 1 } + } else if (st == 22) { + if (k == "W" && v == "exists") st = 2 + else { nn = 1; P[1] = "if"; st = 3; wantpart = 0; again = 1 } + } else if (st == 3) { + if (k == "D") wantpart = 1 + else if (wantpart && (k == "W" || k == "Q")) { P[++nn] = v; wantpart = 0 } + else if (wantpart) { st = 0; nn = 0; again = 1 } + else { emitpair("C"); st = 0; nn = 0; again = 1 } + } else if (st == 4) { + if (k == "W" && v == "table") st = 5 + else { st = 0; again = 1 } + } else if (st == 5) { + # `only` is reserved and can be swallowed. `if` and `exists` are NOT — they are legal + # table names, which the create side already knows. Swallowing them here bound `enable` + # as the table name of `alter table if enable row level security`, so no R record was + # emitted and a compliant file reported a violation. + if (k == "W" && v == "only") { } + else if (k == "W" && v == "if") st = 51 + else if (k == "W" || k == "Q") { nn = 1; P[1] = v; st = 6; wantpart = 0 } + else { st = 0; again = 1 } + } else if (st == 51) { + if (k == "W" && v == "exists") st = 5 + else { nn = 1; P[1] = "if"; st = 6; wantpart = 0; again = 1 } + } else if (st == 6) { + if (k == "D") wantpart = 1 + else if (wantpart && (k == "W" || k == "Q")) { P[++nn] = v; wantpart = 0 } + else if (wantpart) { st = 0; nn = 0; again = 1 } + else { st = 7; es = 0; again = 1 } + } else if (st == 7) { + if (k == "W" && es == 0 && v == "enable") es = 1 + else if (k == "W" && es == 1 && v == "row") es = 2 + else if (k == "W" && es == 2 && v == "level") es = 3 + else if (k == "W" && es == 3 && v == "security") { emitpair("R"); st = 0; nn = 0 } + else { st = 0; nn = 0; again = 1 } + } + } +} +{ + n = length($0); i = 1 + while (i <= n) { + c = substr($0, i, 1) + # COMMENTS ARE STRIPPED HERE, NOT BY A STAGE THAT CANNOT SEE STRINGS. They used to be + # removed upstream by a scanner with no string state, which cost a defect in each + # direction and both were live: + # + # values ('x /* y'); create table public.orders (id int); values ('*/ z'); + # -> the `/*` INSIDE a string opened a block comment that deleted the create table + # on the following line. `ok`, exit 0, over a table PostgreSQL 16 confirms is + # created with RLS off. The two surviving quotes pair up, so the guard below + # never fired either. + # values ('/api/*'); create table public.orders … alter table … enable row level … + # -> the same swallow ate to end of file, the remaining odd `'` read as an + # unterminated string, and a fully compliant migration went RED with a message + # naming a string that does not exist in the source. + # + # One scanner that understands strings, identifiers and comments together has no such + # seam. An unterminated block comment is UNKNOWN, like every other lost sync. + if (inblk) { + if (c == "*" && substr($0, i + 1, 1) == "/") { inblk = 0; i += 2; continue } + i++; continue + } + # A DOUBLE-QUOTED IDENTIFIER, possibly spanning lines. `""` inside it is one embedded + # quote. + if (inq) { + if (c == "\"") { + if (substr($0, i + 1, 1) == "\"") { qv = qv "\""; i += 2; continue } + inq = 0; i++; tok("Q", qv); qv = ""; continue + } + qv = qv c; i++; continue + } + # A SINGLE-QUOTED STRING. This state did not exist, and without it ONE `"` inside an + # ordinary string literal — an inch mark, a quoted word in prose — opened an identifier + # that ate every following statement. Measured: a file whose only `"` was in + # `values ('24" monitor')` and whose `create table` had no RLS anywhere reported `ok`, + # exit 0. A silent pass, introduced by the rewrite that was meant to end them. + if (ins) { + if (c == "'") { + if (substr($0, i + 1, 1) == "'") { sv = sv "'"; i += 2; continue } + ins = 0; i++ + # `create` then at most two modifier words then `table`, not "created" and "tables" + # anywhere in the same sentence — `values ('created three tables last week')` was a + # blocking UNKNOWN on an otherwise compliant file. + # TWO OPTIONAL GROUPS, which is the bound written in a form mawk compiles. `{0,2}` + # was the obvious spelling and mawk 1.3.4 — the awk on Debian/Ubuntu — miscompiles an + # interval with n>=2 applied to a group whose body starts with a `+`-quantified + # bracket: it matches ZERO repetitions, so the allowance was inert, the test was + # exactly `create table`, and `execute 'CREATE UNLOGGED TABLE …'` passed over. + # + # Replacing it with `*` fixed that and broke the other side: unbounded, so ordinary + # English prose matched. Measured on a compliant migration — + # `values ('To create a new monthly revenue table, run the report')` — UNKNOWN, exit + # 1, on a file nothing executes. Seeding help text was enough to block the branch. + # The bound is the point; only its spelling was wrong. + if (tolower(sv) ~ /(^|[^a-z])create[ \t\n]+([a-z]+[ \t\n]+)?([a-z]+[ \t\n]+)?table([^a-z]|$)/) ddl = 1 + sv = ""; continue + } + sv = sv c; i++; continue + } + # A DOLLAR-QUOTED BODY IS DATA, like any other string. Scanning it as SQL made + # `create function f() ... as $$ begin create table public.tmp (id int); end $$;` a FAIL + # naming public.tmp — a table that does not exist at definition time and is created only + # when the function runs. A red on a compliant file, naming a table nobody created. + # + # `do $$ ... $$` DOES execute immediately, so a create table in one is real — but the gate + # cannot see whether RLS follows it inside the body either, so the honest answer for both + # is the same as for `execute '...'`: UNKNOWN, which is still red and still blocks. What + # it must not do is name a table and claim a violation it has not established. + if (indq) { + if (substr($0, i, length(dqtag)) == dqtag) { i += length(dqtag); indq = 0 + if (tolower(dqv) ~ /(^|[^a-z])create[ \t\n]+([a-z]+[ \t\n]+)?([a-z]+[ \t\n]+)?table([^a-z]|$)/) dqddl = 1 + dqv = ""; continue } + dqv = dqv c; i++; continue + } + if (c == "$") { + j = i + 1 + while (j <= n && substr($0, j, 1) ~ /[A-Za-z0-9_]/) j++ + if (j <= n && substr($0, j, 1) == "$") { + dqtag = substr($0, i, j - i + 1); indq = 1; dqv = ""; i = j + 1; continue } - print out } - ' "$1" + if (c == " " || c == "\t" || c == "\r") { i++; continue } + if (c == "-" && substr($0, i + 1, 1) == "-") break + if (c == "/" && substr($0, i + 1, 1) == "*") { inblk = 1; i += 2; continue } + if (c == "\"") { inq = 1; qv = ""; i++; continue } + if (c == "'") { ins = 1; sv = ""; i++; continue } + if (c ~ /[A-Za-z_]/) { + v = "" + while (i <= n) { c = substr($0, i, 1); if (c !~ /[A-Za-z0-9_$]/) break; v = v c; i++ } + tok("W", tolower(v)); continue + } + if (c == ".") { tok("D", "."); i++; continue } + tok("P", c); i++ + } + if (indq) dqv = dqv "\n" + if (inq) qv = qv "\n" + if (ins) sv = sv "\n" } - -STRIPPED="$(mktemp)" -trap 'rm -f "$STRIPPED"' EXIT +END { + if (st == 3) emitpair("C") + # A final `delete from t` with no trailing semicolon is still WHERE-less, and the note said + # so before state 10 existed. State 10 waits for a terminator that end-of-input never sends. + if (st == 10) print "X\t\t" + # A SCANNER THAT LOST SYNC MUST NOT REPORT A CLEAN FILE. An unterminated quoted identifier + # or string means everything after it was read as something it is not, so the only honest + # answer about the rest of the file is that this gate could not read it. + if (indq) print "U\tunterminated-dollar-quoted-body\t" + if (dqddl) print "U\tddl-inside-a-dollar-quoted-body\t" + if (inblk) print "U\tunterminated-block-comment\t" + if (inq) print "U\tunterminated-quoted-identifier\t" + if (ins) print "U\tunterminated-string\t" + # DDL inside a string literal is executed by `execute`, and this scanner treats the string + # as data — the alternative, reading it, named tables nobody created and sent fixers to + # edit their data. Neither is a check, so it says so. + if (ddl) print "U\tddl-inside-a-string-literal\t" +} +SQLSCANAWK +WORK_TOK="$(mktemp)" +WORK_C="$(mktemp)" +WORK_R="$(mktemp)" +trap 'rm -f "$SQLSCAN" "$WORK_TOK" "$WORK_C" "$WORK_R"' EXIT for up in "$MIG"/*.sql; do [ -e "$up" ] || continue case "$up" in *.down.sql) continue;; esac down="${up%.sql}.down.sql" [ -f "$down" ] || fail "no rollback: $(basename "$up") needs $(basename "$down")" - strip_sql_comments "$up" > "$STRIPPED" - # tables created here must enable RLS here - while IFS= read -r tbl; do - if ! grep -qiE "alter\s+table\s+(if\s+exists\s+)?(public\.)?\"?${tbl}\"?\s+enable\s+row\s+level\s+security" "$STRIPPED"; then - fail "$(basename "$up"): table '$tbl' created without 'enable row level security' in the same file" - fi - done < <(grep -ioE "create\s+table\s+(if\s+not\s+exists\s+)?(public\.)?\"?[a-z_][a-z0-9_]*" "$STRIPPED" | sed -E 's/.*[ .]"?([a-z_][a-z0-9_]*)"?$/\1/i') - # destructive statements outside a WHERE are tier-3 by regex (non-negotiable 4); flag, do not block - if grep -qiE "^\s*(drop\s+table|truncate|delete\s+from\s+[a-z_.\"]+\s*;)" "$STRIPPED"; then + # TABLES CREATED HERE MUST ENABLE RLS HERE — DECIDED BY TOKENISING, NOT BY MATCHING. + # + # This check was a regex three times over and produced a finding in each of three + # consecutive review rounds, alternating direction every time: + # + # - `create table "public"."orders"` read `public` as the table. Where a table genuinely + # named `public` had RLS the file PASSED — a false green. + # - Widening the ALTER side to any schema while the create side discarded it made the + # check assert "SOME table called orders, in SOME schema, has RLS" under a message + # naming one table: `alter table archive.orders` satisfied `create table public.orders`. + # Another false green, added by the commit that removed the first one. + # - Carrying the pair fixed that and broke four compliant files instead: + # `mydb.public.orders`, `public."order items"`, `public."a.b"`, and `ALTER TABLE ONLY` + # (which is what pg_dump emits) — reds naming tables that do not exist. + # - And still open after all three: a quoted identifier lost its case, so + # `create table public."Orders"` was satisfied by RLS on `orders`, which PostgreSQL + # treats as a different table and which is what Prisma and Drizzle emit; a `""` inside + # a quoted name ended the name early; a name containing a regex metacharacter built a + # matcher wider than itself; a `create table` whose name sat on the NEXT line was not + # seen at all, because grep is line-scoped — a silent pass. + # + # Every one of those is the same defect: a pattern deciding a question that needs a parse. + # So the statements are tokenised once, both sides through the SAME scanner, and the two + # (schema, table) pairs are compared as STRINGS. There is no interpolated regex left to be + # wider than the name it was given, no case flag to fold a quoted identifier, and no line + # boundary to hide a statement behind. + : > "$WORK_C"; : > "$WORK_R" + awk -f "$SQLSCAN" "$up" > "$WORK_TOK" + + # ONE LINE PER RECORD, TAB-SEPARATED, VALUES ESCAPED. It was three lines per record, and a + # quoted identifier containing a newline then shifted every following triple: a file with + # `public."weird"` and two more non-compliant tables reported ONE violation, naming + # `public.we` — a table that does not exist — and did not report the other two at all. + # Escaping is exact for comparison, because both sides are escaped by the same function. + # + # COMPARED WITH grep, NOT A NESTED SHELL LOOP. The loop was O(created x rls): 2000 compliant + # tables took 33.5s, quadrupling per doubling. That is the same shape the awk accumulator + # had — the fix for which had only moved it from awk into bash, which is worth saying out + # loud rather than calling the quadratic bullet closed. + # `-a` ON EVERY grep THAT READS THESE RECORDS. Without it, one byte that is invalid in the + # ambient locale makes GNU grep declare the file binary and SUPPRESS the matching line while + # still exiting 0 — so the record never reaches $WORK_C and that table is never compared + # against RLS at all. Measured under LC_ALL=C.UTF-8, the locale on ubuntu-latest: a file + # creating `public.clean` and `public."año"` (LATIN1), neither with RLS, reported ONE + # violation where the previous revision reported two. A NUL byte does it in any locale. + # PostgreSQL 16 confirms the dropped table is real and its RLS is off. A silent pass chosen + # by an encoding, and it arrived with the change that made this comparison a grep. + set +e + grep -a '^C ' "$WORK_TOK" | cut -f2- > "$WORK_C" + grep -a '^R ' "$WORK_TOK" | cut -f2- > "$WORK_R" + set -e + + while IFS= read -r reason; do + [ -n "$reason" ] || continue + # THE REASON DECIDES THE SENTENCE. One message covered both kinds, and for a DDL string it + # was false: the scanner reads the statements after it perfectly well; what it does not + # read is the DDL inside the string. A message wider than the thing it describes is the + # defect this repository exists to catch. + case "$reason" in + ddl-inside-a-string-literal|ddl-inside-a-dollar-quoted-body) + unknown "$(basename "$up"): a string or dollar-quoted body here contains a 'create ... table'. This gate reads strings as data, so it cannot say what that statement creates or whether RLS follows it" ;; + *) + unknown "$(basename "$up"): $reason — this gate could not read the statements after it, and will not call the file clean" ;; + esac + done < <(grep -a '^U ' "$WORK_TOK" | cut -f2 || true) + + while IFS="$(printf '\t')" read -r sch tbl; do + [ -n "$tbl" ] || continue + fail "$(basename "$up"): table '$sch.$tbl' created without 'enable row level security' in the same file" + done < <(grep -aFxv -f "$WORK_R" -- "$WORK_C" || true) + + # destructive statements are tier-3 by note, not by block (non-negotiable 4). Emitted by + # the scanner from the token stream, so a commented-out `drop table` is a comment. + if grep -qa '^X ' "$WORK_TOK" 2>/dev/null; then echo "note [$GATE] $(basename "$up"): destructive statement present; this migration is tier-3" fi done diff --git a/gates/nextjs-env.sh b/gates/nextjs-env.sh index 9a74d12..57eb744 100755 --- a/gates/nextjs-env.sh +++ b/gates/nextjs-env.sh @@ -92,14 +92,14 @@ if in_git_repo "$ROOT"; then show_err "$WORK/err" else set +e - grep -E '(^|/)\.env(\..*)?$' "$WORK/tracked" > "$WORK/envcand" 2> "$WORK/err" + grep -aE '(^|/)\.env(\..*)?$' "$WORK/tracked" > "$WORK/envcand" 2> "$WORK/err" grc=$? set -e if [ "$grc" -gt 1 ]; then fail "filtering the tracked file list failed (grep exit $grc):"; show_err "$WORK/err" else set +e - grep -vE '\.example$' "$WORK/envcand" > "$WORK/env" 2> "$WORK/err" + grep -avE '\.example$' "$WORK/envcand" > "$WORK/env" 2> "$WORK/err" grc=$? set -e if [ "$grc" -gt 1 ]; then diff --git a/gates/service-role.sh b/gates/service-role.sh index 6d6ea00..2ab70dc 100755 --- a/gates/service-role.sh +++ b/gates/service-role.sh @@ -29,6 +29,164 @@ source "$(dirname "$0")/lib.sh" WORK="$(mktemp -d)" trap 'rm -rf "$WORK"' EXIT +# --------------------------------------------------------------------------- +# THE EXTRACTOR IS A TOKENISER, NOT A REGEX — and that is the fourth attempt at this code. +# +# `grep -o` matches NON-OVERLAPPING, so a string ending in the word `from` immediately before +# a quote consumed the rest of the line up to the next quote as one "specifier". Every +# version built on that had to choose which way to be wrong, and each of three consecutive +# review rounds found the choice: +# +# - report the invented span -> a blocking UNKNOWN naming an import that does not +# exist, on ordinary source: `Array.from(",")`, a regex +# literal, `{ note: "Imported from " }`. No action a +# fixer can take. +# - drop the invented span -> a REAL import swallowed into the span is dropped with +# it. Measured: `const label = "imported from "; +# import { admin } from "../lib/admin";` on one line +# went `ok`, exit 0, over a module reaching the secret. +# +# Both are the same defect, and neither is fixable by choosing a better filter, because the +# filter is downstream of the damage. So the scan tokenises: strings, template literals, +# line and block comments and regex literals are recognised as what they are, and a +# specifier is emitted only from a real `from`/`import`/`require` position. There is no +# invented span left to report or to drop, and the `flat` pass and its `//`-comment cost are +# gone with it. +# --------------------------------------------------------------------------- +SCAN="$WORK/scan.awk" +cat > "$SCAN" <<'SCANAWK' +# Streaming JavaScript scanner. One record per line: "S" for a literal import +# specifier, or "N" for a branch this gate cannot read. A string that merely ENDS in the word +# `from` produces nothing at all. +# +# LINE-INCREMENTAL, NOT SLURPED, for the reason recorded in the SQL scanner: `buf = buf $0 +# "\n"` is quadratic in mawk and took 12.1s on a 1.1MB generated types file against 0.11s for +# the greps it replaced. State is carried across lines instead. +# `}` IS NOT IN THIS SET, deliberately. It was, and `const x = {} / foo; import { admin } from +# "../lib/admin"` then read the division as the start of a regex literal and consumed the rest +# of the line — the real import with it. `ok`, exit 0, on a module reaching the secret, where +# the previous extractor caught it. A block close CAN precede a regex, so this trades a rare +# false red (a regex body scanned as code) for a false green, which is the trade this +# repository takes every time. +function isregexpos(c) { + return (c == "" || c == "(" || c == "," || c == "=" || c == ":" || c == "[" || c == "!" || + c == "&" || c == "|" || c == "?" || c == "{" || c == ";" || c == "+" || + c == "-" || c == "*" || c == "%" || c == "<" || c == ">" || c == "~" || c == "^" || + c == "k") +} +function want(w) { return (w == "from" || w == "import" || w == "require") } +# JSX text is not JavaScript: `

Copied from "a" to "b"

` puts `from` before a quote and +# nothing short of a JSX parser tells that from an import. A module specifier carries none of +# these characters. Dropping such a candidate is only safe because the extractor no longer +# invents spans — see the note in the gate. +function plausible(v) { return (v != "" && v !~ /[<>{}]/) } +# CLEARING A PENDING import(/require( IS THE REPORT. The first version decided this with a +# lookahead on the rest of the line, which cannot see a `import(` whose argument is on the NEXT +# line — the exact multi-line dynamic import that was a false green two releases ago, silently +# reintroduced. In a token stream the rule is simply: a pending import( closed by anything that +# is not a literal is a branch this gate cannot follow, whatever line that token is on. +function clearpend() { + if (paren == 1 && (pend == "import" || pend == "require")) print "N" + pend = ""; paren = 0 +} +{ + n = length($0); i = 1 + while (i <= n) { + c = substr($0, i, 1) + if (mode == 1) { # inside /* */ + if (c == "*" && substr($0, i + 1, 1) == "/") { mode = 0; i += 2; continue } + i++; continue + } + if (mode == 2) { # inside a template literal + if (c == "\\") { tv = tv substr($0, i + 1, 1); i += 2; continue } + if (c == "$" && substr($0, i + 1, 1) == "{") { + interp = 1; tdepth = 1; i += 2 + while (i <= n && tdepth > 0) { + c = substr($0, i, 1) + if (c == "{") tdepth++ + else if (c == "}") tdepth-- + i++ + } + continue + } + if (c == "`") { + mode = 0; i++ + if (want(pend)) { if (interp) print "N"; else if (plausible(tv)) print "S" tv } + pend = ""; paren = 0; last = "v"; tv = ""; interp = 0 + continue + } + tv = tv c; i++; continue + } + if (c == " " || c == "\t" || c == "\r") { i++; continue } + if (c == "/" && substr($0, i + 1, 1) == "/") break # rest of THIS line + if (c == "/" && substr($0, i + 1, 1) == "*") { mode = 1; i += 2; continue } + if (c == "/" && isregexpos(last)) { + i++; incls = 0 + while (i <= n) { + c = substr($0, i, 1) + if (c == "\\") { i += 2; continue } + if (c == "[") incls = 1 + else if (c == "]") incls = 0 + else if (c == "/" && !incls) { i++; break } + i++ + } + last = "v"; clearpend(); continue + } + if (c == "\"" || c == "'") { + # Line-bounded on purpose. An apostrophe in JSX text would otherwise open a string that + # runs to the end of the file; ending it at the line boundary costs nothing, because a + # real specifier never spans lines. + q = c; i++; v = "" + while (i <= n) { + c = substr($0, i, 1) + if (c == "\\") { v = v substr($0, i + 1, 1); i += 2; continue } + if (c == q) { i++; break } + v = v c; i++ + } + if (want(pend) && plausible(v)) print "S" v + pend = ""; paren = 0; last = "v"; continue + } + if (c == "`") { mode = 2; tv = ""; interp = 0; i++; continue } + if (c ~ /[A-Za-z_$]/) { + v = "" + while (i <= n) { c = substr($0, i, 1); if (c !~ /[A-Za-z0-9_$]/) break; v = v c; i++ } + # `.from` is a method name, not a keyword: `Array.from(",")` put the string after it in + # the specifier slot. But `module.require()` IS a module loader — dropping every dotted + # `require` lost it, and a page calling `module.require("../lib/admin")` on a module + # holding the secret reported ok, exit 0, where the previous extractor caught it. + if (want(v) && last != ".") { clearpend(); pend = v; paren = 0 } + else if (v == "require" && last == "." && prevword == "module") { clearpend(); pend = v; paren = 0 } + else clearpend() + prevword = v + last = (v == "return" || v == "typeof" || v == "case" || v == "in" || v == "of" || + v == "new" || v == "delete" || v == "void" || v == "do" || v == "else" || + v == "yield" || v == "await") ? "k" : "w" + continue + } + if (c == "(") { + # `from` is never followed by `(` in an import — `import x from "y"` has no parens. + if (pend == "from") { clearpend(); last = "("; i++; continue } + if (pend != "" && paren == 0) paren = 1 + else clearpend() + last = "("; i++; continue + } + clearpend(); last = c; i++ + } + if (mode == 2) tv = tv "\n" +} +END { + # A SCANNER THAT LOST SYNC MUST NOT REPORT A CLEAN FILE. A template literal or block comment + # still open at end of file means everything after it was read as something it is not. + # Measured before this guard: one stray backtick in JSX text (`

Press the ` key

`) + # consumed the rest of the file and a `require("@/lib/admin")` below it reported `ok`, + # exit 0, over a module reaching the secret. + if (mode == 2 || mode == 1) print "N" + # A pending import( still open at end of file is the same report as one closed by a + # non-literal: the argument was never a literal this gate could read. + clearpend() +} +SCANAWK + # --------------------------------------------------------------------------- # Applicability. This gate is about Next.js request-path modules; run against a tree that # is not a Next.js app it has nothing to walk. Saying "ok" over an empty walk is the false @@ -38,7 +196,7 @@ trap 'rm -rf "$WORK"' EXIT # and failing to find it means this gate does not understand the layout. # --------------------------------------------------------------------------- PKG="$ROOT/package.json" -if [ ! -f "$PKG" ] || ! grep -qE '"next"[[:space:]]*:' "$PKG"; then +if [ ! -f "$PKG" ] || ! grep -qaE '"next"[[:space:]]*:' "$PKG"; then echo "not a Next.js app (no package.json depending on \"next\") — no request path to walk" finish fi @@ -88,14 +246,69 @@ fi # `@/*` key got a red with no reason in it, which reads as a broken gate rather than as the # gate saying anything. Every command substitution here ends in `|| true` for that reason. # --------------------------------------------------------------------------- -TSCONFIG="$ROOT/tsconfig.json" +TSCONFIG_RAW="$ROOT/tsconfig.json" +# tsconfig.json permits comments, and every read below has to see past them. `grep -q +# '"baseUrl"'` armed the baseUrl fallback on a COMMENTED-OUT key: a config carrying +# `// "baseUrl": ".",` and no live one made `import React from "react"` resolve to a repo +# directory named react/ and reported a violation against the package. A commented-out line +# is not configuration. +TSCONFIG="$WORK/tsconfig.json" +if [ -f "$TSCONFIG_RAW" ]; then + # Line-incremental, like the two token scanners. It slurped, and the README bullet that + # disowns slurping sat two hundred lines above one still doing it. + awk ' + { + n = length($0); i = 1 + while (i <= n) { + c = substr($0, i, 1) + if (blk) { + if (c == "*" && substr($0, i + 1, 1) == "/") { blk = 0; i += 2; continue } + i++; continue + } + if (c == "\"") { + printf "%s", c; i++ + while (i <= n) { + c = substr($0, i, 1) + if (c == "\\") { printf "%s", substr($0, i, 2); i += 2; continue } + printf "%s", c; i++ + if (c == "\"") break + } + continue + } + if (c == "/" && substr($0, i + 1, 1) == "/") break + if (c == "/" && substr($0, i + 1, 1) == "*") { blk = 1; i += 2; continue } + printf "%s", c; i++ + } + printf "\n" + } + ' "$TSCONFIG_RAW" > "$TSCONFIG" 2>/dev/null || cp -- "$TSCONFIG_RAW" "$TSCONFIG" 2>/dev/null || : +fi +# Named in the UNKNOWN below only when it is actually there. The message said "no alias +# declared in tsconfig.json" on a tree with no tsconfig.json at all, which sends a reader +# to open a file that does not exist. +if [ -f "$TSCONFIG" ]; then TSCONFIG_NOTE=" in tsconfig.json"; else TSCONFIG_NOTE=" (no tsconfig.json in this tree)"; fi ALIASES="$WORK/aliases" : > "$ALIASES" BASE_DIR="$ROOT" +BASEURL_SET=0 if [ -f "$TSCONFIG" ]; then base_url="$(sed -nE 's/.*"baseUrl"[[:space:]]*:[[:space:]]*"([^"]*)".*/\1/p' "$TSCONFIG" 2>/dev/null | head -1 || true)" base_url="${base_url#./}"; base_url="${base_url%/}" if [ -n "$base_url" ] && [ "$base_url" != "." ]; then BASE_DIR="$ROOT/$base_url"; fi + # ARMED ON THE KEY'S PRESENCE, NOT ON ITS VALUE. The first version of the baseUrl fallback + # armed on `[ -n "$base_url" ] && [ "$base_url" != "." ]` — the same condition that decides + # whether BASE_DIR moves — and `"baseUrl": "."` is the spelling in Next.js's own Absolute + # Imports documentation and the one create-next-app ships. So the fallback did not arm on + # the commonest spelling, and the false green it was written to close stayed open there: + # + # import { admin } from "lib/supabase-admin"; -> ok exit 0 + # import { admin } from "../lib/supabase-admin"; -> FAIL exit 1 + # + # Identical to the reproduction in the commit that claimed to fix it, on a different value + # of the same key — and the shipped fixture used "src", so the selftest was green over the + # half that worked. BASE_DIR is already $ROOT when the value is "." or "./", so nothing else + # needs to change: the key being there is the whole condition. + if grep -qa '"baseUrl"' "$TSCONFIG" 2>/dev/null; then BASEURL_SET=1; fi # The `paths` object, isolated exactly rather than read line by line. # @@ -125,9 +338,15 @@ if [ -f "$TSCONFIG" ]; then # "": [ "" — extracted by shape, from anywhere in the alias body, so the # same code reads a pretty-printed tsconfig and a minified one. - grep -oE '"[^"]+"[[:space:]]*:[[:space:]]*\[[[:space:]]*"[^"]+"' "$WORK/pathsbody" 2>/dev/null \ + grep -aoE '"[^"]+"[[:space:]]*:[[:space:]]*\[[[:space:]]*"[^"]+"' "$WORK/pathsbody" 2>/dev/null \ | sed -E 's/"([^"]+)"[[:space:]]*:[[:space:]]*\[[[:space:]]*"([^"]+)"/\1\t\2/' > "$ALIASES" 2>/dev/null || true + # `extends` is not followed. A base config holding the aliases leaves $ALIASES empty, and an + # `@/lib/secret` then matches no alias, is called a published package, and is skipped — a + # false green. Following the chain is a real change; saying so is not. + if grep -qa '"extends"' "$TSCONFIG" 2>/dev/null && [ ! -s "$ALIASES" ]; then + unknown "tsconfig.json uses \"extends\" and no alias was parsed from this file — the base config is not followed, so an aliased import here would be mistaken for a published package" + fi if [ -s "$WORK/pathsbody" ] && [ ! -s "$ALIASES" ]; then unknown "tsconfig.json declares compilerOptions.paths but this gate parsed no alias out of it — every aliased import below is therefore unresolved, and none of them will be called clean" fi @@ -183,7 +402,7 @@ QUEUE="$WORK/queue"; SEEN="$WORK/seen"; EDGES="$WORK/edges" # on. Reachability is a property of the whole graph; it cannot be accumulated by a traversal # that visits each node once. enqueue() { # $1 = absolute file to walk - grep -Fxq -- "$1" "$SEEN" 2>/dev/null && return 0 + grep -aFxq -- "$1" "$SEEN" 2>/dev/null && return 0 printf '%s\n' "$1" >> "$SEEN" printf '%s\n' "$1" >> "$QUEUE" } @@ -198,7 +417,7 @@ seeds_of() { # $1 = file while [ -s "$frontier" ]; do awk -F'\t' 'NR==FNR { want[$0]; next } ($2 in want) { print $1 }' "$frontier" "$EDGES" | sort -u > "$nxt" if [ -s "$nxt" ]; then - grep -Fxv -f "$vis" -- "$nxt" > "$nxt.new" 2>/dev/null || : > "$nxt.new" + grep -aFxv -f "$vis" -- "$nxt" > "$nxt.new" 2>/dev/null || : > "$nxt.new" mv "$nxt.new" "$nxt" fi [ -s "$nxt" ] || break @@ -206,7 +425,7 @@ seeds_of() { # $1 = file mv "$nxt" "$frontier" done [ -s "$SEEDS" ] || return 0 - grep -Fxf "$SEEDS" -- "$vis" 2>/dev/null | sort -u + grep -aFxf "$SEEDS" -- "$vis" 2>/dev/null | sort -u } rel() { printf '%s' "${1#"$ROOT"/}"; } @@ -225,8 +444,34 @@ done < "$SEEDS" # The three outcomes are the whole point. 1 is a claim — "this is a published package, not # this repo's source" — and it is only safe to make about a specifier that matches NO # declared alias. Anything that looks local and does not resolve is 2, never 1. +# "It matched no declared alias" and "it is a published package" are not the same sentence, +# and resolve() used to print the second while only having checked the first. `@/lib/secret` +# in a tree whose tsconfig declares some OTHER alias matches nothing here — and it cannot be +# a package either: npm has no empty scope. The gate called it a dependency and skipped it, +# so a request-path module importing it read as clean. Measured on a tree reaching +# SUPABASE_SERVICE_ROLE_KEY: `ok [service-role]`, exit 0. A FALSE GREEN, from a classifier +# whose claim was wider than its test — this repository's recurring defect, again, in the +# branch that decides what is out of scope. +# +# So the claim is now tested. A specifier that is neither a declared alias nor a well-formed +# package name is UNKNOWN: this gate does not know what it is, and will not call it clean. +is_package_specifier() { + local sc nm + case "$1" in + @*/*) + sc="${1#@}"; sc="${sc%%/*}" + case "$sc" in ""|[!A-Za-z0-9]*) return 1 ;; esac + nm="${1#@*/}" + case "$nm" in ""|[!A-Za-z0-9]*) return 1 ;; esac + return 0 ;; + @*) return 1 ;; # a scope with no package under it + [A-Za-z0-9]*) return 0 ;; # react, node:fs, @-less subpaths + *) return 1 ;; # ~/..., #internal/..., ./ and ../ never reach here + esac +} + resolve() { # $1 = specifier, $2 = importing file - local spec="$1" bases="" matched=0 key target prefix t b ext cand found="" + local spec="$1" bases="" matched=0 fallback=0 key target prefix t b bb cands ext cand cdir found="" case "$spec" in ./*|../*) bases="$(dirname -- "$2")/$spec"; matched=1 ;; /*) bases="$ROOT$spec"; matched=1 ;; @@ -255,18 +500,150 @@ $BASE_DIR/$t" ;; esac done < "$ALIASES" + # `baseUrl` WITHOUT A MATCHING `paths` ENTRY IS STILL A LOCAL IMPORT. This is Next.js's + # documented "Absolute Imports" shape: `baseUrl: "src"` alone makes `lib/supabase-admin` + # mean `src/lib/supabase-admin.ts`, with no alias declared anywhere. This gate PARSED + # that baseUrl — it is `BASE_DIR` for every alias target above — and then skipped the + # bare specifier as a published package. Measured, same file and same secret, twice: + # + # import { admin } from "lib/supabase-admin"; -> ok [service-role] exit 0 + # import { admin } from "../lib/supabase-admin"; -> FAIL x2 exit 1 + # + # A FALSE GREEN selected by nothing but the spelling of the import. TypeScript resolves + # baseUrl-relative first and falls back to node_modules, so this does the same: probe + # under BASE_DIR, and if nothing is there the specifier really is a package (rc 1, not a + # red). Only when an explicit baseUrl was declared — without one there is no such shape + # to resolve, and every bare specifier is a dependency exactly as before. + # NOT GATED ON THE NAME LOOKING LIKE A PACKAGE. It was, and that made the probe skip + # exactly the specifiers most likely to be baseUrl-relative: `_components/Button` — a + # leading underscore is not a valid npm name, and underscore-prefixed private folders + # are an ordinary Next.js convention — resolved to a real file under BASE_DIR and was + # reported UNKNOWN anyway. A probe that refuses to look at a path because the path is + # not a package name is answering a different question from the one it was asked. + # + # `#`-prefixed specifiers are the exception and stay out: TypeScript and Node resolve + # those through package.json `imports`, not by appending them to baseUrl, so probing + # BASE_DIR for one would be a guess dressed as a resolution. + case "$spec" in + '#'*) : ;; + *) + if [ "$matched" -ne 1 ] && [ "$BASEURL_SET" = 1 ]; then + matched=1; fallback=1 + bases="$BASE_DIR/$spec" + fi + ;; + esac ;; esac - if [ "$matched" -ne 1 ]; then return 1; fi + if [ "$matched" -ne 1 ]; then + if is_package_specifier "$spec"; then return 1; fi + return 3 + fi while IFS= read -r b; do [ -n "$b" ] || continue - for ext in "" .ts .tsx .js .jsx .mjs .cjs /index.ts /index.tsx /index.js /index.jsx; do - cand="$b$ext" - if [ -f "$cand" ]; then found="$cand"; break; fi + # THE SPECIFIER'S EXTENSION IS THE EMITTED ONE, NOT THE SOURCE'S. Under + # `moduleResolution: nodenext` (and with `verbatimModuleSyntax`) TypeScript requires the + # import to name the file JavaScript will load — `./mod.mjs` — while the file on disk is + # `mod.mts`. The probe below only ever appended extensions, so it tested `mod.mjs.mts` + # and nothing else, resolved to no file, and reported UNKNOWN. A FALSE RED on the modern + # default resolution mode: the gate blocking a tree it simply could not spell. + # + # The mapping is TypeScript's own and is one-to-one: .mjs<-.mts, .cjs<-.cts, .js<-.ts|.tsx, + # .jsx<-.tsx. The emitted spelling is kept in the list as well, because a plain JS project + # has the .js on disk and both must resolve. + # THE SOURCE COMES FIRST, AND THE DECLARATION FILES ARE IN THE LIST. Verified with tsc: + # with both `admin.mts` and a stale emitted `admin.mjs` beside it, `import "./admin.mjs"` + # resolves to the .mts — so probing the emitted spelling first reads the stale artefact, + # and a secret added to the source reads as `ok`, exit 0. And `import type { X } from + # "./types.js"` against a `types.d.ts` type-checks clean under nodenext, which this list + # has to know or it re-opens the very `.d.ts` false red the extension list was widened to + # close one release ago. + # + # AND THE DECLARATION COMES LAST, which the first version of this list got backwards. With + # a `.js` module and a hand-written `.d.ts` beside it, probing the declaration first + # resolved to a file that BY CONSTRUCTION cannot hold a secret, and the module Node + # actually loads was never read. Measured: `lib/admin.js` reaching + # SUPABASE_SERVICE_ROLE_KEY with a `lib/admin.d.ts` next to it gave `ok`, exit 0 — and + # deleting the .d.ts turned the same tree red, which is the sidecar doing the hiding. + # `tsc --traceResolution` does prefer the declaration, but that is TypeScript answering + # "where are the types"; this gate asks "what code runs in the request path", and a + # declaration file is never that answer. Last still closes the false red above, because + # that case has no implementation file to find. + cands="$b" + case "$b" in + *.mjs) cands="${b%.mjs}.mts +$b +${b%.mjs}.d.mts" ;; + *.cjs) cands="${b%.cjs}.cts +$b +${b%.cjs}.d.cts" ;; + *.jsx) cands="${b%.jsx}.tsx +$b" ;; + *.js) cands="${b%.js}.ts +${b%.js}.tsx +$b +${b%.js}.d.ts" ;; + esac + while IFS= read -r bb; do + [ -n "$bb" ] || continue + # .d.ts and friends included: `import type { Database } from "@/types/supabase"` against a + # src/types/supabase.d.ts resolved to nothing, which this gate calls UNKNOWN — a permanent + # blocking red on a perfectly ordinary line. The selftest cannot catch a false red (its own + # note says the fixture model holds bad trees only), so it is fixed here on report. + # Implementations before declarations here too, and for the same reason as the `cands` + # note above: `.d.ts` sat ahead of `.js`, so an extensionless `"../lib/admin"` against a + # `lib/admin.js` with a `lib/admin.d.ts` beside it resolved to the declaration. That one + # is older than this release; the fix for the ordering above is the fix for this. + for ext in "" .ts .tsx .mts .cts .js .jsx .mjs .cjs .d.ts .d.mts .d.cts /index.ts /index.tsx /index.js /index.jsx /index.d.ts; do + cand="$bb$ext" + # CANONICALISED, not merely tested for existence. Without this the resolved path keeps + # whatever `..` the importer's specifier put in it, and TWO SPELLINGS OF ONE FILE ARE + # TWO NODES. That cost two defects, both shipped in v1.1.0: + # + # - a file imported as `@/lib/x` from one module and `../../lib/x` from another was + # counted twice: two findings for one file, each claiming "1 request-path + # module(s)". That is the very miscount the note above says the edge-list rewrite + # fixed. The rewrite fixed seed-carrying. It did not fix this, so the note claimed + # more than the fix delivered — in the comment about that exact defect. + # - an ordinary circular import (a.ts <-> b.ts) grew a longer spelling every hop, so + # the SEEN set never matched and the walk did not terminate. Measured at 20s with + # ZERO output before a timeout killed it. In CI that is a hang, not a red, and a + # hang is the one outcome that reports nothing at all. + # + # `cd` + `pwd -P` resolves `..` and symlinks both, and only works on a path that + # exists — which is why it runs after the -f test rather than as a string rewrite. + if [ -f "$cand" ]; then + cdir="$(cd -- "$(dirname -- "$cand")" 2>/dev/null && pwd -P)" || continue + found="$found$cdir/$(basename -- "$cand") +" + break + fi done - if [ -n "$found" ]; then break; fi + done <<< "$cands" done <<< "$bases" + # EVERY CANDIDATE THAT EXISTS, NOT THE FIRST ONE. Ordering the candidate list was the wrong + # tool for this and it could only ever be wrong in one direction: + # + # - source before emitted, so a stale `admin.mjs` beside its `admin.mts` cannot mask a + # secret added to the source (a real finding, fixed that way); + # - but in a JAVASCRIPT project `require("../lib/admin.js")` means admin.js, and putting + # the `.ts` substitution first scanned a clean `admin.ts` while `admin.js` — the module + # Node actually loads, holding SUPABASE_SERVICE_ROLE_KEY — was never read. `ok`, exit 0. + # + # Whichever candidate is second gets skipped, and either way round that is a false green. + # So both are walked. The cost is at most a module read that Node would not have loaded — + # a false red, and the direction this toolkit errs in; the benefit is that no ordering + # heuristic has to be right about a project type this gate cannot reliably detect. if [ -n "$found" ]; then printf '%s' "$found"; return 0; fi + # A baseUrl probe that found nothing is TypeScript's own fallthrough to node_modules — but + # only for a name that could BE a package. Reporting UNKNOWN for every miss would turn + # `import React from "react"` red in any repo declaring a baseUrl; calling every miss a + # package would silently skip `_components/Button` when that file is simply absent. So the + # claim is tested here, exactly as it is for a specifier that never reached the probe. + if [ "$fallback" = 1 ]; then + if is_package_specifier "$spec"; then return 1; fi + return 3 + fi return 2 } @@ -290,14 +667,35 @@ while :; do file="$(sed -n "${n}p" "$QUEUE")" [ -z "$file" ] && break - # A branch of the graph this gate cannot follow. Reported here, against the module that - # contains it, rather than assumed harmless. - if grep -qE '(^|[^A-Za-z0-9_$.])(import|require)[[:space:]]*\([[:space:]]*[^'"'"'")[:space:]]' -- "$file" 2>/dev/null; then + # One tokenised pass. `N` is a branch this gate cannot follow — an import() or require() + # whose argument is not a literal — reported against the module that contains it rather + # than assumed harmless. `S` is a literal specifier from a real import position. + set +e + recs="$(awk -f "$SCAN" -- "$file" 2>/dev/null)" + scan_rc=$? + set -e + # A TOKENISER THAT FAILED READ NOTHING, and reading nothing is not reading a clean file. + # Without this the walk discarded awk's status, found no records, added no edges and raised + # no UNKNOWN — so every module reachable only through this one dropped out of the graph + # silently. The gate would then report on a smaller tree than the one it was given. + if [ "$scan_rc" -ne 0 ]; then + unknown "$(rel "$file") could not be tokenised (awk exited $scan_rc) — an unread module is not a clean one" + continue + fi + nonliteral=0 + : > "$WORK/specs" + while IFS= read -r rec; do + case "$rec" in + "") continue ;; + N) nonliteral=1 ;; + S*) printf '%s\n' "${rec#S}" >> "$WORK/specs" ;; + esac + done <<< "$recs" + if [ "$nonliteral" = 1 ]; then unknown "$(rel "$file") contains a non-literal import() or require() — this gate cannot tell what it pulls in, so it will not call this path clean" fi - set +e - specs="$(grep -oE "(from|import|require)[[:space:]]*\(?[[:space:]]*['\"][^'\"]+['\"]" -- "$file" 2>/dev/null | sed -E "s/.*['\"]([^'\"]+)['\"]\$/\1/")" + specs="$(sort -u "$WORK/specs" 2>/dev/null)" set -e while IFS= read -r spec; do [ -n "$spec" ] || continue @@ -306,9 +704,16 @@ while :; do rc=$? set -e case "$rc" in - 0) printf '%s\t%s\n' "$file" "$target" >> "$EDGES"; enqueue "$target" ;; + 0) # resolve() may name MORE THAN ONE file — see its note: when both an explicitly + # imported `.js` and a same-named `.ts` exist on disk, either could be the module + # that runs, so both are edges and both are walked. + while IFS= read -r one; do + [ -n "$one" ] || continue + printf '%s\t%s\n' "$file" "$one" >> "$EDGES"; enqueue "$one" + done <<< "$target" ;; 1) : ;; # bare specifier: a published package, out of this gate's reach by design 2) unknown "$(rel "$file") imports '$spec', which this gate could not resolve to a file — an unread module is not a clean one" ;; + 3) unknown "$(rel "$file") imports '$spec', which matches no path alias this gate could read$TSCONFIG_NOTE and is not a well-formed package name — this gate cannot say what it is, and will not call it a dependency to skip it" ;; esac done <<< "$specs" done @@ -327,7 +732,16 @@ while IFS= read -r file; do while IFS= read -r term; do [ -n "$term" ] || continue set +e - hits="$(grep -nF -e "$term" -- "$file" 2>"$WORK/err")" + # `-a`, BECAUSE ONE BYTE MUST NOT HIDE THE SECRET. Without it a NUL anywhere in a module + # makes grep call the file binary, print nothing, and EXIT 0 with a note on stderr. + # Measured: a module holding SUPABASE_SERVICE_ROLE_KEY and a single NUL byte, reached from + # a page, reported `ok [service-role]`, exit 0. + # + # EXIT 0, not 1 — this comment said 1 and was wrong, and the error matters: the + # `grc -gt 1` branch below is the one a reader would expect to have caught this, and it + # would not have. grc was 0, hits was empty, and the emptiness check swallowed it. A + # wrong account of a mechanism is how the next person looks in the wrong place. + hits="$(grep -anF -e "$term" -- "$file" 2>"$WORK/err")" grc=$? set -e if [ "$grc" -gt 1 ]; then diff --git a/selftest.sh b/selftest.sh index 7e25c4a..d6ea52f 100755 --- a/selftest.sh +++ b/selftest.sh @@ -10,11 +10,14 @@ # A gate with no fixture fails this test. check_secrets.py's failure leg is its own # --selftest instead: canary, fingerprint and negative-probe checks that assert the # scanner still detects and still redacts. -# 2. the CASE leg — run it against every fixtures//cases/ and require exit 1 -# from each. The bad fixture is one tree holding many violations, so it proves only -# that SOMETHING in it fails; a shape that stopped being detected hides behind the -# others still failing. A case is one tree holding one shape, so it can only pass by -# that shape still being caught. Every false green a reviewer finds gets a case here. +# 2. the CASE leg — run it against every fixtures//bad/cases/ and require exit 1 +# AND a FAIL line from each (for the shell gates; see the marker note beside the loop). +# The bad fixture is one tree holding many violations, so it proves only that SOMETHING +# in it fails; a shape that stopped being detected hides behind the others still failing. +# A case is one tree holding one shape, so it can only pass by that shape still being +# caught. The FAIL line is half of that: UNKNOWN is also red, so without it a gate that +# lost the shape and merely tripped over a blind spot on the same tree still "passed". +# Every false green a reviewer finds gets a case here. # 3. the UNKNOWN leg — run it against every fixtures//bad/unknown/ and require # a RED exit that carries an UNKNOWN line and no FAIL line. "Could not check" must be # as blocking as "found a violation" and must not be mistakable for one. @@ -160,6 +163,38 @@ for g in "$HERE"/gates/*.sh "$HERE"/gates/*.py; do fi fixture="$FX/$gate/bad" + # THE VIOLATION MARKER IS A PROPERTY OF THE GATE, NOT OF THE HARNESS. `FAIL [` and + # `UNKNOWN [` come from `fail()` and `unknown()` in gates/lib.sh — the SHELL gates' helpers. + # The two vendored Python gates print their own lines (`references: N broken reference(s)`, + # `secrets: N possible credential(s) committed`) and cannot emit either marker, so asserting + # the marker for them would report a gate that detected its planted shape perfectly as + # having a blind spot. That is an assertion BROADER than its message, inside the leg written + # to catch assertions narrower than theirs. + # + # MEASURED, not reasoned. A fixture citing a missing path was planted at + # fixtures/check_references/bad/cases/broken-path/ and run: the gate found it exactly + # ("references: 1 broken reference(s)"), and the unguarded harness answered "went red on + # case 'broken-path' without printing a FAIL line — the red is a blind spot, not the planted + # violation". With the guard below it reports `ok check_references catches case + # 'broken-path'`, which is what happened. + # + # The fixture is not kept, and the reason is a second finding rather than a tidy-up: unlike + # every shell gate, the vendored check_references.py has NO fixtures/ filter, so the planted + # citation is validated as though it were real and the TREE leg goes red on it. No Python + # gate can carry a cases/ fixture until that changes — and changing it means editing a + # vendored file, which is a policy decision and not this file's to make. So this guard is + # correct and currently unexercised, and that is stated here rather than left to look like + # dead code. This file's own header invites the fixture ("Every false green a reviewer finds + # gets a case here"); today the invitation cannot be accepted for these two gates. + # + # Losing the marker check for them costs nothing it was buying: the marker exists to tell a + # violation from an UNKNOWN, and a gate with no UNKNOWN outcome has nothing to confuse. For + # those gates a red is a violation by construction, and the exit code is the whole claim. + case "$name" in + *.sh) markers=1 ;; + *) markers=0 ;; + esac + # ---- 1. the failure leg ---- if [ "$mode" = selftest ]; then # Its own probes are the fixture. Run from a staging directory holding BOTH vendored @@ -209,12 +244,22 @@ for g in "$HERE"/gates/*.sh "$HERE"/gates/*.py; do cname="$(basename "$c")" run_gate "$g" "${c%/}" rc=$? + # THE RED IS NOT ENOUGH, and this leg used to accept it. A case fixture plants one + # violation and the claim printed is "catches case ''" — but UNKNOWN is also red, + # so a gate that lost the ability to see the planted shape and merely tripped over a + # blind spot on the same tree passed this leg while the shape went undetected. That is + # an assertion narrower than its own message, in the code written to catch exactly + # that. The UNKNOWN leg below already asserts both halves; this one now does too. + n_fail="$(grep -c '^FAIL \[' "$OUT" || true)" if [ "$rc" -eq 0 ]; then echo "SELFTEST FAIL: $gate PASSED case '$cname' — that shape is no longer detected" sed 's/^/ /' "$OUT"; bad=1 elif [ "$rc" -ne 1 ]; then echo "SELFTEST FAIL: $gate errored (exit $rc) on case '$cname' instead of reporting a violation" sed 's/^/ /' "$OUT"; bad=1 + elif [ "$markers" = 1 ] && [ "$n_fail" -eq 0 ]; then + echo "SELFTEST FAIL: $gate went red on case '$cname' without printing a FAIL line — the red is a blind spot, not the planted violation" + sed 's/^/ /' "$OUT"; bad=1 else echo "ok $gate catches case '$cname'" fi @@ -245,10 +290,10 @@ for g in "$HERE"/gates/*.sh "$HERE"/gates/*.py; do if [ "$rc" -eq 0 ]; then echo "SELFTEST FAIL: $gate went GREEN on unknown case '$uname' — it could not check and said so with a pass" sed 's/^/ /' "$OUT"; bad=1 - elif [ "$n_unknown" -eq 0 ]; then + elif [ "$markers" = 1 ] && [ "$n_unknown" -eq 0 ]; then echo "SELFTEST FAIL: $gate went red on unknown case '$uname' without printing an UNKNOWN line — the reader cannot tell a blind spot from a violation" sed 's/^/ /' "$OUT"; bad=1 - elif [ "$n_fail" -gt 0 ]; then + elif [ "$markers" = 1 ] && [ "$n_fail" -gt 0 ]; then echo "SELFTEST FAIL: $gate reported $n_fail violation(s) on unknown case '$uname', which plants none — it is blaming the tree for its own blind spot" sed 's/^/ /' "$OUT"; bad=1 else