feat: implement auto-approval for MCP audit records based on approved…#4845
feat: implement auto-approval for MCP audit records based on approved…#4845harshithb3304 wants to merge 2 commits intomasterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds “auto-approval” behavior for MCP audit records by checking whether an MCP server (mcpHost) exists in an account-level approved servers list stored under the MCP registry config.
Changes:
- Adds
approvedServerssupport toConfig.McpRegistryConfig. - Auto-approves MCP audit records on insert/update when the MCP host is in the approved list.
- Adds a runtime job step to auto-approve existing “pending” MCP audit records based on the approved list.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| libs/utils/src/main/java/com/akto/mcp/McpRequestResponseUtils.java | Adds approved-server lookup + auto-approval updates during audit record upsert. |
| libs/dao/src/main/java/com/akto/dto/Config.java | Extends MCP registry config DTO with approvedServers model. |
| apps/api-runtime/src/main/java/com/akto/runtime/McpToolsSyncJobExecutor.java | Adds a pre-sync step to auto-approve existing pending audit records. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Fetch MCP Registry Config from database | ||
| com.akto.dao.ConfigsDao configsDao = com.akto.dao.ConfigsDao.instance; | ||
| org.bson.conversions.Bson filters = com.mongodb.client.model.Filters.eq( | ||
| "_id", Context.accountId.get() + "_MCP_REGISTRY" | ||
| ); | ||
|
|
||
| com.akto.dto.Config config = configsDao.findOne(filters); | ||
|
|
There was a problem hiding this comment.
checkIfServerIsApproved hits the database (ConfigsDao.findOne) on every call. This method is invoked from setAuditData (per MCP event) and also inside a loop in McpToolsSyncJobExecutor, which can create significant DB load (N+1 lookups and added latency). Consider loading the MCP registry config once per job/run (or caching the approved server set per account with a TTL) and then doing in-memory membership checks.
| // If server is approved and not already marked, auto-approve it | ||
| if ("Approved".equals(approvalStatus) && | ||
| (existingRecord.getRemarks() == null || existingRecord.getRemarks().isEmpty())) { | ||
| setFields.put("remarks", "Approved"); | ||
| setFields.put("markedBy", "System (Auto-approved)"); | ||
| setFields.put("approvedAt", Context.now()); | ||
| setFields.put("updatedTimestamp", Context.now()); | ||
| logger.info("Auto-approved MCP server: " + auditInfo.getMcpHost() + " (resource: " + auditInfo.getResourceName() + ")"); |
There was a problem hiding this comment.
The auto-approval guard says "not already marked", but the condition only checks existingRecord.getRemarks() for null/empty. If markedBy was set (or approvedAt populated) while remarks is empty/null (e.g., partial/legacy updates), this would overwrite a human decision. To match the intent, also gate auto-approval on markedBy/approvedAt being unset (or explicitly on a known "pending" state).
| public static String checkIfServerIsApproved(String serverName) { | ||
| try { | ||
| if (serverName == null || serverName.isEmpty()) { | ||
| return null; | ||
| } | ||
|
|
||
| // Fetch MCP Registry Config from database | ||
| com.akto.dao.ConfigsDao configsDao = com.akto.dao.ConfigsDao.instance; | ||
| org.bson.conversions.Bson filters = com.mongodb.client.model.Filters.eq( | ||
| "_id", Context.accountId.get() + "_MCP_REGISTRY" | ||
| ); | ||
|
|
||
| com.akto.dto.Config config = configsDao.findOne(filters); | ||
|
|
||
| if (config instanceof com.akto.dto.Config.McpRegistryConfig) { | ||
| com.akto.dto.Config.McpRegistryConfig mcpConfig = | ||
| (com.akto.dto.Config.McpRegistryConfig) config; | ||
|
|
||
| if (mcpConfig.getApprovedServers() != null) { | ||
| for (com.akto.dto.Config.McpRegistryConfig.ApprovedMcpServer approvedServer : | ||
| mcpConfig.getApprovedServers()) { | ||
| if (serverName.equalsIgnoreCase(approvedServer.getName())) { | ||
| logger.info("MCP server found in approved list: " + serverName); | ||
| return "Approved"; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return null; |
There was a problem hiding this comment.
checkIfServerIsApproved returns the string literal "Approved" or null. This makes callers rely on repeated string comparisons and is easy to break with typos/casing changes. Consider changing the API to return a boolean (or an enum) and centralizing the status value in a single constant if you need to persist/display it.
| // Get all pending MCP audit records | ||
| Bson filter = Filters.or( | ||
| Filters.exists("remarks", false), | ||
| Filters.eq("remarks", ""), | ||
| Filters.eq("remarks", null) | ||
| ); | ||
|
|
||
| List<McpAuditInfo> pendingRecords = McpAuditInfoDao.instance.findAll(filter); | ||
|
|
||
| if (pendingRecords.isEmpty()) { | ||
| logger.debug("No pending MCP audit records found"); | ||
| return; | ||
| } | ||
|
|
||
| logger.info("Found {} pending MCP audit records to check for auto-approval", pendingRecords.size()); | ||
|
|
||
| int currentTime = Context.now(); | ||
| int approvedCount = 0; | ||
|
|
||
| // Check each pending record against approved servers list using shared helper | ||
| for (McpAuditInfo record : pendingRecords) { | ||
| String mcpHost = record.getMcpHost(); | ||
|
|
||
| if (mcpHost == null || mcpHost.isEmpty()) { | ||
| continue; | ||
| } | ||
|
|
||
| // Reuse existing checkIfServerIsApproved from McpRequestResponseUtils | ||
| String approvalStatus = McpRequestResponseUtils.checkIfServerIsApproved(mcpHost); | ||
|
|
There was a problem hiding this comment.
autoApproveExistingPendingRecords loads all pending records (potentially large) and then calls McpRequestResponseUtils.checkIfServerIsApproved per record. Since checkIfServerIsApproved does a DB read each time, this becomes an N+1 query pattern and can be very expensive on large datasets. Consider fetching the approved server list once (per account/run) and either (a) filtering records via a Mongo $in on mcpHost to only fetch candidates, or (b) doing a single pass with an in-memory Set without additional DB reads.
| Filters.exists("remarks", false), | ||
| Filters.eq("remarks", ""), | ||
| Filters.eq("remarks", null) |
There was a problem hiding this comment.
The "pending" filter is based on remarks (exists/eq ""/eq null), but other parts of the codebase treat untriaged audit items as markedBy empty/null (and markedBy is indexed in McpAuditInfoDao). Using remarks here may force a full collection scan and may not align with how "open" items are defined elsewhere. Consider switching the filter to markedBy (or combining both fields) so the query can use the existing index and the semantics stay consistent.
| Filters.exists("remarks", false), | |
| Filters.eq("remarks", ""), | |
| Filters.eq("remarks", null) | |
| Filters.exists("markedBy", false), | |
| Filters.eq("markedBy", ""), | |
| Filters.eq("markedBy", null) |
… servers list