fix: harden Edge Function notification path per final review
Fixes issues raised by the whole-branch review before merge: - Critical: the previous migration hardcoded the live production functions URL and anon key as the committed default, so every local `supabase db reset` silently pointed dev/test triggers at the real project. Adding a new migration instead of editing the already-applied one (checksum/drift safety), with local dev values as the committed default; production values are applied out-of-band, never committed. - Important: verify_jwt=true plus sending the anon key as a bearer token didn't actually stop anyone, since the anon key is public (ships in the frontend bundle) — confirmed empirically, a request with a valid anon key but no shared secret now gets a real 401. Added a random shared secret known only to the database and the Edge Functions' secret store. - Important: notification emails interpolated raw contact fields into HTML with no escaping. Added escapeHtml() and tests proving injected markup renders as literal text, not live markup. - Important: the URL/key/secret functions moved from public to a private schema with EXECUTE revoked from anon/authenticated, closing an unintended PostgREST RPC endpoint (verified 404 after the fix). Also adds a test:functions npm script now that Deno is confirmed available and the shared email builders' tests pass (12 total incl. the new escaping cases). Claude-Session: https://claude.ai/code/session_01NhxA4npAXAxmCDEJt6kxSH
This commit is contained in:
50
deno.lock
generated
Normal file
50
deno.lock
generated
Normal file
@@ -0,0 +1,50 @@
|
|||||||
|
{
|
||||||
|
"version": "5",
|
||||||
|
"specifiers": {
|
||||||
|
"jsr:@std/assert@*": "1.0.19",
|
||||||
|
"jsr:@std/internal@^1.0.12": "1.0.14"
|
||||||
|
},
|
||||||
|
"jsr": {
|
||||||
|
"@std/assert@1.0.19": {
|
||||||
|
"integrity": "eaada96ee120cb980bc47e040f82814d786fe8162ecc53c91d8df60b8755991e",
|
||||||
|
"dependencies": [
|
||||||
|
"jsr:@std/internal"
|
||||||
|
]
|
||||||
|
},
|
||||||
|
"@std/internal@1.0.14": {
|
||||||
|
"integrity": "291516b3d4c35024d6ffbc0a9df5bf4c64116e05b50012cf846710152d2ffdf7"
|
||||||
|
}
|
||||||
|
},
|
||||||
|
"workspace": {
|
||||||
|
"packageJson": {
|
||||||
|
"dependencies": [
|
||||||
|
"npm:@eslint/js@^10.0.1",
|
||||||
|
"npm:@react-three/fiber@^9.7.0",
|
||||||
|
"npm:@supabase/supabase-js@^2.112.2",
|
||||||
|
"npm:@testing-library/jest-dom@7",
|
||||||
|
"npm:@testing-library/react@^16.3.2",
|
||||||
|
"npm:@testing-library/user-event@^14.6.1",
|
||||||
|
"npm:@types/node@^24.13.3",
|
||||||
|
"npm:@types/react-dom@^19.2.3",
|
||||||
|
"npm:@types/react@^19.2.17",
|
||||||
|
"npm:@types/three@~0.185.3",
|
||||||
|
"npm:@vitejs/plugin-react@^6.0.4",
|
||||||
|
"npm:eslint-plugin-react-hooks@^7.1.1",
|
||||||
|
"npm:eslint-plugin-react-refresh@~0.5.3",
|
||||||
|
"npm:eslint@^10.8.0",
|
||||||
|
"npm:framer-motion@^12.43.0",
|
||||||
|
"npm:globals@^17.7.0",
|
||||||
|
"npm:jsdom@^30.0.1",
|
||||||
|
"npm:react-dom@^19.2.8",
|
||||||
|
"npm:react-router-dom@^7.18.2",
|
||||||
|
"npm:react@^19.2.8",
|
||||||
|
"npm:three@~0.185.1",
|
||||||
|
"npm:tsx@^4.23.11",
|
||||||
|
"npm:typescript-eslint@^8.65.0",
|
||||||
|
"npm:typescript@~6.0.2",
|
||||||
|
"npm:vite@^8.2.0",
|
||||||
|
"npm:vitest@^4.1.10"
|
||||||
|
]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -9,6 +9,7 @@
|
|||||||
"lint": "eslint .",
|
"lint": "eslint .",
|
||||||
"preview": "vite preview",
|
"preview": "vite preview",
|
||||||
"test": "vitest run",
|
"test": "vitest run",
|
||||||
|
"test:functions": "deno test supabase/functions/",
|
||||||
"seed": "tsx scripts/seed.ts"
|
"seed": "tsx scripts/seed.ts"
|
||||||
},
|
},
|
||||||
"dependencies": {
|
"dependencies": {
|
||||||
|
|||||||
@@ -47,3 +47,34 @@ Deno.test('buildInquiryNotificationEmail omits the notes paragraph when notes ar
|
|||||||
})
|
})
|
||||||
if (html.includes('Notes:')) throw new Error('expected no Notes: paragraph')
|
if (html.includes('Notes:')) throw new Error('expected no Notes: paragraph')
|
||||||
})
|
})
|
||||||
|
|
||||||
|
Deno.test('buildBookingNotificationEmail escapes HTML in contact fields so they cannot inject markup', () => {
|
||||||
|
const { html } = buildBookingNotificationEmail({
|
||||||
|
serviceName: 'Knotless Braids',
|
||||||
|
date: '2026-09-01',
|
||||||
|
time: '10:00 AM',
|
||||||
|
contact: {
|
||||||
|
name: '<a href="https://evil.example">Click here</a>',
|
||||||
|
phone: '555-0100',
|
||||||
|
email: 'jane@example.com',
|
||||||
|
notes: '<img src=x onerror=alert(1)>',
|
||||||
|
},
|
||||||
|
})
|
||||||
|
if (html.includes('<a href="https://evil.example">')) throw new Error('expected contact name to be escaped')
|
||||||
|
if (html.includes('<img src=x onerror=alert(1)>')) throw new Error('expected notes to be escaped')
|
||||||
|
assertStringIncludes(html, '<a href="https://evil.example">Click here</a>')
|
||||||
|
assertStringIncludes(html, '<img src=x onerror=alert(1)>')
|
||||||
|
})
|
||||||
|
|
||||||
|
Deno.test('buildInquiryNotificationEmail escapes HTML in contact fields so they cannot inject markup', () => {
|
||||||
|
const { html } = buildInquiryNotificationEmail({
|
||||||
|
productName: 'Silk Edge Control',
|
||||||
|
contact: {
|
||||||
|
name: '<script>alert(1)</script>',
|
||||||
|
email: 'nadia@example.com',
|
||||||
|
notes: '',
|
||||||
|
},
|
||||||
|
})
|
||||||
|
if (html.includes('<script>alert(1)</script>')) throw new Error('expected contact name to be escaped')
|
||||||
|
assertStringIncludes(html, '<script>alert(1)</script>')
|
||||||
|
})
|
||||||
|
|||||||
@@ -1,3 +1,12 @@
|
|||||||
|
function escapeHtml(value: string): string {
|
||||||
|
return value
|
||||||
|
.replace(/&/g, '&')
|
||||||
|
.replace(/</g, '<')
|
||||||
|
.replace(/>/g, '>')
|
||||||
|
.replace(/"/g, '"')
|
||||||
|
.replace(/'/g, ''')
|
||||||
|
}
|
||||||
|
|
||||||
export interface BookingEmailInput {
|
export interface BookingEmailInput {
|
||||||
serviceName: string
|
serviceName: string
|
||||||
date: string
|
date: string
|
||||||
@@ -6,11 +15,15 @@ export interface BookingEmailInput {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function buildBookingNotificationEmail(input: BookingEmailInput): { subject: string; html: string } {
|
export function buildBookingNotificationEmail(input: BookingEmailInput): { subject: string; html: string } {
|
||||||
const notesLine = input.contact.notes ? `<p>Notes: ${input.contact.notes}</p>` : ''
|
const serviceName = escapeHtml(input.serviceName)
|
||||||
|
const name = escapeHtml(input.contact.name)
|
||||||
|
const phone = escapeHtml(input.contact.phone)
|
||||||
|
const email = escapeHtml(input.contact.email)
|
||||||
|
const notesLine = input.contact.notes ? `<p>Notes: ${escapeHtml(input.contact.notes)}</p>` : ''
|
||||||
return {
|
return {
|
||||||
subject: `New booking: ${input.serviceName} on ${input.date}`,
|
subject: `New booking: ${input.serviceName} on ${input.date}`,
|
||||||
html: `<p><strong>${input.contact.name}</strong> booked <strong>${input.serviceName}</strong> on ${input.date} at ${input.time}.</p>` +
|
html: `<p><strong>${name}</strong> booked <strong>${serviceName}</strong> on ${escapeHtml(input.date)} at ${escapeHtml(input.time)}.</p>` +
|
||||||
`<p>Phone: ${input.contact.phone}<br/>Email: ${input.contact.email}</p>${notesLine}`,
|
`<p>Phone: ${phone}<br/>Email: ${email}</p>${notesLine}`,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -20,9 +33,12 @@ export interface InquiryEmailInput {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function buildInquiryNotificationEmail(input: InquiryEmailInput): { subject: string; html: string } {
|
export function buildInquiryNotificationEmail(input: InquiryEmailInput): { subject: string; html: string } {
|
||||||
const notesLine = input.contact.notes ? `<p>Notes: ${input.contact.notes}</p>` : ''
|
const productName = escapeHtml(input.productName)
|
||||||
|
const name = escapeHtml(input.contact.name)
|
||||||
|
const email = escapeHtml(input.contact.email)
|
||||||
|
const notesLine = input.contact.notes ? `<p>Notes: ${escapeHtml(input.contact.notes)}</p>` : ''
|
||||||
return {
|
return {
|
||||||
subject: `New inquiry: ${input.productName}`,
|
subject: `New inquiry: ${input.productName}`,
|
||||||
html: `<p><strong>${input.contact.name}</strong> (${input.contact.email}) asked about <strong>${input.productName}</strong>.</p>${notesLine}`,
|
html: `<p><strong>${name}</strong> (${email}) asked about <strong>${productName}</strong>.</p>${notesLine}`,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -5,6 +5,14 @@ const FROM_ADDRESS = 'bookings@braidsbymira.com'
|
|||||||
const MIRA_EMAIL = 'hello@braidsbymira.com'
|
const MIRA_EMAIL = 'hello@braidsbymira.com'
|
||||||
|
|
||||||
Deno.serve(async (req) => {
|
Deno.serve(async (req) => {
|
||||||
|
const notifySecret = Deno.env.get('NOTIFY_WEBHOOK_SECRET')
|
||||||
|
if (notifySecret && req.headers.get('X-Notify-Secret') !== notifySecret) {
|
||||||
|
return new Response(JSON.stringify({ error: 'unauthorized' }), {
|
||||||
|
status: 401,
|
||||||
|
headers: { 'Content-Type': 'application/json' },
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
const payload = await req.json()
|
const payload = await req.json()
|
||||||
const booking = payload.record
|
const booking = payload.record
|
||||||
|
|
||||||
|
|||||||
@@ -5,6 +5,14 @@ const FROM_ADDRESS = 'bookings@braidsbymira.com'
|
|||||||
const MIRA_EMAIL = 'hello@braidsbymira.com'
|
const MIRA_EMAIL = 'hello@braidsbymira.com'
|
||||||
|
|
||||||
Deno.serve(async (req) => {
|
Deno.serve(async (req) => {
|
||||||
|
const notifySecret = Deno.env.get('NOTIFY_WEBHOOK_SECRET')
|
||||||
|
if (notifySecret && req.headers.get('X-Notify-Secret') !== notifySecret) {
|
||||||
|
return new Response(JSON.stringify({ error: 'unauthorized' }), {
|
||||||
|
status: 401,
|
||||||
|
headers: { 'Content-Type': 'application/json' },
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
const payload = await req.json()
|
const payload = await req.json()
|
||||||
const inquiry = payload.record
|
const inquiry = payload.record
|
||||||
|
|
||||||
|
|||||||
112
supabase/migrations/20260812000001_harden_notify_config.sql
Normal file
112
supabase/migrations/20260812000001_harden_notify_config.sql
Normal file
@@ -0,0 +1,112 @@
|
|||||||
|
-- Follow-up to 20260808000003, prompted by final whole-branch review before
|
||||||
|
-- merge. Two problems with the previous migration's approach:
|
||||||
|
--
|
||||||
|
-- 1. It hardcoded the LIVE PRODUCTION base URL and anon key as the committed
|
||||||
|
-- default. Migrations replay on every `supabase db reset`, so any local
|
||||||
|
-- dev/test run was silently wired to the live project — once a Resend key
|
||||||
|
-- is configured, local testing would send real emails to Mira's inbox
|
||||||
|
-- with test data. Fixed by making the committed default the LOCAL dev
|
||||||
|
-- values again; production values are applied out-of-band (see below),
|
||||||
|
-- never committed.
|
||||||
|
--
|
||||||
|
-- 2. `verify_jwt = true` + sending the (public) anon key as a bearer token
|
||||||
|
-- does not actually stop an attacker: the anon key already ships in the
|
||||||
|
-- deployed frontend bundle, so anyone can copy it and POST directly to
|
||||||
|
-- the function URLs to trigger arbitrary outbound emails. Fixed by
|
||||||
|
-- adding a real shared secret (`edge_functions_notify_secret()`), known
|
||||||
|
-- only to this database and the Edge Functions' secret store, checked by
|
||||||
|
-- both functions before doing any work.
|
||||||
|
--
|
||||||
|
-- Both new base-url/anon-key/secret functions move to a `private` schema
|
||||||
|
-- (not exposed via PostgREST — only `public` is listed under `[api]` in
|
||||||
|
-- config.toml) with EXECUTE explicitly revoked from anon/authenticated, so
|
||||||
|
-- they can never become an anon-callable RPC endpoint. The previous
|
||||||
|
-- `public.edge_functions_base_url()` / `public.edge_functions_anon_key()`
|
||||||
|
-- were reachable that way (POST /rest/v1/rpc/edge_functions_anon_key) —
|
||||||
|
-- harmless only because the value itself was already public. Dropped here.
|
||||||
|
--
|
||||||
|
-- PRODUCTION DEPLOY: after this migration is pushed, apply the real values
|
||||||
|
-- out-of-band (never commit these — the anon key is public but the base URL
|
||||||
|
-- is project-specific noise, and the notify secret is a genuine secret):
|
||||||
|
--
|
||||||
|
-- supabase db query --linked "create or replace function private.edge_functions_base_url() returns text language sql immutable as \$\$ select 'https://<project-ref>.supabase.co/functions/v1' \$\$;"
|
||||||
|
-- supabase db query --linked "create or replace function private.edge_functions_anon_key() returns text language sql immutable as \$\$ select '<production anon key>' \$\$;"
|
||||||
|
-- supabase db query --linked "create or replace function private.edge_functions_notify_secret() returns text language sql immutable as \$\$ select '<random secret>' \$\$;"
|
||||||
|
-- supabase secrets set NOTIFY_WEBHOOK_SECRET=<same random secret>
|
||||||
|
-- supabase functions deploy on-booking-created
|
||||||
|
-- supabase functions deploy on-inquiry-created
|
||||||
|
|
||||||
|
create schema if not exists private;
|
||||||
|
|
||||||
|
drop function if exists public.edge_functions_base_url();
|
||||||
|
drop function if exists public.edge_functions_anon_key();
|
||||||
|
|
||||||
|
create or replace function private.edge_functions_base_url()
|
||||||
|
returns text
|
||||||
|
language sql
|
||||||
|
immutable
|
||||||
|
as $$
|
||||||
|
select 'http://host.docker.internal:54321/functions/v1'
|
||||||
|
$$;
|
||||||
|
|
||||||
|
create or replace function private.edge_functions_anon_key()
|
||||||
|
returns text
|
||||||
|
language sql
|
||||||
|
immutable
|
||||||
|
as $$
|
||||||
|
select 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzdXBhYmFzZS1kZW1vIiwicm9sZSI6ImFub24iLCJleHAiOjE5ODM4MTI5OTZ9.CRXP1A7WOeoJeXxjNni43kdQwgnWNReilDMblYTn_I0'
|
||||||
|
$$;
|
||||||
|
|
||||||
|
-- Local dev placeholder only — production sets a real random value via the
|
||||||
|
-- out-of-band command above, never committed to this file.
|
||||||
|
create or replace function private.edge_functions_notify_secret()
|
||||||
|
returns text
|
||||||
|
language sql
|
||||||
|
immutable
|
||||||
|
as $$
|
||||||
|
select 'local-dev-notify-secret'
|
||||||
|
$$;
|
||||||
|
|
||||||
|
revoke execute on function private.edge_functions_base_url() from public, anon, authenticated;
|
||||||
|
revoke execute on function private.edge_functions_anon_key() from public, anon, authenticated;
|
||||||
|
revoke execute on function private.edge_functions_notify_secret() from public, anon, authenticated;
|
||||||
|
|
||||||
|
create or replace function public.notify_booking_created()
|
||||||
|
returns trigger
|
||||||
|
language plpgsql
|
||||||
|
security definer
|
||||||
|
set search_path = ''
|
||||||
|
as $$
|
||||||
|
begin
|
||||||
|
perform net.http_post(
|
||||||
|
url := private.edge_functions_base_url() || '/on-booking-created',
|
||||||
|
headers := jsonb_build_object(
|
||||||
|
'Content-Type', 'application/json',
|
||||||
|
'Authorization', 'Bearer ' || private.edge_functions_anon_key(),
|
||||||
|
'X-Notify-Secret', private.edge_functions_notify_secret()
|
||||||
|
),
|
||||||
|
body := jsonb_build_object('record', to_jsonb(new))
|
||||||
|
);
|
||||||
|
return new;
|
||||||
|
end;
|
||||||
|
$$;
|
||||||
|
|
||||||
|
create or replace function public.notify_inquiry_created()
|
||||||
|
returns trigger
|
||||||
|
language plpgsql
|
||||||
|
security definer
|
||||||
|
set search_path = ''
|
||||||
|
as $$
|
||||||
|
begin
|
||||||
|
perform net.http_post(
|
||||||
|
url := private.edge_functions_base_url() || '/on-inquiry-created',
|
||||||
|
headers := jsonb_build_object(
|
||||||
|
'Content-Type', 'application/json',
|
||||||
|
'Authorization', 'Bearer ' || private.edge_functions_anon_key(),
|
||||||
|
'X-Notify-Secret', private.edge_functions_notify_secret()
|
||||||
|
),
|
||||||
|
body := jsonb_build_object('record', to_jsonb(new))
|
||||||
|
);
|
||||||
|
return new;
|
||||||
|
end;
|
||||||
|
$$;
|
||||||
Reference in New Issue
Block a user