From 6f11d7eaf7f9f44dc58b4b1311c4bc2919ad1cf1 Mon Sep 17 00:00:00 2001 From: Christine Betts Date: Wed, 25 Feb 2026 12:59:45 -0500 Subject: [PATCH] Address comments --- package-lock.json | 27 +---- packages/cli/src/config/extension-manager.ts | 6 +- .../core/src/policy/policy-engine.test.ts | 105 ++++++++++++++++++ packages/core/src/policy/toml-loader.test.ts | 2 +- 4 files changed, 110 insertions(+), 30 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8b12e7f0f3..0f1c98ec4a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2100,7 +2100,6 @@ "resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.26.0.tgz", "integrity": "sha512-Y5RmPncpiDtTXDbLKswIJzTqu2hyBKxTNsgKqKclDbhIgg1wgtf1fRuvxgTnRfcnxtvvgbIEcqUOzZrJ6iSReg==", "license": "MIT", - "peer": true, "dependencies": { "@hono/node-server": "^1.19.9", "ajv": "^8.17.1", @@ -2243,7 +2242,6 @@ "integrity": "sha512-t54CUOsFMappY1Jbzb7fetWeO0n6K0k/4+/ZpkS+3Joz8I4VcvY9OiEBFRYISqaI2fq5sCiPtAjRDOzVYG8m+Q==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@octokit/auth-token": "^6.0.0", "@octokit/graphql": "^9.0.2", @@ -2424,7 +2422,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/api/-/api-1.9.0.tgz", "integrity": "sha512-3giAOQvZiH5F9bMlMiv8+GSPMeqg0dbaeo58/0SlA9sxSqZhnUtxzX9/2FzyhS9sWQf5S0GJE0AKBrFqjpeYcg==", "license": "Apache-2.0", - "peer": true, "engines": { "node": ">=8.0.0" } @@ -2474,7 +2471,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/core/-/core-2.5.0.tgz", "integrity": "sha512-ka4H8OM6+DlUhSAZpONu0cPBtPPTQKxbxVzC4CzVx5+K4JnroJVBtDzLAMx4/3CDTJXRvVFhpFjtl4SaiTNoyQ==", "license": "Apache-2.0", - "peer": true, "dependencies": { "@opentelemetry/semantic-conventions": "^1.29.0" }, @@ -2849,7 +2845,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/resources/-/resources-2.5.0.tgz", "integrity": "sha512-F8W52ApePshpoSrfsSk1H2yJn9aKjCrbpQF1M9Qii0GHzbfVeFUB+rc3X4aggyZD8x9Gu3Slua+s6krmq6Dt8g==", "license": "Apache-2.0", - "peer": true, "dependencies": { "@opentelemetry/core": "2.5.0", "@opentelemetry/semantic-conventions": "^1.29.0" @@ -2883,7 +2878,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/sdk-metrics/-/sdk-metrics-2.5.0.tgz", "integrity": "sha512-BeJLtU+f5Gf905cJX9vXFQorAr6TAfK3SPvTFqP+scfIpDQEJfRaGJWta7sJgP+m4dNtBf9y3yvBKVAZZtJQVA==", "license": "Apache-2.0", - "peer": true, "dependencies": { "@opentelemetry/core": "2.5.0", "@opentelemetry/resources": "2.5.0" @@ -2938,7 +2932,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/sdk-trace-base/-/sdk-trace-base-2.5.0.tgz", "integrity": "sha512-VzRf8LzotASEyNDUxTdaJ9IRJ1/h692WyArDBInf5puLCjxbICD6XkHgpuudis56EndyS7LYFmtTMny6UABNdQ==", "license": "Apache-2.0", - "peer": true, "dependencies": { "@opentelemetry/core": "2.5.0", "@opentelemetry/resources": "2.5.0", @@ -4102,7 +4095,6 @@ "integrity": "sha512-6mDvHUFSjyT2B2yeNx2nUgMxh9LtOWvkhIU3uePn2I2oyNymUAX1NIsdgviM4CH+JSrp2D2hsMvJOkxY+0wNRA==", "devOptional": true, "license": "MIT", - "peer": true, "dependencies": { "csstype": "^3.0.2" } @@ -4376,7 +4368,6 @@ "integrity": "sha512-klQbnPAAiGYFyI02+znpBRLyjL4/BrBd0nyWkdC0s/6xFLkXYQ8OoRrSkqacS1ddVxf/LDyODIKbQ5TgKAf/Fg==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.56.1", "@typescript-eslint/types": "8.56.1", @@ -5224,7 +5215,6 @@ "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.15.0.tgz", "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -7759,7 +7749,6 @@ "integrity": "sha512-VmQ+sifHUbI/IcSopBCF/HO3YiHQx/AVd3UVyYL6weuwW+HvON9VYn5l6Zl1WZzPWXPNZrSQpxwkkZ/VuvJZzg==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -8269,7 +8258,6 @@ "resolved": "https://registry.npmjs.org/express/-/express-5.2.1.tgz", "integrity": "sha512-hIS4idWWai69NezIdRt2xFVofaF4j+6INOpJlVOLDO8zXGpUVEVzIYk12UUi2JzjEzWL3IOAxcTubgz9Po0yXw==", "license": "MIT", - "peer": true, "dependencies": { "accepts": "^2.0.0", "body-parser": "^2.2.1", @@ -9566,7 +9554,6 @@ "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.2.tgz", "integrity": "sha512-gJnaDHXKDayjt8ue0n8Gs0A007yKXj4Xzb8+cNjZeYsSzzwKc0Lr+OZgYwVfB0pHfUs17EPoLvrOsEaJ9mj+Tg==", "license": "MIT", - "peer": true, "engines": { "node": ">=16.9.0" } @@ -9846,7 +9833,6 @@ "resolved": "https://registry.npmjs.org/@jrichman/ink/-/ink-6.4.11.tgz", "integrity": "sha512-93LQlzT7vvZ1XJcmOMwN4s+6W334QegendeHOMnEJBlhnpIzr8bws6/aOEHG8ZCuVD/vNeeea5m1msHIdAY6ig==", "license": "MIT", - "peer": true, "dependencies": { "@alcalzone/ansi-tokenize": "^0.2.1", "ansi-escapes": "^7.0.0", @@ -13496,7 +13482,6 @@ "resolved": "https://registry.npmjs.org/react/-/react-19.2.4.tgz", "integrity": "sha512-9nfp2hYpCwOjAN+8TZFGhtWEwgvWHXqESH8qT89AT/lWklpLON22Lc8pEtnpsZz7VmawabSU0gCjnj8aC0euHQ==", "license": "MIT", - "peer": true, "engines": { "node": ">=0.10.0" } @@ -13507,7 +13492,6 @@ "integrity": "sha512-ePrwPfxAnB+7hgnEr8vpKxL9cmnp7F322t8oqcPshbIQQhDKgFDW4tjhF2wjVbdXF9O/nyuy3sQWd9JGpiLPvA==", "devOptional": true, "license": "MIT", - "peer": true, "dependencies": { "shell-quote": "^1.6.1", "ws": "^7" @@ -15490,7 +15474,6 @@ "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz", "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -15714,8 +15697,7 @@ "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.8.1.tgz", "integrity": "sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w==", "dev": true, - "license": "0BSD", - "peer": true + "license": "0BSD" }, "node_modules/tsx": { "version": "4.20.3", @@ -15723,7 +15705,6 @@ "integrity": "sha512-qjbnuR9Tr+FJOMBqJCW5ehvIo/buZq7vH7qD7JziU98h6l3qGy0a/yPFjwO+y0/T7GFpNgNAvEcPPVfyT8rrPQ==", "devOptional": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "~0.25.0", "get-tsconfig": "^4.7.5" @@ -15871,7 +15852,6 @@ "integrity": "sha512-p1diW6TqL9L07nNxvRMM7hMMw4c5XOo/1ibL4aAIGmSAt9slTE1Xgw5KWuof2uTOvCg9BY7ZRi+GaF+7sfgPeQ==", "devOptional": true, "license": "Apache-2.0", - "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -16080,7 +16060,6 @@ "resolved": "https://registry.npmjs.org/vite/-/vite-7.2.2.tgz", "integrity": "sha512-BxAKBWmIbrDgrokdGZH1IgkIk/5mMHDreLDmCJ0qpyJaAteP8NvMhkwr/ZCQNqNH97bw/dANTE9PDzqwJghfMQ==", "license": "MIT", - "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.5.0", @@ -16194,7 +16173,6 @@ "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz", "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -16207,7 +16185,6 @@ "resolved": "https://registry.npmjs.org/vitest/-/vitest-3.2.4.tgz", "integrity": "sha512-LUCP5ev3GURDysTWiP47wRRUpLKMOfPh+yKTx3kVIEiu5KOMeqzpnYNsKyOoVrULivR8tLcks4+lga33Whn90A==", "license": "MIT", - "peer": true, "dependencies": { "@types/chai": "^5.2.2", "@vitest/expect": "3.2.4", @@ -16849,7 +16826,6 @@ "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", "license": "MIT", - "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } @@ -17394,7 +17370,6 @@ "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz", "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, diff --git a/packages/cli/src/config/extension-manager.ts b/packages/cli/src/config/extension-manager.ts index 5218e332ac..e201c0aeec 100644 --- a/packages/cli/src/config/extension-manager.ts +++ b/packages/cli/src/config/extension-manager.ts @@ -51,7 +51,7 @@ import { applyAdminAllowlist, getAdminBlockedMcpServersMessage, CoreToolCallStatus, - WORKSPACE_POLICY_TIER, + EXTENSION_POLICY_TIER, loadPoliciesFromToml, PolicyDecision, ApprovalMode, @@ -780,7 +780,7 @@ Would you like to attempt to install via "git clone" instead?`, if (fs.existsSync(policyDir)) { const result = await loadPoliciesFromToml( [policyDir], - () => WORKSPACE_POLICY_TIER, + () => EXTENSION_POLICY_TIER, ); rules = result.rules; checkers = result.checkers; @@ -834,7 +834,7 @@ Would you like to attempt to install via "git clone" instead?`, if (result.errors.length > 0) { for (const error of result.errors) { debugLogger.warn( - `[ExtensionManager] Error loading policies from ${config.name}: ${error.message}`, + `[ExtensionManager] Error loading policies from ${config.name}: ${error.message}${error.details ? `\nDetails: ${error.details}` : ''}`, ); } } diff --git a/packages/core/src/policy/policy-engine.test.ts b/packages/core/src/policy/policy-engine.test.ts index 0d110f8b2d..278138fa55 100644 --- a/packages/core/src/policy/policy-engine.test.ts +++ b/packages/core/src/policy/policy-engine.test.ts @@ -406,6 +406,40 @@ describe('PolicyEngine', () => { expect(remainingRules.some((r) => r.toolName === 'tool2')).toBe(true); }); + it('should remove rules for specific tool and source', () => { + engine.addRule({ + toolName: 'tool1', + decision: PolicyDecision.ALLOW, + source: 'source1', + }); + engine.addRule({ + toolName: 'tool1', + decision: PolicyDecision.DENY, + source: 'source2', + }); + engine.addRule({ + toolName: 'tool2', + decision: PolicyDecision.ALLOW, + source: 'source1', + }); + + expect(engine.getRules()).toHaveLength(3); + + engine.removeRulesForTool('tool1', 'source1'); + + const rules = engine.getRules(); + expect(rules).toHaveLength(2); + expect( + rules.some((r) => r.toolName === 'tool1' && r.source === 'source2'), + ).toBe(true); + expect( + rules.some((r) => r.toolName === 'tool2' && r.source === 'source1'), + ).toBe(true); + expect( + rules.some((r) => r.toolName === 'tool1' && r.source === 'source1'), + ).toBe(false); + }); + it('should handle removing non-existent tool', () => { engine.addRule({ toolName: 'existing', decision: PolicyDecision.ALLOW }); @@ -2828,6 +2862,34 @@ describe('PolicyEngine', () => { }); }); + describe('removeRulesBySource', () => { + it('should remove rules matching a specific source', () => { + engine.addRule({ + toolName: 'rule1', + decision: PolicyDecision.ALLOW, + source: 'source1', + }); + engine.addRule({ + toolName: 'rule2', + decision: PolicyDecision.ALLOW, + source: 'source2', + }); + engine.addRule({ + toolName: 'rule3', + decision: PolicyDecision.ALLOW, + source: 'source1', + }); + + expect(engine.getRules()).toHaveLength(3); + + engine.removeRulesBySource('source1'); + + const rules = engine.getRules(); + expect(rules).toHaveLength(1); + expect(rules[0].toolName).toBe('rule2'); + }); + }); + describe('removeCheckersByTier', () => { it('should remove checkers matching a specific tier', () => { engine.addChecker({ @@ -2853,6 +2915,31 @@ describe('PolicyEngine', () => { }); }); + describe('removeCheckersBySource', () => { + it('should remove checkers matching a specific source', () => { + engine.addChecker({ + checker: { type: 'external', name: 'c1' }, + source: 'sourceA', + }); + engine.addChecker({ + checker: { type: 'external', name: 'c2' }, + source: 'sourceB', + }); + engine.addChecker({ + checker: { type: 'external', name: 'c3' }, + source: 'sourceA', + }); + + expect(engine.getCheckers()).toHaveLength(3); + + engine.removeCheckersBySource('sourceA'); + + const checkers = engine.getCheckers(); + expect(checkers).toHaveLength(1); + expect(checkers[0].checker.name).toBe('c2'); + }); + }); + describe('Tool Annotations', () => { it('should match tools by semantic annotations', async () => { engine = new PolicyEngine({ @@ -2916,4 +3003,22 @@ describe('PolicyEngine', () => { ).toBe(PolicyDecision.ALLOW); }); }); + + describe('hook checkers', () => { + it('should add and retrieve hook checkers in priority order', () => { + engine.addHookChecker({ + checker: { type: 'external', name: 'h1' }, + priority: 5, + }); + engine.addHookChecker({ + checker: { type: 'external', name: 'h2' }, + priority: 10, + }); + + const hookCheckers = engine.getHookCheckers(); + expect(hookCheckers).toHaveLength(2); + expect(hookCheckers[0].priority).toBe(10); + expect(hookCheckers[1].priority).toBe(5); + }); + }); }); diff --git a/packages/core/src/policy/toml-loader.test.ts b/packages/core/src/policy/toml-loader.test.ts index 00b1eb2701..4dd1e33397 100644 --- a/packages/core/src/policy/toml-loader.test.ts +++ b/packages/core/src/policy/toml-loader.test.ts @@ -274,7 +274,7 @@ modes = ["autoEdit"] `, ); - const getPolicyTier3 = (_dir: string) => 3; // Tier 3 (Workspace) + const getPolicyTier3 = (_dir: string) => 3; // Tier 3 (User) const result3 = await loadPoliciesFromToml([tempDir], getPolicyTier3); expect(result3.rules).toHaveLength(1);