Require auth on MCP HTTP server + bind localhost by default (issue #90) - #101
Open
Diogo-Damasceno wants to merge 1 commit into
Open
Diogo-Damasceno wants to merge 1 commit into
Diogo-Damasceno wants to merge 1 commit into
Conversation
…05TCREW#90) The MCP HTTP server (transport 'sse') bound to 0.0.0.0:8080 with no authentication, exposing run_task/update_config/get_conversation_history to any network client, which could drive LocalRuntime shell execution on the host (issue GH05TCREW#90). - create_streamable_http_app(router, token=None): when a token is provided, POST/GET/DELETE /mcp require 'Authorization: Bearer <token>', returning 401 otherwise. - interface/main.py: add --mcp-token; default --host changed from 0.0.0.0 to 127.0.0.1; warn when binding to 0.0.0.0 without a token. Token is threaded into both server start paths. - Without a token the server keeps legacy open behaviour but now defaults to localhost, so it is not exposed to the network by accident. Add tests/security/test_mcp_auth.py (8 tests). Full tests/security suite: 145 passed; ruff clean.
This was referenced Sep 14, 2026
sattyamjjain
added a commit
to sattyamjjain/agent-airlock
that referenced
this pull request
Sep 15, 2026
…s 34 (#192) * fix(cves): disposition CVE-2026-90617 at the argument seam Out of scope. The watcher filed it because the shape classifier read CWE-77/CWE-78 plus "run_task" as argument-shaped, and that prior was reasonable: run_task really is a registered MCP tool taking {task, target, scope}, which is the seam this library sits on. The argument does not carry the command. "task" is a natural-language prompt. The shell string is authored downstream by the LLM and executed by LocalRuntime's asyncio.create_subprocess_shell, which is the product working as designed for a caller that got in. target and scope become agent context, not command text. What the defect actually is: the aiohttp /mcp routes carry no authentication and bind 0.0.0.0:8080 by default, so any network client can drive the agent at all. Upstream PR #101 fixes it with Authorization: Bearer middleware plus a 127.0.0.1 default and changes no argument or schema. Neither an auth check on someone else's route nor a bind default is expressible at the tool-call boundary, which is the rule in docs/cve-triage.md under "The seam". This is the "both" case that doc describes, resolved without a second-defence fixture. Unlike CVE-2026-79748, the primitive the missing check hands over is not a stdio spawn config but a sentence of English aimed at an LLM designed to run commands. Refusing that at the argument boundary would mean classifying prose, and on a pentesting agent the malicious task and the legitimate one are the same string. No guard, no fixture, no code under src/ changed, so no version bump. The refusal row and the reasoning land in tests/cves/README.md so the decision is readable without opening the tracker. Also de-rots two counts in that section that this edit made wronger: "all four rows" becomes "every row", and "the two 2026-09 additions" now names the CVEs it means. Primary sources: https://nvd.nist.gov/vuln/detail/CVE-2026-90617 GH05TCREW/pentestagent#90 GH05TCREW/pentestagent#101 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HoFgXfaGtzBk56c4Lu2AXh * feat(cves): clear the triage queue and fix a catalogue that counts 36 and covers 34 Three things, one release. 1. Triage queue PR #188's disposition of CVE-2026-90617 is carried here unchanged; it could not land alone because a docs-only change leaves entries under [Unreleased] on a released version, which check_changelog.py refuses by design. The 0.10.5 bump in this commit is what resolves it. CVE-2026-90898 (Bifrost, 9.8) and CVE-2026-57124 (PraisonAI, 9.8) are both in scope, and cut the opposite way from CVE-2026-90617. Each is the split docs/cve-triage.md calls "both": an unauthenticated HTTP endpoint this library cannot reach, handing the caller a command plus args bound for a stdio spawn, which it already refuses. The missing auth is out of scope in each. The primitive is the CVE-2026-42271 shape, so each gets a second-defence fixture against the existing McpSubprocessArgInjectionGuard rather than a new guard, and neither preset claims either CVE. Filed a day apart, the two records are mirror images of the watcher's own classifier. Bifrost carries CWE-284 and CWE-306, neither argument-shaped, and was admitted only by the sink word "stdio". PraisonAI has no sink word at all and was admitted only by CWE-78. Each signal is load-bearing exactly once across the pair, which is the argument against reducing it to one. Both fixtures also pin two integration footguns rather than describing them: Bifrost nests the spawn fields under stdio_config, so passing the whole request body finds nothing to inspect and allows it; and its envs is a list of names, not the env mapping the dangerous-variable check reads. 2. Catalogue docs/cves/index.md emitted one row per test module, and two CVEs carry two modules each, so it published 36 rows for 34 CVEs. The anchor is derived from the CVE id, so each pair emitted the same <a id> twice and both summary links pointed at it: one link per pair necessarily resolved to the wrong section, and the HTML was invalid besides. --check could not see it. It compares generated output to committed output, and the generator wrote the duplicate into both, so the gate stayed green while the artefact was wrong. A row is now one CVE listing every module behind it, and assert_unique_cve_rows() runs in every mode including --write, so a duplicate cannot be produced in the first place. Three counts were being conflated and are now separate: 45 regression modules, 38 CVE-numbered, covering 36 distinct CVEs. README quotes the last and is asserted against the catalogue's row count; marketplace.json quotes the first two. The old "36" was a module count that equalled today's CVE count only because the catalogue double-counted. 3. Distribution checklist Off DRAFT and dated. All six target lists re-verified live; three no longer resolve. punkpeye/awesome-mcp-servers now scopes itself to servers you install and run, and points libraries at its awesome-mcp-devtools sibling, which replaces it. wong2/awesome-mcp-servers does not accept PRs. And e2b-dev/awesome-ai-agents has no security or tooling section, which the old row was conditional on. The two "search for it" placeholders now name real repositories. Each surviving row carries its exact section heading, what the list requires, a bullet already in that list's format, and a state column. The canonical one-liner still said "A type-checker for AI tool calls" while pyproject, the repo description and the README hero had moved to the contract-layer framing. Primary sources: https://nvd.nist.gov/vuln/detail/CVE-2026-90898 maximhq/bifrost#6757 https://nvd.nist.gov/vuln/detail/CVE-2026-57124 GHSA-p75f-6fp4-p57w Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HoFgXfaGtzBk56c4Lu2AXh --------- Co-authored-by: sattyamj-attri <sattyam.jain@attri.ai> Co-authored-by: Claude Opus 5 (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
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
Fixes #90. The MCP HTTP server (transport
sse) bound to0.0.0.0:8080with no authentication, exposingrun_task/update_config/get_conversation_historyto any network client — which could driveLocalRuntimeshell execution on the operator's host.Changes
mcp_transport_streamable_http.py:create_streamable_http_app(router, token=None). When a token is set, all/mcproutes (POST/GET/DELETE) requireAuthorization: Bearer <token>, returning401otherwise.interface/main.py:--mcp-tokenflag; threaded into both server start paths.--hostchanged from0.0.0.0→127.0.0.1(localhost only).0.0.0.0without a token.Verification
pytest tests/security/test_mcp_auth.py→ 8 passed (auth helper + integration: unauthenticated POST rejected 401, authenticated POST allowed, GET requires auth, no-token stays open).pytest tests/security/→ 145 passed.Test plan for reviewers
pentestagent mcp_server --type sse→ binds127.0.0.1:8080.--mcp-token SECRET:curl -X POST localhost:8080/mcp -d '...'→ 401; with-H "Authorization: Bearer SECRET"→ proceeds.--host 0.0.0.0without--mcp-token→ prints a warning.