Skip to content

fix(mcp): warn when MCPServer starts without authentication#3044

Open
failsafesecurity wants to merge 1 commit into
ag2ai:mainfrom
failsafesecurity:fix/mcp-server-warn-no-auth
Open

fix(mcp): warn when MCPServer starts without authentication#3044
failsafesecurity wants to merge 1 commit into
ag2ai:mainfrom
failsafesecurity:fix/mcp-server-warn-no-auth

Conversation

@failsafesecurity

Copy link
Copy Markdown

Summary

When security=None (the default), the MCP HTTP endpoint serves without any authentication middleware — any network client can invoke the agent's tools (shell, code execution, web fetch) without credentials.

This PR emits a UserWarning and a logging.warning at construction time so deployers are aware of the risk. The default behavior is unchanged — this is not a breaking change, just a loud warning.

Security Impact

High — An exposed MCP server with default settings allows unauthenticated tool invocation. Combined with prompt injection, this enables:

  • Unauthenticated shell command execution
  • Unauthenticated code execution
  • SSRF via web_fetch to internal services / cloud metadata

Changes

  • Add warnings.warn() + logging.warning() in MCPServer.__init__ when security is None
  • Import logging and warnings modules

Testing

  • Existing tests pass (no behavioral change)
  • Warning is emitted on default construction

Exploit Chain

Unauthenticated MCP client → prompt injection → agent executes shell command or fetches cloud metadata. See [CHAIN-001] in our analysis.

Remediation Options (for future PRs)

  1. Make security a required parameter (breaking change)
  2. Default to Requirement with API key from env var
  3. Bind to localhost only by default when unauthenticated

When security=None (the default), the MCP HTTP endpoint serves
without any authentication middleware — any network client can
invoke the agent's tools without credentials.

Emit a UserWarning and a log warning at construction time so
deployers are aware of the risk. This is not a breaking change;
the default behavior is unchanged, just loudly warned.

Refs: AGNT-001
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Joshua Medvinsky seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@marklysze

Copy link
Copy Markdown
Collaborator

@failsafesecurity, thanks for the contribution! Can you please re-instate the PR template?

@Lancetnik Lancetnik added type:bug Existing behavior is broken or doesn't match the docs security area:mcp MCP integration (ag2/mcp) status:needs-review PR is ready and waiting for maintainer review status:needs-cla Gate: CLA not signed; not reviewed until signed type:feature New functionality or API extension and removed status:needs-review PR is ready and waiting for maintainer review type:bug Existing behavior is broken or doesn't match the docs labels Jul 4, 2026
@Lancetnik

Copy link
Copy Markdown
Member

Hi @failsafesecurity, thanks for the contribution!

Before we can review or merge this PR, you need to sign our Contributor License Agreement — the license/cla check on this PR has the signing link. This applies to draft PRs as well: we don't want you to invest work into a change we can't accept.

Once the CLA is signed, the check turns green, the status:needs-cla label is removed, and the PR proceeds through the normal flow.

⏳ If the CLA isn't signed within 3 days, this PR will be marked stale and closed 7 days after that. Signing later and reopening is always an option.

@genisis0x genisis0x left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Real gap to close — with security=None as the default, the streamable HTTP endpoint serves shell/code/web-fetch tools with no auth, and that's easy to ship by accident. Surfacing it at construction time is the right call.

Two small things:

  • The _logger = logging.getLogger(__name__) line lands between the stdlib imports and the from mcp.server.auth... imports. A module-level statement sitting mid-import-block is the kind of thing ruff/isort flags (E402) and it reads a bit oddly — worth moving _logger down below all the imports.
  • Emitting both a warnings.warn(UserWarning) and a _logger.warning(...) for the same condition is a little redundant. For a construction-time misconfiguration the UserWarning is the natural channel (dev-visible, and the warnings filter dedupes it); the log line mostly duplicates it. Keeping one would be cleaner.

Is there a test pinning that the warning fires on security=None and stays silent when a Requirement is passed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:mcp MCP integration (ag2/mcp) security status:needs-cla Gate: CLA not signed; not reviewed until signed type:feature New functionality or API extension

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants