diff --git a/tests/tools/relay-bench/README.md b/tests/tools/relay-bench/README.md index 8661c214469..0db742b5c18 100644 --- a/tests/tools/relay-bench/README.md +++ b/tests/tools/relay-bench/README.md @@ -104,7 +104,7 @@ pbpaste | node $BENCH pair ~/.orca/relay-bench/state.json # Or from a file you protect yourself, which `pair` requires to be mode 0600: umask 077 && printf '%s' '' > ~/.orca/relay-bench/pair.txt -node $BENCH pair ~/.orca/relay-bench/state.json --pairing-url-file=~/.orca/relay-bench/pair.txt +node $BENCH pair ~/.orca/relay-bench/state.json --pairing-url-file="$HOME/.orca/relay-bench/pair.txt" rm ~/.orca/relay-bench/pair.txt # Steady-state foreground reconnect, 10 times, 2 s apart, re-resolving the cell each time. diff --git a/tests/tools/relay-bench/relay-bench-state-file.mjs b/tests/tools/relay-bench/relay-bench-state-file.mjs index 32b1d9c22d4..e5ded5c43c5 100644 --- a/tests/tools/relay-bench/relay-bench-state-file.mjs +++ b/tests/tools/relay-bench/relay-bench-state-file.mjs @@ -8,6 +8,7 @@ import { fchmodSync, constants, fstatSync, + ftruncateSync, lstatSync, mkdirSync, openSync, @@ -43,7 +44,9 @@ export function writeSecretFile(path, contents) { try { fd = openSync( path, - constants.O_WRONLY | constants.O_CREAT | constants.O_TRUNC | NOFOLLOW, + // No O_TRUNC: truncating happens only after the descriptor passes the checks below, so a + // refused file keeps its previous contents. + constants.O_WRONLY | constants.O_CREAT | NOFOLLOW, SECRET_FILE_MODE ) } catch (err) { @@ -57,14 +60,15 @@ export function writeSecretFile(path, contents) { if (!stats.isFile()) { throw new Error(`refusing to write ${path}: not a regular file`) } - // Before the write, not after: a pre-existing file owned by someone else would take the - // token on O_TRUNC and only then fail the chmod, leaving it readable by its owner. + // Before the truncate and write, not after: a pre-existing file owned by someone else would + // otherwise lose its contents and then fail the chmod, leaving the token readable by its owner. if (process.platform !== 'win32' && stats.uid !== process.getuid()) { throw new Error(`refusing to write ${path}: owned by another user`) } if (process.platform !== 'win32' && (stats.mode & GROUP_AND_OTHER_BITS) !== 0) { fchmodSync(fd, SECRET_FILE_MODE) } + ftruncateSync(fd, 0) writeFileSync(fd, contents) } finally { closeSync(fd) diff --git a/tests/tools/relay-bench/relay-bench-state-file.test.mjs b/tests/tools/relay-bench/relay-bench-state-file.test.mjs index 052703e006d..c8ffcb23b1c 100644 --- a/tests/tools/relay-bench/relay-bench-state-file.test.mjs +++ b/tests/tools/relay-bench/relay-bench-state-file.test.mjs @@ -67,6 +67,20 @@ describe('writeSecretFile', () => { expect(modeOf(join(dir, 'nested'))).toBe(0o700) }) + it.runIf(posix)('leaves a refused file untouched instead of truncating it first', () => { + // A writable file owned by someone else must be refused before its contents are destroyed. + const path = join(dir, 'state.json') + writeFileSync(path, 'previous contents') + const realGetuid = process.getuid + process.getuid = () => realGetuid() + 1 + try { + expect(() => writeSecretFile(path, 'secret')).toThrow(/owned by another user/) + } finally { + process.getuid = realGetuid + } + expect(readFileSync(path, 'utf8')).toBe('previous contents') + }) + it('truncates rather than appending to a longer previous file', () => { const path = join(dir, 'state.json') writeSecretFile(path, '{"a":"aaaaaaaaaaaaaaaaaaaa"}') diff --git a/tests/tools/relay-bench/relay-phone-connect-bench.mjs b/tests/tools/relay-bench/relay-phone-connect-bench.mjs index 9ab88fd3e3f..d9f388b1227 100644 --- a/tests/tools/relay-bench/relay-phone-connect-bench.mjs +++ b/tests/tools/relay-bench/relay-phone-connect-bench.mjs @@ -375,6 +375,15 @@ async function loadState(statePath) { } // ---------- commands ---------- +// The peer picks the code, so only a plain identifier is echoed; anything else is named by kind. +const REMOTE_ERROR_CODE = /^[A-Za-z0-9_.-]{1,64}$/ +export function describeRemoteErrorCode(code) { + if (typeof code !== 'string') { + return code === undefined ? 'unknown' : `non-string code (${typeof code})` + } + return REMOTE_ERROR_CODE.test(code) ? code : `unprintable code (${code.length} chars)` +} + async function pair(pairingUrl, statePath) { const offer = decodeOffer(pairingUrl) if (!offer.relay) { @@ -413,7 +422,7 @@ async function pair(pairingUrl, statePath) { if (!endpoints.ok || !endpoints.result.relay) { // Shape only: the reply is peer-supplied and this line lands in the operator's terminal. throw new Error( - `getEndpoints failed: ${endpoints.ok ? 'no relay block in result' : `error ${endpoints.error?.code ?? 'unknown'}`}` + `getEndpoints failed: ${endpoints.ok ? 'no relay block in result' : `error ${describeRemoteErrorCode(endpoints.error?.code)}`}` ) } console.log(`provisionRelay ${provisionMs} ms, getEndpoints ${endpointsMs} ms`) diff --git a/tests/tools/relay-bench/relay-phone-connect-bench.test.mjs b/tests/tools/relay-bench/relay-phone-connect-bench.test.mjs index 05613634592..833a60d6b36 100644 --- a/tests/tools/relay-bench/relay-phone-connect-bench.test.mjs +++ b/tests/tools/relay-bench/relay-phone-connect-bench.test.mjs @@ -2,7 +2,12 @@ // missing or malformed pairing link surfaced as a stack trace rather than usage. The link is a // live credential, so the failure text has to name the problem without echoing the code. import { describe, expect, it, vi } from 'vitest' -import { decodeOffer, vetCellUrl, vetRelayEndpoint } from './relay-phone-connect-bench.mjs' +import { + decodeOffer, + describeRemoteErrorCode, + vetCellUrl, + vetRelayEndpoint +} from './relay-phone-connect-bench.mjs' const encode = (offer) => Buffer.from(JSON.stringify(offer), 'utf8').toString('base64url') @@ -86,3 +91,22 @@ describe('vetRelayEndpoint', () => { ).resolves.toEqual({ ok: true }) }) }) + +describe('describeRemoteErrorCode', () => { + it('echoes a plain protocol identifier', () => { + expect(describeRemoteErrorCode('invalid_install_req')).toBe('invalid_install_req') + }) + + it('never echoes terminal control bytes the peer put in the code', () => { + const out = describeRemoteErrorCode('\u001b[2J\u001b]0;pwned\u0007') + expect(out).not.toContain('\u001b') + expect(out).toMatch(/unprintable code/) + }) + + it('names the type instead of stringifying a non-string code', () => { + expect(describeRemoteErrorCode({ toString: () => '\u001b[31m' })).toBe( + 'non-string code (object)' + ) + expect(describeRemoteErrorCode(undefined)).toBe('unknown') + }) +})