mirror of
https://github.com/tiennm99/bsk.git
synced 2026-09-20 05:11:50 +00:00
fix(scaffold): apply phase-0 review findings
- env: cross-check VERCEL_ENV against NEXT_PUBLIC_APP_ENV at boot so prod credentials cannot silently write into a dev keyspace - upstash: tighten cache-key regex (kebab + colon only); split SCAN patterns into their own validator so glob '*' is allowed only there - eslint: forbid raw @upstash/redis, @upstash/ratelimit, @supabase/supabase-js imports outside the named factory files - supabase/admin: harmonize 'use cache' guidance with CONTRIBUTING.md (safe inside cache; partition key on identity for user-specific reads) - app/layout: clarify global-error.tsx vs error.tsx shell requirements given the passthrough root layout - readme: Next.js 15 -> 16 (matches scaffolded version)
This commit is contained in:
1 parent
b88147059e
commit
10a3693f1b
7 files changed
+112
-16
No files matched your search
+1
-1
@@ -58,7 +58,7 @@ async function getDashboardData(userId: string) {
|
||||
|
||||
`lib/supabase/server.ts` reads cookies → never call it from a `'use cache'` function. Call it at the page/layout/Server-Action level and pass the data (not the client) into cached helpers.
|
||||
|
||||
`lib/supabase/admin.ts` does not depend on cookies and is safe to call from cached scopes — but only when the call is genuinely user-agnostic.
|
||||
`lib/supabase/admin.ts` does not depend on cookies and is safe to call inside `'use cache'`. When the result depends on a caller's identity (user, role, tenant), the cache key MUST partition on that identity — otherwise one user sees another user's data. For genuinely user-agnostic reads (e.g. clinic settings, services list), no key partitioning is needed.
|
||||
|
||||
### 4. Realtime placement
|
||||
|
||||
|
||||
@@ -12,7 +12,7 @@ Planning phase. See [PLAN.md](./PLAN.md) for the architecture and phased roadmap
|
||||
|
||||
## Stack
|
||||
|
||||
- **pnpm** + **Next.js 15** (App Router) + **TypeScript**
|
||||
- **pnpm** + **Next.js 16** (App Router) + **TypeScript**
|
||||
- **Supabase** (Postgres + Auth + Storage) — shared across personal projects via schema-per-app
|
||||
- **Upstash** Redis + QStash — shared across personal projects via key prefixes
|
||||
- **Vercel** for hosting
|
||||
|
||||
@@ -3,6 +3,16 @@ import type { ReactNode } from "react";
|
||||
// Intentionally empty pass-through. The real `<html>` + `<body>` live in
|
||||
// `app/[locale]/layout.tsx` so `lang={locale}` and the i18n provider can
|
||||
// be set per locale. Do not put global UI here.
|
||||
//
|
||||
// `global-error.tsx` (anywhere in the tree) MUST render its own
|
||||
// `<html><body>` — Next.js renders it instead of the root layout. This rule
|
||||
// is universal, not specific to this codebase.
|
||||
//
|
||||
// `error.tsx` normally inherits its closest layout's shell. But because the
|
||||
// root layout here is a passthrough, a hypothetical `app/error.tsx` would
|
||||
// render without `<html><body>`. Prefer placing `error.tsx` files under
|
||||
// `app/[locale]/` (where the real shell lives), or render the document
|
||||
// shell explicitly inside any root-level error file.
|
||||
export default function RootLayout({ children }: { children: ReactNode }) {
|
||||
return children;
|
||||
}
|
||||
@@ -13,8 +13,47 @@ const config = [
|
||||
"error",
|
||||
{ argsIgnorePattern: "^_", varsIgnorePattern: "^_" },
|
||||
],
|
||||
// Force every call site through the prefixed / schema-scoped factories.
|
||||
// Raw clients bypass the bsk:{env}: Redis prefix and the schema='bsk'
|
||||
// scoping, which collide with sibling apps sharing the same project.
|
||||
"no-restricted-imports": [
|
||||
"error",
|
||||
{
|
||||
paths: [
|
||||
{
|
||||
name: "@upstash/redis",
|
||||
message:
|
||||
"Import the `cache` / `createRateLimiter` helpers from '@/lib/upstash' instead. Raw Redis bypasses the bsk:{env}: key prefix and can collide with sibling apps.",
|
||||
},
|
||||
{
|
||||
name: "@upstash/ratelimit",
|
||||
message:
|
||||
"Use `createRateLimiter` from '@/lib/upstash' instead — it bakes in the bsk:{env}:ratelimit prefix.",
|
||||
},
|
||||
{
|
||||
name: "@supabase/supabase-js",
|
||||
message:
|
||||
"Import the schema-scoped factory from '@/lib/supabase/{server,client,admin}' instead. Raw createClient bypasses db.schema='bsk' and reads/writes leak to public.",
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
// Only the named factory files may import the raw infrastructure libs.
|
||||
// Explicit filenames (not a glob) keep the trust boundary tight — adding a
|
||||
// new factory should be a deliberate PR change here, not an accidental
|
||||
// file landing under `lib/supabase/*`.
|
||||
{
|
||||
files: [
|
||||
"lib/upstash.ts",
|
||||
"lib/supabase/server.ts",
|
||||
"lib/supabase/client.ts",
|
||||
"lib/supabase/admin.ts",
|
||||
"lib/supabase/session.ts",
|
||||
],
|
||||
rules: { "no-restricted-imports": "off" },
|
||||
},
|
||||
];
|
||||
|
||||
export default config;
|
||||
Vendored
+33
-10
@@ -2,19 +2,42 @@ import "server-only";
|
||||
import { z } from "zod";
|
||||
import { APP_SLUG, SUPABASE_SCHEMA, clientEnv } from "./client";
|
||||
|
||||
const serverSchema = z.object({
|
||||
NODE_ENV: z.enum(["development", "preview", "production", "test"]).default("development"),
|
||||
const VERCEL_TO_APP_ENV = {
|
||||
production: "prod",
|
||||
preview: "preview",
|
||||
development: "dev",
|
||||
} as const;
|
||||
|
||||
SUPABASE_SECRET_KEY: z.string().min(1),
|
||||
const serverSchema = z
|
||||
.object({
|
||||
NODE_ENV: z.enum(["development", "preview", "production", "test"]).default("development"),
|
||||
// Vercel sets VERCEL_ENV automatically on every deploy. Absent → local dev.
|
||||
VERCEL_ENV: z.enum(["production", "preview", "development"]).optional(),
|
||||
|
||||
UPSTASH_REDIS_REST_URL: z.url(),
|
||||
UPSTASH_REDIS_REST_TOKEN: z.string().min(1),
|
||||
SUPABASE_SECRET_KEY: z.string().min(1),
|
||||
|
||||
QSTASH_URL: z.url().optional(),
|
||||
QSTASH_TOKEN: z.string().optional(),
|
||||
QSTASH_CURRENT_SIGNING_KEY: z.string().optional(),
|
||||
QSTASH_NEXT_SIGNING_KEY: z.string().optional(),
|
||||
});
|
||||
UPSTASH_REDIS_REST_URL: z.url(),
|
||||
UPSTASH_REDIS_REST_TOKEN: z.string().min(1),
|
||||
|
||||
QSTASH_URL: z.url().optional(),
|
||||
QSTASH_TOKEN: z.string().optional(),
|
||||
QSTASH_CURRENT_SIGNING_KEY: z.string().optional(),
|
||||
QSTASH_NEXT_SIGNING_KEY: z.string().optional(),
|
||||
})
|
||||
.superRefine((env, ctx) => {
|
||||
// Cross-check NEXT_PUBLIC_APP_ENV against VERCEL_ENV so prod credentials
|
||||
// never silently write into a dev keyspace (and vice-versa). Skipped
|
||||
// locally (no VERCEL_ENV) where the developer owns their .env.local.
|
||||
if (!env.VERCEL_ENV) return;
|
||||
const expected = VERCEL_TO_APP_ENV[env.VERCEL_ENV];
|
||||
if (clientEnv.NEXT_PUBLIC_APP_ENV !== expected) {
|
||||
ctx.addIssue({
|
||||
code: z.ZodIssueCode.custom,
|
||||
path: ["NEXT_PUBLIC_APP_ENV"],
|
||||
message: `VERCEL_ENV=${env.VERCEL_ENV} requires NEXT_PUBLIC_APP_ENV=${expected}, got "${clientEnv.NEXT_PUBLIC_APP_ENV}". Set NEXT_PUBLIC_APP_ENV in Vercel project settings for this environment.`,
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
const parsed = serverSchema.safeParse(process.env);
|
||||
|
||||
|
||||
@@ -5,7 +5,13 @@ import { serverEnv, SUPABASE_SCHEMA } from "@/lib/env/server";
|
||||
/**
|
||||
* Privileged Supabase client (uses the secret key, bypasses RLS on behalf of no user).
|
||||
* Use only for admin tasks: invites, cron sweeps, system writes.
|
||||
* NEVER expose this client to the browser or pass its results through `'use cache'`.
|
||||
* NEVER expose this client to the browser.
|
||||
*
|
||||
* `'use cache'` interaction: this factory does NOT read cookies, so it is safe
|
||||
* to call inside a cached scope. But if the result depends on a caller's identity
|
||||
* (user / role / tenant), the cache key MUST partition on that identity — otherwise
|
||||
* one user sees another user's data. For genuinely user-agnostic reads (e.g.
|
||||
* clinic settings, services list), no key partitioning is needed.
|
||||
*/
|
||||
export function createSupabaseAdminClient() {
|
||||
return createClient(serverEnv.NEXT_PUBLIC_SUPABASE_URL, serverEnv.SUPABASE_SECRET_KEY, {
|
||||
|
||||
+21
-3
@@ -7,6 +7,15 @@ import { serverEnv, redisKeyPrefix } from "@/lib/env/server";
|
||||
const RATE_LIMIT_NS = "ratelimit";
|
||||
const CACHE_NS = "cache";
|
||||
|
||||
// Cache keys: lowercase alphanumerics, '-' for words, ':' for sub-namespacing.
|
||||
// Forbids spaces, glob chars (* ? [ ]), control chars, uppercase. Matches
|
||||
// the strictness of `createRateLimiter` so callers cannot accidentally write
|
||||
// keys that interact with SCAN globs or pollute neighboring apps.
|
||||
const KEY_RE = /^[a-z0-9][a-z0-9:-]*$/;
|
||||
|
||||
// SCAN patterns allow '*' for sweeps but are otherwise constrained.
|
||||
const SCAN_PATTERN_RE = /^[a-z0-9:*-]+$/;
|
||||
|
||||
type Json = string | number | boolean | null | { [k: string]: Json } | Json[];
|
||||
|
||||
const redis = new Redis({
|
||||
@@ -15,12 +24,21 @@ const redis = new Redis({
|
||||
});
|
||||
|
||||
function withPrefix(ns: string, key: string) {
|
||||
if (!key || key.includes(" ")) {
|
||||
throw new Error(`Invalid cache key: ${JSON.stringify(key)}`);
|
||||
if (!KEY_RE.test(key)) {
|
||||
throw new Error(
|
||||
`Invalid cache key (lowercase + digits + '-' + ':' only, must start with alphanumeric): ${JSON.stringify(key)}`,
|
||||
);
|
||||
}
|
||||
return `${redisKeyPrefix}:${ns}:${key}`;
|
||||
}
|
||||
|
||||
function withScanPattern(ns: string, pattern: string) {
|
||||
if (!SCAN_PATTERN_RE.test(pattern)) {
|
||||
throw new Error(`Invalid SCAN pattern: ${JSON.stringify(pattern)}`);
|
||||
}
|
||||
return `${redisKeyPrefix}:${ns}:${pattern}`;
|
||||
}
|
||||
|
||||
export const cache = {
|
||||
async get<T extends Json>(key: string): Promise<T | null> {
|
||||
return redis.get<T>(withPrefix(CACHE_NS, key));
|
||||
@@ -42,7 +60,7 @@ export const cache = {
|
||||
*/
|
||||
async scan(matchSuffix: string, cursor: string | number = 0) {
|
||||
return redis.scan(cursor, {
|
||||
match: withPrefix(CACHE_NS, matchSuffix),
|
||||
match: withScanPattern(CACHE_NS, matchSuffix),
|
||||
count: 100,
|
||||
});
|
||||
},
|
||||
|
||||
Reference in new issue
Block a user