diff --git a/README.md b/README.md index 0ac0bcd..81d8519 100644 --- a/README.md +++ b/README.md @@ -299,13 +299,19 @@ it, lives in your local signer instead - see `VECTOR_SIGNER_SPEND_LIMIT_PER_TX` > echoes a token value. Commas delimit entries and cannot be escaped, so generate > tokens from a comma-free charset (hex / base64url / alphanumeric). -> **Error responses do not echo configured endpoints.** A failed Ogmios, Koios, or -> submit-API request is reported to the caller by service name and status only - -> for example "Ogmios request failed (queryLedgerState/utxo): 503 Service -> Unavailable" - never the configured `VECTOR_OGMIOS_URL` / `VECTOR_KOIOS_URL` / -> `VECTOR_SUBMIT_URL` value or the raw response body. Full detail goes to the -> server's own `console.error` log for operators; a caller only ever sees the -> service name and status. +> **Error responses do not echo configured endpoints.** A failed Ogmios or Koios +> query is this server's own infrastructure problem, not something the caller can +> act on - it is reported by service name and status only, for example "Ogmios +> request failed (queryLedgerState/utxo): 503 Service Unavailable". A rejected +> transaction submission is different: the ledger's verdict on the CALLER'S OWN +> transaction (a bad input, an unmet script condition, a fee too small) stays in +> the response, since it is the feedback loop an agent needs to self-correct after +> build → sign → submit - only URL-shaped content is scrubbed out of it first, for +> example "Transaction submission rejected (400 Bad Request): +> ValueNotConservedUTxO...". Neither case ever echoes the configured +> `VECTOR_OGMIOS_URL` / `VECTOR_KOIOS_URL` / `VECTOR_SUBMIT_URL` value itself, and +> full unscrubbed detail always goes to the server's own `console.error` log for +> operators. ### Mainnet endpoints diff --git a/packages/shared/src/provider.ts b/packages/shared/src/provider.ts index e5c716c..96e91eb 100644 --- a/packages/shared/src/provider.ts +++ b/packages/shared/src/provider.ts @@ -104,6 +104,72 @@ export function formatServiceError( return `${service} request failed (${operation}): ${status}${statusText ? ` ${statusText}` : ''}`; } +// Longest caller-visible submission-rejection body returned verbatim (after +// scrubbing) - bounds how much of a large upstream error page (some +// reverse proxies return full HTML) ends up in a tool response. +const MAX_SCRUBBED_LENGTH = 2000; + +function escapeRegExp(s: string): string { + return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +/** + * Scrub URL-shaped content out of caller-visible text: every http(s) URL, + * plus - given a caller-supplied list of known configured endpoints - any + * bare occurrence of one of their hostnames, with or without a scheme. Bare + * hostnames matter because the shape a raw reverse-proxy or gateway error + * page most often uses is not a URL at all ("could not connect to + * ogmios.vector.mainnet.apexfusion.org:1337"), which the generic + * https?:// pattern alone would miss. Caps the result length so a large + * upstream error page cannot flood a tool response. + * + * Exported and pure so it can be tested without a network round trip - same + * rationale as parseSubmitResponse above. + */ +export function scrubUrls(text: string, knownHosts: string[] = []): string { + let scrubbed = text.replace(/https?:\/\/\S+/gi, '[endpoint]'); + for (const host of knownHosts) { + if (!host) continue; + let hostname = host; + try { + hostname = new URL(host).hostname || host; + } catch { + // Not a full URL - treat the value itself as the bare hostname. + } + if (!hostname) continue; + scrubbed = scrubbed.replace(new RegExp(escapeRegExp(hostname), 'gi'), '[endpoint]'); + } + return scrubbed.length > MAX_SCRUBBED_LENGTH + ? `${scrubbed.slice(0, MAX_SCRUBBED_LENGTH)} [truncated]` + : scrubbed; +} + +/** + * Format a transaction-submission rejection for the caller. + * + * Unlike formatServiceError (an Ogmios/Koios QUERY failure - our own + * infrastructure's problem, never the caller's), a submit-api rejection is + * the ledger's verdict on the CALLER'S OWN transaction: ValueNotConservedUTxO, + * BadInputsUTxO, ScriptWitnessNotValidating, a missing witness, a fee too + * small. That reason is caller information, not operator information - it is + * the entire feedback loop an agent uses to self-correct after + * build -> sign -> submit, and dropping it to a bare status code breaks the + * workflow this whole non-custodial migration exists to serve. So the body + * survives here, scrubbed of URL-shaped content (scrubUrls) rather than + * discarded outright: the body is still attacker-adjacent (an internal or + * misconfigured endpoint's own response), even though its content is + * otherwise legitimate to return. + */ +export function formatSubmitRejection( + status: number, + statusText: string, + body: string, + knownHosts: string[] = [], +): string { + const suffix = statusText ? ` ${statusText}` : ''; + return `Transaction submission rejected (${status}${suffix}): ${scrubUrls(body, knownHosts)}`; +} + export class OgmiosProvider implements Provider { private ogmiosUrl: string; private submitUrl: string; @@ -543,7 +609,14 @@ export class OgmiosProvider implements Provider { if (!response.ok) { const errorText = await response.text(); console.error(`[OgmiosProvider] Transaction submission failed at ${this.submitUrl}: ${response.status} ${response.statusText} - ${errorText}`); - throw new Error(formatServiceError('Submit API', 'submitTx', response.status, response.statusText)); + throw new Error( + formatSubmitRejection( + response.status, + response.statusText, + errorText, + [this.ogmiosUrl, this.submitUrl, this.koiosUrl].filter((u): u is string => Boolean(u)), + ), + ); } const result = await response.text(); diff --git a/packages/shared/test/provider-error-format.test.ts b/packages/shared/test/provider-error-format.test.ts index e7a4e93..a505a4d 100644 --- a/packages/shared/test/provider-error-format.test.ts +++ b/packages/shared/test/provider-error-format.test.ts @@ -1,16 +1,24 @@ import { describe, test } from 'node:test'; import assert from 'node:assert/strict'; -import { formatServiceError } from '../src/provider.ts'; +import { formatServiceError, scrubUrls, formatSubmitRejection } from '../src/provider.ts'; import { VECTOR_OGMIOS_URL, VECTOR_KOIOS_URL, VECTOR_SUBMIT_URL } from '../src/config.ts'; // The concern (audit E): a hosted deployment's internal Ogmios/Koios/submit-api -// endpoint reaching an untrusted caller through error text - either the -// configured URL itself, or a raw response body from a misconfigured/internal -// endpoint (a reverse-proxy error page, for instance) that can echo back -// hostnames or other operator-only detail. formatServiceError is the pure -// function every throw site in provider.ts now routes its caller-visible -// message through; the full detail (status, body, and which endpoint) goes to -// console.error instead, at each call site. +// endpoint reaching an untrusted caller through error text. +// +// Two distinct cases, two distinct answers: +// +// - An Ogmios/Koios QUERY failure is OUR infrastructure's problem, never the +// caller's - the caller cannot act on it, so it is sanitised down to +// service name + status only (formatServiceError). Never the URL, never +// the response body. +// - A submit-api REJECTION is the ledger's verdict on the CALLER'S OWN +// transaction (ValueNotConservedUTxO, BadInputsUTxO, a missing witness, +// fee too small) - that reason is caller information, the feedback loop an +// agent needs to self-correct after build -> sign -> submit. It survives, +// scrubbed of URL-shaped content only (formatSubmitRejection + scrubUrls), +// since the body is still attacker-adjacent even though its content is +// otherwise legitimate to return. describe('formatServiceError', () => { test('names the service, operation, and HTTP status', () => { @@ -20,15 +28,11 @@ describe('formatServiceError', () => { ); }); - test('covers Koios and the submit API with the same shape', () => { + test('covers Koios with the same shape', () => { assert.equal( formatServiceError('Koios', 'tx_status', 502, 'Bad Gateway'), 'Koios request failed (tx_status): 502 Bad Gateway', ); - assert.equal( - formatServiceError('Submit API', 'submitTx', 400, 'Bad Request'), - 'Submit API request failed (submitTx): 400 Bad Request', - ); }); test('omits the status text when the caller has none (a JSON-RPC error code, not an HTTP status)', () => { @@ -47,7 +51,6 @@ describe('formatServiceError', () => { const messages = [ formatServiceError('Ogmios', 'queryLedgerState/utxo', 503, 'Service Unavailable'), formatServiceError('Koios', 'address_txs', 500, 'Internal Server Error'), - formatServiceError('Submit API', 'submitTx', 400, 'Bad Request'), ]; for (const url of [VECTOR_OGMIOS_URL, VECTOR_KOIOS_URL, VECTOR_SUBMIT_URL]) { for (const msg of messages) { @@ -61,9 +64,81 @@ describe('formatServiceError', () => { // echoes infrastructure detail no caller should see. formatServiceError // is never given the body, so it cannot reproduce it - this pins that a // future edit cannot casually splice ${text} back into the template. + // (This is deliberately NOT true of formatSubmitRejection below - a + // submit-api body is caller information, scrubbed rather than dropped.) const leakyBody = 'upstream connect error: could not connect to ogmios-internal.vector.svc.cluster.local:1337'; const msg = formatServiceError('Ogmios', 'queryLedgerState/utxo', 502, 'Bad Gateway'); assert.ok(!msg.includes(leakyBody)); assert.ok(!msg.includes('cluster.local')); }); }); + +describe('scrubUrls', () => { + test('replaces a full http(s) URL with [endpoint]', () => { + const text = 'upstream connect error: could not connect to https://ogmios.internal.example.com:1337/health'; + const scrubbed = scrubUrls(text); + assert.ok(!scrubbed.includes('https://')); + assert.ok(!scrubbed.includes('ogmios.internal.example.com')); + assert.ok(scrubbed.includes('[endpoint]')); + }); + + test('replaces a bare configured host with no scheme, given knownHosts', () => { + // The shape a raw reverse-proxy error page most often actually uses - + // no "https://" prefix at all, just the bare hostname (optionally with a + // port), which the generic URL pattern alone would miss. + const text = 'upstream connect error: could not connect to ogmios.vector.mainnet.apexfusion.org:1337'; + const scrubbed = scrubUrls(text, ['https://ogmios.vector.mainnet.apexfusion.org']); + assert.ok(!scrubbed.includes('ogmios.vector.mainnet.apexfusion.org'), `host survived scrubbing: ${scrubbed}`); + assert.ok(scrubbed.includes('[endpoint]')); + }); + + test('accepts a bare hostname (no scheme) in knownHosts too', () => { + const text = 'DNS lookup failed for koios.vector.testnet.apexfusion.org'; + const scrubbed = scrubUrls(text, ['koios.vector.testnet.apexfusion.org']); + assert.ok(!scrubbed.includes('koios.vector.testnet.apexfusion.org')); + }); + + test('leaves ordinary text with no URL-shaped content untouched', () => { + assert.equal(scrubUrls('ValueNotConservedUTxO: inputs 5000000, outputs 5100000'), 'ValueNotConservedUTxO: inputs 5000000, outputs 5100000'); + }); + + test('caps the result length so a large upstream page cannot flood a response', () => { + const huge = 'x'.repeat(5000); + const scrubbed = scrubUrls(huge); + assert.ok(scrubbed.length < 5000, `expected truncation, got length ${scrubbed.length}`); + assert.ok(scrubbed.endsWith('[truncated]')); + }); +}); + +describe('formatSubmitRejection', () => { + test('message shape names the rejection, the status, and keeps the body', () => { + assert.equal( + formatSubmitRejection(400, 'Bad Request', 'BadInputsUTxO'), + 'Transaction submission rejected (400 Bad Request): BadInputsUTxO', + ); + }); + + test('keeps a representative ledger reason - the exact feedback loop a caller needs to self-correct', () => { + const body = '{"code":"ValueNotConservedUTxO","message":"inputs 5000000 lovelace, outputs 5100000 lovelace"}'; + const msg = formatSubmitRejection(400, 'Bad Request', body); + assert.ok(msg.includes('ValueNotConservedUTxO'), 'ledger rejection reason must survive for the caller to self-correct'); + assert.ok(msg.includes('5100000')); + }); + + test('removes a URL embedded in that same body while keeping the ledger reason next to it', () => { + const body = 'ValueNotConservedUTxO at https://ogmios.vector.mainnet.apexfusion.org/internal-detail: inputs 5000000, outputs 5100000'; + const msg = formatSubmitRejection(400, 'Bad Request', body); + assert.ok(msg.includes('ValueNotConservedUTxO'), 'ledger reason must survive'); + assert.ok(msg.includes('5100000'), 'ledger reason detail must survive'); + assert.ok(!msg.includes('https://'), 'URL scheme must not survive'); + assert.ok(!msg.includes('ogmios.vector.mainnet.apexfusion.org'), 'endpoint host must not survive'); + }); + + test('scrubs every configured endpoint passed as knownHosts, not just the submit one', () => { + const body = `mirrored via ${VECTOR_OGMIOS_URL} and ${VECTOR_KOIOS_URL} - BadInputsUTxO`; + const msg = formatSubmitRejection(400, 'Bad Request', body, [VECTOR_OGMIOS_URL, VECTOR_SUBMIT_URL, VECTOR_KOIOS_URL]); + assert.ok(msg.includes('BadInputsUTxO')); + assert.ok(!msg.includes(VECTOR_OGMIOS_URL)); + assert.ok(!msg.includes(VECTOR_KOIOS_URL)); + }); +});