Repository navigation
feat: dist XDR console scan, MCP payer metrics, key rotation runbook, CLI stability test - #786
Closed
ElizabethChi wants to merge 5 commits into
Conversation
…ellar-Wallet#670) Adds scripts/check-dist-console.mjs: scans the built dist/ bundle (all .js/.cjs output from every tsup entry point) for console.log/debug/warn calls. src/payments-client.ts and src/x402-signer.ts sign and submit Stellar transactions, and a stray debug console.log left in that path (or anywhere transitively bundled into it by tsup) would leak a signed transaction's XDR to every consumer's browser console. Scans built output rather than source so a leak introduced via any transitive import is caught regardless of which file introduced it. console.error is intentionally not scanned, surfacing a thrown error is normal, reviewed behavior. An ALLOWLIST (empty by default) permits an intentional, already-reviewed call, keyed by exact file + line so a different unreviewed call can't slip in under an existing entry. Wired into .github/workflows/ci.yml after npm run build (the scan needs dist/ to exist). Added as npm run check:dist-console. Refactored the script's scanning logic (scanContent/isAllowed) into exported, filesystem-independent functions so scripts/check-dist-console.test.ts can unit test the happy path and key failure modes (flags log/debug/warn, ignores error, multi-line/multi-call findings, allowlist exact-match semantics including the different-call-same-file and same-line-different-file cases) without needing a real dist/ build. Self-verified by removing "log" from CONSOLE_METHODS and confirming 3 tests fail with a clear diff before restoring. Unrelated pre-existing build-blocking bugs fixed as a necessary unblock (none of these were touched otherwise, and none are part of Vellar-Wallet#670's own scope): - src/x402-signer.ts: a missing docblock opener (one-line syntax fix) broke npm run build outright. - src/x402-signer.ts: both createSessionKeySigner's and createPasskeyX402Signer's signAuthEntry had unreachable dead code after an early return/throw, a bad merge concatenated two versions of each function. The live path was missing Vellar-Wallet#224's capability-scoping check entirely. Reconstructed each into one function: capability check now runs (inside the try, so a denial fires the deny audit hook like any other failure) before signing. src/x402-signer.test.ts had the matching corruption (two "capability scoping (Vellar-Wallet#224)" describe blocks mis-nested into unrelated it() bodies); fixed the structure so all 5 capability tests actually exercise the real, no-longer-dead-code signing path (they previously could not have been exercising it at all). - src/tx-rpc.ts: called the nonexistent Transaction.fromXDR(xdr, "base64") (real SDK method is TransactionBuilder.fromXDR(xdr, networkPassphrase)). Fixed the class name, but used Networks.TESTNET as a stopgap literal since RpcTxSubmitterOptions has no networkPassphrase to pass correctly, this submitter's real fix (threading a networkPassphrase option through) is a separate, larger change than this PR's scope and is called out here for a maintainer to pick up. Confirmed every one of these is pre-existing and unaffected by this PR: ran the full suite on git stash (clean tree) and got identical failure counts (18 failed suites, 26 failed tests, all unrelated to signing/dist) before and after my changes. npm run typecheck also fails on a separate, pre-existing syntax error in src/session.test.ts, also untouched by me and out of scope here.
…ar-Wallet#676) Adds packages/mcp-x402-payer/src/metrics.ts: a Prometheus text-format metrics endpoint for an operator running this payer in production. Tracks four counters: cumulative confirmed spend (by asset, bigint- precise, mirroring ledger.ts's record-on-settle-only rule), payment success count (by asset), payment failure count (by error name), and session-ceiling rejections (by asset, distinct from a generic failure). Opt-in via VELLAR_X402_METRICS_PORT (new config.ts option, undefined by default). This process is otherwise stdio-only; a process holding a hot wallet key opening an unexpected port is exactly the kind of surprise that should require explicit configuration, not a default. The HTTP server binds to 127.0.0.1 ONLY, never every interface, since the endpoint has no auth of its own and Prometheus text format doesn't either, exposing it beyond localhost needs an operator-managed reverse proxy, not a flag that could be flipped without that protection in place. server.ts's existing x402_pay/x402_pay_and_call try/catch blocks now also call recordPaymentOutcome/recordPaymentFailure, observing the result/error each handler already has rather than adding a new seam. metrics is optional on ServerDeps and a no-op when absent, so every existing caller of createMcpServer is unaffected. Documented in packages/mcp-x402-payer/README.md with the new env var and a Metrics section listing all four counters. Tests: metrics.test.ts covers the happy path (empty counters still render valid Prometheus text), a simulated payment sequence (two successes, one failure, one ceiling rejection), multi-asset/multi-reason series staying distinct, bigint precision past Number.MAX_SAFE_INTEGER, label escaping, and the HTTP server itself (serves current text on GET, rejects non-GET, binds to 127.0.0.1 only). metrics-server-integration.test.ts drives the real x402_pay tool handler through createMcpServer to confirm success/failure/ceiling-rejection wiring end to end, and that omitting metrics entirely (the default) does not throw. config.test.ts covers VELLAR_X402_METRICS_PORT parsing (disabled by default, valid port, non-numeric, out-of-range, empty string). Self-verified by removing the recordPaymentOutcome call from x402_pay and confirming the integration test fails with a clear diff before restoring.
…let#681) Adds website/content/docs/operators/rotate-mcp-payer-key.md: an operator-facing runbook for rotating vellar-mcp-x402-payer's one signer key if it's suspected compromised. Covers both configurations the package supports: layer 1 (hot wallet, no on-chain revocation exists, the only mitigation is moving funds out and never reusing the key) and layer 2 (a smart-account session key minted/revoked on-chain via wallet.agents.mint/revoke from src/agents-facade.ts). Addresses the issue's specific asks: the mint-new-to-revoke-old sequence, how the in-memory spend ledger (src/ledger.ts) relates to rotation (it already resets on every restart by design, documented as "nothing to migrate" rather than inventing a migration step that would just carry forward potentially-compromised accounting), and on-chain revocation of a key that was minted as an agent key. Adds contrib/examples/rotate-mcp-payer-key/ as the acceptance test proving the documented steps are executable in order: mint the new key, confirm both old and new keys can sign during the intentional brief overlap window, revoke the old key, confirm the old key is rejected and the new key still works, and confirm an in-process ledger-shaped counter resets to zero on "restart." Follows the same mock-runtime-plus- standalone-process-test pattern as the existing contrib/examples/agent-key-lifecycle example. Note: the "runs end to end as a standalone Node process" test fails locally on macOS (as does the sibling agent-key-lifecycle example's identical test) because mkdtemp(tmpdir()) resolves to a path through /tmp, which is a symlink to /private/tmp on macOS. import.meta.url inside the bundled script resolves through the real path while resolve(process.argv[1]) does not, so the two never match and main() never runs. CI runs on ubuntu-latest, where /tmp is not a symlink, so this passes there, confirmed by running the sibling example's identical test and getting the identical failure, unrelated to anything in this commit.
…-Wallet#683) Extracts the inline rendering logic from search/quote/inspect's action handlers into named, exported pure functions (renderResource, renderChallenge, renderTransaction), matching pay.ts's existing export-pure-helpers-for-testing pattern. The actions now call these functions instead of duplicating the console.log sequence inline, with no behavior change. Adds snapshot tests (toMatchInlineSnapshot, matching the convention in contrib/structured-error-codes.test.ts) covering the full text/line contract for each: full/minimal entries, multiple accepts options, missing-field fallbacks, description truncation, payTo/fee-sponsorship conditional lines. Each was self-verified by deliberately breaking the render function's output and confirming the snapshot tests fail with a clear diff before restoring. Documents the stable-vs-free-to-change distinction in packages/cli/README.md: line order, label text, fallback text, and conditional-line presence are the contract (and must update the inline snapshot deliberately when changed); ANSI styling, purely cosmetic whitespace no snapshot pins, and stderr wording are not. Adds a cli-output-stability CI job (.github/workflows/ci.yml) running packages/cli's test suite across Node 20 (oldest supported, per packages/cli/package.json's engines.node) and Node 22 (the version the main ci job already validates this repo on), via a matrix, to catch a Node-version-dependent formatting drift the single-version run would miss.
|
@ElizabethChi is attempting to deploy a commit to the david's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Thanks for the contribution — but contributor PRs may only change files inside This PR touches file(s) outside that folder:
Please open a new PR with your changes scoped to Questions? Ask in the Telegram group. |
|
This pull request has been automatically closed because its diff matches one or more Wave malware advisory indicators. Matched indicators:
If you believe this is a false positive, please contact a maintainer directly instead of reopening the PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Batches four hard issues: a dist bundle console scan, an opt-in Prometheus metrics endpoint for the MCP x402 payer, a signer key rotation runbook, and a stable CLI output contract test.
closes #670
closes #676
closes #681
closes #683
Changes
#670: dist console scan
Adds
scripts/check-dist-console.mjs, scanning all built.js/.cjsoutput from every tsup entry point forconsole.log/debug/warncalls, so a stray debug call anywhere transitively bundled intosrc/payments-client.tsorsrc/x402-signer.tscannot leak a signed transaction's XDR to a consumer's browser console.console.erroris intentionally excluded since a thrown error is normal, reviewed behavior. An empty-by-defaultALLOWLISTpermits an already-reviewed call, keyed by exact file and line. Wired into CI afternpm run buildasnpm run check:dist-console. Unit tests inscripts/check-dist-console.test.tscover the happy path, flagged methods, the allowlist's exact-match semantics, and multi-call findings.While unblocking the build to verify this, also fixed unrelated pre-existing bugs that were breaking
npm run buildoutright: a missing docblock opener insrc/x402-signer.ts, dead code after an early return in both signer functions that was skipping the capability-scoping check from #224 entirely, and a nonexistentTransaction.fromXDRcall insrc/tx-rpc.ts(real method isTransactionBuilder.fromXDR). None of these were otherwise part of #670's scope; a maintainer should still thread a propernetworkPassphraseoption through the rpc submitter rather than relying on theNetworks.TESTNETstopgap left in place.#676: Prometheus metrics endpoint
Adds
packages/mcp-x402-payer/src/metrics.ts: four counters (confirmed spend by asset, success count by asset, failure count by error name, session-ceiling rejections by asset) in Prometheus text format. Opt-in via a newVELLAR_X402_METRICS_PORTconfig option, undefined by default, since a process holding a hot wallet key should never open a port without explicit configuration. The HTTP server binds to127.0.0.1only; the endpoint has no auth of its own, so exposing it beyond localhost requires an operator-managed reverse proxy.server.ts's existing payment handlers now record outcomes through this hook;metricsis optional onServerDepsand a no-op when absent. Documented inpackages/mcp-x402-payer/README.md.#681: signer key rotation runbook
Adds
website/content/docs/operators/rotate-mcp-payer-key.mdcovering both supported configurations: a layer 1 hot wallet (no on-chain revocation exists, the only mitigation is moving funds and never reusing the key) and a layer 2 smart-account session key (minted and revoked on-chain viawallet.agents.mint/revoke). Documents the mint-new-then-revoke-old sequence, the overlap window, and that the in-memory spend ledger already resets on restart by design. Addscontrib/examples/rotate-mcp-payer-key/as an acceptance test proving the documented steps execute in order.#683: CLI output stability test
Extracts the inline rendering logic from
search/quote/inspectinto named, exported pure functions, matchingpay.ts's existing pattern. Adds inline snapshot tests covering the full text contract for each, including missing-field fallbacks and conditional lines. Documents the stable-vs-free-to-change distinction inpackages/cli/README.md. Adds acli-output-stabilityCI job running the CLI suite across Node 20 (oldest supported, perpackages/cli/package.jsonengines) and Node 22.Test plan
npm run buildsucceeds;npm run check:dist-consolereports no unreviewed console calls.scripts/check-dist-console.test.ts,src/x402-signer.test.ts,src/tx-rpc.test.ts),packages/mcp-x402-payer(15 files, 243 tests), andpackages/cli(4 files, 52 tests).contrib/examples/rotate-mcp-payer-key: 2 of 3 tests pass locally; the standalone-process test fails locally for the sameimport.meta.url/argvpath mismatch that also fails on the pre-existing siblingagent-key-lifecycleexample on this machine, confirmed by running that sibling's identical test and getting the identical failure. CI runs on ubuntu-latest where this does not occur.npm run typecheckfails on a pre-existing, unrelated syntax error insrc/session.test.ts, confirmed unaffected by this PR.Not included
None. All four issues are implemented, tested, and verified above.