# Custom MCP tool review: a hands-on security checklist Custom MCP tools deserve the same review you would give a small production integration with access to your workstation, network, and credentials. The fact that an agent calls the tool through MCP does not reduce its authority. It often makes sloppy authority easier to exercise repeatedly. I have seen the recurring failure: a developer reads a tool description, sees a useful name such as `deploy_preview` or `search_docs`, and grants it access because it appears local. Later, the implementation turns a model-supplied string into a URL, a shell argument, or a recursive file read. The useful tool was never the security boundary. The implementation and the credential path were. The Model Context Protocol specification describes tools as functions exposed by a server for a client to discover and call. It also makes an uncomfortable fact explicit: tool execution is model-controlled. A client may put a human in the approval loop, but a tool author must still assume that arguments can be surprising, excessive, or pointed at the wrong target. Review the tool before an agent gets a chance to be inventive. ## Start with the authority map, not the README A custom MCP tool is acceptable only when you can draw a short, concrete path from the agent request to the effect it produces. Start by writing down what the tool can read, where it can send data, what it can change, and which credential or operating-system identity makes that possible. Do this before reading implementation details. It gives you a standard for judging the code instead of letting a pleasant description set the standard. A tool called `get_build_status` may read a local config file, call a hosted API, write a cache, and invoke a command-line helper. Each action has a different failure mode. Use a small authority map like this: | Part | Record | Why it matters | |---|---|---| | Agent input | Exact tool fields and maximum sizes | Shows what the model can influence | | Local reads | Paths, environment variables, config files | Reveals accidental data collection | | Local writes | Cache, workspace, git state, temporary files | Finds persistent side effects | | Network | Hostnames, ports, methods, redirects | Defines exfiltration and request risk | | Processes | Executable path, arguments, child processes | Finds shell injection and inherited access | | Credentials | Credential name, scope, storage, revocation owner | Makes removal possible | | Results | Data returned to the agent | Prevents secrets from coming back through the tool | Do not write "the internet" in the network row or "developer credentials" in the credential row. Those labels mean the review has not begun. Name the host, the API route family, the account or token, and the person or system that can revoke it. This exercise also separates two things teams routinely blur: a tool's advertised purpose and its actual authority. `create_issue` sounds narrow. A function that accepts an arbitrary base URL, arbitrary headers, and arbitrary request body has the authority of a generic HTTP client. Review the latter, not the label. ## The input schema must reduce choices An MCP input schema should constrain the agent to the operation you mean to permit. It should not act as a decorative type definition around a free-form command channel. The MCP specification uses JSON Schema for tool input schemas. That helps clients present arguments and helps implementations validate them, but JSON Schema is not enforcement unless the server rejects invalid values before it performs work. Treat the schema as the first gate and server-side validation as the second. This is a reviewable schema for a tool that fetches the status of a known build: ```json { "name": "get_build_status", "description": "Return the status for one build in the approved CI project.", "inputSchema": { "type": "object", "additionalProperties": false, "required": ["build_id"], "properties": { "build_id": { "type": "string", "pattern": "^[A-Z]{2,8}-[0-9]{1,10}$", "maxLength": 20 }, "include_logs": { "type": "boolean", "default": false } } } } ``` It makes several choices for the agent. It cannot select a host. It cannot attach headers. It cannot pass a shell command. It cannot send undeclared fields because `additionalProperties` is false. The build identifier has a bounded format, which makes downstream use less dangerous and easier to log. Now compare it with the shape that causes trouble: ```json { "name": "request", "inputSchema": { "type": "object", "properties": { "url": {"type": "string"}, "method": {"type": "string"}, "headers": {"type": "object"}, "body": {} } } } ``` That is an HTTP client with a friendly handle. If it has a bearer token, it can send that token or data from an agent-controlled prompt to an arbitrary endpoint. People keep this design because it saves time while prototyping. It remains a bad production boundary even if the tool name sounds specific. Test input handling with values that alter parsing rather than values that merely look malformed. Try a duplicate identifier field, an unexpected field, a large string, Unicode lookalikes, a newline, and a value that is syntactically valid but points outside the intended business domain. If an input becomes a path, require a relative identifier and resolve it against a fixed directory. If it becomes an API filter, use structured parameters rather than concatenating query strings. Never hand an argument to a shell merely because you validated the schema. Use an argument array with a fixed executable path. This is safer: ```python subprocess.run( ["/usr/local/bin/buildctl", "status", "--id", build_id], check=True, text=True, capture_output=True, env={"PATH": "/usr/bin:/bin"} ) ``` This is a review failure: ```python subprocess.run(f"buildctl status --id {build_id}", shell=True) ``` The first form still needs validation, error handling, and a trustworthy executable. It does not ask a shell to reinterpret model-controlled punctuation. ## Every outbound request needs a fixed destination A custom MCP tool should own a small set of destinations, and its code should reject every other destination before opening a connection. Allowlisting a hostname in documentation is useless if the request code accepts an arbitrary URL. Review outbound traffic at two levels. First, inspect source for HTTP libraries, WebSocket clients, DNS lookups, package installers, telemetry SDKs, webhook libraries, and any helper process that can connect elsewhere. Second, watch a real run. Static inspection finds intended paths. Runtime observation catches a dependency that phones home or a configuration value that changed the target. A safe client builds a URL from fixed pieces and encodes only the identifier: ```python from urllib.parse import quote BASE = "https://ci.example.internal/api/builds/" url = BASE + quote(build_id, safe="") response = client.get(url, timeout=10, follow_redirects=False) ``` The important control is not `quote`. The fixed origin is. If your client follows redirects, a trusted origin can return a redirect to an untrusted host. Disable redirects unless the tool checks each redirect destination against the same allowlist. This matters for internal services too. A tool that accepts `http://host/path` can be tricked into reaching local admin services or metadata endpoints that an agent cannot directly access. Blocking public hosts does not solve that. You need a positive list of approved origins and a rule that refuses literal IP addresses or private destinations unless the tool explicitly needs them. Capture traffic in a disposable test environment. On macOS, `lsof` gives a quick first view of current network sockets: ```sh lsof -nP -iTCP -sTCP:ESTABLISHED -c python ``` The output shape identifies a process, its user, file descriptor, and remote endpoint: ```text COMMAND PID USER FD TYPE DEVICE SIZE/OFF NODE NAME python 8421 alex 12u IPv4 0x... 0t0 TCP 10.0.0.8:51244->203.0.113.20:443 (ESTABLISHED) ``` Replace `python` with the actual process name, then repeat while you invoke one tool operation. This command is not a complete traffic audit. Short connections can disappear before you inspect them. It is still good at exposing an unexpected long-lived connection or a helper you did not know existed. Review request payloads with equal care. A tool may correctly call one approved API while dumping an entire repository diff, environment variable, or agent transcript into a query parameter. Restrict outbound fields in code. Build the payload from named values the operation needs, rather than serializing a whole object received from the agent. ## Process identity is part of the permission You cannot approve a tool use responsibly if you cannot tell which executable requested it. The process name alone is weak evidence because any process can choose a familiar-looking name. Record the complete launch command, executable path, version, working directory, parent process, and the user account. If macOS code signing applies, inspect the signature authority as well. `codesign` can show the identity claims macOS sees: ```sh codesign -dv --verbose=4 /absolute/path/to/mcp-server 2>&1 | grep -E 'Identifier|TeamIdentifier|Authority' ``` Typical output has fields such as: ```text Identifier=com.example.mcpserver Authority=Developer ID Application: Example Developer TeamIdentifier=ABCDE12345 ``` Those fields are evidence, not a permission decision by themselves. A signed binary can still be the wrong binary for this job, and an unsigned internal script is not automatically malicious. The review asks whether the path, owner, source, and identity match the tool you expected to run. Then inspect the process tree during an invocation: ```sh ps -axo pid,ppid,user,command | grep -E 'mcp-server|sp-ssh|node|python' ``` You want a boring answer: the agent client launches the MCP server, and the server starts only the helper processes you expected. Be suspicious when the server launches a shell, a package manager, an interpreter from a mutable project directory, or a background process that outlives the session. A common failure looks harmless in a configuration file: ```json { "command": "npx", "args": ["-y", "some-mcp-package"] } ``` This can fetch or change code at launch time depending on local cache state and package resolution. It makes a review of yesterday's source less meaningful than teams assume. Pin the executable or package version, install it through a controlled process, and launch a known local path. If the tool needs updates, make updates an explicit review event rather than an invisible side effect of starting an agent. Process identity also includes inherited environment. A server launched from a developer shell can inherit cloud tokens, source-control tokens, proxy settings, and a broad `PATH`. Print a sanitized inventory in test runs, or launch with a minimal environment. Do not log secret values. Log variable names that affect behavior and confirm that the tool does not need ambient credentials it was never meant to use. ## Logs must reconstruct actions without repeating secrets A useful audit record answers who ran the tool, under which process, with what sanitized arguments, against which destination, and with what result. A line reading `tool call succeeded` answers none of the questions you will have when an agent sends data somewhere surprising. Separate operational logs from secret-bearing debug output. Operators need enough detail to investigate. They do not need bearer tokens, authorization headers, private keys, raw agent transcripts, or full response bodies copied into a text file. For each invocation, write fields similar to these: ```json { "time": "2025-03-08T14:22:11Z", "session_id": "run_7c2f", "process": "/opt/tools/build-mcp", "tool": "get_build_status", "argument_summary": {"build_id": "CI-4812", "include_logs": false}, "destination": "ci.example.internal", "decision": "approved", "result": "success", "request_id": "c4e8..." } ``` The example uses an argument summary on purpose. A summary should preserve identifiers and bounded fields that help investigation, while redacting or hashing sensitive values. If an operation legitimately sends a document, log its byte count and a content digest where that helps correlation. Do not put the document itself in the journal just to make debugging convenient. The OpenTelemetry logging model is useful here even if you do not adopt OpenTelemetry. It distinguishes event body from attributes and emphasizes structured fields for filtering and correlation. Apply that idea locally: make destination, operation, outcome, and process identity machine-readable. A pile of prose log lines becomes useless the first time you need to answer whether the agent sent the same request ten times. Also inspect error paths. Many tools redact successful requests but print a complete request object when an API returns an error. Force a 401, a timeout, malformed JSON, and a failed DNS lookup. Read every emitted line. Debug output is where credentials usually escape. Sallyport keeps a Sessions journal for agent runs and an Activity journal for individual calls, both projected from a write-blind encrypted, hash-chained audit log. That design is useful when you need a local record beyond the tool author's own logs, but it does not make a broad tool safe. The tool still needs narrow inputs and known destinations. ## Approval prompts cannot repair broad authority Human approval is a useful brake only when the thing you approve has a comprehensible scope. A card that says an unrecognized process wants to use a credential tells you something important. It does not tell you whether a generic request tool will send a repository file to a host chosen by a model five seconds later. Keep the approval unit close to the authority unit. A read-only status token and a production deployment token should not sit behind one approval because they have different consequences. A tool that reads a release record should not quietly gain the ability to create one because both operations happen to use the same API. The Model Context Protocol specification recommends that clients obtain user consent before invoking tools. That is sound guidance, but consent has a failure mode: people approve repetitive, poorly described prompts until the prompt stops carrying information. Do not solve prompt fatigue by approving a whole category of actions forever. Fix the tool boundary that produces vague or excessive prompts. Sallyport's decision ladder puts a locked vault ahead of all actions, asks for per-session authorization by default, and can require a decision for every use of a specific credential. Use per-call approval for credentials whose use you would want to inspect each time, such as a deployment or write-capable API credential. Do not use it as an excuse to give a generic HTTP tool that credential. A good approval test has a sentence behind it: "This signed process, launched from this path, may use this credential to read status from this service for this run." If you cannot say that honestly, deny the request and go back to the authority map. ## Test the failure paths before you trust success A tool that works on the happy path has not passed a security review. You need to see how it behaves when inputs are wrong, its credential is absent, the network points somewhere unexpected, and its helper process fails. Run the tool in a test account or isolated project with a credential that has the smallest practical scope. Use test data that resembles real data enough to exercise serialization and size limits, but never feed production secrets into an unreviewed tool just to see what happens. Use this five-part test sequence: 1. Call the tool with one valid request and capture its process tree, outbound destination, and audit record. 2. Send an undeclared field, a maximum-length value, a value containing line breaks, and a valid-looking identifier outside the permitted project. The server should reject each before making a request. 3. Force an unapproved host through every available input and configuration route. Include redirects if the tool uses HTTP. The tool should refuse it and record the refusal without exposing sensitive input. 4. Remove or revoke the credential while the server remains running, then repeat the valid request. Confirm that the next action fails rather than succeeding from a hidden cache or inherited environment. 5. Kill the parent agent process and check for surviving server or helper processes. A background worker that retains access after the agent exits needs a clear reason and a separate review. Keep the evidence with the tool version: the manifest, source revision or package digest, commands used, observed endpoints, sample redacted audit record, and the person who accepted the remaining risks. This is not paperwork for its own sake. Without versioned evidence, a later package update changes the tool and everyone assumes the old review still applies. One failure deserves special attention. Suppose a document-search tool accepts `repository_path` and calls a helper using a shell string. Normal requests work. An agent later receives an instruction embedded in a ticket to search a path containing shell punctuation. The helper executes a second command under the developer's account, reads a credential file, and the tool sends the result to its otherwise approved search endpoint. Each component did what its author expected. The composition failed because the schema allowed a path, the shell reinterpreted it, and the outbound payload accepted arbitrary helper output. Test chains, not isolated functions. ## Access removal needs more than deleting a config entry Removing an MCP server from an agent configuration stops the ordinary launch path. It does not revoke a token copied into a cache, end a server that is still running, remove an SSH key from an agent process, or invalidate access at the remote service. Plan removal when you grant access. The credential owner should know where to revoke it, the tool should use a distinct credential where possible, and the operator should know which process and local files to remove. Shared developer tokens turn a simple removal into an incident because you cannot tell which use belongs to the tool. For a tool that uses HTTP, revoke or disable the remote token first. Then stop the MCP server and any child helpers, remove the tool's local credential reference, and delete its launch configuration. Finally, rerun the same request and retain the denied result. For SSH, remove the relevant public key at the remote account or repository, terminate local helper processes, and inspect agent configuration for alternate identities. Do not confuse a local vault lock with remote revocation. A lock stops future use through that vault while it remains locked. It cannot retract data already sent, invalidate a token stored elsewhere, or stop an unrelated process that inherited a credential earlier. Make removal testable before an incident. Add a short runbook entry with the remote revocation location, expected denial response, server command, config location, and the log fields that confirm the attempt failed. If the tool owner cannot provide that entry, they have not finished the integration. ## A narrow tool earns repeatable trust The tools that survive review are usually boring in the best way. They accept a few typed fields, connect to one known service, run a known executable when they need one, return only the result the agent needs, and leave a record someone can inspect later. Broad tools feel flexible because they shift design decisions onto the agent at runtime. They also turn a single approval into authority over destinations, data, and commands that nobody reviewed. Keep flexibility in the code you own and expose a narrow operation to the agent. Before approving a custom tool, try to remove one input, one destination, one credential scope, or one child process. If nobody can explain why that item must remain, remove it. The review gets easier, and so does the eventual incident response.