fix(discord): check-then-read gaps name the file by label, never the path (#1873)
A token file, context file, tool root, data root or setspark key file that changes between its check and its read failed with a native error whose message carried the host path. Each read now names the file by label and code (quietRead): same error object, type and code, so exit codes, refusal codes and check order are unchanged.
This commit is contained in:
1 parent
c3fe6b77ad
commit
d9023e28db
3 files changed
+104
-8
No files matched your search
@@ -313,7 +313,13 @@ export function loadBinding(path) {
|
|||||||
// clients. It is never printed, journaled, or put on a command line.
|
// clients. It is never printed, journaled, or put on a command line.
|
||||||
export function readToken(binding) {
|
export function readToken(binding) {
|
||||||
checkPrivateFile(binding.tokenFile, "token file");
|
checkPrivateFile(binding.tokenFile, "token file");
|
||||||
const token = readFileSync(binding.tokenFile, "utf8").trim();
|
// A file that changes after its check fails with a native error that carries the path (#1873).
|
||||||
|
let token;
|
||||||
|
try {
|
||||||
|
token = readFileSync(binding.tokenFile, "utf8").trim();
|
||||||
|
} catch (e) {
|
||||||
|
throw quietRead(e, "token file");
|
||||||
|
}
|
||||||
if (!/^[A-Za-z0-9._-]{20,}$/.test(token)) throw new DiscordError("token file does not hold a bot token");
|
if (!/^[A-Za-z0-9._-]{20,}$/.test(token)) throw new DiscordError("token file does not hold a bot token");
|
||||||
return token;
|
return token;
|
||||||
}
|
}
|
||||||
@@ -345,7 +351,12 @@ export function resolveContextFiles(binding, repo) {
|
|||||||
}
|
}
|
||||||
if (st.isSymbolicLink()) throw new DiscordError(`context file must not be a symlink: ${at}`);
|
if (st.isSymbolicLink()) throw new DiscordError(`context file must not be a symlink: ${at}`);
|
||||||
if (!st.isFile() || st.size === 0) throw new DiscordError(`context file is not a regular non-empty file: ${at}`);
|
if (!st.isFile() || st.size === 0) throw new DiscordError(`context file is not a regular non-empty file: ${at}`);
|
||||||
const real = realpathSync(path);
|
let real;
|
||||||
|
try {
|
||||||
|
real = realpathSync(path);
|
||||||
|
} catch (e) {
|
||||||
|
throw quietRead(e, `context file ${at}`);
|
||||||
|
}
|
||||||
if (real !== path || !real.startsWith(root + sep)) throw new DiscordError(`context file resolves outside the repository: ${at}`);
|
if (real !== path || !real.startsWith(root + sep)) throw new DiscordError(`context file resolves outside the repository: ${at}`);
|
||||||
return path;
|
return path;
|
||||||
});
|
});
|
||||||
@@ -379,7 +390,12 @@ function webConfig(raw, where) {
|
|||||||
|
|
||||||
export function resolveToolRoots(binding, { dataRoot }) {
|
export function resolveToolRoots(binding, { dataRoot }) {
|
||||||
if (!binding.tools) return null;
|
if (!binding.tools) return null;
|
||||||
const data = existsSync(dataRoot) ? realpathSync(dataRoot) : resolve(dataRoot);
|
let data;
|
||||||
|
try {
|
||||||
|
data = existsSync(dataRoot) ? realpathSync(dataRoot) : resolve(dataRoot);
|
||||||
|
} catch (e) {
|
||||||
|
throw quietRead(e, "the data root");
|
||||||
|
}
|
||||||
const roots = binding.tools.roots.map((r) => {
|
const roots = binding.tools.roots.map((r) => {
|
||||||
let st;
|
let st;
|
||||||
try {
|
try {
|
||||||
@@ -389,7 +405,12 @@ export function resolveToolRoots(binding, { dataRoot }) {
|
|||||||
}
|
}
|
||||||
if (st.isSymbolicLink()) throw new DiscordError(`tool root ${r.name} must not be a symlink`);
|
if (st.isSymbolicLink()) throw new DiscordError(`tool root ${r.name} must not be a symlink`);
|
||||||
if (!st.isDirectory()) throw new DiscordError(`tool root ${r.name} is not a directory`);
|
if (!st.isDirectory()) throw new DiscordError(`tool root ${r.name} is not a directory`);
|
||||||
const real = realpathSync(r.path);
|
let real;
|
||||||
|
try {
|
||||||
|
real = realpathSync(r.path);
|
||||||
|
} catch (e) {
|
||||||
|
throw quietRead(e, `tool root ${r.name}`);
|
||||||
|
}
|
||||||
if (real === data || real.startsWith(data + sep) || data.startsWith(real + sep)) throw new DiscordError(`tool root ${r.name} overlaps the data root`);
|
if (real === data || real.startsWith(data + sep) || data.startsWith(real + sep)) throw new DiscordError(`tool root ${r.name} overlaps the data root`);
|
||||||
return { name: r.name, path: real, write: r.write, ...(r.git ? { git: { branch: r.git.branch, identity: r.git.identity, tokenFile: r.git.tokenFile, author: `${r.git.author.name} <${r.git.author.email}>`, ...(r.git.protocol ? { protocol: r.git.protocol } : {}) } } : {}) };
|
return { name: r.name, path: real, write: r.write, ...(r.git ? { git: { branch: r.git.branch, identity: r.git.identity, tokenFile: r.git.tokenFile, author: `${r.git.author.name} <${r.git.author.email}>`, ...(r.git.protocol ? { protocol: r.git.protocol } : {}) } } : {}) };
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -28,7 +28,7 @@
|
|||||||
import { lstatSync, readFileSync } from "node:fs";
|
import { lstatSync, readFileSync } from "node:fs";
|
||||||
import { request as httpsRequest } from "node:https";
|
import { request as httpsRequest } from "node:https";
|
||||||
import { request as httpRequest } from "node:http";
|
import { request as httpRequest } from "node:http";
|
||||||
import { plainKey } from "./errors.mjs";
|
import { plainKey, quietRead } from "./errors.mjs";
|
||||||
|
|
||||||
export const SETSPARK_DEFAULTS = Object.freeze({ timeoutMs: 15000, maxResponseBytes: 262144 });
|
export const SETSPARK_DEFAULTS = Object.freeze({ timeoutMs: 15000, maxResponseBytes: 262144 });
|
||||||
export const USER_AGENT = "mosaic-discord-sage/1 (Mosaic Stack Discord connector; setspark client)";
|
export const USER_AGENT = "mosaic-discord-sage/1 (Mosaic Stack Discord connector; setspark client)";
|
||||||
@@ -133,7 +133,14 @@ export function readKey(config) {
|
|||||||
throw new SetsparkRefusal(SETSPARK_REFUSAL.KEY_FILE);
|
throw new SetsparkRefusal(SETSPARK_REFUSAL.KEY_FILE);
|
||||||
}
|
}
|
||||||
if (st.size > KEY_MAX_BYTES) throw new SetsparkRefusal(SETSPARK_REFUSAL.KEY_SHAPE);
|
if (st.size > KEY_MAX_BYTES) throw new SetsparkRefusal(SETSPARK_REFUSAL.KEY_SHAPE);
|
||||||
const text = readFileSync(config.keyFile, "utf8");
|
// A file that changes after its checks fails with a native error that carries the path: it
|
||||||
|
// keeps its type and code and names the field (#1873).
|
||||||
|
let text;
|
||||||
|
try {
|
||||||
|
text = readFileSync(config.keyFile, "utf8");
|
||||||
|
} catch (e) {
|
||||||
|
throw quietRead(e, "keyFile");
|
||||||
|
}
|
||||||
// Either one bare key line, or the mint's own JSON output
|
// Either one bare key line, or the mint's own JSON output
|
||||||
// ({"key_id", "key", "note"}) stored as is; only "key" is used.
|
// ({"key_id", "key", "note"}) stored as is; only "key" is used.
|
||||||
let key = text;
|
let key = text;
|
||||||
|
|||||||
@@ -3,7 +3,8 @@
|
|||||||
// quotes the sentinel too; each test also checks the scratch root and the label.
|
// quotes the sentinel too; each test also checks the scratch root and the label.
|
||||||
import { test } from 'node:test';
|
import { test } from 'node:test';
|
||||||
import assert from 'node:assert/strict';
|
import assert from 'node:assert/strict';
|
||||||
import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs';
|
import fs, { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs';
|
||||||
|
import { syncBuiltinESMExports } from 'node:module';
|
||||||
import { readApprovals } from '../src/approvals.mjs';
|
import { readApprovals } from '../src/approvals.mjs';
|
||||||
import { tmpdir } from 'node:os';
|
import { tmpdir } from 'node:os';
|
||||||
import { join } from 'node:path';
|
import { join } from 'node:path';
|
||||||
@@ -11,7 +12,7 @@ import { DiscordError } from '../src/errors.mjs';
|
|||||||
import { bindingDataDir, bindingPath, loadDataRoot, readToken, resolveContextFiles, resolveToolRoots, validateBinding } from '../src/binding.mjs';
|
import { bindingDataDir, bindingPath, loadDataRoot, readToken, resolveContextFiles, resolveToolRoots, validateBinding } from '../src/binding.mjs';
|
||||||
import { ensureJournal, lockPath, ownerPath, readInboxIds, recover, requestStop, turnPath, unlock, writeTurn } from '../src/journal.mjs';
|
import { ensureJournal, lockPath, ownerPath, readInboxIds, recover, requestStop, turnPath, unlock, writeTurn } from '../src/journal.mjs';
|
||||||
import { gitPull, loadGitConfig } from '../src/git.mjs';
|
import { gitPull, loadGitConfig } from '../src/git.mjs';
|
||||||
import { loadSetsparkConfig } from '../src/setspark.mjs';
|
import { loadSetsparkConfig, readKey } from '../src/setspark.mjs';
|
||||||
|
|
||||||
const secret = 'sk-test-NOT-A-SECRET-1234';
|
const secret = 'sk-test-NOT-A-SECRET-1234';
|
||||||
function scratch(t) {
|
function scratch(t) {
|
||||||
@@ -134,3 +135,70 @@ test('a git refusal drops stderr lines that carry a host path and a spawn failur
|
|||||||
const failed = () => ({ error: Object.assign(new Error(`spawn ${join(dir, 'git')} ENOENT`), { code: 'ENOENT' }) });
|
const failed = () => ({ error: Object.assign(new Error(`spawn ${join(dir, 'git')} ENOENT`), { code: 'ENOENT' }) });
|
||||||
assert.throws(() => gitPull(rootCfg, { spawn: failed }), e => labelled('git could not start (ENOENT)', root, Error)(e));
|
assert.throws(() => gitPull(rootCfg, { spawn: failed }), e => labelled('git could not start (ENOENT)', root, Error)(e));
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Re-check (#1873): a file removed between its check and its read surfaces a native
|
||||||
|
// error. Each site names the file by label and code; the error's path never reaches the
|
||||||
|
// message. `race` swaps one node:fs function so the target goes away right before the
|
||||||
|
// real call, which then fails with a genuine ENOENT that names the path.
|
||||||
|
function race(t, fn, target, remove) {
|
||||||
|
const original = fs[fn];
|
||||||
|
const swapped = function (p, ...rest) {
|
||||||
|
if (String(p) === target) remove();
|
||||||
|
return original.call(this, p, ...rest);
|
||||||
|
};
|
||||||
|
if (original.native) swapped.native = original.native;
|
||||||
|
fs[fn] = swapped;
|
||||||
|
syncBuiltinESMExports();
|
||||||
|
t.after(() => {
|
||||||
|
fs[fn] = original;
|
||||||
|
syncBuiltinESMExports();
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
test('a token file removed between its check and its read names the token file', t => {
|
||||||
|
const { root, dir } = scratch(t);
|
||||||
|
const tokenFile = join(dir, 'token');
|
||||||
|
writeFileSync(tokenFile, 'x'.repeat(30), { mode: 0o600 });
|
||||||
|
race(t, 'readFileSync', tokenFile, () => rmSync(tokenFile));
|
||||||
|
assert.throws(() => readToken({ tokenFile }), labelled('cannot read token file (ENOENT)', root, Error));
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a context file removed between its check and its realpath names its index', t => {
|
||||||
|
const { root, dir } = scratch(t);
|
||||||
|
const repo = join(dir, 'repo');
|
||||||
|
mkdirSync(repo);
|
||||||
|
writeFileSync(join(repo, 'ok.md'), 'ok');
|
||||||
|
writeFileSync(join(repo, `${secret}.md`), 'x');
|
||||||
|
const target = join(repo, `${secret}.md`);
|
||||||
|
race(t, 'realpathSync', target, () => rmSync(target));
|
||||||
|
assert.throws(() => resolveContextFiles({ context: { files: ['ok.md', `${secret}.md`] } }, repo), labelled('cannot read context file context.files[1] (ENOENT)', root, Error));
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a tool root removed between its check and its realpath names the root', t => {
|
||||||
|
const { root, dir } = scratch(t);
|
||||||
|
const docs = join(dir, 'docs');
|
||||||
|
mkdirSync(docs);
|
||||||
|
const binding = { tools: { roots: [{ name: 'docs', path: docs, write: false }] } };
|
||||||
|
race(t, 'realpathSync', docs, () => rmSync(docs, { recursive: true }));
|
||||||
|
assert.throws(() => resolveToolRoots(binding, { dataRoot: join(dir, 'data') }), labelled('cannot read tool root docs (ENOENT)', root, Error));
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a data root removed between its check and its realpath names the data root', t => {
|
||||||
|
const { root, dir } = scratch(t);
|
||||||
|
const docs = join(dir, 'docs');
|
||||||
|
mkdirSync(docs);
|
||||||
|
const binding = { tools: { roots: [{ name: 'docs', path: docs, write: false }] } };
|
||||||
|
const data = join(dir, 'data');
|
||||||
|
mkdirSync(data);
|
||||||
|
race(t, 'realpathSync', data, () => rmSync(data, { recursive: true }));
|
||||||
|
assert.throws(() => resolveToolRoots(binding, { dataRoot: data }), labelled('cannot read the data root (ENOENT)', root, Error));
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a setspark key file removed between its check and its read names keyFile', t => {
|
||||||
|
const { root, dir } = scratch(t);
|
||||||
|
const keyFile = join(dir, 'key');
|
||||||
|
writeFileSync(keyFile, 'ssk_' + 'a'.repeat(40), { mode: 0o600 });
|
||||||
|
const config = loadSetsparkConfig({ baseUrl: 'https://setspark.example', keyFile, principal: 'sage' });
|
||||||
|
race(t, 'readFileSync', keyFile, () => rmSync(keyFile));
|
||||||
|
assert.throws(() => readKey(config), labelled('cannot read keyFile (ENOENT)', root, Error));
|
||||||
|
});
|
||||||
Reference in new issue
Block a user