fix: replace GUC storage with config table in kit_sync_setup_cron (#383) by mei-artificerinnovations · Pull Request #385 · Artificer-Innovations/BeakerStack

mei-artificerinnovations · 2026-05-25T03:43:59Z

Summary

Fixes #383.

kit_sync_setup_cron() stored the cron bearer token via ALTER DATABASE SET app.kit_cron_secret, which hosted Supabase blocks with a 403/42501 even from a SECURITY DEFINER function. Adopters who complete Kit setup always have KIT_CRON_SECRET in GitHub secrets, so their first deploy hard-fails at the Ensure kit-sync cron job step.

Changes:

New migration 20260524000000_kit_sync_config_table.sql:
- Creates kit_sync_runtime_config (singleton row: worker_url, cron_secret, updated_at) with REVOKE ALL FROM PUBLIC — table is private to the SECURITY DEFINER function and service_role.
- Replaces the ALTER DATABASE SET call in kit_sync_setup_cron() with INSERT ... ON CONFLICT DO UPDATE.
- Updates the pg_cron job body to SELECT URL and secret from the table at runtime — secret is still never inlined in cron.job.command.

No changes to ensure-kit-sync-cron.mjs or any workflow files — the RPC signature (kit_sync_setup_cron(p_url, p_secret)) is unchanged.

Test plan

Run supabase db push locally (or against a branch database) — migration applies cleanly, function compiles, no errors.
Call kit_sync_setup_cron('https://example.supabase.co/functions/v1/kit-sync', 'test-secret') via service-role RPC — row appears in kit_sync_runtime_config, kit-sync-worker job appears in cron.job.
Confirm cron.job.command for kit-sync-worker does not contain the literal secret value.
Call again with updated URL — row is updated, old cron job unscheduled, new one registered.
Re-run a staging deploy with KIT_CRON_SECRET set — Ensure kit-sync cron job step passes (previously returned 403).

🤖 Generated with Claude Code

github-actions · 2026-05-25T03:44:07Z

⚠️ Merge Strategy Reminder

This PR targets the main branch. When merging, please use:

✅ "Create a merge commit" (NOT squash merge)

Using "Squash and merge" on PRs targeting main can cause merge conflicts when merging develop → main later.

See https://github.com/Artificer-Innovations/BeakerStack/blob/develop/docs/ARCHITECTURE.md#pull-request-process for details.

Updated at: May 31, 2026 at 12:24 PM PDT

github-actions · 2026-05-25T03:49:19Z

DB & Integration Test Summary

Check	Status	Details
Migration filename format	✅ Pass	—
Database tests (pgTAP)	❌ Fail	11/12 files · 198/198 assertions
Integration tests (Jest)	✅ Pass	9/9 suites · 50/50 tests
Live Supabase client (Vitest)	✅ Pass	1/1 files · 1/1 tests

Full logs: db-tests-log, integration-log artifacts.

Updated at: May 31, 2026 at 12:29 PM PDT

Copilot

Pull request overview

Fixes a deploy-blocking failure on hosted Supabase by removing the use of ALTER DATABASE SET app.kit_cron_secret (which is rejected with 42501 even under SECURITY DEFINER) from kit_sync_setup_cron(). Instead, a new private singleton config table holds the worker URL and cron secret, and the pg_cron job body reads them at runtime so the secret is still not inlined into cron.job.command. The RPC signature is unchanged, so scripts/ensure-kit-sync-cron.mjs and the deploy workflows keep working unchanged.

Changes:

Adds kit_sync_runtime_config singleton table with REVOKE ALL FROM PUBLIC and service_role grants.
Rewrites kit_sync_setup_cron(p_url, p_secret) to upsert into the new table and re-schedule the cron job.
Pg_cron kit-sync-worker body now SELECTs worker URL and bearer secret from the config table at execution time.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mei-artificerinnovations · 2026-05-25T04:02:01Z

+  updated_at  timestamptz NOT NULL DEFAULT now(),
+  CONSTRAINT singleton_only CHECK (singleton)
+);
+


Good catch — fixed in f04ae3d. Added ALTER TABLE public.kit_sync_runtime_config ENABLE ROW LEVEL SECURITY; immediately after the table definition, matching the pattern from marketing_email_v1.sql.

mei-artificerinnovations · 2026-05-25T04:02:02Z

+CREATE TABLE IF NOT EXISTS kit_sync_runtime_config (
+  singleton   boolean     PRIMARY KEY DEFAULT true,
+  worker_url  text        NOT NULL,
+  cron_secret text        NOT NULL,
+  updated_at  timestamptz NOT NULL DEFAULT now(),
+  CONSTRAINT singleton_only CHECK (singleton)
+);
+
+REVOKE ALL ON TABLE kit_sync_runtime_config FROM PUBLIC;
+GRANT SELECT, INSERT, UPDATE ON TABLE kit_sync_runtime_config TO service_role;


Fixed in f04ae3d. CREATE TABLE IF NOT EXISTS public.kit_sync_runtime_config, and the REVOKE/GRANT statements are now ON TABLE public.kit_sync_runtime_config as well. The INSERT inside kit_sync_setup_cron() is also updated to public.kit_sync_runtime_config.

mei-artificerinnovations · 2026-05-25T04:02:03Z

+  INSERT INTO kit_sync_runtime_config (singleton, worker_url, cron_secret, updated_at)
+  VALUES (true, p_url, p_secret, now())
+  ON CONFLICT (singleton) DO UPDATE SET
+    worker_url  = EXCLUDED.worker_url,
+    cron_secret = EXCLUDED.cron_secret,
+    updated_at  = EXCLUDED.updated_at;


Added in f04ae3d — extended kit_sync_cron.test.sql from 8 to 14 assertions:

Table exists

RLS enabled

anon has no SELECT

kit_sync_setup_cron(url, secret) executes without error (lives_ok)

Config row has correct worker_url and cron_secret after the call

cron.job.command for kit-sync-worker does not contain the literal secret value

mei-artificerinnovations · 2026-05-25T04:18:06Z

CI failure root cause + fix (commit 412921e)

Test 11 failed: anon has no SELECT on kit_sync_runtime_config.

Supabase's local dev setup applies ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT SELECT, INSERT, UPDATE, DELETE ON TABLES TO anon, authenticated during initialization. That's a direct grant to those roles — not a PUBLIC grant — so REVOKE ALL FROM PUBLIC doesn't touch it. The new table was getting SELECT access handed to anon automatically at create time.

Fix: added REVOKE ALL ON TABLE public.kit_sync_runtime_config FROM anon, authenticated; explicitly. The comment in the migration explains why (same reason RLS is also enabled — defense-in-depth against both grant paths). The test should now pass.

ZappoMan

Summary

Solid fix for #383 — moving cron secret storage off ALTER DATABASE SET to a singleton table is the right approach for hosted Supabase, and the pgTAP coverage for RLS, anon revocation, upsert behavior, and secret non-inlining is a meaningful improvement over the original migration.

Blocking before merge: please rebase onto develop and retarget the PR base branch to develop (currently targets main; develop is 3 commits ahead with no migration conflicts, but our merge workflow expects fixes to land on develop first).

Overall: CI is green and the RPC contract is unchanged, so ensure-kit-sync-cron.mjs and deploy workflows need no updates. A few security and test-hardening nits below — the service_role table grant is the most important one.

ZappoMan · 2026-05-31T19:13:23Z

+-- inaccessible via PostgREST regardless of RLS policy state.
+REVOKE ALL ON TABLE public.kit_sync_runtime_config FROM PUBLIC;
+REVOKE ALL ON TABLE public.kit_sync_runtime_config FROM anon, authenticated;
+GRANT SELECT, INSERT, UPDATE ON TABLE public.kit_sync_runtime_config TO service_role;


Security: GRANT SELECT, INSERT, UPDATE … TO service_role lets anyone with the service-role key read cron_secret directly via PostgREST (GET /rest/v1/kit_sync_runtime_config), bypassing the RPC entirely. Supabase's service role also bypasses RLS, so the REVOKE/RLS hardening doesn't help here.

Deploy only calls kit_sync_setup_cron() (SECURITY DEFINER), which runs as the function owner and does not require the caller to hold table privileges. Consider dropping table grants to service_role entirely — keep only GRANT EXECUTE on the function — so the secret is reachable only by the definer owner and the pg_cron runner (postgres), not via REST.

Fixed — the GRANT to service_role is removed. Added a comment above the REVOKE block explaining that the SECURITY DEFINER function accesses the table as owner and needs no grant, so cron_secret is unreachable via PostgREST regardless of the service key. Test 17 now asserts service_role has no direct SELECT.

ZappoMan · 2026-05-31T19:13:23Z

+  worker_url  text        NOT NULL,
+  cron_secret text        NOT NULL,
+  updated_at  timestamptz NOT NULL DEFAULT now(),
+  CONSTRAINT singleton_only CHECK (singleton)


Nit: CHECK (singleton) works but reads oddly on a boolean PK column. CHECK (singleton IS TRUE) (or = true) makes the intent explicit and matches how the tests filter (WHERE singleton = true).

Fixed — changed to CHECK (singleton IS TRUE). Also updated the cron body WHERE clause to match (WHERE c.singleton IS TRUE) for consistency.

ZappoMan · 2026-05-31T19:13:23Z

+    '*/5 * * * *',
+    $sql$
+      SELECT net.http_post(
+        url     := (SELECT worker_url  FROM public.kit_sync_runtime_config WHERE singleton),


The cron body runs two independent subqueries against kit_sync_runtime_config. Consider a single fetch, e.g. (SELECT c FROM public.kit_sync_runtime_config c WHERE c.singleton LIMIT 1) and reference c.worker_url / c.cron_secret, or a small helper function. Not a blocker — this runs every 5 minutes — but it avoids two heap hits and makes the "singleton row" assumption obvious.

Fixed — the cron body now does a single row fetch: FROM public.kit_sync_runtime_config c WHERE c.singleton IS TRUE LIMIT 1, with c.worker_url and c.cron_secret referenced directly. One heap hit instead of two.

ZappoMan · 2026-05-31T19:13:23Z

+SET search_path = extensions, net, public
+AS $$
+BEGIN
+  INSERT INTO public.kit_sync_runtime_config (singleton, worker_url, cron_secret, updated_at)


Worth a one-line comment that existing databases may still have app.kit_cron_secret set from the prior migration. Optional follow-up: EXECUTE format('ALTER DATABASE %I RESET app.kit_cron_secret', current_database()) here for hygiene so stale GUC values don't linger in pg_db_role_setting after deploy.

Fixed — added a comment inside the function body about the stale GUC, plus a best-effort nested BEGIN...EXCEPTION WHEN OTHERS block that executes ALTER DATABASE ... RESET app.kit_cron_secret. The exception handler makes it a no-op on hosted Supabase where RESET may also be restricted, so it cannot break the deploy.

ZappoMan · 2026-05-31T19:13:23Z

+  'kit_sync_runtime_config has RLS enabled'
+);
+
+-- 11. anon role has no SELECT on kit_sync_runtime_config


Good catch revoking anon — the migration also revokes authenticated. Consider mirroring that here:

NOT has_table_privilege('authenticated', 'public.kit_sync_runtime_config', 'SELECT')

Supabase default privileges grant both roles on new public tables; testing only anon leaves a gap if someone regresses the second REVOKE.

Fixed — added test 12 asserting authenticated has no SELECT (mirrors the anon check). Plan is now 17.

ZappoMan · 2026-05-31T19:13:23Z

+);
+
+-- 12. kit_sync_setup_cron executes without error and upserts config row
+SELECT lives_ok(


Calling kit_sync_setup_cron inside a BEGIN … ROLLBACK pgTAP transaction may leave a real cron.job row behind — pg_cron registration often commits independently of the outer test transaction. If CI ever runs pgTAP against a long-lived local DB (not just ephemeral CI), this can accumulate stale jobs.

Consider an explicit PERFORM cron.unschedule('kit-sync-worker') after the assertions (still inside the test block), or document that this test requires a fresh DB.

Fixed — added PERFORM cron.unschedule("kit-sync-worker") after the assertions (still inside the pgTAP block, before SELECT * FROM finish()). This cleans up the cron.job row even if pg_cron committed independently of the test transaction.

ZappoMan · 2026-05-31T19:13:23Z

+  'kit_sync_runtime_config row has correct worker_url and cron_secret after setup'
+);
+
+-- 14. cron.job.command for kit-sync-worker does not contain the literal secret


This assertion only checks that the secret is not inlined in cron.job.command. It would still pass if kit_sync_setup_cron succeeded at the upsert but cron.schedule silently failed (no row in cron.job). Consider also asserting a job exists:

EXISTS (SELECT 1 FROM cron.job WHERE jobname = 'kit-sync-worker')

…either as a separate test or combined with the non-inlining check.

Fixed — added test 15 asserting EXISTS (SELECT 1 FROM cron.job WHERE jobname = "kit-sync-worker") before the non-inlining check. Now the suite verifies both that the job was registered and that the secret is not in its command.

ZappoMan · 2026-05-31T19:13:23Z

+  'kit_sync_setup_cron executes without error'
+);
+
+-- 13. config row contains the expected worker_url and cron_secret


Test 13 reads cron_secret directly as the superuser test runner, which is fine for verifying the upsert. If you drop the service_role table grants (see migration comment on line 23), that's the right posture — the secret should only be readable by the function owner and pg_cron, not by API roles.

Acknowledged — the direct read in test 14 is correct as a superuser assertion. With the service_role table grant now removed, test 17 also explicitly asserts service_role cannot SELECT, covering that posture.

ALTER DATABASE SET app.kit_cron_secret is blocked on hosted Supabase (error 42501) even from a SECURITY DEFINER function, causing deploy to hard-fail at the Ensure kit-sync cron job step. Introduces a singleton config table (kit_sync_runtime_config) that stores the worker URL and cron secret, and rewrites kit_sync_setup_cron() to upsert there instead. The cron job body reads URL and secret from the table at runtime, so the secret is never inlined in cron.job.command. Changes: - New migration: kit_sync_runtime_config singleton table with RLS enabled; anon, authenticated, and service_role all explicitly denied table access so cron_secret is not readable via PostgREST regardless of role. SECURITY DEFINER function accesses as owner, no table grant needed. - CHECK (singleton IS TRUE) for explicit intent. - Cron body uses a single FROM row reference instead of two subqueries. - Best-effort RESET app.kit_cron_secret to clear stale GUC from prior migration (exception-handled; no-op if blocked on hosted Supabase). - pgTAP: config table, RLS, anon + authenticated + service_role privilege checks, upsert correctness, cron.job existence, secret non-inlining, and cron.unschedule cleanup; plan 8 -> 17.

mei-artificerinnovations

Rebased onto develop, PR retargeted, and all review feedback addressed in a single clean commit (ad05441).

What changed

Security (blocking)

Removed GRANT SELECT, INSERT, UPDATE ... TO service_role. The SECURITY DEFINER function accesses the table as its owner — no table grant is needed, and omitting it prevents direct REST reads of cron_secret via the service key. Comment added above the REVOKE block to document the intent. Test 17 now asserts service_role has no direct SELECT.

Migration nits

CHECK (singleton IS TRUE) — explicit form, matches test filter.
Cron body consolidated from two subqueries to a single FROM public.kit_sync_runtime_config c WHERE c.singleton IS TRUE LIMIT 1 row reference.
Best-effort RESET app.kit_cron_secret inside a nested BEGIN...EXCEPTION WHEN OTHERS block — clears the stale GUC from the prior migration; silently no-ops if ALTER DATABASE RESET is also blocked on hosted Supabase.

Test nits

Test 12: authenticated has no SELECT (mirrors the anon check; plan 14 → 17).
Test 15: cron.job row exists after kit_sync_setup_cron (was only checking secret non-inlining).
Test 17: service_role has no direct SELECT.
PERFORM cron.unschedule('kit-sync-worker') added before finish() to clean up any cron.job rows committed independently by pg_cron.

github-actions · 2026-05-31T19:26:26Z

Component	Status	Link
🌐 Web	—	View logs
🗄️ Database	✅ Migrated	—
💳 Billing	❌ Failed	View logs
📱 Mobile	⏭️ Skipped	—

Last deployed: 12:26 PDT

github-actions · 2026-05-31T19:26:55Z

CI Coverage & Test Summary

Metric	Coverage	Covered / Total
Statements	99.89%	20705 / 20727
Branches	98.46%	4991 / 5069
Functions	99.87%	752 / 753
Lines	99.90%	20695 / 20716

Suites: 64 passed, 0 failed (64 total) · Tests: 665 passed, 0 failed (665 total)

✅ All reported test suites passed.

Coverage artifacts: coverage-summary, coverage-packages.

Updated at: May 31, 2026 at 12:26 PM PDT

ZappoMan requested a review from Copilot May 25, 2026 03:45

Copilot started reviewing on behalf of ZappoMan May 25, 2026 03:45 View session

Copilot AI reviewed May 25, 2026

View reviewed changes

ZappoMan reviewed May 31, 2026

View reviewed changes

mei-artificerinnovations force-pushed the fix/kit-sync-cron-guc branch from 412921e to ad05441 Compare May 31, 2026 19:24

mei-artificerinnovations changed the base branch from main to develop May 31, 2026 19:24

mei-artificerinnovations commented May 31, 2026

View reviewed changes

Conversation

mei-artificerinnovations commented May 25, 2026

Summary

Test plan

Uh oh!

github-actions Bot commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ Merge Strategy Reminder

Uh oh!

github-actions Bot commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DB & Integration Test Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mei-artificerinnovations commented May 25, 2026

Uh oh!

ZappoMan left a comment

Choose a reason for hiding this comment

Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mei-artificerinnovations left a comment

Choose a reason for hiding this comment

What changed

Uh oh!

github-actions Bot commented May 31, 2026

Uh oh!

github-actions Bot commented May 31, 2026

CI Coverage & Test Summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented May 25, 2026 •

edited

Loading

github-actions Bot commented May 25, 2026 •

edited

Loading