Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/commit-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ jobs:
submodules: true
fetch-depth: 0

- name: Fetch base branch
run: git fetch origin ${{ github.event.pull_request.base.ref }}:refs/remotes/origin/${{ github.event.pull_request.base.ref }}

- name: Setup Node.js
uses: actions/setup-node@v4
with:
Expand Down
4 changes: 4 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,10 @@
<artifactId>spring-session-data-redis</artifactId>

</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-data-mongodb</artifactId>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
Expand Down
307 changes: 281 additions & 26 deletions src/main/java/com/iemr/admin/service/health/HealthService.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,19 @@
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.util.HashMap;
import java.time.Instant;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Properties;

import javax.sql.DataSource;

import org.bson.Document;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.data.mongodb.core.MongoTemplate;
import org.springframework.data.redis.core.RedisCallback;
import org.springframework.data.redis.core.RedisTemplate;
import org.springframework.stereotype.Service;
Expand All @@ -41,79 +46,329 @@ public class HealthService {

private static final Logger logger = LoggerFactory.getLogger(HealthService.class);
private static final String DB_HEALTH_CHECK_QUERY = "SELECT 1 as health_check";
private static final String DB_VERSION_QUERY = "SELECT VERSION()";

@Autowired
private DataSource dataSource;

@Autowired(required = false)
private RedisTemplate<String, Object> redisTemplate;

@Autowired(required = false)
private MongoTemplate mongoTemplate;

@Value("${spring.datasource.url:unknown}")
private String dbUrl;

@Value("${spring.redis.host:localhost}")
private String redisHost;

@Value("${spring.redis.port:6379}")
private int redisPort;

@Value("${spring.data.mongodb.host:localhost}")
private String mongoHost;

@Value("${spring.data.mongodb.port:27017}")
private int mongoPort;

@Value("${spring.data.mongodb.database:amrit}")
private String mongoDatabase;

public Map<String, Object> checkHealth() {
Map<String, Object> healthStatus = new HashMap<>();
Map<String, Object> healthStatus = new LinkedHashMap<>();
Map<String, Object> components = new LinkedHashMap<>();
boolean overallHealth = true;

// Check database connectivity (details logged internally, not exposed)
boolean dbHealthy = checkDatabaseHealthInternal();
if (!dbHealthy) {
// Check MySQL connectivity
Map<String, Object> mysqlStatus = checkMySQLHealth();
components.put("mysql", mysqlStatus);
if (!"UP".equals(mysqlStatus.get("status"))) {
overallHealth = false;
}

// Check Redis connectivity if configured (details logged internally)
// Check Redis connectivity if configured
if (redisTemplate != null) {
boolean redisHealthy = checkRedisHealthInternal();
if (!redisHealthy) {
Map<String, Object> redisStatus = checkRedisHealth();
components.put("redis", redisStatus);
if (!"UP".equals(redisStatus.get("status"))) {
overallHealth = false;
}
}

// Check MongoDB connectivity if configured
if (mongoTemplate != null) {
Map<String, Object> mongoStatus = checkMongoDBHealth();
components.put("mongodb", mongoStatus);
if (!"UP".equals(mongoStatus.get("status"))) {
overallHealth = false;
}
}

healthStatus.put("status", overallHealth ? "UP" : "DOWN");
healthStatus.put("timestamp", Instant.now().toString());
healthStatus.put("components", components);

logger.info("Health check completed - Overall status: {}", overallHealth ? "UP" : "DOWN");
return healthStatus;
}

private boolean checkDatabaseHealthInternal() {
private Map<String, Object> checkMySQLHealth() {
Map<String, Object> status = new LinkedHashMap<>();
Map<String, Object> details = new LinkedHashMap<>();
long startTime = System.currentTimeMillis();


// Add connection details
details.put("type", "MySQL");
details.put("host", extractHost(dbUrl));
details.put("port", extractPort(dbUrl));
details.put("database", extractDatabaseName(dbUrl));

Comment on lines +121 to +126
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Check for health endpoint configuration and security setup
rg -n -C3 "health|HealthController|HealthService" --type=java | head -100

Repository: PSMRI/Admin-API

Length of output: 10338


🏁 Script executed:

# Look for Spring Boot Actuator configuration
find . -type f \( -name "application.properties" -o -name "application.yml" -o -name "application.yaml" \) -exec grep -l "health\|actuator" {} \;

Repository: PSMRI/Admin-API

Length of output: 41


🏁 Script executed:

# Check for Spring Security configuration
rg -n "EnableWebSecurity|SecurityFilterChain|authorizeRequests" --type=java | head -50

Repository: PSMRI/Admin-API

Length of output: 41


🏁 Script executed:

# Look for any request mapping or endpoint configuration
rg -n -C2 "@RequestMapping|@GetMapping|@PostMapping" --type=java -g "*/controller/*" | head -100

Repository: PSMRI/Admin-API

Length of output: 207


🏁 Script executed:

# Check the actual HealthService implementation
fd -t f "HealthService.java" --exec cat -n {} \;

Repository: PSMRI/Admin-API

Length of output: 16914


🏁 Script executed:

# Find the controller that exposes the health endpoint
rg -n "@RequestMapping|@GetMapping|@RestController|@Controller" -A5 --type=java | grep -A5 "health"

Repository: PSMRI/Admin-API

Length of output: 1796


🏁 Script executed:

# Look for application configuration files
find . -type f \( -name "*.properties" -o -name "*.yml" -o -name "*.yaml" \) | head -20

Repository: PSMRI/Admin-API

Length of output: 453


Restrict database connection details exposure in public health endpoint.

The /health endpoint is intentionally public (JWT validation explicitly skipped in JwtUserIdValidationFilter), yet exposes database host, port, and database names for all components (MySQL, Redis, MongoDB). Additionally, exception messages and class names are exposed, which could leak version information. This aids reconnaissance and should be restricted.

Consider either:

  1. Restricting detail exposure to authenticated/internal requests
  2. Making detail exposure configurable via a property

This applies across all three database components (MySQL at lines 122-125, Redis at lines 177-178, MongoDB at lines 224-226, plus exception details at lines 163-164, 210-211, and 256-257).

🤖 Prompt for AI Agents
In @src/main/java/com/iemr/admin/service/health/HealthService.java around lines
121 - 126, The health endpoint currently always adds sensitive DB details via
details.put("type", "MySQL"), details.put("host", extractHost(dbUrl)),
details.put("port", extractPort(dbUrl)), details.put("database",
extractDatabaseName(dbUrl)) (and similar Redis/MongoDB blocks) and returns
exception messages/classes; make detail exposure conditional on a new config
flag (e.g., boolean exposeHealthDetails) read from properties, and wrap each
sensitive addition behind if (exposeHealthDetails) checks so host/port/database
names are only added when true; for exceptions, stop returning
exception.getMessage()/getClass() to the HTTP response—keep internal logging
(use logger.error with the caught exception) and return a generic error string
in the health response; apply these changes to the MySQL block (details.put
calls), Redis and MongoDB detail blocks, and the three exception handlers that
currently expose exception details.

try (Connection connection = dataSource.getConnection()) {
boolean isConnectionValid = connection.isValid(2); // 2 second timeout per best practices

// 2 second timeout per best practices
boolean isConnectionValid = connection.isValid(2);

if (isConnectionValid) {
// 3 second query timeout
try (PreparedStatement stmt = connection.prepareStatement(DB_HEALTH_CHECK_QUERY)) {
stmt.setQueryTimeout(3); // 3 second query timeout
stmt.setQueryTimeout(3);
try (ResultSet rs = stmt.executeQuery()) {
if (rs.next() && rs.getInt(1) == 1) {
long responseTime = System.currentTimeMillis() - startTime;
logger.debug("Database health check: UP ({}ms)", responseTime);
return true;
logger.debug("MySQL health check: UP ({}ms)", responseTime);

status.put("status", "UP");
details.put("responseTimeMs", responseTime);

// Get database version
String version = getMySQLVersion(connection);
if (version != null) {
details.put("version", version);
}

status.put("details", details);
return status;
}
}
}
}
logger.warn("Database health check: Connection not valid");
return false;
logger.warn("MySQL health check: Connection not valid");
status.put("status", "DOWN");
details.put("error", "Connection validation failed");
status.put("details", details);
return status;
} catch (Exception e) {
logger.error("Database health check failed: {}", e.getMessage());
return false;
logger.error("MySQL health check failed: {}", e.getMessage());
status.put("status", "DOWN");
details.put("error", e.getMessage());
details.put("errorType", e.getClass().getSimpleName());
status.put("details", details);
return status;
}
}

private boolean checkRedisHealthInternal() {
private Map<String, Object> checkRedisHealth() {
Map<String, Object> status = new LinkedHashMap<>();
Map<String, Object> details = new LinkedHashMap<>();
long startTime = System.currentTimeMillis();


// Add connection details
details.put("type", "Redis");
details.put("host", redisHost);
details.put("port", redisPort);

try {
String pong = redisTemplate.execute((RedisCallback<String>) connection -> connection.ping());

// Use ping() from RedisConnection directly
String pong = redisTemplate.execute((RedisCallback<String>) connection ->
connection.ping()
);
Comment on lines 180 to +184
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing timeout for Redis health check.

The Redis ping() operation lacks an explicit timeout. Consider configuring a connection timeout in Redis properties or wrapping the call with a timeout mechanism to prevent blocking.

🤖 Prompt for AI Agents
In @src/main/java/com/iemr/admin/service/health/HealthService.java around lines
180 - 184, The Redis health check in HealthService currently calls
redisTemplate.execute with connection.ping() without any timeout; update the
health check (the method using redisTemplate.execute/RedisCallback) to enforce a
timeout by either configuring a connection timeout on the RedisConnectionFactory
used by redisTemplate (set the client/connection timeout property) or wrap the
execute call in a cancellable timed task (e.g., submit a
Callable/CompletableFuture that calls redisTemplate.execute(...) and use
get(timeout, unit)) and handle TimeoutException by treating Redis as unhealthy
and logging the timeout; ensure you reference redisTemplate and the
HealthService health-check method when making the change and propagate/return a
failure status on timeout.


if ("PONG".equals(pong)) {
long responseTime = System.currentTimeMillis() - startTime;
logger.debug("Redis health check: UP ({}ms)", responseTime);
return true;

status.put("status", "UP");
details.put("responseTimeMs", responseTime);

// Get Redis version
String version = getRedisVersion();
if (version != null) {
details.put("version", version);
}

status.put("details", details);
return status;
}
logger.warn("Redis health check: Ping returned unexpected response");
return false;
status.put("status", "DOWN");
details.put("error", "Ping returned unexpected response");
status.put("details", details);
return status;
} catch (Exception e) {
logger.error("Redis health check failed: {}", e.getMessage());
return false;
status.put("status", "DOWN");
details.put("error", e.getMessage());
details.put("errorType", e.getClass().getSimpleName());
status.put("details", details);
return status;
}
}

private Map<String, Object> checkMongoDBHealth() {
Map<String, Object> status = new LinkedHashMap<>();
Map<String, Object> details = new LinkedHashMap<>();
long startTime = System.currentTimeMillis();

// Add connection details
details.put("type", "MongoDB");
details.put("host", mongoHost);
details.put("port", mongoPort);
details.put("database", mongoDatabase);

try {
// Run ping command to check MongoDB connectivity
Document pingResult = mongoTemplate.getDb().runCommand(new Document("ping", 1));
Comment on lines +228 to +230
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Missing timeout for MongoDB health check.

Unlike the MySQL check (which has 2s connection validation and 3s query timeout), the MongoDB ping command has no timeout. If MongoDB is unresponsive, this could cause the health endpoint to hang indefinitely.

Consider configuring a socket timeout or using MongoDB's maxTimeMS option:

♻️ Proposed fix with timeout
         try {
             // Run ping command to check MongoDB connectivity
-            Document pingResult = mongoTemplate.getDb().runCommand(new Document("ping", 1));
+            Document pingCommand = new Document("ping", 1)
+                .append("maxTimeMS", 3000); // 3 second timeout
+            Document pingResult = mongoTemplate.getDb().runCommand(pingCommand);
📝 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.

Suggested change
try {
// Run ping command to check MongoDB connectivity
Document pingResult = mongoTemplate.getDb().runCommand(new Document("ping", 1));
try {
// Run ping command to check MongoDB connectivity
Document pingCommand = new Document("ping", 1)
.append("maxTimeMS", 3000); // 3 second timeout
Document pingResult = mongoTemplate.getDb().runCommand(pingCommand);
🤖 Prompt for AI Agents
In @src/main/java/com/iemr/admin/service/health/HealthService.java around lines
228 - 230, The MongoDB ping in HealthService
(mongoTemplate.getDb().runCommand(...)) lacks a timeout and can hang; add a
timeout by either supplying a maxTimeMS to the ping command (e.g., include a
maxTimeMS value in the Document/BSON passed to runCommand) or configure the
MongoClient's socket/timeout settings (MongoClientSettings socketTimeout or
serverSelectionTimeout) used by mongoTemplate; update the runCommand invocation
or the MongoClient configuration so the health check uses a bounded timeout
(e.g., 2–3s) to mirror the MySQL check.


if (pingResult != null && pingResult.getDouble("ok") == 1.0) {
Comment on lines +230 to +232
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Potential NullPointerException on MongoDB ping result check.

pingResult.getDouble("ok") returns null if the key doesn't exist, and comparing with == 1.0 will cause auto-unboxing NPE.

🐛 Proposed fix
-            if (pingResult != null && pingResult.getDouble("ok") == 1.0) {
+            if (pingResult != null && Double.valueOf(1.0).equals(pingResult.getDouble("ok"))) {
📝 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.

Suggested change
Document pingResult = mongoTemplate.getDb().runCommand(new Document("ping", 1));
if (pingResult != null && pingResult.getDouble("ok") == 1.0) {
Document pingResult = mongoTemplate.getDb().runCommand(new Document("ping", 1));
if (pingResult != null && Double.valueOf(1.0).equals(pingResult.getDouble("ok"))) {
🤖 Prompt for AI Agents
In @src/main/java/com/iemr/admin/service/health/HealthService.java around lines
230 - 232, The pingResult null-check is incomplete and calling
pingResult.getDouble("ok") can return null and cause an NPE; change the check to
safely retrieve and verify the "ok" field (for example: use pingResult.get("ok",
Number.class) or pingResult.get("ok", Double.class'), ensure the returned value
is non-null, then call .doubleValue() and compare to 1.0), e.g., obtain Number
ok = pingResult.get("ok", Number.class); and use if (pingResult != null && ok !=
null && ok.doubleValue() == 1.0) to avoid auto-unboxing NPE in the HealthService
ping logic that calls mongoTemplate.getDb().runCommand.

long responseTime = System.currentTimeMillis() - startTime;
logger.debug("MongoDB health check: UP ({}ms)", responseTime);

status.put("status", "UP");
details.put("responseTimeMs", responseTime);

// Get MongoDB version
String version = getMongoDBVersion();
if (version != null) {
details.put("version", version);
}

status.put("details", details);
return status;
}
logger.warn("MongoDB health check: Ping returned unexpected response");
status.put("status", "DOWN");
details.put("error", "Ping returned unexpected response");
status.put("details", details);
return status;
} catch (Exception e) {
logger.error("MongoDB health check failed: {}", e.getMessage());
status.put("status", "DOWN");
details.put("error", e.getMessage());
details.put("errorType", e.getClass().getSimpleName());
status.put("details", details);
return status;
}
}

private String getMySQLVersion(Connection connection) {
try (PreparedStatement stmt = connection.prepareStatement(DB_VERSION_QUERY);
ResultSet rs = stmt.executeQuery()) {
if (rs.next()) {
return rs.getString(1);
}
} catch (Exception e) {
logger.debug("Could not retrieve MySQL version: {}", e.getMessage());
}
return null;
}

private String getRedisVersion() {
try {
Properties info = redisTemplate.execute((RedisCallback<Properties>) connection ->
connection.serverCommands().info("server")
);
if (info != null && info.containsKey("redis_version")) {
return info.getProperty("redis_version");
}
} catch (Exception e) {
logger.debug("Could not retrieve Redis version: {}", e.getMessage());
}
return null;
}

private String getMongoDBVersion() {
try {
Document buildInfo = mongoTemplate.getDb().runCommand(new Document("buildInfo", 1));
if (buildInfo != null && buildInfo.containsKey("version")) {
return buildInfo.getString("version");
}
} catch (Exception e) {
logger.debug("Could not retrieve MongoDB version: {}", e.getMessage());
}
return null;
}

/**
* Extracts host from JDBC URL.
* Example: jdbc:mysql://mysql-container:3306/db_iemr -> mysql-container
*/
private String extractHost(String jdbcUrl) {
if (jdbcUrl == null || "unknown".equals(jdbcUrl)) {
return "unknown";
}
try {
// Remove jdbc:mysql:// prefix
String withoutPrefix = jdbcUrl.replaceFirst("jdbc:mysql://", "");
// Get host:port part (before the first /)
int slashIndex = withoutPrefix.indexOf('/');
String hostPort = slashIndex > 0
? withoutPrefix.substring(0, slashIndex)
: withoutPrefix;
// Get host (before the colon)
int colonIndex = hostPort.indexOf(':');
return colonIndex > 0 ? hostPort.substring(0, colonIndex) : hostPort;
} catch (Exception e) {
logger.debug("Could not extract host from URL: {}", e.getMessage());
}
return "unknown";
}

/**
* Extracts port from JDBC URL.
* Example: jdbc:mysql://mysql-container:3306/db_iemr -> 3306
*/
private String extractPort(String jdbcUrl) {
if (jdbcUrl == null || "unknown".equals(jdbcUrl)) {
return "unknown";
}
try {
// Remove jdbc:mysql:// prefix
String withoutPrefix = jdbcUrl.replaceFirst("jdbc:mysql://", "");
// Get host:port part (before the first /)
int slashIndex = withoutPrefix.indexOf('/');
String hostPort = slashIndex > 0
? withoutPrefix.substring(0, slashIndex)
: withoutPrefix;
// Get port (after the colon)
int colonIndex = hostPort.indexOf(':');
return colonIndex > 0 ? hostPort.substring(colonIndex + 1) : "3306";
} catch (Exception e) {
logger.debug("Could not extract port from URL: {}", e.getMessage());
}
return "3306";
}

/**
* Extracts database name from JDBC URL.
* Example: jdbc:mysql://mysql-container:3306/db_iemr?params -> db_iemr
*/
private String extractDatabaseName(String jdbcUrl) {
if (jdbcUrl == null || "unknown".equals(jdbcUrl)) {
return "unknown";
}
try {
int lastSlash = jdbcUrl.lastIndexOf('/');
if (lastSlash >= 0 && lastSlash < jdbcUrl.length() - 1) {
String afterSlash = jdbcUrl.substring(lastSlash + 1);
int queryStart = afterSlash.indexOf('?');
if (queryStart > 0) {
return afterSlash.substring(0, queryStart);
}
return afterSlash;
}
} catch (Exception e) {
logger.debug("Could not extract database name: {}", e.getMessage());
}
return "unknown";
}
}
Loading
Loading