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/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8e74506..5470263 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,6 +43,9 @@ jobs: env: NODE_OPTIONS: --max-old-space-size=12288 + - name: Auth gating and rate-limit integration tests + run: npm run test:integration:auth + - name: Signer smoke test run: npm run test:smoke:signer env: diff --git a/README.md b/README.md index 91b612e..a79dba1 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. @@ -56,7 +58,7 @@ it ships, the security notice above still applies. - **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 @@ -76,6 +78,8 @@ it ships, the security notice above still applies. | `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 @@ -134,6 +138,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`): @@ -165,6 +171,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 | @@ -178,6 +186,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 diff --git a/package.json b/package.json index a126701..77690d5 100644 --- a/package.json +++ b/package.json @@ -15,6 +15,7 @@ "test:smoke": "npm run build && node --import tsx --test packages/builder/test/smoke/*.test.ts", "test:smoke:signer": "npm run build && node --import tsx --test packages/signer/test/smoke/*.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" "test:integration:signer": "node --import tsx --test packages/signer/test/integration/*.test.ts" }, "devDependencies": { diff --git a/packages/builder/src/auth.ts b/packages/builder/src/auth.ts new file mode 100644 index 0000000..5805b8c --- /dev/null +++ b/packages/builder/src/auth.ts @@ -0,0 +1,124 @@ +// 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. + // 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. + return { ok: false, reason: 'Unrecognised bearer token.' }; + } + + return { ok: true, identity }; +} 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); 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/poll.ts b/packages/builder/src/vector/poll.ts new file mode 100644 index 0000000..0db02b9 --- /dev/null +++ b/packages/builder/src/vector/poll.ts @@ -0,0 +1,71 @@ +// 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 +// 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; +} + +/** + * 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/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..9cceda7 100644 --- a/packages/builder/src/vector/vector.ts +++ b/packages/builder/src/vector/vector.ts @@ -7,13 +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 { rateLimiter } from './rate-limiter.js'; +import { limiterFor } from './rate-limiter.js'; +import { pollUntilConfirmed, buildConfirmationCheck } from './poll.js'; import { registerAgentNetworkTools } from './agent-network.js'; import { registerSelfImprovementTools } from './self-improvement.js'; import type { @@ -473,7 +474,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( @@ -1081,6 +1083,100 @@ 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( + 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, + ); + 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", @@ -1286,8 +1382,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/auth.test.ts b/packages/builder/test/auth.test.ts new file mode 100644 index 0000000..7f6cb04 --- /dev/null +++ b/packages/builder/test/auth.test.ts @@ -0,0 +1,314 @@ +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); + }); + + // "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); + 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\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. + 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\u00A0nbsp' }), /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('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' }); + + 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)); + }); +}); 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/poll.test.ts b/packages/builder/test/poll.test.ts new file mode 100644 index 0000000..c5256e2 --- /dev/null +++ b/packages/builder/test/poll.test.ts @@ -0,0 +1,293 @@ +import { describe, test } from 'node:test'; +import assert from 'node:assert/strict'; +import { pollUntilConfirmed, buildConfirmationCheck } 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'); + }); +}); + +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/builder/test/rate-limiter.test.ts b/packages/builder/test/rate-limiter.test.ts new file mode 100644 index 0000000..830990d --- /dev/null +++ b/packages/builder/test/rate-limiter.test.ts @@ -0,0 +1,118 @@ +import { describe, test, beforeEach } from 'node:test'; +import assert from 'node:assert/strict'; +import { RateLimiter, limiterFor, resetLimiters } from '../src/vector/rate-limiter.ts'; + +beforeEach(() => resetLimiters()); + +describe('RateLimiter', () => { + test('allows calls up to the limit', () => { + const l = new RateLimiter(3); + assert.equal(l.check().allowed, true); + assert.equal(l.check().allowed, true); + assert.equal(l.check().allowed, true); + }); + + test('refuses the call after the limit and reports a retry delay', () => { + const l = new RateLimiter(2); + l.check(); + l.check(); + const r = l.check(); + assert.equal(r.allowed, false); + assert.ok(typeof r.retryAfterMs === 'number' && r.retryAfterMs > 0); + }); + + test('a refused call does not consume budget', () => { + const l = new RateLimiter(1); + assert.equal(l.check().allowed, true); + assert.equal(l.check().allowed, false); + assert.equal(l.check().allowed, false, 'still refused, not compounding'); + }); +}); + +describe('limiterFor', () => { + test('returns the same limiter for the same identity', () => { + assert.strictEqual(limiterFor('alice'), limiterFor('alice')); + }); + + test('returns different limiters for different identities', () => { + assert.notStrictEqual(limiterFor('alice'), limiterFor('bob')); + }); + + test('one identity exhausting its budget does not affect another', () => { + const alice = limiterFor('alice'); + for (let i = 0; i < 100; i++) alice.check(); + assert.equal(alice.check().allowed, false, 'alice should be exhausted'); + assert.equal(limiterFor('bob').check().allowed, true, 'bob must be unaffected'); + }); + + test('the same identity reconnecting keeps its budget', () => { + const first = limiterFor('alice'); + for (let i = 0; i < 100; i++) first.check(); + // A fresh lookup models a reconnect: it must not hand back a clean bucket. + assert.equal(limiterFor('alice').check().allowed, false); + }); +}); + +// Beyond the brief's floor: unusual identity strings. `limiters` is a plain +// Map keyed by the identity string, so nothing here is expected to need +// special-casing - these tests exist to prove that empirically rather than +// assume it, the same way auth.ts's tests proved __proto__/constructor are +// inert against a Map-backed identities table. +describe('limiterFor — unusual identity strings', () => { + test('the empty string is accepted as an ordinary, distinct identity key', () => { + assert.strictEqual(limiterFor(''), limiterFor(''), 'stable across repeated lookups'); + assert.notStrictEqual(limiterFor(''), limiterFor('alice'), 'distinct from a real identity'); + }); + + test('prototype-property-shaped, very long, whitespace, and unicode identities are ordinary distinct keys with no collisions', () => { + const ids = [ + '__proto__', + 'constructor', + 'prototype', + 'toString', + 'hasOwnProperty', + ' ', + '\x00', + '🤖ünïcödé-identity', + 'a'.repeat(10_000), + ]; + const gotten = ids.map((id) => limiterFor(id)); + for (let i = 0; i < ids.length; i++) { + assert.strictEqual(limiterFor(ids[i]), gotten[i], `identity ${JSON.stringify(ids[i]).slice(0, 40)} must be stable across lookups`); + for (let j = i + 1; j < ids.length; j++) { + assert.notStrictEqual(gotten[i], gotten[j], `identities at index ${i} and ${j} must not collide onto the same bucket`); + } + } + }); + + test('exhausting the "__proto__" identity does not pollute Object.prototype or bleed into other identities', () => { + const protoLimiter = limiterFor('__proto__'); + for (let i = 0; i < 100; i++) protoLimiter.check(); + assert.equal(protoLimiter.check().allowed, false, '__proto__ identity itself is exhausted'); + assert.equal(limiterFor('alice').check().allowed, true, 'an unrelated identity is unaffected'); + assert.equal(({} as Record).polluted, undefined, 'no prototype pollution occurred'); + }); +}); + +// Beyond the brief's floor: the RateLimiter class hard-codes a real 60_000ms +// window (the constructor's `perMinute` argument only sets the call budget, +// not the window length - there is no public way to shrink it). Sleeping 60 +// real seconds in a test is both slow and, at the exact boundary, at the +// mercy of scheduler jitter. node:test's built-in Date mock advances the +// clock deterministically without touching RateLimiter itself, so this +// exercises the real, unmodified sliding-window comparison +// (`now - t < this.windowMs`) at its exact edge. +describe('RateLimiter — genuine sliding-window expiry', () => { + test('a bucket exhausted at time T is still refused one ms before the window elapses, and allowed the instant it does', (t) => { + t.mock.timers.enable({ apis: ['Date'] }); + const l = new RateLimiter(1); + assert.equal(l.check().allowed, true, 'first call consumes the only slot'); + assert.equal(l.check().allowed, false, 'second call refused within the same window'); + + t.mock.timers.tick(59_999); + assert.equal(l.check().allowed, false, '59.999s later: window has not yet elapsed'); + + t.mock.timers.tick(2); // now 60.001s after the original call + assert.equal(l.check().allowed, true, '60.001s later: window elapsed, budget renewed'); + }); +}); 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 }>; 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": { diff --git a/packages/shared/src/provider.ts b/packages/shared/src/provider.ts index 5e893b6..8f48852 100644 --- a/packages/shared/src/provider.ts +++ b/packages/shared/src/provider.ts @@ -21,6 +21,63 @@ 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.' + ); +} + +/** + * 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; @@ -248,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 @@ -413,14 +516,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-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, + ); + }); +}); 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, + ); + }); + }); +});