Closes an Opus review round on Task 5's registry integration work.
Adds packages/builder/test/sign-helper.test.ts (top-level test/, picked up
by the unit glob) as the first hermetic gate that would fail if
signWithMnemonic's witness-preservation fix regressed. Before this, the
fix had zero coverage under any required gate: sign-helper.ts lives under
test/integration/, outside the unit glob; run.test.ts's
assertSuccessOrKnownError swallows the resulting "Failed to submit
transaction" as an indistinguishable known error; and the E2E's
hash-equality check can't catch it either, since dropping witnesses never
changes the transaction body hash - only the ledger tells the two apart,
and this test never talks to it. The new test builds a synthetic,
fully-offline script-bearing transaction directly via CML (one input/
output, a witness set carrying one PlutusV3 script, one map-form redeemer,
one datum), signs it with a public, well-known, never-funded BIP39 test
vector (computed from the installed bip39 package rather than transcribed
from memory - the first guess had the wrong checksum word and failed
loudly), and asserts every witness category plus the vkey witness survive
signing with the body hash unchanged. Proved it's load-bearing: reverted
the fix to the buggy from-scratch witness set, watched the test fail
naming the exact lost witness, reverted back (byte-identical, zero diff),
re-ran green.
registry-keyless.test.ts: the non-owner-update assertion drops its second
regex alternative (`Failed to build agent update`), which was a superset
match able to hide a real, unrelated build failure behind the same
"ownership was correctly refused" label. Now asserts the specific
`Ownership check failed` message only.
run.test.ts: buildSignSubmit now hard-asserts `Transaction Submitted` on
the submit leg when walletHasAda is true, instead of routing through the
tolerant assertSuccessOrKnownError - on a funded wallet a submit failure
is a real regression, not indistinguishable from an unfunded wallet, and
should fail the test rather than pass silently as a "known error". The
graceful path is kept for the unfunded case. Also documents, in a comment,
the known/accepted risk that a register-succeeds/deregister-fails run in
this file leaves the 10 AP3X test deposit locked on-chain (same shape as
the pre-keyless suite).
Hermetic only this round, per instruction: no live network calls, no
transaction submissions, no real secrets. Full account in
.superpowers/sdd/task-5-report.md's Addendum section.
Gates: typecheck 0, unit 351/351 (350 + 1 new), smoke 26/26, auth 12/12,
boundary 4/4 (confirms the new test's mnemonic constant sits outside the
custody scan by design, same as the rest of test/, and is labeled
TEST_ONLY_PUBLIC_MNEMONIC regardless per public-repo hygiene).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds the three remaining integration layers for spec PR 7 (agent-registry
keyless): a tier-1 suite that exercises the five build_* tools against the
live testnet registry with zero secrets (build-only, nothing signed or
submitted), a gated full lifecycle E2E (register -> update -> deregister)
that actually submits, and a shared signWithMnemonic() helper extracted
from keyless-e2e.test.ts so registry-e2e.test.ts and run.test.ts's
registry section don't each re-inline the same CML signing sequence.
run.test.ts's "Agent Network Tools" section is rewritten: each of the five
old custodial calls is now build_* -> signWithMnemonic -> submit, via a
new buildSignSubmit() helper that reuses the file's existing
assertSuccessOrKnownError convention at both the build and submit legs.
Found and fixed one load-bearing bug while proving this live, not just in
review: the inline signing block moved verbatim per the brief's Step 1
built the signed tx's witness set from scratch, which is lossless only for
a script-less payment (the one case it had been proven against). The
registry family's build_* tools attach a minting policy and/or spending
validator, so their unsigned CBOR already carries scripts/redeemers that
the from-scratch witness set silently dropped - the ledger rejected the
first live E2E attempt with MissingScriptWitnessesUTXOW /
PPViewHashesDontMatch. Verified zero funds were lost (balance identical
before/after the rejected submission - a mempool-level rejection never
reaches a block) before fixing it by merging into the tx's existing
witness set via CML's add_all_witnesses instead of discarding it. Re-run
clean: all 3 E2E transactions confirmed on-chain, plus all 5 of the legacy
suite's registry operations submitted successfully too (independent
confirmation across both the mint and spend redeemer paths). Also widened
one tier-1 regex that didn't tolerate the live tool's Markdown-bold label
formatting.
Live evidence (testnet, funded wallet, 8 real transactions total across
the gated E2E and the legacy suite): balance 986.689510 -> 982.280960
AP3X, fully reconciled against transaction fees plus one message delivery
to an owner-derived enterprise address. All tx hashes, explorer links, and
the full reconciliation are in task-5-report.md. mnemonic.txt and the two
temporary evidence-capture scripts were removed and confirmed absent
before this commit.
Gates: typecheck 0, unit 350/350, smoke 26/26, auth 12/12, boundary 4/4 -
all re-verified after cleanup. E2E skip-path (VECTOR_E2E_SUBMIT unset)
exits 0 with tests 0 for both keyless-e2e.test.ts and registry-e2e.test.ts,
confirmed with mnemonic.txt genuinely absent.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rewrites agent-network.ts as a typed, keyless module: the five custodial
signing tools (vector_register_agent, vector_update_agent,
vector_transfer_agent, vector_deregister_agent, vector_message_agent) are
replaced by unsigned-build equivalents (vector_build_register_agent,
vector_build_update_agent, vector_build_transfer_agent,
vector_build_deregister_agent, vector_build_message_agent) that take no
mnemonic - the 4-call flow is get_address -> build_* -> signer sign ->
submit -> confirm. The two read-only tools (vector_get_agent_profile,
vector_discover_agents) keep identical behavior, description, and schema,
now backed by registry-build.ts's typed helpers (Tasks 2-3) instead of
local duplicated logic.
@ts-nocheck is dropped - the file now typechecks clean under strict mode
(server.tool()'s generic inference did not explode; the TS2589 concern
that originally motivated @ts-nocheck is confirmed dead under zod/v4).
boundary.test.ts: CUSTODIAL_UNTIL_LATER_PR shrinks from
{agent-network.ts, self-improvement.ts} to {self-improvement.ts} - size
pin 2 -> 1, message updated for spec PR 8 (the last remaining family).
tool-inventory.test.ts: EXPECTED_TOOLS updated for the 5 renames,
alphabetical order preserved, count stays 23.
tool-schemas.snapshot.json regenerated; delta verified surgical - exactly
the 5 old names removed and 5 new names added, all other 18 entries
(11 vector.ts + 5 self-improvement + the 2 unchanged read-only agent
tools) byte-identical.
Also fixes a stale cross-reference this rename exposed:
self-improvement.ts's registry-NFT-not-found error told the caller to
"re-register with vector_register_agent", a tool name that no longer
exists post-rename. Updated to vector_build_register_agent. This is the
only reference to the old names left in packages/builder/src outside the
deferred integration test (test/integration/run.test.ts, explicitly
scoped to a later task per progress.md).
One addition beyond the task brief (Task 3 review, M-c context): the
build_deregister_agent troubleshooting copy gets a 4th tip pointing at
registry-build.ts's I-1 error (deregistration needs a plain AP3X-only
UTxO to clear the validator's returned-change floor).
Gates: typecheck 0, unit 350/350, smoke 26/26 (23 tools), auth+rate-limit
12/12, boundary 4/4.
Four items from the final whole-branch review, all docs-only:
1. Architecture diagram: the ASCII diagram routed every tool call through
Rate Limiter -> Safety Layer -> Lucid/Ogmios, which is false for the
keyless build_* path (safety.ts is never consulted there). Annotated the
Safety Layer box with a footnote marker instead of redrawing the diagram -
box alignment verified programmatically before and after (same column
positions for every vertical bar).
2. Configuration table: added one sentence stating VECTOR_SPEND_LIMIT_PER_TX/
DAILY govern only the still-custodial families, matching .env.example's
existing scope note.
3. Testing section: "builds every build_* tool's unsigned transaction"
overclaimed - keyless-build.test.ts covers 4 of 5 (verified directly
against the test source: vector_build_send_tokens is absent from it,
covered instead by unit tests and the legacy integration suite). Reworded
to name the gap explicitly.
4. packages/builder/test/integration/keyless-e2e.test.ts:9 (comment only):
"which lives on PR #7's branch" was stale now that #7 is merged and
packages/signer is in-tree. Reworded to name the in-tree package and be
precise about what the stand-in does and doesn't exercise (same CML
primitives, without decode/policy/audit).
Opus review round 2 on Task 6:
- agent-network.ts: seedUtxo -> oneShotUtxo (5 occurrences). This is
one-shot-minting-policy vocabulary that survives PR 7's keyless rewrite,
but the boundary test's FORBIDDEN regex is an unanchored case-insensitive
substring match on "seed" - left as-is it would have false-red PR 7's
guard the moment agent-network.ts drops off the custodial allowlist. Only
selectWallet.fromSeed (genuine key-material vocabulary, deleted by PR 7
anyway) still matches `seed` in this file. File is @ts-nocheck, so this
was verified mechanically, not just visually: line-match counts before
(seedUtxo=5) and after (seedUtxo=0, oneShotUtxo=5) line up exactly, and a
fresh `import()` of the module still loads error-free.
- keyless-build.test.ts: the first tier-1 test summed every output
(`paid >= 3_000_000n`), but a self-send's change output lands at the same
address as the payment, so the sum floor could never actually detect a
dropped or wrong payment amount - change alone clears it. Added an
exact-count check (outputs at exactly 3_000_000n lovelace, >=1) alongside
the existing sum/fee/unsigned assertions, matching the pattern already
used by the build_transaction test in the same file. Also added the
missing assertBuildNotSubmitted check on the dry-run test's own build
response.
- run.test.ts: deleted walletBalanceAda - write-only since the UTxO
Consolidation block (its only reader) was removed earlier in this task.
Re-verified full gate plus one live re-run of the tightened tier-1 suite
(build-only, no funds spent, no E2E resubmission).
Addresses opus reviewer findings on the Task 5 commit (33c4b52).
Important:
- agent-network.ts (3 sites) and self-improvement.ts (2 sites): response
copy no longer tells a caller to run vector_get_spend_limits (deleted in
Task 5) after a safety-limit error. Replaced with
"Server spend limits for this tool are set via VECTOR_SPEND_LIMIT_PER_TX /
VECTOR_SPEND_LIMIT_DAILY" (matching each site's existing phrasing).
Nothing else in either file touched - both stay custodial until PRs 7/8.
Also reworded the two comments (of my own, from Task 5) that named the
deleted tool - tool-inventory.test.ts's header docblock and
rate-limit-gating.test.ts's probe-choice comment - so a final
`grep -rn vector_get_spend_limits packages/` is clean except
run.test.ts (Task 6's, left untouched as instructed).
- boundary.test.ts: added a third test pinning
CUSTODIAL_UNTIL_LATER_PR.size === 2, so PR 7/8 shrinking it is a visible,
reviewable number change rather than a silent list edit.
- boundary.test.ts: FORBIDDEN is no longer word-bounded
(`/(mnemonic|seed|bip39|fromSeed|walletFromSeed|selectWallet\.fromSeed)/i`,
\b dropped) so it catches `seedPhrase`, `WALLET_SEED`, `bip39x`-shaped
vocabulary, not just whole-word matches. Verified empirically (not just by
inspection) that this introduces zero new hits outside the two allowlisted
files - grepped `seed`/`bip39`/`mnemonic` case-insensitively across all of
packages/builder/src first, confirmed every hit was already confined to
agent-network.ts/self-improvement.ts, then reran the full boundary suite
to prove it: no benign collisions, nothing to report back before
committing. Added a fourth test pinning every FORBIDDEN alternative
against a fixture corpus (mnemonic/MNEMONIC/seedPhrase/WALLET_SEED/bip39/
fromSeed/walletFromSeed/selectWallet.fromSeed/seed all match;
"address-only wallet"/"unsigned transaction"/"changeAddress" don't) so
deleting an alternative from the regex fails a named test instead of
silently narrowing the guard.
Cheap folds:
- vector.ts: the five build_* tool DESCRIPTIONS (not response copy - that
already taught the confirm step as of Task 5) now end with "...then
broadcast with vector_submit_transaction and confirm with
vector_await_transaction." Regenerated the snapshot; verified structurally
that the delta is description-field-only on exactly those five entries
(inputSchema untouched, all 23 keys unchanged) - this is also what makes
the Task 5 addition-A docblock's "descriptions teach the full flow" claim
actually true end-to-end.
- tool-inventory.test.ts: each per-tool test now also asserts
'description' in schemaSnapshot[toolName] before the deepStrictEqual, so
a regen that silently drops a description (e.g. JSON.stringify eating an
undefined value) fails by naming the tool, not as an opaque diff.
- regen-tool-schemas.mjs: server entry point and snapshot output path are
now resolved from import.meta.dirname instead of hardcoded
repo-root-relative strings, so the script runs from any cwd. Verified by
actually invoking it from packages/builder (not the repo root) and
confirming it wrote to the one correct
packages/builder/test/smoke/tool-schemas.snapshot.json.
Full gate re-run after all of the above: typecheck 0 errors, test:unit
144/144 (140 baseline + 4 boundary tests, up from 2), test:smoke 26/26,
test:integration:auth 12/12.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Finishes the builder-side wallet/tx family's non-custodial split (spec PR 6).
Task 4 registered the keyless build_* tools; this task deletes the last
custodial surface in vector.ts and makes every remaining query tool keyless:
- Deleted initLucid, getWalletInfo, the vector_get_address and
vector_get_spend_limits tool registrations, and the now-unused safetyLayer
and Lucid imports. SafetyLayer/safety.ts itself stays (still used by
agent-network.ts and self-improvement.ts until PRs 7-8).
- vector_get_utxos, vector_get_transaction_history: address is now required,
mnemonic param and branch deleted, provider construction routed through
makeProvider().
- vector_dry_run: mnemonic param removed; the outputs-branch now builds via
lucidForAddress + buildMultiOutput (keyless, UNSIGNED) instead of signing
with a mnemonic-derived wallet.
- packages/shared/src/types.ts: removed the now-dead VectorToken,
VectorWalletInfo, VectorAdaTransactionResult, VectorTokenTransactionResult,
VectorBuildTransactionResult, VectorDeployContractResult and
VectorInteractContractResult interfaces (and their re-exports from
vector/index.ts). SpendLimits/SpendStatus/AuditEntry stay - safety.ts is
still live for the two custodial families.
- vector_build_transaction / vector_build_deploy_contract /
vector_build_interact_contract response copy now teaches the full
build -> sign -> submit -> confirm flow (they previously stopped at
submit), matching vector_build_send_apex/send_tokens.
- rate-limit-gating.test.ts: repointed its hermetic probe from the
now-deleted vector_get_spend_limits to vector_build_send_apex with an
invalid changeAddress - lucidForAddress rejects it in-process before any
provider call, so the suite stays hermetic.
- Added packages/builder/test/boundary.test.ts: a mechanical grep-based
custody boundary (design-spec success criterion 1). Verified failing before
the rewrite (vector.ts's mnemonic vocabulary) and passing after; also
verified the guard actually fails by injecting `// mnemonic` into build.ts
and watching it name the exact file:line before reverting.
- Tool surface is now 23 (25 - 2 deletions). tool-inventory.test.ts updated
accordingly.
Snapshot shape widened (this once, deliberately): tool-schemas.snapshot.json
entries are now { description, inputSchema } instead of a bare inputSchema,
so the regression guard also covers the security-teaching copy in tool
descriptions (the sign/submit/confirm flow callers learn from them), not just
JSON Schema shape. tool-inventory.test.ts's per-tool test now deep-compares
the whole entry and is relabeled "schema+description unchanged". This is why
the snapshot diff touches every entry, not just the 10 that actually changed.
Committed the regen script at packages/builder/scripts/regen-tool-schemas.mjs
(previously scratchpad-only) with this shape change applied.
Verified against the feat/builder-auth-and-submit baseline: the snapshot key
delta is exactly 4 renames (send_apex/send_tokens/deploy_contract/
interact_contract -> build_*), 4 changed-in-place (build_transaction,
dry_run, get_utxos, get_transaction_history), and 2 removals (get_address,
get_spend_limits) - confirmed with a structural JSON diff, since the uniform
shape-widening makes raw line-based git diff noisy (e.g. it shows
vector_message_agent's key line as touched even though its inputSchema is
byte-identical old vs new - a Myers-diff line-matching artifact, not real
agent-network.ts drift).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Move fixture-provider.test.ts up to test/ (test:unit's glob is
packages/*/test/*.test.ts, deliberately shallow, so a nested
test/fixtures/ file was silently never run). The fixture module itself
(fixture-provider.ts, the capture script, the JSON fixture) stays in
test/fixtures/; only the *.test.ts file moved, importing it via
./fixtures/fixture-provider.ts.
- Harden capture-protocol-params.mjs: refuse to write a fixture whose
costModels/coinsPerUtxoByte/keyDeposit could have come from
OgmiosProvider's own `??` fallback defaults instead of a genuine
response. The brief's sanity numbers (minFeeA 45, minFeeB 156253, ...)
are literally those same fallback literals, so matching them proves
nothing; costModels has no populated fallback, so a well-formed,
appropriately-sized cost model per Plutus version is the one signal a
schema-mismatched response cannot fake.
- packages/builder/tsconfig.json: include test/**/* so `npm run
typecheck` actually checks builder's test files (it previously covered
src/**/* only, unlike packages/shared). Needed
allowImportingTsExtensions for the now-in-scope relative .ts-suffixed
imports across every existing test file. Surfaced one real pre-existing
gap: auth.test.ts calls loadAuthConfig with deliberately minimal env
objects, but the exported signature demanded the full
env.d.ts-augmented NodeJS.ProcessEnv (7 required custom fields)
even though the function only ever reads MCP_AUTH_TOKENS. Narrowed the
parameter to Record<string, string | undefined> - the function's real
contract - which is a type-only change with a single production call
site (index.ts's parameterless loadAuthConfig()) that already relies on
the default.
Verified: test:unit 117/117 (was 113 - the 4 fixture tests were never
counted before), typecheck 0 errors, test:smoke 28/28, build unaffected.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The "unusual identity strings" fixture embedded a literal 0x00 byte in a
string literal instead of the \x00 escape sequence. Consequence: the file
had no valid text encoding as far as git's binary-diff heuristic is
concerned, so the whole 5278-byte file rendered as "Binary files differ"
with zero textual diff - invisible to review, and would have been
invisible to any GitHub PR reviewer too. It also made the raw NUL
character indistinguishable from the adjacent ASCII-space case when
viewed directly.
Same runtime string (a genuine one-character NUL string, confirmed via
'\x00'.charCodeAt(0) === 0), no behavior change - the file is just
diffable text again.
vector_await_transaction's confirmation check only ever looked for an
unspent output at index 0 - a heuristic that can read a landed transaction
as unconfirmed, since these tools broadcast CBOR signed elsewhere with no
guarantee of that output existing or staying unspent. The check also wrapped
the lookup in .catch(() => []), collapsing genuine infrastructure failures
(Ogmios down, malformed response) into the same "not confirmed yet" result,
defeating pollUntilConfirmed's documented contract that a check rejection
should propagate.
OgmiosProvider.getKoiosTxStatus + koiosIndicatesConfirmed (shared/provider.ts)
now provide a direct, non-swallowing Koios tx_status check; poll.ts's new
buildConfirmationCheck picks it when VECTOR_KOIOS_URL is configured and
falls back to the outRef heuristic (also no longer swallowing errors)
otherwise - the same preference order OgmiosProvider.awaitTx already used,
without reusing awaitTx itself (still not caller-configurable, still
swallows every failure). awaitTx stays in place only because the Provider
interface requires it; nothing else calls it.
buildConfirmationCheck and koiosIndicatesConfirmed are unit tested via
dependency injection, the same pattern poll.test.ts already uses for
now/sleep, with no network access required.
The snapshot guard flagged both additions, which is what it is for. Delta
verified as exactly two ADDED entries with no existing schema changed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Completes the keyless half of the non-custodial flow: the signer returns
signed CBOR, and these broadcast it and wait for confirmation.
submitTx previously returned the first 64 hex characters of the submitted
CBOR when the response failed to parse - a string indistinguishable from a
real transaction hash. It now refuses via a new exported
parseSubmitResponse, since these tools are the first callers to actually
surface that value to an agent. Extracted to module scope and tested
directly (18 cases) rather than through submitTx itself, since provider.ts
imports fetch from cross-fetch rather than reading the global - stubbing it
would leave the real implementation live and hit a genuine DNS lookup.
vector_await_transaction's polling loop is extracted into vector/poll.ts
(pollUntilConfirmed) rather than left inline in the tool. Two reasons:
- The original loop slept a fixed 3s every iteration regardless of budget
remaining, so a short timeoutSeconds could overshoot its deadline by
close to 3x (e.g. timeoutSeconds=1 could block ~3s). The sleep is now
clamped to the time left before the deadline.
- Extracting it makes the loop unit-testable (11 cases) with an injected
fake clock, instead of only being reachable through a live network call.
Does not reuse the provider's own awaitTx: it hard-codes a 60-attempt
budget with no caller control and returns a bare boolean, collapsing
"still pending" and "gave up" into one signal - the tool needs a
configurable timeout and distinct wording for the two, since a pending tx
may still land and must not be reported as failed.
Judgement call on the brief's fixture: its final test asserted
parseSubmitResponse(CBOR.slice(0, 64)) throws. With the simple 64-hex-char
isTxHash check specified in the brief, 'ab'.repeat(32) legitimately
qualifies as a hash - it IS 64 valid hex characters - so that assertion was
changed to assert.equal instead of contorting the implementation to add
stricter validation the brief didn't ask for. The invariant that matters
still holds: parseSubmitResponse only ever receives the HTTP response body,
never the submitted tx, so this value can no longer be invented by our own
code from data it already held - only echoed back by a server, in which
case treating it like any other bare-text hash is correct.
Beyond the brief: added coverage for JSON responses in shapes the parser
doesn't recognize (array/number/null/boolean/nested object/unhandled field
name), the "id" field alias, a zero-budget/never-yielding poll, and that a
rejection from the confirmation check propagates instead of being read as
"not yet confirmed".
test:unit 96/96, typecheck clean. test:smoke fails as expected (23 -> 25,
naming vector_await_transaction and vector_submit_transaction as added) -
regenerating that snapshot is Task 5's job. test:integration:auth 12/12
unaffected.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Tasks 2 and 3 verified their work with throwaway scripts against a real
spawned server that were never committed, so npm test protected none of it -
index.ts's auth gate had zero automated coverage, and the @ts-nocheck
agent-network.ts/self-improvement.ts rate-limit wiring had none beyond
listTools().
Adds two hermetic integration suites (real spawned server, localhost only,
no wallet/chain calls):
- auth-gating.test.ts: both HTTP routes' 401/403 gating, the auth-before-
session-lookup ordering, cross-identity rejection, token-leak scanning,
and that the server survives all of it (the reviewer's severe finding -
a missing early `return` after a 401 falls through into a second
writeHead() and crashes the whole process via an unhandled rejection).
- rate-limit-gating.test.ts: per-identity isolation and that exhaustion on
a vector.ts tool also rate-limits agent-network.ts and self-improvement.ts
tools for that identity, through the real registered tools rather than
limiterFor() in isolation.
Extends test/setup.ts additively (spawnServer/stopRawServer/RawServerHandle)
so these tests can set MCP_AUTH_TOKENS and drive per-request Authorization
headers by hand; startServer()/stopServer()/callTool() are unchanged and the
existing run.test.ts still fails for exactly its pre-existing reason.
Wires a new test:integration:auth script into CI's build-and-test job.
All five mutations from the review were applied to a working-tree copy,
built, and reverted per-mutation to prove the new tests actually fail:
- removing either route's early `return` after a 401: caught (cascading
ECONNREFUSED once the process crashes; a dedicated liveness test targets
this directly)
- the cross-identity check's undefined-lookup branch: not independently
catchable - transports/sessionIdentities are always populated and
cleared together, synchronously, so that state is unreachable via HTTP;
documented in the report, verified a same-region operator-flip mutation
IS caught cleanly
- module-scope `identity` reference in self-improvement.ts: crashes the
server at startup (ReferenceError at import time), caught via the
before() hook
- checkRateLimit restored to module scope in agent-network.ts: caught
cleanly, isError:true / "rateLimiter is not defined" instead of the
well-formed rate-limit text
No file under any src/ changed. test:unit 67/67, test:smoke 26/26 (23
tools, 0 drift), typecheck clean, new suite 12/12.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The limiter was a module-level singleton, so one busy caller throttled every
other caller on the box and a shared daily budget was trivially exhausted.
Buckets are now keyed by authenticated identity, and keyed centrally so
reconnecting does not hand back a clean bucket.
All four loadAuthConfig throw sites could be made to interpolate the secret
that triggered them with the suite staying fully green - the one constraint
this module states twice in its own header had no regression protection.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Hosted deployments set MCP_AUTH_TOKENS; localhost self-hosters are unaffected.
The resolved identity doubles as the rate-limit bucket key. Rejection reasons
never echo the supplied token, and a bare token's log label is a hash prefix.
Hardened beyond the brief after an adversarial pass (20 additional tests):
- A malformed MCP_AUTH_TOKENS that parses to zero usable identities (e.g.
",,," from a broken template) now throws instead of silently resolving to
`enabled: false` - the exact "operator believes auth is on, server is
actually open" failure shape this module exists to prevent. An explicitly
empty/unset value still disables cleanly, unchanged.
- A token containing embedded whitespace is now rejected at config-load
time: resolveIdentity's \S+ match can never span it, so without this check
the token would be configured yet permanently unpresentable.
- Documented and pinned two non-fixable format limitations: only the first
colon in an entry separates label from token (a label can't itself contain
one), and a literal comma inside a label/token silently fragments the
entry since the delimiter has no escape mechanism.
- Pinned correct existing behaviour: exact case-sensitive token match, no
prefix/superstring matching, Map-backed storage is immune to
__proto__/constructor prototype pollution, one label may legitimately
cover multiple tokens (rotation), empty-string header treated as missing.
- Map.get token lookup is not constant-time; judged acceptable and
documented inline rather than swapped for crypto.timingSafeEqual - see
the comment above the lookup in auth.ts for the reasoning.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pure file moves plus import repointing at the shared package. Removes
duplicated endpoint constants and formatting helpers that were copied
across three files. Verified: 23 tools with identical schemas.
