Skip to content
Merged
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
2 changes: 1 addition & 1 deletion src/server/lib/validation/agentSessionConfigSchemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const toolRuleSchema = {
type: 'object',
properties: {
toolKey: { type: 'string', minLength: 1, maxLength: 255 },
mode: { type: 'string', enum: ['allow', 'deny'] },
mode: { type: 'string', enum: ['allow', 'require_approval', 'deny'] },
},
required: ['toolKey', 'mode'],
additionalProperties: false,
Expand Down
2 changes: 1 addition & 1 deletion src/server/lib/validation/agentSessionConfigValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export function validateAgentSessionControlPlaneConfig(config: Partial<AgentSess
if (rule.toolKey.length > 255) {
throw new AgentSessionConfigValidationError(`toolRules entry "${rule.toolKey}" exceeds maximum toolKey length.`);
}
if (rule.mode !== 'allow' && rule.mode !== 'deny') {
if (rule.mode !== 'allow' && rule.mode !== 'require_approval' && rule.mode !== 'deny') {
throw new AgentSessionConfigValidationError(
`toolRules entry "${rule.toolKey}" has unsupported mode "${rule.mode}".`
);
Expand Down
75 changes: 75 additions & 0 deletions src/server/services/__tests__/agentSessionConfig.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,81 @@ describe('AgentSessionConfigService', () => {
});
});

it('persists require-approval tool overrides in control-plane config', async () => {
const service = makeService();

await expect(
service.setGlobalConfig({
toolRules: [
{
toolKey: 'mcp__sandbox__workspace_read_file',
mode: 'require_approval',
},
],
})
).resolves.toEqual({
toolRules: [
{
toolKey: 'mcp__sandbox__workspace_read_file',
mode: 'require_approval',
},
],
});

expect(mockGlobalConfigSetConfig).toHaveBeenCalledWith('agentSessionDefaults', {
controlPlane: {
toolRules: [
{
toolKey: 'mcp__sandbox__workspace_read_file',
mode: 'require_approval',
},
],
},
});
});

it('treats explicit tool rules as effective overrides in the inventory', async () => {
const service = makeService();

jest.spyOn(service, 'getGlobalConfig').mockResolvedValue({
toolRules: [
{
toolKey: 'mcp__sandbox__workspace_read_file',
mode: 'allow',
},
],
});
jest.spyOn(service, 'getEffectiveConfig').mockResolvedValue({
systemPrompt: 'base',
appendSystemPrompt: 'append',
toolRules: [
{
toolKey: 'mcp__sandbox__workspace_read_file',
mode: 'allow',
},
],
});
jest.spyOn(AgentPolicyService, 'getEffectivePolicy').mockResolvedValue({
...DEFAULT_AGENT_APPROVAL_POLICY,
rules: {
...DEFAULT_AGENT_APPROVAL_POLICY.rules,
read: 'deny',
},
});

const entries = await service.listToolInventory('global');
const readFileEntry = entries.find((entry) => entry.toolName === 'workspace.read_file');

expect(readFileEntry).toEqual(
expect.objectContaining({
approvalMode: 'deny',
scopeRuleMode: 'allow',
effectiveRuleMode: 'allow',
availability: 'available',
})
);
});

it('updates runtime settings without overwriting control-plane settings', async () => {
const service = makeService();

Expand Down
28 changes: 22 additions & 6 deletions src/server/services/agent/CapabilityService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,17 @@ function resolvePrimaryRepo(session: AgentSession): string | undefined {
return session.selectedServices?.[0]?.repo || undefined;
}

function isToolAllowed(toolRules: AgentSessionToolRule[] | undefined, toolKey: string): boolean {
function resolveToolApprovalMode({
toolRules,
toolKey,
capabilityMode,
}: {
toolRules: AgentSessionToolRule[] | undefined;
toolKey: string;
capabilityMode: AgentApprovalMode;
}): AgentApprovalMode {
const rule = toolRules?.find((item) => item.toolKey === toolKey);
return rule?.mode !== 'deny';
return rule?.mode || capabilityMode;
}

function resolveSessionWorkspaceGatewayBaseUrl(session: AgentSession): string | null {
Expand Down Expand Up @@ -344,9 +352,13 @@ export default class AgentCapabilityService {
entry.toolName,
entry.annotations || discoveredTool.annotations
);
const mode = AgentPolicyService.modeForCapability(approvalPolicy, capabilityKey);
const mode = resolveToolApprovalMode({
toolRules,
toolKey: entry.toolKey,
capabilityMode: AgentPolicyService.modeForCapability(approvalPolicy, capabilityKey),
});

if (mode === 'deny' || !isToolAllowed(toolRules, entry.toolKey)) {
if (mode === 'deny') {
continue;
}

Expand Down Expand Up @@ -485,10 +497,14 @@ export default class AgentCapabilityService {
}

const capabilityKey = AgentPolicyService.capabilityForMcpTool(discoveredTool.name, discoveredTool.annotations);
const mode = AgentPolicyService.modeForCapability(approvalPolicy, capabilityKey);
const toolName = buildAgentToolKey(server.slug, discoveredTool.name);
const mode = resolveToolApprovalMode({
toolRules,
toolKey: toolName,
capabilityMode: AgentPolicyService.modeForCapability(approvalPolicy, capabilityKey),
});

if (mode === 'deny' || !isToolAllowed(toolRules, toolName)) {
if (mode === 'deny') {
continue;
}

Expand Down
29 changes: 28 additions & 1 deletion src/server/services/agent/__tests__/CapabilityService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const mockListTools = jest.fn();
const mockCallTool = jest.fn();
const mockClose = jest.fn();
const mockLoggerWarn = jest.fn();
const mockModeForCapability = jest.fn(() => 'allow');

let currentTransport: Record<string, unknown> | null = null;

Expand Down Expand Up @@ -57,7 +58,7 @@ jest.mock('../PolicyService', () => ({
__esModule: true,
default: {
capabilityForMcpTool: jest.fn(() => 'external_mcp_read'),
modeForCapability: jest.fn(() => 'allow'),
modeForCapability: (...args: unknown[]) => mockModeForCapability(...args),
},
}));

Expand Down Expand Up @@ -107,6 +108,7 @@ describe('AgentCapabilityService.buildToolSet', () => {

beforeEach(() => {
jest.clearAllMocks();
mockModeForCapability.mockReturnValue('allow');
currentTransport = null;
mockResolveServersForRepo.mockResolvedValue([stdioServer]);
mockConnect.mockImplementation(async (transport) => {
Expand Down Expand Up @@ -228,4 +230,29 @@ describe('AgentCapabilityService.buildToolSet', () => {
expect(tools.mcp__figma__get_design_context).toBeUndefined();
expect(mockLoggerWarn).toHaveBeenCalled();
});

it('lets session tool rules override the family approval mode for sandbox tools', async () => {
mockModeForCapability.mockReturnValue('deny');

const tools = await AgentCapabilityService.buildToolSet({
session,
repoFullName: 'example-org/example-repo',
userIdentity,
approvalPolicy: {} as any,
toolRules: [
{
toolKey: 'mcp__sandbox__workspace_read_file',
mode: 'allow',
},
],
workspaceToolDiscoveryTimeoutMs: 4500,
workspaceToolExecutionTimeoutMs: 22000,
});

expect(tools.mcp__sandbox__workspace_read_file).toEqual(
expect.objectContaining({
needsApproval: false,
})
);
});
});
22 changes: 22 additions & 0 deletions src/server/services/agent/__tests__/sandboxToolCatalog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,26 @@ describe('sandboxToolCatalog', () => {
'- do not claim a tool is unavailable unless it is not equipped here or a real tool call fails',
]);
});

it('keeps explicitly allowed tools in the prompt summary even when the family is denied', () => {
const lines = buildSessionWorkspacePromptLines({
approvalPolicy: {
...DEFAULT_AGENT_APPROVAL_POLICY,
rules: {
...DEFAULT_AGENT_APPROVAL_POLICY.rules,
read: 'deny',
},
},
toolRules: [
{
toolKey: 'mcp__sandbox__workspace_read_file',
mode: 'allow',
},
],
includeSkills: false,
});

expect(lines.join('\n')).toContain('workspace.read_file');
expect(lines.join('\n')).not.toContain('workspace.glob');
});
});
7 changes: 2 additions & 5 deletions src/server/services/agent/sandboxToolCatalog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,13 +284,10 @@ function isSessionWorkspaceToolAllowed(
toolRules: AgentSessionToolRule[] = []
): boolean {
const rule = toolRules.find((item) => item.toolKey === entry.toolKey);
if (rule?.mode === 'deny') {
return false;
}

const capabilityKey = AgentPolicyService.capabilityForMcpTool(entry.toolName, entry.annotations);
const mode = rule?.mode || AgentPolicyService.modeForCapability(approvalPolicy, capabilityKey);

return AgentPolicyService.modeForCapability(approvalPolicy, capabilityKey) !== 'deny';
return mode !== 'deny';
}

export function buildSessionWorkspacePromptLines({
Expand Down
11 changes: 6 additions & 5 deletions src/server/services/agentSessionConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ function normalizeToolRules(value: unknown): AgentSessionToolRule[] {
const toolKey =
typeof (entry as { toolKey?: unknown }).toolKey === 'string' ? (entry as { toolKey: string }).toolKey : '';
const mode = (entry as { mode?: unknown }).mode;
if (!toolKey || (mode !== 'allow' && mode !== 'deny')) {
if (!toolKey || (mode !== 'allow' && mode !== 'require_approval' && mode !== 'deny')) {
continue;
}
deduped.set(toolKey, { toolKey, mode });
Expand Down Expand Up @@ -502,11 +502,12 @@ export default class AgentSessionConfigService extends BaseService {
const approvalMode = AgentPolicyService.modeForCapability(approvalPolicy, capabilityKey);
const scopeRuleMode = toRuleSelection(activeScopeConfig.toolRules || [], toolKey);
const effectiveRuleMode = toRuleSelection(effectiveConfig.toolRules, toolKey);
const resolvedApprovalMode = effectiveRuleMode === 'inherit' ? approvalMode : effectiveRuleMode;
const availability =
approvalMode === 'deny'
? 'blocked_by_policy'
: effectiveRuleMode === 'deny'
? 'blocked_by_tool_rule'
resolvedApprovalMode === 'deny'
? effectiveRuleMode === 'deny'
? 'blocked_by_tool_rule'
: 'blocked_by_policy'
: 'available';

entries.push({
Expand Down
2 changes: 1 addition & 1 deletion src/server/services/types/agentSessionConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import type { AgentApprovalMode, AgentCapabilityKey } from 'server/services/agent/types';

export type AgentSessionToolRuleMode = 'allow' | 'deny';
export type AgentSessionToolRuleMode = AgentApprovalMode;
export type AgentSessionToolRuleSelection = AgentSessionToolRuleMode | 'inherit';

export interface AgentSessionToolRule {
Expand Down
6 changes: 3 additions & 3 deletions src/shared/openApiSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1506,7 +1506,7 @@ export const openApiSpecificationForV2Api: OAS3Options = {
toolKey: { type: 'string' },
mode: {
type: 'string',
enum: ['allow', 'deny'],
enum: ['allow', 'require_approval', 'deny'],
},
},
required: ['toolKey', 'mode'],
Expand Down Expand Up @@ -1631,11 +1631,11 @@ export const openApiSpecificationForV2Api: OAS3Options = {
approvalMode: { $ref: '#/components/schemas/AgentApprovalMode' },
scopeRuleMode: {
type: 'string',
enum: ['inherit', 'allow', 'deny'],
enum: ['inherit', 'allow', 'require_approval', 'deny'],
},
effectiveRuleMode: {
type: 'string',
enum: ['inherit', 'allow', 'deny'],
enum: ['inherit', 'allow', 'require_approval', 'deny'],
},
availability: {
type: 'string',
Expand Down
Loading