Skip to content

test(http): cover Origin validation matrix for MCP endpoint#1075

Merged
andreasronge merged 1 commit into
mainfrom
claude/1041-origin-validation-tests
Jun 3, 2026
Merged

test(http): cover Origin validation matrix for MCP endpoint#1075
andreasronge merged 1 commit into
mainfrom
claude/1041-origin-validation-tests

Conversation

@andreasronge

Copy link
Copy Markdown
Owner

Summary

Regression test coverage for the shared Host/Origin validation path on the
/mcp HTTP endpoint. The behavioral fix (validation runs before method
dispatch for every method) already landed; this fills the test gap for the
security-critical Origin policy branches.

  • New mcp_server/test/ptc_runner_mcp/http/origin_test.exs unit suite
    (mirrors host_test.exs) exercising the previously untested branches of
    PtcRunnerMcp.Http.Origin: the allowed_origins allowlist, Origin: null
    rejection, and the non-loopback fail-closed path. Also covers header
    extraction and the loopback default-allow branch for a coherent matrix.
  • Router-level test in http_router_test.exs: GET /mcp with an invalid
    Origin returns 403 before the 405 method catch-all, complementing the
    existing bad-Host GET test.

No production code change — testing confirmed the existing validation is
correct.

Test plan

  • mix test test/ptc_runner_mcp/http/origin_test.exs test/ptc_runner_mcp/http_router_test.exs (49 tests, 0 failures)
  • mix precommit full suite (653 tests, 0 failures)

Closes #1041

🤖 Generated with Claude Code

Add regression tests for the shared Host/Origin validation path. A new
origin_test.exs unit suite exercises the previously untested allowlist,
Origin: null, and non-loopback fail-closed branches of
PtcRunnerMcp.Http.Origin. A router-level test confirms GET /mcp with an
invalid Origin returns 403 before the 405 method catch-all.

No production code change; the behavioral fix already landed.

Closes #1041

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@claude

claude Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

PR Review: test(http): cover Origin validation matrix for MCP endpoint

Summary

Test-only PR that fills a regression-coverage gap for PtcRunnerMcp.Http.Origin. It adds a new origin_test.exs unit suite mirroring the existing host_test.exs, plus one router-level test confirming GET /mcp with an invalid Origin returns 403 before the 405 method catch-all. No production code changes. Overall: clean, accurate, and ready to merge.

What's Good

  • Coverage matches the rescoped issue. Issue fix(http): validate Host and Origin for every MCP endpoint method #1041 was rescoped to "add regression coverage for the Origin policy matrix" after confirming the behavioral fix already landed. The four documented branches — allowlist, Origin: null, non-loopback fail-closed, and loopback default-allow — are all exercised.
  • Assertions are correct against the source. I traced each case against origin.ex:
    • Allowlist hit/miss and the HTTPS://App.Example.com:443 normalization case all resolve correctly through normalize/1 (origin.ex:41) and normalized_port/2 (origin.ex:56).
    • Origin: nullnormalize("null") returns nil → rejected, even with an allowlist (origin.ex:39, origin.ex:20).
    • Non-loopback fail-closed correctly hits the true -> false branch (origin.ex:29).
    • Loopback default-allow correctly delegates to loopback_origin?/1 (origin.ex:34).
  • Consistent style. Mirrors host_test.exs structure, uses async: true, and the module attributes carry clear comments explaining each bind/allowlist combination.
  • Router test is meaningful. It complements the existing bad-Host GET test (http_router_test.exs:57) and proves validation ordering (403 precedes 405) for the Origin branch using the established auth/with_host/call helpers.

Issues (Must Fix)

None.

Suggestions (Optional)

  • OPTIONAL — file size. http_router_test.exs is now 902 lines (over the 800 threshold). This PR only adds +10 lines, so no action is needed here, but the file is a future candidate for splitting by concern (auth, host, origin, dispatch). Worth a separate issue rather than blocking this PR.

Security

No concerns. This is the right kind of change for a high-risk-labeled security path: it adds explicit regression coverage for fail-closed Origin policy (allowlist, null-origin rejection, non-loopback rejection) without altering behavior. No new TODO/FIXME/skip tags introduced.

Documentation

No updates needed — test-only change, no public API or behavior change.

Verdict

Approve. Well-scoped, accurate, and consistent with existing test conventions. Note: CI must run the suite for final confirmation, as the dependencies were not installed in my review environment (I verified the assertions by tracing origin.ex manually).

@claude claude Bot added the ready-to-merge PR is ready to be merged label Jun 3, 2026
@claude

claude Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Auto-Triage Summary

Issue Decision Action
http_router_test.exs at 902 lines (over 800-line threshold) DEFER_ISSUE Created #1076

No FIX_NOW items. Added ready-to-merge label.

@andreasronge andreasronge enabled auto-merge (squash) June 3, 2026 09:18
@andreasronge andreasronge merged commit 4c0d6dd into main Jun 3, 2026
10 checks passed
@andreasronge andreasronge deleted the claude/1041-origin-validation-tests branch June 3, 2026 09:19
@andreasronge andreasronge restored the claude/1041-origin-validation-tests branch June 3, 2026 09:52
@andreasronge andreasronge deleted the claude/1041-origin-validation-tests branch June 3, 2026 09:56
andreasronge added a commit that referenced this pull request Jun 4, 2026
Add regression tests for the shared Host/Origin validation path. A new
origin_test.exs unit suite exercises the previously untested allowlist,
Origin: null, and non-loopback fail-closed branches of
PtcRunnerMcp.Http.Origin. A router-level test confirms GET /mcp with an
invalid Origin returns 403 before the 405 method catch-all.

No production code change; the behavioral fix already landed.

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

Labels

ready-to-merge PR is ready to be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(http): validate Host and Origin for every MCP endpoint method

1 participant