-
Notifications
You must be signed in to change notification settings - Fork 73
feat: secure and validate environment configuration #160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,31 @@ | ||
| # ─── API ─────────────────────────────────────────── | ||
| NEXT_PUBLIC_API_URL=http://localhost:5000 | ||
| # Base URL of the dnb-backend REST API | ||
|
|
||
| NEXT_PUBLIC_AI_API_URL=http://localhost:8000 | ||
| # Base URL of the dnb-ai FastAPI service for the AI assistant | ||
|
|
||
| # ─── Stellar ─────────────────────────────────────── | ||
| NEXT_PUBLIC_STELLAR_NETWORK=testnet | ||
| # Stellar network to use: "testnet" or "mainnet" | ||
|
|
||
| # ─── Cloudinary ──────────────────────────────────── | ||
| NEXT_PUBLIC_CLOUDINARY_CLOUD_NAME=your_cloudinary_cloud_name | ||
| NEXT_PUBLIC_CLOUDINARY_API_KEY=your_cloudinary_api_key | ||
| NEXT_PUBLIC_CLOUDINARY_API_SECRET=your_cloudinary_api_secret | ||
| NEXT_PUBLIC_CLOUDINARY_URL=your_cloudinary_url | ||
| # Cloudinary cloud name for unsigned uploads (required) | ||
|
|
||
| # ─── Jitsi ───────────────────────────────────────── | ||
| NEXT_PUBLIC_JITSI_DOMAIN=https://meet.jit.si | ||
| # NODE_ENV=production | ||
| NEXT_PUBLIC_SOCKET_URL=https://dnb-backend-api.onrender.com | ||
| DNB_API_URL=https://dnb-backend-api.onrender.com | ||
| NEXT_PUBLIC_API_URL=https://dnb-backend-api.onrender.com | ||
| NEXT_PUBLIC_STELLAR_NETWORK=testnet | ||
| # Jitsi Meet domain for video spaces | ||
|
|
||
| NEXT_PUBLIC_JITSI_REQUIRE_JWT=false | ||
| # Set to "true" if the Jitsi deployment requires a signed JWT token | ||
|
|
||
| # ─── Firebase ───────────────────────────────────── | ||
| NEXT_PUBLIC_FIREBASE_API_KEY=AIzaSyC8LlmtlWXvbcbyVbdyv4r-tDsGhhukdag | ||
| NEXT_PUBLIC_FIREBASE_AUTH_DOMAIN=deen-bridge-22195.firebaseapp.com | ||
| NEXT_PUBLIC_FIREBASE_PROJECT_ID=deen-bridge-22195 | ||
| NEXT_PUBLIC_FIREBASE_STORAGE_BUCKET=deen-bridge-22195.firebasestorage.app | ||
| NEXT_PUBLIC_FIREBASE_MESSAGING_SENDER_ID=368531944242 | ||
| NEXT_PUBLIC_FIREBASE_APP_ID=1:368531944242:web:7994b11820741a69d35d2b | ||
| NEXT_PUBLIC_FIREBASE_MEASUREMENT_ID=G-ZZ81THLVCC | ||
| # Firebase Web SDK config values (not secrets, but environment-specific) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,13 @@ | ||
| import axios from "axios"; | ||
| import { config } from "@/lib/config/env"; | ||
|
|
||
| export async function POST(req) { | ||
| try { | ||
| const { message, chat_id, user_id } = await req.json(); | ||
| console.log("Sending message to AI:", message); | ||
| console.log("Chat ID:", chat_id, "User ID:", user_id); | ||
|
Comment on lines
7
to
8
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Do not log raw AI prompts or user identifiers.
🤖 Prompt for AI Agents |
||
|
|
||
| // Use environment variable or fallback to localhost | ||
| const AI_API_URL = | ||
| process.env.NEXT_PUBLIC_AI_API_URL || "http://localhost:8000"; | ||
| const AI_API_URL = config.aiApiUrl; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Make environment fallbacks development-only. The centralized configuration currently allows missing production values to resolve to localhost or testnet, silently targeting the wrong service or Stellar network instead of failing startup.
📍 Affects 8 files
🤖 Prompt for AI Agents |
||
| const endpoint = `${AI_API_URL}/chat`; | ||
|
|
||
| console.log("Using AI API URL:", endpoint); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ import JaasMeetingComponent from "@/components/organisms/jitsi/JitsiMeeting"; | |
| import { joinSpaceWaitlist } from "@/lib/actions/spaces/joinSpaceWaitlist"; | ||
| import { updateSpace } from "@/lib/actions/spaces/updateSpace"; | ||
| import { getSpaceMeetingToken } from "@/lib/actions/calls/get-space-meeting-token"; | ||
| import { config } from "@/lib/config/env"; | ||
|
|
||
| const normalizeDomain = (domain = "meet.jit.si") => | ||
| domain.replace(/^https?:\/\//i, "").replace(/\/+$/g, ""); | ||
|
|
@@ -39,9 +40,7 @@ export default function JaasMeetingClientButtons({ space }) { | |
| const [isCopying, setIsCopying] = useState(false); | ||
| const [joinLoading, setJoinLoading] = useState(false); | ||
| const [tokenLoading, setTokenLoading] = useState(false); | ||
| const envRequiresJwt = | ||
| typeof process !== "undefined" && | ||
| process.env.NEXT_PUBLIC_JITSI_REQUIRE_JWT === "true"; | ||
| const envRequiresJwt = config.jitsiRequireJwt; | ||
|
|
||
| const [meetingToken, setMeetingToken] = useState(null); | ||
| const [requiresJwt, setRequiresJwt] = useState( | ||
|
|
@@ -52,8 +51,7 @@ export default function JaasMeetingClientButtons({ space }) { | |
| [requiresJwt, envRequiresJwt] | ||
| ); | ||
| const [meetingMeta, setMeetingMeta] = useState(() => { | ||
| const domain = | ||
| process.env.NEXT_PUBLIC_JITSI_DOMAIN || "meet.jit.si"; | ||
| const domain = config.jitsiDomain; | ||
| const normalizedDomain = normalizeDomain(domain); | ||
|
Comment on lines
+54
to
55
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Treat a blank Jitsi domain as unset. The shared schema accepts Also applies to: 82-84 🤖 Prompt for AI Agents |
||
| const fallbackRoom = | ||
| space?.meetingRoom || (space?._id ? `deenbridge-space-${space._id}` : ""); | ||
|
|
@@ -82,7 +80,7 @@ export default function JaasMeetingClientButtons({ space }) { | |
| }, [waitListIds, user?._id]); | ||
|
|
||
| const baseDomain = useMemo( | ||
| () => normalizeDomain(process.env.NEXT_PUBLIC_JITSI_DOMAIN || "meet.jit.si"), | ||
| () => normalizeDomain(config.jitsiDomain), | ||
| [] | ||
| ); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,132 @@ | ||
| import { z } from "zod"; | ||
|
|
||
| function warn(field, fallback) { | ||
| if (process.env.NODE_ENV !== "production") { | ||
| console.warn( | ||
| `\u26a0\ufe0f ${field} is not set. Using "${fallback}" as fallback.` | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| const dangerous = Object.keys(process.env).filter((k) => | ||
| /^NEXT_PUBLIC_.*SECRET/.test(k) | ||
| ); | ||
|
|
||
| if (dangerous.length > 0) { | ||
| throw new Error( | ||
| `\u274c Dangerous environment variable(s) detected: ${dangerous.join(", ")}.\n` + | ||
| `Variables prefixed with NEXT_PUBLIC_ are exposed to the browser bundle and must never contain secrets.\n` + | ||
| `Rename the variable(s) to remove the NEXT_PUBLIC_ prefix.` | ||
| ); | ||
| } | ||
|
|
||
| const raw = { | ||
| NEXT_PUBLIC_API_URL: process.env.NEXT_PUBLIC_API_URL, | ||
| NEXT_PUBLIC_AI_API_URL: process.env.NEXT_PUBLIC_AI_API_URL, | ||
| NEXT_PUBLIC_STELLAR_NETWORK: process.env.NEXT_PUBLIC_STELLAR_NETWORK, | ||
| NEXT_PUBLIC_CLOUDINARY_CLOUD_NAME: | ||
| process.env.NEXT_PUBLIC_CLOUDINARY_CLOUD_NAME, | ||
| NEXT_PUBLIC_JITSI_DOMAIN: process.env.NEXT_PUBLIC_JITSI_DOMAIN, | ||
| NEXT_PUBLIC_JITSI_REQUIRE_JWT: process.env.NEXT_PUBLIC_JITSI_REQUIRE_JWT, | ||
| NEXT_PUBLIC_FIREBASE_API_KEY: process.env.NEXT_PUBLIC_FIREBASE_API_KEY, | ||
| NEXT_PUBLIC_FIREBASE_AUTH_DOMAIN: | ||
| process.env.NEXT_PUBLIC_FIREBASE_AUTH_DOMAIN, | ||
| NEXT_PUBLIC_FIREBASE_PROJECT_ID: | ||
| process.env.NEXT_PUBLIC_FIREBASE_PROJECT_ID, | ||
| NEXT_PUBLIC_FIREBASE_STORAGE_BUCKET: | ||
| process.env.NEXT_PUBLIC_FIREBASE_STORAGE_BUCKET, | ||
| NEXT_PUBLIC_FIREBASE_MESSAGING_SENDER_ID: | ||
| process.env.NEXT_PUBLIC_FIREBASE_MESSAGING_SENDER_ID, | ||
| NEXT_PUBLIC_FIREBASE_APP_ID: process.env.NEXT_PUBLIC_FIREBASE_APP_ID, | ||
| NEXT_PUBLIC_FIREBASE_MEASUREMENT_ID: | ||
| process.env.NEXT_PUBLIC_FIREBASE_MEASUREMENT_ID, | ||
| }; | ||
|
|
||
| const envSchema = z.object({ | ||
| NEXT_PUBLIC_API_URL: z.string().url().optional(), | ||
| NEXT_PUBLIC_AI_API_URL: z.string().url().optional(), | ||
| NEXT_PUBLIC_STELLAR_NETWORK: z.enum(["testnet", "mainnet"]).optional(), | ||
| NEXT_PUBLIC_CLOUDINARY_CLOUD_NAME: z | ||
| .string() | ||
| .min(1, "Cloudinary cloud name is required"), | ||
| NEXT_PUBLIC_JITSI_DOMAIN: z.string().optional(), | ||
| NEXT_PUBLIC_JITSI_REQUIRE_JWT: z.enum(["true", "false"]).optional(), | ||
| NEXT_PUBLIC_FIREBASE_API_KEY: z.string().optional(), | ||
| NEXT_PUBLIC_FIREBASE_AUTH_DOMAIN: z.string().optional(), | ||
| NEXT_PUBLIC_FIREBASE_PROJECT_ID: z.string().optional(), | ||
| NEXT_PUBLIC_FIREBASE_STORAGE_BUCKET: z.string().optional(), | ||
| NEXT_PUBLIC_FIREBASE_MESSAGING_SENDER_ID: z.string().optional(), | ||
| NEXT_PUBLIC_FIREBASE_APP_ID: z.string().optional(), | ||
| NEXT_PUBLIC_FIREBASE_MEASUREMENT_ID: z.string().optional(), | ||
| }); | ||
|
|
||
| const parsed = envSchema.safeParse(raw); | ||
|
|
||
| if (!parsed.success) { | ||
| const issues = parsed.error.errors.map((e) => { | ||
| const path = e.path.join("."); | ||
| return ` - ${path}: ${e.message}`; | ||
| }); | ||
| throw new Error( | ||
| `\u274c Environment configuration is invalid:\n${issues.join("\n")}\n\nPlease check your .env.local file.` | ||
| ); | ||
| } | ||
|
|
||
| const env = parsed.data; | ||
|
|
||
| export const config = Object.freeze({ | ||
| get apiUrl() { | ||
| return ( | ||
| env.NEXT_PUBLIC_API_URL ?? | ||
| (warn("NEXT_PUBLIC_API_URL", "http://localhost:5000"), | ||
| "http://localhost:5000") | ||
| ); | ||
| }, | ||
|
|
||
| get aiApiUrl() { | ||
| return ( | ||
| env.NEXT_PUBLIC_AI_API_URL ?? | ||
| (warn("NEXT_PUBLIC_AI_API_URL", "http://localhost:8000"), | ||
| "http://localhost:8000") | ||
| ); | ||
| }, | ||
|
|
||
| get stellarNetwork() { | ||
| return env.NEXT_PUBLIC_STELLAR_NETWORK ?? "testnet"; | ||
| }, | ||
|
|
||
| get cloudinaryCloudName() { | ||
| return env.NEXT_PUBLIC_CLOUDINARY_CLOUD_NAME; | ||
| }, | ||
|
|
||
| get jitsiDomain() { | ||
| return env.NEXT_PUBLIC_JITSI_DOMAIN ?? "meet.jit.si"; | ||
| }, | ||
|
|
||
| get jitsiRequireJwt() { | ||
| return env.NEXT_PUBLIC_JITSI_REQUIRE_JWT === "true"; | ||
| }, | ||
|
|
||
| get firebase() { | ||
| return { | ||
| apiKey: | ||
| env.NEXT_PUBLIC_FIREBASE_API_KEY ?? | ||
| "AIzaSyC8LlmtlWXvbcbyVbdyv4r-tDsGhhukdag", | ||
| authDomain: | ||
| env.NEXT_PUBLIC_FIREBASE_AUTH_DOMAIN ?? | ||
| "deen-bridge-22195.firebaseapp.com", | ||
| projectId: | ||
| env.NEXT_PUBLIC_FIREBASE_PROJECT_ID ?? "deen-bridge-22195", | ||
| storageBucket: | ||
| env.NEXT_PUBLIC_FIREBASE_STORAGE_BUCKET ?? | ||
| "deen-bridge-22195.firebasestorage.app", | ||
| messagingSenderId: | ||
| env.NEXT_PUBLIC_FIREBASE_MESSAGING_SENDER_ID ?? "368531944242", | ||
| appId: | ||
| env.NEXT_PUBLIC_FIREBASE_APP_ID ?? | ||
| "1:368531944242:web:7994b11820741a69d35d2b", | ||
| measurementId: | ||
| env.NEXT_PUBLIC_FIREBASE_MEASUREMENT_ID ?? "G-ZZ81THLVCC", | ||
| }; | ||
| }, | ||
|
Comment on lines
+76
to
+132
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Fallback warnings are inconsistent across getters. Only ♻️ Suggested refactor: unify via a shared helper+function withFallback(field, value, fallback) {
+ if (value === undefined) {
+ warn(field, fallback);
+ return fallback;
+ }
+ return value;
+}
+
export const config = Object.freeze({
get apiUrl() {
- return (
- env.NEXT_PUBLIC_API_URL ??
- (warn("NEXT_PUBLIC_API_URL", "http://localhost:5000"),
- "http://localhost:5000")
- );
+ return withFallback("NEXT_PUBLIC_API_URL", env.NEXT_PUBLIC_API_URL, "http://localhost:5000");
},
get aiApiUrl() {
- return (
- env.NEXT_PUBLIC_AI_API_URL ??
- (warn("NEXT_PUBLIC_AI_API_URL", "http://localhost:8000"),
- "http://localhost:8000")
- );
+ return withFallback("NEXT_PUBLIC_AI_API_URL", env.NEXT_PUBLIC_AI_API_URL, "http://localhost:8000");
},
get stellarNetwork() {
- return env.NEXT_PUBLIC_STELLAR_NETWORK ?? "testnet";
+ return withFallback("NEXT_PUBLIC_STELLAR_NETWORK", env.NEXT_PUBLIC_STELLAR_NETWORK, "testnet");
},
get jitsiDomain() {
- return env.NEXT_PUBLIC_JITSI_DOMAIN ?? "meet.jit.si";
+ return withFallback("NEXT_PUBLIC_JITSI_DOMAIN", env.NEXT_PUBLIC_JITSI_DOMAIN, "meet.jit.si");
},
get jitsiRequireJwt() {
- return env.NEXT_PUBLIC_JITSI_REQUIRE_JWT === "true";
+ return withFallback("NEXT_PUBLIC_JITSI_REQUIRE_JWT", env.NEXT_PUBLIC_JITSI_REQUIRE_JWT, "false") === "true";
},
get firebase() {
return {
apiKey:
- env.NEXT_PUBLIC_FIREBASE_API_KEY ??
- "AIzaSyC8LlmtlWXvbcbyVbdyv4r-tDsGhhukdag",
+ withFallback("NEXT_PUBLIC_FIREBASE_API_KEY", env.NEXT_PUBLIC_FIREBASE_API_KEY, "AIzaSyC8LlmtlWXvbcbyVbdyv4r-tDsGhhukdag"),
// ...apply the same pattern to authDomain, projectId, storageBucket,
// messagingSenderId, appId, measurementId
};
},
});This also removes the repeated comma-operator idiom, which is easy to misread. 🧰 Tools🪛 Betterleaks (1.7.0)[high] 114-114: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches. (gcp-api-key) 🤖 Prompt for AI Agents
Comment on lines
+111
to
+132
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Real Firebase Web API key/project identifiers hardcoded and duplicated across two files. The same live Firebase credentials appear both as source-code fallback defaults and as ".env.example" values, and Betterleaks independently flags both sites as a leaked GCP API key. As per path instructions,
Since these values are already committed to git history, also consider rotating/restricting the Firebase Web API key (HTTP referrer + API restrictions in Google Cloud Console) as a follow-up, independent of this code change. 🧰 Tools🪛 Betterleaks (1.7.0)[high] 114-114: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches. (gcp-api-key) 📍 Affects 2 files
🤖 Prompt for AI AgentsSources: Path instructions, Linters/SAST tools |
||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Example value includes a protocol prefix that the code doesn't expect.
lib/config/env.js'sjitsiDomaingetter falls back to the bare domain"meet.jit.si"(no protocol), and the README documents the same bare-domain default. This example useshttps://meet.jit.si. If a contributor copies this literally and downstream code builds a URL likehttps://${jitsiDomain}/...(or passes it to the Jitsi Meet External API, which expects a bare domain), the protocol gets duplicated.🩹 Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents