Fix: Resolved startup NullPointerException and environmental placeholders#402
Fix: Resolved startup NullPointerException and environmental placeholders#402Sneha41sb wants to merge 1 commit intoPSMRI:mainfrom
Conversation
📝 WalkthroughWalkthroughThis PR establishes Docker infrastructure with MySQL containerization and introduces application configuration properties for database credentials, CORS settings, JWT authentication, communication services (SMS/email), Redis connectivity, and file handling. Database configuration classes are updated with null-check fallbacks and credential management. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
|
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/iemr/common/config/CorsConfig.java (1)
12-25:⚠️ Potential issue | 🟠 MajorDo not use wildcard origins with credentials enabled; configure explicit trusted origins instead.
The combination of
cors.allowed-origins:*default withallowCredentials(true)violates the CORS specification. Browsers will reject credentialed requests whenAccess-Control-Allow-Originis set to*, rendering this configuration either broken or a security liability. Always use an explicit allow-list of trusted origins when credentials are enabled.Suggested change
- `@Value`("${cors.allowed-origins:*}") + `@Value`("${cors.allowed-origins:http://localhost:3000}") private String allowedOrigins;For production, configure only the specific origins your application trusts (e.g., via environment variables or properties files) instead of relying on defaults.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/iemr/common/config/CorsConfig.java` around lines 12 - 25, The CORS config uses a wildcard default for allowedOrigins together with allowCredentials(true), which violates the CORS spec; update addCorsMappings in CorsConfig to require explicit trusted origins (parse allowedOrigins and reject or warn on "*" or empty), and when credentials are enabled only call allowedOriginPatterns/allowedOrigins with a concrete list (from the allowedOrigins property) or disable allowCredentials; specifically, modify the logic around the allowedOrigins field and addCorsMappings to validate that allowedOrigins does not contain "*" before applying allowCredentials(true) or otherwise set allowCredentials(false) and log/throw an error so production must supply explicit origins.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker-compose.yml`:
- Around line 8-11: The docker-compose service currently hardcodes
MYSQL_ROOT_PASSWORD ("password"), which is insecure; update the
docker-compose.yml to remove the fixed credential and instead read
MYSQL_ROOT_PASSWORD from an external source (e.g., a .env file or an environment
variable) and ensure a strong default is not committed—replace the literal
assignment with a variable reference (keep the key MYSQL_ROOT_PASSWORD) and
document that developers must supply a secure password via .env or CI secrets;
if necessary, add a note to ignore the .env in version control.
In `@src/main/java/com/iemr/common/config/PrimaryDBConfig.java`:
- Around line 76-87: The code in PrimaryDBConfig currently masks missing
datasource credentials by defaulting dbUser/dbPass to "root"/"password"; remove
these dangerous fallbacks and make the configuration fail-fast: when
ConfigProperties.getPropertyByName("spring.datasource.username") or
"...password" returns null or blank, log a clear error and throw an
IllegalStateException (or propagate a configuration exception) rather than
calling datasource.setUsername/dbPass fallback; update the initialization around
dbUser, dbPass, and the datasource.setUsername/datasource.setPassword calls to
validate the values and abort startup if either credential is missing.
In `@src/main/java/com/iemr/common/config/SecondaryDBConfig.java`:
- Around line 73-83: The code in SecondaryDBConfig currently falls back to
hardcoded "root"/"password" when
ConfigProperties.getPropertyByName("secondary.datasource.username"/"secondary.datasource.password")
is missing, which is unsafe; instead validate the configuration via
ConfigProperties in the SecondaryDBConfig initialization, log a clear error with
logger (include the missing property key), and fail fast by throwing an
unchecked exception (e.g., IllegalStateException) before calling
datasource.setUsername or datasource.setPassword; ensure you check both the
username and password (no silent defaults), and update the datasource only after
successful validation.
In `@src/main/resources/application.properties`:
- Line 385: There are duplicate property keys (spring.datasource.url and
send-message-url) defined multiple times causing last-value-wins behavior; open
the properties file and consolidate each duplicated key into a single canonical
declaration (or move environment/profile-specific variants into proper spring
profiles or externalized config), remove or merge the redundant entries
referencing spring.datasource.url and send-message-url, and ensure any intended
different values are placed under distinct profiles (e.g.,
application-dev.properties) or renamed so the correct value is explicit and
unambiguous.
- Around line 379-388: The properties file currently contains committed
plaintext secrets (jwt.secret, spring.datasource.username,
spring.datasource.password, encryption-key, mail/sms passwords and similar keys)
— replace these concrete values with environment-backed placeholders (e.g.
${JWT_SECRET:}, ${SPRING_DATASOURCE_USERNAME:}, ${SPRING_DATASOURCE_PASSWORD:})
or remove them entirely, and load actual secrets from environment
variables/secret store at runtime; ensure default empty or non-sensitive
fallbacks are not used, update any references to jwt.secret,
spring.datasource.*, encryption-key, and mail/sms credentials to use the
placeholders so secrets are not present in the repo.
- Around line 358-359: The file currently forces
springdoc.swagger-ui.enabled=true and springdoc.api-docs.enabled=true which
enables Swagger/docs in all environments; change these defaults to false and
make them environment-configurable so production remains safe: set
springdoc.swagger-ui.enabled=false and springdoc.api-docs.enabled=false in the
shared defaults, and document/expect overrides via environment properties or
Spring profiles (e.g. override with JVM/env vars or profile-specific
application-*.properties) when you want to enable Swagger; update any README or
deployment configs to show how to set springdoc.swagger-ui.enabled=true for
non-production environments.
---
Outside diff comments:
In `@src/main/java/com/iemr/common/config/CorsConfig.java`:
- Around line 12-25: The CORS config uses a wildcard default for allowedOrigins
together with allowCredentials(true), which violates the CORS spec; update
addCorsMappings in CorsConfig to require explicit trusted origins (parse
allowedOrigins and reject or warn on "*" or empty), and when credentials are
enabled only call allowedOriginPatterns/allowedOrigins with a concrete list
(from the allowedOrigins property) or disable allowCredentials; specifically,
modify the logic around the allowedOrigins field and addCorsMappings to validate
that allowedOrigins does not contain "*" before applying allowCredentials(true)
or otherwise set allowCredentials(false) and log/throw an error so production
must supply explicit origins.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 70651994-a381-4599-beb2-4b4cdceff111
📒 Files selected for processing (5)
docker-compose.ymlsrc/main/java/com/iemr/common/config/CorsConfig.javasrc/main/java/com/iemr/common/config/PrimaryDBConfig.javasrc/main/java/com/iemr/common/config/SecondaryDBConfig.javasrc/main/resources/application.properties
| MYSQL_ROOT_PASSWORD: password | ||
| MYSQL_DATABASE: amrit_common | ||
| ports: | ||
| - "3306:3306" |
There was a problem hiding this comment.
Avoid committing fixed database root credentials.
Line 8 hardcodes a weak root password (password). This tends to leak into non-local usage and weakens default security posture.
Suggested change
services:
db:
image: mysql:8.0
container_name: amrit-mysql
restart: always
environment:
- MYSQL_ROOT_PASSWORD: password
+ MYSQL_ROOT_PASSWORD: ${MYSQL_ROOT_PASSWORD:?set MYSQL_ROOT_PASSWORD}
MYSQL_DATABASE: amrit_common
+ MYSQL_USER: ${MYSQL_USER:amrit_user}
+ MYSQL_PASSWORD: ${MYSQL_PASSWORD:?set MYSQL_PASSWORD}📝 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.
| MYSQL_ROOT_PASSWORD: password | |
| MYSQL_DATABASE: amrit_common | |
| ports: | |
| - "3306:3306" | |
| MYSQL_ROOT_PASSWORD: ${MYSQL_ROOT_PASSWORD:?set MYSQL_ROOT_PASSWORD} | |
| MYSQL_DATABASE: amrit_common | |
| MYSQL_USER: ${MYSQL_USER:amrit_user} | |
| MYSQL_PASSWORD: ${MYSQL_PASSWORD:?set MYSQL_PASSWORD} | |
| ports: | |
| - "3306:3306" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docker-compose.yml` around lines 8 - 11, The docker-compose service currently
hardcodes MYSQL_ROOT_PASSWORD ("password"), which is insecure; update the
docker-compose.yml to remove the fixed credential and instead read
MYSQL_ROOT_PASSWORD from an external source (e.g., a .env file or an environment
variable) and ensure a strong default is not committed—replace the literal
assignment with a variable reference (keep the key MYSQL_ROOT_PASSWORD) and
document that developers must supply a secure password via .env or CI secrets;
if necessary, add a note to ignore the .env in version control.
| String dbUser = ConfigProperties.getPropertyByName("spring.datasource.username"); | ||
| String dbPass = ConfigProperties.getPropertyByName("spring.datasource.password"); | ||
|
|
||
|
|
||
| if (dbUser == null) { | ||
| logger.error("Critical Error: 'spring.datasource.username' is missing from configuration!"); | ||
|
|
||
| dbUser = "root"; | ||
| } | ||
|
|
||
| datasource.setUsername(dbUser); | ||
| datasource.setPassword(dbPass != null ? dbPass : "password"); |
There was a problem hiding this comment.
Do not mask datasource credential misconfiguration with root/password fallback.
Reading credentials via ConfigProperties + fallback defaults can hide broken env/secret injection and unintentionally connect with privileged local creds. Prefer required properties and fail fast.
Suggested change
- String dbUser = ConfigProperties.getPropertyByName("spring.datasource.username");
- String dbPass = ConfigProperties.getPropertyByName("spring.datasource.password");
-
- if (dbUser == null) {
- logger.error("Critical Error: 'spring.datasource.username' is missing from configuration!");
- dbUser = "root";
- }
-
- datasource.setUsername(dbUser);
- datasource.setPassword(dbPass != null ? dbPass : "password");
+ String dbUser = ConfigProperties.getPropertyByName("spring.datasource.username");
+ String dbPass = ConfigProperties.getPropertyByName("spring.datasource.password");
+ if (dbUser == null || dbUser.isBlank() || dbPass == null || dbPass.isBlank()) {
+ throw new IllegalStateException(
+ "Missing required primary datasource credentials (spring.datasource.username/password)");
+ }
+ datasource.setUsername(dbUser);
+ datasource.setPassword(dbPass);📝 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.
| String dbUser = ConfigProperties.getPropertyByName("spring.datasource.username"); | |
| String dbPass = ConfigProperties.getPropertyByName("spring.datasource.password"); | |
| if (dbUser == null) { | |
| logger.error("Critical Error: 'spring.datasource.username' is missing from configuration!"); | |
| dbUser = "root"; | |
| } | |
| datasource.setUsername(dbUser); | |
| datasource.setPassword(dbPass != null ? dbPass : "password"); | |
| String dbUser = ConfigProperties.getPropertyByName("spring.datasource.username"); | |
| String dbPass = ConfigProperties.getPropertyByName("spring.datasource.password"); | |
| if (dbUser == null || dbUser.isBlank() || dbPass == null || dbPass.isBlank()) { | |
| throw new IllegalStateException( | |
| "Missing required primary datasource credentials (spring.datasource.username/password)"); | |
| } | |
| datasource.setUsername(dbUser); | |
| datasource.setPassword(dbPass); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/common/config/PrimaryDBConfig.java` around lines 76 -
87, The code in PrimaryDBConfig currently masks missing datasource credentials
by defaulting dbUser/dbPass to "root"/"password"; remove these dangerous
fallbacks and make the configuration fail-fast: when
ConfigProperties.getPropertyByName("spring.datasource.username") or
"...password" returns null or blank, log a clear error and throw an
IllegalStateException (or propagate a configuration exception) rather than
calling datasource.setUsername/dbPass fallback; update the initialization around
dbUser, dbPass, and the datasource.setUsername/datasource.setPassword calls to
validate the values and abort startup if either credential is missing.
| String secondaryUser = ConfigProperties.getPropertyByName("secondary.datasource.username"); | ||
| String secondaryPass = ConfigProperties.getPropertyByName("secondary.datasource.password"); | ||
|
|
||
| if (secondaryUser == null) { | ||
| logger.error("Critical Error: 'secondary.datasource.username' is missing!"); | ||
| secondaryUser = "root"; // Defaulting to root prevents the crash | ||
| } | ||
|
|
||
| datasource.setUsername(secondaryUser); | ||
| datasource.setPassword(secondaryPass != null ? secondaryPass : "password"); | ||
|
|
There was a problem hiding this comment.
Fail fast instead of silently defaulting to root/password.
This change prevents an NPE, but it introduces risky behavior: misconfiguration now silently falls back to privileged credentials. That can mask environment issues and connect to the wrong database.
Suggested change
- if (secondaryUser == null) {
- logger.error("Critical Error: 'secondary.datasource.username' is missing!");
- secondaryUser = "root"; // Defaulting to root prevents the crash
- }
-
- datasource.setUsername(secondaryUser);
- datasource.setPassword(secondaryPass != null ? secondaryPass : "password");
+ if (secondaryUser == null || secondaryUser.isBlank() ||
+ secondaryPass == null || secondaryPass.isBlank()) {
+ throw new IllegalStateException(
+ "Missing required secondary datasource credentials (secondary.datasource.username/password)");
+ }
+ datasource.setUsername(secondaryUser);
+ datasource.setPassword(secondaryPass);📝 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.
| String secondaryUser = ConfigProperties.getPropertyByName("secondary.datasource.username"); | |
| String secondaryPass = ConfigProperties.getPropertyByName("secondary.datasource.password"); | |
| if (secondaryUser == null) { | |
| logger.error("Critical Error: 'secondary.datasource.username' is missing!"); | |
| secondaryUser = "root"; // Defaulting to root prevents the crash | |
| } | |
| datasource.setUsername(secondaryUser); | |
| datasource.setPassword(secondaryPass != null ? secondaryPass : "password"); | |
| String secondaryUser = ConfigProperties.getPropertyByName("secondary.datasource.username"); | |
| String secondaryPass = ConfigProperties.getPropertyByName("secondary.datasource.password"); | |
| if (secondaryUser == null || secondaryUser.isBlank() || | |
| secondaryPass == null || secondaryPass.isBlank()) { | |
| throw new IllegalStateException( | |
| "Missing required secondary datasource credentials (secondary.datasource.username/password)"); | |
| } | |
| datasource.setUsername(secondaryUser); | |
| datasource.setPassword(secondaryPass); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/iemr/common/config/SecondaryDBConfig.java` around lines 73
- 83, The code in SecondaryDBConfig currently falls back to hardcoded
"root"/"password" when
ConfigProperties.getPropertyByName("secondary.datasource.username"/"secondary.datasource.password")
is missing, which is unsafe; instead validate the configuration via
ConfigProperties in the SecondaryDBConfig initialization, log a clear error with
logger (include the missing property key), and fail fast by throwing an
unchecked exception (e.g., IllegalStateException) before calling
datasource.setUsername or datasource.setPassword; ensure you check both the
username and password (no silent defaults), and update the datasource only after
successful validation.
| springdoc.swagger-ui.enabled=true | ||
| springdoc.api-docs.enabled=true |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the recommended production defaults for springdoc.swagger-ui.enabled and springdoc.api-docs.enabled in Spring Boot applications?
💡 Result:
Recommended Springdoc production defaults (i.e., what you should set to in production) for Spring Boot are: 1) springdoc.swagger-ui.enabled=false This disables the Swagger UI endpoint (default is /swagger-ui.html). 2) springdoc.api-docs.enabled=false This disables the OpenAPI JSON endpoint (default is /v3/api-docs). If you do not set these properties, both are enabled by default (their documented default value is true).
Citations:
- 1: https://springdoc.org/v1/ui-properties.html
- 2: https://springdoc.org/features.html
- 3: https://springdoc.org/properties.html
- 4: https://springdoc.org/v1/properties.html
- 5: http://springdoc.org/
- 6: https://springdoc.org/
Don't hard-enable Swagger/docs in shared defaults.
Setting both springdoc flags to true in application.properties exposes Swagger UI and API documentation endpoints across all environments by default. Production deployments should disable these unless explicitly needed. Use environment-driven configuration with safe defaults (false).
Suggested change
-springdoc.swagger-ui.enabled=true
-springdoc.api-docs.enabled=true
+springdoc.swagger-ui.enabled=${SPRINGDOC_SWAGGER_UI_ENABLED:false}
+springdoc.api-docs.enabled=${SPRINGDOC_API_DOCS_ENABLED:false}📝 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.
| springdoc.swagger-ui.enabled=true | |
| springdoc.api-docs.enabled=true | |
| springdoc.swagger-ui.enabled=${SPRINGDOC_SWAGGER_UI_ENABLED:false} | |
| springdoc.api-docs.enabled=${SPRINGDOC_API_DOCS_ENABLED:false} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/application.properties` around lines 358 - 359, The file
currently forces springdoc.swagger-ui.enabled=true and
springdoc.api-docs.enabled=true which enables Swagger/docs in all environments;
change these defaults to false and make them environment-configurable so
production remains safe: set springdoc.swagger-ui.enabled=false and
springdoc.api-docs.enabled=false in the shared defaults, and document/expect
overrides via environment properties or Spring profiles (e.g. override with
JVM/env vars or profile-specific application-*.properties) when you want to
enable Swagger; update any README or deployment configs to show how to set
springdoc.swagger-ui.enabled=true for non-production environments.
| jwt.secret=AbCdEfGhIjKlMnOpQrStUvWxYz1234567890 | ||
| jwt.expiration=3600000 | ||
|
|
||
|
|
||
|
|
||
| # Primary Database Settings | ||
| spring.datasource.url=jdbc:mysql://localhost:3306/amrit_common?useSSL=false&serverTimezone=UTC | ||
| spring.datasource.username=root | ||
| spring.datasource.password=password | ||
| spring.datasource.driver-class-name=com.mysql.cj.jdbc.Driver |
There was a problem hiding this comment.
Remove committed plaintext secrets/credentials from shared properties.
This block includes sensitive defaults (jwt.secret, DB passwords, mail/sms passwords, encryption-key). These should be externalized to env/secret stores and not committed as active values.
Suggested change
-jwt.secret=AbCdEfGhIjKlMnOpQrStUvWxYz1234567890
-spring.datasource.username=root
-spring.datasource.password=password
+jwt.secret=${JWT_SECRET}
+spring.datasource.username=${SPRING_DATASOURCE_USERNAME}
+spring.datasource.password=${SPRING_DATASOURCE_PASSWORD}
...
-sms-password=password
-mail-password=password
+sms-password=${SMS_PASSWORD:}
+mail-password=${MAIL_PASSWORD:}
...
-secondary.datasource.username=root
-secondary.datasource.password=password
+secondary.datasource.username=${SECONDARY_DATASOURCE_USERNAME}
+secondary.datasource.password=${SECONDARY_DATASOURCE_PASSWORD}
...
-encryption-key=dummy_key_12345
+encryption-key=${ENCRYPTION_KEY}Also applies to: 424-426, 431-432, 437-438, 461-461
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/application.properties` around lines 379 - 388, The
properties file currently contains committed plaintext secrets (jwt.secret,
spring.datasource.username, spring.datasource.password, encryption-key, mail/sms
passwords and similar keys) — replace these concrete values with
environment-backed placeholders (e.g. ${JWT_SECRET:},
${SPRING_DATASOURCE_USERNAME:}, ${SPRING_DATASOURCE_PASSWORD:}) or remove them
entirely, and load actual secrets from environment variables/secret store at
runtime; ensure default empty or non-sensitive fallbacks are not used, update
any references to jwt.secret, spring.datasource.*, encryption-key, and mail/sms
credentials to use the placeholders so secrets are not present in the repo.
|
|
||
|
|
||
| # Primary Database Settings | ||
| spring.datasource.url=jdbc:mysql://localhost:3306/amrit_common?useSSL=false&serverTimezone=UTC |
There was a problem hiding this comment.
Consolidate duplicate property keys to avoid last-value-wins surprises.
spring.datasource.url and send-message-url are each declared multiple times in the same file. The later entry silently overrides earlier values and makes behavior harder to reason about.
Also applies to: 444-445, 441-441, 452-453
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/application.properties` at line 385, There are duplicate
property keys (spring.datasource.url and send-message-url) defined multiple
times causing last-value-wins behavior; open the properties file and consolidate
each duplicated key into a single canonical declaration (or move
environment/profile-specific variants into proper spring profiles or
externalized config), remove or merge the redundant entries referencing
spring.datasource.url and send-message-url, and ensure any intended different
values are placed under distinct profiles (e.g., application-dev.properties) or
renamed so the correct value is explicit and unambiguous.



Code Fixes: Added null-safety guards in PrimaryDBConfig.java and SecondaryDBConfig.java to prevent the application from crashing when database environment variables are missing.
Configuration: Updated application.properties to fix invalid boolean placeholders (like Swagger and API docs) that were causing startup failures.
Local Dev Support: Added a docker-compose.yml for easy local MySQL setup.
Validation: Successfully verified the fix; the application now initializes fully, connects to the database, and starts background services (Quartz and HealthService) without crashing.
Summary by CodeRabbit
New Features
Bug Fixes