feat: env validate with zod for nestjs projects#257
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughRuntime configuration validation is added: Zod is introduced, TypeScript config interfaces are replaced by Zod schemas, environment validation modules are added, and ConfigModule.forRoot(...) is wired to call the new validate functions in both document and search-worker apps. ChangesSchema-driven configuration validation (Zod)
Sequence Diagram(s)sequenceDiagram
participant NestJS_App as NestJS App
participant ConfigModule as ConfigModule.forRoot
participant EnvSchema as envSchema.validate
participant AppSchema as appConfigSchema.parse
NestJS_App->>ConfigModule: startup -> forRoot({ validate })
ConfigModule->>EnvSchema: validate(process.env)
EnvSchema-->>ConfigModule: validated Env
ConfigModule->>AppSchema: parse(Env-derived inputs)
AppSchema-->>ConfigModule: validated AppConfig
ConfigModule-->>NestJS_App: boot with validated config
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/document/src/config/config.factory.ts`:
- Line 62: The brokers array is built with .split(',') without trimming or
filtering empty tokens, causing entries like "host1:9092, host2:9092" to include
whitespace and invalid brokers; update the brokers construction (the brokers
property in the config factory) to normalize entries the same way as the
authentication CSVs by splitting on ',', then applying .map(v => v.trim()) and
.filter(Boolean) so whitespace is removed and empty tokens dropped before schema
validation and Kafka client use.
In `@apps/document/src/config/config.ts`:
- Around line 42-44: The authenticationConfigSchema currently allows empty
arrays and arbitrary strings for jwksUrls; update authenticationConfigSchema so
jwksUrls is a non-empty array of valid URLs (e.g., use
z.array(z.string().url()).nonempty() or equivalent) to enforce URL format and
non-empty constraint at schema level; keep AuthenticationService constructor
checks as a safeguard but remove reliance on those to catch basic validation
errors (reference authenticationConfigSchema, jwksUrls, and the
AuthenticationService constructor/new URL() logic when making the change).
In `@apps/document/src/config/env.validation.ts`:
- Around line 8-11: Validate port environment variables NOTOPIA_DOCUMENT_PORT
and NOTOPIA_DOCUMENT_DB_PORT to only allow integers in the TCP port range:
replace the current z.coerce.number() usage with a coerced integer and range
checks (e.g., z.coerce.number().int().min(1).max(65535).default(...)) so
negative, zero, decimal, and >65535 values are rejected during schema
validation; update the schema entries for NOTOPIA_DOCUMENT_PORT and
NOTOPIA_DOCUMENT_DB_PORT accordingly.
In `@apps/search-worker/src/config.ts`:
- Line 6: The port schema currently uses port: z.number() which allows invalid
TCP ports; update the port schema to enforce an integer in the valid TCP range
(1–65535) by replacing z.number() with a constrained integer (e.g., use
.int().min(1).max(65535)) on the port symbol in apps/search-worker/src/config.ts
and keep compatibility with the parseInt usage in config.factory.ts.
- Line 12: The brokers schema currently uses brokers: z.array(z.string()) and
thus accepts empty or whitespace-only strings; update the validation to reject
such entries by using a refined string predicate (e.g., trim() length > 0) or
z.string().min(1).refine(...), and ensure config.factory.ts that splits the env
var produces trimmed entries before validation (trim each item and filter out
empty strings) so invalid broker entries like '' or ' ' are caught and
rejected by the schema (reference brokers, config.factory.ts, and the Zod schema
declaration).
- Line 18: The current config schema in apps/search-worker/src/config.ts lets
empty/invalid Meilisearch hosts through; update the host property validation
(the "host" field in your config/schema) to require a full URL with protocol by
using a URL validator instead of a plain z.string(), e.g. replace z.string()
with a zod URL check (z.string().url(...)) or add a refine/regex that enforces
http:// or https:// and non-empty value so invalid hosts fail at parse time;
update any error message to clearly state "host must be a full URL including
http:// or https://".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0c4b8664-443d-45ca-9016-3a750f744a0b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
apps/document/package.jsonapps/document/src/app.module.tsapps/document/src/config/config.factory.tsapps/document/src/config/config.tsapps/document/src/config/env.validation.tsapps/document/src/config/index.tsapps/document/src/global.d.tsapps/search-worker/package.jsonapps/search-worker/src/app.module.tsapps/search-worker/src/config.factory.tsapps/search-worker/src/config.tsapps/search-worker/src/env.validation.ts
💤 Files with no reviewable changes (1)
- apps/document/src/global.d.ts
|
@coderabbitai autofix please fix those you suggested. But by the way, please remove those local and default env stuff, we want some envs mandatory provided, not autofill. And remove global type node env in search worker if you can |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 5 file(s) based on 6 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 5 file(s) based on 6 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/search-worker/src/config.ts (1)
12-12:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRequire at least one Kafka broker after sanitisation.
Line 12 allows an empty brokers array. With the current split/trim/filter flow, values like an empty env var or
",,"can resolve to[], pass schema parsing, and fail later during Kafka client setup.Suggested fix
export const kafkaConfigSchema = z.object({ clientId: z.string(), - brokers: z.array(z.string().min(1)), + brokers: z.array(z.string().min(1)).min(1, 'at least one kafka broker is required'), groupId: z.string(), });#!/bin/bash set -euo pipefail echo "Inspect broker schema:" cat -n apps/search-worker/src/config.ts | sed -n '10,15p' echo echo "Inspect broker sanitisation path:" cat -n apps/search-worker/src/config.factory.ts | sed -n '23,29p' echo echo "Verify array-level minimum is currently missing:" rg -n "brokers:\s*z\.array\(z\.string\(\)\.min\(1\)\)(?!\.min)" apps/search-worker/src/config.ts -PExpected result: schema line matches without
.min(1)on the array, while factory shows.filter(Boolean), confirming[]is currently reachable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/search-worker/src/config.ts` at line 12, The brokers array in the Zod config schema currently allows an empty array after sanitisation; update the brokers entry (the brokers property in your config schema, currently defined as brokers: z.array(z.string().min(1))) to enforce at least one element by appending .min(1) to the array (e.g., z.array(z.string().min(1)).min(1)); keep the existing string-level .min(1) and leave the sanitisation in config.factory.ts as-is so values are still split/trim/filtered but will now fail schema validation when the resulting array is empty.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/document/src/config/config.ts`:
- Around line 35-39: The kafkaConfigSchema currently allows an empty brokers
array which can pass validation but later break Kafka client init; update the
schema definition for kafkaConfigSchema so the brokers property is validated as
a non-empty array (use z.array(...).nonempty()) so configuration parsing fails
fast when NOTOPIA_DOCUMENT_KAFKA_BROKERS yields an empty/whitespace-only result;
modify the brokers field in kafkaConfigSchema accordingly.
In `@apps/document/src/config/env.validation.ts`:
- Around line 9-27: The listed env schema entries currently use .default('')
which permits missing values; update each required key
(NOTOPIA_DOCUMENT_API_URL, NOTOPIA_DOCUMENT_DB_PASSWORD,
NOTOPIA_DOCUMENT_SERVICES_NOTE_GRPC_URL,
NOTOPIA_DOCUMENT_SERVICES_AUTHORIZATION_GRPC_URL, NOTOPIA_DOCUMENT_S3_ENDPOINT,
NOTOPIA_DOCUMENT_S3_ACCESS_KEY_ID, NOTOPIA_DOCUMENT_S3_SECRET_ACCESS_KEY,
NOTOPIA_DOCUMENT_AUTHENTICATION_JWKS_URLS) to use z.string().min(1) instead of
.default('') so validation enforces a non-empty value; locate these properties
in the env validation object (the schema entries like NOTOPIA_DOCUMENT_API_URL,
NOTOPIA_DOCUMENT_DB_PASSWORD, etc.) and replace .default('') with .min(1) while
leaving other entries (e.g., DB port, optional issuers/audiences) unchanged.
---
Duplicate comments:
In `@apps/search-worker/src/config.ts`:
- Line 12: The brokers array in the Zod config schema currently allows an empty
array after sanitisation; update the brokers entry (the brokers property in your
config schema, currently defined as brokers: z.array(z.string().min(1))) to
enforce at least one element by appending .min(1) to the array (e.g.,
z.array(z.string().min(1)).min(1)); keep the existing string-level .min(1) and
leave the sanitisation in config.factory.ts as-is so values are still
split/trim/filtered but will now fail schema validation when the resulting array
is empty.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bc3c0ff4-9172-47a9-9d00-ca150f87daa8
📒 Files selected for processing (5)
apps/document/src/config/config.factory.tsapps/document/src/config/config.tsapps/document/src/config/env.validation.tsapps/search-worker/src/config.factory.tsapps/search-worker/src/config.ts
| export const kafkaConfigSchema = z.object({ | ||
| clientId: z.string(), | ||
| brokers: z.array(z.string()), | ||
| groupId: z.string(), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current broker schema and CSV normalisation path
rg -n -C2 "kafkaConfigSchema|brokers:" apps/document/src/config/config.ts apps/document/src/config/config.factory.tsRepository: notopia-uit/notopia
Length of output: 1697
🏁 Script executed:
#!/bin/bash
# Get complete kafkaConfig factory function to see full broker processing
sed -n '59,70p' apps/document/src/config/config.factory.tsRepository: notopia-uit/notopia
Length of output: 523
🏁 Script executed:
#!/bin/bash
# Check if there's any .filter() or other processing after .map()
rg -A10 "brokers.*\.split" apps/document/src/config/config.factory.tsRepository: notopia-uit/notopia
Length of output: 45
Enforce non-empty brokers array in Kafka schema.
The brokers field currently allows an empty array. If NOTOPIA_DOCUMENT_KAFKA_BROKERS is set to whitespace only (e.g., ' '), the processing chain .split(',').map(v => v.trim()).filter(Boolean) yields an empty array that passes schema validation but fails at Kafka client initialisation. Add .nonempty() to fail fast at configuration parsing time.
Suggested fix
export const kafkaConfigSchema = z.object({
clientId: z.string(),
- brokers: z.array(z.string()),
+ brokers: z.array(z.string()).nonempty(),
groupId: z.string(),
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const kafkaConfigSchema = z.object({ | |
| clientId: z.string(), | |
| brokers: z.array(z.string()), | |
| groupId: z.string(), | |
| }); | |
| export const kafkaConfigSchema = z.object({ | |
| clientId: z.string(), | |
| brokers: z.array(z.string()).nonempty(), | |
| groupId: z.string(), | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/document/src/config/config.ts` around lines 35 - 39, The
kafkaConfigSchema currently allows an empty brokers array which can pass
validation but later break Kafka client init; update the schema definition for
kafkaConfigSchema so the brokers property is validated as a non-empty array (use
z.array(...).nonempty()) so configuration parsing fails fast when
NOTOPIA_DOCUMENT_KAFKA_BROKERS yields an empty/whitespace-only result; modify
the brokers field in kafkaConfigSchema accordingly.
| NOTOPIA_DOCUMENT_API_URL: z.string().default(''), | ||
| NOTOPIA_DOCUMENT_DB_HOST: z.string().default('localhost'), | ||
| NOTOPIA_DOCUMENT_DB_PORT: z.coerce.number().int().min(1).max(65535).default(5434), | ||
| NOTOPIA_DOCUMENT_DB_USER: z.string().default('postgres'), | ||
| NOTOPIA_DOCUMENT_DB_PASSWORD: z.string().default(''), | ||
| NOTOPIA_DOCUMENT_DB_NAME: z.string().default('document'), | ||
| NOTOPIA_DOCUMENT_SERVICES_NOTE_GRPC_URL: z.string().default(''), | ||
| NOTOPIA_DOCUMENT_SERVICES_AUTHORIZATION_GRPC_URL: z.string().default(''), | ||
| NOTOPIA_DOCUMENT_S3_ENDPOINT: z.string().default(''), | ||
| NOTOPIA_DOCUMENT_S3_REGION: z.string().default('us-east-1'), | ||
| NOTOPIA_DOCUMENT_S3_ACCESS_KEY_ID: z.string().default(''), | ||
| NOTOPIA_DOCUMENT_S3_SECRET_ACCESS_KEY: z.string().default(''), | ||
| NOTOPIA_DOCUMENT_S3_BUCKET_NAME: z.string().default('document'), | ||
| NOTOPIA_DOCUMENT_KAFKA_CLIENT_ID: z.string().default('document'), | ||
| NOTOPIA_DOCUMENT_KAFKA_BROKERS: z.string().default('localhost:19092'), | ||
| NOTOPIA_DOCUMENT_KAFKA_GROUP_ID: z.string().default('document'), | ||
| NOTOPIA_DOCUMENT_AUTHENTICATION_JWKS_URLS: z.string().default(''), | ||
| NOTOPIA_DOCUMENT_AUTHENTICATION_ISSUERS: z.string().optional(), | ||
| NOTOPIA_DOCUMENT_AUTHENTICATION_AUDIENCES: z.string().optional(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify which env keys are currently defaulted to empty string
rg -n "default\\(''\\)" apps/document/src/config/env.validation.tsRepository: notopia-uit/notopia
Length of output: 588
Remove .default('') from operationally required environment variables.
Eight keys currently default to empty strings, allowing missing environment variables to pass validation and cause runtime failures downstream. Apply constraints to force these mandatory settings to be explicitly provided.
Lines affected: 9, 13, 15–17, 19–20, 25
NOTOPIA_DOCUMENT_API_URL, NOTOPIA_DOCUMENT_DB_PASSWORD, NOTOPIA_DOCUMENT_SERVICES_NOTE_GRPC_URL, NOTOPIA_DOCUMENT_SERVICES_AUTHORIZATION_GRPC_URL, NOTOPIA_DOCUMENT_S3_ENDPOINT, NOTOPIA_DOCUMENT_S3_ACCESS_KEY_ID, NOTOPIA_DOCUMENT_S3_SECRET_ACCESS_KEY, and NOTOPIA_DOCUMENT_AUTHENTICATION_JWKS_URLS must be non-empty. Replace .default('') with .min(1) for each.
Suggested changes
- NOTOPIA_DOCUMENT_API_URL: z.string().default(''),
+ NOTOPIA_DOCUMENT_API_URL: z.string().min(1),
- NOTOPIA_DOCUMENT_DB_PASSWORD: z.string().default(''),
+ NOTOPIA_DOCUMENT_DB_PASSWORD: z.string().min(1),
- NOTOPIA_DOCUMENT_SERVICES_NOTE_GRPC_URL: z.string().default(''),
- NOTOPIA_DOCUMENT_SERVICES_AUTHORIZATION_GRPC_URL: z.string().default(''),
+ NOTOPIA_DOCUMENT_SERVICES_NOTE_GRPC_URL: z.string().min(1),
+ NOTOPIA_DOCUMENT_SERVICES_AUTHORIZATION_GRPC_URL: z.string().min(1),
- NOTOPIA_DOCUMENT_S3_ENDPOINT: z.string().default(''),
- NOTOPIA_DOCUMENT_S3_ACCESS_KEY_ID: z.string().default(''),
- NOTOPIA_DOCUMENT_S3_SECRET_ACCESS_KEY: z.string().default(''),
+ NOTOPIA_DOCUMENT_S3_ENDPOINT: z.string().min(1),
+ NOTOPIA_DOCUMENT_S3_ACCESS_KEY_ID: z.string().min(1),
+ NOTOPIA_DOCUMENT_S3_SECRET_ACCESS_KEY: z.string().min(1),
- NOTOPIA_DOCUMENT_AUTHENTICATION_JWKS_URLS: z.string().default(''),
+ NOTOPIA_DOCUMENT_AUTHENTICATION_JWKS_URLS: z.string().min(1),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| NOTOPIA_DOCUMENT_API_URL: z.string().default(''), | |
| NOTOPIA_DOCUMENT_DB_HOST: z.string().default('localhost'), | |
| NOTOPIA_DOCUMENT_DB_PORT: z.coerce.number().int().min(1).max(65535).default(5434), | |
| NOTOPIA_DOCUMENT_DB_USER: z.string().default('postgres'), | |
| NOTOPIA_DOCUMENT_DB_PASSWORD: z.string().default(''), | |
| NOTOPIA_DOCUMENT_DB_NAME: z.string().default('document'), | |
| NOTOPIA_DOCUMENT_SERVICES_NOTE_GRPC_URL: z.string().default(''), | |
| NOTOPIA_DOCUMENT_SERVICES_AUTHORIZATION_GRPC_URL: z.string().default(''), | |
| NOTOPIA_DOCUMENT_S3_ENDPOINT: z.string().default(''), | |
| NOTOPIA_DOCUMENT_S3_REGION: z.string().default('us-east-1'), | |
| NOTOPIA_DOCUMENT_S3_ACCESS_KEY_ID: z.string().default(''), | |
| NOTOPIA_DOCUMENT_S3_SECRET_ACCESS_KEY: z.string().default(''), | |
| NOTOPIA_DOCUMENT_S3_BUCKET_NAME: z.string().default('document'), | |
| NOTOPIA_DOCUMENT_KAFKA_CLIENT_ID: z.string().default('document'), | |
| NOTOPIA_DOCUMENT_KAFKA_BROKERS: z.string().default('localhost:19092'), | |
| NOTOPIA_DOCUMENT_KAFKA_GROUP_ID: z.string().default('document'), | |
| NOTOPIA_DOCUMENT_AUTHENTICATION_JWKS_URLS: z.string().default(''), | |
| NOTOPIA_DOCUMENT_AUTHENTICATION_ISSUERS: z.string().optional(), | |
| NOTOPIA_DOCUMENT_AUTHENTICATION_AUDIENCES: z.string().optional(), | |
| NOTOPIA_DOCUMENT_API_URL: z.string().min(1), | |
| NOTOPIA_DOCUMENT_DB_HOST: z.string().default('localhost'), | |
| NOTOPIA_DOCUMENT_DB_PORT: z.coerce.number().int().min(1).max(65535).default(5434), | |
| NOTOPIA_DOCUMENT_DB_USER: z.string().default('postgres'), | |
| NOTOPIA_DOCUMENT_DB_PASSWORD: z.string().min(1), | |
| NOTOPIA_DOCUMENT_DB_NAME: z.string().default('document'), | |
| NOTOPIA_DOCUMENT_SERVICES_NOTE_GRPC_URL: z.string().min(1), | |
| NOTOPIA_DOCUMENT_SERVICES_AUTHORIZATION_GRPC_URL: z.string().min(1), | |
| NOTOPIA_DOCUMENT_S3_ENDPOINT: z.string().min(1), | |
| NOTOPIA_DOCUMENT_S3_REGION: z.string().default('us-east-1'), | |
| NOTOPIA_DOCUMENT_S3_ACCESS_KEY_ID: z.string().min(1), | |
| NOTOPIA_DOCUMENT_S3_SECRET_ACCESS_KEY: z.string().min(1), | |
| NOTOPIA_DOCUMENT_S3_BUCKET_NAME: z.string().default('document'), | |
| NOTOPIA_DOCUMENT_KAFKA_CLIENT_ID: z.string().default('document'), | |
| NOTOPIA_DOCUMENT_KAFKA_BROKERS: z.string().default('localhost:19092'), | |
| NOTOPIA_DOCUMENT_KAFKA_GROUP_ID: z.string().default('document'), | |
| NOTOPIA_DOCUMENT_AUTHENTICATION_JWKS_URLS: z.string().min(1), | |
| NOTOPIA_DOCUMENT_AUTHENTICATION_ISSUERS: z.string().optional(), | |
| NOTOPIA_DOCUMENT_AUTHENTICATION_AUDIENCES: z.string().optional(), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/document/src/config/env.validation.ts` around lines 9 - 27, The listed
env schema entries currently use .default('') which permits missing values;
update each required key (NOTOPIA_DOCUMENT_API_URL,
NOTOPIA_DOCUMENT_DB_PASSWORD, NOTOPIA_DOCUMENT_SERVICES_NOTE_GRPC_URL,
NOTOPIA_DOCUMENT_SERVICES_AUTHORIZATION_GRPC_URL, NOTOPIA_DOCUMENT_S3_ENDPOINT,
NOTOPIA_DOCUMENT_S3_ACCESS_KEY_ID, NOTOPIA_DOCUMENT_S3_SECRET_ACCESS_KEY,
NOTOPIA_DOCUMENT_AUTHENTICATION_JWKS_URLS) to use z.string().min(1) instead of
.default('') so validation enforces a non-empty value; locate these properties
in the env validation object (the schema entries like NOTOPIA_DOCUMENT_API_URL,
NOTOPIA_DOCUMENT_DB_PASSWORD, etc.) and replace .default('') with .min(1) while
leaving other entries (e.g., DB port, optional issuers/audiences) unchanged.
Summary by CodeRabbit