Skip to content

Fix MCP auth bypass by failing on missing credentials#74

Closed
DavidJBianco wants to merge 1 commit into
mainfrom
codex/fix-authentication-bypass-in-mcp-server
Closed

Fix MCP auth bypass by failing on missing credentials#74
DavidJBianco wants to merge 1 commit into
mainfrom
codex/fix-authentication-bypass-in-mcp-server

Conversation

@DavidJBianco

Copy link
Copy Markdown
Collaborator

Motivation

  • Authentication failures were returning empty headers which let _connect_http_server/SSE treat missing credentials as a successful connection and record unauthenticated clients.
  • The goal is to restore a clear failure signal when configured authentication is missing or incomplete so connections requiring auth are not created without credentials.

Description

  • Changed MCPClientManager._get_auth_headers return type to Optional[Dict[str, str]] and made it return None on auth failures (missing bearer token, incomplete API key config, or unavailable OAuth credentials) while still returning {} for servers with no auth configured.
  • Updated HTTP connection gating in _connect_http_server to abort when _get_auth_headers returns None.
  • Updated OAuth-related unit tests in tests/unit_tests/test_mcp_oauth_env_vars.py to expect None for auth-failure cases instead of an empty dict.
  • Modified files: peak_assistant/utils/mcp_config.py and tests/unit_tests/test_mcp_oauth_env_vars.py.

Testing

  • Ran python -m py_compile peak_assistant/utils/mcp_config.py tests/unit_tests/test_mcp_oauth_env_vars.py which succeeded.
  • Ran pytest -q tests/unit_tests/test_mcp_oauth_env_vars.py and PYTHONPATH=. pytest -q tests/unit_tests/test_mcp_oauth_env_vars.py, both of which could not complete in this environment due to a missing dependency (python-dotenv).
  • The changed unit assertions were updated to match the new None failure contract; remaining test failures are environmental (missing package) rather than regressions in the change set.

Codex Task

DavidJBianco added a commit that referenced this pull request Apr 23, 2026
Returns None (not {}) from _get_auth_headers on auth failure so
_connect_http_server correctly rejects unauthenticated connections.

Supersedes PR #74, closes PR #71.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@DavidJBianco

Copy link
Copy Markdown
Collaborator Author

Superseded by #85, which cherry-picked this fix. Merged into dev via merge commit 27a476d.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant