fix: mcp auth TLS gate falls back to entry.url when caller omits url

[dylanyunlon]

Summary

Fixes #3785 — the require_tls gate in McpAuthPolicy.check() never consulted the configured entry.url when the caller-supplied url was empty (or omitted). A caller that simply omits url could bypass the TLS requirement even when the server entry had require_tls: true and a configured non-TLS entry.url.

Changes

Core fix (mcp_auth_enforcement.py)

The TLS gate now resolves an effective URL via url or entry.url before evaluating the scheme. When both sources are empty, the gate permits the connection — preserving spec S10.12 compatibility (add_server() with default url="" must remain allowed).

Before:

if entry.require_tls and url:  # caller omits url → skipped entirely

After:

if entry.require_tls:
    effective_url = url or entry.url
    if effective_url:  # only skip when truly no URL exists

Tests (test_mcp_auth_enforcement.py)

12 new tests in TestTlsGateEntryUrlFallback covering:

• http/https/wss/ws/ftp entry.url fallback when caller omits url
• Caller-supplied url overrides entry.url (both directions)
• Both-urls-empty S10.12 compatibility
• require_tls=False bypass
• No-scheme entry.url rejection
• Explicit empty string treated as absent
• YAML end-to-end integration

All 29 tests pass (17 existing + 12 new).

Documentation (docs/mcp-auth-tls.md)

New doc covering URL resolution order, configuration examples (Python + YAML), allowed TLS schemes, and security considerations.

README

Added doc link under the Reference section.

Checklist

• Follows conventional commit format
• All existing tests pass (17/17)
• New regression tests added (12 tests)
• Documentation added
• cspell clean (no new words needed)
• Signed-off-by on all commits

Signed-off-by: dylanyunlon dogechat@163.com

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[github-actions]

Welcome to the Agent Governance Toolkit! Thanks for your first pull request.
Please ensure tests pass, code follows style (ruff check), and you have signed the CLA.
See our Contributing Guide.

[github-actions]

🟡 Contributor Check: MEDIUM

Check	Result
Profile	MEDIUM
Credential	LOW
Overall	MEDIUM

Automated check by AGT Contributor Check.

[github-actions]

PR Review Summary

Check	Status	Details
🔍 Code Review	⚠️ Missing	No current-run comment
🛡️ Security Scan	⚠️ Missing	No current-run comment
🔄 Breaking Changes	⚠️ Missing	No current-run comment
📝 Docs Sync	⚠️ Missing	No current-run comment
🧪 Test Coverage	⚠️ Missing	No current-run comment

Verdict: ⚠️ AI review incomplete; ready for human review

AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims.

[dylanyunlon]

@microsoft-github-policy-service agree

[Imran Siddique (imran-siddique)]

Ran this branch's suite locally against its own tree (PYTHONPATH=src python -m pytest tests/test_mcp_auth_enforcement.py): 29 passed. The fix is correct and the issue it cites,
#3785, is real.

Confirmed the defect on main: the gate reads if entry.require_tls and url:, so a registered entry
configured with url: http://mcp.internal/api and require_tls: true is allowed whenever the caller
calls policy.check(name, auth_method=...) without repeating the URL. The url parameter was
documented as "for logging/audit", so a caller has no reason to pass it, which makes the bypass the
default path rather than an edge case.

Two things I want to call out as done well, because they are the parts a reviewer would otherwise
have to guess at.

The precedence is the right way round. Caller URL wins over entry.url, so a caller connecting to
somewhere other than the configured address is judged on where it is actually going. Your
test_caller_url_overrides_entry_url and test_caller_https_overrides_entry_http pin both
directions of that, which matters because getting it backwards would turn a config value into an
override of live connection facts.

Keeping "both URLs empty" as allowed is also correct, and worth having stated explicitly the way
your docstring does. That case is fail-open on a missing URL, not fail-open on TLS, and conflating
the two would break add_server() with the default url="".

Overlap you and the maintainers should know about. #3793 fixes the same defect with the same
mechanism (effective_url = url or entry.url) and is functionally identical to this branch. The
difference is coverage shape: #3793 has a single parametrized test that sweeps the full nine-way
caller-URL by configured-URL matrix crossed with require_tls, and this branch has twelve named
tests that reach schemes #3793 does not (ws, wss, ftp, bare hostname with no scheme) plus an
end-to-end YAML case plus docs/mcp-auth-tls.md. The two will conflict textually, since they edit
the same lines.

This branch is the more complete one. If it lands, the piece of #3793 worth keeping is its
parametrized precedence matrix, which is a tidier statement of the same property.

Separately, #3849 fixes the sibling hole (#3814, unregistered server names skipping the gate
entirely) in the fallback branch further down the same method. It does not overlap textually with
either of these, so it can land independently.

One thing none of the three touch, so purely a note: min_tls_version is accepted on
McpServerEntry, parsed by from_yaml, and documented in docs/specs/MCP-SECURITY-GATEWAY-1.0.md
and agent-mesh/docs/zero-trust.md, but it is never read by any enforcement code in the repo. A
policy that sets min_tls_version: "1.3" gets no enforcement today. Worth its own issue.

Nothing blocking.

[Carlos Hernandez (carloshvp)]

Reviewed head d4297add54e1d92c2d1ef206c47ce892d38fee57. All 29 auth-enforcement tests pass on Python 3.11; restoring the parent source makes six of them fail. I also ran a 200-case configured-URL/caller-URL/TLS-required matrix against this PR and #3793. Both preserve caller precedence, reject non-TLS and malformed configured URLs when no caller URL is supplied, preserve the both-empty compatibility case, and continue denying none/unknown auth methods in the tested defaults.

The full gateway-conformance module could not collect because the local environment lacks agent_control_specification. I extracted and ran its unchanged TestAuthEnforcement class in isolation: all 12 S10 tests pass, including S10.12. This does not establish full gateway conformance.

I recommend this PR for #3785, given its scheme coverage, YAML regression and documentation. Treat #3793 as the alternative fix, and land #3849 separately for unregistered-server fallback. Non-blocking documentation nit: update the check() URL argument description from "for logging/audit" to explain TLS enforcement and configured-URL fallback, as #3793 does.

No blocking finding. The four reported lint findings also occur on the parent. Signoff and merge simulation against main 359a2332 check out. The later PR-title checks passed after the earlier failure.

[liamcrumm]

The implementation is correct, but the url parameter is still documented as logging and audit metadata even though it now takes precedence in the TLS decision. The guide also says this closes the bypass for an untrusted caller, but a caller that controls url can claim an HTTPS value regardless of the actual destination. Please document that url must be the trusted live destination and remove the untrusted-caller claim.

[dylanyunlon]

Updated in abf1f0a. The docstring now reads:

url: The actual destination URL for this connection. When
    require_tls is true, the scheme of this URL is checked
    against the TLS allowlist (https, wss). If empty, the
    configured entry.url is used as fallback. Pass the
    trusted, live destination -- not an unverified value
    from the caller.

Also removed the untrusted-caller claim from docs/mcp-auth-tls.md and replaced it with a note that url is trusted input -- a caller that controls it can claim any scheme, so verifying the actual destination is the transport layer's job, not the policy gate's.

[MohammadHaroonAbuomar]

Closing as superseded: the TLS fallback itself landed via #3793, so what remains here is the extra test matrix and docs, on unsigned commits. Thanks for the work; a signed follow-up PR carrying just the added tests would be welcome.