From 45949ba68a2edb78f76d7258a9c8b9802e1329c6 Mon Sep 17 00:00:00 2001 From: d-robotics Date: Sun, 11 Oct 2026 01:34:07 +0800 Subject: [PATCH] fix(config): user trustedTools wins over trusted project list (O55) Reverts the trustedTools union from #79. When the user sets trustedTools, that list is used exactly; a trusted project's list applies only when the user left it unset. Untrusted projects' trustedTools stay dropped. --- src/cli/config.ts | 5 +- test/cli-config-merge.spec.mjs | 14 +++- .../cli-config-untrusted-permissions.spec.mjs | 6 +- test/o55-user-trustedtools.spec.mjs | 76 +++++++++++++++++++ 4 files changed, 92 insertions(+), 9 deletions(-) create mode 100644 test/o55-user-trustedtools.spec.mjs diff --git a/src/cli/config.ts b/src/cli/config.ts index a660e848..9db768a8 100644 --- a/src/cli/config.ts +++ b/src/cli/config.ts @@ -946,7 +946,8 @@ export function mergeConfigFiles( // A cloned repo's .moss/config.json must not silently lower the user's // safety stance (e.g. approvalPolicy: 'never', safetyMode: 'full-access'). // If the user hasn't set a scalar, the project value is still used. - // trustedTools is a grant list, so a trusted project unions with the user. + // trustedTools follows that rule: the user's array wins exactly. A trusted + // project's list is used only when the user did not set one. // loadCliConfigFile removes an untrusted project's trustedTools, // permissions.allow, permissions.defaultMode, permissions.deviceTrust, // permissions.trustedDevices, and a looser profile before this merge, so @@ -956,7 +957,7 @@ export function mergeConfigFiles( // override both (resolveCliConfig). safetyMode: userConfig.safetyMode ?? projectConfig.safetyMode, approvalPolicy: userConfig.approvalPolicy ?? projectConfig.approvalPolicy, - trustedTools: unionOptionalStringList(userConfig.trustedTools, projectConfig.trustedTools), + trustedTools: userConfig.trustedTools ?? projectConfig.trustedTools, deniedTools: userConfig.deniedTools ?? projectConfig.deniedTools, permissions: mergePermissionsConfig(userConfig.permissions, projectConfig.permissions), promptCache: mergePromptCacheConfig(userConfig.promptCache, projectConfig.promptCache), diff --git a/test/cli-config-merge.spec.mjs b/test/cli-config-merge.spec.mjs index 7b21e35a..f6eb7945 100644 --- a/test/cli-config-merge.spec.mjs +++ b/test/cli-config-merge.spec.mjs @@ -7,7 +7,8 @@ * over user for ALL top-level fields, so a project could silently lower the * user's safety stance (approvalPolicy: 'never', safetyMode: 'full-access', * widening trustedTools when the user had set a narrower list). Scalars stay - * user-priority. trustedTools is a grant list and unions both layers; + * user-priority. trustedTools is user-priority too: the user's array wins + * exactly, and a project list applies only when the user did not set one. * loadCliConfigFile drops an untrusted project's trustedTools before merge. * Non-safety fields keep project-priority. */ @@ -23,15 +24,20 @@ import { mergeConfigFiles } from '../dist/cli/config.js'; assert.equal(merged.approvalPolicy, 'prompt', 'user approvalPolicy wins over project'); } -// ─── 2. trustedTools unions both layers (untrusted projects are stripped first) +// ─── 2. trustedTools: the user's list wins; project only when user unset ── { const user = { trustedTools: ['safe_tool'] }; const project = { trustedTools: ['safe_tool', 'dangerous_tool'] }; const merged = mergeConfigFiles(project, user); assert.deepEqual( merged.trustedTools, - ['safe_tool', 'dangerous_tool'], - 'trustedTools unions user and project; an untrusted project list is removed before merge' + ['safe_tool'], + 'user trustedTools wins exactly; a project list does not widen it' + ); + assert.deepEqual( + mergeConfigFiles({ trustedTools: ['exec'] }, {}).trustedTools, + ['exec'], + 'project trustedTools used when the user did not set the list' ); } diff --git a/test/cli-config-untrusted-permissions.spec.mjs b/test/cli-config-untrusted-permissions.spec.mjs index e77b3a85..21299af7 100644 --- a/test/cli-config-untrusted-permissions.spec.mjs +++ b/test/cli-config-untrusted-permissions.spec.mjs @@ -215,12 +215,12 @@ function noticeLine(stderr, field) { assert.equal(trusted.droppedProjectPermissions, undefined); assert.equal(trusted.config.permissions.defaultMode, 'manual'); assert.deepEqual(trusted.config.permissions.allow, ['exec']); - assert.deepEqual(trusted.config.trustedTools, ['safe_tool', 'exec']); + // O55: the user's trustedTools array wins exactly. Project allow still unions. + assert.deepEqual(trusted.config.trustedTools, ['safe_tool']); assert.deepEqual(trusted.config.permissions.deny, ['read_file', 'exec']); const resolvedTrusted = resolveCliConfig(env, trusted.config, {}, trusted); assert.ok(resolvedTrusted.permissions.allow.includes('exec')); - assert.ok(resolvedTrusted.trustedTools.includes('safe_tool')); - assert.ok(resolvedTrusted.trustedTools.includes('exec')); + assert.deepEqual(resolvedTrusted.trustedTools, ['safe_tool']); } { diff --git a/test/o55-user-trustedtools.spec.mjs b/test/o55-user-trustedtools.spec.mjs new file mode 100644 index 00000000..238ac483 --- /dev/null +++ b/test/o55-user-trustedtools.spec.mjs @@ -0,0 +1,76 @@ +#!/usr/bin/env node +/** + * O55: a user's trustedTools array wins exactly over a TRUSTED project's list. + * The project list applies only when the user did not set trustedTools. + * An untrusted project's trustedTools stays dropped. + */ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { isolatedCliEnv } from './helpers/isolated-cli-env.mjs'; +import { loadCliConfigFile, resolveCliConfig } from '../dist/cli/config.js'; + +function put(file, value) { + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, JSON.stringify(value)); +} + +function setup(project, { trust = false, user = {} } = {}) { + const env = isolatedCliEnv({ prefix: 'moss-o55-' }); + for (const key of Object.keys(env)) { + if (/^MOSS_(SAFETY|APPROVAL|ASK_FOR|TRUST|AUTO_APPROVE|CLI_|DEVICE)/.test(key)) { + delete env[key]; + } + } + delete env.MOSS_TRUSTED_TOOLS; + delete env.MOSS_PROFILE; + delete env.MOSS_CONFIG_PROFILE; + const ws = path.join(env.HOME, 'ws'); + put(path.join(env.MOSS_CONFIG_DIR, 'config.json'), user); + put(path.join(ws, '.moss', 'config.json'), project); + const loaded = loadCliConfigFile(env, [], ws, { trustProjectRouting: trust }); + const resolved = resolveCliConfig(env, loaded.config, {}, loaded); + return { loaded, resolved }; +} + +const failures = []; +function check(label, fn) { + try { + fn(); + console.log(`[PASS] ${label}`); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + failures.push(label); + console.log(`[FAIL] ${label}: ${message}`); + } +} + +check('user [read_file] wins over trusted project [read_file, exec]', () => { + const { loaded, resolved } = setup( + { trustedTools: ['read_file', 'exec'] }, + { trust: true, user: { trustedTools: ['read_file'] } } + ); + assert.deepEqual(loaded.config.trustedTools, ['read_file']); + assert.deepEqual(resolved.trustedTools, ['read_file']); + assert.ok(!resolved.trustedTools.includes('exec')); +}); + +check('user unset + trusted project [exec] resolves to [exec]', () => { + const { loaded, resolved } = setup({ trustedTools: ['exec'] }, { trust: true }); + assert.deepEqual(loaded.config.trustedTools, ['exec']); + assert.deepEqual(resolved.trustedTools, ['exec']); + assert.equal(loaded.droppedProjectPermissions, undefined); +}); + +check('untrusted project [exec] with user unset does not grant exec', () => { + const { loaded, resolved } = setup({ trustedTools: ['exec'] }); + assert.deepEqual(loaded.droppedProjectPermissions, ['trustedTools']); + assert.equal(loaded.config.trustedTools, undefined); + assert.ok(!resolved.trustedTools.includes('exec')); +}); + +if (failures.length > 0) { + console.log(`${failures.length} failing`); + process.exit(1); +} +console.log('[PASS] o55-user-trustedtools');