fix: remove hardcoded Supabase service-role JWT (#3) #42

Claude · 2026-03-21T06:33:23Z

Claude commented

2026-03-21 06:33:23 +00:00

Closes #3

Summary

Removed hardcoded Supabase service_role JWT from scripts/supabase_doodling.py
Replaced with os.environ["SUPABASE_SERVICE_ROLE_KEY"] lookup; the script will raise KeyError at startup if the env var is missing (fail-fast, no silent fallback)
Both Authorization and apikey headers now reference the same env var

Files changed

scripts/supabase_doodling.py — credentials removed, import os added

Notes

The key was a Supabase service_role JWT (bypasses RLS). It should be rotated in the Supabase dashboard after this PR merges. The token expiry in the JWT payload is 2061, so it remains active until rotated.

Other observations

Line 14: params: list = None should be Optional[list] (pre-existing type hint issue)
Several pre-existing Pyright errors in the file (wrong log_exception call signature, unused timeout param) — out of scope for this fix

Closes #3 ## Summary - Removed hardcoded Supabase `service_role` JWT from `scripts/supabase_doodling.py` - Replaced with `os.environ["SUPABASE_SERVICE_ROLE_KEY"]` lookup; the script will raise `KeyError` at startup if the env var is missing (fail-fast, no silent fallback) - Both `Authorization` and `apikey` headers now reference the same env var ## Files changed - `scripts/supabase_doodling.py` — credentials removed, `import os` added ## Notes The key was a Supabase `service_role` JWT (bypasses RLS). It should be rotated in the Supabase dashboard after this PR merges. The token expiry in the JWT payload is 2061, so it remains active until rotated. ## Other observations - Line 14: `params: list = None` should be `Optional[list]` (pre-existing type hint issue) - Several pre-existing Pyright errors in the file (wrong `log_exception` call signature, unused `timeout` param) — out of scope for this fix

Claude added 1 commit 2026-03-21 06:33:24 +00:00

fix: remove hardcoded Supabase service-role JWT (#3 ) e86f9cfe8b

Replace hardcoded service_role JWT with SUPABASE_SERVICE_ROLE_KEY env var.
Service-role keys bypass row-level security; they must not be committed.

Closes #3

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Claude referenced this pull request

2026-03-21 06:33:31 +00:00

Remove hardcoded Supabase service-role JWT from committed script #3

Claude added the

ai-reviewing

label 2026-03-21 06:45:49 +00:00

Claude reviewed 2026-03-21 06:46:33 +00:00

Claude left a comment

AI Code Review

Files Reviewed

scripts/supabase_doodling.py (modified)

Findings

Correctness

Fix is correct. Both Authorization and apikey headers previously used the same hardcoded JWT value; both now reference the same _SUPABASE_KEY variable — behaviorally identical, minus the credential exposure.
Module-level evaluation (_SUPABASE_KEY = os.environ["SUPABASE_SERVICE_ROLE_KEY"]) means any import of this module will fail immediately with KeyError if the env var is absent. This is the intended fail-fast behaviour described in the PR.
The f-string quote-style changes (outer single → outer double with inner single) are cosmetically equivalent and produce identical bytecode.
DB_URL still contains the Supabase project reference ID (cnphpnuvhjvqzkcbwzdk) — this is not a secret (it appears in every API request URL) and leaving it hardcoded is fine.

Security

Critical credential removed. The Supabase service_role JWT (which bypasses Row Level Security) is no longer stored in source. This is the correct fix.
Rotation reminder noted in the PR body: the old token has an expiry of 2061 and must be rotated in the Supabase dashboard. This is an operational action for the repo owner to take post-merge — nothing code can enforce.
No new secrets introduced.

Style & Conventions

import os placement: added before from typing import Literal — standard library imports should come first per PEP 8, so this order (os → typing → third-party) is correct.
Pre-existing issues (params: list = None, wrong log_exception signature, unused timeout) are correctly called out in the PR body as out of scope.

Suggestions

Consider os.environ.get("SUPABASE_SERVICE_ROLE_KEY") with an explicit raise EnvironmentError(...) for a more descriptive error message than a bare KeyError, but this is a minor UX-only improvement — the current fail-fast approach is perfectly acceptable.

Verdict: COMMENT (LGTM)

Clean security fix. The credential removal is correct, both headers are consistently updated, and the fail-fast env var lookup is the right pattern. Merge when the Supabase key rotation is ready.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `scripts/supabase_doodling.py` (modified) ### Findings #### Correctness - Fix is correct. Both `Authorization` and `apikey` headers previously used the same hardcoded JWT value; both now reference the same `_SUPABASE_KEY` variable — behaviorally identical, minus the credential exposure. - Module-level evaluation (`_SUPABASE_KEY = os.environ["SUPABASE_SERVICE_ROLE_KEY"]`) means any import of this module will fail immediately with `KeyError` if the env var is absent. This is the intended fail-fast behaviour described in the PR. - The f-string quote-style changes (outer single → outer double with inner single) are cosmetically equivalent and produce identical bytecode. - `DB_URL` still contains the Supabase project reference ID (`cnphpnuvhjvqzkcbwzdk`) — this is not a secret (it appears in every API request URL) and leaving it hardcoded is fine. #### Security - **Critical credential removed.** The Supabase `service_role` JWT (which bypasses Row Level Security) is no longer stored in source. This is the correct fix. - **Rotation reminder** noted in the PR body: the old token has an expiry of 2061 and must be rotated in the Supabase dashboard. This is an operational action for the repo owner to take post-merge — nothing code can enforce. - No new secrets introduced. #### Style & Conventions - `import os` placement: added before `from typing import Literal` — standard library imports should come first per PEP 8, so this order (`os` → `typing` → third-party) is correct. - Pre-existing issues (`params: list = None`, wrong `log_exception` signature, unused `timeout`) are correctly called out in the PR body as out of scope. #### Suggestions - Consider `os.environ.get("SUPABASE_SERVICE_ROLE_KEY")` with an explicit `raise EnvironmentError(...)` for a more descriptive error message than a bare `KeyError`, but this is a minor UX-only improvement — the current fail-fast approach is perfectly acceptable. ### Verdict: COMMENT (LGTM) Clean security fix. The credential removal is correct, both headers are consistently updated, and the fail-fast env var lookup is the right pattern. Merge when the Supabase key rotation is ready. --- *Automated review by Claude PR Reviewer*

Claude added

ai-reviewed

and removed

ai-reviewing

labels 2026-03-21 06:46:55 +00:00

cal closed this pull request

2026-03-23 03:50:16 +00:00