MCP server, security hardening, Astro 7 upgrade, and test coverage - #1
Merged
Conversation
…ethods, sanitize errors - Extract ALLOWED_RPC_METHODS into app/src/lib/rpc-allowlist.ts so the test file imports it directly instead of maintaining a stale copy - Fix incorrect method names: open_wallet→wallet_open, create_wallet→wallet_create - Remove wallet_show_seed_phrase and wallet_unlock_private_keys from the browser-callable allowlist (handled server-side only) - Sanitize non-WalletRpcError messages so internal hostnames/stack traces never reach the browser (returns "An internal error occurred" instead) - Add console.error logging for all 502 errors - Extend rpc.test.ts: verify the four removed/fixed methods are now blocked, and add a dedicated test for error message sanitization
…list comment
- Make jsonError() return consistent {ok: false, error: {message, code}} shape with code=0 default
- Add afterEach hook to rpc.test.ts for proper mock cleanup between tests
- Move address_send from Transactions section to Addresses section in allowlist
…rsal guard - Replace execSync/exec shell interpolation with execFileSync + argument arrays throughout installPlugin (tar, find, cp) to eliminate shell injection vectors - After tar extraction, run `find` over tmpDir and verify every resolved path (via realpathSync) stays inside the extraction directory, blocking ../ entries and symlink escapes - makePluginContext now rejects any walletRpc call whose method is absent from ALLOWED_RPC_METHODS, preventing plugins from reaching sensitive methods such as wallet_show_seed_phrase or wallet_unlock_private_keys - Update plugins.test.ts: mock execFileSync instead of execSync, add realpathSync mock, add a path-traversal rejection test, and add allowlist acceptance/rejection tests for makePluginContext
…wallet_open/create from plugin allowlist - Wrap realpathSync in try/catch so dangling symlinks (ENOENT) are treated as path-traversal violations instead of propagating uncontrolled - Wrap the find call in try/catch with a clear error message - Remove wallet_open and wallet_create from ALLOWED_RPC_METHODS — these accept server-filesystem paths and must only be called server-side (setup.astro, wallet.astro), not via the browser proxy or plugin context - Add explicit blocked-method assertions for wallet_open/wallet_create in rpc.test.ts - Add dangling-symlink test case to plugins.test.ts
…y, Permissions-Policy headers
Adds a stdio MCP server exposing wallet functions to AI clients, with permissions managed from Management → Settings and gated by risk tier (read / actions / spend). Seed phrase, key unlock, and wallet open/create are never exposed; write/spend toggles require a valid TOTP. Security fixes from review: - Propagate the session_version into tokens issued by login.astro and passkey/auth-verify.ts (was defaulting to 0, causing a lockout loop after any password change). - Harden password change against File form parts (coerce non-strings so they can't bypass the length check or crash hashPassword). Other: - Astro 6 → 7 upgrade (0 vulnerabilities); pin compressHTML to v6 behavior. - Remove the Windows PowerShell installer. - Extract the MCP TOTP-gate into lib/mcp-settings for unit testing. Tests: 448 passing; coverage 94% lines / 92% branches. MCP server and permission gate brought under the coverage threshold.
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
1 participant
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
app/scripts/mcp-server.mjs): stdio server exposing wallet functions to AI clients, permissions managed from Management → Settings and gated by risk tier (read / actions / spend). Seed phrase, key unlock, and wallet open/create are never exposed; write/spend toggles require a valid TOTP. Permissions read fresh per call and fail closed if the prefs DB is missing.session_versioninto tokens issued bylogin.astroandpasskey/auth-verify.ts(both defaulted to0, causing a lockout loop after any password change).Fileform parts (coerce non-strings so they can't bypass the length check or crashhashPassword).compressHTMLpinned to v6 behavior.lib/mcp-settings.tsso the fund-moving-access decision is unit-tested.Tests
scripts/mcp-server.test.mjs(fail-closed perms, gate denials before RPC, all 11 tool handlers, escape-hatch can't reachwallet_show_seed_phrase),lib/mcp-settings.test.ts(TOTP gate), plus regression tests for the session-version fix and the passwordFilepath.includeso the bar is enforced going forward.CI note
The
build.ymlworkflow triggers on push tomainand tags, not onpull_request, so no check will appear on this PR. I reproduced the CItestjob locally (cleannpm cion node 22 + the CI env vars) andnpm run test:coverageexits 0 / green. Happy to add apull_requesttrigger in a follow-up so future PRs run CI automatically.