From 1d07df0d61392b0ce4d258d7a2e04409da0093c6 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Tue, 10 Nov 2026 16:56:18 -0600 Subject: [PATCH 1/2] feat(organization): org definitions, template hash and diff reads, definition verbs (#1873) Roles round 2, slice T1 (roles-and-rails design sections 4, 5 and 13). - The org file's definitions key: each entry {rev, from?, role, instructions, personality?} passes checkComposition; a bad entry refuses the whole file, naming the definition and key, never a value. - org: instance references resolve to the file's definition; a missing one refuses definition-missing, naming the instance. Template names are unchanged. - Template identity is the name plus a sha256 over the template JSON and its contract; templateHashes reads the current hash for each template. - The writer's setDefinition is the only path that changes definitions: it checks the composition, bumps rev, refuses agent actors, and refuses definition-in-use for a definition an instance names (A4 adds the change decision). write() refuses organization-definitions-protected. - Definition verbs import, edit, invent and remove need org.hire (definition.*.console in VERB_RIGHTS) and refuse every agent caller. - The diff read lists the primitives that changed and a line diff of the instructions. It applies nothing. - CLI: mosaic org definitions templates|list|show|diff|import|invent|edit|remove. --- packages/business/src/definition-verbs.mjs | 2 + packages/business/src/definitions.mjs | 2 + packages/cli/src/launcher.mjs | 4 +- packages/cli/src/org-definitions.mjs | 64 ++++ packages/cli/src/org.mjs | 5 +- packages/cli/tests/org-definitions.test.mjs | 85 +++++ .../organization/src/definition-verbs.mjs | 122 +++++++ packages/organization/src/definitions.mjs | 268 ++++++++++++++ packages/organization/src/org-rights.mjs | 5 + .../organization/src/organization-config.mjs | 22 +- packages/organization/src/organization.mjs | 57 ++- packages/organization/src/resolve.mjs | 3 + .../organization/tests/definitions.test.mjs | 336 ++++++++++++++++++ .../organization/tests/org-rights.test.mjs | 6 + 14 files changed, 970 insertions(+), 11 deletions(-) create mode 100644 packages/business/src/definition-verbs.mjs create mode 100644 packages/business/src/definitions.mjs create mode 100644 packages/cli/src/org-definitions.mjs create mode 100644 packages/cli/tests/org-definitions.test.mjs create mode 100644 packages/organization/src/definition-verbs.mjs create mode 100644 packages/organization/src/definitions.mjs create mode 100644 packages/organization/tests/definitions.test.mjs diff --git a/packages/business/src/definition-verbs.mjs b/packages/business/src/definition-verbs.mjs new file mode 100644 index 000000000..c82eedf07 --- /dev/null +++ b/packages/business/src/definition-verbs.mjs @@ -0,0 +1,2 @@ +// Old path of packages/organization/src/definition-verbs.mjs (rename design r2, stage B). +export * from "../../organization/src/definition-verbs.mjs"; diff --git a/packages/business/src/definitions.mjs b/packages/business/src/definitions.mjs new file mode 100644 index 000000000..72bde3172 --- /dev/null +++ b/packages/business/src/definitions.mjs @@ -0,0 +1,2 @@ +// Old path of packages/organization/src/definitions.mjs (rename design r2, stage B). +export * from "../../organization/src/definitions.mjs"; diff --git a/packages/cli/src/launcher.mjs b/packages/cli/src/launcher.mjs index e4968e2fe..7b82d65ee 100644 --- a/packages/cli/src/launcher.mjs +++ b/packages/cli/src/launcher.mjs @@ -412,7 +412,7 @@ export function createLauncher({ }; const primary = routeOf("primary"); const fallback = hasFallback(resolved) ? routeOf("fallback") : null; - if (!resolved.contract) throw new Refusal("no-role-contract", `role ${resolved.definition} has no contract file`); + if (!resolved.contract && typeof resolved.instructions !== "string") throw new Refusal("no-role-contract", `role ${resolved.definition} has no contract file`); // Row 64 (#1557): every skill on the role's list is enabled and readable before a run exists. try { readSkills(resolved.skills, skillsDir); @@ -723,7 +723,7 @@ export function createLauncher({ models: b.launch.models, proxy: sessionProxy, resolver, - contract: readFileSync(resolved.contract, "utf8"), + contract: resolved.contract ? readFileSync(resolved.contract, "utf8") : resolved.instructions, arbiter: Object.values(b.arbiters).includes(instance), tracker, library: Object.values(b.projects ?? {}).some(project => project.kind === "library"), diff --git a/packages/cli/src/org-definitions.mjs b/packages/cli/src/org-definitions.mjs new file mode 100644 index 000000000..50024d35f --- /dev/null +++ b/packages/cli/src/org-definitions.mjs @@ -0,0 +1,64 @@ +// #1873 T1: `mosaic org definitions`, the human verbs on an organization's own role definitions and +// the template hash and diff reads. A write runs only from a human shell (refuseInsideAgent) and acts +// as the organization's human holding the owner's rights: the OS user who owns the config directory +// owns the organization, the same trust `mosaic org update` rests on. Each write prints its receipt. +import { readFile } from 'node:fs/promises'; +import { configDir } from '../../organization/src/organization-config.mjs'; +import { organizationWriter } from '../../organization/src/organization.mjs'; +import { definitionVerbs } from '../../organization/src/definition-verbs.mjs'; +import { ROLE_RIGHTS } from '../../organization/src/org-rights.mjs'; +import { rolesDir } from './config.mjs'; +import { CliError } from './errors.mjs'; +import { refuseInsideAgent } from './transport.mjs'; + +export const DEFINITIONS_USAGE = 'mosaic org definitions templates | definitions ID list | definitions ID show NAME | definitions ID diff NAME | definitions ID import TEMPLATE [--as NAME] | definitions ID invent NAME --from FILE | definitions ID edit NAME --from FILE --if-rev N | definitions ID remove NAME --if-rev N'; + +const WRITES = ['import', 'invent', 'edit', 'remove']; +const VALUES = ['--as', '--from', '--if-rev']; + +function parse(args) { + const pos = []; + const values = {}; + for (let i = 0; i < args.length; i++) { + const a = args[i]; + if (VALUES.includes(a)) { + if (values[a] !== undefined || !args[i + 1] || args[i + 1].startsWith('--')) throw new CliError(`${a} needs one value`, 4); + values[a] = args[++i]; + } else if (a.startsWith('--')) throw new CliError(`unknown option ${a}`, 4); + else pos.push(a); + } + return { pos, values }; +} + +async function definitionFile(file) { + try { + return JSON.parse(await readFile(file, 'utf8')); + } catch (e) { + // Never echo the file's text: it may hold what the composition check refuses. + throw new CliError(`cannot read definition ${file}: ${e instanceof SyntaxError ? 'not valid JSON' : e.code ?? 'unreadable'}`, 2); + } +} + +export async function definitionsCommand(args, io, deps = {}) { + const { pos, values } = parse(args); + const allowed = (...keys) => { for (const k of Object.keys(values)) if (!keys.includes(k)) throw new CliError(`unexpected ${k}`, 4); }; + const need = (key) => { if (!values[key]) throw new CliError(`${key} is required`, 4); return values[key]; }; + const ifRev = () => { const r = need('--if-rev'); if (!/^[1-9][0-9]*$/.test(r)) throw new CliError('invalid --if-rev', 4); return Number(r); }; + const [id, verb, name] = pos; + if (WRITES.includes(verb)) refuseInsideAgent(io.env); + const writer = deps.writer ?? organizationWriter({ dir: configDir(io.env), rolesDir: rolesDir(io.env) }); + const verbs = definitionVerbs({ writer, rolesDir: rolesDir(io.env), assertClean: deps.assertClean }); + let result; + if (id === 'templates' && pos.length === 1) { allowed(); result = verbs.templates(); } + else if (verb === 'list' && pos.length === 2) { allowed(); result = verbs.list(id); } + else if (['show', 'diff'].includes(verb) && pos.length === 3) { allowed(); result = verbs[verb](id, name); } + else if (WRITES.includes(verb) && pos.length === 3) { + const { document, revision } = writer.read(id); + const caller = { actor: document.human, rights: [...ROLE_RIGHTS.owner], agent: false }; + if (verb === 'import') { allowed('--as'); result = verbs.import(id, { revision, template: name, as: values['--as'] ?? name }, caller); } + else if (verb === 'invent') { allowed('--from'); result = verbs.invent(id, { revision, name, definition: await definitionFile(need('--from')) }, caller); } + else if (verb === 'edit') { allowed('--from', '--if-rev'); const rev = ifRev(); result = verbs.edit(id, { revision, rev, name, definition: await definitionFile(need('--from')) }, caller); } + else { allowed('--if-rev'); result = verbs.remove(id, { revision, rev: ifRev(), name }, caller); } + } else throw new CliError(DEFINITIONS_USAGE, 4); + io.stdout.write(JSON.stringify(result, null, 2) + '\n'); +} diff --git a/packages/cli/src/org.mjs b/packages/cli/src/org.mjs index 22503dff9..2a05dd892 100644 --- a/packages/cli/src/org.mjs +++ b/packages/cli/src/org.mjs @@ -9,13 +9,16 @@ import { CliError } from './errors.mjs'; import { busStopForDelete } from './org-bus.mjs'; import { consoleBusRestart } from './org-console.mjs'; import { refuseInsideAgent } from './transport.mjs'; -export const ORG_USAGE = 'mosaic org system-types installed | system-types enabled ID | lock show|clear ID | reload ID | archive ID --if-revision N [--json] | restore ID --if-revision N [--json] | delete ID --confirm ID [--if-revision N] [--json] | template render ID [--fixture local] --output FILE | validate FILE | create ID --from FILE | show ID [--json] | list [--json] | update ID --from FILE --if-revision N | queue init ID (--local-only --accept-local-loss-risk | --remote URL) | queue show ID [--json] | project repo init ID PROJECT (--local-only --accept-local-loss-risk | --remote URL) | project repo show ID PROJECT [--json] | config validate ID | config resolve ID INSTANCE [--project ID]'; +import { definitionsCommand, DEFINITIONS_USAGE } from './org-definitions.mjs'; +export const ORG_USAGE = 'mosaic org system-types installed | system-types enabled ID | lock show|clear ID | reload ID | archive ID --if-revision N [--json] | restore ID --if-revision N [--json] | delete ID --confirm ID [--if-revision N] [--json] | template render ID [--fixture local] --output FILE | validate FILE | create ID --from FILE | show ID [--json] | list [--json] | update ID --from FILE --if-revision N | queue init ID (--local-only --accept-local-loss-risk | --remote URL) | queue show ID [--json] | project repo init ID PROJECT (--local-only --accept-local-loss-risk | --remote URL) | project repo show ID PROJECT [--json] | ' + DEFINITIONS_USAGE.slice('mosaic org '.length) + ' | config validate ID | config resolve ID INSTANCE [--project ID]'; function parse(args) { const pos = [], values = {}, flags = new Set(); for (let i = 0; i < args.length; i++) { const a = args[i]; if (['--json', '--local-only', '--accept-local-loss-risk'].includes(a)) { if (flags.has(a)) throw new CliError(`duplicate ${a}`, 4); flags.add(a); } else if (['--fixture', '--output', '--from', '--if-revision', '--remote', '--confirm'].includes(a)) { if (values[a] !== undefined || !args[i+1] || args[i+1].startsWith('--')) throw new CliError(`${a} needs one value`, 4); values[a] = args[++i]; } else if (a.startsWith('--')) throw new CliError(`unknown option ${a}`, 4); else pos.push(a); } return { pos, values, flags }; } // A refusal names the document by label and the error by its code, never by host path or bytes (PR #1916). const errorCode = e => typeof e?.code === 'string' && /^E[A-Z0-9]+$/.test(e.code) ? e.code : 'unreadable'; async function document(file) { try { return JSON.parse(await readFile(file, 'utf8')); } catch (e) { throw new CliError(`cannot read organization document: ${e instanceof SyntaxError ? 'not valid JSON' : errorCode(e)}`, 2); } } export async function orgCommand(args, io, deps = {}) { try { + // #1873 T1: the definitions verbs take their own options. + if (args[0] === 'definitions') return await definitionsCommand(args.slice(1), io, deps); const p = parse(args); const words = p.pos; let result; let accepted = []; const allowed = (flags, values) => { for (const f of p.flags) if (!flags.includes(f)) throw new CliError(`unexpected ${f}`, 4); for (const f of Object.keys(p.values)) if (!values.includes(f)) throw new CliError(`unexpected ${f}`, 4); }; const need = key => { if (!p.values[key]) throw new CliError(`${key} is required`, 4); return p.values[key]; }; diff --git a/packages/cli/tests/org-definitions.test.mjs b/packages/cli/tests/org-definitions.test.mjs new file mode 100644 index 000000000..627fd0c16 --- /dev/null +++ b/packages/cli/tests/org-definitions.test.mjs @@ -0,0 +1,85 @@ +// #1873 T1: org: definitions at launch, and `mosaic org definitions`. +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { join } from 'node:path'; +import { readFileSync, writeFileSync } from 'node:fs'; +import { createLauncher } from '../src/launcher.mjs'; +import { main } from '../src/cli.mjs'; +import { initializeControl } from '../../processing-control/src/state.mjs'; +import { launchLogFile } from '../../seat/src/session.mjs'; +import { entryFromTemplate, readTemplate, templateHash } from '../../organization/src/definitions.mjs'; +import { REPO_ROLES, writeJson } from '../../organization/tests/helpers.mjs'; +import { fixture, io, tmp, withoutHostCredentials } from './helpers.mjs'; + +const SCOUT = { rev: 1, role: { tools: ['read'], network: 'none', authority: { withinRole: [], crossRole: [] }, credentials: [] }, instructions: 'Read and report.\n' }; +const stop = () => Object.assign(Error('fixture authorization stop'), { code: 'fixture-policy' }); +const host = { business: 'acme', beforeClose: () => {}, op: async (x) => { + if (x.op === 'identity') return { business: 'acme', role: 'pm' }; + if (x.op === 'mayLaunch') return { allow: true }; + if (x.op === 'authorizeLaunch') throw stop(); + return {}; +} }; + +test('T1 an org: instance passes every launch check, and a missing definition refuses naming instance and definition', async (t) => { + const root = tmp(t); + // The org's own copy of the coder template, so the instance's credentials and tracker still match. + const builder = entryFromTemplate(readTemplate(REPO_ROLES, 'coder')); + const f = fixture(root, 'acme', (doc) => ({ ...doc, definitions: { builder }, roles: { ...doc.roles, coder: { ...doc.roles.coder, definition: 'org:builder' } } })); + const file = join(root, 'config/businesses/acme.json'); + withoutHostCredentials(f.env, { piKey: true }); + initializeControl(f.dataRoot); + const launcher = createLauncher({ host, system: JSON.parse(readFileSync(f.config, 'utf8')), env: f.env, bwrap: '/usr/bin/true', log: () => {} }); + // Resolve, route, contract (the definition's instructions) and skills all pass; the fixture stops at authorization. + assert.equal((await launcher.fromSession('fixture-cap', 'coder')).error, 'fixture-policy'); + + // The chief names a definition the file doesn't declare: the whole file refuses, so a session's + // launch and the chief's launch both refuse. + writeJson(file, { ...f.doc, roles: { ...f.doc.roles, chief: { ...f.doc.roles.chief, definition: 'org:ghost' } } }); + assert.deepEqual(await launcher.fromSession('fixture-cap', 'coder'), { ok: false, error: 'organization-refused' }); + await assert.rejects(launcher.launchPm(), (e) => e.code === 'organization-refused' && /roles\.chief\.definition names org:ghost/.test(e.message)); + const event = JSON.parse(readFileSync(launchLogFile(f.dataRoot, 'acme'), 'utf8').trim().split('\n').at(-1)); + assert.equal(event.code, 'organization-refused'); + assert.equal(event.instance, 'chief'); + assert.match(event.why, /definition-missing: .*roles\.chief\.definition names org:ghost/); + await launcher.close?.(); + + // A host does not start on a file that names a missing definition. + assert.throws(() => createLauncher({ host, system: JSON.parse(readFileSync(f.config, 'utf8')), env: f.env, bwrap: '/usr/bin/true', log: () => {} }), /org:ghost/); +}); + +test('T1 mosaic org definitions: each write prints its receipt, and an agent shell refuses', async (t) => { + const root = tmp(t); + const f = fixture(root); + const file = join(root, 'config/businesses/acme.json'); + const run = async (args, env = f.env) => { const x = io(env); await main(['org', 'definitions', ...args], x, {}); return JSON.parse(x.out.text); }; + const hash = templateHash(f.roles, 'coder'); + assert.ok((await run(['templates'])).some((h) => h.template === 'coder' && h.sha256 === hash)); + + const imported = await run(['acme', 'import', 'coder', '--as', 'builder']); + assert.deepEqual({ ...imported, revision: 0, transaction: 0 }, { verb: 'import', organization: 'acme', definition: 'builder', rev: 1, revision: 0, transaction: 0, from: { template: 'coder', sha256: hash } }); + const spec = join(root, 'scout.json'); + writeFileSync(spec, JSON.stringify({ role: SCOUT.role, instructions: SCOUT.instructions })); + assert.equal((await run(['acme', 'invent', 'scout', '--from', spec])).rev, 1); + writeFileSync(spec, JSON.stringify({ role: SCOUT.role, instructions: 'Read, then report.\n' })); + const edited = await run(['acme', 'edit', 'scout', '--from', spec, '--if-rev', '1']); + assert.equal(edited.verb, 'edit'); + assert.equal(edited.rev, 2); + assert.deepEqual((await run(['acme', 'list'])).definitions.map((d) => [d.name, d.rev]), [['builder', 1], ['scout', 2]]); + assert.equal((await run(['acme', 'show', 'scout'])).instructions, 'Read, then report.\n'); + assert.equal((await run(['acme', 'diff', 'builder'])).changed, false); + assert.equal((await run(['acme', 'remove', 'scout', '--if-rev', '2'])).removedRev, 2); + + // A held-secret-shaped file never echoes; a stale --if-rev refuses. + writeFileSync(spec, '{"role": sk-test-NOT-A-SECRET-1234'); + await assert.rejects(run(['acme', 'invent', 'other', '--from', spec]), (e) => e.exitCode === 2 && !e.message.includes('sk-test')); + await assert.rejects(run(['acme', 'remove', 'builder', '--if-rev', '7']), (e) => /^definition-rev-conflict: definition builder is not at rev 7/.test(e.message)); + + // Inside an agent run every write refuses before it reads anything; reads still work. + const before = readFileSync(file); + const agent = { ...f.env, MOSAIC_RUN_ID: 'run-1' }; + for (const args of [['acme', 'import', 'reviewer'], ['acme', 'invent', 'x', '--from', spec], ['acme', 'edit', 'builder', '--from', spec, '--if-rev', '1'], ['acme', 'remove', 'builder', '--if-rev', '1']]) { + await assert.rejects(run(args, agent), (e) => e.exitCode === 3 && /human shell/.test(e.message)); + } + assert.deepEqual(readFileSync(file), before); + assert.equal((await run(['acme', 'list'], agent)).definitions.length, 1); +}); diff --git a/packages/organization/src/definition-verbs.mjs b/packages/organization/src/definition-verbs.mjs new file mode 100644 index 000000000..00b85d411 --- /dev/null +++ b/packages/organization/src/definition-verbs.mjs @@ -0,0 +1,122 @@ +// Roles round 2, slice T1 (#1873): the human verbs on an organization's definitions, and the reads. +// Design: docs/plans/2026-11-10_roles-and-rails-design.md section 4. +// +// Each write needs the caller's org.hire right (VERB_RIGHTS) and refuses every agent caller, then +// goes through the writer's setDefinition: only that definition changes, rev counts up, the entry +// passes checkComposition, and a definition an instance names refuses definition-in-use. +// +// caller: { actor, rights, agent } — actor is the authenticated account the writer logs; rights is +// the caller's resolved right list (an owner holds ROLE_RIGHTS.owner); agent is true for any agent +// session, which never writes a definition. The receipt names the verb, organization, definition, +// rev, revision and transaction, never role text. + +import { VERB_RIGHTS } from "./org-rights.mjs"; +import { OrganizationError } from "./organization.mjs"; +import { + diffDefinition, entryFromTemplate, instancesNaming, readTemplate, templateHashes, checkDefinitionName, ORG_PREFIX, +} from "./definitions.mjs"; + +// The verb names the policy table maps, following R1's hire.resolve.console. +export const DEFINITION_VERBS = Object.freeze({ + import: "definition.import.console", + edit: "definition.edit.console", + invent: "definition.invent.console", + remove: "definition.remove.console", +}); + +const fail = (code, message = code) => { + throw new OrganizationError(code, message); +}; + +function authorize(caller, verb) { + if (caller === null || typeof caller !== "object") fail("caller-required"); + if (caller.agent !== false) fail("human-required", "an agent never writes a definition"); + const right = VERB_RIGHTS[DEFINITION_VERBS[verb]]; + if (typeof right !== "string") fail("verb-unmapped"); + if (!Array.isArray(caller.rights) || !caller.rights.includes(right)) fail("right-required", `${verb} needs ${right}`); +} + +// The parts of an entry a caller may supply for invent and edit: role, instructions, personality. +const SUPPLIED = ["role", "instructions", "personality"]; +function supplied(input) { + if (input === null || typeof input !== "object" || Array.isArray(input)) fail("definition-invalid", "the definition must be a JSON object"); + const bad = Object.keys(input).findIndex((key) => !SUPPLIED.includes(key)); + if (bad !== -1) fail("definition-invalid", `the definition has an unknown key (key ${bad + 1}; allowed: ${SUPPLIED.join(", ")})`); + const out = { role: input.role, instructions: input.instructions }; + if (input.personality !== undefined) out.personality = input.personality; + return out; +} + +function receipt(verb, id, name, result) { + return { verb, organization: id, definition: name, rev: result.rev, revision: result.revision, transaction: result.transaction }; +} + +export function definitionVerbs({ writer, rolesDir, assertClean }) { + const options = (verb, caller, extra) => ({ ...extra, actor: caller.actor, assertClean, detail: { verb: DEFINITION_VERBS[verb] } }); + + // Copies a template into definitions with from set to its name and current hash. + function importTemplate(id, { revision, template, as = template }, caller) { + authorize(caller, "import"); + checkDefinitionName(as, "definition name"); + const source = readTemplate(rolesDir, template); + const result = writer.setDefinition(id, as, (current) => { + if (current) fail("definition-exists", `definitions already declares ${as}`); + const { rev: _, ...entry } = entryFromTemplate(source); + return entry; + }, options("import", caller, { revision })); + return { ...receipt("import", id, as, result), from: { template, sha256: source.sha256 } }; + } + + // Changes an existing definition: the caller supplies the whole role, instructions and + // personality; from stays as it was. + function edit(id, { revision, rev, name, definition }, caller) { + authorize(caller, "edit"); + if (!Number.isSafeInteger(rev)) fail("definition-rev-required", "edit needs the definition's current rev"); + const entry = supplied(definition); + const result = writer.setDefinition(id, name, (current) => { + if (!current) fail("definition-missing", `definitions declares no ${name}`); + return current.from ? { from: current.from, ...entry } : entry; + }, options("edit", caller, { revision, rev })); + return receipt("edit", id, name, result); + } + + // Writes a new definition with no from. + function invent(id, { revision, name, definition }, caller) { + authorize(caller, "invent"); + const entry = supplied(definition); + const result = writer.setDefinition(id, name, (current) => { + if (current) fail("definition-exists", `definitions already declares ${name}`); + return entry; + }, options("invent", caller, { revision })); + return receipt("invent", id, name, result); + } + + function remove(id, { revision, rev, name }, caller) { + authorize(caller, "remove"); + if (!Number.isSafeInteger(rev)) fail("definition-rev-required", "remove needs the definition's current rev"); + const result = writer.setDefinition(id, name, () => null, options("remove", caller, { revision, rev })); + return { ...receipt("remove", id, name, result), rev: null, removedRev: rev }; + } + + // Reads. Each reads the validated document. + function list(id) { + const { document, revision } = writer.read(id); + const definitions = Object.entries(document.definitions ?? {}).map(([name, entry]) => ({ + name, reference: ORG_PREFIX + name, rev: entry.rev, from: entry.from ?? null, usedBy: instancesNaming(document, name), + })); + return { organization: id, revision, definitions }; + } + + function show(id, name) { + const { document, revision } = writer.read(id); + if (!Object.hasOwn(document.definitions ?? {}, name)) fail("definition-missing", `definitions declares no ${name}`); + return { organization: id, revision, name, reference: ORG_PREFIX + name, usedBy: instancesNaming(document, name), ...structuredClone(document.definitions[name]) }; + } + + function diff(id, name) { + const { document, revision } = writer.read(id); + return { organization: id, revision, ...diffDefinition(document, name, rolesDir) }; + } + + return { import: importTemplate, edit, invent, remove, list, show, diff, templates: () => templateHashes(rolesDir) }; +} diff --git a/packages/organization/src/definitions.mjs b/packages/organization/src/definitions.mjs new file mode 100644 index 000000000..c5b68bdb1 --- /dev/null +++ b/packages/organization/src/definitions.mjs @@ -0,0 +1,268 @@ +// Roles round 2, slice T1 (#1873): an organization's own role definitions and the templates they +// copy. Design: docs/plans/2026-11-10_roles-and-rails-design.md sections 4 and 5, and the T1 +// paragraph of section 13. +// +// The organization file's `definitions` key holds one entry per name: +// { rev, from?: { template, sha256 }, role: { tools, network, authority, credentials, +// messaging?, skills? }, instructions, personality? } +// An instance names one as `definition: "org:"`. Every entry passes checkComposition (P1) +// whenever the file is validated, so no path writes or loads a definition that skips it. A refusal +// names the definition and the key at fault, never a value: composition.mjs quotes only registered +// names, and this module quotes only definition and template names, which match ID_PATTERN. +// +// A template is a shipped roles/.json (role version 2) with its contract. Its identity is the +// name plus templateHash: sha256 over the JSON bytes and the contract bytes together, each preceded +// by its byte length and a newline, so no field goes into roles/. +// +// Pure reads only. The writer verbs live in definition-verbs.mjs, which organization.mjs's writer +// backs; this module is imported by organization-config.mjs and must not import the writer. + +import { readFileSync, readdirSync } from "node:fs"; +import { join } from "node:path"; +import { createHash } from "node:crypto"; +import { refuse } from "./errors.mjs"; +import { buildRegistry, checkComposition } from "./composition.mjs"; +import { validateRoleDocument } from "./role.mjs"; +import { ID_PATTERN } from "./vocabulary.mjs"; +import { canonicalJson, deepFreeze } from "./util.mjs"; + +export const ORG_PREFIX = "org:"; +export const ENTRY_KEYS = Object.freeze(["rev", "from", "role", "instructions", "personality"]); +// The primitives an entry keeps under `role`; instructions and personality sit beside it. +export const ROLE_KEYS = Object.freeze(["tools", "network", "authority", "credentials", "messaging", "skills"]); +// What the diff read compares, in section 4's order. +export const PRIMITIVES = Object.freeze([...ROLE_KEYS, "instructions", "personality"]); +const FROM_KEYS = Object.freeze(["template", "sha256"]); +const HEX64 = /^[0-9a-f]{64}$/; + +// The core registry. Plugin-declared primitives arrive with PL1; until then an org definition uses +// core names only. +const CORE = buildRegistry(); +// Validation keeps no flagged lines: A4 computes them at the decision, against the chief's live +// role. Step 7 only flags, so a chief holding nothing refuses nothing here. +const NO_CHIEF = Object.freeze({ + tools: Object.freeze([]), network: "none", + authority: Object.freeze({ withinRole: Object.freeze([]), crossRole: Object.freeze([]) }), + credentials: Object.freeze([]), messaging: null, +}); +// A caller with no credential store holds no token, so its secret check is the empty store's. +const NO_STORE = () => {}; + +const plain = (value) => value !== null && typeof value === "object" && !Array.isArray(value) + && [Object.prototype, null].includes(Object.getPrototypeOf(value)); + +// The definition name an instance's `definition` value names, or null for a template name. +export function orgDefinitionName(value) { + return typeof value === "string" && value.startsWith(ORG_PREFIX) ? value.slice(ORG_PREFIX.length) : null; +} + +export function checkDefinitionName(name, where) { + if (typeof name !== "string" || !ID_PATTERN.test(name)) refuse(`${where} must be a definition name matching ${ID_PATTERN}`); + return name; +} + +function keysOnly(object, allowed, where) { + Object.keys(object).forEach((key, i) => { + if (!allowed.includes(key)) refuse(`${where} has an unknown key (key ${i + 1}; allowed: ${allowed.join(", ")})`); + }); +} + +// The composition an entry describes, for checkComposition. Absent optional keys stay absent. +export function compositionOf(entry) { + const out = { ...entry.role, instructions: entry.instructions }; + if (entry.personality !== undefined) out.personality = entry.personality; + return out; +} + +// One entry: its own keys, then checkComposition. Returns { composition, flagged }. options.assertClean +// is the credential store's check when the caller has one. +export function checkDefinition(name, entry, { assertClean = NO_STORE, where = "definitions" } = {}) { + const at = `${where}.${checkDefinitionName(name, `${where} key`)}`; + if (!plain(entry)) refuse(`${at} must be a JSON object`); + keysOnly(entry, ENTRY_KEYS, at); + if (!Number.isSafeInteger(entry.rev) || entry.rev < 1) refuse(`${at}.rev must be a positive integer`); + if (entry.from !== undefined) { + if (!plain(entry.from)) refuse(`${at}.from must be a JSON object`); + keysOnly(entry.from, FROM_KEYS, `${at}.from`); + if (typeof entry.from.template !== "string" || !ID_PATTERN.test(entry.from.template)) refuse(`${at}.from.template must be a template name matching ${ID_PATTERN}`); + if (typeof entry.from.sha256 !== "string" || !HEX64.test(entry.from.sha256)) refuse(`${at}.from.sha256 must be 64 lowercase hex characters`); + } + if (!plain(entry.role)) refuse(`${at}.role must be a JSON object`); + keysOnly(entry.role, ROLE_KEYS, `${at}.role`); + try { + return checkComposition(compositionOf(entry), CORE, NO_CHIEF, { assertClean }); + } catch (error) { + // composition.mjs's refusals quote registered names only; keep its text, add where. + if (error?.name === "BusinessError") refuse(`${at}: ${error.message}`); + throw error; + } +} + +// The resolved role an `org:` instance launches with: loadRole's shape, so the business checks, +// the resolver and the broker read it as they read a template. Instructions stand in for the +// contract file (contractPath null). A definition hires nobody: hireable stays empty. +function resolved(name, entry, composition) { + return { + roleVersion: 2, name: ORG_PREFIX + name, title: name, contract: null, contractPath: null, + tools: composition.tools, network: composition.network, authority: composition.authority, + credentials: composition.credentials, messaging: composition.messaging, skills: composition.skills, + hireable: [], instructions: composition.instructions, personality: composition.personality, + rev: entry.rev, from: entry.from === undefined ? null : { ...entry.from }, + }; +} + +// The whole `definitions` key. Returns a frozen map "org:" -> resolved role. A bad entry +// refuses the whole file. +export function checkDefinitions(definitions, file, options = {}) { + if (definitions === undefined) return deepFreeze({}); + const where = `${file} definitions`; + if (!plain(definitions)) refuse(`${where} must be a JSON object`); + const out = {}; + Object.keys(definitions).forEach((name, i) => { + checkDefinitionName(name, `${where} key ${i + 1}`); + const { composition } = checkDefinition(name, definitions[name], { ...options, where }); + out[ORG_PREFIX + name] = resolved(name, definitions[name], composition); + }); + return deepFreeze(out); +} + +// The instances that name `org:`, sorted. +export function instancesNaming(document, name) { + return Object.entries(document?.roles ?? {}) + .filter(([, entry]) => entry?.definition === ORG_PREFIX + name).map(([instance]) => instance).sort(); +} + +// ---- Templates ---- + +function frame(...parts) { + const hash = createHash("sha256"); + for (const part of parts) { + hash.update(`${part.length}\n`); + hash.update(part); + } + return hash.digest("hex"); +} + +// One template, read once: its validated role, its contract text and its hash. Refuses a missing, +// invalid or version 1 template. +export function readTemplate(rolesDir, name) { + if (typeof name !== "string" || !ID_PATTERN.test(name)) refuse(`template name must match ${ID_PATTERN}`); + const file = join(rolesDir, `${name}.json`); + let json; + try { + json = readFileSync(file); + } catch { + refuse(`template ${name} not found`); + } + let document; + try { + document = JSON.parse(json.toString("utf8")); + } catch { + refuse(`template ${name} is not valid JSON`); + } + const role = validateRoleDocument(document, file, { hireable: false }); + if (role.roleVersion !== 2) refuse(`template ${name} is role version 1 and has no contract`); + const contract = readFileSync(role.contractPath); + return { name, role, contract: contract.toString("utf8"), sha256: frame(json, contract) }; +} + +export function templateHash(rolesDir, name) { + return readTemplate(rolesDir, name).sha256; +} + +// The read: every version 2 template in rolesDir with its current hash, sorted by name. A file that +// is not a usable version 2 role (conductor-policy.json, a version 1 role) is not a template. +export function templateHashes(rolesDir) { + const out = []; + for (const entry of readdirSync(rolesDir).filter((f) => f.endsWith(".json")).sort()) { + const name = entry.slice(0, -".json".length); + if (!ID_PATTERN.test(name)) continue; + let template; + try { + template = readTemplate(rolesDir, name); + } catch (error) { + if (error?.name === "BusinessError") continue; + throw error; + } + out.push({ template: name, sha256: template.sha256 }); + } + return out; +} + +// The entry an import writes: the template's primitives and its contract as instructions. +export function entryFromTemplate(template) { + const { role } = template; + const out = { + tools: [...role.tools], network: role.network, + authority: { withinRole: [...role.authority.withinRole], crossRole: [...role.authority.crossRole] }, + credentials: role.credentials.map((c) => (c.scopes === undefined ? { service: c.service } : { service: c.service, scopes: structuredClone(c.scopes) })), + }; + if (role.messaging) out.messaging = { sendTo: [...role.messaging.sendTo], receiveFrom: [...role.messaging.receiveFrom] }; + if (role.skills.length) out.skills = [...role.skills]; + return { rev: 1, from: { template: template.name, sha256: template.sha256 }, role: out, instructions: template.contract }; +} + +// ---- The diff read ---- + +// A line diff of two texts: [{ op: " " | "-" | "+", line }]. Longest common subsequence over lines; +// past MAX_CELLS it reports the old text removed and the new one added, which is still exact. +const MAX_CELLS = 4_000_000; +export function lineDiff(before, after) { + const a = before.split("\n"); + const b = after.split("\n"); + if (a.length * b.length > MAX_CELLS) { + return [...a.map((line) => ({ op: "-", line })), ...b.map((line) => ({ op: "+", line }))]; + } + const width = b.length + 1; + const table = new Uint32Array((a.length + 1) * width); + for (let i = a.length - 1; i >= 0; i--) { + for (let j = b.length - 1; j >= 0; j--) { + table[i * width + j] = a[i] === b[j] ? table[(i + 1) * width + j + 1] + 1 + : Math.max(table[(i + 1) * width + j], table[i * width + j + 1]); + } + } + const out = []; + let i = 0; + let j = 0; + while (i < a.length && j < b.length) { + if (a[i] === b[j]) { + out.push({ op: " ", line: a[i] }); + i++; + j++; + } else if (table[(i + 1) * width + j] >= table[i * width + j + 1]) { + out.push({ op: "-", line: a[i++] }); + } else { + out.push({ op: "+", line: b[j++] }); + } + } + while (i < a.length) out.push({ op: "-", line: a[i++] }); + while (j < b.length) out.push({ op: "+", line: b[j++] }); + return out; +} + +// For a copy whose from.sha256 differs from its template's current hash: which primitives differ +// between the copy as it stands and the template as it stands, and the instructions' line diff. +// The file keeps only the hash of what was imported, so a customized copy's own edits show here +// too. Nothing is applied: apply or keep is a change decision (A4). +export function diffDefinition(document, name, rolesDir) { + const definitions = document?.definitions ?? {}; + if (!Object.hasOwn(definitions, name)) refuse(`definitions declares no ${name}`); + const entry = definitions[name]; + const base = { definition: name, rev: entry.rev, from: entry.from ?? null }; + if (!entry.from) return { ...base, current: null, changed: false, primitives: [], instructions: [] }; + let template; + try { + template = readTemplate(rolesDir, entry.from.template); + } catch (error) { + if (error?.name !== "BusinessError") throw error; + return { ...base, current: null, changed: true, templateMissing: true, primitives: [], instructions: [] }; + } + if (template.sha256 === entry.from.sha256) return { ...base, current: template.sha256, changed: false, primitives: [], instructions: [] }; + const now = compositionOf(entryFromTemplate(template)); + const copy = compositionOf(entry); + const primitives = PRIMITIVES.filter((key) => canonicalJson(copy[key] ?? null) !== canonicalJson(now[key] ?? null)); + return { + ...base, current: template.sha256, changed: true, primitives, + instructions: primitives.includes("instructions") ? lineDiff(copy.instructions, now.instructions) : [], + }; +} diff --git a/packages/organization/src/org-rights.mjs b/packages/organization/src/org-rights.mjs index c1065d868..17009c681 100644 --- a/packages/organization/src/org-rights.mjs +++ b/packages/organization/src/org-rights.mjs @@ -67,6 +67,11 @@ export const VERB_RIGHTS = record({ 'decision.preview.console': 'decide', 'decision.resolve.console': 'decide', 'hire.resolve.console': 'org.hire', + // #1873 T1: the human verbs on an organization's own definitions. + 'definition.import.console': 'org.hire', + 'definition.edit.console': 'org.hire', + 'definition.invent.console': 'org.hire', + 'definition.remove.console': 'org.hire', 'request.answer.console': 'request.write', 'request.attach.console': 'request.write', 'request.cancel.console': 'request.write', diff --git a/packages/organization/src/organization-config.mjs b/packages/organization/src/organization-config.mjs index 1fe6b9620..60c789493 100644 --- a/packages/organization/src/organization-config.mjs +++ b/packages/organization/src/organization-config.mjs @@ -10,6 +10,7 @@ import { checkModels, checkLimits, checkAccount } from "./launch.mjs"; import { checkVars } from "./vars.mjs"; import { parseCredentialRef } from "./credentials.mjs"; import { loadRole } from "./role.mjs"; +import { checkDefinitions, checkDefinitionName, orgDefinitionName, ORG_PREFIX } from "./definitions.mjs"; import { END_PROTECTED } from "./vocabulary.mjs"; import { checkPlugins, checkUsageSettings } from "../../plugins/src/kinds.mjs"; import { hasModule } from "../../plugins/src/loader.mjs"; @@ -26,7 +27,7 @@ const BOT_NAME = /^bot-[a-z0-9][a-z0-9._-]{0,60}$/; // How long a thread's session stays up with no turn before the launcher stops // it, when launch.threadIdleSeconds is absent (REQ-THREAD-2, row 121). export const THREAD_IDLE_SECONDS = 900; -const TOP_KEYS = ["businessVersion", "id", "human", "arbiters", "projects", "vars", "tracker", "roles", "launch", "plugins", "hosts", "retired", "replyLimits", "usage", "appearance"]; +const TOP_KEYS = ["businessVersion", "id", "human", "arbiters", "projects", "vars", "tracker", "roles", "launch", "plugins", "hosts", "retired", "replyLimits", "usage", "appearance", "definitions"]; // The config directory: next to the system config, $MOSAIC_CONFIG or // ~/.config/mosaic-dev/config.json, the same rule mosaic-config.mjs uses. @@ -122,7 +123,7 @@ function checkPersonaVars(vars, definition, at) { // so the business file can't name them. The key stays known so the refusal says where they went. const HOSTS_MOVED = "lives in the bus store now: enroll a host with a host-enroll grant (mosaic hosts status)"; -function checkRoles(roles, file, rolesDir, business, hasLibrary) { +function checkRoles(roles, file, rolesDir, business, hasLibrary, orgDefinitions) { const where = `${file} roles`; requireObject(roles, where); if (Object.keys(roles).length === 0) refuse(`${where} must declare at least one role instance`); @@ -135,8 +136,14 @@ function checkRoles(roles, file, rolesDir, business, hasLibrary) { const at = `${where}.${instance}`; requireObject(entry, at); rejectUnknownKeys(entry, ["definition", "holder", "vars", "tracker", "credentials", "identity", "publish", "hosts"], at); - requireId(entry.definition, `${at}.definition`); - const definition = Object.hasOwn(definitions, entry.definition) ? definitions[entry.definition] : loadRole(rolesDir, entry.definition); + // #1873 T1: "org:" names one of this file's definitions; anything else is a template. + const orgName = orgDefinitionName(entry.definition); + if (orgName === null) requireId(entry.definition, `${at}.definition`); + else if (!Object.hasOwn(orgDefinitions, ORG_PREFIX + checkDefinitionName(orgName, `${at}.definition after org:`))) { + refuse(`definition-missing: ${at}.definition names ${ORG_PREFIX}${orgName}, which definitions doesn't declare`); + } + const definition = Object.hasOwn(definitions, entry.definition) ? definitions[entry.definition] + : orgName === null ? loadRole(rolesDir, entry.definition) : orgDefinitions[entry.definition]; definitions[entry.definition] = definition; if (definition.roleVersion !== 2) refuse(`${at}: role definition ${entry.definition} is version 1 and can't back a business role instance`); const services = definition.credentials.map((c) => c.service); @@ -239,7 +246,9 @@ function checkRetired(retired, roles, file) { // Validate a parsed business document read from `file`. `rolesDir` is where // role definitions live. Returns a frozen business with its definitions. -export function validateOrganizationDocument(document, file, { rolesDir, dataRoot = null }) { +// options.assertClean is the credential store's secret check for the definitions' text, when the +// caller holds one (#1873 T1); without it the check is the empty store's. +export function validateOrganizationDocument(document, file, { rolesDir, dataRoot = null, assertClean } = {}) { requireObject(document, "business file"); rejectUnknownKeys(document, TOP_KEYS, "business file"); if (document.businessVersion !== 1) refuse(`business file "businessVersion" must be 1 (${file})`); @@ -253,7 +262,8 @@ export function validateOrganizationDocument(document, file, { rolesDir, dataRoo const vars = checkVars(document.vars ?? {}, "business", `${file} vars`); if (document.hosts !== undefined) refuse(`${file} hosts ${HOSTS_MOVED}`); const hasLibrary = Object.values(document.projects ?? {}).some((project) => project?.kind === "library"); - const { roles, definitions } = checkRoles(document.roles, file, rolesDir, document.id, hasLibrary); + const orgDefinitions = checkDefinitions(document.definitions, file, assertClean ? { assertClean } : {}); + const { roles, definitions } = checkRoles(document.roles, file, rolesDir, document.id, hasLibrary, orgDefinitions); requireObject(document.arbiters, `${file} arbiters`); rejectUnknownKeys(document.arbiters, ["delivery", "technical"], `${file} arbiters`); diff --git a/packages/organization/src/organization.mjs b/packages/organization/src/organization.mjs index e06e70086..4dea73672 100644 --- a/packages/organization/src/organization.mjs +++ b/packages/organization/src/organization.mjs @@ -14,6 +14,7 @@ import { ADDITION_VERB, additionProblem } from "./project-bootstrap.mjs"; import { remoteProblem, remoteTarget } from "./project-remotes.mjs"; import { withRegistryWriter } from "./org-registry.mjs"; import { entryForm, projectAgentProblem, rootForProjectCheck } from "./agent-levels-write.mjs"; +import { checkDefinition, checkDefinitionName, instancesNaming } from "./definitions.mjs"; export class OrganizationError extends Error { constructor(code, message = code) { @@ -352,8 +353,60 @@ export function organizationWriter({ dir, rolesDir, dataRoot = null, barrier = ( const projectsProtected = (old, document) => { if (canonicalJson(document?.projects) !== canonicalJson(old.projects)) fail("organization-projects-protected"); }; + // #1873 T1: definitions change only through setDefinition, which bumps rev and checks the entry. + const definitionsProtected = (old, document) => { + if (canonicalJson(document?.definitions ?? {}) !== canonicalJson(old.definitions ?? {})) fail("organization-definitions-protected"); + }; function write(id, document, options) { - return commit(id, document, { ...options, guard: projectsProtected }); + return commit(id, document, { ...options, guard: (old, next) => { projectsProtected(old, next); definitionsProtected(old, next); } }); + } + + // #1873 T1: the only writer of `definitions`. `next(current)` returns the new entry without its + // rev, or null to remove it; current is the entry or null. Nothing else in the document may change. + // An agent or the tester never writes one. A write bumps rev (1 for a new name); `rev`, when given, + // must be the current one (definition-rev-conflict). Editing or removing a definition an instance + // names refuses definition-in-use: changing a hired agent's role is a change decision (A4). + // assertClean is the credential store's check, when the caller holds one. + function setDefinition(id, name, next, { revision, rev, actor, detail, assertClean }) { + checkDefinitionName(name, "definition name"); + const logged = { ...detail, definition: name, rev: null }; + const guard = (old, document) => { + const agents = Object.entries(old.roles ?? {}).flatMap(([n, role]) => [n, `${role?.definition}.${n}`]); + if (actor === "tester" || agents.includes(actor) || (old.retired ?? []).includes(actor)) fail("human-required"); + const { definitions: a = {}, ...restOld } = old; + const { definitions: b = {}, ...restNew } = document ?? {}; + if (canonicalJson(restNew) !== canonicalJson(restOld)) fail("organization-definitions-only"); + const others = (d) => canonicalJson(Object.fromEntries(Object.entries(d).filter(([k]) => k !== name))); + if (others(a) !== others(b)) fail("organization-definitions-only"); + const current = Object.hasOwn(a, name) ? a[name] : null; + const sansRev = (e) => canonicalJson({ ...e, rev: 0 }); + if (current && Object.hasOwn(b, name) && sansRev(current) === sansRev(b[name])) fail("definition-unchanged", `definition ${name} is unchanged`); + if (current) { + const users = instancesNaming(old, name); + if (users.length) fail("definition-in-use", `definition ${name} is named by ${users.join(", ")}; a change to a hired agent's role is a change decision`); + } + if (Object.hasOwn(b, name)) { + const want = current ? current.rev + 1 : 1; + if (b[name]?.rev !== want) fail("definition-rev-conflict", `definition ${name} must be written at rev ${want}`); + checkDefinition(name, b[name], assertClean ? { assertClean } : {}); + } + }; + const out = commit(id, (old) => { + const definitions = { ...(old.definitions ?? {}) }; + const current = Object.hasOwn(definitions, name) ? definitions[name] : null; + if (rev !== undefined && (current?.rev ?? null) !== rev) fail("definition-rev-conflict", `definition ${name} is not at rev ${rev}; read again`); + const entry = next(current); + if (entry === null) { + if (!current) fail("definition-missing", `definitions declares no ${name}`); + delete definitions[name]; + } else { + definitions[name] = { ...entry, rev: current ? current.rev + 1 : 1 }; + logged.rev = definitions[name].rev; + } + const { definitions: _, ...rest } = old; + return Object.keys(definitions).length ? { ...rest, definitions } : rest; + }, { revision, actor, guard: (old, document) => { projectsProtected(old, document); guard(old, document); }, detail: logged }); + return { ...out, rev: logged.rev }; } // Lifecycle Task 2: the only writer that adds a project mapping, and only the exact bootstrap @@ -483,5 +536,5 @@ export function organizationWriter({ dir, rolesDir, dataRoot = null, barrier = ( }); } - return { logFile, read, readRaw, write, addProject, setProjectRemote, setProjectAgent, hold, holdAsync, findAddition, record, recover: id => withLock(id, () => recover(id)) }; + return { logFile, read, readRaw, write, addProject, setProjectRemote, setProjectAgent, setDefinition, hold, holdAsync, findAddition, record, recover: id => withLock(id, () => recover(id)) }; } diff --git a/packages/organization/src/resolve.mjs b/packages/organization/src/resolve.mjs index bcf4bd3e7..99f55aa1f 100644 --- a/packages/organization/src/resolve.mjs +++ b/packages/organization/src/resolve.mjs @@ -116,6 +116,9 @@ export function resolveInstance({ system, business, project = null, instance, en webSource: business.plugins?.["search-source"] ?? null, // The role file's skill names. The bundle resolves each against /skills-enabled. skills: [...definition.skills], + // #1873 T1: an org definition has no contract file; its instructions are the contract. Added + // only then, so a template instance resolves, and digests, exactly as before. + ...(definition.contractPath === null && typeof definition.instructions === "string" ? { instructions: definition.instructions } : {}), }; return deepFreeze({ ...resolved, digest: sha256(canonicalJson(resolved)) }); } diff --git a/packages/organization/tests/definitions.test.mjs b/packages/organization/tests/definitions.test.mjs new file mode 100644 index 000000000..b182f2455 --- /dev/null +++ b/packages/organization/tests/definitions.test.mjs @@ -0,0 +1,336 @@ +// #1873 T1: organization definitions and templates (roles-and-rails design sections 4, 5 and 13). +import test from "node:test"; +import assert from "node:assert/strict"; +import { appendFileSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { organizationWriter } from "../src/organization.mjs"; +import { validateBusinessDocument } from "../src/organization-config.mjs"; +import { definitionVerbs, DEFINITION_VERBS } from "../src/definition-verbs.mjs"; +import { templateHash, templateHashes, diffDefinition, lineDiff } from "../src/definitions.mjs"; +import { resolveInstance } from "../src/resolve.mjs"; +import { ROLE_RIGHTS, VERB_RIGHTS, normalizeRights } from "../src/org-rights.mjs"; +import { businessDoc, rolesCopy, systemFor, tmp, writeJson } from "./helpers.mjs"; + +const SECRET = "sk-test-NOT-A-SECRET-1234"; +// The credential store's check, as if it held SECRET. +const assertClean = (value) => { + if (String(value).includes(SECRET)) throw new Error(`credential-leak ${value}`); +}; +const OWNER = { actor: "jason", rights: [...ROLE_RIGHTS.owner], agent: false }; +const MEMBER = { actor: "alice", rights: [...ROLE_RIGHTS.member], agent: false }; +const HIRER = { actor: "alice", rights: normalizeRights(ROLE_RIGHTS.member, { add: ["org.hire"] }), agent: false }; +const PLAIN = { role: { tools: ["read"], network: "none", authority: { withinRole: [], crossRole: [] }, credentials: [] }, instructions: "Read and report.\n" }; + +function fixture(t, edit = (doc) => doc) { + const root = tmp(); + t.after(() => rmSync(root, { recursive: true, force: true })); + const dir = join(root, "config"); + const rolesDir = rolesCopy(root); + const file = writeJson(join(dir, "businesses/acme.json"), edit(businessDoc(root))); + const writer = organizationWriter({ dir, rolesDir }); + return { root, dir, file, rolesDir, writer, verbs: definitionVerbs({ writer, rolesDir, assertClean }) }; +} +const rev = (f) => f.writer.read("acme").revision; +const doc = (f) => JSON.parse(readFileSync(f.file, "utf8")); + +test("T1 import pins the template hash and copies its primitives and contract", (t) => { + const f = fixture(t); + const hash = templateHash(f.rolesDir, "coder"); + assert.match(hash, /^[0-9a-f]{64}$/); + const receipt = f.verbs.import("acme", { revision: rev(f), template: "coder" }, OWNER); + assert.deepEqual(Object.keys(receipt).sort(), ["definition", "from", "organization", "rev", "revision", "transaction", "verb"]); + assert.equal(receipt.rev, 1); + assert.deepEqual(receipt.from, { template: "coder", sha256: hash }); + const copy = doc(f).definitions.coder; + assert.deepEqual(copy.from, { template: "coder", sha256: hash }); + assert.equal(copy.rev, 1); + assert.equal(copy.instructions, readFileSync(join(f.rolesDir, "coder.md"), "utf8")); + const template = JSON.parse(readFileSync(join(f.rolesDir, "coder.json"), "utf8")); + assert.deepEqual(copy.role.tools, template.tools); + assert.equal(copy.role.network, template.network); + assert.equal(receipt.revision, rev(f)); + // The hash read lists every version 2 template, and only those. + const hashes = templateHashes(f.rolesDir); + assert.ok(hashes.some((h) => h.template === "coder" && h.sha256 === hash)); + assert.ok(!hashes.some((h) => h.template === "conductor-policy")); + // --as names the copy. + const second = f.verbs.import("acme", { revision: rev(f), template: "coder", as: "coder-two" }, OWNER); + assert.equal(second.definition, "coder-two"); + assert.throws(() => f.verbs.import("acme", { revision: rev(f), template: "coder" }, OWNER), { code: "definition-exists" }); +}); + +test("T1 the hash covers the template JSON and its contract together", (t) => { + const f = fixture(t); + const before = templateHash(f.rolesDir, "coder"); + appendFileSync(join(f.rolesDir, "coder.md"), "\nOne more line.\n"); + const contractChanged = templateHash(f.rolesDir, "coder"); + assert.notEqual(contractChanged, before); + const json = JSON.parse(readFileSync(join(f.rolesDir, "coder.json"), "utf8")); + writeJson(join(f.rolesDir, "coder.json"), { ...json, title: `${json.title} (renamed)` }, 0o644); + assert.notEqual(templateHash(f.rolesDir, "coder"), contractChanged); +}); + +test("T1 editing a template file changes no copy, and the diff read shows the change", (t) => { + const f = fixture(t); + f.verbs.import("acme", { revision: rev(f), template: "coder" }, OWNER); + const bytes = readFileSync(f.file); + const pinned = doc(f).definitions.coder.from.sha256; + assert.deepEqual(f.verbs.diff("acme", "coder"), { organization: "acme", revision: rev(f), definition: "coder", rev: 1, from: { template: "coder", sha256: pinned }, current: pinned, changed: false, primitives: [], instructions: [] }); + + appendFileSync(join(f.rolesDir, "coder.md"), "Always run the suites twice.\n"); + const json = JSON.parse(readFileSync(join(f.rolesDir, "coder.json"), "utf8")); + writeJson(join(f.rolesDir, "coder.json"), { ...json, network: "none" }, 0o644); + assert.notEqual(json.network, "none", "the fixture must change network"); + + // No copy changed, and the file still loads. + assert.deepEqual(readFileSync(f.file), bytes); + assert.equal(f.writer.read("acme").document.definitions.coder.from.sha256, pinned); + + const diff = f.verbs.diff("acme", "coder"); + assert.equal(diff.changed, true); + assert.equal(diff.current, templateHash(f.rolesDir, "coder")); + assert.notEqual(diff.current, pinned); + assert.deepEqual(diff.primitives, ["network", "instructions"]); + assert.deepEqual(diff.instructions.filter((l) => l.op !== " "), [{ op: "+", line: "Always run the suites twice." }]); + // Nothing applied. + assert.deepEqual(readFileSync(f.file), bytes); +}); + +test("T1 diff read: an invented definition has nothing to compare, a missing template is named", (t) => { + const f = fixture(t); + f.verbs.invent("acme", { revision: rev(f), name: "scout", definition: PLAIN }, OWNER); + assert.equal(f.verbs.diff("acme", "scout").changed, false); + assert.equal(f.verbs.diff("acme", "scout").current, null); + f.verbs.import("acme", { revision: rev(f), template: "proofreader" }, OWNER); + rmSync(join(f.rolesDir, "proofreader.json")); + const gone = diffDefinition(f.writer.read("acme").document, "proofreader", f.rolesDir); + assert.equal(gone.templateMissing, true); + assert.throws(() => f.verbs.diff("acme", "nobody"), { name: "BusinessError" }); + assert.deepEqual(lineDiff("a\nb", "a\nc"), [{ op: " ", line: "a" }, { op: "-", line: "b" }, { op: "+", line: "c" }]); +}); + +test("T1 rev counts up on every write, and a stale write refuses", (t) => { + const f = fixture(t); + assert.equal(f.verbs.invent("acme", { revision: rev(f), name: "scout", definition: PLAIN }, OWNER).rev, 1); + const edited = { ...PLAIN, instructions: "Read, then report.\n" }; + assert.equal(f.verbs.edit("acme", { revision: rev(f), rev: 1, name: "scout", definition: edited }, OWNER).rev, 2); + assert.equal(f.verbs.edit("acme", { revision: rev(f), rev: 2, name: "scout", definition: { ...edited, personality: { summary: "Terse." } } }, OWNER).rev, 3); + assert.equal(doc(f).definitions.scout.rev, 3); + const bytes = readFileSync(f.file); + // A stale rev, a stale file revision, an unchanged edit. + assert.throws(() => f.verbs.edit("acme", { revision: rev(f), rev: 2, name: "scout", definition: PLAIN }, OWNER), { code: "definition-rev-conflict" }); + const old = rev(f); + assert.throws(() => f.verbs.edit("acme", { revision: "0".repeat(64), rev: 3, name: "scout", definition: PLAIN }, OWNER), { code: "revision-conflict" }); + assert.throws(() => f.verbs.edit("acme", { revision: old, rev: 3, name: "scout", definition: { ...edited, personality: { summary: "Terse." } } }, OWNER), { code: "definition-unchanged" }); + assert.throws(() => f.verbs.edit("acme", { revision: old, name: "scout", definition: PLAIN }, OWNER), { code: "definition-rev-required" }); + assert.throws(() => f.verbs.remove("acme", { revision: old, rev: 1, name: "scout" }, OWNER), { code: "definition-rev-conflict" }); + assert.deepEqual(readFileSync(f.file), bytes); + // Edit keeps from; remove takes the entry out. + f.verbs.import("acme", { revision: rev(f), template: "coder" }, OWNER); + const from = doc(f).definitions.coder.from; + f.verbs.edit("acme", { revision: rev(f), rev: 1, name: "coder", definition: PLAIN }, OWNER); + assert.deepEqual(doc(f).definitions.coder.from, from); + const removed = f.verbs.remove("acme", { revision: rev(f), rev: 3, name: "scout" }, OWNER); + assert.equal(removed.removedRev, 3); + assert.ok(!Object.hasOwn(doc(f).definitions, "scout")); + // Each write is logged with its verb, name and rev, never role text. + const log = readFileSync(f.writer.logFile("acme"), "utf8").trim().split("\n").map(JSON.parse).filter((e) => e.state === "committed"); + assert.deepEqual(log.map((e) => [e.detail.verb, e.detail.definition, e.detail.rev]), [ + ["definition.invent.console", "scout", 1], ["definition.edit.console", "scout", 2], ["definition.edit.console", "scout", 3], + ["definition.import.console", "coder", 1], ["definition.edit.console", "coder", 2], ["definition.remove.console", "scout", null], + ]); +}); + +test("T1 invent and customize refuse what checkComposition refuses, with no value in the error", (t) => { + const f = fixture(t); + f.verbs.invent("acme", { revision: rev(f), name: "scout", definition: PLAIN }, OWNER); + const bytes = readFileSync(f.file); + const bad = [ + ["held credential in instructions", { ...PLAIN, instructions: `Use ${SECRET} to log in.\n` }], + ["held credential in personality", { ...PLAIN, personality: { voice: `say ${SECRET}` } }], + ["unregistered tool", { ...PLAIN, role: { ...PLAIN.role, tools: [SECRET] } }], + ["unregistered service", { ...PLAIN, role: { ...PLAIN.role, credentials: [{ service: SECRET }] } }], + ["hire chain", { ...PLAIN, role: { ...PLAIN.role, authority: { withinRole: ["role.hire"], crossRole: [] } } }], + ["over bound", { ...PLAIN, instructions: "x".repeat(16385) }], + ["unknown entry key", { ...PLAIN, [SECRET]: 1 }], + ["unknown role key", { ...PLAIN, role: { ...PLAIN.role, [SECRET]: [] } }], + ]; + for (const [what, definition] of bad) { + for (const call of [ + () => f.verbs.invent("acme", { revision: rev(f), name: "other", definition }, OWNER), + () => f.verbs.edit("acme", { revision: rev(f), rev: 1, name: "scout", definition }, OWNER), + ]) { + assert.throws(call, (e) => !e.message.includes(SECRET) && !e.message.includes("x".repeat(64)) && !String(e.stack).includes(SECRET), what); + } + } + assert.deepEqual(readFileSync(f.file), bytes); + // Import refuses a template that holds a hire right (R9): the chief of staff. + assert.throws(() => f.verbs.import("acme", { revision: rev(f), template: "chief-of-staff" }, OWNER), /cannot hire or fire/); + assert.deepEqual(readFileSync(f.file), bytes); +}); + +test("T1 a bad entry in the file refuses the whole file, naming the definition and key, never the value", (t) => { + const root = tmp(); + t.after(() => rmSync(root, { recursive: true, force: true })); + const rolesDir = rolesCopy(root); + const file = join(root, "config/businesses/acme.json"); + const base = businessDoc(root); + const entry = { rev: 1, ...PLAIN }; + const refusal = (definitions) => { + try { + validateBusinessDocument({ ...base, definitions }, file, { rolesDir, assertClean }); + } catch (e) { + assert.equal(e.name, "BusinessError"); + return e.message; + } + assert.fail("expected a refusal"); + }; + assert.match(refusal({ scout: { ...entry, instructions: `see ${SECRET}` } }), /definitions\.scout: composition instructions holds a credential/); + assert.match(refusal({ scout: { ...entry, role: { ...PLAIN.role, tools: [SECRET] } } }), /definitions\.scout: .*tools/); + assert.match(refusal({ scout: { ...entry, rev: 0 } }), /definitions\.scout\.rev must be a positive integer/); + assert.match(refusal({ scout: { ...entry, from: { template: "coder", sha256: SECRET } } }), /definitions\.scout\.from\.sha256/); + assert.match(refusal({ scout: { ...entry, [SECRET]: true } }), /definitions\.scout has an unknown key \(key \d+;/); + assert.match(refusal({ [SECRET]: entry }), /definitions key 1 must be a definition name/); + for (const d of [{ scout: { ...entry, instructions: `see ${SECRET}` } }, { scout: { ...entry, role: { ...PLAIN.role, tools: [SECRET] } } }, { [SECRET]: entry }, { scout: { ...entry, [SECRET]: true } }]) { + assert.ok(!refusal(d).includes(SECRET)); + } + // A good definition validates; the resolved map holds only what instances name. + const ok = validateBusinessDocument({ ...base, definitions: { scout: entry } }, file, { rolesDir }); + assert.ok(!Object.hasOwn(ok.definitions, "org:scout")); +}); + +test("T1 an org: reference resolves to the definition, and a missing one refuses naming instance and definition", (t) => { + const root = tmp(); + t.after(() => rmSync(root, { recursive: true, force: true })); + const rolesDir = rolesCopy(root); + const file = join(root, "config/businesses/acme.json"); + const base = businessDoc(root); + const named = { ...base, definitions: { scout: { rev: 4, ...PLAIN, personality: { summary: "Terse." } } }, roles: { ...base.roles, helper: { definition: "org:scout" } } }; + const business = validateBusinessDocument(named, file, { rolesDir }); + const role = business.definitions["org:scout"]; + assert.equal(role.name, "org:scout"); + assert.equal(role.contractPath, null); + assert.equal(role.instructions, PLAIN.instructions); + assert.equal(role.rev, 4); + const resolved = resolveInstance({ system: systemFor(root), business, instance: "helper" }); + assert.equal(resolved.definition, "org:scout"); + assert.equal(resolved.contract, null); + assert.equal(resolved.instructions, PLAIN.instructions); + assert.deepEqual(resolved.limits.tools, ["read"]); + + const missing = { ...base, roles: { ...base.roles, helper: { definition: "org:ghost" } } }; + assert.throws(() => validateBusinessDocument(missing, file, { rolesDir }), (e) => /definition-missing: .*roles\.helper\.definition names org:ghost/.test(e.message)); + assert.throws(() => validateBusinessDocument({ ...base, roles: { ...base.roles, helper: { definition: `org:${SECRET.toLowerCase()}!` } } }, file, { rolesDir }), (e) => !e.message.includes(SECRET.toLowerCase())); + // Template names work exactly as before, including one shaped like a definition name. + assert.throws(() => validateBusinessDocument({ ...base, roles: { ...base.roles, helper: { definition: "scout" } } }, file, { rolesDir }), { name: "BusinessError" }); +}); + +test("T1 an org whose instances name only templates loads exactly as before", (t) => { + const root = tmp(); + t.after(() => rmSync(root, { recursive: true, force: true })); + const rolesDir = rolesCopy(root); + const file = join(root, "config/businesses/acme.json"); + const base = businessDoc(root); + const plain = validateBusinessDocument(base, file, { rolesDir }); + const withUnused = validateBusinessDocument({ ...base, definitions: { scout: { rev: 1, ...PLAIN } } }, file, { rolesDir }); + assert.deepEqual(withUnused, plain); + assert.deepEqual(Object.keys(plain).sort(), ["arbiters", "businessVersion", "definitions", "file", "human", "id", "launch", "plugins", "projects", "roles", "tracker", "vars"]); + assert.deepEqual(Object.keys(plain.definitions).sort(), ["chief-of-staff", "coder", "cto", "pm", "reviewer"]); + const system = systemFor(root); + for (const instance of Object.keys(base.roles)) { + const a = resolveInstance({ system, business: plain, instance }); + const b = resolveInstance({ system, business: withUnused, instance }); + assert.equal(a.digest, b.digest); + assert.ok(!Object.hasOwn(a, "instructions")); + assert.equal(a.contract, plain.definitions[base.roles[instance].definition].contractPath); + } +}); + +test("T1 definition-in-use: an edit or removal of a definition an instance names refuses, naming it", (t) => { + const f = fixture(t); + f.verbs.invent("acme", { revision: rev(f), name: "scout", definition: PLAIN }, OWNER); + // A human adds the instance through the ordinary writer; definitions stay as they are. + const next = doc(f); + next.roles.helper = { definition: "org:scout" }; + f.writer.write("acme", next, { revision: rev(f), actor: "jason" }); + const bytes = readFileSync(f.file); + assert.throws(() => f.verbs.edit("acme", { revision: rev(f), rev: 1, name: "scout", definition: { ...PLAIN, instructions: "Changed.\n" } }, OWNER), + (e) => e.code === "definition-in-use" && /named by helper/.test(e.message)); + assert.throws(() => f.verbs.remove("acme", { revision: rev(f), rev: 1, name: "scout" }, OWNER), + (e) => e.code === "definition-in-use" && /named by helper/.test(e.message)); + assert.deepEqual(readFileSync(f.file), bytes); + assert.deepEqual(f.verbs.list("acme").definitions, [{ name: "scout", reference: "org:scout", rev: 1, from: null, usedBy: ["helper"] }]); + assert.deepEqual(f.verbs.show("acme", "scout").usedBy, ["helper"]); +}); + +test("T1 the ordinary writer cannot change definitions, and setDefinition changes nothing else", (t) => { + const f = fixture(t); + f.verbs.invent("acme", { revision: rev(f), name: "scout", definition: PLAIN }, OWNER); + const next = doc(f); + next.definitions.scout.instructions = "Sneaked in.\n"; + next.definitions.scout.rev = 2; + assert.throws(() => f.writer.write("acme", next, { revision: rev(f), actor: "jason" }), { code: "organization-definitions-protected" }); + const added = doc(f); + added.definitions.other = { rev: 1, ...PLAIN }; + assert.throws(() => f.writer.write("acme", added, { revision: rev(f), actor: "jason" }), { code: "organization-definitions-protected" }); + // The writer itself checks rev and the composition, whatever builds the entry. + assert.throws(() => f.writer.setDefinition("acme", "scout", () => ({ ...PLAIN, role: { ...PLAIN.role, tools: ["no-such-tool"] } }), { revision: rev(f), actor: "jason" }), { name: "BusinessError" }); +}); + +test("T1 an agent cannot call the edit verbs", (t) => { + const f = fixture(t); + f.verbs.invent("acme", { revision: rev(f), name: "scout", definition: PLAIN }, OWNER); + const bytes = readFileSync(f.file); + const calls = (caller) => [ + () => f.verbs.import("acme", { revision: rev(f), template: "coder" }, caller), + () => f.verbs.invent("acme", { revision: rev(f), name: "other", definition: PLAIN }, caller), + () => f.verbs.edit("acme", { revision: rev(f), rev: 1, name: "scout", definition: { ...PLAIN, instructions: "x\n" } }, caller), + () => f.verbs.remove("acme", { revision: rev(f), rev: 1, name: "scout" }, caller), + ]; + // An agent session, even one holding every right; an agent's name or the tester as actor. + for (const caller of [ + { ...OWNER, agent: true }, { actor: "coder", rights: OWNER.rights }, { ...OWNER, actor: "coder" }, + { ...OWNER, actor: "coder.coder" }, { ...OWNER, actor: "tester" }, { ...OWNER, actor: "chief" }, + ]) { + for (const call of calls(caller)) assert.throws(call, { code: "human-required" }); + } + // The writer refuses an agent actor by itself too. + assert.throws(() => f.writer.setDefinition("acme", "other", () => PLAIN, { revision: rev(f), actor: "pm" }), { code: "human-required" }); + assert.deepEqual(readFileSync(f.file), bytes); +}); + +test("T1 a member without org.hire refuses on each verb; the owner and a member granted org.hire succeed", (t) => { + const f = fixture(t); + for (const verb of Object.values(DEFINITION_VERBS)) assert.equal(VERB_RIGHTS[verb], "org.hire"); + f.verbs.invent("acme", { revision: rev(f), name: "scout", definition: PLAIN }, OWNER); + const bytes = readFileSync(f.file); + const refused = [ + () => f.verbs.import("acme", { revision: rev(f), template: "coder" }, MEMBER), + () => f.verbs.invent("acme", { revision: rev(f), name: "other", definition: PLAIN }, MEMBER), + () => f.verbs.edit("acme", { revision: rev(f), rev: 1, name: "scout", definition: { ...PLAIN, instructions: "x\n" } }, MEMBER), + () => f.verbs.remove("acme", { revision: rev(f), rev: 1, name: "scout" }, MEMBER), + () => f.verbs.invent("acme", { revision: rev(f), name: "other", definition: PLAIN }, { actor: "alice", agent: false }), + ]; + for (const call of refused) assert.throws(call, (e) => e.code === "right-required" && /needs org\.hire/.test(e.message)); + assert.deepEqual(readFileSync(f.file), bytes); + // Owner: every verb. + f.verbs.import("acme", { revision: rev(f), template: "coder" }, OWNER); + f.verbs.edit("acme", { revision: rev(f), rev: 1, name: "scout", definition: { ...PLAIN, instructions: "x\n" } }, OWNER); + f.verbs.remove("acme", { revision: rev(f), rev: 2, name: "scout" }, OWNER); + // A member granted org.hire: every verb, logged as that member. + f.verbs.import("acme", { revision: rev(f), template: "reviewer" }, HIRER); + f.verbs.invent("acme", { revision: rev(f), name: "scout", definition: PLAIN }, HIRER); + f.verbs.edit("acme", { revision: rev(f), rev: 1, name: "scout", definition: { ...PLAIN, instructions: "y\n" } }, HIRER); + f.verbs.remove("acme", { revision: rev(f), rev: 2, name: "scout" }, HIRER); + const log = readFileSync(f.writer.logFile("acme"), "utf8").trim().split("\n").map(JSON.parse).filter((e) => e.state === "committed"); + assert.deepEqual(log.slice(-4).map((e) => e.actor), ["alice", "alice", "alice", "alice"]); +}); + +test("T1 the template files on disk are never written", (t) => { + const f = fixture(t); + const before = readFileSync(join(f.rolesDir, "coder.json")); + f.verbs.import("acme", { revision: rev(f), template: "coder" }, OWNER); + f.verbs.edit("acme", { revision: rev(f), rev: 1, name: "coder", definition: PLAIN }, OWNER); + assert.deepEqual(readFileSync(join(f.rolesDir, "coder.json")), before); + writeFileSync(join(f.root, "touched"), ""); +}); diff --git a/packages/organization/tests/org-rights.test.mjs b/packages/organization/tests/org-rights.test.mjs index 1097c0a4f..53caecd25 100644 --- a/packages/organization/tests/org-rights.test.mjs +++ b/packages/organization/tests/org-rights.test.mjs @@ -60,6 +60,10 @@ const VERBS = { 'decision.preview.console': 'decide', 'decision.resolve.console': 'decide', 'hire.resolve.console': 'org.hire', + 'definition.import.console': 'org.hire', + 'definition.edit.console': 'org.hire', + 'definition.invent.console': 'org.hire', + 'definition.remove.console': 'org.hire', 'request.answer.console': 'request.write', 'request.attach.console': 'request.write', 'request.cancel.console': 'request.write', @@ -186,6 +190,8 @@ test('M1 every actual console, user, board and person-task verb has a reviewed a assert.deepEqual(Object.keys(VERBS).sort(), [...new Set([ ...CONSOLE_VERBS, ...USER_VERBS, ...BOARD_VERBS, ...PERSON_TASK_VERBS, 'plugins', 'plugins.set', 'limits', 'hire.resolve.console', // R1 reserves the policy mapping; A4 implements the verb. + // T1: the definition verbs. The CLI calls them today; the Console route comes later. + 'definition.import.console', 'definition.edit.console', 'definition.invent.console', 'definition.remove.console', ])].sort()); }); -- 2.54.0 From b7c9eee6038189d3a98b72f473f1d8abe96426ff Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Tue, 10 Nov 2026 17:44:43 -0600 Subject: [PATCH 2/2] fix(organization): definition writes go through the bus with the held-secret check (#1873) Review P1: the CLI wrote definitions with a no-op secret check. setDefinition now refuses definition-secret-check-required when a write that adds or changes an entry has no assertClean. New bus verbs definition.import|invent|edit|remove run the writes for the business's human at the cli with the credential store's assertClean; the tester, agents and readers refuse. The CLI's definition writes call the bus and map its codes to messages that never quote the definition. --- packages/bus/src/broker.mjs | 20 ++++- packages/bus/src/runtime.mjs | 5 ++ packages/bus/tests/definition-verbs.test.mjs | 74 ++++++++++++++++ packages/cli/src/cli.mjs | 3 + packages/cli/src/org-definitions.mjs | 56 +++++++++--- packages/cli/tests/org-definitions.test.mjs | 88 +++++++++++++++---- .../organization/src/definition-verbs.mjs | 7 ++ packages/organization/src/definitions.mjs | 6 +- packages/organization/src/organization.mjs | 6 +- .../organization/tests/definitions.test.mjs | 17 +++- 10 files changed, 246 insertions(+), 36 deletions(-) create mode 100644 packages/bus/tests/definition-verbs.test.mjs diff --git a/packages/bus/src/broker.mjs b/packages/bus/src/broker.mjs index 1dc43d718..2e6cdcade 100644 --- a/packages/bus/src/broker.mjs +++ b/packages/bus/src/broker.mjs @@ -193,6 +193,15 @@ const userLevel = (verb) => (VIEWER_VERBS.includes(verb) ? 'viewer' : 'collabora export const BOARD_VERBS = Object.freeze(['inbox', 'agents', 'thread.claims', 'tasks', 'trail', 'decisions', 'credentials', 'replies', 'messages', 'state', 'usage']); // The person's task-store writes, distinct from the agent's singular task.* actions. export const PERSON_TASK_VERBS = Object.freeze(['tasks.create', 'tasks.edit', 'tasks.archive', 'tasks.unarchive', 'tasks.delete']); +// #1873 T1: the human's definition writes, each with its [allowed, required] argument keys. Kept here, +// not imported, as the sandbox client loads this file; runtime.mjs maps each verb to the verb module +// (organization/src/definition-verbs.mjs names the same four, and a test holds them equal). +export const DEFINITION_BUS_KEYS = Object.freeze({ + 'definition.import': [['revision', 'template', 'as'], ['revision', 'template']], + 'definition.invent': [['revision', 'name', 'definition'], ['revision', 'name', 'definition']], + 'definition.edit': [['revision', 'rev', 'name', 'definition'], ['revision', 'rev', 'name', 'definition']], + 'definition.remove': [['revision', 'rev', 'name'], ['revision', 'rev', 'name']], +}); // Row 175: the organization verbs a tester may call. Its writes run every check the human's do // and log actor `tester`; row 176 adds routes reads; limits.set, organization.set and organization.revision refuse it. export const TESTER_ORGANIZATION_VERBS = Object.freeze(['limits', 'plugins', 'instances', 'plugins.set', 'instances.set', 'routes.set', 'routes']); @@ -6082,7 +6091,7 @@ export class Broker { } if (request.verb === 'grants.scope' || request.verb === 'grants.scope.set') return this.#grantScopeVerb(s, request.verb, structuredClone(args)); - if (["limits", "limits.set", "organization.set", "organization.revision", "plugins", "plugins.set", "instances", "instances.set", "routes", "routes.set", "veto.set", "library.limits.set"].includes(request.verb)) { + if (["limits", "limits.set", "organization.set", "organization.revision", "plugins", "plugins.set", "instances", "instances.set", "routes", "routes.set", "veto.set", "library.limits.set"].includes(request.verb) || Object.hasOwn(DEFINITION_BUS_KEYS, request.verb)) { // Row 123 (#1617, REQ-COS-1, REQ-LIMIT-1): organization.set is limits.set for a role whose // file lists organization.change. It takes the same keys, runs the same write path and // logs the same receipt; only the actor differs: the role definition and instance. @@ -6137,6 +6146,13 @@ export class Broker { else if (request.verb === "library.limits.set") { keys(args, ["revision", "replyLimits"], ["revision", "replyLimits"]); this.#human(s); + } + // #1873 T1: the definition writes need org.hire. Until row 193, as #mayHuman, the business's + // one human (the owner) passes; the tester is refused above and an agent here. + else if (Object.hasOwn(DEFINITION_BUS_KEYS, request.verb)) { + keys(args, ...DEFINITION_BUS_KEYS[request.verb]); + this.#human(s); + if (s.human !== this.#business(s.business).human) fail("human-required"); } else { keys(args, ORGANIZATION_KEYS, ["revision"]); // Row 194 (D2.4, B7): the chief can't change a kept instance's model, as that replaces it at @@ -6413,6 +6429,8 @@ export class Broker { if ((verb === "instance.hire" || verb === "instance.fire") && e.code === "definition-not-hireable") fail(e.code); // Row 194 (B7, webby-0094 N3): the limits writer's keep check, against the file, as well. if (e.code === "instance-kept") fail(e.code); + // #1873 T1: the definition writer's refusals pass by code; a composition refusal stays invalid-request. + if (Object.hasOwn(DEFINITION_BUS_KEYS, verb) && /^definition-[a-z-]+$/.test(e.code ?? "")) fail(e.code); // Row 194 (brief item 8): a fire's or a delete's revoke failed, and nothing was written. if ((verb === "instance.fire" || verb === "instances.set") && e.code === "credential-revoke-failed") fail(e.code); // Row 65: the route checks every write of an instance's route vars runs. diff --git a/packages/bus/src/runtime.mjs b/packages/bus/src/runtime.mjs index f931dcf0f..c64cfc07e 100644 --- a/packages/bus/src/runtime.mjs +++ b/packages/bus/src/runtime.mjs @@ -35,6 +35,8 @@ import { createProxyLock } from './proxy-lock.mjs'; import { ModelKeys, modelOrganization, stopModelRuns, MODEL_STOP_TIMEOUT_MS } from './model-keys.mjs'; import { instanceSettings } from '../../organization/src/instance-settings.mjs'; import { organizationWriter, OrganizationError, setReplyLimits } from '../../organization/src/organization.mjs'; +import { definitionVerbs, DEFINITION_BUS_VERBS } from '../../organization/src/definition-verbs.mjs'; +import { ROLE_RIGHTS } from '../../organization/src/org-rights.mjs'; import { getProjectRemote } from '../../organization/src/project-remotes.mjs'; import { credentialVerbs } from './credential-verbs.mjs'; import { loadWithCopies } from '../../organization/src/agent-copies.mjs'; @@ -849,6 +851,9 @@ export async function startBroker({ else if(verb==='grants.scope.set')result=instances.grantScope(id,args,actor); // C2-06 M11: the human's replyLimits, through the organization writer's fence. else if(verb==='library.limits.set')result=setReplyLimits(organizationWriter({dir:organization.dir,rolesDir:organization.rolesDir}),id,args,actor); + // #1873 T1: the human's definition writes. The broker passes only the business's human (the + // owner, so ROLE_RIGHTS.owner); the store's assertClean is the held-secret check each write needs. + else if(Object.hasOwn(DEFINITION_BUS_VERBS,verb))result=definitionVerbs({writer:organizationWriter({dir:organization.dir,rolesDir:organization.rolesDir}),rolesDir:organization.rolesDir,assertClean:value=>credentials.assertClean(value)})[DEFINITION_BUS_VERBS[verb]](id,args,{actor,rights:[...ROLE_RIGHTS.owner],agent:false}); // Row 194 (D2.1, D2.2): the chief's hire and fire, with the reason in the change context. else if(verb==='instance.hire')result=instances.hire(id,args,{actor,context}); else if(verb==='instance.fire')result=instances.fire(id,{name:args.name},{actor,context}); diff --git a/packages/bus/tests/definition-verbs.test.mjs b/packages/bus/tests/definition-verbs.test.mjs new file mode 100644 index 000000000..a0f91d2ce --- /dev/null +++ b/packages/bus/tests/definition-verbs.test.mjs @@ -0,0 +1,74 @@ +// #1873 T1: the definition writes on the bus. Only the business's human at the cli reaches them, +// and the writer runs the credential store's held-secret check on what it would write. +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { appendFileSync, chmodSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { DEFINITION_BUS_KEYS } from '../src/broker.mjs'; +import { startBroker } from '../src/runtime.mjs'; +import { DEFINITION_BUS_VERBS } from '../../organization/src/definition-verbs.mjs'; +import { businessDoc, rolesCopy, systemFor, writeJson } from '../../organization/tests/helpers.mjs'; + +const TOKEN = 'fixture-held-token-t1-5c0e9a'; +const SCOUT = { role: { tools: ['read'], network: 'none', authority: { withinRole: [], crossRole: [] }, credentials: [] }, instructions: 'Read and report.\n' }; + +function files(dir) { + return readdirSync(dir, { withFileTypes: true }).flatMap((e) => e.isDirectory() ? files(join(dir, e.name)) : e.isFile() ? [join(dir, e.name)] : []); +} + +test('T1 the broker checks the same four definition verbs the runtime maps', () => { + assert.deepEqual(Object.keys(DEFINITION_BUS_KEYS).sort(), Object.keys(DEFINITION_BUS_VERBS).sort()); +}); + +test('T1 definition writes: the human at the cli writes, every other caller refuses, and a held token never lands', async (t) => { + const root = mkdtempSync(join(tmpdir(), 'mosaic-def-bus-')); + const rolesDir = rolesCopy(root); + const doc = businessDoc(root, 'demo'); + const file = writeJson(join(root, 'businesses/demo.json'), { ...doc, plugins: { ...doc.plugins, harness: ['pi'] } }); + const data = join(root, 'data'); + mkdirSync(data, { mode: 0o700 }); + const token = join(root, 'held.token'); + writeFileSync(token, TOKEN, { mode: 0o600 }); + chmodSync(token, 0o600); + const bus = await startBroker({ dataRoot: data, businesses: { demo: { id: 'demo', human: 'jason', arbiters: { technical: 'chief', delivery: 'chief' }, vars: { 'bus.tester': true }, roles: { + chief: { definition: 'chief-of-staff', authority: { withinRole: ['decision.resolve.technical', 'decision.resolve.delivery'], crossRole: [] } }, + coder: { authority: { withinRole: [], crossRole: [] }, credentials: { gitea: { file: token, rotateBy: '2099-01-01' } } }, + } } }, organization: { dir: root, rolesDir, system: systemFor(root) } }); + t.after(async () => { await bus.close(); rmSync(root, { recursive: true, force: true }); }); + const b = bus.broker; + const human = b.bindHuman({ business: 'demo', human: 'jason', via: 'cli', outsideAgent: true }); + const revision = () => b.request(human, { verb: 'limits' }).revision; + const invent = (name) => ({ verb: 'definition.invent', args: { revision: revision(), name, definition: SCOUT } }); + + // An agent, a reader and the tester never reach the writer. + const before = readFileSync(file); + const agent = b.bindLaunch({ business: 'demo', role: 'coder', run: 'coder-run', harness: 'pi', address: 'coder-run' }); + for (const cap of [agent, b.bindReader({ business: 'demo' })]) assert.throws(() => b.request(cap, invent('scout')), /human-required/); + assert.throws(() => b.request(b.bindTester({ business: 'demo' }), invent('scout')), /tester-refused/); + // Unknown and missing arguments refuse; an actor can't be forged through them. + assert.throws(() => b.request(human, { verb: 'definition.invent', args: { ...invent('scout').args, actor: 'mallory' } }), /invalid-request/); + assert.throws(() => b.request(human, { verb: 'definition.remove', args: { revision: revision(), name: 'scout' } }), /invalid-request/); + assert.deepEqual(readFileSync(file), before); + + // The human's invent commits, as the human, with a receipt that carries no role text. + const receipt = b.request(human, invent('scout')); + assert.deepEqual({ ...receipt, revision: 0, transaction: 0 }, { verb: 'invent', organization: 'demo', definition: 'scout', rev: 1, revision: 0, transaction: 0 }); + assert.equal(JSON.parse(readFileSync(file, 'utf8')).definitions.scout.instructions, SCOUT.instructions); + const last = JSON.parse(readFileSync(join(root, 'businesses/.history/demo/changes.jsonl'), 'utf8').trim().split('\n').at(-1)); + assert.equal(last.actor, 'jason'); + assert.equal(last.state, 'committed'); + // A writer refusal passes by its code. + assert.throws(() => b.request(human, invent('scout')), /definition-exists/); + + // The bus refuses a request that carries the held token, and the writer refuses a template whose + // contract carries it (the import request itself holds no token). Neither writes. + const written = readFileSync(file); + assert.throws(() => b.request(human, { verb: 'definition.invent', args: { revision: revision(), name: 'leak', definition: { ...SCOUT, instructions: `Use ${TOKEN}.\n` } } }), + (e) => e.code === 'credential-leak' && !e.message.includes(TOKEN)); + appendFileSync(join(rolesDir, 'coder.md'), `\nUse ${TOKEN} for the forge.\n`); + assert.throws(() => b.request(human, { verb: 'definition.import', args: { revision: revision(), template: 'coder' } }), + (e) => e.code === 'invalid-request' && !e.message.includes(TOKEN) && !String(e.stack).includes(TOKEN)); + assert.deepEqual(readFileSync(file), written); + for (const path of [...files(data), ...files(join(root, 'businesses'))]) assert.ok(!readFileSync(path).includes(TOKEN), path); +}); diff --git a/packages/cli/src/cli.mjs b/packages/cli/src/cli.mjs index 3d836473e..0d314951c 100644 --- a/packages/cli/src/cli.mjs +++ b/packages/cli/src/cli.mjs @@ -37,6 +37,7 @@ import { bootConfig, loadSystem, orgPaths, socketPath } from "./config.mjs"; import { bootLine, readBootStatus, recordBootOk, recordBootRefused } from "./boot-status.mjs"; import { businessFilePath, configDir } from "../../organization/src/index.mjs"; import { busExit, humanTransport, refuseInsideAgent, testerTransport } from "./transport.mjs"; +import { definitionsCommand } from "./org-definitions.mjs"; import { formatAgents, formatDecision, formatInbox, formatTasks, formatTrail, shortId } from "./format.mjs"; import { hostStatus, readHostState, startHost, stopHost } from "./host.mjs"; import { createLauncher } from "./launcher.mjs"; @@ -737,6 +738,8 @@ export async function main(argv, io = { env: process.env, stdin: process.stdin, // builds; `org validate FILE` stays orgCommand's draft check. if (verb === 'org' && rest[0] === 'config') return orgConfig(rest.slice(1), io); if (verb === 'org' && ["instances", "instance", "veto"].includes(rest[0])) return orgInstancesCommand(rest, io, (p) => callFor(p, io, deps)); + // #1873 T1: definition writes go through the organization's bus, as the human. + if (verb === 'org' && rest[0] === 'definitions') return definitionsCommand(rest.slice(1), io, { ...deps, callFor: (p) => callFor(p, io, deps) }); if (verb === 'org') return orgCommand(rest, io, deps); // A2 (#1873): the owner's root and project agent levels. `mosaic agents` without `root` stays the bus read. if (verb === "agents" && rest[0] === "root") return agentsRootCommand(rest.slice(1), io); diff --git a/packages/cli/src/org-definitions.mjs b/packages/cli/src/org-definitions.mjs index 50024d35f..bb009c08f 100644 --- a/packages/cli/src/org-definitions.mjs +++ b/packages/cli/src/org-definitions.mjs @@ -1,15 +1,16 @@ // #1873 T1: `mosaic org definitions`, the human verbs on an organization's own role definitions and -// the template hash and diff reads. A write runs only from a human shell (refuseInsideAgent) and acts -// as the organization's human holding the owner's rights: the OS user who owns the config directory -// owns the organization, the same trust `mosaic org update` rests on. Each write prints its receipt. +// the template hash and diff reads. Reads run here, from the file. A write runs only from a human +// shell (refuseInsideAgent) and goes through the organization's bus as its human: the bus holds the +// credential store, whose held-secret check every definition write needs (R1), and no other process +// holds the tokens it looks for. Each write prints its receipt. import { readFile } from 'node:fs/promises'; import { configDir } from '../../organization/src/organization-config.mjs'; import { organizationWriter } from '../../organization/src/organization.mjs'; import { definitionVerbs } from '../../organization/src/definition-verbs.mjs'; -import { ROLE_RIGHTS } from '../../organization/src/org-rights.mjs'; +import { checkDefinition } from '../../organization/src/definitions.mjs'; import { rolesDir } from './config.mjs'; import { CliError } from './errors.mjs'; -import { refuseInsideAgent } from './transport.mjs'; +import { busExit, refuseInsideAgent } from './transport.mjs'; export const DEFINITIONS_USAGE = 'mosaic org definitions templates | definitions ID list | definitions ID show NAME | definitions ID diff NAME | definitions ID import TEMPLATE [--as NAME] | definitions ID invent NAME --from FILE | definitions ID edit NAME --from FILE --if-rev N | definitions ID remove NAME --if-rev N'; @@ -39,6 +40,27 @@ async function definitionFile(file) { } } +// The bus answers a refusal with its code only. These say what the code means for this request; +// none quotes the definition's text. +function refusal(code, verb, name, rev, read) { + const why = { + 'definition-in-use': () => `definition ${name} is named by ${read().usedBy.join(', ')}; a change to a hired agent's role is a change decision`, + 'definition-rev-conflict': () => `definition ${name} is not at rev ${rev}`, + 'definition-exists': () => `definitions already declares ${name}`, + 'definition-missing': () => `definitions declares no ${name}`, + 'revision-conflict': () => 'the organization file changed while this ran; run it again', + 'credential-leak': () => `the ${verb} request holds a credential the organization's bus holds; nothing was written`, + 'invalid-request': () => `the bus refused the ${verb}: the definition fails the composition check or holds a held credential`, + }[code]; + let detail = null; + try { detail = why?.() ?? null; } catch { detail = null; } + const error = new CliError(detail ? `${code}: ${detail}` : code, busExit(code)); + error.code = code; + return error; +} + +// deps.callFor(parsed) returns the human's call(verb, args) for the organization parsed names (cli.mjs +// passes its callFor). Without one, a write refuses: it never runs without the bus's secret check. export async function definitionsCommand(args, io, deps = {}) { const { pos, values } = parse(args); const allowed = (...keys) => { for (const k of Object.keys(values)) if (!keys.includes(k)) throw new CliError(`unexpected ${k}`, 4); }; @@ -47,18 +69,28 @@ export async function definitionsCommand(args, io, deps = {}) { const [id, verb, name] = pos; if (WRITES.includes(verb)) refuseInsideAgent(io.env); const writer = deps.writer ?? organizationWriter({ dir: configDir(io.env), rolesDir: rolesDir(io.env) }); - const verbs = definitionVerbs({ writer, rolesDir: rolesDir(io.env), assertClean: deps.assertClean }); + const verbs = definitionVerbs({ writer, rolesDir: rolesDir(io.env) }); let result; if (id === 'templates' && pos.length === 1) { allowed(); result = verbs.templates(); } else if (verb === 'list' && pos.length === 2) { allowed(); result = verbs.list(id); } else if (['show', 'diff'].includes(verb) && pos.length === 3) { allowed(); result = verbs[verb](id, name); } else if (WRITES.includes(verb) && pos.length === 3) { - const { document, revision } = writer.read(id); - const caller = { actor: document.human, rights: [...ROLE_RIGHTS.owner], agent: false }; - if (verb === 'import') { allowed('--as'); result = verbs.import(id, { revision, template: name, as: values['--as'] ?? name }, caller); } - else if (verb === 'invent') { allowed('--from'); result = verbs.invent(id, { revision, name, definition: await definitionFile(need('--from')) }, caller); } - else if (verb === 'edit') { allowed('--from', '--if-rev'); const rev = ifRev(); result = verbs.edit(id, { revision, rev, name, definition: await definitionFile(need('--from')) }, caller); } - else { allowed('--if-rev'); result = verbs.remove(id, { revision, rev: ifRev(), name }, caller); } + let request, rev; + if (verb === 'import') { allowed('--as'); request = { template: name, as: values['--as'] ?? name }; } + else if (verb === 'invent') { allowed('--from'); request = { name, definition: await definitionFile(need('--from')) }; } + else if (verb === 'edit') { allowed('--from', '--if-rev'); rev = ifRev(); request = { rev, name, definition: await definitionFile(need('--from')) }; } + else { allowed('--if-rev'); rev = ifRev(); request = { rev, name }; } + // The file's own checks first, so a bad key is named here; the bus repeats them with its secret check. + if (request.definition) checkDefinition(name, { ...request.definition, rev: rev ?? 1 }); + if (typeof deps.callFor !== 'function') throw new CliError('a definition write goes through the organization bus; run it as `mosaic org definitions`', 3); + const call = deps.callFor({ positional: [id], flags: new Set(), values: { '--business': id } }); + const { revision } = writer.read(id); + try { + result = await call(`definition.${verb}`, { revision, ...request }); + } catch (e) { + if (typeof e?.code !== 'string' || e.code === '') throw e; + throw refusal(e.code, verb, request.as ?? name, rev, () => verbs.show(id, request.as ?? name)); + } } else throw new CliError(DEFINITIONS_USAGE, 4); io.stdout.write(JSON.stringify(result, null, 2) + '\n'); } diff --git a/packages/cli/tests/org-definitions.test.mjs b/packages/cli/tests/org-definitions.test.mjs index 627fd0c16..1f2f6371b 100644 --- a/packages/cli/tests/org-definitions.test.mjs +++ b/packages/cli/tests/org-definitions.test.mjs @@ -2,9 +2,13 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import { join } from 'node:path'; -import { readFileSync, writeFileSync } from 'node:fs'; +import { readdirSync, readFileSync, writeFileSync } from 'node:fs'; import { createLauncher } from '../src/launcher.mjs'; import { main } from '../src/cli.mjs'; +import { orgCommand } from '../src/org.mjs'; +import { bootConfig, loadSystem } from '../src/config.mjs'; +import { BusError } from '../../bus/src/broker.mjs'; +import { startBroker } from '../../bus/src/runtime.mjs'; import { initializeControl } from '../../processing-control/src/state.mjs'; import { launchLogFile } from '../../seat/src/session.mjs'; import { entryFromTemplate, readTemplate, templateHash } from '../../organization/src/definitions.mjs'; @@ -47,16 +51,36 @@ test('T1 an org: instance passes every launch check, and a missing definition re assert.throws(() => createLauncher({ host, system: JSON.parse(readFileSync(f.config, 'utf8')), env: f.env, bwrap: '/usr/bin/true', log: () => {} }), /org:ghost/); }); -test('T1 mosaic org definitions: each write prints its receipt, and an agent shell refuses', async (t) => { +// The fixture bus holds SENTINEL as coder's gitea token, so the bus's secret check refuses it. +const SENTINEL = 'sk-test-NOT-A-SECRET-1234'; +function files(dir) { + return readdirSync(dir, { withFileTypes: true }).flatMap((e) => e.isDirectory() ? files(join(dir, e.name)) : e.isFile() ? [join(dir, e.name)] : []); +} + +test('T1 mosaic org definitions: writes go through the bus and its secret check, each prints its receipt, and an agent shell refuses', async (t) => { const root = tmp(t); - const f = fixture(root); + const builder = entryFromTemplate(readTemplate(REPO_ROLES, 'coder')); + const f = fixture(root, 'acme', (doc) => ({ ...doc, definitions: { builder }, roles: { ...doc.roles, coder: { ...doc.roles.coder, definition: 'org:builder' } } })); const file = join(root, 'config/businesses/acme.json'); - const run = async (args, env = f.env) => { const x = io(env); await main(['org', 'definitions', ...args], x, {}); return JSON.parse(x.out.text); }; + const changes = join(root, 'config/businesses/.history/acme/changes.jsonl'); + writeFileSync(f.doc.roles.coder.credentials.gitea.file, SENTINEL); + const boot = bootConfig({ system: loadSystem({ env: f.env }), businessId: 'acme', env: f.env }); + const b = await startBroker({ dataRoot: f.dataRoot, businesses: boot.businesses, organization: boot.organization }); + t.after(() => b.close()); + const human = b.broker.bindHuman({ business: 'acme', human: f.doc.human, via: 'cli', outsideAgent: true }); + const calls = []; + // The human transport's shape: a bus refusal arrives as its code. + const deps = { system: { dataRoot: f.dataRoot }, transport: () => async (verb, args = {}) => { + calls.push(verb); + try { return structuredClone(b.broker.request(human, { verb, args })); } + catch (e) { if (!(e instanceof BusError)) throw e; throw Object.assign(new Error(e.code), { code: e.code }); } + } }; + const run = async (args, env = f.env) => { const x = io(env); await main(['org', 'definitions', ...args], x, deps); return JSON.parse(x.out.text); }; const hash = templateHash(f.roles, 'coder'); assert.ok((await run(['templates'])).some((h) => h.template === 'coder' && h.sha256 === hash)); - const imported = await run(['acme', 'import', 'coder', '--as', 'builder']); - assert.deepEqual({ ...imported, revision: 0, transaction: 0 }, { verb: 'import', organization: 'acme', definition: 'builder', rev: 1, revision: 0, transaction: 0, from: { template: 'coder', sha256: hash } }); + const imported = await run(['acme', 'import', 'coder', '--as', 'copy']); + assert.deepEqual({ ...imported, revision: 0, transaction: 0 }, { verb: 'import', organization: 'acme', definition: 'copy', rev: 1, revision: 0, transaction: 0, from: { template: 'coder', sha256: hash } }); const spec = join(root, 'scout.json'); writeFileSync(spec, JSON.stringify({ role: SCOUT.role, instructions: SCOUT.instructions })); assert.equal((await run(['acme', 'invent', 'scout', '--from', spec])).rev, 1); @@ -64,22 +88,50 @@ test('T1 mosaic org definitions: each write prints its receipt, and an agent she const edited = await run(['acme', 'edit', 'scout', '--from', spec, '--if-rev', '1']); assert.equal(edited.verb, 'edit'); assert.equal(edited.rev, 2); - assert.deepEqual((await run(['acme', 'list'])).definitions.map((d) => [d.name, d.rev]), [['builder', 1], ['scout', 2]]); + assert.deepEqual((await run(['acme', 'list'])).definitions.map((d) => [d.name, d.rev]), [['builder', 1], ['copy', 1], ['scout', 2]]); assert.equal((await run(['acme', 'show', 'scout'])).instructions, 'Read, then report.\n'); - assert.equal((await run(['acme', 'diff', 'builder'])).changed, false); + assert.equal((await run(['acme', 'diff', 'copy'])).changed, false); assert.equal((await run(['acme', 'remove', 'scout', '--if-rev', '2'])).removedRev, 2); + assert.deepEqual(calls, ['definition.import', 'definition.invent', 'definition.edit', 'definition.remove']); + const committed = readFileSync(changes, 'utf8').trim().split('\n').map((l) => JSON.parse(l)).filter((e) => e.state === 'committed').slice(-4); + assert.deepEqual(committed.map((e) => e.actor), Array(4).fill(f.doc.human)); - // A held-secret-shaped file never echoes; a stale --if-rev refuses. - writeFileSync(spec, '{"role": sk-test-NOT-A-SECRET-1234'); - await assert.rejects(run(['acme', 'invent', 'other', '--from', spec]), (e) => e.exitCode === 2 && !e.message.includes('sk-test')); - await assert.rejects(run(['acme', 'remove', 'builder', '--if-rev', '7']), (e) => /^definition-rev-conflict: definition builder is not at rev 7/.test(e.message)); - - // Inside an agent run every write refuses before it reads anything; reads still work. + // A definition holding a token the bus holds refuses on invent and on edit, in valid JSON, and + // the token reaches no message, no organization file and nothing the bus wrote. const before = readFileSync(file); - const agent = { ...f.env, MOSAIC_RUN_ID: 'run-1' }; - for (const args of [['acme', 'import', 'reviewer'], ['acme', 'invent', 'x', '--from', spec], ['acme', 'edit', 'builder', '--from', spec, '--if-rev', '1'], ['acme', 'remove', 'builder', '--if-rev', '1']]) { - await assert.rejects(run(args, agent), (e) => e.exitCode === 3 && /human shell/.test(e.message)); + const beforeChanges = readFileSync(changes); + writeFileSync(spec, JSON.stringify({ role: SCOUT.role, instructions: `Use ${SENTINEL} to report.\n` })); + for (const args of [['acme', 'invent', 'leak', '--from', spec], ['acme', 'edit', 'copy', '--from', spec, '--if-rev', '1']]) { + await assert.rejects(run(args), (e) => e.code === 'credential-leak' && e.exitCode === 2 && !e.message.includes(SENTINEL) && !String(e.stack).includes(SENTINEL)); } assert.deepEqual(readFileSync(file), before); - assert.equal((await run(['acme', 'list'], agent)).definitions.length, 1); + assert.deepEqual(readFileSync(changes), beforeChanges); + for (const path of files(f.dataRoot)) assert.ok(!readFileSync(path).includes(SENTINEL), path); + + // An unparsable file never echoes; a key the file's checks refuse is named before the bus is + // called; a stale --if-rev and an in-use definition refuse with what they mean. + writeFileSync(spec, `{"role": ${SENTINEL}`); + await assert.rejects(run(['acme', 'invent', 'other', '--from', spec]), (e) => e.exitCode === 2 && !e.message.includes('sk-test')); + writeFileSync(spec, JSON.stringify({ role: { ...SCOUT.role, network: 'everywhere' }, instructions: 'x\n' })); + const asked = calls.length; + await assert.rejects(run(['acme', 'invent', 'other', '--from', spec]), /definitions\.other/); + assert.equal(calls.length, asked); + await assert.rejects(run(['acme', 'remove', 'copy', '--if-rev', '7']), (e) => /^definition-rev-conflict: definition copy is not at rev 7/.test(e.message)); + await assert.rejects(run(['acme', 'remove', 'builder', '--if-rev', '1']), (e) => /^definition-in-use: definition builder is named by coder/.test(e.message)); + assert.deepEqual(readFileSync(file), before); + + // Inside an agent run every write refuses before it reads anything or calls the bus; reads still work. + const agent = { ...f.env, MOSAIC_RUN_ID: 'run-1' }; + const sent = calls.length; + for (const args of [['acme', 'import', 'reviewer'], ['acme', 'invent', 'x', '--from', spec], ['acme', 'edit', 'copy', '--from', spec, '--if-rev', '1'], ['acme', 'remove', 'copy', '--if-rev', '1']]) { + await assert.rejects(run(args, agent), (e) => e.exitCode === 3 && /human shell/.test(e.message)); + } + assert.equal(calls.length, sent); + assert.deepEqual(readFileSync(file), before); + assert.equal((await run(['acme', 'list'], agent)).definitions.length, 2); + + // With no bus call to make, a write refuses rather than writing without the secret check. + const x = io(f.env); + await assert.rejects(orgCommand(['definitions', 'acme', 'remove', 'copy', '--if-rev', '1'], x), (e) => e.exitCode === 3 && /organization bus/.test(e.message)); + assert.deepEqual(readFileSync(file), before); }); diff --git a/packages/organization/src/definition-verbs.mjs b/packages/organization/src/definition-verbs.mjs index 00b85d411..87316f0a5 100644 --- a/packages/organization/src/definition-verbs.mjs +++ b/packages/organization/src/definition-verbs.mjs @@ -24,6 +24,13 @@ export const DEFINITION_VERBS = Object.freeze({ remove: "definition.remove.console", }); +// The bus verbs for the writes (the broker's DEFINITION_BUS_KEYS checks their arguments). The bus +// runs them for the business's human at the cli, with its credential store's secret check; the CLI's +// writes go through them, as no other process holds the tokens that check looks for. +export const DEFINITION_BUS_VERBS = Object.freeze({ + "definition.import": "import", "definition.invent": "invent", "definition.edit": "edit", "definition.remove": "remove", +}); + const fail = (code, message = code) => { throw new OrganizationError(code, message); }; diff --git a/packages/organization/src/definitions.mjs b/packages/organization/src/definitions.mjs index c5b68bdb1..79740b2da 100644 --- a/packages/organization/src/definitions.mjs +++ b/packages/organization/src/definitions.mjs @@ -45,7 +45,9 @@ const NO_CHIEF = Object.freeze({ authority: Object.freeze({ withinRole: Object.freeze([]), crossRole: Object.freeze([]) }), credentials: Object.freeze([]), messaging: null, }); -// A caller with no credential store holds no token, so its secret check is the empty store's. +// The file-load check has no credential store: a load runs before the bus builds one from the file. +// The held-secret scan runs where definitions are written (setDefinition requires it, and the bus +// passes credentials.assertClean). A load still runs every other composition check. const NO_STORE = () => {}; const plain = (value) => value !== null && typeof value === "object" && !Array.isArray(value) @@ -75,7 +77,7 @@ export function compositionOf(entry) { } // One entry: its own keys, then checkComposition. Returns { composition, flagged }. options.assertClean -// is the credential store's check when the caller has one. +// is the credential store's check; a write always passes it (setDefinition), a file load can't. export function checkDefinition(name, entry, { assertClean = NO_STORE, where = "definitions" } = {}) { const at = `${where}.${checkDefinitionName(name, `${where} key`)}`; if (!plain(entry)) refuse(`${at} must be a JSON object`); diff --git a/packages/organization/src/organization.mjs b/packages/organization/src/organization.mjs index 4dea73672..32f36f1d3 100644 --- a/packages/organization/src/organization.mjs +++ b/packages/organization/src/organization.mjs @@ -366,7 +366,8 @@ export function organizationWriter({ dir, rolesDir, dataRoot = null, barrier = ( // An agent or the tester never writes one. A write bumps rev (1 for a new name); `rev`, when given, // must be the current one (definition-rev-conflict). Editing or removing a definition an instance // names refuses definition-in-use: changing a hired agent's role is a change decision (A4). - // assertClean is the credential store's check, when the caller holds one. + // assertClean is the credential store's held-secret check (the bus's credentials.assertClean). A + // write that adds or changes an entry needs it: with no check to run, the write refuses (R1). function setDefinition(id, name, next, { revision, rev, actor, detail, assertClean }) { checkDefinitionName(name, "definition name"); const logged = { ...detail, definition: name, rev: null }; @@ -388,7 +389,8 @@ export function organizationWriter({ dir, rolesDir, dataRoot = null, barrier = ( if (Object.hasOwn(b, name)) { const want = current ? current.rev + 1 : 1; if (b[name]?.rev !== want) fail("definition-rev-conflict", `definition ${name} must be written at rev ${want}`); - checkDefinition(name, b[name], assertClean ? { assertClean } : {}); + if (typeof assertClean !== "function") fail("definition-secret-check-required", "a definition write needs the credential store's secret check; write it through the bus"); + checkDefinition(name, b[name], { assertClean }); } }; const out = commit(id, (old) => { diff --git a/packages/organization/tests/definitions.test.mjs b/packages/organization/tests/definitions.test.mjs index b182f2455..23598e026 100644 --- a/packages/organization/tests/definitions.test.mjs +++ b/packages/organization/tests/definitions.test.mjs @@ -274,7 +274,7 @@ test("T1 the ordinary writer cannot change definitions, and setDefinition change added.definitions.other = { rev: 1, ...PLAIN }; assert.throws(() => f.writer.write("acme", added, { revision: rev(f), actor: "jason" }), { code: "organization-definitions-protected" }); // The writer itself checks rev and the composition, whatever builds the entry. - assert.throws(() => f.writer.setDefinition("acme", "scout", () => ({ ...PLAIN, role: { ...PLAIN.role, tools: ["no-such-tool"] } }), { revision: rev(f), actor: "jason" }), { name: "BusinessError" }); + assert.throws(() => f.writer.setDefinition("acme", "scout", () => ({ ...PLAIN, role: { ...PLAIN.role, tools: ["no-such-tool"] } }), { revision: rev(f), actor: "jason", assertClean }), { name: "BusinessError" }); }); test("T1 an agent cannot call the edit verbs", (t) => { @@ -334,3 +334,18 @@ test("T1 the template files on disk are never written", (t) => { assert.deepEqual(readFileSync(join(f.rolesDir, "coder.json")), before); writeFileSync(join(f.root, "touched"), ""); }); + +test("T1 a write that adds or changes a definition refuses without the credential store's secret check", (t) => { + const f = fixture(t); + f.verbs.invent("acme", { revision: rev(f), name: "scout", definition: PLAIN }, OWNER); + const unchecked = definitionVerbs({ writer: f.writer, rolesDir: f.rolesDir }); + const before = readFileSync(f.file); + const required = { code: "definition-secret-check-required" }; + assert.throws(() => unchecked.import("acme", { revision: rev(f), template: "coder" }, OWNER), required); + assert.throws(() => unchecked.invent("acme", { revision: rev(f), name: "other", definition: PLAIN }, OWNER), required); + assert.throws(() => unchecked.edit("acme", { revision: rev(f), rev: 1, name: "scout", definition: { ...PLAIN, instructions: "Report.\n" } }, OWNER), required); + assert.throws(() => f.writer.setDefinition("acme", "other", () => PLAIN, { revision: rev(f), actor: "jason" }), required); + assert.deepEqual(readFileSync(f.file), before); + // A removal writes no role text, so it needs no secret check. + assert.equal(unchecked.remove("acme", { revision: rev(f), rev: 1, name: "scout" }, OWNER).removedRev, 1); +}); -- 2.54.0