feat: validate server and client env variables #149
Conversation
WalkthroughAdds runtime environment validation using Zod for server and client envs, invoked from next.config.mjs at module load. Introduces shouldSkipValidation() controlled by SKIP_ENV_VALIDATION (set to "true" in CI Build job). Includes new JS env files in TypeScript project scope and updates .env.example values. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CI as CI / Local Env
participant Next as Next.js startup
participant Cfg as next.config.mjs
participant Server as validateServerEnv()
participant Client as validateClientEnv()
participant Helper as shouldSkipValidation()
participant Zod as Zod
CI->>Next: start build / dev
Next->>Cfg: load next.config.mjs
Cfg->>Server: call validateServerEnv()
Server->>Helper: check SKIP_ENV_VALIDATION
alt SKIP_ENV_VALIDATION == "true"
Helper-->>Server: true (skip)
Server-->>Cfg: return (skipped)
else
Helper-->>Server: false
Server->>Zod: safeParse(server env)
Zod-->>Server: success / failure
alt failure
Server->>Server: log field errors
Server-->>Cfg: throw Error
else
Server-->>Cfg: return
end
end
Cfg->>Client: call validateClientEnv()
Client->>Helper: check SKIP_ENV_VALIDATION
alt SKIP_ENV_VALIDATION == "true"
Helper-->>Client: true (skip)
Client-->>Cfg: return (skipped)
else
Helper-->>Client: false
Client->>Zod: safeParse(client env)
Zod-->>Client: success / failure
alt failure
Client->>Client: log field errors
Client-->>Cfg: throw Error
else
Client-->>Cfg: return
end
end
Cfg-->>Next: export config (if validations pass)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings🪛 dotenv-linter (3.3.0).env.example[warning] 3-3: [QuoteCharacter] The value has quote characters (', ") (QuoteCharacter) [warning] 4-4: [QuoteCharacter] The value has quote characters (', ") (QuoteCharacter) [warning] 5-5: [QuoteCharacter] The value has quote characters (', ") (QuoteCharacter) [warning] 6-6: [QuoteCharacter] The value has quote characters (', ") (QuoteCharacter) [warning] 6-6: [UnorderedKey] The CURRENT_ENCRYPTION_VERSION key should go before the ENCRYPTION_KEY key (UnorderedKey) [warning] 7-7: [QuoteCharacter] The value has quote characters (', ") (QuoteCharacter) [warning] 8-8: [UnorderedKey] The REQUEST_API_KEY key should go before the REQUEST_API_URL key (UnorderedKey) [warning] 14-14: [UnorderedKey] The FEE_ADDRESS_FOR_PAYMENT key should go before the FEE_PERCENTAGE_FOR_PAYMENT key (UnorderedKey) [warning] 20-20: [QuoteCharacter] The value has quote characters (', ") (QuoteCharacter) 🔇 Additional comments (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/lib/env/helpers.ts (1)
1-5: Make isBuildTime robust to common truthy forms (true/TRUE/1).Prevents surprises across CI providers (some set CI=1).
Apply this diff:
-export function isBuildTime(): boolean { - return ( - process.env.SKIP_ENV_VALIDATION === "true" || process.env.CI === "true" - ); -} +export function isBuildTime(): boolean { + const isTrue = (v?: string) => + typeof v === "string" && (v === "1" || v.toLowerCase() === "true"); + return isTrue(process.env.SKIP_ENV_VALIDATION) || isTrue(process.env.CI); +}.github/workflows/build-and-lint.yml (1)
74-76: Minor: SKIP_ENV_VALIDATION may be redundant in GHA.GitHub Actions sets CI="true", which already triggers isBuildTime(). Keeping SKIP_ENV_VALIDATION is harmless, but you can drop it if you want fewer knobs.
src/lib/env/client.ts (1)
45-45: Remove clientEnv export or return the parsed env.validateClientEnv() returns void, so clientEnv is always undefined and can mislead consumers.
Apply this diff to remove the dead export:
-export const clientEnv = validateClientEnv();Optionally, return the parsed env instead (non-breaking for current usage):
-export function validateClientEnv() { +export function validateClientEnv(): ClientEnv | undefined { if (isBuildTime()) { console.warn( "⚠️ Skipping client environment variable validation at build time.", ); - return; + return undefined; } @@ - if (!result.success) { + if (!result.success) { console.error( "❌ Invalid client environment variables:", result.error.flatten().fieldErrors, ); throw new Error("Invalid client environment variables"); } + return result.data; } - -export const clientEnv = validateClientEnv();src/lib/env/server.ts (2)
33-47: Include VERCEL_URL in the validated env mapping (present in schema).Keeps schema and validation input in sync.
Apply this diff:
const env = { @@ INVOICE_PROCESSING_TTL: process.env.INVOICE_PROCESSING_TTL, + VERCEL_URL: process.env.VERCEL_URL, };
60-60: Remove serverEnv export or return the parsed env.validateServerEnv() returns void, so serverEnv is always undefined and can confuse future consumers.
Apply this diff to remove the dead export:
-export const serverEnv = validateServerEnv();Optionally, mirror the “return parsed env” pattern as in the client suggestion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/build-and-lint.yml(1 hunks)src/app/layout.tsx(1 hunks)src/lib/env/client.ts(1 hunks)src/lib/env/helpers.ts(1 hunks)src/lib/env/server.ts(1 hunks)src/middleware.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#147
File: src/lib/env/client.ts:4-12
Timestamp: 2025-09-23T11:39:27.397Z
Learning: In the RequestNetwork/easy-invoice project, bassgeta prefers to keep environment variable parsing localized to the consuming component when there's only one consumer, rather than centralizing the parsing logic in the schema validation layer.
🧬 Code graph analysis (4)
src/app/layout.tsx (1)
src/lib/env/client.ts (1)
validateClientEnv(16-43)
src/lib/env/server.ts (1)
src/lib/env/helpers.ts (1)
isBuildTime(1-5)
src/middleware.ts (1)
src/lib/env/server.ts (1)
validateServerEnv(25-58)
src/lib/env/client.ts (1)
src/lib/env/helpers.ts (1)
isBuildTime(1-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (1)
src/app/layout.tsx (1)
12-15: Client env validation wired at the root layout — looks good.This ensures early failure if required NEXT_PUBLIC_* vars are missing.
bassgeta
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
tsconfig.json (1)
24-31: Include all env JS helpers via a glob to avoid missing files (client.js is currently excluded).Only server.js and helpers.js are included; client.js isn’t in the TS project scope. Prefer a glob to cover present and future env JS files.
"include": [ "next-env.d.ts", "**/*.ts", "**/*.tsx", ".next/types/**/*.ts", - "src/lib/env/server.js", - "src/lib/env/helpers.js" + "src/lib/env/*.js" ],src/lib/env/helpers.js (1)
1-3: Make SKIP_ENV_VALIDATION parsing more robust (accept true/1/yes, case-insensitive).Prevents surprises across CI providers and local shells.
-export function shouldSkipValidation() { - return process.env.SKIP_ENV_VALIDATION === "true"; -} +export function shouldSkipValidation() { + const v = String(process.env.SKIP_ENV_VALIDATION ?? "").toLowerCase().trim(); + return v === "true" || v === "1" || v === "yes"; +}src/lib/env/server.js (1)
14-21: Optional: treat empty strings for optional vars as “unset”.Right now, optional strings pass even if set to "". If you want empty strings to fail validation (or be coerced to undefined), preprocess them.
Example pattern you can reuse:
const optionalNonEmpty = z.preprocess( (v) => (typeof v === "string" && v.trim() === "" ? undefined : v), z.string().min(1).optional() ); // Then: ENCRYPTION_KEY: optionalNonEmpty, CURRENT_ENCRYPTION_VERSION: optionalNonEmpty, FEE_PERCENTAGE_FOR_PAYMENT: optionalNonEmpty, FEE_ADDRESS_FOR_PAYMENT: optionalNonEmpty, REDIS_URL: z.preprocess((v) => (v === "" ? undefined : v), z.string().url().optional()), INVOICE_PROCESSING_TTL: optionalNonEmpty, VERCEL_URL: optionalNonEmpty,src/lib/env/client.js (1)
8-8: Validate GTM ID format for stronger guarantees.GTM IDs have a known shape (GTM-XXXXXXX). Enforce it instead of only min length.
- NEXT_PUBLIC_GTM_ID: z.string().min(1, "NEXT_PUBLIC_GTM_ID is required"), + NEXT_PUBLIC_GTM_ID: z + .string() + .regex(/^GTM-[A-Z0-9]+$/, "NEXT_PUBLIC_GTM_ID must look like GTM-XXXXXXX"),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
next.config.mjs(1 hunks)src/lib/env/client.js(1 hunks)src/lib/env/helpers.js(1 hunks)src/lib/env/server.js(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#147
File: src/lib/env/client.ts:4-12
Timestamp: 2025-09-23T11:39:27.397Z
Learning: In the RequestNetwork/easy-invoice project, bassgeta prefers to keep environment variable parsing localized to the consuming component when there's only one consumer, rather than centralizing the parsing logic in the schema validation layer.
🧬 Code graph analysis (3)
src/lib/env/server.js (2)
src/lib/env/helpers.js (1)
shouldSkipValidation(1-3)src/lib/env/client.js (2)
env(20-28)result(30-30)
next.config.mjs (2)
src/lib/env/server.js (1)
validateServerEnv(23-54)src/lib/env/client.js (1)
validateClientEnv(14-39)
src/lib/env/client.js (2)
src/lib/env/helpers.js (1)
shouldSkipValidation(1-3)src/lib/env/server.js (2)
env(29-43)result(45-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (2)
src/lib/env/client.js (1)
14-18: Skip guard is good; ensure this file isn’t imported from client bundles.This module references process.env directly. Today it’s only invoked from next.config.mjs, which is safe. Please keep it out of browser bundles.
Would you like me to scan the repo to confirm there are no client-side imports of src/lib/env/client.js?
next.config.mjs (1)
4-5: LGTM: early, centralized validation matches the “fail fast” objective without changing consumption sites.This aligns with the project preference to avoid centralizing parsing at call sites while still enforcing config at startup.
bassgeta
force-pushed
the
fix/110-fail-on-misconfigured-env
branch
from
c1fa3ce to
8f188bf
Compare
bassgeta
force-pushed
the
fix/110-fail-on-misconfigured-env
branch
from
8f188bf to
9bccecc
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
next.config.mjs (1)
4-5: Option: Invoke validation inside the exported factory to avoid surprising imports.Some tooling may import next.config without loading envs first. Moving validation into the exported function confines side effects to Next’s lifecycle.
Apply:
-import { validateClientEnv } from "./src/lib/env/client.js"; -import { validateServerEnv } from "./src/lib/env/server.js"; - -validateServerEnv(); -validateClientEnv(); +import { validateClientEnv } from "./src/lib/env/client.js"; +import { validateServerEnv } from "./src/lib/env/server.js"; @@ -const nextConfig = {}; - -export default nextConfig; +const nextConfig = {}; + +export default function defineConfig() { + validateServerEnv(); + validateClientEnv(); + return nextConfig; +}src/lib/env/client.js (1)
4-12: Strengthen schema: trim inputs, validate GTM shape, allow absolute or site-relative URLs, and validate TRUSTED_ORIGINS as CSV of origins.Prevents false negatives due to whitespace and better matches common patterns (GTM-*, /terms paths, comma-separated origins).
Apply:
-const clientEnvSchema = z.object({ - NEXT_PUBLIC_REOWN_PROJECT_ID: z - .string() - .min(1, "NEXT_PUBLIC_REOWN_PROJECT_ID is required"), - NEXT_PUBLIC_GTM_ID: z.string().min(1, "NEXT_PUBLIC_GTM_ID is required"), - NEXT_PUBLIC_DEMO_MEETING: z.string().url().optional(), - NEXT_PUBLIC_API_TERMS_CONDITIONS: z.string().url().optional(), - NEXT_PUBLIC_CRYPTO_TO_FIAT_TRUSTED_ORIGINS: z.string().optional(), -}); +const clientEnvSchema = z.object({ + NEXT_PUBLIC_REOWN_PROJECT_ID: z + .string() + .trim() + .min(1, "NEXT_PUBLIC_REOWN_PROJECT_ID is required"), + NEXT_PUBLIC_GTM_ID: z + .string() + .trim() + .regex(/^GTM-[A-Z0-9]+$/, "NEXT_PUBLIC_GTM_ID must look like GTM-XXXXXXX"), + NEXT_PUBLIC_DEMO_MEETING: z + .union([ + z.string().trim().url(), + z.string().trim().regex(/^\/[^\s]*$/, "Must be a site‑relative path"), + ]) + .optional(), + NEXT_PUBLIC_API_TERMS_CONDITIONS: z + .union([ + z.string().trim().url(), + z.string().trim().regex(/^\/[^\s]*$/, "Must be a site‑relative path"), + ]) + .optional(), + NEXT_PUBLIC_CRYPTO_TO_FIAT_TRUSTED_ORIGINS: z + .preprocess( + (v) => + typeof v === "string" + ? v.split(",").map((s) => s.trim()).filter(Boolean) + : v, + z.array( + z + .string() + .url() + .refine((s) => { + try { + const u = new URL(s); + return u.pathname === "/" && !u.search && !u.hash; + } catch { + return false; + } + }, "Must be an origin (e.g., https://example.com/)"), + ), + ) + .optional(), +});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
next.config.mjs(1 hunks)src/lib/env/client.js(1 hunks)src/lib/env/helpers.js(1 hunks)src/lib/env/server.js(1 hunks)tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/lib/env/helpers.js
- tsconfig.json
- src/lib/env/server.js
🧰 Additional context used
🧬 Code graph analysis (2)
next.config.mjs (2)
src/lib/env/server.js (1)
validateServerEnv(23-54)src/lib/env/client.js (1)
validateClientEnv(14-39)
src/lib/env/client.js (2)
src/lib/env/helpers.js (1)
shouldSkipValidation(1-3)src/lib/env/server.js (2)
env(29-43)result(45-45)
🔇 Additional comments (3)
src/lib/env/client.js (2)
32-38: Good failure mode and non-sensitive logging.You log field-level errors without echoing values and throw to fail fast. Perfect.
20-28: Sanity-check: absolute vs site-relative values for URLs.NEXT_PUBLIC_API_TERMS_CONDITIONS is already an absolute URL in .env.example (https://request.network/api-terms) — OK. NEXT_PUBLIC_DEMO_MEETING is optional and not present in the example; because client schema uses z.string().url().optional(), any site-relative value (e.g. "/demo" or "/terms") will fail validateClientEnv and throw. Either relax the schema to accept site-relative URLs or confirm all deployments provide absolute URLs. Relevant files: src/lib/env/client.js (schema), src/components/header.tsx and src/app/page.tsx (usages).
next.config.mjs (1)
1-5: Approve — zod present in dependenciesEarly fail-fast env validation at config load — good placement. package.json shows zod in dependencies (^3.24.2), not devDependencies.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/lib/env/server.js (3)
5-13: Trim required strings to reject whitespace-only values.
Without trimming, " " passes min(1). Add .trim() to required strings/URLs.Apply this diff:
DATABASE_URL: z.string().min(1, "DATABASE_URL is required"), - GOOGLE_CLIENT_ID: z.string().min(1, "GOOGLE_CLIENT_ID is required"), - GOOGLE_CLIENT_SECRET: z.string().min(1, "GOOGLE_CLIENT_SECRET is required"), - REQUEST_API_URL: z.string().url("REQUEST_API_URL must be a valid URL"), - REQUEST_API_KEY: z.string().min(1, "REQUEST_API_KEY is required"), - WEBHOOK_SECRET: z.string().min(1, "WEBHOOK_SECRET is required"), + GOOGLE_CLIENT_ID: z.string().trim().min(1, "GOOGLE_CLIENT_ID is required"), + GOOGLE_CLIENT_SECRET: z.string().trim().min(1, "GOOGLE_CLIENT_SECRET is required"), + REQUEST_API_URL: z.string().trim().url("REQUEST_API_URL must be a valid URL"), + REQUEST_API_KEY: z.string().trim().min(1, "REQUEST_API_KEY is required"), + WEBHOOK_SECRET: z.string().trim().min(1, "WEBHOOK_SECRET is required"), GOOGLE_REDIRECT_URI: z - .string() + .string().trim() .url("GOOGLE_REDIRECT_URI must be a valid URL"),
22-26: Prevent skipping validation in production.
If SKIP_ENV_VALIDATION leaks to prod, it bypasses safety. Guard it.Apply this diff:
export function validateServerEnv() { if (shouldSkipValidation()) { - console.warn("⚠️ Skipping server environment variable validation."); + if (process.env.NODE_ENV === "production") { + throw new Error("SKIP_ENV_VALIDATION is set in production; refusing to start."); + } + console.warn("⚠️ Skipping server environment variable validation."); return; }
44-52: Surface structured errors in the thrown message for CI logs.
Keeps values masked while making failures clearer in build output.Apply this diff:
- console.error( - "❌ Invalid server environment variables:", - result.error.flatten().fieldErrors, - ); - throw new Error("Invalid server environment variables"); + const fieldErrors = result.error.flatten().fieldErrors; + console.error("❌ Invalid server environment variables:", fieldErrors); + throw new Error( + "Invalid server environment variables: " + JSON.stringify(fieldErrors) + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
next.config.mjs(1 hunks)src/lib/env/client.js(1 hunks)src/lib/env/helpers.js(1 hunks)src/lib/env/server.js(1 hunks)tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- next.config.mjs
- src/lib/env/helpers.js
- tsconfig.json
- src/lib/env/client.js
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-24T12:28:41.766Z
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#149
File: src/lib/env/server.js:29-43
Timestamp: 2025-09-24T12:28:41.766Z
Learning: In the RequestNetwork/easy-invoice project, VERCEL_URL is a code remnant that is not actually used, so it should be removed from environment schemas rather than validated.
Applied to files:
src/lib/env/server.js
📚 Learning: 2025-09-23T11:39:27.397Z
Learnt from: bassgeta
PR: RequestNetwork/easy-invoice#147
File: src/lib/env/client.ts:4-12
Timestamp: 2025-09-23T11:39:27.397Z
Learning: In the RequestNetwork/easy-invoice project, bassgeta prefers to keep environment variable parsing localized to the consuming component when there's only one consumer, rather than centralizing the parsing logic in the schema validation layer.
Applied to files:
src/lib/env/server.js
🧬 Code graph analysis (1)
src/lib/env/server.js (2)
src/lib/env/helpers.js (1)
shouldSkipValidation(1-3)src/lib/env/client.js (2)
env(20-28)result(30-30)
🔇 Additional comments (2)
src/lib/env/server.js (2)
1-3: LGTM on imports and ESM usage.
Consistent ESM imports and correct Zod named import.
4-20: Good call removing VERCEL_URL from the schema.
Matches the project’s intent (unused), keeping validation focused on real vars.
Went with a very simple solution that just validates the environment, but never changes the import paths to the validated config.
We validate server environment variables when we create the middleware, so we explicitly do it only once.
Resolves #110
Summary by CodeRabbit