fix(cli,seat,discord,organization,control-board,harness,plugins,webui): review round: launcher reasons, session, discord reads, mosaic-config and keys by label (#1873)
Review "changes 8" fix round for org-redact6. - launcher: a harness module's throw names its system code or class, never its message; preflight, loader, bundle, checkout, sandbox and route text is relayed with host paths replaced (relay()). - claude-code and pi preflights name MOSAIC_CLAUDE_BIN and the repository's node_modules, never a path; persona and skill bundle errors and sandboxProblem name their file or the sandbox by label. - seat session: thread store, legacy store, overlap, busy lock and newRun refusals name the place under the data root. - discord: unknown keys, binding names and turn ids are described when not plain; approvals, journal and repository read errors name the file and code; git drops stderr lines that carry a host path and a spawn failure names its code only. - seat, control-board: registration fields and seen keys are described when not plain; invalid seat name and layout state the constraint. - organization: the registry names the config directory and registry file by label; --project is quoted only when it is a plain name; systemConfigProblem moves to its own module. - scripts/mosaic-config.mjs: the config by where it came from, the data root and its marker by role, keys and arguments only when plain, and error codes in place of raw messages. Exit codes unchanged. - webui sso-config: an unknown console key is named only when plain. Sentinel tests in each package's path-redaction-labels file fail on BASE; pins that quoted a path now assert the label and its absence.
This commit is contained in:
1 parent
f3f33a6171
commit
87db40e0cd
37 files changed
+513
-179
No files matched your search
+37
-27
@@ -67,6 +67,16 @@ function fail(exitCode, message) {
|
||||
throw new Failure(exitCode, message);
|
||||
}
|
||||
|
||||
// #1873: a refusal names the config by where it came from, the data root and its marker by
|
||||
// their roles, and a key or argument only when it is a plain name; never a host path or a raw
|
||||
// error message. CONFIG is set when the selection is made.
|
||||
let CONFIG = "the default system config";
|
||||
function configLabel(instance) {
|
||||
return isNamed(instance) ? `the config of instance ${instance}` : process.env.MOSAIC_CONFIG ? "the system config MOSAIC_CONFIG names" : "the default system config";
|
||||
}
|
||||
const plain = (value, otherwise) => /^[A-Za-z_][A-Za-z0-9_.-]{0,63}$/.test(String(value)) ? JSON.stringify(String(value)) : otherwise;
|
||||
const MARKER_LABEL = "the data root's instance marker";
|
||||
|
||||
function instancePath(name) {
|
||||
return path.join(os.homedir(), ".config", `mosaic-${name}`, "config.json");
|
||||
}
|
||||
@@ -82,6 +92,7 @@ function realOrNull(file) {
|
||||
// The config file for a selection. Without --instance, as before. With it,
|
||||
// NAME's file; a $MOSAIC_CONFIG beside it must be the same file.
|
||||
function configPath(instance) {
|
||||
CONFIG = configLabel(instance);
|
||||
if (instance === null) {
|
||||
return process.env.MOSAIC_CONFIG
|
||||
? path.resolve(process.env.MOSAIC_CONFIG)
|
||||
@@ -92,7 +103,7 @@ function configPath(instance) {
|
||||
const given = path.resolve(process.env.MOSAIC_CONFIG);
|
||||
const same = given === named || (realOrNull(given) !== null && realOrNull(given) === realOrNull(named));
|
||||
if (!same) {
|
||||
fail(4, `instance-config-mismatch: --instance ${instance} names ${named}, but MOSAIC_CONFIG names ${given}`);
|
||||
fail(4, `instance-config-mismatch: --instance ${instance} names its own config, but MOSAIC_CONFIG names another file`);
|
||||
}
|
||||
}
|
||||
return named;
|
||||
@@ -106,7 +117,7 @@ function parseArgs(args) {
|
||||
}
|
||||
const name = args[1];
|
||||
if (!INSTANCE_PATTERN.test(name)) {
|
||||
fail(4, `instance-name-invalid: ${JSON.stringify(name)} must match ${INSTANCE_PATTERN}`);
|
||||
fail(4, `instance-name-invalid: ${plain(name, "that name")} must match ${INSTANCE_PATTERN}`);
|
||||
}
|
||||
return name;
|
||||
}
|
||||
@@ -124,20 +135,20 @@ function readMarker(dataRoot) {
|
||||
stat = fs.lstatSync(file);
|
||||
} catch (error) {
|
||||
if (error.code === "ENOENT") return null;
|
||||
fail(3, `instance-root-unclaimed: cannot read ${file} (${error.code})`);
|
||||
fail(3, `instance-root-unclaimed: cannot read ${MARKER_LABEL} (${error.code})`);
|
||||
}
|
||||
if (!stat.isFile() || stat.isSymbolicLink()) {
|
||||
fail(3, `instance-root-unclaimed: ${file} must be a regular, non-symbolic-link file`);
|
||||
fail(3, `instance-root-unclaimed: ${MARKER_LABEL} must be a regular, non-symbolic-link file`);
|
||||
}
|
||||
let marker;
|
||||
try {
|
||||
marker = JSON.parse(fs.readFileSync(file, "utf8"));
|
||||
} catch (error) {
|
||||
fail(3, `instance-root-unclaimed: ${file} is not a valid marker (${error instanceof SyntaxError ? 'not valid JSON' : error.message})`);
|
||||
fail(3, `instance-root-unclaimed: ${MARKER_LABEL} is not a valid marker (${error instanceof SyntaxError ? 'not valid JSON' : `cannot read it, ${error.code ?? "error"}`})`);
|
||||
}
|
||||
if (!isPlainObject(marker) || Object.keys(marker).sort().join(",") !== "instanceVersion,name"
|
||||
|| marker.instanceVersion !== 1 || typeof marker.name !== "string" || !INSTANCE_PATTERN.test(marker.name)) {
|
||||
fail(3, `instance-root-unclaimed: ${file} is not a valid marker ({"instanceVersion":1,"name":NAME})`);
|
||||
fail(3, `instance-root-unclaimed: ${MARKER_LABEL} is not a valid marker ({"instanceVersion":1,"name":NAME})`);
|
||||
}
|
||||
return marker.name;
|
||||
}
|
||||
@@ -148,13 +159,13 @@ function checkClaim(dataRoot, instance) {
|
||||
const owner = readMarker(dataRoot);
|
||||
if (isNamed(instance)) {
|
||||
if (owner === null) {
|
||||
fail(3, `instance-root-unclaimed: ${dataRoot} holds no ${MARKER} naming ${instance} (scripts/bootstrap.sh --instance ${instance} claims a new root)`);
|
||||
fail(3, `instance-root-unclaimed: the data root holds no ${MARKER} naming ${instance} (scripts/bootstrap.sh --instance ${instance} claims a new root)`);
|
||||
}
|
||||
if (owner !== instance) {
|
||||
fail(3, `instance-root-mismatch: ${dataRoot} is claimed by instance ${owner}, not ${instance}`);
|
||||
fail(3, `instance-root-mismatch: the data root is claimed by instance ${owner}, not ${instance}`);
|
||||
}
|
||||
} else if (owner !== null && owner !== DEFAULT_INSTANCE) {
|
||||
fail(3, `instance-root-mismatch: ${dataRoot} is claimed by instance ${owner}; select it with --instance ${owner}`);
|
||||
fail(3, `instance-root-mismatch: the data root is claimed by instance ${owner}; select it with --instance ${owner}`);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -166,19 +177,19 @@ function readRaw(file) {
|
||||
return null; // missing
|
||||
}
|
||||
if (!stat.isFile() || stat.isSymbolicLink()) {
|
||||
fail(2, `configuration path must be a regular, non-symbolic-link file: ${file}`);
|
||||
fail(2, `configuration path must be a regular, non-symbolic-link file: ${CONFIG}`);
|
||||
}
|
||||
try {
|
||||
return fs.readFileSync(file, "utf8");
|
||||
} catch {
|
||||
fail(2, `configuration file is not readable: ${file}`);
|
||||
fail(2, `configuration file is not readable: ${CONFIG}`);
|
||||
}
|
||||
}
|
||||
|
||||
function rejectUnknownKeys(object, allowed, where) {
|
||||
for (const key of Object.keys(object)) {
|
||||
if (!allowed.includes(key)) {
|
||||
fail(2, `unsupported ${where} key: "${key}"`);
|
||||
fail(2, `unsupported ${where} key: ${plain(key, "that is not a plain name")}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -206,11 +217,11 @@ function validateDataRoot(value, file) {
|
||||
}
|
||||
const home = path.resolve(os.homedir());
|
||||
if (resolved === home || isAncestorOf(resolved, home)) {
|
||||
fail(2, `execution "dataRoot" must not be or contain the home directory (${home})`);
|
||||
fail(2, 'execution "dataRoot" must not be or contain the home directory');
|
||||
}
|
||||
const configDir = path.dirname(path.resolve(file));
|
||||
if (resolved === configDir || isAncestorOf(resolved, configDir)) {
|
||||
fail(2, `execution "dataRoot" must not be or contain the configuration directory (${configDir})`);
|
||||
fail(2, 'execution "dataRoot" must not be or contain the configuration directory');
|
||||
}
|
||||
return resolved;
|
||||
}
|
||||
@@ -378,14 +389,13 @@ async function load(file, instance = null) {
|
||||
if (raw === null) {
|
||||
const bootstrap = isNamed(instance) ? `scripts/bootstrap.sh --instance ${instance}` : "scripts/bootstrap.sh";
|
||||
// #1873: named by where it came from, never by host path.
|
||||
const which = isNamed(instance) ? `the config of instance ${instance}` : process.env.MOSAIC_CONFIG ? "the system config MOSAIC_CONFIG names" : "the default system config";
|
||||
fail(3, `configuration not found: ${which} (run ${bootstrap} to create it)`);
|
||||
fail(3, `configuration not found: ${configLabel(instance)} (run ${bootstrap} to create it)`);
|
||||
}
|
||||
let document;
|
||||
try {
|
||||
document = JSON.parse(raw);
|
||||
} catch {
|
||||
fail(2, `configuration is not valid JSON (${file})`);
|
||||
fail(2, `configuration is not valid JSON (${CONFIG})`);
|
||||
}
|
||||
return await validate(document, file);
|
||||
}
|
||||
@@ -411,7 +421,7 @@ const DEFAULT_CONFIG = defaultConfig(path.join(os.homedir(), ".mosaic-dev"));
|
||||
|
||||
// 'wx': creation is exclusive; an existing file is never overwritten, and a
|
||||
// symbolic link at the path is never followed.
|
||||
function createExclusive(file, text, label = file) {
|
||||
function createExclusive(file, text, label) {
|
||||
let fd;
|
||||
try {
|
||||
fd = fs.openSync(file, "wx", 0o644);
|
||||
@@ -419,7 +429,7 @@ function createExclusive(file, text, label = file) {
|
||||
return true;
|
||||
} catch (error) {
|
||||
if (error.code === "EEXIST") return false;
|
||||
fail(1, `unable to create ${label}: ${error.message}`);
|
||||
fail(1, `unable to create ${label} (${error.code ?? "error"})`);
|
||||
} finally {
|
||||
if (fd !== undefined) fs.closeSync(fd);
|
||||
}
|
||||
@@ -454,23 +464,23 @@ async function bootstrapNamed(file, instance) {
|
||||
let entries = null;
|
||||
try {
|
||||
const stat = fs.lstatSync(dataRoot);
|
||||
if (!stat.isDirectory()) fail(3, `instance-root-unclaimed: ${dataRoot} exists and is not a directory; nothing was written`);
|
||||
if (!stat.isDirectory()) fail(3, `instance-root-unclaimed: the instance's data root exists and is not a directory; nothing was written`);
|
||||
entries = fs.readdirSync(dataRoot);
|
||||
} catch (error) {
|
||||
if (error instanceof Failure) throw error;
|
||||
if (error.code !== "ENOENT") fail(3, `instance-root-unclaimed: cannot read ${dataRoot} (${error.code}); nothing was written`);
|
||||
if (error.code !== "ENOENT") fail(3, `instance-root-unclaimed: cannot read the instance's data root (${error.code}); nothing was written`);
|
||||
}
|
||||
if (entries !== null && entries.length > 0 && readMarker(dataRoot) !== instance) {
|
||||
fail(3, `instance-root-unclaimed: ${dataRoot} exists, is not empty and holds no ${MARKER} naming ${instance}; nothing was written`);
|
||||
fail(3, `instance-root-unclaimed: the instance's data root exists, is not empty and holds no ${MARKER} naming ${instance}; nothing was written`);
|
||||
}
|
||||
const configDir = path.dirname(file);
|
||||
fs.mkdirSync(path.dirname(configDir), { recursive: true });
|
||||
try {
|
||||
fs.mkdirSync(configDir, { mode: 0o700 });
|
||||
} catch (error) {
|
||||
if (error.code !== "EEXIST") fail(1, `unable to create ${configDir}: ${error.message}`);
|
||||
if (error.code !== "EEXIST") fail(1, `unable to create the config directory of instance ${instance} (${error.code ?? "error"})`);
|
||||
}
|
||||
if (!createExclusive(file, `${JSON.stringify(defaultConfig(dataRoot), null, 2)}\n`)) {
|
||||
if (!createExclusive(file, `${JSON.stringify(defaultConfig(dataRoot), null, 2)}\n`, CONFIG)) {
|
||||
const resolved = await load(file, instance);
|
||||
checkClaim(resolved.dataRoot, instance);
|
||||
return;
|
||||
@@ -479,9 +489,9 @@ async function bootstrapNamed(file, instance) {
|
||||
try {
|
||||
fs.mkdirSync(resolved.dataRoot, { mode: 0o700 });
|
||||
} catch (error) {
|
||||
if (error.code !== "EEXIST") fail(1, `unable to create ${resolved.dataRoot}: ${error.message}`);
|
||||
if (error.code !== "EEXIST") fail(1, `unable to create the data root (${error.code ?? "error"})`);
|
||||
}
|
||||
createExclusive(path.join(resolved.dataRoot, MARKER), `${JSON.stringify({ instanceVersion: 1, name: instance })}\n`);
|
||||
createExclusive(path.join(resolved.dataRoot, MARKER), `${JSON.stringify({ instanceVersion: 1, name: instance })}\n`, MARKER_LABEL);
|
||||
checkClaim(resolved.dataRoot, instance);
|
||||
process.stderr.write(`mosaic-config: created configuration for instance ${instance}: ${file}, data root ${resolved.dataRoot}\n`);
|
||||
}
|
||||
@@ -533,7 +543,7 @@ const operation = process.argv[2];
|
||||
|
||||
try {
|
||||
if (!["bootstrap", "validate", "env", "instance"].includes(operation)) {
|
||||
fail(1, `unknown operation: ${JSON.stringify(operation ?? "")} (expected bootstrap | validate | env | instance)`);
|
||||
fail(1, `unknown operation: ${plain(operation ?? "", "that is not one")} (expected bootstrap | validate | env | instance)`);
|
||||
}
|
||||
const instance = parseArgs(process.argv.slice(3));
|
||||
switch (operation) {
|
||||
|
||||
Reference in new issue
Block a user