diff --git a/packages/discord/src/binding.mjs b/packages/discord/src/binding.mjs index c2e492f72..359aa2534 100644 --- a/packages/discord/src/binding.mjs +++ b/packages/discord/src/binding.mjs @@ -313,7 +313,13 @@ export function loadBinding(path) { // clients. It is never printed, journaled, or put on a command line. export function readToken(binding) { 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"); 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.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}`); return path; }); @@ -379,7 +390,12 @@ function webConfig(raw, where) { export function resolveToolRoots(binding, { dataRoot }) { 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) => { let st; 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.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`); 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 } : {}) } } : {}) }; }); diff --git a/packages/discord/src/setspark.mjs b/packages/discord/src/setspark.mjs index 7978003aa..aceabe88c 100644 --- a/packages/discord/src/setspark.mjs +++ b/packages/discord/src/setspark.mjs @@ -28,7 +28,7 @@ import { lstatSync, readFileSync } from "node:fs"; import { request as httpsRequest } from "node:https"; 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 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); } 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 // ({"key_id", "key", "note"}) stored as is; only "key" is used. let key = text; diff --git a/packages/discord/tests/path-redaction-labels.test.mjs b/packages/discord/tests/path-redaction-labels.test.mjs index 5c5615791..46a8a74e6 100644 --- a/packages/discord/tests/path-redaction-labels.test.mjs +++ b/packages/discord/tests/path-redaction-labels.test.mjs @@ -3,7 +3,8 @@ // quotes the sentinel too; each test also checks the scratch root and the label. import { test } from 'node:test'; 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 { tmpdir } from 'node:os'; 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 { ensureJournal, lockPath, ownerPath, readInboxIds, recover, requestStop, turnPath, unlock, writeTurn } from '../src/journal.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'; 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' }) }); 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)); +});