fix(redaction): close independent review findings (#1873)

packages/cli/src/config.mjs

@@ -86,7 +86,7 @@ function trackerFor(system, b, warn, dataRoot) {
     if (dataRoot && managedState({ dataRoot, business: b.id, project: id, entry }) === 'reserved') continue;
     if (!existsSync(projectFilePath(entry.root))) continue;
     const project = loadProject(entry.root);
-    if (project.id !== id) throw new CliError(`project file ${project.file} has id ${project.id}, but business ${b.id} declares it as ${id}`, 3);
+    if (project.id !== id) throw new CliError(`project file ${project.file} has id that does not match the declared project key`, 3);
     const vars = resolveInstance({ system, business: b, project, instance: first }).vars;
     if (vars["tracker.project"] !== undefined) named.push({ id, vars });
   }

packages/cli/tests/last-values.test.mjs

@@ -46,7 +46,7 @@ for (const [field, edit] of configCases) test(`config ${field} omits the configu
 test('config-command refuses system config without relaying stderr file values', t => {
  const root = scratch(t), f = fixture(root), d = JSON.parse(readFileSync(f.config, 'utf8')); d.execution.model = secret + '\n'; writeFileSync(f.config, JSON.stringify(d));
  let err = ''; const status = runConfigCommand(['validate', 'acme'], { env: f.env, stdout: { write() {} }, stderr: { write(s) { err += s; } } });
- assert.equal(status, 3); assert.match(err, /system config problem \(mosaic-config exit 2\)/); assert.ok(err.includes(f.config), err); assert.doesNotMatch(err, /sk-test|NOT-A-SECRET|execution.model/);
+ assert.equal(status, 3); assert.match(err, /system config problem \(mosaic-config exit 2\)/); assert.ok(err.includes(f.config), err); assert.doesNotMatch(err, /sk-test|NOT-A-SECRET/); assert.match(err, /execution.model/); assert.match(err, /expected a 1-200 character name/);
 });
 test('project id mismatch omits the file value in config-command', t => {
  const root = scratch(t), f = fixture(root), file = join(f.doc.projects.stack.root, '.mosaic', 'project.json');
@@ -71,12 +71,21 @@ for (const mission of [false, true]) test(`duplicate ${mission ? 'mission' : 'ta
  const r = spawnSync(process.execPath, [join(repo, 'scripts/mosaic-task.mjs'), 'validate', file], { encoding: 'utf8', env: { PATH: process.env.PATH, HOME: process.env.HOME, TMPDIR: process.env.TMPDIR } });
  assert.equal(r.status, 2); assert.match(r.stderr, /duplicate tool in/); assert.doesNotMatch(r.stderr, /read/);
 });
-test('config-command does not relay arbitrary child stderr', t => {
+for (const childStderr of [secret, `mosaic-config: execution.model must match ${secret}`]) test(`config-command does not relay arbitrary child stderr${childStderr.startsWith('mosaic-config:') ? ' after a known field' : ''}`, t => {
  const root = scratch(t), file = join(root, 'config.json');
  const code = `import cp from 'node:child_process'; import { syncBuiltinESMExports } from 'node:module';
-cp.spawnSync = () => ({ status: 2, stderr: ${JSON.stringify(secret)}, stdout: '' }); syncBuiltinESMExports();
+cp.spawnSync = () => ({ status: 2, stderr: ${JSON.stringify(childStderr)}, stdout: '' }); syncBuiltinESMExports();
 const { runConfigCommand } = await import(${JSON.stringify(new URL('../../organization/src/config-command.mjs', import.meta.url).href)});
 process.exitCode = runConfigCommand(['validate', 'acme'], { env: { MOSAIC_CONFIG: ${JSON.stringify(file)} } });`;
  const r = spawnSync(process.execPath, ['--input-type=module', '-e', code], { encoding: 'utf8', env: { PATH: process.env.PATH, HOME: process.env.HOME, TMPDIR: process.env.TMPDIR } });
  assert.equal(r.status, 3, r.stderr); assert.ok(r.stderr.includes(file), r.stderr); assert.doesNotMatch(r.stderr, /sk-test|NOT-A-SECRET/);
 });
+
+import { bootConfig, loadSystem } from '../src/config.mjs';
+import { CliError } from '../src/errors.mjs';
+test('CLI boot project id mismatch omits the configured value and keeps exit 3', t => {
+ const root = scratch(t), f = fixture(root), file = join(f.doc.projects.stack.root, '.mosaic', 'project.json');
+ mkdirSync(join(f.doc.projects.stack.root, '.mosaic'));
+ writeFileSync(file, JSON.stringify({ projectVersion: 1, id: secret.toLowerCase() }));
+ assert.throws(() => bootConfig({ system: loadSystem({ env: f.env }), businessId: 'acme', env: f.env }), safe(file, CliError, { exitCode: 3 }, secret.toLowerCase()));
+});

packages/discord/src/binding.mjs

@@ -343,7 +343,7 @@ function setsparkConfig(raw, where, users) {
   const approvers = {};
   for (const u of users) {
     const name = u.name.trim().toLowerCase();
-    if (Object.hasOwn(approvers, name)) throw new DiscordError(`${where}: two users named ${name}; approver names must be distinct`);
+    if (Object.hasOwn(approvers, name)) throw new DiscordError(`${where}: two users share a name; approver names must be distinct (case-insensitive)`);
     approvers[name] = u.id;
   }
   try {

packages/discord/src/setspark.mjs

@@ -96,7 +96,7 @@ export function loadSetsparkConfig(raw, where = "setspark") {
 function loadApprovers(raw, where) {
   if (!isObject(raw) || Object.keys(raw).length > 64) throw new Error(`${where}: must be an object of at most 64 names`);
   for (const [name, id] of Object.entries(raw)) {
-    if (name.length === 0 || name.length > 64 || name !== name.trim().toLowerCase() || /[\u0000-\u001f\u007f]/.test(name)) throw new Error(`${where}: ${JSON.stringify(name.slice(0, 64))} must be a trimmed lower-case name`);
+    if (name.length === 0 || name.length > 64 || name !== name.trim().toLowerCase() || /[\u0000-\u001f\u007f]/.test(name)) throw new Error(`${where}: an approver name must be trimmed, lower-case, 1-64 characters, with no control characters`);
     if (typeof id !== "string" || !SNOWFLAKE.test(id)) throw new Error(`${where}.${name}: must be a Discord user id`);
   }
   if (new Set(Object.values(raw)).size !== Object.keys(raw).length) throw new Error(`${where}: one Discord user id under two names`);

packages/discord/tests/last-values.test.mjs

@@ -32,3 +32,12 @@ for (const mode of ['read', 'list']) test(`turn ${mode} parser omits file bytes
  const file = join(root, 'turns', 'turn-1.json'); writeFileSync(file, secret);
  assert.throws(() => mode === 'read' ? readTurn(root, 'turn-1') : listTurns(root), safe(file, SyntaxError));
 });
+
+for (const duplicate of [true, false]) test(`binding approver ${duplicate ? 'duplicate' : 'invalid'} name omits user values`, t => {
+ const root = scratch(t), keyFile = join(root, 'fixture-key'); writeFileSync(keyFile, 'fixture\n', { mode: 0o600 });
+ const name = secret.toLowerCase() + (duplicate ? '' : '\u0001');
+ const users = [{ id: '100000000000000100', name }];
+ if (duplicate) users.push({ id: '100000000000000101', name });
+ const raw = rawBinding({ users, tools: { roots: [{ name: 'docs', path: root }], setspark: { baseUrl: 'https://api.setspark.io', keyFile, principal: 'sage' } } });
+ assert.throws(() => validateBinding(raw), safe(duplicate ? 'users' : 'approvers', DiscordError, { exitCode: 2 }, secret.toLowerCase()));
+});

packages/discord/tests/setspark.test.mjs

@@ -157,7 +157,7 @@ test("setspark config: approvers come from the binding's users, never from the b
   const b = validateBinding(rawBinding({ users, tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } }));
   assert.deepEqual(b.tools.setspark.approvers, { jason: "100000000000000100", carmen: "100000000000000101" });
   assert.throws(() => validateBinding(rawBinding({ users, tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage", approvers: { mallory: "100000000000000199" } } } })), /approvers come from users/);
-  assert.throws(() => validateBinding(rawBinding({ users: [{ id: "100000000000000100", name: "Jason" }, { id: "100000000000000101", name: "jason" }], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })), /two users named jason/);
+  assert.throws(() => validateBinding(rawBinding({ users: [{ id: "100000000000000100", name: "Jason" }, { id: "100000000000000101", name: "jason" }], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } })), /two users share a name/);
   const withCarmenOut = validateBinding(rawBinding({ users: [users[0]], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } }));
   assert.throws(() => reloadDiff(b, withCarmenOut), /tools cannot change/, "pi's approvers are fixed at start, so a user change needs a restart");
   const channelsOnly = validateBinding(rawBinding({ users: [{ ...users[0], channels: [rawBinding().channels[0].id] }, users[1]], tools: { roots: [{ name: "docs", path: docs }], setspark: { baseUrl: "https://api.setspark.io", keyFile: kf, principal: "sage" } } }));

packages/organization/src/config-command.mjs

@@ -67,12 +67,19 @@ export function runConfigCommand(argv, { stdout = process.stdout, stderr = proce
       maxBuffer: 1024 * 1024,
       env,
     });
-    // Keep the parser-refusal fragment callers already recognize, without relaying
-    // any child stderr bytes (including parser excerpts or unsupported values).
+    // Render known key paths with fixed constraints; never relay child stderr.
     if (proc.status !== 0) {
-      const syntax = proc.stderr.trim().startsWith("mosaic-config: configuration is not valid JSON (")
-        ? "configuration is not valid JSON: " : "";
-      fail(3, `system config problem (mosaic-config exit ${proc.status}): ${syntax}${env.MOSAIC_CONFIG || join(env.HOME || "~", ".config", "mosaic-dev", "config.json")}${proc.error?.code ? ` (${proc.error.code})` : ""}`);
+      const problem = proc.stderr.trim();
+      const field = problem.startsWith('mosaic-config: execution "dataRoot"') ? "dataRoot"
+        : /^mosaic-config: (?:unsupported )?"?((?:configVersion|environment|dataRoot|execution|console|federation|compactionReadBytes|compactionStreams)(?:\.[A-Za-z][A-Za-z0-9]*|\[\d+\])*)"?(?=[: ])/u.exec(problem)?.[1];
+      const name = "a 1-200 character name starting with a letter or digit and containing only letters, digits, . _ : / -";
+      const expected = { configVersion: "1", environment: "development or production", dataRoot: "a canonical absolute instance data path",
+        "execution.backend": "docker", "execution.adapter": "pi or mock", "execution.provider": name, "execution.model": name };
+      const detail = problem.startsWith("mosaic-config: configuration is not valid JSON (")
+        ? "configuration is not valid JSON; expected readable JSON: "
+        : field ? `${field}: expected ${expected[field] ?? "a value satisfying the system configuration schema"}: `
+          : "expected valid system configuration: ";
+      fail(3, `system config problem (mosaic-config exit ${proc.status}): ${detail}${env.MOSAIC_CONFIG || join(env.HOME || "~", ".config", "mosaic-dev", "config.json")}${proc.error?.code ? ` (${proc.error.code})` : ""}`);
     }
     try {
       return JSON.parse(proc.stdout);

packages/queue/src/queue.mjs

@@ -311,7 +311,7 @@ function checkRound(v, n) {
 }
 
 export function validateRow(row) {
-  keysExactly(row, ROW_KEYS, `row id`);
+  keysExactly(row, ROW_KEYS, `row`);
   const w = `row ${row.id}`;
   checkId(row.id);
   checkCellText(row.piece, `${w} piece`, { max: 300 });

packages/queue/tests/last-values.test.mjs

@@ -22,7 +22,7 @@ for (const [field, fn] of [
  ['owner', () => checkName(secret, 'owner')],
  ['path', () => checkRepoPath(secret + '/', 'path')],
  ['manifest', () => parseManifest(secret + '\n')],
- ['row id', () => validateRow({ id: secret })],
+ ['row', () => validateRow({ id: secret })],
  ['version', () => loadDoc(Buffer.from(serialize({ version: secret, canonicalRoot: '/x', revision: 0, rows: [], log: [] })))],
 ]) test(`queue ${field} semantic refusal omits the value`, () => assert.throws(fn, safe(field, QueueError, { code: 2 }, secret.toLowerCase())));
 import { checkTransport, parseBriefSpec, genesisRows, parseMigrationMap, render, sha256, genesisReceipt } from '../src/queue.mjs';
@@ -123,7 +123,7 @@ test('migration map duplicate owner omits unvalidated values', () => {
  const row = structuredClone(MAP_ROWS[1]); row.owner = secret; row.reviewers = [secret];
  assert.throws(() => parseMigrationMap(mapText([row])), safe('owner', QueueError, { code: 2 }));
 });
-test('genesis expected claim omits the stored op', () => {
+test('BASE control: genesis expected claim already omits the stored op', () => {
  const d = doc(); d.log[0].op = seatValue; d.log[0].result.receipt = genesisReceipt(seatValue, d.rows.length);
  assert.throws(() => loadDoc(Buffer.from(serialize(d))), safe('claim', QueueError, { code: 2 }, seatValue));
 });