diff --git a/packages/bus/src/broker.mjs b/packages/bus/src/broker.mjs index 12d152e68..83c8a5691 100644 --- a/packages/bus/src/broker.mjs +++ b/packages/bus/src/broker.mjs @@ -280,6 +280,11 @@ const fail = (code) => { throw new BusError(code); }; const object = (x) => x !== null && typeof x === 'object' && !Array.isArray(x); +// #1873 CB5: a copy binding proposal leaves the bus only as its copy and its change. The copy id, +// incarnation, definition pin, services, grant ids and slots stay in the raise event, which the +// resolve reads; the inbox preview and the trail read carry this summary (design lines 162-163). +const bindingSummary = (v) => (object(v?.binding) + ? { ...v, binding: { instance: v.binding.instance, change: v.binding.after === null ? 'unbind' : 'bind' } } : v); // Lifecycle Task 5a: keep these pure helpers here because broker.mjs is also loaded by the // sandbox client. A new host-only import would widen its mounted source closure. const createJson = value => { @@ -2539,7 +2544,7 @@ export class Broker { options.some(o => !object(o) || !id(o.key) || !text(o.text, 1024)) || new Set(options.map(o => o.key)).size !== options.length) return null; const digest = sha256(JSON.stringify([{ ...d, options }, raise])); - if (['role.hire','role.fire','role.change'].includes(d.action)) return raise.hire ? {kind:'hire',options,...raise.hire} : null; + if (['role.hire','role.fire','role.change'].includes(d.action)) return raise.hire ? {kind:'hire',options,...bindingSummary(raise.hire)} : null; if (d.action !== 'credential.mint') return 'grant' in raise ? null : { kind: 'decision', options, digest }; if (!object(raise.grant) || typeof raise.grant.method !== 'string' || options.length !== 2) return null; const approve = options.find((o) => o.key === raise.approvalChoice); @@ -6784,7 +6789,7 @@ export class Broker { this.#event(s,'action.allowed',{operation:'decision.raise',decision:did,action,approvalChoice:'yes',target:p.name,hire:p,autonomyLevel:level},did); }); if(immediate) return this.#hireConsole(s,'hire.resolve.console',{id:did,choice:'yes',digest:p.digest}); - return {...this.#decisionView(this.#decision(s,did)),hire:p}; + return {...this.#decisionView(this.#decision(s,did)),hire:bindingSummary(p)}; } // The harness fact comes from the host's loaded modules; no host, no account coverage. #budgetCovered(b) { return authorityBudgetCovered(b,{countsAccount:this.#autonomyHost?.countsAccount}); } @@ -9483,6 +9488,7 @@ export class Broker { try { r[k] = JSON.parse(r[k]); } catch {} + if (table === 'events' && object(r.body?.hire)) r.body = { ...r.body, hire: bindingSummary(r.body.hire) }; rows.set(table + ':' + row.seq, r); } }; diff --git a/packages/bus/src/copy-launch.mjs b/packages/bus/src/copy-launch.mjs index 71746b951..3370b9f54 100644 --- a/packages/bus/src/copy-launch.mjs +++ b/packages/bus/src/copy-launch.mjs @@ -381,10 +381,13 @@ export function copyLaunchGate({ credentials, writer, configDir, rolesDir, snaps // { status, next }. The same checks a launch runs, without the loss report (the sweep makes it) // and without returning anything the check read. An approved binding not yet issued, or one a // refused issue left pending, needs the owner's issue before anything else. `entry` is the copy's - // boot entry: only one bound at boot, or unbound for want of a bindable service, has a status. + // boot entry: only one bound at boot, or unbound for want of a service, has a status. One that + // lacks a service a copy cannot bind is refused at boot whatever it binds (above), so it reads + // copy-service-unbindable and never asks for a gitea binding. status(business, instance, entry) { - const needs = Array.isArray(entry?.bound) || (entry?.state === 'unbound' && Array.isArray(entry.missing) && entry.missing.some((s) => COPY_BINDABLE.includes(s))); - if (!needs) return null; + const unbound = entry?.state === 'unbound' && Array.isArray(entry.missing) && entry.missing.length > 0; + if (!Array.isArray(entry?.bound) && !unbound) return null; + if (unbound && entry.missing.some((s) => !COPY_BINDABLE.includes(s))) return copyBindingStatus('copy-service-unbindable'); let code = 'ready'; try { check(business, instance); diff --git a/packages/bus/tests/copy-bindings.test.mjs b/packages/bus/tests/copy-bindings.test.mjs index 2a44096ac..04fd2a6d3 100644 --- a/packages/bus/tests/copy-bindings.test.mjs +++ b/packages/bus/tests/copy-bindings.test.mjs @@ -133,3 +133,40 @@ test('CB1 bus: every new refusal has a table entry with a next step', () => { assert.ok(!/(^|[\s("'`])(\/|~\/)/.test(REFUSALS[code].nextStep), code); } }); + +// #1873 CB5 (security review P2-1, P3-1): the binding record the raise holds (copy id, incarnation, +// definition pin, services, grant ids) leaves the bus only through the owner's preview verb. The +// raise answer, the inbox preview and the trail give every other session the copy and the change. +test('CB5 bus: the raise answer, the inbox preview and the trail carry a binding as its copy and change only', async t => { + const f = await fixture(t); + const d = await f.raise(); + const raised = f.store.all('SELECT body FROM events').map((e) => JSON.parse(e.body)).find((b) => b.operation === 'decision.raise' && b.decision === d.id); + const held = raised.hire.binding; + assert.equal(typeof held.copyId, 'string'); + const secrets = [held.copyId, held.orgIncarnation, held.definition?.sha256].filter((v) => typeof v === 'string' && v.length >= 8); + assert.equal(secrets.length, 3); + const summary = { instance: 'rev', change: 'bind' }; + assert.deepEqual(d.hire.binding, summary); + const sessions = { owner: f.b.bindHuman({ business: 'demo', human: 'jason', via: 'cli', outsideAgent: true }), tester: f.b.bindTester({ business: 'demo' }), reader: f.b.bindReader({ business: 'demo' }), chief: f.caps.chief, coder: f.caps.coder }; + const seen = [JSON.stringify(d)]; + for (const name of ['owner', 'tester', 'reader']) { + const rows = await f.call(sessions[name], 'inbox'); + const row = rows.find((r) => r.id === d.id); + assert.deepEqual(row?.console?.binding, summary, `${name} inbox`); + seen.push(JSON.stringify(rows)); + } + for (const name of Object.keys(sessions)) { + const trail = await f.call(sessions[name], 'trail', { subject: d.id }); + const ev = trail.find((r) => r.table === 'events' && r.body?.operation === 'decision.raise'); + assert.deepEqual(ev?.body?.hire?.binding, summary, `${name} trail`); + seen.push(JSON.stringify(trail)); + } + for (const text of seen) for (const v of secrets) assert.equal(text.includes(v), false); + // The owner's preview still has the record, and the resolve (which reads the raise) still writes it. + const preview = await f.call(f.owner, 'hire.preview.console', { id: d.id }); + assert.equal(preview.binding.copyId, held.copyId); + await f.call(f.owner, 'hire.resolve.console', { id: d.id, choice: 'yes', digest: d.hire.digest }); + assert.equal(f.bindings().rev.state, 'pending'); + const after = JSON.stringify(await f.call(sessions.tester, 'trail', { subject: d.id })); + for (const v of secrets) assert.equal(after.includes(v), false); +}); diff --git a/packages/cli/src/instances.mjs b/packages/cli/src/instances.mjs index 0f36694a9..572fd854c 100644 --- a/packages/cli/src/instances.mjs +++ b/packages/cli/src/instances.mjs @@ -12,6 +12,7 @@ import { readFileSync } from "node:fs"; import { CliError } from "./errors.mjs"; import { copyBindingStatus } from "../../organization/src/copy-binding-status.mjs"; +import { SERVICES } from "../../organization/src/vocabulary.mjs"; export const INSTANCES_USAGE = "mosaic business instances BUSINESS [--json] [--tester] | " + "business instance add BUSINESS INSTANCE --definition DEF [--vars FILE] [--launch] [--bot BOT --bot-id N] --revision REV [--tester] | " + @@ -226,7 +227,9 @@ export function formatInstances(view) { for (const [name, copy] of Object.entries(view.copies ?? {})) { if (!copy?.binding) continue; const { status, next } = copyBindingStatus(copy.binding.status); - lines.push(` ${name} (copy): ${status}. ${next}`); + // The services the copy lacks, as registered service labels only; nothing else the bus sent. + const missing = copy.state === "unbound" && Array.isArray(copy.missing) ? SERVICES.filter((s) => copy.missing.includes(s)) : []; + lines.push(` ${name} (copy): ${status}${missing.length ? ` (missing ${missing.join(", ")})` : ""}. ${next}`); } const definitions = Object.entries(view.definitions); const accepted = definitions.filter(([, d]) => d.accepted).map(([name]) => name); diff --git a/packages/cli/tests/agent-copies.test.mjs b/packages/cli/tests/agent-copies.test.mjs index 1677b53b0..d6ae04507 100644 --- a/packages/cli/tests/agent-copies.test.mjs +++ b/packages/cli/tests/agent-copies.test.mjs @@ -1089,3 +1089,23 @@ test("CB5 operator: the owner's instances read and mosaic org instances show eac assert.deepEqual(leaks(said.join("")), []); assert.deepEqual(leaks([...first.logs, ...s.logs].map((l) => (typeof l === "string" ? l : JSON.stringify(l))).join("\n")), []); }); + +// #1873 CB5 review: a shipped role that needs gitea and vikunja. Boot denies such a copy whatever it +// binds, so its status must not send the owner to approve a gitea binding. +test("CB5 operator: a copy whose role needs a service a copy cannot bind reads copy-service-unbindable and names its missing services", { skip, timeout: 60000 }, async (t) => { + const p = prepare(t); + const file = join(p.root, "config", "agents.json"); + const doc = JSON.parse(readFileSync(file, "utf8")); + doc.agents.rev = { definition: "reviewer", vars: VARS }; + writeJson(file, doc, 0o600); + const s = await start(t, p); + const v = s.call("instances"); + assert.equal(v.copies.rev.state, "unbound"); + assert.deepEqual(v.copies.rev.binding, { status: "copy-service-unbindable", next: COPY_BINDING_STATUS["copy-service-unbindable"] }); + // The gitea-only copy beside it still reads credential-unbound. + assert.deepEqual(v.copies.researcher.binding, { status: "credential-unbound", next: COPY_BINDING_STATUS["credential-unbound"] }); + const text = formatInstances(v); + assert.ok(text.includes(` rev (copy): copy-service-unbindable (missing gitea, vikunja). ${COPY_BINDING_STATUS["copy-service-unbindable"]}\n`), text); + assert.ok(text.includes(` researcher (copy): credential-unbound (missing gitea). ${COPY_BINDING_STATUS["credential-unbound"]}\n`), text); + assert.equal(COPY_BINDING_STATUS["copy-service-unbindable"].includes("credentials.gitea"), false); +}); diff --git a/packages/organization/src/copy-binding-status.mjs b/packages/organization/src/copy-binding-status.mjs index 034d02fe2..d75f40118 100644 --- a/packages/organization/src/copy-binding-status.mjs +++ b/packages/organization/src/copy-binding-status.mjs @@ -9,6 +9,8 @@ export const COPY_BINDING_STATUS = Object.freeze({ "issue-required": "The binding is approved; the owner must issue the copy's token with mosaic credentials issue, then restart the organization bus.", "restart-required": "Restart the organization bus so it verifies the binding before the copy launches.", "credential-unbound": "credentials.gitea needs an owner-approved binding.", + // Boot denies a copy whose role needs a service a copy cannot bind yet, whatever it binds. + "copy-service-unbindable": "this copy's role needs a credential service a copy cannot bind yet, so it cannot launch in this organization; binding gitea alone does not change that.", "credential-revoking": "credentials.gitea revocation is in progress; read the copy's status again when it finishes.", "credential-revoke-failed": "credentials.gitea revocation is incomplete; the owner must retry cleanup.", "credential-revoked": "credentials.gitea was revoked; the owner must approve a new binding.", diff --git a/packages/webui/src/decision-http.mjs b/packages/webui/src/decision-http.mjs index 61e13cc48..8806a53fc 100644 --- a/packages/webui/src/decision-http.mjs +++ b/packages/webui/src/decision-http.mjs @@ -64,7 +64,8 @@ export const BINDING_NEXT = Object.freeze({ unbind: "Unbind approved. The bus stops the copy's runs and revokes its credential. The copy's status on the Agents grid shows when the revoke finishes.", }); // A binding decision's preview names its copy and whether it binds or unbinds. Only those two leave -// this server: the copy id, incarnation, definition pin and services stay with the bus. +// this server: the copy id, incarnation, definition pin and services stay with the bus. The owner's +// preview verb is the one bus answer that carries them; the inbox and the trail send the same two. const bindingOf = (v) => (v?.binding && typeof v.binding === 'object' && subjectOk(v.binding.instance) && (v.binding.after === null || (typeof v.binding.after === 'object' && !Array.isArray(v.binding.after))) ? { instance: v.binding.instance, change: v.binding.after === null ? 'unbind' : 'bind' } : null); diff --git a/packages/webui/src/public/bus.js b/packages/webui/src/public/bus.js index 9b9acaed2..804c956e0 100644 --- a/packages/webui/src/public/bus.js +++ b/packages/webui/src/public/bus.js @@ -400,7 +400,7 @@ // #1873 CB5: a copy credential binding decision names its copy and whether it binds or unbinds; // the scope lines above are the bus's computed ones. The confirmation says `change`; the inbox // row carries the bus's preview, where an unbind is `after: null`. - const bindingChange = b => b.change === 'unbind' || (b.change === undefined && b.after === null) ? 'unbind' : 'bind'; + const bindingChange = b => b.change === 'unbind' ? 'unbind' : 'bind'; const bindingLine = c => c?.binding && typeof c.binding === 'object' ? `
${bindingChange(c.binding) === 'unbind' ? 'Unbind and revoke the credentials of' : 'Bind credentials for'} copy ${txt(c.binding.instance)}.