From 83de69f6f38e0525ae33c7b687233923a6e950cb Mon Sep 17 00:00:00 2001 From: FilipBl4gojevic <84703395+FilipBl4gojevic@users.noreply.github.com> Date: Tue, 28 Jul 2026 19:00:34 +0200 Subject: [PATCH 01/11] feat(builder): bearer-token auth, enforced when configured Hosted deployments set MCP_AUTH_TOKENS; localhost self-hosters are unaffected. The resolved identity doubles as the rate-limit bucket key. Rejection reasons never echo the supplied token, and a bare token's log label is a hash prefix. Hardened beyond the brief after an adversarial pass (20 additional tests): - A malformed MCP_AUTH_TOKENS that parses to zero usable identities (e.g. ",,," from a broken template) now throws instead of silently resolving to `enabled: false` - the exact "operator believes auth is on, server is actually open" failure shape this module exists to prevent. An explicitly empty/unset value still disables cleanly, unchanged. - A token containing embedded whitespace is now rejected at config-load time: resolveIdentity's \S+ match can never span it, so without this check the token would be configured yet permanently unpresentable. - Documented and pinned two non-fixable format limitations: only the first colon in an entry separates label from token (a label can't itself contain one), and a literal comma inside a label/token silently fragments the entry since the delimiter has no escape mechanism. - Pinned correct existing behaviour: exact case-sensitive token match, no prefix/superstring matching, Map-backed storage is immune to __proto__/constructor prototype pollution, one label may legitimately cover multiple tokens (rotation), empty-string header treated as missing. - Map.get token lookup is not constant-time; judged acceptable and documented inline rather than swapped for crypto.timingSafeEqual - see the comment above the lookup in auth.ts for the reasoning. Co-Authored-By: Claude Opus 5 --- packages/builder/src/auth.ts | 121 +++++++++++++++ packages/builder/test/auth.test.ts | 233 +++++++++++++++++++++++++++++ 2 files changed, 354 insertions(+) create mode 100644 packages/builder/src/auth.ts create mode 100644 packages/builder/test/auth.test.ts diff --git a/packages/builder/src/auth.ts b/packages/builder/src/auth.ts new file mode 100644 index 0000000..3d809a7 --- /dev/null +++ b/packages/builder/src/auth.ts @@ -0,0 +1,121 @@ +// Bearer-token auth for the hosted builder. +// +// Enforced only when MCP_AUTH_TOKENS is configured: hosted deployments set it, +// localhost self-hosters are unaffected. The resolved identity is also the +// rate-limit bucket key, so limits are per-caller rather than per-process. +// +// A token is a secret. It must never appear in a log line, an error message, +// or a rejection reason. +import { createHash } from 'node:crypto'; + +export interface AuthConfig { + enabled: boolean; + /** token -> non-secret identity label */ + identities: Map; +} + +export const ANONYMOUS = 'anonymous'; + +/** Stable, non-reversible label for a bare token, so logs can name a caller safely. */ +function labelForToken(token: string): string { + return `client-${createHash('sha256').update(token).digest('hex').slice(0, 8)}`; +} + +// Parsing notes for MCP_AUTH_TOKENS (comma-separated "label:token" or bare-token entries): +// +// - Only the FIRST colon in an entry separates label from token. Everything after it, +// including further colons, belongs to the token (so a token may itself contain colons, +// but a label never can - "team:alpha:tok" means label "team", token "alpha:tok"). +// - Comma is the entry delimiter and cannot be escaped. A label or token that itself +// contains a literal comma will silently fragment into extra, unintended entries +// (e.g. "alice:correct,horse,battery,staple" yields FOUR entries, three of them +// unintended standalone bearer tokens). Generate tokens from a comma-free charset +// (hex / base64url / alphanumeric) to avoid this - there is no way for this parser +// to detect the fragmentation after the fact, since the comma is already consumed +// by the top-level split before any entry is inspected. +export function loadAuthConfig(env: NodeJS.ProcessEnv = process.env): AuthConfig { + const raw = (env.MCP_AUTH_TOKENS ?? '').trim(); + if (raw === '') return { enabled: false, identities: new Map() }; + + const identities = new Map(); + for (const entry of raw.split(',')) { + const trimmed = entry.trim(); + if (trimmed === '') continue; + + const sep = trimmed.indexOf(':'); + const label = sep === -1 ? undefined : trimmed.slice(0, sep).trim(); + const token = sep === -1 ? trimmed : trimmed.slice(sep + 1).trim(); + + if (token === '') { + throw new Error( + 'MCP_AUTH_TOKENS contains an entry with an empty token. Use "label:token" or a bare token.' + ); + } + if (/\s/.test(token)) { + // resolveIdentity matches the token with \S+, which cannot span whitespace (this + // includes ASCII space/tab and Unicode space separators, but not the zero-width + // space U+200B - the token would be configured yet could never be presented via + // "Authorization: Bearer ". Fail loudly now instead of creating a dead entry. + throw new Error( + 'MCP_AUTH_TOKENS contains a token with embedded whitespace, which can never be ' + + 'presented in an Authorization header. Remove the whitespace from the token value.' + ); + } + if (identities.has(token)) { + // Silently overwriting would merge two callers into one rate-limit bucket. + throw new Error('MCP_AUTH_TOKENS contains a duplicate token.'); + } + identities.set(token, label && label !== '' ? label : labelForToken(token)); + } + + if (identities.size === 0) { + // raw was non-empty (the early return above already handled unset/blank), yet every + // comma-delimited segment parsed to nothing (e.g. ",,," or " , , "). That is almost + // certainly a broken template or interpolation, not an intentional choice - unlike an + // explicitly empty/unset MCP_AUTH_TOKENS, which is the documented way to disable auth. + // Failing loudly here matters: the alternative is silently treating a botched "enable + // auth" attempt as "auth disabled", exposing an unauthenticated server the operator + // believed was locked down. + throw new Error( + 'MCP_AUTH_TOKENS is set but contains no usable tokens (only empty/blank entries after ' + + 'parsing). Leave it unset to disable auth, or provide at least one "label:token" or bare token.' + ); + } + return { enabled: true, identities }; +} + +export function resolveIdentity( + authHeader: string | undefined, + config: AuthConfig +): { ok: true; identity: string } | { ok: false; reason: string } { + if (!config.enabled) return { ok: true, identity: ANONYMOUS }; + + if (!authHeader) { + return { ok: false, reason: 'Missing Authorization header. Expected: Authorization: Bearer ' }; + } + + const match = /^Bearer (\S+)$/i.exec(authHeader); + if (!match) { + return { ok: false, reason: 'Malformed Authorization header. Expected: Authorization: Bearer ' }; + } + + // Map.get is not constant-time (a hash step, then a native string-equality check on a + // bucket hit that may short-circuit on the first mismatched byte). Judged acceptable here, + // not applied blindly: the hash step's cost depends on the LENGTH of the caller-supplied + // string, which the caller already knows, not on how many of its leading bytes match a + // stored token, so it leaks no "getting warmer" gradient the way a naive sequential + // comparison of one fixed secret would. A false hash-bucket hit against a specific + // high-entropy token is not a practical target, and the comparison sits behind an HTTP + // round trip (Task 2), where network jitter dwarfs any nanosecond-scale residual signal. + // crypto.timingSafeEqual defends a different shape (one supplied value against one known + // secret via naive comparison); it does not fit a multi-key lookup without a linear scan + // over every configured token on every request, which is a worse trade for a risk that + // is not actually present here. + const identity = config.identities.get(match[1]); + if (!identity) { + // Never echo the supplied value. + return { ok: false, reason: 'Unrecognised bearer token.' }; + } + + return { ok: true, identity }; +} diff --git a/packages/builder/test/auth.test.ts b/packages/builder/test/auth.test.ts new file mode 100644 index 0000000..4418453 --- /dev/null +++ b/packages/builder/test/auth.test.ts @@ -0,0 +1,233 @@ +import { describe, test } from 'node:test'; +import assert from 'node:assert/strict'; +import { loadAuthConfig, resolveIdentity } from '../src/auth.ts'; + +describe('loadAuthConfig', () => { + test('is disabled when MCP_AUTH_TOKENS is unset', () => { + assert.equal(loadAuthConfig({}).enabled, false); + }); + + test('is disabled when MCP_AUTH_TOKENS is empty or whitespace', () => { + assert.equal(loadAuthConfig({ MCP_AUTH_TOKENS: '' }).enabled, false); + assert.equal(loadAuthConfig({ MCP_AUTH_TOKENS: ' ' }).enabled, false); + }); + + test('parses a bare token', () => { + const c = loadAuthConfig({ MCP_AUTH_TOKENS: 'secret-one' }); + assert.equal(c.enabled, true); + assert.equal(c.identities.size, 1); + assert.ok(c.identities.has('secret-one')); + }); + + test('parses label:token pairs', () => { + const c = loadAuthConfig({ MCP_AUTH_TOKENS: 'alice:tok-a,bob:tok-b' }); + assert.equal(c.identities.get('tok-a'), 'alice'); + assert.equal(c.identities.get('tok-b'), 'bob'); + }); + + test('tolerates whitespace around entries', () => { + const c = loadAuthConfig({ MCP_AUTH_TOKENS: ' alice:tok-a , bob:tok-b ' }); + assert.equal(c.identities.get('tok-a'), 'alice'); + assert.equal(c.identities.get('tok-b'), 'bob'); + }); + + test('a bare token gets a stable label that is not the token itself', () => { + const label = loadAuthConfig({ MCP_AUTH_TOKENS: 'secret-one' }).identities.get('secret-one')!; + assert.ok(!label.includes('secret-one'), 'the label must not embed the token'); + assert.equal(label, loadAuthConfig({ MCP_AUTH_TOKENS: 'secret-one' }).identities.get('secret-one')); + }); + + test('rejects a duplicate token rather than silently overwriting', () => { + assert.throws(() => loadAuthConfig({ MCP_AUTH_TOKENS: 'alice:tok,bob:tok' }), /duplicate/i); + }); + + test('rejects an entry with an empty token', () => { + assert.throws(() => loadAuthConfig({ MCP_AUTH_TOKENS: 'alice:' }), /empty token/i); + }); +}); + +describe('resolveIdentity — auth disabled', () => { + const off = loadAuthConfig({}); + + test('allows a request with no header', () => { + assert.deepStrictEqual(resolveIdentity(undefined, off), { ok: true, identity: 'anonymous' }); + }); + + test('allows a request with a bogus header', () => { + assert.deepStrictEqual(resolveIdentity('Bearer whatever', off), { ok: true, identity: 'anonymous' }); + }); +}); + +describe('resolveIdentity — auth enabled', () => { + const on = loadAuthConfig({ MCP_AUTH_TOKENS: 'alice:tok-a,bob:tok-b' }); + + test('accepts a valid bearer token and returns its label', () => { + assert.deepStrictEqual(resolveIdentity('Bearer tok-a', on), { ok: true, identity: 'alice' }); + }); + + test('is case-insensitive on the Bearer scheme', () => { + assert.deepStrictEqual(resolveIdentity('bearer tok-a', on), { ok: true, identity: 'alice' }); + }); + + test('rejects a missing header', () => { + const r = resolveIdentity(undefined, on); + assert.equal(r.ok, false); + }); + + test('rejects an unknown token', () => { + assert.equal(resolveIdentity('Bearer nope', on).ok, false); + }); + + test('rejects a valid token without the Bearer scheme', () => { + assert.equal(resolveIdentity('tok-a', on).ok, false); + }); + + test('rejects a token with trailing whitespace padding', () => { + // Exact match only. Tolerating padding here would mean two distinct + // strings resolve to one identity, which muddies rate-limit accounting. + assert.equal(resolveIdentity('Bearer tok-a ', on).ok, false); + }); + + test('a rejection reason never contains the supplied token', () => { + const r = resolveIdentity('Bearer super-secret-value', on); + assert.equal(r.ok, false); + assert.ok(!JSON.stringify(r).includes('super-secret-value'), 'must not echo the token'); + }); +}); + +// --- Adversarial pass, beyond the brief's 17-test floor --------------------- + +describe('loadAuthConfig — adversarial: delimiter and charset edge cases', () => { + test('a colon inside the token value only splits on the first colon', () => { + // "label:token" splits on the FIRST colon only; everything after belongs + // to the token. A label can therefore never itself contain a colon. + const c = loadAuthConfig({ MCP_AUTH_TOKENS: 'alice:tok:with:colons' }); + assert.equal(c.identities.size, 1); + assert.equal(c.identities.get('tok:with:colons'), 'alice'); + assert.deepStrictEqual(resolveIdentity('Bearer tok:with:colons', c), { ok: true, identity: 'alice' }); + }); + + test('handles a very long token', () => { + const token = 'a'.repeat(5000); + const c = loadAuthConfig({ MCP_AUTH_TOKENS: `bob:${token}` }); + assert.equal(c.identities.get(token), 'bob'); + assert.deepStrictEqual(resolveIdentity(`Bearer ${token}`, c), { ok: true, identity: 'bob' }); + }); + + test('accepts non-whitespace unicode characters in a token', () => { + const token = 'tok-é中文-😀'; // "tok-é中文-😀" + const c = loadAuthConfig({ MCP_AUTH_TOKENS: `bob:${token}` }); + assert.deepStrictEqual(resolveIdentity(`Bearer ${token}`, c), { ok: true, identity: 'bob' }); + }); + + test('a zero-width space (U+200B) in a token is not a bug: preserved by trim, ' + + 'excluded from neither \\s nor \\S, so config and header matching stay consistent', () => { + const token = 'tok-a​'; + const c = loadAuthConfig({ MCP_AUTH_TOKENS: `bob:${token}` }); + assert.deepStrictEqual(resolveIdentity(`Bearer ${token}`, c), { ok: true, identity: 'bob' }); + // The visually-identical token WITHOUT the invisible character must NOT match. + assert.equal(resolveIdentity('Bearer tok-a', c).ok, false); + }); + + test('rejects a token containing an embedded ASCII space', () => { + // Such a token could never be presented via "Authorization: Bearer " - + // resolveIdentity's \S+ capture cannot span a space - so it would be configured + // yet permanently unusable. Fail loudly at load time instead of creating a dead entry. + assert.throws(() => loadAuthConfig({ MCP_AUTH_TOKENS: 'alice:tok with space' }), /whitespace/i); + }); + + test('rejects a token containing an embedded tab', () => { + assert.throws(() => loadAuthConfig({ MCP_AUTH_TOKENS: 'alice:tok\twith\ttabs' }), /whitespace/i); + }); + + test('rejects a token containing an embedded non-breaking space', () => { + assert.throws(() => loadAuthConfig({ MCP_AUTH_TOKENS: 'alice:tok nbsp' }), /whitespace/i); + }); + + test('a comma inside an intended token silently fragments into extra, unintended entries ' + + '(documented delimiter limitation: comma has no escape mechanism)', () => { + // Operator intent: ONE identity "alice" with token "correct,horse,battery,staple". + // Actual result: FOUR entries. The three trailing fragments become independently + // valid bearer tokens under auto-generated identities. This is why token generators + // must avoid commas - hex/base64url/alphanumeric charsets are all safe. + const c = loadAuthConfig({ MCP_AUTH_TOKENS: 'alice:correct,horse,battery,staple' }); + assert.equal(c.identities.size, 4); + assert.equal(c.identities.get('correct'), 'alice'); + assert.ok(resolveIdentity('Bearer horse', c).ok, 'fragment "horse" became an unintended standalone valid token'); + assert.ok(resolveIdentity('Bearer battery', c).ok, 'fragment "battery" became an unintended standalone valid token'); + assert.ok(resolveIdentity('Bearer staple', c).ok, 'fragment "staple" became an unintended standalone valid token'); + }); +}); + +describe('loadAuthConfig — adversarial: malformed config must not silently disable auth', () => { + test('MCP_AUTH_TOKENS of only commas throws rather than silently disabling auth', () => { + assert.throws(() => loadAuthConfig({ MCP_AUTH_TOKENS: ',,,' }), /no usable/i); + }); + + test('MCP_AUTH_TOKENS of only whitespace and commas throws rather than silently disabling auth', () => { + assert.throws(() => loadAuthConfig({ MCP_AUTH_TOKENS: ' , , ' }), /no usable/i); + }); + + test('a genuinely empty MCP_AUTH_TOKENS still disables cleanly (contrast with the above)', () => { + // Pins the boundary: raw === '' short-circuits before the "no usable tokens" + // check even exists, so this must keep behaving like the floor tests require. + assert.equal(loadAuthConfig({ MCP_AUTH_TOKENS: '' }).enabled, false); + }); +}); + +describe('loadAuthConfig — adversarial: identity mapping', () => { + test('two entries with the same label but different tokens are both valid ' + + '(rotation / multiple devices under one identity is a legitimate pattern)', () => { + const c = loadAuthConfig({ MCP_AUTH_TOKENS: 'alice:tok-old,alice:tok-new' }); + assert.equal(c.identities.get('tok-old'), 'alice'); + assert.equal(c.identities.get('tok-new'), 'alice'); + assert.deepStrictEqual(resolveIdentity('Bearer tok-old', c), { ok: true, identity: 'alice' }); + assert.deepStrictEqual(resolveIdentity('Bearer tok-new', c), { ok: true, identity: 'alice' }); + }); + + test('a label of only whitespace falls back to a hash-derived label, like a bare token', () => { + const c = loadAuthConfig({ MCP_AUTH_TOKENS: ' :tok-a' }); + const label = c.identities.get('tok-a')!; + assert.ok(label.startsWith('client-')); + }); + + test('__proto__ and constructor as label/token values are inert (backed by a Map, not a plain object)', () => { + const c = loadAuthConfig({ MCP_AUTH_TOKENS: '__proto__:constructor' }); + assert.equal(c.identities.get('constructor'), '__proto__'); + assert.deepStrictEqual(resolveIdentity('Bearer constructor', c), { ok: true, identity: '__proto__' }); + assert.equal(Object.prototype.hasOwnProperty.call({}, 'polluted'), false); + }); +}); + +describe('resolveIdentity — adversarial', () => { + const on = loadAuthConfig({ MCP_AUTH_TOKENS: 'alice:tok-a,bob:tok-b' }); + + test('token comparison is case-sensitive (exact match only)', () => { + assert.equal(resolveIdentity('Bearer TOK-A', on).ok, false); + assert.equal(resolveIdentity('Bearer Tok-A', on).ok, false); + }); + + test('a strict prefix of a valid token is rejected', () => { + assert.equal(resolveIdentity('Bearer tok', on).ok, false); + }); + + test('a superstring of a valid token is rejected (no partial/prefix matching)', () => { + assert.equal(resolveIdentity('Bearer tok-a-extra', on).ok, false); + }); + + test('an empty string Authorization header is treated exactly the same as a missing one', () => { + assert.deepStrictEqual(resolveIdentity('', on), resolveIdentity(undefined, on)); + }); + + test('"Bearer" with no token at all is rejected as malformed', () => { + assert.equal(resolveIdentity('Bearer', on).ok, false); + assert.equal(resolveIdentity('Bearer ', on).ok, false); + }); + + test('a malformed-header rejection reason never contains the header value', () => { + const weird = 'not-even-bearer-scheme-' + 'x'.repeat(50); + const r = resolveIdentity(weird, on); + assert.equal(r.ok, false); + assert.ok(!JSON.stringify(r).includes(weird)); + }); +}); From 59d30a763952499f8d631eff7febc390a9ca6319 Mon Sep 17 00:00:00 2001 From: FilipBl4gojevic <84703395+FilipBl4gojevic@users.noreply.github.com> Date: Tue, 28 Jul 2026 19:36:04 +0200 Subject: [PATCH 02/11] test(builder): pin that config errors never echo a token All four loadAuthConfig throw sites could be made to interpolate the secret that triggered them with the suite staying fully green - the one constraint this module states twice in its own header had no regression protection. Co-Authored-By: Claude Opus 5 --- packages/builder/src/auth.ts | 3 ++ packages/builder/test/auth.test.ts | 85 +++++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/packages/builder/src/auth.ts b/packages/builder/src/auth.ts index 3d809a7..5805b8c 100644 --- a/packages/builder/src/auth.ts +++ b/packages/builder/src/auth.ts @@ -111,6 +111,9 @@ export function resolveIdentity( // secret via naive comparison); it does not fit a multi-key lookup without a linear scan // over every configured token on every request, which is a worse trade for a risk that // is not actually present here. + // Caveat: this relies on V8 hashing the whole input before any per-key comparison, which + // is an engine implementation detail, not an ECMAScript guarantee - worth re-checking if + // this ever runs on a different JS engine. const identity = config.identities.get(match[1]); if (!identity) { // Never echo the supplied value. diff --git a/packages/builder/test/auth.test.ts b/packages/builder/test/auth.test.ts index 4418453..7f6cb04 100644 --- a/packages/builder/test/auth.test.ts +++ b/packages/builder/test/auth.test.ts @@ -12,6 +12,16 @@ describe('loadAuthConfig', () => { assert.equal(loadAuthConfig({ MCP_AUTH_TOKENS: ' ' }).enabled, false); }); + // "Whitespace" above was only ever exercised with an ASCII space. .trim() strips a much + // wider Unicode set (the WhiteSpace + LineTerminator productions), so pin that the same + // disabled path is reached for representative non-ASCII members of that set too - NBSP, + // BOM and the line separator all collapse to '' under trim exactly like an ASCII space. + test('is disabled when MCP_AUTH_TOKENS is a non-ASCII whitespace character', () => { + assert.equal(loadAuthConfig({ MCP_AUTH_TOKENS: '\u00A0' }).enabled, false); // NBSP + assert.equal(loadAuthConfig({ MCP_AUTH_TOKENS: '\uFEFF' }).enabled, false); // BOM + assert.equal(loadAuthConfig({ MCP_AUTH_TOKENS: '\u2028' }).enabled, false); // line separator + }); + test('parses a bare token', () => { const c = loadAuthConfig({ MCP_AUTH_TOKENS: 'secret-one' }); assert.equal(c.enabled, true); @@ -122,7 +132,7 @@ describe('loadAuthConfig — adversarial: delimiter and charset edge cases', () test('a zero-width space (U+200B) in a token is not a bug: preserved by trim, ' + 'excluded from neither \\s nor \\S, so config and header matching stay consistent', () => { - const token = 'tok-a​'; + const token = 'tok-a\u200B'; // trailing zero-width space (U+200B) const c = loadAuthConfig({ MCP_AUTH_TOKENS: `bob:${token}` }); assert.deepStrictEqual(resolveIdentity(`Bearer ${token}`, c), { ok: true, identity: 'bob' }); // The visually-identical token WITHOUT the invisible character must NOT match. @@ -141,7 +151,7 @@ describe('loadAuthConfig — adversarial: delimiter and charset edge cases', () }); test('rejects a token containing an embedded non-breaking space', () => { - assert.throws(() => loadAuthConfig({ MCP_AUTH_TOKENS: 'alice:tok nbsp' }), /whitespace/i); + assert.throws(() => loadAuthConfig({ MCP_AUTH_TOKENS: 'alice:tok\u00A0nbsp' }), /whitespace/i); }); test('a comma inside an intended token silently fragments into extra, unintended entries ' + @@ -199,6 +209,77 @@ describe('loadAuthConfig — adversarial: identity mapping', () => { }); }); +describe('loadAuthConfig — adversarial: config-error messages never leak the value that triggered them', () => { + // Every throw site in loadAuthConfig is a candidate for exactly the leak the file's own + // header forbids ("A token is a secret. It must never appear in ... an error message"). + // Each test below places a distinctive sentinel where it would be in scope for that + // specific throw site, confirms the right error fires (so the test can't pass by hitting + // some unrelated throw), and confirms the sentinel is absent from the message. See + // pr5-task-1-report.md for a manual mutation re-run proving each test catches its site. + const SENTINEL = 'SENTINEL-must-never-appear-in-any-message'; + + test('the empty-token error never echoes the label that triggered it', () => { + // Trailing colon, no token: the sentinel lands in `label`, not `token` (token is '' + // by definition of this failure) - `label` is the variable a careless "interpolate + // whatever the operator typed" edit would most plausibly reach for here. + assert.throws(() => loadAuthConfig({ MCP_AUTH_TOKENS: `${SENTINEL}:` }), (err: unknown) => { + assert.ok(err instanceof Error); + assert.match(err.message, /empty token/i); + assert.ok(!err.message.includes(SENTINEL), 'must not echo the sentinel label'); + return true; + }); + }); + + test('the embedded-whitespace error never echoes the token that triggered it', () => { + assert.throws(() => loadAuthConfig({ MCP_AUTH_TOKENS: `alice:${SENTINEL} has a space` }), (err: unknown) => { + assert.ok(err instanceof Error); + assert.match(err.message, /whitespace/i); + assert.ok(!err.message.includes(SENTINEL), 'must not echo the sentinel token'); + return true; + }); + }); + + test('the duplicate-token error never echoes the token that triggered it', () => { + assert.throws(() => loadAuthConfig({ MCP_AUTH_TOKENS: `alice:${SENTINEL},bob:${SENTINEL}` }), (err: unknown) => { + assert.ok(err instanceof Error); + assert.match(err.message, /duplicate/i); + assert.ok(!err.message.includes(SENTINEL), 'must not echo the sentinel token'); + return true; + }); + }); + + test('the no-usable-tokens error never echoes the raw value that triggered it', () => { + // Unlike the three tests above, this throw fires only when EVERY comma-delimited + // segment trims to '' - which structurally means `raw` can never hold alphanumeric + // content at this point (any non-blank segment either succeeds, making this throw + // unreachable, or fails an earlier check first - see the report for the full proof). + // There is no way to smuggle a realistic secret in here, so the "sentinel" is a + // distinctive whitespace/comma pattern instead - still enough to prove the message + // never echoes `raw`, which is the invariant a careless `${raw}` interpolation would + // violate. Commas sit at both ends deliberately: loadAuthConfig's own top-level + // .trim() would otherwise strip a leading/trailing whitespace char, making the `raw` + // actually in scope at the throw site differ from this literal. All characters are + // written as \u escapes rather than literal bytes, deliberately - to keep this file's + // source free of invisible/exotic characters that are easy to mis-transcribe. + const distinctiveBlank = ',\t,\u00A0,\uFEFF,'; // comma, tab, comma, NBSP, comma, BOM, comma + assert.throws(() => loadAuthConfig({ MCP_AUTH_TOKENS: distinctiveBlank }), (err: unknown) => { + assert.ok(err instanceof Error); + assert.match(err.message, /no usable/i); + assert.ok(!err.message.includes(distinctiveBlank), 'must not echo the raw env value verbatim'); + // Belt-and-braces beyond the verbatim check above: a well-meaning but still-unsafe + // interpolation might run `raw` through JSON.stringify first (e.g. to make control + // characters readable in a log). That escapes the tab to the two-char sequence \t, + // which breaks the verbatim substring check above - but JSON.stringify leaves NBSP + // and BOM as literal, unescaped bytes, so checking for either of those individually + // still catches it. Confirmed empirically before relying on it: JSON.stringify of + // this exact distinctiveBlank value contains NBSP and BOM unescaped, tab escaped. + assert.ok(!err.message.includes('\u00A0'), 'must not echo the raw env value\'s NBSP, even JSON-escaped'); + assert.ok(!err.message.includes('\uFEFF'), 'must not echo the raw env value\'s BOM, even JSON-escaped'); + return true; + }); + }); +}); + describe('resolveIdentity — adversarial', () => { const on = loadAuthConfig({ MCP_AUTH_TOKENS: 'alice:tok-a,bob:tok-b' }); From 915350062969a9c548ef9ec04b586badb843190f Mon Sep 17 00:00:00 2001 From: FilipBl4gojevic <84703395+FilipBl4gojevic@users.noreply.github.com> Date: Tue, 28 Jul 2026 19:44:15 +0200 Subject: [PATCH 03/11] feat(builder): require a bearer token on both HTTP routes when auth is on POST /messages is gated too, and cross-checked against the session's own identity - otherwise a valid token for one caller could drive another caller's session by guessing its id. --- packages/builder/src/index.ts | 40 +++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/packages/builder/src/index.ts b/packages/builder/src/index.ts index 418519e..68f2f44 100644 --- a/packages/builder/src/index.ts +++ b/packages/builder/src/index.ts @@ -26,31 +26,57 @@ if (existsSync(envPath)) { import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { SSEServerTransport } from "@modelcontextprotocol/sdk/server/sse.js"; import { registerVectorTools } from "./vector/index.js"; +import { loadAuthConfig, resolveIdentity } from "./auth.js"; -function createMcpServer() { +function createMcpServer(identity: string) { const server = new McpServer({ name: "vector-mcp-server", version: "1.0.0", }); - registerVectorTools(server); + registerVectorTools(server, identity); return server; } console.error('Vector MCP Server starting...'); +const authConfig = loadAuthConfig(); +if (authConfig.enabled) { + console.error(`Auth: enabled (${authConfig.identities.size} identit${authConfig.identities.size === 1 ? 'y' : 'ies'} configured)`); +} else { + console.error('Auth: DISABLED — every caller is treated as "anonymous" and shares one rate-limit bucket.'); + console.error('Auth: set MCP_AUTH_TOKENS to require a bearer token. Do not run a public instance without it.'); +} + const PORT = parseInt(process.env.PORT || '3000'); const transports = new Map(); +const sessionIdentities = new Map(); const httpServer = createServer(async (req, res) => { const url = new URL(req.url!, `http://localhost:${PORT}`); if (req.method === 'GET' && url.pathname === '/sse') { + const auth = resolveIdentity(req.headers.authorization, authConfig); + if (!auth.ok) { + res.writeHead(401, { 'Content-Type': 'text/plain', 'WWW-Authenticate': 'Bearer' }); + res.end(auth.reason); + return; + } const transport = new SSEServerTransport('/messages', res); - const session = createMcpServer(); + const session = createMcpServer(auth.identity); + sessionIdentities.set(transport.sessionId, auth.identity); transports.set(transport.sessionId, transport); - res.on('close', () => transports.delete(transport.sessionId)); + res.on('close', () => { + transports.delete(transport.sessionId); + sessionIdentities.delete(transport.sessionId); + }); await session.connect(transport); } else if (req.method === 'POST' && url.pathname === '/messages') { + const auth = resolveIdentity(req.headers.authorization, authConfig); + if (!auth.ok) { + res.writeHead(401, { 'Content-Type': 'text/plain', 'WWW-Authenticate': 'Bearer' }); + res.end(auth.reason); + return; + } const sessionId = url.searchParams.get('sessionId'); if (!sessionId) { res.writeHead(400); @@ -63,6 +89,12 @@ const httpServer = createServer(async (req, res) => { res.end('Session not found'); return; } + // A valid token for identity A must not be able to drive identity B's session. + if (sessionIdentities.get(sessionId) !== auth.identity) { + res.writeHead(403, { 'Content-Type': 'text/plain' }); + res.end('This session belongs to a different identity.'); + return; + } await transport.handlePostMessage(req, res); } else { res.writeHead(404); From dda81ad67169d9717c023192fb475a4f669455de Mon Sep 17 00:00:00 2001 From: FilipBl4gojevic <84703395+FilipBl4gojevic@users.noreply.github.com> Date: Tue, 28 Jul 2026 19:59:24 +0200 Subject: [PATCH 04/11] feat(builder): rate limit per identity instead of per process The limiter was a module-level singleton, so one busy caller throttled every other caller on the box and a shared daily budget was trivially exhausted. Buckets are now keyed by authenticated identity, and keyed centrally so reconnecting does not hand back a clean bucket. --- packages/builder/src/vector/agent-network.ts | 26 ++++++++++-------- packages/builder/src/vector/rate-limiter.ts | 21 +++++++++++++- .../builder/src/vector/self-improvement.ts | 25 ++++++++++------- packages/builder/src/vector/vector.ts | 9 +++--- packages/builder/test/rate-limiter.test.ts | Bin 0 -> 5278 bytes 5 files changed, 54 insertions(+), 27 deletions(-) create mode 100644 packages/builder/test/rate-limiter.test.ts diff --git a/packages/builder/src/vector/agent-network.ts b/packages/builder/src/vector/agent-network.ts index 5733450..4157e59 100644 --- a/packages/builder/src/vector/agent-network.ts +++ b/packages/builder/src/vector/agent-network.ts @@ -11,7 +11,7 @@ import { } from '@apexfusion/vector-mcp-shared/config'; export { deriveNftAssetName }; import { safetyLayer } from './safety.js'; -import { rateLimiter } from './rate-limiter.js'; +import { limiterFor } from './rate-limiter.js'; // Registry constants - agent-registry v2 (audited, compliant) // Source blueprint: vector-ai-agents/agent-registry/deploy/agent-registry/plutus.json @@ -156,19 +156,21 @@ async function getRegistryAddress() { return _registryAddress; } -// ─── Rate limit wrapper ────────────────────────────────────────────────────── - -function checkRateLimit() { - const rateCheck = rateLimiter.check(); - if (!rateCheck.allowed) { - return { content: [{ type: "text", text: `Rate limit exceeded. Retry after ${rateCheck.retryAfterMs}ms.` }] }; - } - return null; -} - // ─── Tool Registration ─────────────────────────────────────────────────────── -export function registerAgentNetworkTools(server) { +export function registerAgentNetworkTools(server, identity) { + const rateLimiter = limiterFor(identity); + + // Rate limit wrapper - defined inside the register function (not at module + // scope) so this closes over the per-identity `rateLimiter` above rather + // than a module-wide global. + function checkRateLimit() { + const rateCheck = rateLimiter.check(); + if (!rateCheck.allowed) { + return { content: [{ type: "text", text: `Rate limit exceeded. Retry after ${rateCheck.retryAfterMs}ms.` }] }; + } + return null; + } // vector_get_agent_profile (read-only - no mnemonic needed) server.tool( diff --git a/packages/builder/src/vector/rate-limiter.ts b/packages/builder/src/vector/rate-limiter.ts index 7d32428..8bfc581 100644 --- a/packages/builder/src/vector/rate-limiter.ts +++ b/packages/builder/src/vector/rate-limiter.ts @@ -24,4 +24,23 @@ export class RateLimiter { } const RATE_LIMIT_PER_MINUTE = parseInt(process.env.VECTOR_RATE_LIMIT_PER_MINUTE || '60'); -export const rateLimiter = new RateLimiter(RATE_LIMIT_PER_MINUTE); + +// One bucket per identity. A process-global limiter meant one busy caller +// throttled everyone on the box; keying by identity fixes that. The registry +// is keyed rather than per-connection so a caller cannot reset its own budget +// by reconnecting. +const limiters = new Map(); + +export function limiterFor(identity: string): RateLimiter { + let limiter = limiters.get(identity); + if (!limiter) { + limiter = new RateLimiter(RATE_LIMIT_PER_MINUTE); + limiters.set(identity, limiter); + } + return limiter; +} + +/** Test-only. Clears every bucket. */ +export function resetLimiters(): void { + limiters.clear(); +} diff --git a/packages/builder/src/vector/self-improvement.ts b/packages/builder/src/vector/self-improvement.ts index 1bbe97d..9bb9b9a 100644 --- a/packages/builder/src/vector/self-improvement.ts +++ b/packages/builder/src/vector/self-improvement.ts @@ -23,7 +23,7 @@ import { VECTOR_OGMIOS_URL, VECTOR_SUBMIT_URL, VECTOR_KOIOS_URL, VECTOR_EXPLORER_URL, explorerTxLink, } from '@apexfusion/vector-mcp-shared/config'; import { safetyLayer } from './safety.js'; -import { rateLimiter } from './rate-limiter.js'; +import { limiterFor } from './rate-limiter.js'; // Agent Registry policy ID (shared across all modules) const AGENT_REGISTRY_POLICY = process.env.AGENT_REGISTRY_POLICY || 'be1a0a2912da180757ed3cd61b56bb8eab0188c19dc3c0e3912d2c01'; @@ -85,14 +85,6 @@ function lovelaceToApex(lovelace: number | bigint): string { return (Number(BigInt(String(lovelace))) / 1_000_000).toFixed(6); } -function checkRateLimit() { - const rateCheck = rateLimiter.check(); - if (!rateCheck.allowed) { - return { content: [{ type: "text", text: `Rate limit exceeded. Retry after ${rateCheck.retryAfterMs}ms.` }] }; - } - return null; -} - function scriptHashToAddress(hash: string): string { return credentialToAddress('Mainnet', { type: 'Script', hash }); } @@ -263,7 +255,20 @@ function parseEndorsementDatum(datumCbor: string): any | null { // ─── Tool Registration ────────────────────────────────────────────────────── -export function registerSelfImprovementTools(server) { +export function registerSelfImprovementTools(server, identity) { + const rateLimiter = limiterFor(identity); + + // Rate limit wrapper - defined inside the register function (not at module + // scope, where it used to live alongside the other helpers above) so this + // closes over the per-identity `rateLimiter` above rather than a + // module-wide global. + function checkRateLimit() { + const rateCheck = rateLimiter.check(); + if (!rateCheck.allowed) { + return { content: [{ type: "text", text: `Rate limit exceeded. Retry after ${rateCheck.retryAfterMs}ms.` }] }; + } + return null; + } // ─── vector_self_improvement_browse (read-only) ─────────────────────────────── diff --git a/packages/builder/src/vector/vector.ts b/packages/builder/src/vector/vector.ts index 0b79e35..c8dcffd 100644 --- a/packages/builder/src/vector/vector.ts +++ b/packages/builder/src/vector/vector.ts @@ -13,7 +13,7 @@ import { VECTOR_OGMIOS_URL, VECTOR_SUBMIT_URL, VECTOR_KOIOS_URL, VECTOR_EXPLORER_URL, explorerTxLink, } from '@apexfusion/vector-mcp-shared/config'; import { safetyLayer } from './safety.js'; -import { rateLimiter } from './rate-limiter.js'; +import { limiterFor } from './rate-limiter.js'; import { registerAgentNetworkTools } from './agent-network.js'; import { registerSelfImprovementTools } from './self-improvement.js'; import type { @@ -473,7 +473,8 @@ export async function interactWithContract( } // Register all Vector MCP tools -export function registerVectorTools(server: McpServer) { +export function registerVectorTools(server: McpServer, identity: string) { + const rateLimiter = limiterFor(identity); // vector_get_balance - Get balance for any address server.tool( @@ -1286,8 +1287,8 @@ Funds ${actionVerb} the script address. // Agent network tools (register, discover, message, profile) live in agent-network.ts // to isolate C (Cardano WASM) imports from tsc's complex type inference - registerAgentNetworkTools(server); + registerAgentNetworkTools(server, identity); // Self-Improvement Module tools (Module 6) - registerSelfImprovementTools(server); + registerSelfImprovementTools(server, identity); } diff --git a/packages/builder/test/rate-limiter.test.ts b/packages/builder/test/rate-limiter.test.ts new file mode 100644 index 0000000000000000000000000000000000000000..6434eb9fbe1255e93d82d09e668002ce2588aed0 GIT binary patch literal 5278 zcmcIo%Z?nk74=$QaUmG4YN)%W$Bqq*oHR`K`#ebVF-v_LOK-$jAI^^5lVv^MrE}(+9uDHz&?pl@A79(@WvyGqqP<+Buc+ z;X8w8E1mSsFRt4x%TAolPu|Mh8+&3gdC^SDyh8vF`T)~(M(0n*>!h<8#5*{okMH9SD4oQ(^3!66X~T zxyfwaB!p6}b)obo>0k&@^wrUq<0xUS^)vz0wbhYBP5_!_G(jK&lfp|?NbQxM@}@c7 zC4nxGFnobH_3NfQ3HRyA%$X!{ONfm5OVHOZwAU@Ct=0v2w-r9+y*Gt6a$`? z(v!ldcrjM*3@MI^>6c$MPyBfUBkgai~*f? z<9=b*IjoCcX%Ap{&00qo+C@ibvf_@Y1Retz4cHjeos^XuKzk>IYh|(1e+^-n-ewXZ zJUO9n(BGunQfbT%&0&u=h&I6j(uDUIVZ|I-LAI=^E0OOoBrm1V;6lEWKD(nk3{VXC zC$o0<9>-n>7_I;rg8HL88tlWMt*nyk1T}yF?PGvkJG7pjnOuB&<1)HznB^`>aROB- zlzxE4o{I|j9wCOSV_33db(}jVXIMNh2xkQb{`W&_vIS0erE zZ$Ewz*%1|^XmZ5J%^xWu7y!?6SutP&;A=7#nk?*bP6cpL8tTx&j>8x&+EpK~miybX zHPul5Ke_=YL?XAwIncF|uu(hYmk7(j_@rJ6{p^H2?KrV>^@%nbtDR?*{ZsH3( zRXCI5!fi#xw&<;d^(c+r%;~U&OLUdUp&wjKwN^Qbu7wA(&ga?z*y9J-937P3f~@G| zWvQHkUzrAe`WPyOR3 z{o}uX{`*hw|Eb^qn!o=?@&1=Z%YuG{NMLyQ^wQJi#d5iv#Teg2iHiAbjQ3I_B_x*Z zMU|$i2)?){F~132j6%Phwi2FLO1<&hrlVXpNPN7BW7=!=27HSV;CW7$Z4UJ5_4lt{ z{5gwNpw>sk5Qb-tTYtKo(;t?zo32BQDH|2nT{b}v!oK1}=@Feip(}L3Vdz)f4BODV z!kN%}5Nq>zH3&N}4CSZ``3@PVn?S=)udkXwS>^-1KuV*V+d}AFQR@4EZvuq;_o&wm z%mORL!;XCqTav?ux0bX5k!vsM#p()zn{^CC0TAk&HH>2j#-g2m+xkS7gvrlh+nMyq zE{0YZK5jRLE4584@^NJ8?;1AgZP1Z!?>7D;cY0Z_W1|Vqsx=MWk=}M9AMBxx7lG>5 z+Twb{GT2cRy=-bsCVQmUV1T2SX!y4HI&yiANjq&0`spldP6#Rnqb=9yHh9E+nR}Br ztalWLK#j=cd2MZ*&HZ{hlVos(g|7g}a2mE)urfwB0_puK$iv=o&pUv==0GXW9ZH#_ zODOMirNOC729O%wNM(9vz- zI(9h5Sw^qG(uxhiS4&3uN7>@Rp}Sl=}$O$7&!K3#ap5Ccs1sg@{->c z5_9)Tb3=}%gu98U*|yYqus_3Nh#U>>2;L~5F5e*})!w-|QzivGqgpGF4ywmDu|IU&>-TG^2(tzj8g~ccIlUA7iVW@P0PFQ z)?c1wnB<714R?TsUhpBIa*wz(#={A>xex7(PdOB7{eVKvmdjJ Date: Tue, 28 Jul 2026 20:57:39 +0200 Subject: [PATCH 05/11] test(builder): add integration coverage for auth gating and per-identity rate limiting Tasks 2 and 3 verified their work with throwaway scripts against a real spawned server that were never committed, so npm test protected none of it - index.ts's auth gate had zero automated coverage, and the @ts-nocheck agent-network.ts/self-improvement.ts rate-limit wiring had none beyond listTools(). Adds two hermetic integration suites (real spawned server, localhost only, no wallet/chain calls): - auth-gating.test.ts: both HTTP routes' 401/403 gating, the auth-before- session-lookup ordering, cross-identity rejection, token-leak scanning, and that the server survives all of it (the reviewer's severe finding - a missing early `return` after a 401 falls through into a second writeHead() and crashes the whole process via an unhandled rejection). - rate-limit-gating.test.ts: per-identity isolation and that exhaustion on a vector.ts tool also rate-limits agent-network.ts and self-improvement.ts tools for that identity, through the real registered tools rather than limiterFor() in isolation. Extends test/setup.ts additively (spawnServer/stopRawServer/RawServerHandle) so these tests can set MCP_AUTH_TOKENS and drive per-request Authorization headers by hand; startServer()/stopServer()/callTool() are unchanged and the existing run.test.ts still fails for exactly its pre-existing reason. Wires a new test:integration:auth script into CI's build-and-test job. All five mutations from the review were applied to a working-tree copy, built, and reverted per-mutation to prove the new tests actually fail: - removing either route's early `return` after a 401: caught (cascading ECONNREFUSED once the process crashes; a dedicated liveness test targets this directly) - the cross-identity check's undefined-lookup branch: not independently catchable - transports/sessionIdentities are always populated and cleared together, synchronously, so that state is unreachable via HTTP; documented in the report, verified a same-region operator-flip mutation IS caught cleanly - module-scope `identity` reference in self-improvement.ts: crashes the server at startup (ReferenceError at import time), caught via the before() hook - checkRateLimit restored to module scope in agent-network.ts: caught cleanly, isError:true / "rateLimiter is not defined" instead of the well-formed rate-limit text No file under any src/ changed. test:unit 67/67, test:smoke 26/26 (23 tools, 0 drift), typecheck clean, new suite 12/12. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 5 + package.json | 3 +- .../test/integration/auth-gating.test.ts | 223 ++++++++++++++++++ .../integration/rate-limit-gating.test.ts | 120 ++++++++++ packages/builder/test/setup.ts | 65 +++++ 5 files changed, 415 insertions(+), 1 deletion(-) create mode 100644 packages/builder/test/integration/auth-gating.test.ts create mode 100644 packages/builder/test/integration/rate-limit-gating.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2f5ea27..3ba3af9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,6 +43,11 @@ jobs: env: NODE_OPTIONS: --max-old-space-size=12288 + - name: Auth gating and rate-limit integration tests + run: npm run test:integration:auth + env: + NODE_OPTIONS: --max-old-space-size=12288 + typecheck: runs-on: ubuntu-latest timeout-minutes: 15 diff --git a/package.json b/package.json index 821206d..40816d2 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,8 @@ "test": "npm run test:unit", "test:unit": "node --import tsx --test packages/*/test/*.test.ts", "test:smoke": "npm run build && node --import tsx --test packages/builder/test/smoke/*.test.ts", - "test:integration": "npm run build && node --import tsx --test packages/builder/test/integration/*.test.ts" + "test:integration": "npm run build && node --import tsx --test packages/builder/test/integration/*.test.ts", + "test:integration:auth": "npm run build && node --import tsx --test packages/builder/test/integration/auth-gating.test.ts packages/builder/test/integration/rate-limit-gating.test.ts" }, "devDependencies": { "@types/node": "^22.13.1", diff --git a/packages/builder/test/integration/auth-gating.test.ts b/packages/builder/test/integration/auth-gating.test.ts new file mode 100644 index 0000000..13a48f7 --- /dev/null +++ b/packages/builder/test/integration/auth-gating.test.ts @@ -0,0 +1,223 @@ +import { describe, test, before, after } from 'node:test'; +import assert from 'node:assert/strict'; +import { request as httpRequest } from 'node:http'; +import type { IncomingHttpHeaders, IncomingMessage } from 'node:http'; +import { spawnServer, stopRawServer } from '../setup.ts'; +import type { RawServerHandle } from '../setup.ts'; + +// Covers the auth gate in packages/builder/src/index.ts against a real spawned +// server. No unit test can see this: it's entirely in the (untyped-by-tsc-only- +// in-the-sense-that-test-files-aren't-checked, but otherwise ordinary) HTTP +// request handler wiring - who returns after a 401, what order the checks run +// in, whether a token ever leaks. See .superpowers/sdd/pr5-integration-tests-report.md +// for the mutation-testing evidence that these assertions are load-bearing. +// +// Deliberately built on raw node:http, not the MCP SDK Client/SSEClientTransport: +// this file's whole point is exercising exact status codes, exact headers, and +// - for the cross-identity case - sending a *different* Authorization header on +// a POST than the one that opened the SSE session. SSEClientTransport fixes its +// headers per transport instance and hides the session id entirely, so it can't +// express that scenario at all. + +const TOKEN_ALICE = 'tok-alice-9f2d7c1a'; +const TOKEN_BOB = 'tok-bob-4e8b02f5'; +const AUTH_ENV = { MCP_AUTH_TOKENS: `alice:${TOKEN_ALICE},bob:${TOKEN_BOB}` }; + +interface RawResponse { + status: number; + headers: IncomingHttpHeaders; + body: string; +} + +interface SseResponse extends RawResponse { + sessionId: string | null; + /** Ends the underlying request/response so the server's res.on('close', ...) cleanup fires. */ + destroy: () => void; +} + +/** + * Raw GET /sse. Resolves as soon as either (a) a 200 arrives and the + * `event: endpoint` line has been read (session established, connection left + * open for the caller to POST against or destroy), or (b) the response ends + * without ever reaching (a) (the 401/etc. cases - short body, stream closes + * immediately since index.ts calls res.end() right after writeHead()). + */ +function getSse(port: number, headers: Record = {}): Promise { + return new Promise((resolve, reject) => { + const req = httpRequest({ host: '127.0.0.1', port, path: '/sse', method: 'GET', headers }, (res: IncomingMessage) => { + let buf = ''; + let settled = false; + res.setEncoding('utf8'); + res.on('data', (chunk: string) => { + buf += chunk; + if (!settled && res.statusCode === 200 && buf.includes('event: endpoint')) { + settled = true; + const m = buf.match(/data:[^\n]*[?&]sessionId=([0-9a-fA-F-]+)/); + resolve({ status: res.statusCode!, headers: res.headers, body: buf, sessionId: m ? m[1] : null, destroy: () => req.destroy() }); + } + }); + res.on('end', () => { + if (!settled) { + settled = true; + resolve({ status: res.statusCode ?? 0, headers: res.headers, body: buf, sessionId: null, destroy: () => {} }); + } + }); + res.on('error', (err) => { if (!settled) { settled = true; reject(err); } }); + }); + req.on('error', reject); + req.end(); + }); +} + +function postMessages(port: number, path: string, headers: Record, payload: string): Promise { + return new Promise((resolve, reject) => { + const body = Buffer.from(payload, 'utf8'); + const req = httpRequest({ + host: '127.0.0.1', + port, + path, + method: 'POST', + headers: { 'Content-Type': 'application/json', 'Content-Length': String(body.length), ...headers }, + }, (res: IncomingMessage) => { + let buf = ''; + res.setEncoding('utf8'); + res.on('data', (chunk: string) => { buf += chunk; }); + res.on('end', () => resolve({ status: res.statusCode ?? 0, headers: res.headers, body: buf })); + res.on('error', reject); + }); + req.on('error', reject); + req.end(body); + }); +} + +// A minimal, genuinely valid JSON-RPC notification (matches the SDK's +// JSONRPCNotificationSchema: {jsonrpc, method, params?} with .strict()). This +// is the real message a client sends right after initialize - using it (over +// some arbitrary method name) means there's no doubt handlePostMessage's own +// parsing could be what rejects it. +const INIT_NOTIFICATION = JSON.stringify({ jsonrpc: '2.0', method: 'notifications/initialized' }); + +// Every response this file observes, scanned once at the end of the +// auth-enabled block for a leaked token. Populated by record(). +const seenBodies: string[] = []; +const seenHeaderDumps: string[] = []; +function record(r: T): T { + seenBodies.push(r.body); + seenHeaderDumps.push(JSON.stringify(r.headers)); + return r; +} + +describe('auth gating - MCP_AUTH_TOKENS set', () => { + let ctx: RawServerHandle; + + before(async () => { + ctx = await spawnServer(AUTH_ENV); + }); + + after(async () => { + await stopRawServer(ctx); + }); + + test('GET /sse with no Authorization header is rejected with 401 and WWW-Authenticate: Bearer', async () => { + const r = record(await getSse(ctx.port)); + assert.equal(r.status, 401); + assert.equal(r.headers['www-authenticate'], 'Bearer'); + }); + + test('GET /sse with an unrecognised token is rejected with 401', async () => { + const r = record(await getSse(ctx.port, { Authorization: 'Bearer not-a-real-token' })); + assert.equal(r.status, 401); + assert.equal(r.headers['www-authenticate'], 'Bearer'); + }); + + test('GET /sse with a valid token connects', async () => { + const r = record(await getSse(ctx.port, { Authorization: `Bearer ${TOKEN_ALICE}` })); + assert.equal(r.status, 200); + assert.ok(r.sessionId, 'expected an "endpoint" SSE event carrying a sessionId'); + r.destroy(); + }); + + test('POST /messages with no Authorization header is rejected with 401, not 400 or 404', async () => { + // No sessionId at all - not even a bogus one. If auth were checked *after* + // session lookup (the ordering bug this test exists to pin), a missing + // sessionId would surface as 400 instead of 401. + const r = record(await postMessages(ctx.port, '/messages', {}, INIT_NOTIFICATION)); + assert.equal(r.status, 401); + assert.equal(r.headers['www-authenticate'], 'Bearer'); + }); + + test('POST /messages with a valid token for a different identity than the session owner is rejected with 403', async () => { + const sse = record(await getSse(ctx.port, { Authorization: `Bearer ${TOKEN_ALICE}` })); + assert.equal(sse.status, 200); + assert.ok(sse.sessionId); + try { + const r = record(await postMessages(ctx.port, `/messages?sessionId=${sse.sessionId}`, { Authorization: `Bearer ${TOKEN_BOB}` }, INIT_NOTIFICATION)); + assert.equal(r.status, 403); + } finally { + sse.destroy(); + } + }); + + test("POST /messages with the session's own identity is accepted", async () => { + const sse = record(await getSse(ctx.port, { Authorization: `Bearer ${TOKEN_ALICE}` })); + assert.equal(sse.status, 200); + assert.ok(sse.sessionId); + try { + const r = record(await postMessages(ctx.port, `/messages?sessionId=${sse.sessionId}`, { Authorization: `Bearer ${TOKEN_ALICE}` }, INIT_NOTIFICATION)); + assert.equal(r.status, 202); + } finally { + sse.destroy(); + } + }); + + test('no token value leaked into any captured response body, response header, or stderr', () => { + const stderr = ctx.stderr(); + for (const body of seenBodies) { + assert.ok(!body.includes(TOKEN_ALICE), 'a response body must never contain the alice token'); + assert.ok(!body.includes(TOKEN_BOB), 'a response body must never contain the bob token'); + } + for (const dump of seenHeaderDumps) { + assert.ok(!dump.includes(TOKEN_ALICE), 'a response header must never contain the alice token'); + assert.ok(!dump.includes(TOKEN_BOB), 'a response header must never contain the bob token'); + } + assert.ok(!stderr.includes(TOKEN_ALICE), "server stderr must never contain the alice token"); + assert.ok(!stderr.includes(TOKEN_BOB), "server stderr must never contain the bob token"); + }); + + test('the server process is still alive and accepting new connections after all of the above', async () => { + // The regression this whole file exists to catch: a missing early `return` + // after a 401 falls through into a second writeHead() on an already-ended + // response, throws ERR_HTTP_HEADERS_SENT as an unhandled rejection inside + // the request handler, and takes the whole process down with it - every + // other in-flight and future connection drops. A per-request status-code + // assertion can pass even when this happened to a *different* request, so + // this checks the process itself, then proves liveness by actually using it. + assert.equal(ctx.process.exitCode, null, 'server process must not have exited'); + assert.equal(ctx.process.killed, false, 'server process must not have been killed'); + const r = await getSse(ctx.port, { Authorization: `Bearer ${TOKEN_ALICE}` }); + assert.equal(r.status, 200, 'server must still accept new connections'); + r.destroy(); + }); +}); + +describe('auth gating - MCP_AUTH_TOKENS unset (pre-auth behavior unchanged)', () => { + let ctx: RawServerHandle; + + before(async () => { + // Explicit empty string, not just "don't pass the key": makes the + // disabled state independent of whatever the ambient shell/CI environment + // happens to have set, rather than relying on it happening to be unset. + ctx = await spawnServer({ MCP_AUTH_TOKENS: '' }); + }); + + after(async () => { + await stopRawServer(ctx); + }); + + test('GET /sse with no Authorization header still connects, exactly as before auth existed', async () => { + const r = await getSse(ctx.port); + assert.equal(r.status, 200); + assert.ok(r.sessionId); + r.destroy(); + }); +}); diff --git a/packages/builder/test/integration/rate-limit-gating.test.ts b/packages/builder/test/integration/rate-limit-gating.test.ts new file mode 100644 index 0000000..fec0196 --- /dev/null +++ b/packages/builder/test/integration/rate-limit-gating.test.ts @@ -0,0 +1,120 @@ +import { describe, test, before, after } from 'node:test'; +import assert from 'node:assert/strict'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; +import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js'; +import { spawnServer, stopRawServer } from '../setup.ts'; +import type { RawServerHandle } from '../setup.ts'; + +// Covers callTool()-level behavior of per-identity rate limiting, against a +// real spawned server driving the real registered tools. rate-limiter.test.ts +// unit-tests RateLimiter/limiterFor in isolation; test:smoke only ever calls +// listTools(). Neither exercises a single tool handler, so neither would +// notice checkRateLimit/rateLimiter falling out of scope in agent-network.ts +// or self-improvement.ts (both @ts-nocheck - tsc can't catch it either). See +// .superpowers/sdd/pr5-integration-tests-report.md for the mutation-testing +// evidence. +// +// Tool choice: vector_get_spend_limits (vector.ts) reads only in-process +// SafetyLayer state - confirmed by reading safety.ts, no network/wallet. +// vector_get_agent_profile (agent-network.ts) and +// vector_self_improvement_analyze_metrics (self-improvement.ts) DO reach +// Ogmios/Koios in their normal bodies, but only after their own +// checkRateLimit() call, which is the first statement in both handlers and +// returns before any network code runs. This file only ever calls those two +// once alice's budget is already exhausted, so that network-touching code is +// never reached in a passing run - confirmed empirically below (response time, +// and the exact rate-limit text rather than a network error). That is also +// exactly why this suite is hermetic and safe for CI: see the report for the +// full argument. + +const TOKEN_ALICE = 'tok-alice-rl-3c9f1a'; +const TOKEN_BOB = 'tok-bob-rl-7d2e6b'; + +const RATE_LIMIT_ENV = { + MCP_AUTH_TOKENS: `alice:${TOKEN_ALICE},bob:${TOKEN_BOB}`, + // Deliberately tiny: the first call consumes the identity's only slot, the + // second is refused, for the rest of the real 60s sliding window - which + // easily outlasts this whole test file. + VECTOR_RATE_LIMIT_PER_MINUTE: '1', +}; + +const RATE_LIMIT_PATTERN = /^Rate limit exceeded\. Retry after \d+ms\.$/; + +async function connect(port: number, token: string): Promise { + const transport = new SSEClientTransport(new URL(`http://localhost:${port}/sse`), { + requestInit: { headers: { Authorization: `Bearer ${token}` } }, + }); + const client = new Client({ name: 'rate-limit-integration-test', version: '1.0.0' }); + await client.connect(transport); + return client; +} + +interface ToolOutcome { + text: string; + isError: boolean; +} + +async function call(client: Client, name: string, args: Record = {}): Promise { + const result = await client.callTool({ name, arguments: args }); + const content = result.content as Array<{ type: string; text?: string }>; + return { text: content?.[0]?.text ?? '', isError: result.isError === true }; +} + +describe('per-identity rate limiting across real registered tools', () => { + let ctx: RawServerHandle; + let alice: Client; + let bob: Client; + + before(async () => { + ctx = await spawnServer(RATE_LIMIT_ENV); + alice = await connect(ctx.port, TOKEN_ALICE); + bob = await connect(ctx.port, TOKEN_BOB); + }); + + after(async () => { + try { await alice.close(); } catch {} + try { await bob.close(); } catch {} + await stopRawServer(ctx); + }); + + test('alice exhausts her budget on a vector.ts tool; bob is unaffected (isolation via the real tools, not just limiterFor)', async () => { + const first = await call(alice, 'vector_get_spend_limits'); + assert.equal(first.isError, false, 'first call must not be a tool error'); + assert.ok(!RATE_LIMIT_PATTERN.test(first.text), 'first call must not be rate-limited yet'); + + const second = await call(alice, 'vector_get_spend_limits'); + assert.equal(second.isError, false, 'a rate-limit refusal is a normal, well-formed result - not a thrown tool error'); + assert.match(second.text, RATE_LIMIT_PATTERN, 'alice must now be rate-limited'); + + // Bob has never called anything: this proves his bucket is independent of + // alice's, driven through a real connection and a real tool call - not an + // inference from limiterFor() in isolation (rate-limiter.test.ts already + // covers that in isolation; this covers the wiring rate-limiter.test.ts + // cannot see). + const bobFirst = await call(bob, 'vector_get_spend_limits'); + assert.equal(bobFirst.isError, false); + assert.ok(!RATE_LIMIT_PATTERN.test(bobFirst.text), "bob must be unaffected by alice's exhausted budget"); + }); + + test('alice, still exhausted, is also rate-limited on an agent-network.ts tool', async () => { + const r = await call(alice, 'vector_get_agent_profile', { agent_id: 'did:vector:agent:test:test' }); + assert.equal(r.isError, false, 'must be a well-formed rate-limit result, not a thrown error'); + assert.match( + r.text, + RATE_LIMIT_PATTERN, + 'a checkRateLimit/rateLimiter scoping regression in agent-network.ts would surface here as ' + + 'isError:true with a ReferenceError message ("rateLimiter is not defined") instead of this text' + ); + }); + + test('alice, still exhausted, is also rate-limited on a self-improvement.ts tool', async () => { + const r = await call(alice, 'vector_self_improvement_analyze_metrics', {}); + assert.equal(r.isError, false, 'must be a well-formed rate-limit result, not a thrown error'); + assert.match( + r.text, + RATE_LIMIT_PATTERN, + 'a checkRateLimit/rateLimiter scoping regression in self-improvement.ts would surface here as ' + + 'isError:true with a ReferenceError message ("rateLimiter is not defined") instead of this text' + ); + }); +}); diff --git a/packages/builder/test/setup.ts b/packages/builder/test/setup.ts index 37a7cac..c0ec336 100644 --- a/packages/builder/test/setup.ts +++ b/packages/builder/test/setup.ts @@ -54,6 +54,71 @@ export async function stopServer(ctx: ServerContext): Promise { ctx.process.kill('SIGTERM'); } +export interface RawServerHandle { + process: ChildProcess; + port: number; + /** Snapshot of everything the server has written to stderr so far. */ + stderr(): string; +} + +/** + * Spawn the built server directly, without connecting an SDK client. + * + * startServer() above always connects a default Client with no Authorization + * header, which is exactly right for the pre-existing suites (auth is never + * configured there) but wrong for auth-gating/rate-limit tests: those need + * MCP_AUTH_TOKENS set, several distinct Authorization headers driven by hand + * (including deliberately-wrong ones), and a captured stderr stream to prove + * no token leaks into it. A bare "add an env param" to startServer() can't + * serve those needs without either connecting a client that immediately 401s + * (auth on, no header) or changing ServerContext's shape for every existing + * caller - so this is a separate, additive function instead. + */ +export async function spawnServer(envOverrides: Record = {}): Promise { + // Different port range than startServer()'s 3100-3999, and retried on + // failure: this suite spawns more servers per run than the pre-existing + // ones, so a single random draw is more likely to collide under CI + // parallelism (multiple test files spawning concurrently). + const maxAttempts = 5; + let lastErr: Error | undefined; + for (let attempt = 0; attempt < maxAttempts; attempt++) { + const port = 4100 + Math.floor(Math.random() * 1800); + const child = spawn('node', ['build/index.js'], { + cwd: PROJECT_ROOT, + env: { ...process.env, PORT: String(port), ...envOverrides }, + stdio: ['ignore', 'ignore', 'pipe'], + }); + + let stderrBuf = ''; + child.stderr!.on('data', (chunk: Buffer) => { stderrBuf += chunk.toString(); }); + + try { + await new Promise((resolve, reject) => { + const timeout = setTimeout(() => reject(new Error('Server start timeout (60s)')), 60_000); + const onData = () => { + if (stderrBuf.includes('listening on port')) { + clearTimeout(timeout); + child.stderr!.off('data', onData); + resolve(); + } + }; + child.stderr!.on('data', onData); + child.on('error', (err) => { clearTimeout(timeout); reject(err); }); + child.on('exit', (code) => { clearTimeout(timeout); reject(new Error(`Server exited with code ${code}. stderr so far: ${stderrBuf}`)); }); + }); + return { process: child, port, stderr: () => stderrBuf }; + } catch (err) { + lastErr = err as Error; + try { child.kill('SIGKILL'); } catch {} + } + } + throw lastErr ?? new Error('spawnServer: exhausted retries'); +} + +export async function stopRawServer(handle: RawServerHandle): Promise { + handle.process.kill('SIGTERM'); +} + export async function callTool(client: Client, name: string, args: Record = {}): Promise { const result = await client.callTool({ name, arguments: args }); const content = result.content as Array<{ type: string; text?: string }>; From 2b7bc397fce27865c15397850aeada263cf712a6 Mon Sep 17 00:00:00 2001 From: FilipBl4gojevic <84703395+FilipBl4gojevic@users.noreply.github.com> Date: Wed, 29 Jul 2026 07:10:23 +0200 Subject: [PATCH 06/11] feat(builder): add submit_transaction and await_transaction Completes the keyless half of the non-custodial flow: the signer returns signed CBOR, and these broadcast it and wait for confirmation. submitTx previously returned the first 64 hex characters of the submitted CBOR when the response failed to parse - a string indistinguishable from a real transaction hash. It now refuses via a new exported parseSubmitResponse, since these tools are the first callers to actually surface that value to an agent. Extracted to module scope and tested directly (18 cases) rather than through submitTx itself, since provider.ts imports fetch from cross-fetch rather than reading the global - stubbing it would leave the real implementation live and hit a genuine DNS lookup. vector_await_transaction's polling loop is extracted into vector/poll.ts (pollUntilConfirmed) rather than left inline in the tool. Two reasons: - The original loop slept a fixed 3s every iteration regardless of budget remaining, so a short timeoutSeconds could overshoot its deadline by close to 3x (e.g. timeoutSeconds=1 could block ~3s). The sleep is now clamped to the time left before the deadline. - Extracting it makes the loop unit-testable (11 cases) with an injected fake clock, instead of only being reachable through a live network call. Does not reuse the provider's own awaitTx: it hard-codes a 60-attempt budget with no caller control and returns a bare boolean, collapsing "still pending" and "gave up" into one signal - the tool needs a configurable timeout and distinct wording for the two, since a pending tx may still land and must not be reported as failed. Judgement call on the brief's fixture: its final test asserted parseSubmitResponse(CBOR.slice(0, 64)) throws. With the simple 64-hex-char isTxHash check specified in the brief, 'ab'.repeat(32) legitimately qualifies as a hash - it IS 64 valid hex characters - so that assertion was changed to assert.equal instead of contorting the implementation to add stricter validation the brief didn't ask for. The invariant that matters still holds: parseSubmitResponse only ever receives the HTTP response body, never the submitted tx, so this value can no longer be invented by our own code from data it already held - only echoed back by a server, in which case treating it like any other bare-text hash is correct. Beyond the brief: added coverage for JSON responses in shapes the parser doesn't recognize (array/number/null/boolean/nested object/unhandled field name), the "id" field alias, a zero-budget/never-yielding poll, and that a rejection from the confirmation check propagates instead of being read as "not yet confirmed". test:unit 96/96, typecheck clean. test:smoke fails as expected (23 -> 25, naming vector_await_transaction and vector_submit_transaction as added) - regenerating that snapshot is Task 5's job. test:integration:auth 12/12 unaffected. Co-Authored-By: Claude Opus 5 --- packages/builder/src/vector/poll.ts | 46 +++++ packages/builder/src/vector/vector.ts | 91 +++++++++ packages/builder/test/poll.test.ts | 183 +++++++++++++++++++ packages/shared/src/provider.ts | 47 ++++- packages/shared/test/provider-submit.test.ts | 125 +++++++++++++ 5 files changed, 484 insertions(+), 8 deletions(-) create mode 100644 packages/builder/src/vector/poll.ts create mode 100644 packages/builder/test/poll.test.ts create mode 100644 packages/shared/test/provider-submit.test.ts diff --git a/packages/builder/src/vector/poll.ts b/packages/builder/src/vector/poll.ts new file mode 100644 index 0000000..31d16ee --- /dev/null +++ b/packages/builder/src/vector/poll.ts @@ -0,0 +1,46 @@ +// Generic bounded polling loop used by vector_await_transaction. +// +// OgmiosProvider already has an `awaitTx`, but it is not reused here: it +// hard-codes a 60-attempt budget (~3 minutes at a fixed 3s interval) with no +// caller control, and it collapses "gave up waiting" and every internal +// error into the same bare boolean. The tool needs a caller-configurable +// timeout and to tell "not yet confirmed" apart from "failed to check". + +/** + * Poll `check` on a fixed interval until it resolves true or the time + * budget elapses. + * + * Each sleep is clamped to the time remaining before the deadline, so a + * budget shorter than one interval (e.g. a 1s timeout against a 3s poll + * interval) cannot overshoot by a full extra interval. An earlier version of + * this loop slept the fixed interval unconditionally on every iteration and + * could run up to ~3x past a short requested budget. + * + * `now` and `sleep` default to the real clock and a real timer, but are + * injectable so tests can run a full multi-iteration poll deterministically + * and instantly with a fake clock, instead of waiting on the wall clock or + * driving node:test's mock timers through a chain of awaited setTimeouts + * (which does not fire cleanly across await/microtask boundaries - see + * poll.test.ts for what was tried). + * + * Never throws for "still not confirmed" - only resolves false. A rejection + * from `check` itself propagates, since that means the caller could not even + * determine the status, which is a different situation from "checked and it + * is not there yet". + */ +export async function pollUntilConfirmed( + check: () => Promise, + budgetMs: number, + intervalMs: number, + now: () => number = Date.now, + sleep: (ms: number) => Promise = (ms) => new Promise((resolve) => setTimeout(resolve, ms)), +): Promise { + const deadline = now() + budgetMs; + while (now() < deadline) { + if (await check()) return true; + const remaining = deadline - now(); + if (remaining <= 0) break; + await sleep(Math.min(intervalMs, remaining)); + } + return false; +} diff --git a/packages/builder/src/vector/vector.ts b/packages/builder/src/vector/vector.ts index c8dcffd..6f6f508 100644 --- a/packages/builder/src/vector/vector.ts +++ b/packages/builder/src/vector/vector.ts @@ -14,6 +14,7 @@ import { } from '@apexfusion/vector-mcp-shared/config'; import { safetyLayer } from './safety.js'; import { limiterFor } from './rate-limiter.js'; +import { pollUntilConfirmed } from './poll.js'; import { registerAgentNetworkTools } from './agent-network.js'; import { registerSelfImprovementTools } from './self-improvement.js'; import type { @@ -1082,6 +1083,96 @@ No transaction was submitted to the network.`, } ); + // vector_submit_transaction - submit an externally signed transaction + server.tool( + "vector_submit_transaction", + "Submit an already-signed transaction to Vector. Use this to broadcast CBOR signed elsewhere (for example by a local signer). Does not sign anything.", + { + signedTxCbor: z.string().describe("Hex-encoded CBOR of a fully signed transaction"), + }, + async ({ signedTxCbor }) => { + const rateCheck = rateLimiter.check(); + if (!rateCheck.allowed) { + return { content: [{ type: "text", text: `Rate limit exceeded. Retry after ${rateCheck.retryAfterMs}ms.` }] }; + } + try { + const provider = new OgmiosProvider({ + ogmiosUrl: VECTOR_OGMIOS_URL, + submitUrl: VECTOR_SUBMIT_URL, + koiosUrl: VECTOR_KOIOS_URL, + }); + const txHash = await provider.submitTx(signedTxCbor); + return { + content: [{ + type: "text", + text: `# Transaction Submitted + +Transaction Hash: ${txHash} + +[View on Explorer](${explorerTxLink(txHash)})`, + }], + }; + } catch (err) { + const error = err as Error; + return { + content: [{ + type: "text", + text: `Failed to submit transaction: ${error.message} + +**Troubleshooting Tips:** +1. Ensure the CBOR is a fully signed transaction, not an unsigned one +2. A transaction can only be submitted once - check the explorer if unsure +3. Verify the submit endpoint is reachable`, + }], + }; + } + } + ); + + // vector_await_transaction - wait for on-chain confirmation + server.tool( + "vector_await_transaction", + "Wait for a submitted transaction to be confirmed on Vector. Polls until it appears on-chain or the timeout elapses.", + { + txHash: z.string().describe("Transaction hash returned by vector_submit_transaction"), + timeoutSeconds: z.number().min(1).max(300).optional().describe("How long to wait before giving up (default: 120)"), + }, + async ({ txHash, timeoutSeconds }) => { + const rateCheck = rateLimiter.check(); + if (!rateCheck.allowed) { + return { content: [{ type: "text", text: `Rate limit exceeded. Retry after ${rateCheck.retryAfterMs}ms.` }] }; + } + try { + const provider = new OgmiosProvider({ + ogmiosUrl: VECTOR_OGMIOS_URL, + submitUrl: VECTOR_SUBMIT_URL, + koiosUrl: VECTOR_KOIOS_URL, + }); + const budgetMs = (timeoutSeconds ?? 120) * 1000; + const intervalMs = 3000; + const confirmed = await pollUntilConfirmed( + async () => { + const utxos = await provider.getUtxosByOutRef([{ txHash, outputIndex: 0 }]).catch(() => []); + return utxos.length > 0; + }, + budgetMs, + intervalMs, + ); + return { + content: [{ + type: "text", + text: confirmed + ? `# Transaction Confirmed\n\nTransaction Hash: ${txHash}\n\n[View on Explorer](${explorerTxLink(txHash)})` + : `# Not Confirmed Yet\n\nTransaction ${txHash} did not appear on-chain within ${timeoutSeconds ?? 120}s.\n\nThis does not mean it failed - it may still be pending. Check the explorer.\n\n[View on Explorer](${explorerTxLink(txHash)})`, + }], + }; + } catch (err) { + const error = err as Error; + return { content: [{ type: "text", text: `Failed to check transaction status: ${error.message}` }] }; + } + } + ); + // vector_get_transaction_history - Get transaction history via Koios server.tool( "vector_get_transaction_history", diff --git a/packages/builder/test/poll.test.ts b/packages/builder/test/poll.test.ts new file mode 100644 index 0000000..46cc936 --- /dev/null +++ b/packages/builder/test/poll.test.ts @@ -0,0 +1,183 @@ +import { describe, test } from 'node:test'; +import assert from 'node:assert/strict'; +import { pollUntilConfirmed } from '../src/vector/poll.ts'; + +/** + * A fake clock: `sleep` advances `t` by exactly the requested amount instead + * of waiting in real time, so a multi-iteration poll runs in microseconds + * and produces a deterministic elapsed-time trace. `now`/`sleep` are the + * same seam vector_await_transaction leaves at their defaults (Date.now and + * a real setTimeout), so this drives the exact loop the tool runs — just + * without waiting on the wall clock. + * + * (node:test's built-in `mock.timers` was tried first, per the convention in + * rate-limiter.test.ts. It works cleanly for synchronous Date reads, but + * this loop chains a fresh `setTimeout` after each awaited `check()`, and + * that next timer isn't registered until a microtask turn after `tick()` + * already returned - a single tick() only fires the one timer that existed + * when it ran, so multi-iteration polls silently stalled after the first + * attempt. Confirmed experimentally before choosing dependency injection + * instead: it is deterministic without needing to interleave manual + * microtask flushes with tick() calls.) + */ +function fakeClock(start = 0) { + let t = start; + const sleeps: number[] = []; + return { + now: () => t, + sleep: async (ms: number) => { + sleeps.push(ms); + t += ms; + }, + sleeps, + elapsed: () => t - start, + }; +} + +describe('pollUntilConfirmed', () => { + test('returns true immediately, without sleeping, when check succeeds on the first attempt', async () => { + const clock = fakeClock(); + let calls = 0; + const result = await pollUntilConfirmed( + async () => { + calls++; + return true; + }, + 9000, + 3000, + clock.now, + clock.sleep, + ); + assert.equal(result, true); + assert.equal(calls, 1); + assert.deepEqual(clock.sleeps, []); + }); + + test('returns true on a later attempt once check starts succeeding', async () => { + const clock = fakeClock(); + let calls = 0; + const result = await pollUntilConfirmed( + async () => { + calls++; + return calls >= 3; + }, + 9000, + 3000, + clock.now, + clock.sleep, + ); + assert.equal(result, true); + assert.equal(calls, 3); + assert.deepEqual(clock.sleeps, [3000, 3000]); + }); + + test('returns false, not an error, when the budget elapses without confirmation', async () => { + const clock = fakeClock(); + const result = await pollUntilConfirmed(async () => false, 9000, 3000, clock.now, clock.sleep); + assert.strictEqual(result, false); + }); + + test('a transaction that never confirms resolves false, never throws and never resolves true', async () => { + // vector_await_transaction treats `false` as "not yet, might still land" + // and reports that in its own wording - never "failed". That framing + // depends on this function having exactly two outcomes (true/false), no + // third "error" signal smuggled through a throw for the ordinary + // still-pending case. + const clock = fakeClock(); + await assert.doesNotReject(() => + pollUntilConfirmed(async () => false, 6000, 3000, clock.now, clock.sleep), + ); + }); + + test('total simulated elapsed time equals the budget exactly when it divides evenly by the interval', async () => { + const clock = fakeClock(); + await pollUntilConfirmed(async () => false, 9000, 3000, clock.now, clock.sleep); + assert.equal(clock.elapsed(), 9000); + }); + + test('does not overshoot a budget smaller than one interval - the historical bug', async () => { + // The original inline loop slept a fixed 3000ms every iteration + // regardless of how much budget was left, so a timeoutSeconds=1 request + // would actually block for ~3s - a 3x overshoot. This pins the fix: the + // sleep is clamped to the time remaining before the deadline. + const clock = fakeClock(); + const result = await pollUntilConfirmed(async () => false, 1000, 3000, clock.now, clock.sleep); + assert.equal(result, false); + assert.deepEqual(clock.sleeps, [1000], 'sleep was clamped to the 1000ms remaining, not the full 3000ms interval'); + assert.equal(clock.elapsed(), 1000, 'elapsed time matches the requested budget, not the interval'); + }); + + test('does not overshoot when the budget is not an exact multiple of the interval', async () => { + const clock = fakeClock(); + const result = await pollUntilConfirmed(async () => false, 7000, 3000, clock.now, clock.sleep); + assert.equal(result, false); + assert.deepEqual(clock.sleeps, [3000, 3000, 1000]); + assert.equal(clock.elapsed(), 7000); + }); + + test('a zero budget returns false without ever calling check or sleeping', async () => { + const clock = fakeClock(); + let calls = 0; + const result = await pollUntilConfirmed( + async () => { + calls++; + return false; + }, + 0, + 3000, + clock.now, + clock.sleep, + ); + assert.equal(result, false); + assert.equal(calls, 0); + assert.deepEqual(clock.sleeps, []); + }); + + test('always yields between attempts instead of busy-spinning: call count is bounded by budget/interval', async () => { + const clock = fakeClock(); + let calls = 0; + await pollUntilConfirmed( + async () => { + calls++; + return false; + }, + 30_000, + 3000, + clock.now, + clock.sleep, + ); + // 30000 / 3000 = 10 attempts before the deadline is reached - bounded + // and small, not the thousands of calls a non-yielding spin would rack + // up. Every attempt above also awaits `check()` itself (an async + // function), so even a near-zero interval could not busy-loop the event + // loop synchronously. + assert.equal(calls, 10); + }); + + test('propagates a rejection from check rather than treating it as "not yet confirmed"', async () => { + const clock = fakeClock(); + await assert.rejects( + () => + pollUntilConfirmed( + async () => { + throw new Error('boom'); + }, + 9000, + 3000, + clock.now, + clock.sleep, + ), + /boom/, + ); + }); + + test('uses real Date.now and a real timer by default when no clock is injected', async () => { + // Smoke test for the production path: the default parameters (used by + // vector_await_transaction's real call site) must work without a test + // ever having to supply now/sleep explicitly. + const start = Date.now(); + const result = await pollUntilConfirmed(async () => true, 5000, 3000); + assert.equal(result, true); + assert.ok(Date.now() - start < 1000, 'first attempt succeeds without waiting a full interval'); + }); +}); diff --git a/packages/shared/src/provider.ts b/packages/shared/src/provider.ts index 5e893b6..8d787fe 100644 --- a/packages/shared/src/provider.ts +++ b/packages/shared/src/provider.ts @@ -21,6 +21,44 @@ interface OgmiosProviderConfig { koiosUrl?: string; } +/** + * Interpret a submit-api response body as a transaction hash. + * + * Exported and pure so it can be tested without a network round trip — the + * class method that calls it cannot be, because provider.ts imports `fetch` + * from cross-fetch rather than reading the global. + * + * Refuses rather than guessing. The previous implementation fell back to the + * first 64 characters of the *submitted CBOR*, which is a well-formed 64-char + * hex string and therefore indistinguishable from a real hash to any caller. + */ +export function parseSubmitResponse(body: string): string { + const trimmed = body.trim(); + + // A transaction hash is 32 bytes: exactly 64 hex characters. + const isTxHash = (v: unknown): v is string => + typeof v === 'string' && /^[0-9a-fA-F]{64}$/.test(v); + + try { + const parsed = JSON.parse(trimmed); + if (isTxHash(parsed)) return parsed; + const candidate = parsed?.txId ?? parsed?.txHash ?? parsed?.id; + if (isTxHash(candidate)) return candidate; + } catch { + // Not JSON — fall through to the bare-text form below. + } + + const bare = trimmed.replace(/^"|"$/g, ''); + if (isTxHash(bare)) return bare; + + // Deliberately does not claim the submission failed: the request succeeded, + // only the response was unreadable. + throw new Error( + 'Transaction was submitted but the response could not be parsed as a transaction hash. ' + + 'The transaction may or may not have been accepted - check the explorer before retrying.' + ); +} + export class OgmiosProvider implements Provider { private ogmiosUrl: string; private submitUrl: string; @@ -413,14 +451,7 @@ export class OgmiosProvider implements Provider { } const result = await response.text(); - // submit-api returns the tx hash as a JSON string - try { - const parsed = JSON.parse(result); - return typeof parsed === 'string' ? parsed : parsed.txId || parsed.txHash || tx.slice(0, 64); - } catch { - // If response is plain text tx hash - return result.replace(/"/g, '').trim(); - } + return parseSubmitResponse(result); } /** diff --git a/packages/shared/test/provider-submit.test.ts b/packages/shared/test/provider-submit.test.ts new file mode 100644 index 0000000..404fd8e --- /dev/null +++ b/packages/shared/test/provider-submit.test.ts @@ -0,0 +1,125 @@ +import { describe, test } from 'node:test'; +import assert from 'node:assert/strict'; +import { parseSubmitResponse } from '../src/provider.ts'; + +const CBOR = 'ab'.repeat(200); // 400 hex chars — its first 64 look exactly like a hash +const HASH = 'a'.repeat(64); + +describe('parseSubmitResponse', () => { + test('accepts a JSON-quoted hash', () => { + assert.equal(parseSubmitResponse(`"${HASH}"`), HASH); + }); + + test('accepts a bare-text hash', () => { + assert.equal(parseSubmitResponse(HASH), HASH); + }); + + test('tolerates surrounding whitespace', () => { + assert.equal(parseSubmitResponse(` ${HASH}\n`), HASH); + }); + + test('accepts a hash carried on a JSON object field', () => { + assert.equal(parseSubmitResponse(JSON.stringify({ txId: HASH })), HASH); + assert.equal(parseSubmitResponse(JSON.stringify({ txHash: HASH })), HASH); + }); + + test('accepts a hash carried on the "id" field too', () => { + // The brief's fixture exercises txId and txHash; the implementation also + // recognizes `id` as a fallback field. Pin that third branch explicitly + // instead of leaving it covered only incidentally. + assert.equal(parseSubmitResponse(JSON.stringify({ id: HASH })), HASH); + }); + + test('accepts uppercase hex', () => { + const upper = 'A'.repeat(64); + assert.equal(parseSubmitResponse(upper), upper); + }); + + // The bug: the old code returned the first 64 chars of the SUBMITTED CBOR + // here, which is a well-formed 64-char hex string and therefore + // indistinguishable from a real hash to any caller. + test('throws on an unparseable response instead of fabricating a hash', () => { + assert.throws(() => parseSubmitResponse('not a hash at all'), /could not be parsed/i); + }); + + test('throws on an empty response', () => { + assert.throws(() => parseSubmitResponse(''), /could not be parsed/i); + }); + + test('rejects a hex string of the wrong length', () => { + assert.throws(() => parseSubmitResponse('abc123'), /could not be parsed/i); + assert.throws(() => parseSubmitResponse('a'.repeat(63)), /could not be parsed/i); + assert.throws(() => parseSubmitResponse('a'.repeat(65)), /could not be parsed/i); + }); + + test('rejects a 64-character string that is not hex', () => { + assert.throws(() => parseSubmitResponse('z'.repeat(64)), /could not be parsed/i); + }); + + test('the error never echoes the response body', () => { + try { + parseSubmitResponse('some-unexpected-body-value'); + assert.fail('expected a throw'); + } catch (err) { + assert.ok(!(err as Error).message.includes('some-unexpected-body-value')); + } + }); + + // NOTE on this test: the brief's literal assertion here was + // `assert.throws(...)`. It was changed to `assert.equal(...)` - see the + // "judgement call" section of the task report for the full reasoning. + // Short version: parseSubmitResponse only ever receives the HTTP response + // body, never the tx that was submitted, so this value can no longer be + // *invented* by our code from the request. `'ab'.repeat(32)` is also a + // genuinely well-formed 64-character hex string, so if a server's response + // literally equals it, accepting it is correct - the same as accepting any + // other bare-text hash. That is a materially different bug shape than the + // original: our own code deriving a fake answer from data it already held, + // with no server involved at all. + test('the submitted CBOR is not an input, so it cannot leak into the result', () => { + // Regression guard on the shape of the old bug: this function only ever + // sees the response, never the transaction that was sent. + assert.equal(parseSubmitResponse(CBOR.slice(0, 64)), CBOR.slice(0, 64)); + }); + + // Beyond the brief: valid JSON, but not a shape the parser recognizes. + describe('valid JSON the parser does not recognize as a hash', () => { + test('a bare JSON array throws rather than guessing', () => { + assert.throws(() => parseSubmitResponse(JSON.stringify([HASH])), /could not be parsed/i); + }); + + test('a bare JSON number throws rather than guessing', () => { + assert.throws(() => parseSubmitResponse('42'), /could not be parsed/i); + }); + + test('a bare JSON null throws rather than guessing', () => { + assert.throws(() => parseSubmitResponse('null'), /could not be parsed/i); + }); + + test('a bare JSON boolean throws rather than guessing', () => { + assert.throws(() => parseSubmitResponse('true'), /could not be parsed/i); + }); + + test('a hash nested one level too deep is not found', () => { + // Only top-level txId/txHash/id are checked. A wrapper shape like + // { result: { txId } } is deliberately not unwrapped - guessing at + // nesting is exactly the kind of leniency that produced the original + // bug. + assert.throws( + () => parseSubmitResponse(JSON.stringify({ result: { txId: HASH } })), + /could not be parsed/i, + ); + }); + + test('a hash on a field name the parser does not recognize throws', () => { + // Confirms the field allowlist is exact: `hash` is a plausible name a + // future server might use, but until the parser is explicitly taught + // about it, it must refuse rather than pattern-match any hex-looking + // value anywhere in the object. + assert.throws( + () => parseSubmitResponse(JSON.stringify({ hash: HASH })), + /could not be parsed/i, + ); + }); + }); +}); From ede813bd7455de3f43d5f4ffebf3946e6a102ed5 Mon Sep 17 00:00:00 2001 From: FilipBl4gojevic <84703395+FilipBl4gojevic@users.noreply.github.com> Date: Wed, 29 Jul 2026 07:18:40 +0200 Subject: [PATCH 07/11] test: pin the 25-tool surface after adding submit and await The snapshot guard flagged both additions, which is what it is for. Delta verified as exactly two ADDED entries with no existing schema changed. Co-Authored-By: Claude Opus 5 --- .../builder/test/smoke/tool-inventory.test.ts | 8 +++-- .../test/smoke/tool-schemas.snapshot.json | 32 +++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/packages/builder/test/smoke/tool-inventory.test.ts b/packages/builder/test/smoke/tool-inventory.test.ts index 3d67c60..ffcf808 100644 --- a/packages/builder/test/smoke/tool-inventory.test.ts +++ b/packages/builder/test/smoke/tool-inventory.test.ts @@ -14,6 +14,7 @@ import { startServer, stopServer, ServerContext } from '../setup.ts'; * no network beyond localhost — this never needs a mnemonic. */ const EXPECTED_TOOLS = [ + 'vector_await_transaction', 'vector_build_transaction', 'vector_deploy_contract', 'vector_deregister_agent', @@ -35,6 +36,7 @@ const EXPECTED_TOOLS = [ 'vector_self_improvement_submit_proposal', 'vector_send_apex', 'vector_send_tokens', + 'vector_submit_transaction', 'vector_transfer_agent', 'vector_update_agent', ]; @@ -80,11 +82,11 @@ after(async () => { }); describe('tool inventory smoke test', () => { - test('exposes exactly 23 tools', () => { + test('exposes exactly 25 tools', () => { assert.equal( actualNames.length, - 23, - `expected 23 tools, got ${actualNames.length}: ${JSON.stringify(actualNames)}` + 25, + `expected 25 tools, got ${actualNames.length}: ${JSON.stringify(actualNames)}` ); }); diff --git a/packages/builder/test/smoke/tool-schemas.snapshot.json b/packages/builder/test/smoke/tool-schemas.snapshot.json index a3d29e2..e6de772 100644 --- a/packages/builder/test/smoke/tool-schemas.snapshot.json +++ b/packages/builder/test/smoke/tool-schemas.snapshot.json @@ -1,4 +1,23 @@ { + "vector_await_transaction": { + "type": "object", + "properties": { + "txHash": { + "description": "Transaction hash returned by vector_submit_transaction", + "type": "string" + }, + "timeoutSeconds": { + "description": "How long to wait before giving up (default: 120)", + "type": "number", + "minimum": 1, + "maximum": 300 + } + }, + "required": [ + "txHash" + ], + "$schema": "http://json-schema.org/draft-07/schema#" + }, "vector_build_transaction": { "type": "object", "properties": { @@ -711,6 +730,19 @@ ], "$schema": "http://json-schema.org/draft-07/schema#" }, + "vector_submit_transaction": { + "type": "object", + "properties": { + "signedTxCbor": { + "description": "Hex-encoded CBOR of a fully signed transaction", + "type": "string" + } + }, + "required": [ + "signedTxCbor" + ], + "$schema": "http://json-schema.org/draft-07/schema#" + }, "vector_transfer_agent": { "type": "object", "properties": { From 104f08a691e11285bfaf5d3af45a86d7d2d5b433 Mon Sep 17 00:00:00 2001 From: FilipBl4gojevic <84703395+FilipBl4gojevic@users.noreply.github.com> Date: Wed, 29 Jul 2026 07:23:27 +0200 Subject: [PATCH 08/11] docs: document MCP_AUTH_TOKENS and the two new chain-operation tools Covers what Task 1 actually enforces: auth is only on when MCP_AUTH_TOKENS is set (otherwise every caller is one anonymous, shared-bucket identity), rate limits are per identity, and the token parser rejects empty tokens, whitespace, duplicates, and all-blank values at startup rather than degrading silently. Notes that vector_submit_transaction and vector_await_transaction take no mnemonic and sign nothing. Updates the tool count from 23 to 25 everywhere it appears in the README. Co-Authored-By: Claude Opus 5 --- .env.example | 5 +++++ README.md | 25 +++++++++++++++++++++---- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/.env.example b/.env.example index f9cfa5e..fc847bc 100644 --- a/.env.example +++ b/.env.example @@ -13,3 +13,8 @@ VECTOR_SPEND_LIMIT_DAILY=500000000 # 500 ADA daily limit # Audit Log VECTOR_AUDIT_LOG_PATH=./vector-audit-log.json # Path for persistent audit log (default: ./vector-audit-log.json) + +# Auth - when set, every request must carry `Authorization: Bearer `. +# Comma-separated; each entry is `label:token` or a bare token. +# Leave unset for localhost use. Set it on any instance reachable by others. +# MCP_AUTH_TOKENS=alice:replace-me,bob:replace-me-too diff --git a/README.md b/README.md index 57d018f..709cce0 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ Built on [Ogmios](https://ogmios.dev/) + [Koios](https://www.koios.rest/) - no B ## Hosted servers - no install -Hosted instances run on both networks, exposing all 23 tools: +Hosted instances run on both networks, exposing all 25 tools: | Network | Endpoint | |---------|----------| @@ -25,8 +25,10 @@ claude mcp add --transport sse vector-mcp https://mcp.vector.mainnet.apexfusion. > tool-call parameter. It therefore passes through your MCP client, your model provider, > and the hosted server's memory — it is not written to disk, but it is exposed. Use only > a hot wallet holding funds you can afford to lose, and prefer self-hosting until the -> non-custodial migration lands. See -> [docs/architecture/non-custodial-split.md](docs/architecture/non-custodial-split.md). +> non-custodial migration lands. `vector_submit_transaction` and `vector_await_transaction` +> are the exception — they take no mnemonic and sign nothing; they only broadcast a +> transaction signed elsewhere (for example by a local signer) and poll for confirmation. +> See [docs/architecture/non-custodial-split.md](docs/architecture/non-custodial-split.md). Self-hosting instructions are below. @@ -41,7 +43,7 @@ Self-hosting instructions are below. - **Safety controls** - per-transaction and daily spend limits, persistent audit log, rate limiting - **SSE transport** - HTTP server with Server-Sent Events for MCP client connectivity -## MCP Tools (23) +## MCP Tools (25) ### Wallet & Queries @@ -61,6 +63,8 @@ Self-hosting instructions are below. | `vector_send_tokens` | Send native tokens with optional AP3X | | `vector_build_transaction` | Build multi-output transactions (sign+submit or return unsigned CBOR) | | `vector_dry_run` | Simulate a transaction without submitting - estimate fees and validate | +| `vector_submit_transaction` | Broadcast an already-signed transaction (for example one signed by the local signer) | +| `vector_await_transaction` | Wait for a submitted transaction to be confirmed on-chain | ### Smart Contracts @@ -163,6 +167,19 @@ docker run -p 3000:3000 vector-mcp | `VECTOR_SPEND_LIMIT_DAILY` | Max lovelace per day | `500000000` (500 AP3X) | | `VECTOR_AUDIT_LOG_PATH` | Persistent audit log file path | `./vector-audit-log.json` | | `VECTOR_RATE_LIMIT_PER_MINUTE` | Max tool calls per minute | `60` | +| `MCP_AUTH_TOKENS` | Bearer tokens that may call this server. Comma-separated; each entry is `label:token` or a bare token. **When unset, the server is open to anyone who can reach it.** | _(unset — auth disabled)_ | + +> **Running a public instance?** Set `MCP_AUTH_TOKENS`. Without it every caller +> is treated as one anonymous identity and shares a single rate-limit bucket, so +> one busy client throttles everyone. Rate limits are applied per identity, so +> give each client its own token. + +> **Malformed values fail loudly, not silently.** The server refuses to start if +> `MCP_AUTH_TOKENS` contains an empty token, a token with embedded whitespace, a +> duplicate token, or a value where every comma-separated entry is blank (e.g. a +> stray `,,,`) — each raises a startup error that names the problem but never +> echoes a token value. Commas delimit entries and cannot be escaped, so generate +> tokens from a comma-free charset (hex / base64url / alphanumeric). ### Mainnet endpoints From ab21f32ca95577b016ddec08bc18768d35edae75 Mon Sep 17 00:00:00 2001 From: FilipBl4gojevic <84703395+FilipBl4gojevic@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:08:26 +0200 Subject: [PATCH 09/11] fix(builder): prefer Koios tx_status for await_transaction, stop swallowing check errors vector_await_transaction's confirmation check only ever looked for an unspent output at index 0 - a heuristic that can read a landed transaction as unconfirmed, since these tools broadcast CBOR signed elsewhere with no guarantee of that output existing or staying unspent. The check also wrapped the lookup in .catch(() => []), collapsing genuine infrastructure failures (Ogmios down, malformed response) into the same "not confirmed yet" result, defeating pollUntilConfirmed's documented contract that a check rejection should propagate. OgmiosProvider.getKoiosTxStatus + koiosIndicatesConfirmed (shared/provider.ts) now provide a direct, non-swallowing Koios tx_status check; poll.ts's new buildConfirmationCheck picks it when VECTOR_KOIOS_URL is configured and falls back to the outRef heuristic (also no longer swallowing errors) otherwise - the same preference order OgmiosProvider.awaitTx already used, without reusing awaitTx itself (still not caller-configurable, still swallows every failure). awaitTx stays in place only because the Provider interface requires it; nothing else calls it. buildConfirmationCheck and koiosIndicatesConfirmed are unit tested via dependency injection, the same pattern poll.test.ts already uses for now/sleep, with no network access required. --- packages/builder/src/vector/poll.ts | 27 ++++- packages/builder/src/vector/vector.ts | 16 ++- packages/builder/test/poll.test.ts | 112 +++++++++++++++++- packages/shared/src/provider.ts | 65 ++++++++++ .../shared/test/provider-confirmation.test.ts | 56 +++++++++ 5 files changed, 268 insertions(+), 8 deletions(-) create mode 100644 packages/shared/test/provider-confirmation.test.ts diff --git a/packages/builder/src/vector/poll.ts b/packages/builder/src/vector/poll.ts index 31d16ee..0db02b9 100644 --- a/packages/builder/src/vector/poll.ts +++ b/packages/builder/src/vector/poll.ts @@ -1,4 +1,5 @@ -// Generic bounded polling loop used by vector_await_transaction. +// Generic bounded polling loop used by vector_await_transaction, plus the +// logic that picks what each poll attempt actually checks. // // OgmiosProvider already has an `awaitTx`, but it is not reused here: it // hard-codes a 60-attempt budget (~3 minutes at a fixed 3s interval) with no @@ -44,3 +45,27 @@ export async function pollUntilConfirmed( } return false; } + +/** + * Choose which confirmation check `pollUntilConfirmed` should run: Koios's + * `tx_status` when configured (a direct signal), the outRef heuristic when + * it is not - the same preference order as `OgmiosProvider.awaitTx`, without + * reusing it (see the file header above). + * + * Takes both checks as already-built functions, rather than a provider and + * txHash, so the selection itself can be pinned with fakes the same way + * `pollUntilConfirmed`'s own `now`/`sleep` are injected above - no network + * call happens here. Whichever function is selected is returned as-is, not + * wrapped in a try/catch, so a rejection from it propagates unchanged + * through `pollUntilConfirmed` to the tool's outer `try/catch` - it is not + * reinterpreted as "not yet confirmed". `checkKoios` and `checkOutRef` + * carrying that same no-swallow contract is the caller's responsibility + * (see the vector_await_transaction call site). + */ +export function buildConfirmationCheck( + koiosConfigured: boolean, + checkKoios: () => Promise, + checkOutRef: () => Promise, +): () => Promise { + return koiosConfigured ? checkKoios : checkOutRef; +} diff --git a/packages/builder/src/vector/vector.ts b/packages/builder/src/vector/vector.ts index 6f6f508..9cceda7 100644 --- a/packages/builder/src/vector/vector.ts +++ b/packages/builder/src/vector/vector.ts @@ -7,14 +7,14 @@ import * as dotenv from 'dotenv'; import { resolve } from 'path'; import { fileURLToPath } from 'url'; import { existsSync } from 'fs'; -import { OgmiosProvider } from '@apexfusion/vector-mcp-shared/provider'; +import { OgmiosProvider, koiosIndicatesConfirmed } from '@apexfusion/vector-mcp-shared/provider'; import { lovelaceToAda, formatAssetName } from '@apexfusion/vector-mcp-shared/tx'; import { VECTOR_OGMIOS_URL, VECTOR_SUBMIT_URL, VECTOR_KOIOS_URL, VECTOR_EXPLORER_URL, explorerTxLink, } from '@apexfusion/vector-mcp-shared/config'; import { safetyLayer } from './safety.js'; import { limiterFor } from './rate-limiter.js'; -import { pollUntilConfirmed } from './poll.js'; +import { pollUntilConfirmed, buildConfirmationCheck } from './poll.js'; import { registerAgentNetworkTools } from './agent-network.js'; import { registerSelfImprovementTools } from './self-improvement.js'; import type { @@ -1151,10 +1151,14 @@ Transaction Hash: ${txHash} const budgetMs = (timeoutSeconds ?? 120) * 1000; const intervalMs = 3000; const confirmed = await pollUntilConfirmed( - async () => { - const utxos = await provider.getUtxosByOutRef([{ txHash, outputIndex: 0 }]).catch(() => []); - return utxos.length > 0; - }, + buildConfirmationCheck( + Boolean(VECTOR_KOIOS_URL), + async () => koiosIndicatesConfirmed(await provider.getKoiosTxStatus(txHash)), + async () => { + const utxos = await provider.getUtxosByOutRef([{ txHash, outputIndex: 0 }]); + return utxos.length > 0; + }, + ), budgetMs, intervalMs, ); diff --git a/packages/builder/test/poll.test.ts b/packages/builder/test/poll.test.ts index 46cc936..c5256e2 100644 --- a/packages/builder/test/poll.test.ts +++ b/packages/builder/test/poll.test.ts @@ -1,6 +1,6 @@ import { describe, test } from 'node:test'; import assert from 'node:assert/strict'; -import { pollUntilConfirmed } from '../src/vector/poll.ts'; +import { pollUntilConfirmed, buildConfirmationCheck } from '../src/vector/poll.ts'; /** * A fake clock: `sleep` advances `t` by exactly the requested amount instead @@ -181,3 +181,113 @@ describe('pollUntilConfirmed', () => { assert.ok(Date.now() - start < 1000, 'first attempt succeeds without waiting a full interval'); }); }); + +describe('buildConfirmationCheck', () => { + // vector_await_transaction wires this up as: + // buildConfirmationCheck(Boolean(VECTOR_KOIOS_URL), koiosCheck, outRefCheck) + // Both checkKoios/checkOutRef stand in for provider calls (Koios tx_status, + // Ogmios getUtxosByOutRef) without ever touching the network - this pins + // the selection and propagation behavior, not the HTTP calls themselves. + + test('selects checkKoios when Koios is configured, and never calls checkOutRef', async () => { + let outRefCalls = 0; + const check = buildConfirmationCheck( + true, + async () => true, + async () => { outRefCalls++; return true; }, + ); + assert.equal(await check(), true); + assert.equal(outRefCalls, 0, 'the outRef check must not run when Koios is preferred'); + }); + + test('selects checkOutRef when Koios is not configured, and never calls checkKoios', async () => { + let koiosCalls = 0; + const check = buildConfirmationCheck( + false, + async () => { koiosCalls++; return true; }, + async () => false, + ); + assert.equal(await check(), false); + assert.equal(koiosCalls, 0, 'the Koios check must not run as a fallback is not a retry path'); + }); + + test('propagates the selected check\'s resolved value unchanged, true or false, in either branch', async () => { + assert.equal(await buildConfirmationCheck(true, async () => true, async () => { throw new Error('unused'); })(), true); + assert.equal(await buildConfirmationCheck(true, async () => false, async () => { throw new Error('unused'); })(), false); + assert.equal(await buildConfirmationCheck(false, async () => { throw new Error('unused'); }, async () => true)(), true); + assert.equal(await buildConfirmationCheck(false, async () => { throw new Error('unused'); }, async () => false)(), false); + }); + + test('propagates a rejection from checkKoios rather than treating it as "not yet confirmed"', async () => { + const check = buildConfirmationCheck( + true, + async () => { throw new Error('Koios tx_status query failed (503): upstream down'); }, + async () => false, + ); + await assert.rejects(check, /Koios tx_status query failed/); + }); + + test('propagates a rejection from checkOutRef rather than treating it as "not yet confirmed"', async () => { + const check = buildConfirmationCheck( + false, + async () => false, + async () => { throw new Error('Ogmios RPC error (500): boom'); }, + ); + await assert.rejects(check, /Ogmios RPC error/); + }); + + describe('composed with pollUntilConfirmed (matching the real vector_await_transaction call site)', () => { + function fakeClock(start = 0) { + let t = start; + const sleeps: number[] = []; + return { + now: () => t, + sleep: async (ms: number) => { sleeps.push(ms); t += ms; }, + sleeps, + }; + } + + test('a Koios failure on the first attempt rejects immediately, without ever sleeping or falling back to outRef', async () => { + const clock = fakeClock(); + let outRefCalls = 0; + const check = buildConfirmationCheck( + true, + async () => { throw new Error('Koios unreachable'); }, + async () => { outRefCalls++; return true; }, + ); + await assert.rejects( + () => pollUntilConfirmed(check, 9000, 3000, clock.now, clock.sleep), + /Koios unreachable/, + ); + assert.deepEqual(clock.sleeps, [], 'a check that cannot determine status must not be retried like a pending tx'); + assert.equal(outRefCalls, 0); + }); + + test('outRef polling that never confirms resolves false (not an error) once the budget elapses, with Koios not configured', async () => { + const clock = fakeClock(); + let koiosCalls = 0; + const check = buildConfirmationCheck( + false, + async () => { koiosCalls++; return true; }, + async () => false, + ); + const result = await pollUntilConfirmed(check, 9000, 3000, clock.now, clock.sleep); + assert.equal(result, false); + assert.equal(koiosCalls, 0); + }); + + test('Koios reporting confirmed on a later attempt resolves true without exhausting the budget', async () => { + const clock = fakeClock(); + let attempts = 0; + const check = buildConfirmationCheck( + true, + async () => { attempts++; return attempts >= 2; }, + async () => { throw new Error('outRef must not run when Koios is configured'); }, + ); + const result = await pollUntilConfirmed(check, 9000, 3000, clock.now, clock.sleep); + assert.equal(result, true); + assert.equal(attempts, 2); + assert.deepEqual(clock.sleeps, [3000]); + }); + }); +}); diff --git a/packages/shared/src/provider.ts b/packages/shared/src/provider.ts index 8d787fe..8f48852 100644 --- a/packages/shared/src/provider.ts +++ b/packages/shared/src/provider.ts @@ -59,6 +59,25 @@ export function parseSubmitResponse(body: string): string { ); } +/** + * Interpret a parsed Koios `tx_status` response body as a confirmation + * signal. + * + * Exported and pure so it can be tested without a network round trip - same + * rationale as `parseSubmitResponse` above. By the time this runs, the + * caller (`OgmiosProvider.getKoiosTxStatus`) has already thrown on a genuine + * transport/HTTP failure, so anything reaching here is a syntactically valid + * response from a reachable Koios. A missing hash (empty array - Koios has + * not indexed it yet), a present-but-zero-confirmation entry, or a body that + * does not match the expected shape are all treated as "not yet confirmed" + * (false) rather than an error: none of them mean the check itself failed. + */ +export function koiosIndicatesConfirmed(data: unknown): boolean { + if (!Array.isArray(data) || data.length === 0) return false; + const entry = data[0] as { num_confirmations?: unknown } | undefined; + return typeof entry?.num_confirmations === 'number' && entry.num_confirmations > 0; +} + export class OgmiosProvider implements Provider { private ogmiosUrl: string; private submitUrl: string; @@ -286,6 +305,52 @@ export class OgmiosProvider implements Provider { throw new Error(`Datum not found for hash: ${datumHash}. Datum lookups require Koios indexer.`); } + /** + * Query Koios for a transaction's confirmation status, once. + * + * Thin and deliberately does not interpret the response - see + * `koiosIndicatesConfirmed` above, kept separate so it can be unit tested + * without a network round trip. Unlike the Koios branch inside `awaitTx` + * below (which swallows every failure into "keep polling"), this throws on + * a network failure or a non-OK response: the caller could not determine + * status, which is a different situation from "asked, and it is not + * confirmed yet", and callers such as `pollUntilConfirmed` + * (packages/builder/src/vector/poll.ts) need to be able to tell the two + * apart. + */ + async getKoiosTxStatus(txHash: string): Promise { + if (!this.koiosUrl) { + throw new Error( + 'Koios URL is not configured. Transaction status checks require Koios indexer. ' + + 'Set VECTOR_KOIOS_URL in your environment.' + ); + } + + const response = await fetch(`${this.koiosUrl}/api/v1/tx_status`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ _tx_hashes: [txHash] }), + }); + if (!response.ok) { + const text = await response.text(); + throw new Error(`Koios tx_status query failed (${response.status}): ${text}`); + } + return response.json(); + } + + /** + * Required by the @lucid-evolution/lucid `Provider` interface that this + * class implements, but otherwise unused repo-wide: `vector_await_transaction` + * builds its own poll loop on top of `getKoiosTxStatus`/`getUtxosByOutRef` + * (see packages/builder/src/vector/poll.ts) instead of calling this, + * because this method's fixed 60-attempt/3s-interval budget cannot be + * configured by a caller and it swallows every failure - network error, + * non-OK response, malformed body - into "keep polling", which is + * indistinguishable from "checked and it is not there yet". Left + * unmodified rather than deleted or reworked: TypeScript requires it to + * satisfy `implements Provider`, and changing its own retry/error + * semantics is out of scope here since nothing depends on them. + */ async awaitTx(txHash: string, checkInterval: number = 3000): Promise { // Poll for tx confirmation const maxAttempts = 60; // ~3 minutes at 3s intervals diff --git a/packages/shared/test/provider-confirmation.test.ts b/packages/shared/test/provider-confirmation.test.ts new file mode 100644 index 0000000..f75a624 --- /dev/null +++ b/packages/shared/test/provider-confirmation.test.ts @@ -0,0 +1,56 @@ +import { describe, test } from 'node:test'; +import assert from 'node:assert/strict'; +import { koiosIndicatesConfirmed } from '../src/provider.ts'; + +// koiosIndicatesConfirmed only ever sees a body that Koios already answered +// with an OK response - getKoiosTxStatus throws before this runs for a +// network failure or a non-2xx status. So every case here is "reachable and +// answered", and the only question is whether that answer means confirmed. + +describe('koiosIndicatesConfirmed', () => { + test('a positive confirmation count means confirmed', () => { + assert.equal(koiosIndicatesConfirmed([{ tx_hash: 'a'.repeat(64), num_confirmations: 1 }]), true); + }); + + test('a larger confirmation count still means confirmed', () => { + assert.equal(koiosIndicatesConfirmed([{ tx_hash: 'a'.repeat(64), num_confirmations: 42 }]), true); + }); + + test('an empty array (Koios has not indexed the hash yet) means not confirmed', () => { + assert.equal(koiosIndicatesConfirmed([]), false); + }); + + test('zero confirmations means not confirmed', () => { + assert.equal(koiosIndicatesConfirmed([{ tx_hash: 'a'.repeat(64), num_confirmations: 0 }]), false); + }); + + test('a missing num_confirmations field means not confirmed, not an error', () => { + assert.equal(koiosIndicatesConfirmed([{ tx_hash: 'a'.repeat(64) }]), false); + }); + + test('a non-array body means not confirmed, not an error', () => { + assert.equal(koiosIndicatesConfirmed(null), false); + assert.equal(koiosIndicatesConfirmed(undefined), false); + assert.equal(koiosIndicatesConfirmed({}), false); + assert.equal(koiosIndicatesConfirmed('unexpected'), false); + }); + + test('a num_confirmations that is not a number means not confirmed, not an error', () => { + // Defensive: guards against a future Koios response shape change (e.g. a + // stringified count) silently being coerced by a loose truthiness check + // instead of being treated as "cannot tell, so say not yet". + assert.equal(koiosIndicatesConfirmed([{ num_confirmations: '3' }]), false); + assert.equal(koiosIndicatesConfirmed([{ num_confirmations: null }]), false); + }); + + test('a negative confirmation count means not confirmed', () => { + assert.equal(koiosIndicatesConfirmed([{ num_confirmations: -1 }]), false); + }); + + test('only the first entry is consulted, matching the single-hash query this is always called with', () => { + assert.equal( + koiosIndicatesConfirmed([{ num_confirmations: 0 }, { num_confirmations: 5 }]), + false, + ); + }); +}); From 325494a590afe5d647ba9ee7a1eae26d53d572d2 Mon Sep 17 00:00:00 2001 From: FilipBl4gojevic <84703395+FilipBl4gojevic@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:08:35 +0200 Subject: [PATCH 10/11] docs: warn about MCP_AUTH_TOKENS at the point of going live, not just in the reference table The auth guidance was accurate but lived only in the Configuration reference table, after both quickstart paths (npm start / docker run) that a self-hoster would follow top to bottom. Following either to completion on a public host reproduces the unauthenticated-server problem this PR exists to fix, without ever seeing a warning at the point of action. Add one line after each quickstart's run command, pointing to Configuration. The Docker line also notes that docker-compose.yml in this repo already binds 127.0.0.1 by default, unlike the inline `docker run` example. --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 709cce0..8d6e6c9 100644 --- a/README.md +++ b/README.md @@ -123,6 +123,8 @@ npm start # Server listens on port 3000 (configurable via PORT env var) ``` +If this instance will be reachable by anyone but you, set `MCP_AUTH_TOKENS` first - see [Configuration](#configuration) below. + ### 4. Add to Claude Desktop Add to your Claude Desktop MCP config (`claude_desktop_config.json`): @@ -154,6 +156,8 @@ docker build -t vector-mcp . docker run -p 3000:3000 vector-mcp ``` +If this instance will be reachable by anyone but you, set `MCP_AUTH_TOKENS` first - see [Configuration](#configuration) below (`docker-compose.yml` in this repo binds `127.0.0.1` by default; the command above does not). + ## Configuration | Variable | Description | Default | From 3b2a8aba211f361eb823244bbf9645e9375789d1 Mon Sep 17 00:00:00 2001 From: FilipBl4gojevic <84703395+FilipBl4gojevic@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:09:11 +0200 Subject: [PATCH 11/11] test(builder): replace a raw NUL byte in rate-limiter.test.ts with \x00 The "unusual identity strings" fixture embedded a literal 0x00 byte in a string literal instead of the \x00 escape sequence. Consequence: the file had no valid text encoding as far as git's binary-diff heuristic is concerned, so the whole 5278-byte file rendered as "Binary files differ" with zero textual diff - invisible to review, and would have been invisible to any GitHub PR reviewer too. It also made the raw NUL character indistinguishable from the adjacent ASCII-space case when viewed directly. Same runtime string (a genuine one-character NUL string, confirmed via '\x00'.charCodeAt(0) === 0), no behavior change - the file is just diffable text again. --- packages/builder/test/rate-limiter.test.ts | Bin 5278 -> 5281 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/packages/builder/test/rate-limiter.test.ts b/packages/builder/test/rate-limiter.test.ts index 6434eb9fbe1255e93d82d09e668002ce2588aed0..830990dee3423beef109f944bd5b196ba31db4a0 100644 GIT binary patch delta 17 YcmbQIxlnUMB{xeo=1TO#p