Skip to content

fix(#574): stop handing the RLS-bypassing service-role key to both required checks - #577

Merged
TortoiseWolfe merged 1 commit into
mainfrom
fix/574-secrets-on-required-checks
Aug 6, 2026
Merged

fix(#574): stop handing the RLS-bypassing service-role key to both required checks#577
TortoiseWolfe merged 1 commit into
mainfrom
fix/574-secrets-on-required-checks

Conversation

@TortoiseWolfe

Copy link
Copy Markdown
Owner

Closes #574.

ci.yml and accessibility.yml — the two workflows that run on every PR,
and the only two that can block a merge — were each handed
SUPABASE_SERVICE_ROLE_KEY and TEST_USER_PRIMARY_PASSWORD. Neither can use
them.

  • tests/setup.ts:236-245 mocks @/lib/supabase/{client,server} globally
  • vitest.config.ts excludes every test that would need a real backend, with
    reasons that say so outright — "requires service role key", "requires live
    Supabase with admin RPC functions"

So the credential sat in the environment of the highest-frequency workflow in
the repo, for its entire history, with nothing able to reach it.

Why this one matters

The service-role key bypasses RLS entirely. It is the credential that #281,
#265 and the whole conformance suite exist to make unnecessary. Every PR run
materialised it in a runner environment where any package in the install graph
could read it from process.env.

Fork PRs were never the exposure — GitHub withholds secrets from those — which
means fork isolation was the only thing limiting it.

Verified by deletion, not by reading the config

"Nothing uses it" is exactly the kind of claim that turns out to be wrong in one
forgotten file, so I ran it both ways with the variables unset:

vitest, both unset          ->  395 files, 4282 tests, 0 failures
production build, both unset ->  succeeds; 121 <script src="*.css"> stripped,
                                 service worker stamped

validateSupabaseConfig() only warns on a missing service-role key
(src/config/payment.ts:59-63) and is guarded by typeof window === 'undefined',
so the build never depended on it either.

Scope

TEST_USER_PRIMARY_EMAIL goes too — a public var rather than a secret, but a
test-user email with no password is orphaned config.

conformance.yml, signup-mailer.yml and e2e.yml keep the key. They drive
real backends and genuinely need it. This is deliberately not a sweep.

After merge

Rotate SUPABASE_SERVICE_ROLE_KEY and TEST_USER_PRIMARY_PASSWORD. They
have been present in these jobs since the workflows were written; removing the
reference does not un-expose what already ran.

Expected CI

This touches .github/**, which is deliberately not in paths-ignore, so it
triggers E2E — and E2E will be red because Supabase is returning HTTP 402
(#567), not because of this diff. The two required checks are the real signal
here: if either goes red, that means something in them was reaching a live
backend from a job that claims to be fully mocked, which would be a more
interesting finding than this one.

🤖 Generated with Claude Code

…quired checks

ci.yml and accessibility.yml — the two workflows that run on every single PR,
and the only two that can block a merge — were each given
SUPABASE_SERVICE_ROLE_KEY and TEST_USER_PRIMARY_PASSWORD. Neither can use them.

tests/setup.ts:236-245 mocks @/lib/supabase/{client,server} globally, and
vitest.config.ts excludes every test that would need a real backend with
reasons that say so outright ("requires service role key", "requires live
Supabase with admin RPC functions"). So the credential was materialised in the
environment of the highest-frequency workflow in the repo, for its whole
history, with nothing able to reach it.

That key bypasses RLS entirely. It is the credential #281, #265 and the whole
conformance suite exist to make unnecessary, and every PR run put it in a
runner environment where any package in the install graph could read it from
process.env. Fork PRs were never the exposure — GitHub withholds secrets there
— which means fork isolation was the only thing limiting it.

Verified by deletion rather than by reading the config, because "nothing uses
it" is exactly the kind of claim that is wrong in one forgotten file:

  vitest with both vars unset  ->  395 files, 4282 tests, 0 failures
  production build with both unset  ->  succeeds, 121 CSS tags stripped,
                                       service worker stamped

validateSupabaseConfig() only warns on a missing service-role key
(src/config/payment.ts:59-63) and is guarded by typeof window === 'undefined',
so the build never depended on it either.

TEST_USER_PRIMARY_EMAIL goes with it — it is a public var, not a secret, but a
test-user email with no password is orphaned config.

conformance.yml, signup-mailer.yml and e2e.yml keep the key. They drive real
backends and genuinely need it.

The keys should be rotated once this lands, since they have been present in
these jobs since the workflows were written.

Closes #574
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Both required checks are handed SUPABASE_SERVICE_ROLE_KEY, and every Supabase call in them is mocked

2 participants