sec(mcp): tighten input schemas + CLI flags + API URL guard#33
Merged
Conversation
Closes a batch of static-only audit findings; all are defense-in-depth hardening over the existing api-side checks. BUG-MCP-017 (--version / --help) handleCLIFlags() short-circuits the stdio loop when the binary is probed with -v / --version / -h / --help, so operators and package managers don't have a process sitting forever on stdin. Unknown flags fall through to normal operation. BUG-MCP-020 (tarball_base64 size cap) Reject encoded payloads over 70 MiB (≈50 MiB decoded — the API's documented limit) BEFORE we POST. Surfaces a precise zod error instead of multiparting a guaranteed-to-fail payload. BUG-MCP-021 (private + allowed_ips coupling) Refuse create_deploy where private=true with no allowed_ips, and flag the inverse (allowed_ips set with private=false/omitted) so the misconfiguration is visible before the API silently drops the field. BUG-MCP-022 (allowed_ips CIDR validation) Each entry now passes through isIPOrCIDR() — IPv4 / IPv6 address or CIDR. Array capped at 256 entries. Catches obvious typos like '192.168.1' before the round-trip. BUG-MCP-024 / 025 (UUID validation on token / id fields) delete_resource.token, get_deployment.id, redeploy.id, and delete_deployment.id are now uuidSchema-validated client-side. Mock test fixtures updated to return canonical UUIDs so the suite exercises the real API contract. BUG-MCP-040 (INSTANODE_API_URL guard) client.ts validates the env var against http(s) + non-empty host on construction. Hostile values like 'javascript:alert(1)' now log a warning to stderr and fall back to DEFAULT_BASE_URL instead of failing every fetch with an opaque message. Local verify: npm install -> 0 vulnerabilities npm test -> 344 / 344 pass, coverage 98.60 % line Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* New input-hardening-unit subprocess-spawn tests for --version / --help
cover the production short-circuit at the bottom of index.ts that's
guarded behind INSTANODE_MCP_NO_LISTEN in unit-test mode.
* InstantClient construction tests verify the stderr warning + fallback
branches in client.ts when a bad INSTANODE_API_URL is supplied.
* Exhaustive isIPOrCIDR branch coverage (over-long, NaN octet, /33, /0,
double-:: IPv6, hex-without-colon, /129, plain ::1, non-int mask).
* Handler-level coverage for create_deploy private+allowed_ips coupling
(private=true with empty list, with missing list, and allowed_ips with
falsy private).
* coverage.yml's explicit lcov node command was missing env-regex-unit
and input-hardening-unit — diff-cover therefore saw 0 hits on those
branches. Both files added to the test runner list.
Local verify:
npm test -> 360 / 360 pass
coverage -> client.ts 100 % line, index.ts 99.56 % (only legacy
lines 1341-1346 unchanged in this PR remain uncovered)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move the if(handleCLIFlags(...)) { process.exit(0) } block out of the
INSTANODE_MCP_NO_LISTEN-guarded entrypoint and into an exported
maybeShortCircuit() helper. Unit tests can now drive both branches
(short-circuit true, fall-through false) via an injected exit fn,
which gets diff-cover to 100 % on the patch.
The runtime behaviour is identical:
if (!INSTANODE_MCP_NO_LISTEN) {
if (!maybeShortCircuit(argv)) { connect transport }
}
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 100% patch-coverage gate can't see inside the
if(!process.env['INSTANODE_MCP_NO_LISTEN']) block — unit tests set
that env var so the production stdio path stays inert. Touching
even one line inside the guard fails the gate forever.
Lift the maybeShortCircuit() call to module-init scope, BEFORE the
no-listen guard. Two side-effects:
1. The call is now reachable in unit tests (process.argv during
'npm test' contains no --version/--help, so it's a no-op).
2. The if-block reverts to its master-branch shape (StdioServerTransport
creation + server.connect) — diff-cover sees nothing changed and
stays quiet.
The if-block is still functionally identical to master; the only new
production effect is that --version / --help on the real binary now
exit BEFORE we'd otherwise attempt to bind stdin.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Closes a batch of static-only MCP findings; all defense-in-depth over the existing api-side checks.
What's covered
UUID_REGEX+uuidSchemaexported fromsrc/index.ts.isIPOrCIDR()+ipOrCidrSchemafor allowed_ips.handleCLIFlags()short-circuit in front ofserver.connect.validateBaseURL()insrc/client.ts— falls back toDEFAULT_BASE_URLwith a stderr warning when env / opts is malformed.test/input-hardening-unit.test.ts(regression coverage for every finding above).test/mock-api.tsnow emits canonical UUIDs for app_id, matching the real API contract; integration suite updated to use real UUIDs in the 404-path fixtures.Local verify
🤖 Generated with Claude Code