[Security] telemetry-ingest uses service role key — bypasses RLS unnecessarily

[mr-k-man]

Summary

supabase/functions/telemetry-ingest/index.ts:46-49 — The edge function creates a Supabase client with SUPABASE_SERVICE_ROLE_KEY, which bypasses Row Level Security entirely. This is unnecessary — RLS policies already allow anon INSERT and SELECT on telemetry_events, plus INSERT/SELECT/UPDATE on installations (for upsert).

Using the service role key means any caller (even without proper credentials) gets unrestricted database access through this endpoint, enabling telemetry data poisoning.

Impact

Medium — permits unauthenticated telemetry poisoning and write amplification. The service role key bypass is unnecessary given existing RLS policies.

Fix

PR #664 replaces the service role key with the caller-provided apikey header (anon key). Returns 401 if the header is missing. RLS enforces access control. The client (bin/gstack-telemetry-sync) already sends the anon key — no client changes needed.

Found via sqry AST-based semantic code graph analysis.

[mr-k-man]

Update: While investigating Gemini's follow-up about anon_select RLS policies, we discovered that migration 002_tighten_rls.sql already drops anon_select on all three tables — so that concern is pre-addressed.

However, 002 also drops anon_update_last_seen on installations. Our fix (switching from service role key to caller's anon key) means the installations upsert now needs an UPDATE policy to work. Without it, upserts silently fail after 002 is applied.

Added migration 003_installations_upsert_policy.sql to re-add a scoped UPDATE policy for installations, enabling the upsert to work with the anon key.

This is included in PR #664.

Found via sqry AST-based semantic code graph analysis.

[garagon]

The finding is valid — SUPABASE_SERVICE_ROLE_KEY bypasses RLS and isn't needed here since migration 001 already grants anon INSERT on both tables.

Two observations on the fix in PR #664:

Migration 003 re-opens what 002 closed. Migration 002 explicitly dropped anon_update_last_seen because it was "unrestricted on all columns." Migration 003's anon_update_tracking uses the same USING (true) WITH CHECK (true) — same permissiveness. The comment in 003 acknowledges this ("this still allows updating all columns"). Worth noting as residual risk since anon callers can now UPDATE any column on installations, not just last_seen/gstack_version/os.

The apikey header is caller-controlled. The fix trusts req.headers.get("apikey") as-is. Supabase Edge Functions receive this from the client — any value passes the if (!callerKey) check, including a service role key if one leaks. Since the anon key is public by design in Supabase (it's in the client bundle), this is fine for the threat model here. But it's not authentication — it's just "use caller's privilege level instead of god mode." The real protection is RLS, which is correct.

Neither of these block the PR — the fix is a net improvement over the current state. Just flagging for completeness.

[mr-k-man]

Both observations are valid — thanks for the careful review.

Column-level restriction (addressed): You're right that migration 003 re-opened the same broad UPDATE that 002 explicitly closed. PostgreSQL supports column-level GRANT UPDATE which is a better fit than RLS for this. Updated migration 003 to:

REVOKE UPDATE ON installations FROM anon;
GRANT UPDATE (last_seen, gstack_version, os) ON installations TO anon;

Now anon callers can only UPDATE the three tracking columns the edge function actually writes. Any UPDATE touching first_seen or installation_id gets a permission error at the query level.

apikey header trust: Agreed — it's privilege-level selection, not authentication. The real protection is RLS + the column-level GRANT, which is correct for this threat model. The anon key is public by design in Supabase (shipped in client bundles), so trusting the caller's key here just means "use their privilege level instead of god mode."

Fix pushed to PR #664.