feat(branding): per-restaurant branding schema (promoted M11 migration)#3
Conversation
Adds six nullable columns to the `restaurants` table for per-restaurant brand identity. Promoted ahead of M11 because the customer-facing post-payment landing pages (Stripe Checkout success/cancel) must render with the restaurant's brand, not DialTone's — a customer paying for an order is interacting with the restaurant they ordered from. Schema (supabase/migrations/0007_restaurant_branding.sql): - display_name text ≤80 — public-facing name override; falls back to restaurants.name - logo_url text — Supabase Storage or external CDN URL - primary_color text hex — dominant accent on customer-facing pages - secondary_color text hex — supporting accent; falls back to a derivation of primary - font text ≤80 — Google Fonts entry or CSS-safe stack; falls back to system - tagline text ≤120 — used in SMS signatures and customer pages All nullable so existing rows don't need backfill. Render code falls back to safe defaults (system font, neutral palette, restaurants.name as display) when fields are null. Why columns on restaurants rather than a separate restaurant_brands table: 1:1 relationship, branding loaded on every customer-facing render (frequent), no need for a JOIN per page. If the relationship ever becomes 1:N, split out then. Other changes: - developer/planned-migrations/m11-restaurant-branding.sql removed (promoted to supabase/migrations/, no longer planned) - AGENTS.md "Planned migrations" entry updated to note the queue is empty and explain why m11-restaurant-branding was promoted early - developer/branding.md rewritten to describe the chosen approach (columns on restaurants, not a separate table) plus query patterns, render-time fallback rules, and where the data is consumed - supabase/seed.sql populates Sui's Sushi with demo branding values (#8B0000 / #F5C14A / Playfair Display / "Fresh sushi, made to order."); leaves Rival Ramen branding null so RLS isolation tests and the render-side fallback path stay exercised - packages/shared/src/supabase/types.ts regenerated against the new schema (six new optional fields on restaurants Row/Insert/Update) Applied locally + cloud (klzznfagrtormretqsgb) ahead of this commit; pnpm ci:fast green (188 unit tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
|
||
| alter table restaurants | ||
| add column display_name text check (display_name is null or char_length(display_name) <= 80), | ||
| add column logo_url text, |
There was a problem hiding this comment.
logo_url accepts arbitrary text with no format validation
The column accepts any string — including javascript:... or data:text/html,... URIs. When the render code places this value in an <img src>, <a href>, or CSS background-image without prior server-side validation, a malicious or misconfigured value can bypass browser defenses. Consider adding a check constraint to at least enforce http:// or https:// schemes:
add column logo_url text check (logo_url is null or logo_url ~ '^https?://')…check Two findings on PR #3: P1 (architectural gap): the unauthenticated post-payment landing pages can't read `restaurants` directly — RLS is `authenticated`-only, so the schema added in 0007 was unreachable from the surfaces that need it. 0008 adds a `security definer` RPC `get_restaurant_branding(order_id)` that returns only the safe brand fields (no phone, no Stripe IDs, no payment config) and is granted `execute` to `anon`. Scoped by `order_id` (opaque UUID, not enumerable) rather than restaurant slug to keep platform-wide scraping out of reach. P2 (security): `logo_url` accepted any string including `javascript:` and `data:` URIs. A check constraint now requires the value to start with `http://` or `https://`, so a malicious or misconfigured value can't reach an `<img src>` / CSS `background-image` and bypass browser defenses. Added as 0008 (separate migration) because 0007 was already applied to cloud. Other changes: - developer/branding.md "Public read access" section rewritten to document the concrete RPC, the call shape from React, and the rationale for choosing RPC + order_id scoping over column-level RLS + restaurant_id scoping - packages/shared/src/supabase/types.ts regenerated against the new RPC return shape Applied locally and on cloud project klzznfagrtormretqsgb. Verified via `pnpm ci:fast` (188 unit tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…check Two findings on PR #3: P1 (architectural gap): the unauthenticated post-payment landing pages can't read `restaurants` directly — RLS is `authenticated`-only, so the schema added in 0007 was unreachable from the surfaces that need it. 0008 adds a `security definer` RPC `get_restaurant_branding(order_id)` that returns only the safe brand fields (no phone, no Stripe IDs, no payment config) and is granted `execute` to `anon`. Scoped by `order_id` (opaque UUID, not enumerable) rather than restaurant slug to keep platform-wide scraping out of reach. P2 (security): `logo_url` accepted any string including `javascript:` and `data:` URIs. A check constraint now requires the value to start with `http://` or `https://`, so a malicious or misconfigured value can't reach an `<img src>` / CSS `background-image` and bypass browser defenses. Added as 0008 (separate migration) because 0007 was already applied to cloud. Other changes: - developer/branding.md "Public read access" section rewritten to document the concrete RPC, the call shape from React, and the rationale for choosing RPC + order_id scoping over column-level RLS + restaurant_id scoping - packages/shared/src/supabase/types.ts regenerated against the new RPC return shape Applied locally and on cloud project klzznfagrtormretqsgb. Verified via `pnpm ci:fast` (188 unit tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fda80a3 to
a872c17
Compare
|
Both Greptile findings addressed in P1 — anon read access for branding. Added Scoped by
P2 — alter table restaurants
add constraint restaurants_logo_url_scheme_chk
check (logo_url is null or logo_url ~ '^https?://');Rejects Both migrations applied to local + cloud (project |
|
Updated |
…ages (#4) * feat(orders): add branded customer-facing /orders/:id/{paid,cancel} pages Closes the M8 success_url 404 follow-up. Stripe Checkout was redirecting paying customers to dialtone.menu/orders/:id/{paid,cancel}, which is the marketing site (no such routes — 404). The order itself flipped to paid correctly via webhook (server-to-server, independent of the redirect), but the customer-facing landing was broken. After deciding the pages must be PER-RESTAURANT branded (a customer paying for Sui's Sushi shouldn't see DialTone Menu chrome), the landing pages moved into the admin app where the restaurant data lives. Branding fetched via the new get_restaurant_branding(order_id) RPC (PR #3) which is granted to anon and exposes only safe brand fields. Routes — apps/admin/src/app.tsx - Two PUBLIC routes added above the ProtectedRoute wrapper: /orders/:id/paid — success_url target /orders/:id/cancel — cancel_url target Both bypass auth so customers without a session can land on them. Reusing the OrderResultPage component with kind="paid"|"cancel". Component — apps/admin/src/pages/orders/order-result-page.tsx - Reads :id from the URL, calls supabase.rpc('get_restaurant_branding') - Renders headline + lede that name the restaurant ("Payment received from Sui's Sushi") - Logo block if logo_url is set, otherwise the display name styled as a wordmark in primary_color - Tagline below logo if set - Card with check/x icon, headline, lede, and (for paid) a "What happens next" panel tinted with secondary_color - Loads the restaurant's font dynamically from Google Fonts at mount, scrubs unsafe characters first - Falls back to neutral DialTone defaults if the RPC returns null (we never want to show a paying customer a 404) Helpers — apps/admin/src/lib/branding.ts (split for unit testability) - safeFontFamily(font) → CSS-injection-safe font-family value - googleFontHref(font) → Google Fonts CSS2 stylesheet URL or null - FALLBACK_PRIMARY / FALLBACK_SECONDARY / SYSTEM_FONT_STACK constants - 22 unit tests in apps/admin/test/branding.test.ts cover null/empty/ whitespace/invalid-char inputs and CSS-injection attempts Edge Functions — admin_create_manual_order + vapi_finalize_order - Default DIALTONE_PUBLIC_BASE_URL changed from 'https://dialtone.menu' to 'https://admin.dialtone.menu' so the routes resolve out of the box. DIALTONE_PUBLIC_BASE_URL secret can still override (e.g. for staging environments). 210 unit tests pass (188 prior + 22 new). Lint + typecheck green. Test plan after merge: - Place a fresh manual card order, pay with 4242 - Confirm landing on the new branded paid page (not the 404) - Confirm Sui's Sushi branding renders (#8B0000 primary, #F5C14A secondary, Playfair Display font, "Fresh sushi, made to order." tagline) - Visit /orders/<unknown-uuid>/paid to confirm fallback rendering with DialTone defaults Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(orders): address Greptile P1+P2 — rejection handler + safe color tint P1 (eternal spinner): the RPC's `.then()` had no rejection handler. Supabase v2 normally resolves with {data, error}, but any unhandled sync throw inside the success handler or a network-layer rejection would leave `setLoaded` uncalled — trapping a paying customer on a blank spinner with no recovery. Worst possible UX immediately after a successful payment. Switched to the two-argument `.then(onFulfilled, onRejected)` form so we always flip to loaded and render fallback defaults if anything goes sideways. (Used the two-arg form because the Supabase query builder is a PromiseLike, not a full Promise — no `.catch()` available.) P2 (broken CSS for non-six-digit hex): `${secondary}1A` was building an `#RRGGBBAA` color by string-concatenating an alpha hex pair onto the brand color. This worked only when secondary_color was exactly `#RRGGBB`. The 0007 migration's check constraint enforces that today, but if the constraint ever loosens or input bypasses it, `#FFC` + `1A` = `#FFC1A` (5 hex digits — invalid CSS, browser silently drops the background). Replaced with a new `hexToRgba(hex, alpha)` helper in `apps/admin/src/lib/branding.ts` that parses #RRGGBB → `rgba(r,g,b,a)` and falls back to returning the input opaquely for any unexpected format. 10 new unit tests cover six-digit / mixed-case / shorthand / malformed / alpha-clamping inputs. 220 unit tests pass (210 prior + 10 new), lint + typecheck green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Captures the May 2 work after PRs #2/#3/#4 merged so picking up cold in 2-3 days doesn't require re-discovering anything. README.md - Status bumped from "M1–M7 complete... Next: M8" to "M1–M8 complete, Path A demo proven live" - Stack updated (Telnyx replaces Twilio; Stripe Connect routing now M11 not M7) - "Where things live" table refreshed: planned-migrations now empty, added Edge Function + payment + branding entries, called out that customer-facing post-payment pages live in admin (not the marketing site at dialtone_menu) - Added pointer to developer/m8-live-demo-checklist.md for the deploy runbook + lessons learned AGENTS.md - Current state header rewritten — single paragraph covering today's four PRs and where to look next - Conventions extended with five new bullets agents commonly miss: - Browser-callable Edge Functions need handlePreflight() (M8 lesson) - Auth init must bootstrap from getSession() before subscribing (PR #1) - Per-restaurant branding location + access patterns (RPC vs useAuth-loaded row) (PRs #3 + #4) - Customer-facing /orders/:id/{paid,cancel} live in admin app, not marketing site (PR #4) - Three-Stripe-environment gotcha (M8 lesson) - New "Open follow-ups" section at the bottom with three items — admin chrome branding, kitchen Ready SMS, Path B Vapi — each with scope, estimated effort, and where to start developer/developer-journal.md - New dated entry "May 2 (later)" picking up after the existing May 2 entry (which only covered through Path A demo). Documents PR #3 + PR #4, the dialtone_menu PR #27 reversal, the SQL cleanup of orphaned test orders, and the doc refresh itself - Decisions section: why columns on restaurants over a separate table, why RPC scoped by order_id over slug, why helpers extracted for testability, why hexToRgba falls back to opaque on bad input - Follow-ups section mirrors AGENTS.md "Open follow-ups" — same three items so journal readers + agent readers see the same plan 220 unit tests still pass; lint + typecheck green. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Adds six nullable columns to
restaurantsfor per-restaurant brand identity. Promoted ahead of M11 because the customer-facing post-payment landing pages (Stripe Checkout success/cancel) must render with the restaurant's brand, not DialTone's — a customer paying for an order is interacting with the restaurant they ordered from, not with DialTone the platform.Schema (
supabase/migrations/0007_restaurant_branding.sql)display_namerestaurants.namelogo_urlprimary_color#RRGGBBsecondary_color#RRGGBBfonttaglineAll nullable so existing rows don't need backfill. Render code falls back to safe defaults when fields are null.
Why columns on
restaurantsrather than a separaterestaurant_brandstableIf the relationship ever becomes 1:N (a tenant operating multiple branded storefronts), split out then.
Other changes
developer/planned-migrations/m11-restaurant-branding.sqlremoved — promoted, no longer plannedAGENTS.md"Planned migrations" line updated to note the queue is now empty and why this was promoted earlydeveloper/branding.mdrewritten — describes the chosen schema, query patterns, render-time fallbacks, and where the data is consumedsupabase/seed.sqlpopulates Sui's Sushi demo branding (#8B0000/#F5C14A/ Playfair Display / "Fresh sushi, made to order."); Rival Ramen left null to keep the fallback path exercisedpackages/shared/src/supabase/types.tsregeneratedApply state
klzznfagrtormretqsgb(db push). Three pre-existing pending_payment orders from earlier debugging still in place; new schema columns null on all rows except Sui's Sushi (seed-populated).Test plan
pnpm ci:fastgreen (188 unit tests, lint, typecheck)/orders/:id/{paid,cancel}pages in the admin app using these columns🤖 Generated with Claude Code
Greptile Summary
This PR adds six nullable branding columns to
restaurants(migration0007) and aSECURITY DEFINERRPC (get_restaurant_branding) in0008that exposes only those safe fields to theanonrole — enabling the upcoming unauthenticated post-payment landing pages to render with per-restaurant identity. All three issues flagged in the previous review round have been addressed: thelogo_urlscheme constraint, theanonaccess path, and theSECURITY DEFINERscoping by opaqueorder_idUUID are all correctly implemented in0008.get_restaurant_brandingRPC return type intypes.tsstill types all six branding fields asstringinstead ofstring | null, meaning TypeScript won't surface null-access errors that will occur at runtime for restaurants like Rival Ramen where all branding fields are null.Confidence Score: 4/5
Safe to merge with one outstanding P1 (nullable RPC return types in types.ts, previously flagged) still unresolved.
The schema and access-control work is solid — the SECURITY DEFINER RPC, the logo_url scheme constraint, and the revoke/grant pattern are all correct. The P1 ceiling is triggered by the unresolved nullable-typing issue in types.ts for get_restaurant_branding's return fields, which was raised in the previous review round but not fixed in this iteration and will cause runtime TypeErrors on null branding fields.
packages/shared/src/supabase/types.ts — the get_restaurant_branding return type needs branding fields typed as string | null.
Important Files Changed
restaurants;primary_color/secondary_colorhave correct hex regex constraints, butfontlacks a character-set guard against CSS injection.https?://scheme constraint onlogo_urland aSECURITY DEFINERRPC scoped by opaqueorder_idUUID, correctly revokes PUBLIC execute and grants only toanon/authenticated.restaurantscolumns are correctly typed as `stringorder_id; comprehensive and accurate.primary_color,secondary_color,font, andtagline; intentionally leaves Rival Ramen and thedisplay_name/logo_urlcolumns null to keep fallback paths exercised.m11-restaurant-branding.sqlwas promoted early.0007_restaurant_branding.sql.Sequence Diagram
sequenceDiagram participant Browser as Customer Browser (unauthenticated) participant AdminApp as Admin App (Cloudflare Worker) participant SupabaseAnon as Supabase (anon client) participant RPC as get_restaurant_branding() SECURITY DEFINER participant DB as PostgreSQL (orders + restaurants) Browser->>AdminApp: GET /orders/:id/paid AdminApp->>SupabaseAnon: rpc('get_restaurant_branding', { p_order_id }) SupabaseAnon->>RPC: EXECUTE (anon role) RPC->>DB: SELECT r.id, r.name, r.display_name, r.logo_url, r.primary_color FROM orders o JOIN restaurants r WHERE o.id = p_order_id DB-->>RPC: branding row (or empty if order not found) RPC-->>SupabaseAnon: { restaurant_id, name, display_name, logo_url, primary_color, ... } SupabaseAnon-->>AdminApp: data AdminApp-->>Browser: Render branded success page (falls back to DialTone defaults if any field null)Reviews (3): Last reviewed commit: "fix(branding): address Greptile P1+P2 — ..." | Re-trigger Greptile