feat(approval): reject dead grant configurations at creation

This commit is contained in:
matevip 2026-07-23 11:46:15 +08:00
parent be7b236cb5
commit 8174006b01
5 changed files with 90 additions and 18 deletions

View File

@ -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");

View File

@ -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))

View File

@ -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).',

View File

@ -4516,7 +4516,7 @@ export default {
severityMedium: '放行低/中危命中',
severityHigh: '放行低/中/高危命中',
scopeIdPickAgent: '选择智能体…',
scopeIdWorkspaceHint: '选择策略生效的工作区(必须与会话实际所属工作区一致)。',
scopeIdWorkspaceHint: '固定为当前工作区。如需为其他工作区创建策略,请先在控制台切换工作区。',
scopeIdAgentHint: '选择智能体后,该智能体在本工作区的所有会话均适用。',
scopeIdUserHint: '只能为当前登录用户创建 USER 范围策略。',
scopeIdConversationHint: '填写会话 ID可在审计日志的"会话"列查看(如 wecom:xxx。',

View File

@ -207,20 +207,16 @@
<label>{{ t('approval.grant.form.scopeId') }} <span class="required">*</span></label>
<!-- Scope-typed pickers: free-text snowflake input made it far too
easy to paste the wrong kind of id (e.g. a workspace id into an
AGENT-scope grant), producing a grant that never matches. -->
<select
AGENT-scope grant), producing a grant that never matches.
WORKSPACE is locked to the current workspace the backend
rejects any other id as a dead configuration. -->
<input
v-if="form.scopeType === 'WORKSPACE'"
v-model="form.scopeId"
:value="currentWorkspaceLabel"
type="text"
class="form-input"
>
<option
v-for="ws in workspaceStore.workspaces"
:key="String(ws.id)"
:value="String(ws.id)"
>
{{ ws.name }} (#{{ ws.id }})
</option>
</select>
disabled
/>
<select
v-else-if="form.scopeType === 'AGENT'"
v-model="form.scopeId"