chore(hardening): enforce configured permission rules and fail closed on approval

.changeset/hardening-permission-enforcement.md

@@ -0,0 +1,14 @@
+---
+'@moonshot-ai/agent-core-v2': minor
+---
+
+Make the permission layer enforce what it is configured to enforce:
+
+- Seed each agent with the user's `[permission]` rules from config. The rules Op
+  is not persisted, so the model always started empty and configured
+  allow/deny/ask rules never reached the policy chain.
+- Order the sensitive-file and git-control asks ahead of the blanket auto-mode
+  approval, so auto mode no longer waives them. An explicit prior approval or a
+  user `allow` rule still wins, so nothing already approved re-prompts.
+- Fail closed when no approval broker is bound: a call that reached the "ask"
+  path is now blocked instead of being treated as approved.

packages/agent-core-v2/src/agent/permissionPolicy/permissionPolicyService.ts

@@ -42,15 +42,24 @@ export class AgentPermissionPolicyService
     @IInstantiationService private readonly instantiation: IInstantiationService,
   ) {
     super();
+    // Order matters: the first policy to return a result wins.
+    //
+    // `AutoModeApprove` sits after the two content-sensitive asks (secrets on
+    // disk, and the .git control directory) rather than ahead of them, so
+    // enabling auto mode speeds up ordinary work without also silently
+    // waiving the checks that exist for the highest-consequence paths.
+    // Everything else keeps its previous relative order: an explicit prior
+    // approval (`SessionApprovalHistory`) or a user `allow` rule still wins,
+    // so this does not re-prompt for something already approved.
     this.policies = [
       this.instantiation.createInstance(AutoModeAskUserQuestionDenyPermissionPolicyService),
       this.instantiation.createInstance(UserConfiguredDenyPermissionPolicyService),
-      this.instantiation.createInstance(AutoModeApprovePermissionPolicyService),
       this.instantiation.createInstance(SessionApprovalHistoryPermissionPolicyService),
       this.instantiation.createInstance(UserConfiguredAskPermissionPolicyService),
       this.instantiation.createInstance(UserConfiguredAllowPermissionPolicyService),
       this.instantiation.createInstance(SensitiveFileAccessAskPermissionPolicyService),
       this.instantiation.createInstance(GitControlPathAccessAskPermissionPolicyService),
+      this.instantiation.createInstance(AutoModeApprovePermissionPolicyService),
       this.instantiation.createInstance(YoloModeApprovePermissionPolicyService),
       this.instantiation.createInstance(DefaultToolApprovePermissionPolicyService),
       this.instantiation.createInstance(GitCwdWriteApprovePermissionPolicyService),

packages/agent-core-v2/src/agent/toolApproval/toolApprovalService.ts

@@ -131,7 +131,14 @@ export class AgentToolApprovalService extends Service implements IAgentToolAppro
     let response: ApprovalResponse;
     const approvalService = this.tryApprovalService();
     if (approvalService === undefined) {
-      response = { decision: 'approved' };
+      // Fail closed. Reaching here means a policy decided this call needs
+      // confirmation, but no broker is bound to ask (an embedding host that
+      // never wired one up). Treating "nobody to ask" as consent would let
+      // every gated tool call through unreviewed.
+      response = {
+        decision: 'rejected',
+        feedback: 'No approval broker is available to confirm this tool call.',
+      };
     } else {
       this.eventBus.publish({ type: 'permission.approval.requested', ...approvalContext });
       try {

packages/agent-core-v2/src/session/agentLifecycle/agentLifecycleService.ts

@@ -36,6 +36,8 @@ import { IConfigService } from '#/app/config/config';
 import { IEventBus } from '#/app/event/eventBus';
 import { DEFAULT_PERMISSION_MODE_SECTION } from '#/agent/permissionMode/configSection';
 import { PermissionModeConfiguredModel } from '#/agent/permissionMode/permissionModeOps';
+import { PERMISSION_SECTION, type PermissionConfig } from '#/agent/permissionRules/configSection';
+import { IAgentPermissionRulesService } from '#/agent/permissionRules/permissionRules';
 import type { PermissionMode } from '#/agent/permissionPolicy/types';
 import { IAgentTaskService } from '#/agent/task/task';
 import { ISessionContext } from '#/session/sessionContext/sessionContext';
@@ -196,6 +198,14 @@ export class AgentLifecycleService extends Disposable implements IAgentLifecycle
     if (permissionMode !== undefined && !hasRestoredPermissionMode) {
       handle.accessor.get(IAgentPermissionModeService).setMode(permissionMode);
     }
+    // Seed the agent with the user's persisted `[permission]` rules. The
+    // `permission.rules.add` Op is not persisted, so a rules model always
+    // starts empty and has to be filled here — otherwise the config section
+    // parses fine but the user-configured policies never see a rule to match.
+    const configuredRules = this.config.get<PermissionConfig>(PERMISSION_SECTION)?.rules;
+    if (configuredRules !== undefined && configuredRules.length > 0) {
+      handle.accessor.get(IAgentPermissionRulesService).addRules(configuredRules);
+    }
   }
 
   async fork(sourceAgentId: string, opts?: ForkAgentOptions): Promise<IAgentScopeHandle> {

packages/agent-core-v2/test/agent/permissionPolicy/permissionPolicyService.test.ts

@@ -298,6 +298,32 @@ describe('AgentPermissionPolicyService git cwd write approval', () => {
     });
   });
 
+  it('still asks for sensitive files in auto mode', async () => {
+    // Auto mode speeds up ordinary work; it must not silently waive the
+    // secrets check.
+    mode = 'auto';
+    await expect(evaluate({
+      toolName: 'Write',
+      args: { path: '.env', content: 'SECRET=1' },
+      accesses: ToolAccesses.writeFile(join(workspaceDir, '.env')),
+    })).resolves.toMatchObject({
+      policyName: 'sensitive-file-access-ask',
+      result: { kind: 'ask' },
+    });
+  });
+
+  it('still asks for git control files in auto mode', async () => {
+    mode = 'auto';
+    await expect(evaluate({
+      toolName: 'Write',
+      args: { path: '.git/config', content: 'x' },
+      accesses: ToolAccesses.writeFile(join(workspaceDir, '.git/config')),
+    })).resolves.toMatchObject({
+      policyName: 'git-control-path-access-ask',
+      result: { kind: 'ask' },
+    });
+  });
+
   it('does not use git-cwd approval in auto mode', async () => {
     mode = 'auto';
     await expect(evaluate({

packages/agent-core-v2/test/agent/toolApproval/toolApproval.test.ts

@@ -241,28 +241,37 @@ describe('AgentToolApprovalService', () => {
   });
 
   describe('requestToolApproval', () => {
-    it('auto-approves when no approval broker is registered', async () => {
+    it('fails closed when no approval broker is registered', async () => {
+      // A policy already decided this call needs confirmation. With no broker
+      // to ask, the call must be blocked rather than treated as approved.
       const events = subscribeApprovalEvents();
       const svc = make();
 
       await expect(
         svc.requestToolApproval(makeContext('Bash', { command: 'printf hi' }), ask(), 'fallback-ask'),
-      ).resolves.toBeUndefined();
+      ).resolves.toEqual({
+        veto: {
+          output:
+            'Tool "Bash" was not run because the user rejected the approval request. ' +
+            'Reason: No approval broker is available to confirm this tool call.',
+          isError: true,
+        },
+      });
 
       expect(events.requested).not.toHaveBeenCalled();
       expect(events.resolved).not.toHaveBeenCalled();
       expect(recorded).toHaveLength(1);
       expect(recorded[0]).toMatchObject({
         toolName: 'Bash',
         sessionApprovalRule: undefined,
-        result: { decision: 'approved' },
+        result: { decision: 'rejected' },
       });
       expect(records).toContainEqual({
         event: 'permission_approval_result',
         properties: expect.objectContaining({
           policy_name: 'fallback-ask',
           tool_name: 'Bash',
-          result: 'approved',
+          result: 'rejected',
           session_cache_written: false,
         }),
       });

packages/agent-core-v2/test/session/agentLifecycle/agentLifecycle.test.ts

@@ -22,6 +22,9 @@ import { IAgentMcpService } from '#/agent/mcp/mcp';
 import { McpConnectionManager } from '#/mcpCore/connection-manager';
 import { IAgentPermissionModeService } from '#/agent/permissionMode/permissionMode';
 import '#/agent/permissionMode/permissionModeOps';
+import { PERMISSION_SECTION } from '#/agent/permissionRules/configSection';
+import { IAgentPermissionRulesService } from '#/agent/permissionRules/permissionRules';
+import '#/agent/permissionRules/permissionRulesService';
 import { IAgentStateService } from '#/agent/state/agentState';
 import { AgentStateService } from '#/agent/state/agentStateService';
 import { ISessionStateService } from '#/session/state/sessionState';
@@ -586,6 +589,32 @@ describe('AgentLifecycleService', () => {
     expect(permissionModeSetMode).toHaveBeenCalledWith('auto');
   });
 
+  it('seeds the agent with the permission rules from config', async () => {
+    // The rules Op is not persisted, so without this seeding a user's
+    // `[permission]` deny/allow/ask config would parse but never reach the
+    // policy chain.
+    ix.stub(IConfigService, {
+      ready: Promise.resolve(),
+      get: ((section: string) =>
+        section === PERMISSION_SECTION
+          ? { rules: [{ decision: 'deny', scope: 'user', pattern: 'Bash' }] }
+          : undefined) as IConfigService['get'],
+      onDidSectionChange: (() => ({ dispose: () => {} })) as IConfigService['onDidSectionChange'],
+    } as unknown as IConfigService);
+
+    const handle = await ix.get(IAgentLifecycleService).create({ agentId: 'main' });
+
+    expect(handle.accessor.get(IAgentPermissionRulesService).rules).toEqual([
+      { decision: 'deny', scope: 'user', pattern: 'Bash' },
+    ]);
+  });
+
+  it('leaves the rules empty when config has no permission section', async () => {
+    const handle = await ix.get(IAgentLifecycleService).create({ agentId: 'main' });
+
+    expect(handle.accessor.get(IAgentPermissionRulesService).rules).toEqual([]);
+  });
+
   it('keeps the restored permission mode instead of overwriting it with the default', async () => {
     ix.stub(IAppendLogStore, recordingAppendLog([
       createWireMetadataRecord(1),