diff --git a/.changeset/safe-local-file-reads.md b/.changeset/safe-local-file-reads.md new file mode 100644 index 00000000..94ef4602 --- /dev/null +++ b/.changeset/safe-local-file-reads.md @@ -0,0 +1,5 @@ +--- +"nansen-cli": patch +--- + +Reject symlinks, hard links, and replaced files when reading saved authentication and trading quotes. Validate authentication lock descriptors before use. Claim bridge quotes before signing and block reuse if a post-broadcast execution marker cannot be saved. Authentication reads now require user-owned paths without group or other write permissions on POSIX; repair permissions before retrying an unsafe saved-key read. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3c9cf643..1ed39e2a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -68,6 +68,24 @@ jobs: - name: Run tests run: npm test + local-file-windows: + runs-on: windows-latest + permissions: + contents: read + strategy: + matrix: + node-version: [22, 24] + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: ${{ matrix.node-version }} + cache: 'npm' + - run: npm ci + - run: npx vitest run src/__tests__/local-file-security.test.js src/__tests__/local-file-platform.test.js + native-optional-install: name: "Native auth on ${{ matrix.image }}: ${{ matrix.outcome }}" runs-on: ubuntu-latest @@ -112,7 +130,7 @@ jobs: printf '%s\n' "- $NANSEN_IMAGE: $NANSEN_AUTH_OUTCOME. Confirmed with published 2.0.0 and the PR tarball." >> "$GITHUB_STEP_SUMMARY" publish: - needs: [lint, test, native-optional-install] + needs: [lint, test, local-file-windows, native-optional-install] runs-on: ubuntu-latest if: github.ref == 'refs/heads/main' && github.event_name == 'push' outputs: diff --git a/docs/local-file-security.md b/docs/local-file-security.md new file mode 100644 index 00000000..ddec0236 --- /dev/null +++ b/docs/local-file-security.md @@ -0,0 +1,29 @@ +# Managed local file reads + +Authentication and trading storage must contain regular files. The CLI refuses symlinked files, hard links, and symlinked directories below its storage root. Trading and cache storage may use an intentionally symlinked root; its target identity is checked again after opening. Authentication requires a link-free root as well. Authentication also rejects files and directories owned by another user or writable by group or other users on POSIX platforms. + +`local-file.js` opens with `O_NOFOLLOW` and `O_NONBLOCK` where the platform provides them. It checks the opened descriptor's type, link count, identity, and applicable permissions before reading. It compares the descriptor with the path and checks the containing directories again. A rejected descriptor closes immediately. An unlocked authentication config read retries once after an inode change caused by an atomic update, repeating every check before reading; continued replacement still fails closed. Authentication journals keep their existing 4 KiB limit, measured on the opened descriptor. + +These checks do not sandbox a process already running as the user's OS identity. Such a process can edit a regular file in place, change HOME, or manipulate ancestor directories. Directory comparisons detect ordinary replacements but do not provide descriptor-relative traversal like `openat`. Home ancestors may legitimately be symlinks, as with `/var` on macOS. On Windows, POSIX ownership and mode checks do not apply and `O_NOFOLLOW` may be absent; the path and descriptor comparisons still run. + +## API-733 paths + +| Path | Source and validation | +| --- | --- | +| Authentication config | Fixed `config.json` below the absolute HOME/USERPROFILE directory. Development config is a package-local fallback. A present but unsafe user config fails without selecting the development config. An explicitly supplied environment API key retains precedence. | +| Authentication journals | Fixed `auth-operations` directory, UUID filenames, journal schema validation, and descriptor checks before JSON parsing. | +| Authentication locks | Fixed lock names or UUID journal lock names. Descriptor checks run before native locking. The secure-store worker accepts an absolute directory from its parent and independently validates its lock. | +| Cache inspection | Fixed cache namespaces. Response entries must have 64 hex digits and `.json`. Descriptor checks run before timestamp extraction. Unsafe entries are skipped. Statistics never return cached payloads. | +| Trading and bridge quotes | Quote IDs and transaction hashes form a single filename component. Separators, colons, NUL, and traversal are rejected. Reads, execution-claim release, marker reads, and cleanup reads use descriptor checks. | + +Explicit file import options are user-authorized reads and retain their documented behavior. API-733 concerns managed state, where following a link could load unrelated data as credentials or transaction input. Authentication locks and quote-marker writes also validate their descriptors before use. Other writes remain outside this hardening. + +Bridge execution shares the swap quote claim. Before signing, it renames the quote to `.executing.json`, flushes the directory on POSIX, and validates the claimed data against the reviewed quote. A definite pre-broadcast rejection releases the claim. A successful or uncertain broadcast requires a flushed execution marker before release. If the marker cannot be read, written, or flushed, execution stops and the claim stays in place. Check bridge status before requesting a new quote. Dry runs and declined confirmation do not claim or consume a quote. + +## Verification + +`src/__tests__/local-file-security.test.js` exercises real temporary files and production entry points. It checks symlinks, hard links, directory redirection, path traversal, a file replacement during open, journal limits, lock substitution, and quote claims. It asserts that rejected targets never reach `readFileSync`. The secure-store test starts the real guard process and confirms that a linked lock never reaches readiness. + +The existing auth recovery, process lifetime, trading, bridge, and cache suites cover normal operation. Bridge regression tests inject marker read, write, and flush failures after a mocked broadcast and verify that a second attempt never signs. They also check concurrent execution and preservation of pre-broadcast retries. No real credential store, funded wallet, or production API is used for the attack fixtures. + +Windows CI runs these filesystem fixtures and a real CLI saved-key research request followed by quote creation and a dry run on Node 22 and 24, using only a loopback mock server. diff --git a/src/__tests__/auth-store-lifetime.test.js b/src/__tests__/auth-store-lifetime.test.js index 16f55421..09d017e2 100644 --- a/src/__tests__/auth-store-lifetime.test.js +++ b/src/__tests__/auth-store-lifetime.test.js @@ -21,7 +21,7 @@ async function until(test, timeout = 5000) { function harness() { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'auth-lifetime-')); roots.push(root); fs.mkdirSync(path.join(root, 'entries')); - for (const file of ['auth-state.js', 'auth-device.js', 'auth-store.js', 'auth-store-worker.js', 'auth-credentials.js']) fs.copyFileSync(path.join(src, file), path.join(root, file)); + for (const file of ['auth-state.js', 'auth-device.js', 'auth-store.js', 'auth-store-worker.js', 'auth-credentials.js', 'local-file.js']) fs.copyFileSync(path.join(src, file), path.join(root, file)); fs.writeFileSync(path.join(root, 'auth-store-worker.js'), `import ${JSON.stringify(guard)};\n` + fs.readFileSync(path.join(root, 'auth-store-worker.js'), 'utf8')); fs.writeFileSync(path.join(root, 'package.json'), '{"type":"module"}'); fs.symlinkSync(path.join(src, '../node_modules'), path.join(root, 'node_modules'), process.platform === 'win32' ? 'junction' : 'dir'); diff --git a/src/__tests__/bridge-broadcast-failclosed.test.js b/src/__tests__/bridge-broadcast-failclosed.test.js index 1772f346..17a73b43 100644 --- a/src/__tests__/bridge-broadcast-failclosed.test.js +++ b/src/__tests__/bridge-broadcast-failclosed.test.js @@ -46,7 +46,7 @@ import os from 'os'; import path from 'path'; import { exportWallet, getWalletConfig, showWallet } from '../wallet.js'; -import { buildBridgeCommands } from '../bridge.js'; +import { buildBridgeCommands, loadBridgeQuote } from '../bridge.js'; const ADDR = '0x' + 'ab'.repeat(20); // Real Base -> Hyperliquid deposit router/selector — the preflight rejects @@ -128,10 +128,63 @@ describe('bridge EVM broadcast fail-closed', () => { }); afterEach(() => { + vi.restoreAllMocks(); process.env.HOME = prevHome; fs.rmSync(tmpHome, { recursive: true, force: true }); }); + it.each(['read', 'write', 'flush'])('keeps the quote claimed when the post-broadcast marker %s fails', async failure => { + const quoteId = `bridge-marker-${failure}`; + writeQuote(quoteId); + const claimed = path.join(quotesDir, `${quoteId}.executing.json`); + const extraLink = path.join(tmpHome, 'linked-quote.json'); + const sync = fs.fsyncSync.bind(fs); + evmRpcCall.mockImplementation(async (_chain, method) => { + if (method === 'eth_getBlockByNumber') return { baseFeePerGas: '0x1' }; + if (method === 'eth_sendRawTransaction') { + if (failure === 'read') fs.linkSync(claimed, extraLink); + if (failure === 'write') vi.spyOn(fs, 'writeSync').mockImplementation(() => { throw new Error('fixture write failure'); }); + if (failure === 'flush') vi.spyOn(fs, 'fsyncSync').mockImplementation(fd => { + if (fs.fstatSync(fd).isFile()) throw new Error('fixture flush failure'); + return sync(fd); + }); + return '0xbroadcast'; + } + return '0x0'; + }); + const cmds = buildBridgeCommands({ log: () => {} }); + await expect(cmds.execute([], api, {}, { quote: quoteId, wallet: 'w' })) + .rejects.toMatchObject({ code: 'QUOTE_STATE_UNSAFE' }); + vi.restoreAllMocks(); + if (fs.existsSync(extraLink)) fs.unlinkSync(extraLink); + expect(fs.existsSync(claimed)).toBe(true); + expect(fs.existsSync(path.join(quotesDir, `${quoteId}.json`))).toBe(false); + expect(() => loadBridgeQuote(quoteId)).toThrow('claimed'); + signEvmTransaction.mockClear(); + await expect(cmds.execute([], api, {}, { quote: quoteId, wallet: 'w' })).rejects.toThrow('claimed'); + expect(signEvmTransaction).not.toHaveBeenCalled(); + expect(evmRpcCall.mock.calls.filter(([, method]) => method === 'eth_sendRawTransaction')).toHaveLength(1); + }); + + it('refuses a simultaneous execute while the first broadcast is awaiting its receipt', async () => { + const quoteId = 'bridge-concurrent'; writeQuote(quoteId); + evmRpcCall.mockImplementation(async (_chain, method) => method === 'eth_getBlockByNumber' ? { baseFeePerGas: '0x1' } : '0xbroadcast'); + let entered, finish; + const waiting = new Promise(resolve => { entered = resolve; }); + waitForReceipt.mockImplementationOnce(() => { + entered(); + return new Promise(resolve => { finish = () => resolve({ status: '0x1' }); }); + }); + const cmds = buildBridgeCommands({ log: () => {} }); + const first = cmds.execute([], api, {}, { quote: quoteId, wallet: 'w' }); + await waiting; + try { + await expect(cmds.execute([], api, {}, { quote: quoteId, wallet: 'w' })).rejects.toThrow('claimed'); + expect(signEvmTransaction).toHaveBeenCalledTimes(1); + } finally { finish(); await first; } + expect(() => loadBridgeQuote(quoteId)).toThrow('already executed'); + }); + it('marks the quote spent on an AMBIGUOUS send failure (non-JSON 502) and refuses a re-execute', async () => { stubSend(Object.assign(new Error('RPC endpoint returned non-JSON response (HTTP 502)'), { code: 'RPC_HTTP_ERROR', status: 502 })); const cmds = buildBridgeCommands({ log: () => {} }); diff --git a/src/__tests__/local-file-platform.test.js b/src/__tests__/local-file-platform.test.js new file mode 100644 index 00000000..2c9db1cc --- /dev/null +++ b/src/__tests__/local-file-platform.test.js @@ -0,0 +1,66 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import http from 'node:http'; +import { spawn } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; +import { expect, it, vi } from 'vitest'; +import { createWallet } from '../wallet.js'; + +it('uses a saved API key and reads a CLI-generated quote through a dry run', async () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'nansen-platform-')); + const requests = []; + const quote = { success: true, quotes: [{ + aggregator: 'lifi', inputMint: '0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee', + outputMint: '0x833589fcd6edb6e08f4c7c32d4f71b54bda02913', + inAmount: '1000000000000000000', outAmount: '3000000000', + transaction: { to: '0x1231deb6f5749ef6ce6943a275a1d3e7486f4eae', data: '0x12345678', + value: '1000000000000000000', gas: '100000', maxFeePerGas: '1000000', maxPriorityFeePerGas: '1000000' }, + }] }; + const server = http.createServer(async (req, res) => { + let raw = ''; for await (const chunk of req) raw += chunk; + const body = raw ? JSON.parse(raw) : {}; + requests.push({ url: req.url, key: req.headers.apikey, method: body.method }); + let response = { data: [] }; + if (req.url.startsWith('/quote')) response = quote; + else if (req.url.includes('/sanctions/screen')) response = { results: body.addresses.map(address => ({ address, sanctioned: false })) }; + else if (body.method) { + const results = { eth_getCode: '0x6080604052', eth_call: '0x', eth_estimateGas: '0x186a0', + eth_getBalance: '0x100000000000000000000', eth_getTransactionCount: '0x5', eth_getBlockByNumber: { baseFeePerGas: '0x1' } }; + response = { jsonrpc: '2.0', id: body.id || 1, result: results[body.method] ?? null }; + } + res.setHeader('Content-Type', 'application/json'); res.end(JSON.stringify(response)); + }); + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); + const origin = `http://127.0.0.1:${server.address().port}`; + const env = { ...process.env, HOME: home, USERPROFILE: home, NANSEN_BASE_URL: origin, + NANSEN_TRADING_API_URL: origin, NANSEN_BASE_RPC: origin, NANSEN_BASE_SIM_RPC: origin, + NANSEN_NO_TELEMETRY: '1', DO_NOT_TRACK: '1', NO_UPDATE_NOTIFIER: '1' }; + delete env.NANSEN_API_KEY; + async function cli(args) { + const child = spawn(process.execPath, [fileURLToPath(new URL('../index.js', import.meta.url)), ...args], { env }); + let out = ''; child.stdout.on('data', chunk => { out += chunk; }); child.stderr.on('data', chunk => { out += chunk; }); + const timer = setTimeout(() => child.kill(), 20000); + try { + const code = await new Promise((resolve, reject) => { child.once('exit', resolve); child.once('error', reject); }); + expect(code, out).toBe(0); return out; + } finally { clearTimeout(timer); } + } + try { + vi.stubEnv('HOME', home); + createWallet('smoke', 'fixture-password'); + fs.writeFileSync(path.join(home, '.nansen', 'config.json'), JSON.stringify({ apiKey: 'fixture-saved-key' }), { mode: 0o600 }); + await cli(['research', 'token', 'screener', '--chain', 'base']); + expect(requests.some(req => req.key === 'fixture-saved-key')).toBe(true); + const output = await cli(['trade', 'quote', '--chain', 'base', '--from', 'ETH', '--to', 'USDC', '--amount', '1000000000000000000', '--wallet', 'smoke']); + const id = output.match(/Quote ID:\s+(\S+)/)?.[1]; expect(id).toBeTruthy(); + const plan = await cli(['trade', 'execute', '--quote', id, '--dry-run']); + expect(plan).toContain('Trade plan'); + expect(fs.existsSync(path.join(home, '.nansen', 'quotes', `${id}.json`))).toBe(true); + expect(requests.some(req => req.url.startsWith('/execute') || req.method === 'eth_sendRawTransaction')).toBe(false); + } finally { + vi.unstubAllEnvs(); server.closeAllConnections(); + await new Promise(resolve => server.close(resolve)); + fs.rmSync(home, { recursive: true, force: true }); + } +}); diff --git a/src/__tests__/local-file-security.test.js b/src/__tests__/local-file-security.test.js new file mode 100644 index 00000000..f8a2ed9d --- /dev/null +++ b/src/__tests__/local-file-security.test.js @@ -0,0 +1,275 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { randomUUID } from 'node:crypto'; +import { spawnSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { openLocalFile, readLocalFile } from '../local-file.js'; +import { readAuthConfig, resolveCredential } from '../auth-credentials.js'; +import { createAuthState, renewalStatus } from '../auth-state.js'; +import { claimQuoteForExecution, getQuotesDir, loadQuote, loadTxRecord, markQuoteExecuted, safeQuotesPath, saveQuote, saveTxRecord } from '../trading.js'; +import { loadBridgeQuote } from '../bridge.js'; + +let home, root; +beforeEach(() => { + home = fs.mkdtempSync(path.join(os.tmpdir(), 'nansen-file-security-')); + root = path.join(home, '.nansen'); + fs.mkdirSync(root, { mode: 0o700 }); + vi.stubEnv('HOME', home); +}); +afterEach(() => { + vi.restoreAllMocks(); vi.unstubAllEnvs(); + fs.rmSync(home, { recursive: true, force: true }); +}); +function write(file, value) { + fs.mkdirSync(path.dirname(file), { recursive: true, mode: 0o700 }); + fs.writeFileSync(file, JSON.stringify(value), { mode: 0o600 }); +} +function config() { return path.join(root, 'config.json'); } +function snapshot() { return readAuthConfig({ HOME: home }, path.join(home, 'dev-config.json')); } + +describe('managed authentication files', () => { + it('preserves saved-key and explicit-key precedence for ordinary files', () => { + write(config(), { apiKey: 'saved-key', setting: 42 }); + expect(resolveCredential({ env: {}, snapshot: snapshot() })).toMatchObject({ kind: 'api-key', apiKey: 'saved-key' }); + expect(resolveCredential({ env: { NANSEN_API_KEY: 'env-key' }, snapshot: snapshot() })).toMatchObject({ apiKey: 'env-key' }); + expect(snapshot().config.setting).toBe(42); + }); + + it.each(['symlink', 'hardlink', 'dangling'])('rejects a %s config without falling back to a development key', kind => { + const outside = path.join(home, 'secret.json'); + write(outside, { apiKey: 'outside-secret' }); + write(path.join(home, 'dev-config.json'), { apiKey: 'dev-key' }); + if (kind === 'hardlink') fs.linkSync(outside, config()); + else fs.symlinkSync(kind === 'dangling' ? path.join(home, 'missing') : outside, config()); + const result = snapshot(); + expect(result).toMatchObject({ config: {}, configFileExists: true, configError: 'unreadable' }); + expect(resolveCredential({ env: {}, snapshot: result }).kind).toBe('invalid'); + }); + + it('rejects a symlinked storage directory', () => { + const outside = path.join(home, 'outside'); + write(path.join(outside, 'config.json'), { apiKey: 'outside-secret' }); + fs.rmdirSync(root); fs.symlinkSync(outside, root, 'dir'); + expect(snapshot().configError).toBe('unreadable'); + }); + + it.skipIf(process.platform === 'win32')('rejects a writable config and an unexpected file owner', () => { + write(config(), { apiKey: 'saved-key' }); + fs.chmodSync(config(), 0o666); + expect(snapshot().configError).toBe('unreadable'); + fs.chmodSync(config(), 0o600); + vi.spyOn(process, 'getuid').mockReturnValue(process.getuid() + 1); + expect(snapshot().configError).toBe('unreadable'); + }); + + it.each(['symlink', 'replacement', 'hardlink'])('reads no secret bytes when config changes to a %s at open', kind => { + write(config(), { apiKey: 'saved-key' }); + const outside = path.join(home, 'secret.json'); + write(outside, { apiKey: 'outside-secret' }); + const original = fs.openSync.bind(fs); + vi.spyOn(fs, 'openSync').mockImplementation((file, ...args) => { + if (file === config()) { + fs.renameSync(file, `${file}.original-${randomUUID()}`); + if (kind === 'symlink') fs.symlinkSync(outside, file); + else if (kind === 'hardlink') fs.linkSync(outside, file); + else fs.copyFileSync(outside, file); + } + return original(file, ...args); + }); + const read = vi.spyOn(fs, 'readFileSync'); + expect(snapshot().configError).toBe('unreadable'); + expect(read).not.toHaveBeenCalled(); + }); + + it('revalidates a legitimate atomic config update once before reading', () => { + write(config(), { apiKey: 'old-key' }); + const original = fs.openSync.bind(fs); + let replaced = false; + vi.spyOn(fs, 'openSync').mockImplementation((file, ...args) => { + if (file === config() && !replaced) { + replaced = true; + write(`${file}.new`, { apiKey: 'new-key' }); + fs.renameSync(`${file}.new`, file); + } + return original(file, ...args); + }); + expect(snapshot()).toMatchObject({ config: { apiKey: 'new-key' }, configError: null }); + }); + + it('refuses linked journal metadata and oversized journals before reading', () => { + const id = randomUUID(); + const file = path.join(root, 'auth-operations', `${id}.json`); + const outside = path.join(home, 'secret.json'); + write(outside, { id, generations: [] }); + fs.mkdirSync(path.dirname(file), { mode: 0o700 }); + fs.symlinkSync(outside, file); + expect(renewalStatus(root, { selectionEpoch: id })).toBe('metadata_unreadable'); + fs.unlinkSync(file); fs.writeFileSync(file, ' '.repeat(4097), { mode: 0o600 }); + const read = vi.spyOn(fs, 'readFileSync'); + expect(renewalStatus(root, { selectionEpoch: id })).toBe('metadata_unreadable'); + expect(read).not.toHaveBeenCalled(); + }); + + it('does not acquire a substituted authentication lock', async () => { + const file = path.join(root, 'auth.lock'); + write(file, {}); + const outside = path.join(home, 'outside.lock'); write(outside, {}); + const original = fs.openSync.bind(fs); + vi.spyOn(fs, 'openSync').mockImplementation((target, ...args) => { + if (target === file) { fs.unlinkSync(file); fs.symlinkSync(outside, file); } + return original(target, ...args); + }); + const tryLock = vi.fn().mockReturnValue(true); + const state = createAuthState({ directory: root, locks: async () => ({ tryLock, unlock: vi.fn() }), store: {} }); + await expect(state.logout()).rejects.toMatchObject({ code: 'AUTH_STATE_INVALID' }); + expect(tryLock).not.toHaveBeenCalled(); + }); + + it('the real secure-store worker rejects a linked lock before accepting an operation', () => { + const outside = path.join(home, 'outside.lock'); write(outside, {}); + fs.symlinkSync(outside, path.join(root, 'auth-store.lock')); + const result = spawnSync(process.execPath, [fileURLToPath(new URL('../auth-store-worker.js', import.meta.url))], { + input: `${JSON.stringify({ directory: root })}\n`, encoding: 'utf8', timeout: 3000, + }); + expect(result.error).toBeUndefined(); + if (process.platform === 'win32') expect(result.status).toBe(1); + else expect(result.signal).toBe('SIGKILL'); + expect(result.stdout).not.toContain('ready'); + expect(fs.readFileSync(outside, 'utf8')).toBe('{}'); + }); +}); + +describe('managed trading files', () => { + it('preserves quotes, bridge quotes, tx records and cache stats through a symlinked storage root', async () => { + const storage = path.join(home, 'persistent'); fs.renameSync(root, storage); fs.symlinkSync(storage, root, 'dir'); + const id = saveQuote({ quotes: [] }, 'base'); + expect(loadQuote(id).quoteId).toBe(id); + const claim = claimQuoteForExecution(id); claim.release(); + saveTxRecord('hash', { aggregator: 'relay' }); expect(loadTxRecord('hash').aggregator).toBe('relay'); + write(path.join(getQuotesDir(), 'bridge.json'), { type: 'bridge', timestamp: Date.now(), response: {} }); + expect(loadBridgeQuote('bridge').type).toBe('bridge'); + write(path.join(root, 'cache', `${'a'.repeat(64)}.json`), { timestamp: Date.now(), data: [] }); + vi.resetModules(); + const { collectCacheStats } = await import('../cache-inspect.js'); + expect(collectCacheStats().caches[0].entries).toBe(1); + }); + + it.each(['../config', 'nested/id', 'nested\\id', 'C:secret', 'id\0suffix'])('rejects quote path %s', id => { + expect(safeQuotesPath(`${id}.json`)).toBeNull(); + expect(() => loadQuote(id)).toThrow('not found'); + expect(loadTxRecord(id)).toBeNull(); + }); + + it.each(['symlink', 'hardlink', 'parent'])('refuses swap, bridge and tx files reached through a %s', kind => { + const outside = path.join(home, 'outside'); fs.mkdirSync(outside); + const file = path.join(outside, 'quote.json'); + write(file, { quoteId: 'linked', timestamp: Date.now(), response: {}, type: 'swap' }); + const dir = getQuotesDir(); + if (kind === 'parent') { + fs.symlinkSync(outside, dir, 'dir'); + fs.copyFileSync(file, path.join(outside, 'linked.json')); + fs.copyFileSync(file, path.join(outside, 'tx-linked.json')); + } else { + fs.mkdirSync(dir); + for (const name of ['linked.json', 'tx-linked.json']) { + if (kind === 'symlink') fs.symlinkSync(file, path.join(dir, name)); + else fs.linkSync(file, path.join(dir, name)); + } + } + const read = vi.spyOn(fs, 'readFileSync'); + expect(() => loadQuote('linked')).toThrow(); + expect(() => loadBridgeQuote('linked')).toThrow(); + expect(loadTxRecord('linked')).toBeNull(); + expect(read).not.toHaveBeenCalled(); + }); + + it('keeps ordinary quote claims and transaction records working', () => { + const id = saveQuote({ quotes: [] }, 'base'); + expect(loadQuote(id).quoteId).toBe(id); + const claim = claimQuoteForExecution(id); + expect(() => loadQuote(id)).toThrow('claimed'); + markQuoteExecuted(id, { broadcast: { txHash: 'hash' } }); + claim.release({ handedOff: true }); + expect(() => loadQuote(id)).toThrow('already executed'); + saveTxRecord('hash', { aggregator: 'relay', fromChain: 'base', toChain: 'solana' }); + expect(loadTxRecord('hash').aggregator).toBe('relay'); + }); + + it('leaves a handed-off claim in place if its file is replaced by a symlink', () => { + const id = saveQuote({ quotes: [] }, 'base'); + const claim = claimQuoteForExecution(id); + const claimed = path.join(getQuotesDir(), `${id}.executing.json`); + const outside = path.join(home, 'outside.json'); write(outside, { executedAt: Date.now() }); + fs.unlinkSync(claimed); fs.symlinkSync(outside, claimed); + const read = vi.spyOn(fs, 'readFileSync'); + claim.release({ handedOff: true }); + expect(fs.lstatSync(claimed).isSymbolicLink()).toBe(true); + expect(read).not.toHaveBeenCalled(); + }); +}); + +describe('descriptor validation', () => { + it('checks directory identity even when the file inode is unchanged', () => { + const dir = path.join(root, 'nested'); const file = path.join(dir, 'state.json'); write(file, {}); + const original = fs.openSync.bind(fs); + vi.spyOn(fs, 'openSync').mockImplementation((target, ...args) => { + fs.renameSync(dir, `${dir}.original`); fs.mkdirSync(dir); + fs.renameSync(path.join(`${dir}.original`, 'state.json'), file); + return original(target, ...args); + }); + const read = vi.spyOn(fs, 'readFileSync'); + expect(() => readLocalFile(file, { root })).toThrow(`at ${dir}: directory was replaced`); + expect(read).not.toHaveBeenCalled(); + }); + + it('rejects a symlink replacement even when no-follow is unavailable', () => { + const file = config(); write(file, {}); + const original = fs.openSync.bind(fs); + vi.spyOn(fs, 'openSync').mockImplementation((target, flags, mode) => { + fs.renameSync(target, `${target}.original`); + fs.symlinkSync(`${target}.original`, target); + return original(target, flags & ~(fs.constants.O_NOFOLLOW ?? 0), mode); + }); + const read = vi.spyOn(fs, 'readFileSync'); + expect(() => readLocalFile(file, { root })).toThrow('unsafe'); + expect(read).not.toHaveBeenCalled(); + }); + + it('rejects a parent directory replaced during open', () => { + const dir = path.join(root, 'nested'); const file = path.join(dir, 'state.json'); write(file, {}); + const outside = path.join(home, 'outside'); write(path.join(outside, 'state.json'), {}); + const original = fs.openSync.bind(fs); + vi.spyOn(fs, 'openSync').mockImplementation((target, ...args) => { + fs.renameSync(dir, `${dir}.original`); fs.symlinkSync(outside, dir, 'dir'); + return original(target, ...args); + }); + const read = vi.spyOn(fs, 'readFileSync'); + expect(() => readLocalFile(file, { root })).toThrow('unsafe'); + expect(read).not.toHaveBeenCalled(); + }); + + it.skipIf(process.platform === 'win32')('refuses a FIFO substituted at open without blocking', () => { + const file = config(); write(file, {}); + const original = fs.openSync.bind(fs); + vi.spyOn(fs, 'openSync').mockImplementation((target, ...args) => { + fs.renameSync(target, `${target}.original`); + expect(spawnSync('mkfifo', [target]).status).toBe(0); + return original(target, ...args); + }); + const read = vi.spyOn(fs, 'readFileSync'); + expect(() => readLocalFile(file, { root })).toThrow('unsafe'); + expect(read).not.toHaveBeenCalled(); + }); + + it('closes a rejected descriptor and rejects traversal outside the root', () => { + const file = config(); write(file, {}); + const original = fs.fstatSync.bind(fs); + vi.spyOn(fs, 'fstatSync').mockImplementation(fd => ({ ...original(fd), isFile: () => false })); + const close = vi.spyOn(fs, 'closeSync'); + expect(() => readLocalFile(file, { root })).toThrow('unsafe'); + expect(close).toHaveBeenCalledTimes(1); + expect(() => openLocalFile(path.join(home, 'outside.json'), { root })).toThrow('unsafe'); + }); +}); diff --git a/src/__tests__/trading.test.js b/src/__tests__/trading.test.js index 847c7159..b807d252 100644 --- a/src/__tests__/trading.test.js +++ b/src/__tests__/trading.test.js @@ -503,8 +503,8 @@ describe('safeQuotesPath', () => { }); it('should handle edge cases safely', () => { - expect(safeQuotesPath('')).not.toBeNull(); // empty string resolves to base dir - expect(safeQuotesPath('.')).not.toBeNull(); // current dir is safe + expect(safeQuotesPath('')).toBeNull(); + expect(safeQuotesPath('.')).toBeNull(); expect(safeQuotesPath('..')).toBeNull(); // parent dir is rejected }); }); diff --git a/src/auth-credentials.js b/src/auth-credentials.js index 80c4c6d1..3b67a4da 100644 --- a/src/auth-credentials.js +++ b/src/auth-credentials.js @@ -2,6 +2,7 @@ import fs from 'node:fs'; import path from 'node:path'; import os from 'node:os'; import { fileURLToPath } from 'node:url'; +import { readLocalFile } from './local-file.js'; export const DEFAULT_API_ORIGIN = 'https://api.nansen.ai'; const DEV_CONFIG = fileURLToPath(new URL('../config.json', import.meta.url)); @@ -32,17 +33,28 @@ export function validAuthPointer(auth) { } export function readAuthConfig(env = process.env, devConfigPath = DEV_CONFIG) { const userPath = path.join(authDirectory(env), 'config.json'); - const configPath = fs.existsSync(userPath) ? userPath : fs.existsSync(devConfigPath) ? devConfigPath : null; + let configFileExists; + try { configFileExists = Boolean(fs.lstatSync(userPath, { throwIfNoEntry: false })); } + catch { return { config: {}, configPath: userPath, configError: 'unreadable', configFileExists: true, devConfigPath }; } + const configPath = configFileExists ? userPath : fs.existsSync(devConfigPath) ? devConfigPath : null; let config = {}; let configError = null; if (configPath) { try { - config = JSON.parse(fs.readFileSync(configPath, 'utf8')); + const options = { root: path.dirname(configPath), privateFile: configPath === userPath }; + let contents; + try { contents = readLocalFile(configPath, options); } + catch (error) { + // Login and renewal replace config atomically. Revalidate once from scratch. + if (!error.localFileRace) throw error; + contents = readLocalFile(configPath, options); + } + config = JSON.parse(contents); if (!config || typeof config !== 'object' || Array.isArray(config)) throw new Error(); } catch (err) { config = {}; configError = err instanceof SyntaxError ? 'parse' : 'unreadable'; } } if (config.auth && !validAuthPointer(config.auth)) configError = 'format'; - return { config, configPath, configError, configFileExists: fs.existsSync(userPath), devConfigPath }; + return { config, configPath, configError, configFileExists, devConfigPath }; } export function resolveCredential({ env = process.env, explicitKey, snapshot = readAuthConfig(env) } = {}) { if (explicitKey !== undefined) return explicitKey === null ? { kind: 'anonymous', source: null } : { kind: 'api-key', source: 'explicit', apiKey: explicitKey }; diff --git a/src/auth-state.js b/src/auth-state.js index d0d6585b..964c6943 100644 --- a/src/auth-state.js +++ b/src/auth-state.js @@ -5,6 +5,7 @@ import { setTimeout as delay } from 'node:timers/promises'; import { AuthError, authDirectory, validAuthPointer } from './auth-credentials.js'; import { createAuthStore } from './auth-store.js'; import { refreshSession, retireSession, validateSession } from './auth-device.js'; +import { openLocalFile, readLocalFile } from './local-file.js'; const queues = new Map(); const MAX_RETRYABLE_RENEWALS = 5; @@ -16,8 +17,8 @@ const journalExists = file => { }; const stateError = () => new AuthError('AUTH_STATE_INVALID', 'Saved authentication cannot be read safely. Repair config.json permissions or restore the file; no other credential was selected.'); function safePath(file, directory = false) { - if (!fs.existsSync(file)) return; - const stat = fs.lstatSync(file); + const stat = fs.lstatSync(file, { throwIfNoEntry: false }); + if (!stat) return; if (stat.isSymbolicLink() || (directory ? !stat.isDirectory() : !stat.isFile()) || (process.platform !== 'win32' && ((stat.mode & 0o022) || stat.uid !== process.getuid()))) throw stateError(); } @@ -25,8 +26,7 @@ const uuid = value => typeof value === 'string' && /^[a-f0-9-]{36}$/.test(value) function readJournal(file, id) { try { safePath(file); - if (fs.statSync(file).size > 4096) throw journalError(); - const value = JSON.parse(fs.readFileSync(file, 'utf8')); + const value = JSON.parse(readLocalFile(file, { root: path.dirname(path.dirname(file)), privateFile: true, maxBytes: 4096 })); if (value?.id !== id) throw journalError(); if (value.kind === 'rotation') { if (value.version !== 2 || value.id !== value.selectionEpoch || !uuid(value.selectionEpoch) || @@ -96,7 +96,7 @@ export function createAuthState({ directory = authDirectory(), store = createAut safePath(configFile); if (!fs.existsSync(configFile)) return {}; try { - const value = JSON.parse(fs.readFileSync(configFile, 'utf8')); + const value = JSON.parse(readLocalFile(configFile, { root: directory, privateFile: true })); if (!value || typeof value !== 'object' || Array.isArray(value) || (value.auth && !validAuthPointer(value.auth))) throw new Error(); return value; } catch { throw stateError(); } @@ -129,7 +129,9 @@ export function createAuthState({ directory = authDirectory(), store = createAut const native = await locks(); signal?.throwIfAborted(); safePath(file); - const fd = fs.openSync(file, 'a+', 0o600); + let fd; + try { fd = openLocalFile(file, { root: directory, flags: fs.constants.O_RDWR | fs.constants.O_CREAT | fs.constants.O_APPEND, privateFile: true }); } + catch { throw stateError(); } try { while (!native.tryLock(fd)) { diff --git a/src/auth-store-worker.js b/src/auth-store-worker.js index 4b7a8701..1c7d046a 100644 --- a/src/auth-store-worker.js +++ b/src/auth-store-worker.js @@ -4,6 +4,7 @@ import fs from 'node:fs'; import path from 'node:path'; import { Worker } from 'node:worker_threads'; import locks from 'fs-native-extensions'; +import { openLocalFile } from './local-file.js'; const stop = () => process.kill(process.pid, 'SIGKILL'); const timer = setTimeout(stop, 10000); @@ -13,8 +14,8 @@ process.stdout.on('error', stop); let input = '', phase = 'guard'; let fd; function safe(file, directory = false) { - if (!fs.existsSync(file)) return; - const stat = fs.lstatSync(file); + const stat = fs.lstatSync(file, { throwIfNoEntry: false }); + if (!stat) return; if (stat.isSymbolicLink() || (directory ? !stat.isDirectory() : !stat.isFile()) || (process.platform !== 'win32' && ((stat.mode & 0o022) || stat.uid !== process.getuid()))) throw new Error(); } @@ -27,7 +28,7 @@ function receive(line) { fs.mkdirSync(directory, { recursive: true, mode: 0o700 }); const file = path.join(directory, 'auth-store.lock'); safe(file); - fd = fs.openSync(file, 'a+', 0o600); + fd = openLocalFile(file, { root: directory, flags: fs.constants.O_RDWR | fs.constants.O_CREAT | fs.constants.O_APPEND, privateFile: true }); // Independent acquisition: no cross-platform inherited-lock assumptions. // This stable inode is never unlinked, including by recovery. if (!locks.tryLock(fd)) throw new Error(); diff --git a/src/bridge.js b/src/bridge.js index 880e9f66..ff7b4976 100644 --- a/src/bridge.js +++ b/src/bridge.js @@ -9,11 +9,13 @@ import * as crypto from 'node:crypto'; import * as fs from 'node:fs'; import * as path from 'node:path'; +import { readLocalFile, writeLocalFile } from './local-file.js'; import { CommandError, validateAddress } from './api.js'; import { signSecp256k1 } from './crypto.js'; import { assertEvmFeeWithinCap, + claimQuoteForExecution, convertToBaseUnits, evmRpcCall, getEvmNonce, @@ -242,11 +244,19 @@ function saveBridgeQuote(response, originChain, destinationChain, walletProvider } export function loadBridgeQuote(quoteId) { + const claimedPath = safeQuotesPath(`${quoteId}.executing.json`); + if (claimedPath && fs.lstatSync(claimedPath, { throwIfNoEntry: false })) { + throw new Error(`Bridge quote "${quoteId}" is claimed by another execution. Funds may be in flight. Check "nansen bridge status" before requesting a new quote.`); + } const filePath = safeQuotesPath(`${quoteId}.json`); if (!filePath || !fs.existsSync(filePath)) { throw new Error(`Bridge quote "${quoteId}" not found. Quotes expire after 1 hour.`); } - const data = JSON.parse(fs.readFileSync(filePath, 'utf8')); + return readBridgeQuoteFile(filePath, quoteId); +} + +function readBridgeQuoteFile(filePath, quoteId) { + const data = JSON.parse(readLocalFile(filePath, { root: path.dirname(getQuotesDir()) })); if (Date.now() - data.timestamp > QUOTE_TTL_MS) { fs.unlinkSync(filePath); throw new Error('Bridge quote has expired. Please request a new quote.'); @@ -289,10 +299,15 @@ export function loadBridgeQuote(quoteId) { // later step throwing must not leave the quote reusable. The step counters make // a partial failure legible to the operator (see loadBridgeQuote). export function markBridgeQuoteExecuted(quoteId, progress = {}) { - const filePath = safeQuotesPath(`${quoteId}.json`); - if (!filePath || !fs.existsSync(filePath)) return; + const claimedPath = safeQuotesPath(`${quoteId}.executing.json`); + const filePath = claimedPath && fs.lstatSync(claimedPath, { throwIfNoEntry: false }) + ? claimedPath : safeQuotesPath(`${quoteId}.json`); try { - const data = JSON.parse(fs.readFileSync(filePath, 'utf8')); + if (!filePath || !fs.existsSync(filePath)) { + if (progress.required) throw new Error('Missing execution state'); + return; + } + const data = JSON.parse(readLocalFile(filePath, { root: path.dirname(getQuotesDir()) })); data.executedAt = data.executedAt || Date.now(); if (progress.broadcast) { data.broadcasts = [...(data.broadcasts || []), { ...progress.broadcast, at: Date.now() }]; @@ -301,11 +316,9 @@ export function markBridgeQuoteExecuted(quoteId, progress = {}) { data.broadcastSteps = Math.max(data.broadcastSteps || 0, progress.broadcastSteps); } if (progress.totalSteps !== undefined) data.totalSteps = progress.totalSteps; - fs.writeFileSync(filePath, JSON.stringify(data, null, 2), { mode: 0o600 }); + writeLocalFile(filePath, JSON.stringify(data, null, 2), { root: path.dirname(getQuotesDir()) }); } catch { - // Best-effort: if the marker can't be written, the next execute attempt - // will still proceed, but that's preferable to crashing after a successful - // broadcast. + throw new CommandError('Could not safely record bridge execution. Funds may already be in flight. The quote remains claimed; check "nansen bridge status" before requesting a new quote.', 'QUOTE_STATE_UNSAFE'); } } @@ -2043,90 +2056,105 @@ from a quote are the same ones that got stuck. Check the stuck nonce with preflightResult ??= await preflightPlan(); const { evmIntent, hlIntent } = preflightResult; - // Consume the quote at each INDIVIDUAL broadcast, before any receipt wait. - // A tx can be accepted by the network and then have waitForReceipt time - // out; if the quote were still unspent, a retry would re-sign and re-send - // it with a fresh nonce. markBridgeQuoteExecuted pins executedAt on the - // first call, so the quote is spent the instant anything is in flight. - const onBroadcast = (stepId, txHash) => - markBridgeQuoteExecuted(quoteId, { - broadcast: { step: stepId, txHash: txHash || null }, - totalSteps: steps.length, - }); - - // Step-level counter, recorded only once a step fully completes. - const markBroadcast = (index) => - markBridgeQuoteExecuted(quoteId, { - broadcastSteps: index + 1, - totalSteps: steps.length, - }); - - if (execution_type === 'evm_transaction') { - // Overrides move real money differently from what was quoted, so say so - // rather than letting them apply silently. - if (overridesSummary) { - log(` Overrides: ${overridesSummary}`); + const claim = claimQuoteForExecution(quoteId, readBridgeQuoteFile); + let handedOff = false; + let keepClaimed = false; + try { + if (JSON.stringify(claim.quote) !== JSON.stringify(quoteData)) { + throw new CommandError('Bridge quote changed while awaiting execution. Request a fresh quote.', 'QUOTE_STATE_UNSAFE'); } - for (const [index, step] of steps.entries()) { - await processEvmStep(step, { - chain: quoteData.originChain, - privateKeyHex: creds.privateKey, - signerAddress: signer.address, - log, - onBroadcast, - feeOverrides, - nonceSequence, - intent: evmIntent, + // Consume the quote at each INDIVIDUAL broadcast, before any receipt wait. + // A tx can be accepted by the network and then have waitForReceipt time + // out; if the quote were still unspent, a retry would re-sign and re-send + // it with a fresh nonce. markBridgeQuoteExecuted pins executedAt on the + // first call, so the quote is spent the instant anything is in flight. + const onBroadcast = (stepId, txHash) => { + handedOff = true; + markBridgeQuoteExecuted(quoteId, { + required: true, + broadcast: { step: stepId, txHash: txHash || null }, + totalSteps: steps.length, }); - markBroadcast(index); - } - } else if (execution_type === 'hyperliquid_signature') { - if (creds.provider === 'privy') { - const { PrivyClient } = await import('./privy.js'); - const privyClient = new PrivyClient(process.env.PRIVY_APP_ID, process.env.PRIVY_APP_SECRET); - for (const [index, step] of steps.entries()) { - await processSignatureStepPrivy(step, { - privyClient, - // `signer` above — the same resolution that was screened and - // matched against the quote, rather than resolving a second time. - walletId: signer.privyWalletIds?.evm, - log, - apiInstance, - onBroadcast, - intent: hlIntent, - }); - markBroadcast(index); + }; + + // Step-level counter, recorded only once a step fully completes. + const markBroadcast = (index) => + markBridgeQuoteExecuted(quoteId, { + required: true, + broadcastSteps: index + 1, + totalSteps: steps.length, + }); + + if (execution_type === 'evm_transaction') { + // Overrides move real money differently from what was quoted, so say so + // rather than letting them apply silently. + if (overridesSummary) { + log(` Overrides: ${overridesSummary}`); } - } else { for (const [index, step] of steps.entries()) { - await processSignatureStepLocal(step, { + await processEvmStep(step, { + chain: quoteData.originChain, privateKeyHex: creds.privateKey, + signerAddress: signer.address, log, - apiInstance, onBroadcast, - intent: hlIntent, + feeOverrides, + nonceSequence, + intent: evmIntent, }); markBroadcast(index); } + } else if (execution_type === 'hyperliquid_signature') { + if (creds.provider === 'privy') { + const { PrivyClient } = await import('./privy.js'); + const privyClient = new PrivyClient(process.env.PRIVY_APP_ID, process.env.PRIVY_APP_SECRET); + for (const [index, step] of steps.entries()) { + await processSignatureStepPrivy(step, { + privyClient, + // `signer` above — the same resolution that was screened and + // matched against the quote, rather than resolving a second time. + walletId: signer.privyWalletIds?.evm, + log, + apiInstance, + onBroadcast, + intent: hlIntent, + }); + markBroadcast(index); + } + } else { + for (const [index, step] of steps.entries()) { + await processSignatureStepLocal(step, { + privateKeyHex: creds.privateKey, + log, + apiInstance, + onBroadcast, + intent: hlIntent, + }); + markBroadcast(index); + } + } + } else { + throw new Error(`Unknown execution type: ${execution_type}`); } - } else { - throw new Error(`Unknown execution type: ${execution_type}`); - } - // Every step is out; the marker above already consumed the quote. - markBridgeQuoteExecuted(quoteId, { broadcastSteps: steps.length, totalSteps: steps.length }); + // Every step is out; the marker above already consumed the quote. + markBridgeQuoteExecuted(quoteId, { required: true, broadcastSteps: steps.length, totalSteps: steps.length }); - log(`\n Bridge submitted. Polling for completion...`); - const status = await pollBridgeCompletion(apiInstance, { requestId: request_id, log }); + log(`\n Bridge submitted. Polling for completion...`); + const status = await pollBridgeCompletion(apiInstance, { requestId: request_id, log }); - if (status.status === 'success') { - log(`\n Bridge completed!`); - if (status.destination_tx_hashes?.length) { - log(` Destination tx: ${status.destination_tx_hashes[0]}`); + if (status.status === 'success') { + log(`\n Bridge completed!`); + if (status.destination_tx_hashes?.length) { + log(` Destination tx: ${status.destination_tx_hashes[0]}`); + } } - } - log(''); - return undefined; + log(''); + return undefined; + } catch (error) { + keepClaimed = handedOff && error.code === 'QUOTE_STATE_UNSAFE'; + throw error; + } finally { claim.release({ handedOff, keepClaimed }); } }, 'status': async (args, apiInstance, flags, options) => { diff --git a/src/cache-inspect.js b/src/cache-inspect.js index 95413758..094bcad4 100644 --- a/src/cache-inspect.js +++ b/src/cache-inspect.js @@ -21,6 +21,7 @@ import fs from 'fs'; import path from 'path'; +import { openLocalFile } from './local-file.js'; import { getCacheDir, getConfigDir, DEFAULT_CACHE_TTL, NansenError, ErrorCode } from './api.js'; import { getCostMapFile, COST_MAP_TTL_MS } from './cost-cache.js'; @@ -86,15 +87,10 @@ function readTimestamp(file, field) { let fd; let raw; try { - // O_NOFOLLOW prevents a final-component symlink replacement after lstat. - // Windows does not define it. There, only the earlier lstat check applies; - // opening with O_RDONLY does not prevent this race. - const flags = fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW ?? 0); - fd = fs.openSync(file, flags); - if (!fs.fstatSync(fd).isFile()) return { raced: true }; + fd = openLocalFile(file, { root: getConfigDir() }); raw = fs.readFileSync(fd, 'utf8'); } catch (error) { - if (error?.code === 'ENOENT' || error?.code === 'ELOOP') return { raced: true }; + if (error?.code === 'ENOENT' || error?.code === 'ELOOP' || error?.code === 'LOCAL_FILE_UNSAFE') return { raced: true }; throw fsError('read', file, error); } finally { if (fd !== undefined) fs.closeSync(fd); diff --git a/src/local-file.js b/src/local-file.js new file mode 100644 index 00000000..ee0bb6f7 --- /dev/null +++ b/src/local-file.js @@ -0,0 +1,75 @@ +import fs from 'node:fs'; +import path from 'node:path'; + +const unsafe = (file, reason) => Object.assign(new Error(`Refusing an unsafe local file at ${file}: ${reason}. Repair that path before retrying.`), { code: 'LOCAL_FILE_UNSAFE' }); +const sameFile = (a, b) => a.dev === b.dev && a.ino === b.ino; +function checkStat(stat, directory, privateFile, file) { + if (!stat || (directory ? !stat.isDirectory() : !stat.isFile())) throw unsafe(file, directory ? 'expected a directory without symlinks' : 'expected a regular file without symlinks'); + if (!directory && stat.nlink !== 1) throw unsafe(file, 'file has unexpected hard links'); + if (privateFile && process.platform !== 'win32') { + if (stat.mode & 0o022) throw unsafe(file, 'authentication path is writable by group or other users'); + if (stat.uid !== process.getuid()) throw unsafe(file, 'authentication path belongs to another user'); + } +} + +// Non-private stores may use an intentionally symlinked storage root. +// Authentication and all components below either root must be link-free. +// Descriptor checks reject replacements before any bytes are read or written. +export function openLocalFile(file, { root, flags = fs.constants.O_RDONLY, mode = 0o600, privateFile = false, maxBytes = Infinity } = {}) { + const base = path.resolve(root); + const target = path.resolve(file); + const relative = path.relative(base, target); + if (!relative || relative === '..' || relative.startsWith(`..${path.sep}`) || path.isAbsolute(relative)) throw unsafe(target, 'path is outside its storage root'); + const directories = [base]; + for (const part of relative.split(path.sep).slice(0, -1)) directories.push(path.join(directories.at(-1), part)); + const directoryStat = (dir, i) => i === 0 && !privateFile ? fs.statSync(dir) : fs.lstatSync(dir); + const parents = directories.map((dir, i) => { + const stat = directoryStat(dir, i); + checkStat(stat, true, privateFile, dir); + return stat; + }); + const before = fs.lstatSync(target, { throwIfNoEntry: false }); + if (before) checkStat(before, false, privateFile, target); + let fd; + try { + fd = fs.openSync(target, flags | (fs.constants.O_NOFOLLOW ?? 0) | (fs.constants.O_NONBLOCK ?? 0), mode); + const opened = fs.fstatSync(fd); + checkStat(opened, false, privateFile, target); + if (opened.size > maxBytes) throw unsafe(target, `file exceeds its ${maxBytes}-byte limit`); + if (before && !sameFile(before, opened)) throw Object.assign(unsafe(target, 'file was replaced while opening'), { localFileRace: true }); + const after = fs.lstatSync(target); + checkStat(after, false, privateFile, target); + if (!sameFile(opened, after)) throw Object.assign(unsafe(target, 'file was replaced while opening'), { localFileRace: true }); + directories.forEach((dir, i) => { + const stat = directoryStat(dir, i); + checkStat(stat, true, privateFile, dir); + if (!sameFile(parents[i], stat)) throw unsafe(dir, 'directory was replaced while opening'); + }); + return fd; + } catch (error) { + if (fd !== undefined) fs.closeSync(fd); + if (error.code === 'ELOOP') throw unsafe(target, 'file became a symbolic link while opening'); + throw error; + } +} + +export function readLocalFile(file, options) { + const fd = openLocalFile(file, options); + try { return fs.readFileSync(fd, 'utf8'); } + finally { fs.closeSync(fd); } +} + +export function writeLocalFile(file, contents, options) { + const fd = openLocalFile(file, { ...options, flags: fs.constants.O_WRONLY }); + try { + const bytes = Buffer.from(contents, 'utf8'); + fs.ftruncateSync(fd, 0); + let offset = 0; + while (offset < bytes.length) { + const written = fs.writeSync(fd, bytes, offset, bytes.length - offset, offset); + if (!written) throw new Error('Could not write the local file.'); + offset += written; + } + fs.fsyncSync(fd); + } finally { fs.closeSync(fd); } +} diff --git a/src/trading.js b/src/trading.js index 8dae29cc..5b5df420 100644 --- a/src/trading.js +++ b/src/trading.js @@ -9,6 +9,7 @@ import { rejectBlankOption } from './query-options.js'; import crypto from 'crypto'; import fs from 'fs'; import path from 'path'; +import { readLocalFile, writeLocalFile } from './local-file.js'; import { base58Encode, exportWallet, getWalletConfig, showWallet, listWallets } from './wallet.js'; import { base58Decode, encodeCompactU16 } from './transfer.js'; import { buildMessageV0, fetchAddressLookupTables, fetchRecentBlockhash } from './x402-svm.js'; @@ -135,11 +136,8 @@ export function getQuotesDir() { // Resolve a filename inside the quotes dir, rejecting path traversal. export function safeQuotesPath(filename) { - const base = path.resolve(getQuotesDir()); - const target = path.resolve(base, filename); - const relative = path.relative(base, target); - if (relative.startsWith('..') || path.isAbsolute(relative)) return null; - return target; + if (typeof filename !== 'string' || !filename || filename === '.' || filename === '..' || /[/\\:]/.test(filename) || filename.includes('\0')) return null; + return path.join(path.resolve(getQuotesDir()), filename); } // ============= Trading API Client ============= @@ -457,7 +455,7 @@ export function loadTxRecord(txHash) { if (!filePath) return null; if (!fs.existsSync(filePath)) return null; try { - const data = JSON.parse(fs.readFileSync(filePath, 'utf8')); + const data = JSON.parse(readLocalFile(filePath, { root: path.dirname(getQuotesDir()) })); if (Date.now() - data.timestamp > TX_RECORD_TTL_MS) { fs.unlinkSync(filePath); return null; @@ -516,7 +514,7 @@ export function loadQuote(quoteId) { } function readQuoteFile(filePath, quoteId) { - const data = JSON.parse(fs.readFileSync(filePath, 'utf8')); + const data = JSON.parse(readLocalFile(filePath, { root: path.dirname(getQuotesDir()) })); if (Date.now() - data.timestamp > 3600000) { fs.unlinkSync(filePath); throw new Error('Quote has expired. Please request a new quote.'); @@ -543,7 +541,7 @@ function readQuoteFile(filePath, quoteId) { function quoteClaimedError(quoteId, claimedPath) { return new Error( - `Quote "${quoteId}" is claimed by another execution (${claimedPath}). If that run is still going, wait for it to finish. If it was interrupted, a swap may or may not have been broadcast: check the wallet on the explorer, then request a new quote with "nansen trade quote". The claimed quote is removed when it expires.`, + `Quote "${quoteId}" is claimed by another execution (${claimedPath}). If that run is still going, wait for it to finish. If it was interrupted, a transaction may or may not have been broadcast: check the wallet on the explorer, then request a new quote with the matching quote command. The claimed quote is removed when it expires.`, ); } @@ -574,12 +572,14 @@ function quoteClaimedError(quoteId, claimedPath) { * after its 1-hour lifetime. * * @param {string} quoteId - * @returns {{ quote: object, release: (opts?: { handedOff?: boolean }) => void }} + * @param {function} [readQuote] Validates the claimed quote, including its type. + * @returns {{ quote: object, release: (opts?: { handedOff?: boolean, keepClaimed?: boolean }) => void }} */ -export function claimQuoteForExecution(quoteId) { +export function claimQuoteForExecution(quoteId, readQuote = readQuoteFile) { const quotePath = safeQuotesPath(`${quoteId}.json`); const claimedPath = safeQuotesPath(`${quoteId}.executing.json`); if (!quotePath || !claimedPath) throw new Error(`Invalid quote id "${quoteId}".`); + if (fs.lstatSync(claimedPath, { throwIfNoEntry: false })) throw quoteClaimedError(quoteId, claimedPath); try { fs.renameSync(quotePath, claimedPath); @@ -592,19 +592,24 @@ export function claimQuoteForExecution(quoteId) { } let released = false; - const release = ({ handedOff = false } = {}) => { + const release = ({ handedOff = false, keepClaimed = false } = {}) => { if (released) return; released = true; + if (keepClaimed) return; try { - if (handedOff && !JSON.parse(fs.readFileSync(claimedPath, 'utf8')).executedAt) return; + if (handedOff && !JSON.parse(readLocalFile(claimedPath, { root: path.dirname(getQuotesDir()) })).executedAt) return; fs.renameSync(claimedPath, quotePath); } catch { /* gone (expired and cleaned up): nothing to release */ } }; try { - return { quote: readQuoteFile(claimedPath, quoteId), release }; + if (process.platform !== 'win32') { + const fd = fs.openSync(path.dirname(claimedPath), 'r'); + try { fs.fsyncSync(fd); } finally { fs.closeSync(fd); } + } + return { quote: readQuote(claimedPath, quoteId), release }; } catch (err) { - // Already executed, expired or not a swap quote: nothing is signed, so + // Invalid, already executed, or expired: nothing is signed, so // put back whatever is still there. release(); throw err; @@ -624,16 +629,15 @@ export function markQuoteExecuted(quoteId, progress = {}) { : safeQuotesPath(`${quoteId}.json`); if (!filePath || !fs.existsSync(filePath)) return; try { - const data = JSON.parse(fs.readFileSync(filePath, 'utf8')); + const data = JSON.parse(readLocalFile(filePath, { root: path.dirname(getQuotesDir()) })); data.executedAt = data.executedAt || Date.now(); if (progress.broadcast) { data.broadcasts = [...(data.broadcasts || []), { ...progress.broadcast, at: Date.now() }]; } - fs.writeFileSync(filePath, JSON.stringify(data, null, 2), { mode: 0o600 }); + writeLocalFile(filePath, JSON.stringify(data, null, 2), { root: path.dirname(getQuotesDir()) }); } catch { - // Best-effort: if the marker can't be written, the next execute attempt - // will still proceed, but that's preferable to crashing after a successful - // broadcast. + // The execution claim stays in place without a readable marker, so a + // later execution cannot sign again after an uncertain broadcast. } } @@ -650,7 +654,7 @@ export function cleanupQuotes() { if (!file.endsWith('.json')) continue; const ttl = file.startsWith('tx-') ? TX_RECORD_TTL_MS : 3600000; try { - const data = JSON.parse(fs.readFileSync(path.join(dir, file), 'utf8')); + const data = JSON.parse(readLocalFile(path.join(dir, file), { root: path.dirname(dir) })); if (now - data.timestamp > ttl) fs.unlinkSync(path.join(dir, file)); } catch { /* ignore */ } }