From 8174006b01b3e6996312b476b6457d8d0e1989e7 Mon Sep 17 00:00:00 2001 From: matevip Date: Thu, 23 Jul 2026 11:46:15 +0800 Subject: [PATCH] feat(approval): reject dead grant configurations at creation --- .../controller/ApprovalGrantController.java | 30 +++++++++++ .../ApprovalGrantControllerTest.java | 54 +++++++++++++++++-- mateclaw-ui/src/i18n/locales/en-US.ts | 2 +- mateclaw-ui/src/i18n/locales/zh-CN.ts | 2 +- .../Security/AutoApproveGrants/index.vue | 20 +++---- 5 files changed, 90 insertions(+), 18 deletions(-) diff --git a/mateclaw-server/src/main/java/vip/mate/approval/grant/controller/ApprovalGrantController.java b/mateclaw-server/src/main/java/vip/mate/approval/grant/controller/ApprovalGrantController.java index 6074c80b..42ded287 100644 --- a/mateclaw-server/src/main/java/vip/mate/approval/grant/controller/ApprovalGrantController.java +++ b/mateclaw-server/src/main/java/vip/mate/approval/grant/controller/ApprovalGrantController.java @@ -11,6 +11,7 @@ import org.springframework.security.core.Authentication; import org.springframework.web.bind.annotation.*; import vip.mate.approval.grant.entity.ApprovalGrant; import vip.mate.approval.grant.entity.ApprovalResolutionLog; +import vip.mate.agent.repository.AgentMapper; import vip.mate.approval.grant.repository.ApprovalGrantMapper; import vip.mate.approval.grant.repository.ApprovalResolutionLogMapper; import vip.mate.approval.grant.service.ApprovalGrantService; @@ -55,6 +56,7 @@ public class ApprovalGrantController { private static final long DEFAULT_WORKSPACE_ID = 1L; private final ApprovalGrantService grantService; + private final AgentMapper agentMapper; private final ApprovalGrantMapper grantMapper; private final ApprovalResolutionLogMapper resolutionMapper; private final AuthService authService; @@ -295,6 +297,7 @@ public class ApprovalGrantController { } } case ApprovalGrant.ScopeType.AGENT -> { + requireExistingAgent(body.scopeId); if (toolNull) { requireAdminPlusPassword(isAdmin, body.password, actorId); } else if (!isAdmin) { @@ -304,6 +307,15 @@ public class ApprovalGrantController { } } case ApprovalGrant.ScopeType.WORKSPACE -> { + // WORKSPACE-scope matching requires scope_id == the invocation's + // workspaceId AND the grant row's workspace_id (tenant column) to + // equal that same workspace — a scopeId pointing anywhere else can + // never fire. Reject the dead configuration outright. + if (!String.valueOf(workspaceId).equals(body.scopeId)) { + throw new MateClawException("err.approval.workspace_scope_mismatch", 400, + "WORKSPACE-scope scopeId must equal the current workspace id (" + + workspaceId + "); a cross-workspace grant can never match"); + } workspaceService.requirePermission(workspaceId, actorId, "admin"); if (toolNull) { requireAdminPlusPassword(true, body.password, actorId); @@ -314,6 +326,24 @@ public class ApprovalGrantController { } } + /** + * AGENT-scope scopeId must reference an existing agent — a workspace or + * conversation id pasted here compiles into a grant that never matches. + */ + private void requireExistingAgent(String scopeId) { + Long agentId; + try { + agentId = Long.parseLong(scopeId); + } catch (NumberFormatException e) { + throw new MateClawException("err.approval.agent_not_found", 400, + "AGENT-scope scopeId must be a numeric agent id: " + scopeId); + } + if (agentMapper.selectById(agentId) == null) { + throw new MateClawException("err.approval.agent_not_found", 400, + "AGENT-scope scopeId does not reference an existing agent: " + scopeId); + } + } + private void requireAdminPlusPassword(boolean isAdmin, String rawPassword, Long actorId) { if (!isAdmin) { throw new MateClawException("err.approval.admin_required", 403, "admin role required"); diff --git a/mateclaw-server/src/test/java/vip/mate/approval/grant/controller/ApprovalGrantControllerTest.java b/mateclaw-server/src/test/java/vip/mate/approval/grant/controller/ApprovalGrantControllerTest.java index 1085efab..fcf3f03b 100644 --- a/mateclaw-server/src/test/java/vip/mate/approval/grant/controller/ApprovalGrantControllerTest.java +++ b/mateclaw-server/src/test/java/vip/mate/approval/grant/controller/ApprovalGrantControllerTest.java @@ -10,6 +10,8 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.Authentication; +import vip.mate.agent.model.AgentEntity; +import vip.mate.agent.repository.AgentMapper; import vip.mate.approval.grant.entity.ApprovalGrant; import vip.mate.approval.grant.repository.ApprovalGrantMapper; import vip.mate.approval.grant.repository.ApprovalResolutionLogMapper; @@ -51,8 +53,10 @@ class ApprovalGrantControllerTest { private static final long WORKSPACE_ID = 100L; private static final long MEMBER_ID = 1001L; private static final long ADMIN_ID = 2002L; + private static final long AGENT_ID = 3003L; @Mock ApprovalGrantService grantService; + @Mock AgentMapper agentMapper; @Mock ApprovalGrantMapper grantMapper; @Mock ApprovalResolutionLogMapper resolutionMapper; @Mock AuthService authService; @@ -79,6 +83,12 @@ class ApprovalGrantControllerTest { // flag the unused one. Using lenient here keeps setUp shared. lenient().when(authService.findByUsername("member-user")).thenReturn(member); lenient().when(authService.findByUsername("admin-user")).thenReturn(admin); + + // AGENT-scope creation now verifies the target agent exists; most tests + // use AGENT_ID as a valid reference. Lenient: non-AGENT tests never hit it. + AgentEntity agent = new AgentEntity(); + agent.setId(AGENT_ID); + lenient().when(agentMapper.selectById(AGENT_ID)).thenReturn(agent); } @Nested @@ -118,7 +128,7 @@ class ApprovalGrantControllerTest { @Test void agent_scope_explicit_tool_requires_admin() { - ApprovalGrantController.CreateGrantRequest body = baseBody("AGENT", "agent-1", "read_file"); + ApprovalGrantController.CreateGrantRequest body = baseBody("AGENT", String.valueOf(AGENT_ID), "read_file"); doThrow(new MateClawException("err.workspace.insufficient_permission", 403, "admin required")) .when(workspaceService).requirePermission(WORKSPACE_ID, MEMBER_ID, "admin"); @@ -128,7 +138,7 @@ class ApprovalGrantControllerTest { @Test void agent_scope_null_tool_requires_admin_plus_password() { - ApprovalGrantController.CreateGrantRequest body = baseBody("AGENT", "agent-1", null); + ApprovalGrantController.CreateGrantRequest body = baseBody("AGENT", String.valueOf(AGENT_ID), null); // admin true; missing password → 403 when(workspaceService.hasPermission(WORKSPACE_ID, ADMIN_ID, "admin")).thenReturn(true); body.password = null; @@ -140,7 +150,7 @@ class ApprovalGrantControllerTest { @Test void agent_scope_null_tool_admin_with_password_passes() { - ApprovalGrantController.CreateGrantRequest body = baseBody("AGENT", "agent-1", null); + ApprovalGrantController.CreateGrantRequest body = baseBody("AGENT", String.valueOf(AGENT_ID), null); body.password = "correct-password"; when(workspaceService.hasPermission(WORKSPACE_ID, ADMIN_ID, "admin")).thenReturn(true); // verifyCurrentUserPassword passes silently when correct. @@ -186,9 +196,45 @@ class ApprovalGrantControllerTest { .hasMessageContaining("CRITICAL is not auto-approvable"); } + @Test + void workspace_scope_id_must_match_current_workspace() { + // A WORKSPACE-scope grant whose scopeId is a different workspace can + // never match at runtime (tenant column + scope match both fail) — + // rejected at creation as a dead configuration. + ApprovalGrantController.CreateGrantRequest body = baseBody("WORKSPACE", + "2079860736482390017", "read_file"); + + assertThatThrownBy(() -> controller.create(body, WORKSPACE_ID, adminAuth)) + .isInstanceOf(MateClawException.class) + .hasMessageContaining("must equal the current workspace id"); + verify(grantMapper, never()).insert(any(ApprovalGrant.class)); + } + + @Test + void agent_scope_nonexistent_agent_is_rejected() { + ApprovalGrantController.CreateGrantRequest body = baseBody("AGENT", "424242", "read_file"); + when(agentMapper.selectById(424242L)).thenReturn(null); + + assertThatThrownBy(() -> controller.create(body, WORKSPACE_ID, adminAuth)) + .isInstanceOf(MateClawException.class) + .hasMessageContaining("does not reference an existing agent"); + verify(grantMapper, never()).insert(any(ApprovalGrant.class)); + } + + @Test + void agent_scope_non_numeric_id_is_rejected() { + // e.g. a conversation id or workspace name pasted into the AGENT scope. + ApprovalGrantController.CreateGrantRequest body = baseBody("AGENT", "wecom:xxx", "read_file"); + + assertThatThrownBy(() -> controller.create(body, WORKSPACE_ID, adminAuth)) + .isInstanceOf(MateClawException.class) + .hasMessageContaining("must be a numeric agent id"); + verify(grantMapper, never()).insert(any(ApprovalGrant.class)); + } + @Test void until_conversation_end_requires_conversation_scope() { - ApprovalGrantController.CreateGrantRequest body = baseBody("AGENT", "agent-1", "read_file"); + ApprovalGrantController.CreateGrantRequest body = baseBody("AGENT", String.valueOf(AGENT_ID), "read_file"); body.grantKind = "UNTIL_CONVERSATION_END"; assertThatThrownBy(() -> controller.create(body, WORKSPACE_ID, memberAuth)) diff --git a/mateclaw-ui/src/i18n/locales/en-US.ts b/mateclaw-ui/src/i18n/locales/en-US.ts index a10dd973..81241841 100644 --- a/mateclaw-ui/src/i18n/locales/en-US.ts +++ b/mateclaw-ui/src/i18n/locales/en-US.ts @@ -4424,7 +4424,7 @@ export default { severityMedium: 'auto-approve LOW/MEDIUM findings', severityHigh: 'auto-approve LOW/MEDIUM/HIGH findings', scopeIdPickAgent: 'Pick an agent…', - scopeIdWorkspaceHint: 'Pick the workspace this grant applies to (must be the workspace the conversation actually belongs to).', + scopeIdWorkspaceHint: 'Fixed to the current workspace. To create a grant for another workspace, switch workspaces in the console first.', scopeIdAgentHint: 'Applies to every conversation of the selected agent within this workspace.', scopeIdUserHint: 'USER-scope grants can only target the currently logged-in user.', scopeIdConversationHint: 'Conversation id, as shown in the audit log "session" column (e.g. wecom:xxx).', diff --git a/mateclaw-ui/src/i18n/locales/zh-CN.ts b/mateclaw-ui/src/i18n/locales/zh-CN.ts index 0a01d598..91a6aa31 100644 --- a/mateclaw-ui/src/i18n/locales/zh-CN.ts +++ b/mateclaw-ui/src/i18n/locales/zh-CN.ts @@ -4516,7 +4516,7 @@ export default { severityMedium: '放行低/中危命中', severityHigh: '放行低/中/高危命中', scopeIdPickAgent: '选择智能体…', - scopeIdWorkspaceHint: '选择策略生效的工作区（必须与会话实际所属工作区一致）。', + scopeIdWorkspaceHint: '固定为当前工作区。如需为其他工作区创建策略，请先在控制台切换工作区。', scopeIdAgentHint: '选择智能体后，该智能体在本工作区的所有会话均适用。', scopeIdUserHint: '只能为当前登录用户创建 USER 范围策略。', scopeIdConversationHint: '填写会话 ID，可在审计日志的"会话"列查看（如 wecom:xxx）。', diff --git a/mateclaw-ui/src/views/Security/AutoApproveGrants/index.vue b/mateclaw-ui/src/views/Security/AutoApproveGrants/index.vue index c895123d..ca48f12f 100644 --- a/mateclaw-ui/src/views/Security/AutoApproveGrants/index.vue +++ b/mateclaw-ui/src/views/Security/AutoApproveGrants/index.vue @@ -207,20 +207,16 @@ - - - + disabled + />