From 2eb4af53b3b2eeee3212fa2499def2bb76562967 Mon Sep 17 00:00:00 2001 From: d-robotics Date: Sun, 11 Oct 2026 00:52:04 +0800 Subject: [PATCH] fix(config): ignore untrusted project device trust; allowlist permission merge (O56) Untrusted folders drop project permissions.deviceTrust and permissions.trustedDevices. mergePermissionsConfig no longer spreads the project block: every PermissionsConfig field is handled explicitly and a project may only tighten the user (deviceTrust full never comes from a project; trustedDevices is intersected with the user's list). --- src/cli/config.ts | 121 ++++++++++++++--- test/o56-untrusted-permissions.spec.mjs | 166 ++++++++++++++++++++++++ 2 files changed, 268 insertions(+), 19 deletions(-) create mode 100644 test/o56-untrusted-permissions.spec.mjs diff --git a/src/cli/config.ts b/src/cli/config.ts index 9b70fa24..a660e848 100644 --- a/src/cli/config.ts +++ b/src/cli/config.ts @@ -226,10 +226,14 @@ export interface PermissionsConfig { deny?: string[]; /** * `full` opts every device into the destructive tier (reboot, flash, system - * paths). Absent or `gated` keeps the safe default. + * paths). Absent or `gated` keeps the safe default. A project file may set + * `gated` to tighten the user, and cannot set `full`. */ deviceTrust?: 'gated' | 'full' | string; - /** Hosts or device ids that may run the destructive tier without a prompt. */ + /** + * Hosts or device ids that may run the destructive tier without a prompt. + * A project file cannot add an id the user did not already list. + */ trustedDevices?: string[]; } @@ -243,7 +247,9 @@ export interface LoadedCliConfigFile { ignoredProjectRouting?: string[]; /** * Project permission fields that would loosen the user's mode, dropped - * because the folder is not trusted. `deny` and `ask` are not in this list. + * because the folder is not trusted. Includes allow, defaultMode, + * deviceTrust, trustedDevices, trustedTools, and a looser profile. + * `deny` and `ask` are not in this list. */ droppedProjectPermissions?: string[]; /** @@ -749,10 +755,12 @@ function projectProfileLoosens(user: ConfigFile, project: ConfigFile): boolean { } /** - * Project fields that grant tools or replace the permission mode. - * An untrusted folder drops these. `permissions.ask` and `permissions.deny` - * only tighten, so they are not listed. A project profile is listed only - * when it is looser than the user's. + * Project fields that grant tools, trust a device, or replace the permission + * mode. An untrusted folder drops these. `permissions.ask` and + * `permissions.deny` only tighten, so they are not listed. A project profile + * is listed only when it is looser than the user's. Device trust is never + * taken from an untrusted folder, so any set `deviceTrust` or + * `trustedDevices` list is listed. */ function listDroppedProjectPermissionFields(user: ConfigFile, project: ConfigFile): string[] { const dropped: string[] = []; @@ -760,6 +768,13 @@ function listDroppedProjectPermissionFields(user: ConfigFile, project: ConfigFil if (permissionRuleListLoosens(project.trustedTools)) dropped.push('trustedTools'); const mode = project.permissions?.defaultMode; if (typeof mode === 'string' && mode.trim().length > 0) dropped.push('permissions.defaultMode'); + const deviceTrust = project.permissions?.deviceTrust; + if (typeof deviceTrust === 'string' && deviceTrust.trim().length > 0) { + dropped.push('permissions.deviceTrust'); + } + if (permissionRuleListLoosens(project.permissions?.trustedDevices)) { + dropped.push('permissions.trustedDevices'); + } if (projectProfileLoosens(user, project)) dropped.push('profile'); return dropped; } @@ -779,6 +794,14 @@ function omitLooseningProjectPermissions(user: ConfigFile, project: ConfigFile): delete permissions.defaultMode; changed = true; } + if (typeof permissions.deviceTrust === 'string' && permissions.deviceTrust.trim().length > 0) { + delete permissions.deviceTrust; + changed = true; + } + if (permissionRuleListLoosens(permissions.trustedDevices)) { + delete permissions.trustedDevices; + changed = true; + } if (!changed) return next; const remaining = Object.values(permissions).some((value) => value !== undefined); next.permissions = remaining ? permissions : undefined; @@ -925,9 +948,12 @@ export function mergeConfigFiles( // 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. // loadCliConfigFile removes an untrusted project's trustedTools, - // permissions.allow, permissions.defaultMode, and a looser profile before - // this merge, so those grants cannot widen the user's mode. CLI flags and - // env vars override both (resolveCliConfig). + // permissions.allow, permissions.defaultMode, permissions.deviceTrust, + // permissions.trustedDevices, and a looser profile before this merge, so + // those grants cannot widen the user's mode. mergePermissionsConfig also + // refuses a project deviceTrust of full and any trusted device the user + // did not list, including in a trusted folder. CLI flags and env vars + // override both (resolveCliConfig). safetyMode: userConfig.safetyMode ?? projectConfig.safetyMode, approvalPolicy: userConfig.approvalPolicy ?? projectConfig.approvalPolicy, trustedTools: unionOptionalStringList(userConfig.trustedTools, projectConfig.trustedTools), @@ -1008,25 +1034,82 @@ function unionOptionalStringList( } /** - * v0.26 permission-block merge, safety-directional like the other safety - * fields: user-wins on every scalar knob; rule lists union BOTH layers - * (deny rules union across levels by design — a project cannot silently - * remove the user's deny; allow/ask dedupe). An untrusted project's - * `allow` and `defaultMode` are removed before this function runs. + * A project `gated` tightens `full`. A project `full` never applies, trusted + * folder or not. Any other project value is ignored. + */ +function mergePermissionDeviceTrust( + user: PermissionsConfig['deviceTrust'], + project: PermissionsConfig['deviceTrust'] +): PermissionsConfig['deviceTrust'] | undefined { + if (typeof project === 'string' && project.trim().toLowerCase() === 'gated') return 'gated'; + if (typeof user === 'string' && user.trim().length > 0) return user; + return undefined; +} + +/** + * Project ids never add a device the user did not list. When the project sets + * a list, the result is the intersection; when it does not, the user's list + * stands. No user list means the project list is ignored. + */ +function mergePermissionTrustedDevices( + user: readonly string[] | undefined, + project: readonly string[] | undefined +): string[] | undefined { + if (!Array.isArray(user)) return undefined; + if (!Array.isArray(project)) return [...user]; + const projectIds = new Set(); + for (const entry of project) { + if (typeof entry !== 'string') continue; + const trimmed = entry.trim(); + if (trimmed.length > 0) projectIds.add(trimmed); + } + const seen = new Set(); + const intersection: string[] = []; + for (const entry of user) { + if (typeof entry !== 'string') continue; + const trimmed = entry.trim(); + if (trimmed.length === 0 || seen.has(trimmed) || !projectIds.has(trimmed)) continue; + seen.add(trimmed); + intersection.push(trimmed); + } + return intersection; +} + +/** + * Permission-block merge. Allowlist of PermissionsConfig: the project object + * is never spread, so unknown project fields are dropped. A project may only + * tighten the user. + * - deny / ask union (a project cannot remove the user's rules) + * - allow unions only what the caller still passes. loadCliConfigFile removes + * an untrusted project's allow before this runs + * - defaultMode keeps the user's value when set, so a project cannot loosen + * it. An untrusted project's defaultMode is removed before this runs + * - deviceTrust: project `gated` over user `full`; never project `full` + * - trustedDevices: intersection with the user's list, never an addition */ function mergePermissionsConfig( user: PermissionsConfig | undefined, project: PermissionsConfig | undefined ): PermissionsConfig | undefined { if (!user && !project) return undefined; - return { - ...(user ?? {}), - ...(project ?? {}), + // Every PermissionsConfig key is required here, so a new field fails the + // build until the allowlist handles it. Unknown project keys are not copied. + const fields = { defaultMode: user?.defaultMode ?? project?.defaultMode, allow: unionOptionalStringList(user?.allow, project?.allow), ask: unionOptionalStringList(user?.ask, project?.ask), deny: unionOptionalStringList(user?.deny, project?.deny), - }; + deviceTrust: mergePermissionDeviceTrust(user?.deviceTrust, project?.deviceTrust), + trustedDevices: mergePermissionTrustedDevices(user?.trustedDevices, project?.trustedDevices), + } satisfies Record; + const merged: PermissionsConfig = {}; + if (fields.defaultMode !== undefined) merged.defaultMode = fields.defaultMode; + if (fields.allow !== undefined) merged.allow = fields.allow; + if (fields.ask !== undefined) merged.ask = fields.ask; + if (fields.deny !== undefined) merged.deny = fields.deny; + if (fields.deviceTrust !== undefined) merged.deviceTrust = fields.deviceTrust; + if (fields.trustedDevices !== undefined) merged.trustedDevices = fields.trustedDevices; + return Object.keys(merged).length > 0 ? merged : undefined; } function argvTrustsWorkspace(argv: readonly string[]): boolean { diff --git a/test/o56-untrusted-permissions.spec.mjs b/test/o56-untrusted-permissions.spec.mjs new file mode 100644 index 00000000..ac7c5ffd --- /dev/null +++ b/test/o56-untrusted-permissions.spec.mjs @@ -0,0 +1,166 @@ +#!/usr/bin/env node +/** + * O56: an untrusted project .moss/config.json must not grant + * permissions.deviceTrust or permissions.trustedDevices. A trusted project + * still cannot set deviceTrust full over an unset or gated user, and cannot + * add trustedDevices the user did not list. + * O54 stays here as anti-regression: untrusted approvalPolicy / safetyMode + * must not loosen the resolved mode. + */ +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-o56-' }); + for (const key of Object.keys(env)) { + if (/^MOSS_(SAFETY|APPROVAL|ASK_FOR|TRUST|AUTO_APPROVE|CLI_|DEVICE)/.test(key)) { + delete env[key]; + } + } + 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('O54 approvalPolicy=never from untrusted project is ignored', () => { + const { resolved } = setup({ approvalPolicy: 'never' }); + assert.notEqual( + resolved.approvalPolicySource, + 'project', + `approvalPolicy=${resolved.approvalPolicy} source=${resolved.approvalPolicySource}` + ); +}); + +check('O54 safetyMode=full-access from untrusted project is ignored', () => { + const { resolved } = setup({ safetyMode: 'full-access' }); + assert.notEqual( + resolved.safetyMode, + 'full-access', + `safetyMode=${resolved.safetyMode} source=${resolved.safetyModeSource}` + ); +}); + +check('O56 permissions.deviceTrust=full from untrusted project is ignored', () => { + const { loaded, resolved } = setup({ permissions: { deviceTrust: 'full' } }); + assert.equal(resolved.permissions.deviceTrust, 'gated'); + assert.notEqual(loaded.config.permissions?.deviceTrust, 'full'); + assert.deepEqual(loaded.droppedProjectPermissions, ['permissions.deviceTrust']); +}); + +check('O56 permissions.trustedDevices from untrusted project is ignored', () => { + const { loaded, resolved } = setup({ permissions: { trustedDevices: ['rdk-x5'] } }); + assert.deepEqual(resolved.permissions.trustedDevices, []); + assert.deepEqual(loaded.config.permissions?.trustedDevices ?? [], []); + assert.deepEqual(loaded.droppedProjectPermissions, ['permissions.trustedDevices']); +}); + +check('O56 untrusted project device trust is recorded and user full still stands', () => { + const { loaded, resolved } = setup( + { permissions: { deviceTrust: 'full', trustedDevices: ['rdk-x5'] } }, + { user: { permissions: { deviceTrust: 'full', trustedDevices: ['board-a'] } } } + ); + assert.equal(resolved.permissions.deviceTrust, 'full'); + assert.deepEqual(resolved.permissions.trustedDevices, ['board-a']); + assert.deepEqual(loaded.droppedProjectPermissions, [ + 'permissions.deviceTrust', + 'permissions.trustedDevices', + ]); +}); + +check('trusted folder: user unset + project deviceTrust full is not full', () => { + const { loaded, resolved } = setup( + { permissions: { deviceTrust: 'full', trustedDevices: ['rdk-x5'] } }, + { trust: true } + ); + assert.equal(resolved.permissions.deviceTrust, 'gated'); + assert.notEqual(loaded.config.permissions?.deviceTrust, 'full'); + assert.deepEqual(resolved.permissions.trustedDevices, []); + assert.equal(loaded.droppedProjectPermissions, undefined); +}); + +check('trusted folder: user gated + project deviceTrust full is not full', () => { + const { resolved } = setup( + { permissions: { deviceTrust: 'full', trustedDevices: ['rdk-x5'] } }, + { trust: true, user: { permissions: { deviceTrust: 'gated', trustedDevices: ['board-a'] } } } + ); + assert.equal(resolved.permissions.deviceTrust, 'gated'); + assert.deepEqual(resolved.permissions.trustedDevices, []); + assert.ok(!resolved.permissions.trustedDevices.includes('rdk-x5')); +}); + +check('trusted folder: project trustedDevices outside the user list are not trusted', () => { + const { loaded, resolved } = setup( + { permissions: { trustedDevices: ['rdk-x5', 'evil'] } }, + { trust: true, user: { permissions: { trustedDevices: ['board-a', 'rdk-x5'] } } } + ); + assert.deepEqual(resolved.permissions.trustedDevices, ['rdk-x5']); + assert.deepEqual(loaded.config.permissions.trustedDevices, ['rdk-x5']); + assert.ok(!resolved.permissions.trustedDevices.includes('evil')); +}); + +check('trusted folder: project deviceTrust gated tightens user full', () => { + const { resolved } = setup( + { permissions: { deviceTrust: 'gated' } }, + { trust: true, user: { permissions: { deviceTrust: 'full' } } } + ); + assert.equal(resolved.permissions.deviceTrust, 'gated'); +}); + +check('allowlist unions deny/ask, drops unknown project fields, and does not widen allow', () => { + const untrusted = setup( + { + permissions: { + allow: ['exec'], + ask: ['write_file'], + deny: ['exec'], + defaultMode: 'full', + notAPermission: ['device_exec'], + }, + }, + { user: { permissions: { defaultMode: 'manual', deny: ['read_file'], allow: ['read_file'] } } } + ); + assert.equal(untrusted.resolved.permissions.defaultMode, 'manual'); + assert.deepEqual(untrusted.resolved.permissions.allow, ['read_file']); + assert.deepEqual(untrusted.resolved.permissions.ask, ['write_file']); + assert.deepEqual(untrusted.resolved.permissions.deny, ['read_file', 'exec']); + assert.equal('notAPermission' in (untrusted.loaded.config.permissions ?? {}), false); + assert.ok(untrusted.loaded.droppedProjectPermissions.includes('permissions.allow')); + assert.ok(untrusted.loaded.droppedProjectPermissions.includes('permissions.defaultMode')); + + const trusted = setup( + { permissions: { allow: ['exec'], defaultMode: 'full', notAPermission: ['device_exec'] } }, + { trust: true, user: { permissions: { defaultMode: 'manual', allow: ['read_file'] } } } + ); + assert.equal(trusted.resolved.permissions.defaultMode, 'manual'); + assert.deepEqual(trusted.resolved.permissions.allow, ['read_file', 'exec']); + assert.equal('notAPermission' in (trusted.loaded.config.permissions ?? {}), false); +}); + +if (failures.length > 0) { + console.log(`${failures.length} failing`); + process.exit(1); +} +console.log('[PASS] o56-untrusted-permissions');