[security] fix(mcp): harden signed upload URL authority

[Hinotoi-agent]

Summary

This PR hardens the MCP signed-upload URL authority boundary so upload instructions are not derived from attacker-controlled request authority headers.

• Stops X-Forwarded-Host / X-Forwarded-Proto from controlling the returned signed upload URL unless an operator explicitly configures a trusted public base URL.
• Preserves local MCP compatibility by allowing only syntactically safe loopback Host authorities such as localhost:1933, 127.0.0.1:1933, and [::1]:1933.
• Adds regression coverage for malicious forwarded hosts, malicious raw hosts, configured public_base_url precedence, and authority-confusion inputs such as localhost:1933@attacker.example.
• This is related to the existing MCP upload/public-base-url feature work, but it fixes the trust boundary for request-derived URL authority rather than the upload flow itself.

Security issues covered

Issue	Impact	Severity
MCP signed upload URL authority poisoning	A request-controlled authority could be reflected into signed upload instructions, potentially sending clients and short-lived upload tokens to an attacker-controlled origin in exposed/proxied deployments.	High, deployment-dependent

Before this PR

• _resolve_public_base_url() could derive signed upload URLs from request headers.
• X-Forwarded-Host / X-Forwarded-Proto were trusted as URL authority inputs even though they can be supplied by clients unless sanitized by a trusted proxy.
• Raw Host fallback was needed for localhost clients, but it needed a strict authority parser before interpolation into http://{host}.
• Tests covered upload behavior, but not the signed URL trust boundary for hostile forwarded or malformed authorities.

After this PR

• Operator-configured OPENVIKING_PUBLIC_BASE_URL / server.public_base_url remains the authoritative way to publish a non-local MCP upload origin.
• X-Forwarded-Host / X-Forwarded-Proto no longer control signed upload URLs by default.
• Raw request Host is accepted only for loopback/local authorities with safe syntax.
• Userinfo, path, query, fragment, backslash, whitespace, non-numeric ports, and malformed bracketed IPv6 suffixes are rejected before a request-derived authority is used.
• Regression tests now lock the safe fallback and hostile-header behavior in place.

Explicit operator choice

• Secure default: when no public base URL is configured, non-loopback request authorities are ignored and OpenViking falls back to its configured server host/port.
• Trusted public deployment: operators can set OPENVIKING_PUBLIC_BASE_URL or server.public_base_url to the externally reachable base URL.
• Local clients: localhost/loopback Host values remain supported so local MCP clients can receive usable upload URLs without extra configuration.

Why this matters

MCP add_resource() can return upload instructions containing a signed temporary upload URL. If that URL authority is chosen from untrusted request headers, an attacker who can influence those headers in a proxied or exposed deployment can cause clients to receive attacker-origin upload URLs. That can leak signed upload tokens and divert client uploads away from the intended OpenViking server.

How this differs from related issue/PR

• feat(mcp): progressive single-entrypoint upload for local files #1847 introduced the progressive MCP upload flow; this PR hardens how that flow chooses the URL authority returned to clients.
• docs(server): document public_base_url and upload_signed_ttl_seconds (#1847) #2153 documented public_base_url / upload TTL behavior; this PR enforces that public origins come from explicit operator configuration rather than request-supplied forwarding headers.
• Duplicate checks for public_base_url, X-Forwarded-Host, and Host poisoning did not find an existing open PR fixing this request-authority trust boundary.

Attack flow

attacker-controlled or unsanitized request authority header
    -> MCP add_resource upload-instruction path
        -> signed upload URL generated from request-derived authority
            -> client receives attacker-origin upload URL
                -> short-lived upload token and upload traffic can be redirected

Affected code

Issue	Files
MCP signed upload URL authority poisoning	openviking/server/mcp_endpoint.py, tests/server/test_mcp_endpoint.py, tests/unit/test_mcp_public_base_url_trust.py

Root cause

MCP signed upload URL authority poisoning:

• The signed upload URL helper mixed operator configuration with request-derived authority headers.
• Forwarded authority headers are only trustworthy behind a correctly configured trusted proxy, but the MCP endpoint treated them as safe defaults.
• The localhost compatibility fallback needed stricter syntax validation before embedding the authority into a URL.

CVSS assessment

Issue	CVSS v3.1	Vector
MCP signed upload URL authority poisoning	7.3 High, deployment-dependent	CVSS:3.1/AV:N/AC:L/PR:L/UI:R/S:U/C:H/I:H/A:N

Rationale:

• The issue requires a reachable MCP path and a deployment where request authority headers can influence the server-side request context.
• User interaction is modeled because a client must follow the returned upload instruction.
• Impact is primarily token/URL confidentiality and upload integrity for the affected signed upload operation; availability is not the focus of this patch.

Safe reproduction steps

1. Forwarded authority poisoning

1. Configure OpenViking without OPENVIKING_PUBLIC_BASE_URL / server.public_base_url.
2. Call the MCP upload instruction path with a hostile X-Forwarded-Host such as attacker.example.
3. On vulnerable code, observe that the returned signed upload URL can use the attacker-controlled authority.

2. Raw Host authority confusion

1. Configure OpenViking without a public base URL.
2. Provide a raw Host value such as localhost:1933@attacker.example or localhost/path.
3. On vulnerable/insufficiently guarded code, these values risk being misinterpreted if inserted directly into http://{host}.

Expected vulnerable behavior

• Signed upload instructions can be constructed with a request-controlled authority.
• Short-lived upload tokens can appear in URLs whose host is not the intended OpenViking server.
• Localhost compatibility can become unsafe if the raw authority is accepted without rejecting URL authority-confusion syntax.

Changes in this PR

• Adds a request-context helper for the MCP endpoint's URL-resolution path.
• Gives explicit public base URL configuration precedence over request headers.
• Removes default trust in X-Forwarded-Host / X-Forwarded-Proto for signed upload URL authority generation.
• Adds strict loopback authority parsing for the raw Host fallback.
• Adds regression tests for configured public base URLs, malicious forwarded hosts, malicious raw hosts, loopback compatibility, and authority-confusion rejection.

Files changed

Category	Files	What changed
MCP endpoint hardening	openviking/server/mcp_endpoint.py	Hardened signed upload base URL resolution and loopback authority validation.
Server regression tests	tests/server/test_mcp_endpoint.py	Added endpoint-level coverage for upload URL authority behavior.
Unit regression tests	tests/unit/test_mcp_public_base_url_trust.py	Added focused tests for public base URL precedence, hostile headers, loopback fallback, and malformed authorities.

Maintainer impact

• The patch is scoped to MCP signed upload URL generation and its tests.
• Existing local MCP clients using loopback hosts continue to receive usable upload URLs.
• Public/reverse-proxy deployments should use the already-supported explicit public base URL setting rather than relying on request-forwarded authority headers.
• No unrelated storage, CLI, bot, or documentation behavior is changed.

Fix rationale

• Signed upload URL authority is a trust boundary and should come from explicit operator configuration, not ambient request headers.
• The loopback exception keeps developer/local-client usability while avoiding broad trust in arbitrary Host values.
• Rejecting authority-confusion syntax before interpolation makes the fallback durable against URL parsing edge cases.
• The new tests cover both the expected compatibility path and the hostile inputs that previously lacked direct coverage.

Type of change

• Security fix
• Tests
• Documentation update
• Refactor with no behavior change

Test plan

• Focused public-base-url trust unit tests passed.
• Ruff check passed on touched files.
• Full server MCP test file not run successfully in this local environment because the native ragfs_python binding is unavailable.

Executed with:

• PYTHONPATH=. /Users/$USER/.hermes/hermes-agent/venv/bin/python -m pytest -o addopts='' tests/unit/test_mcp_public_base_url_trust.py -q
• /Users/$USER/.hermes/hermes-agent/venv/bin/python -m ruff check openviking/server/mcp_endpoint.py tests/server/test_mcp_endpoint.py tests/unit/test_mcp_public_base_url_trust.py

Local result:

• tests/unit/test_mcp_public_base_url_trust.py: 5 passed
• ruff check: All checks passed!

Not run successfully:

• tests/server/test_mcp_endpoint.py could not run locally because importing the server fixture path failed with ImportError: ragfs_python native library is not available: Rust binding not available.

Disclosure notes

• This PR is bounded to signed upload URL authority derivation in the MCP upload flow.
• It does not claim unauthenticated reachability by itself; impact depends on how the MCP endpoint is exposed and whether request authority headers can be influenced before they reach OpenViking.
• No production systems were tested.
• No unrelated files were changed.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 1847 - Fully compliant Compliant requirements: (This PR is a follow-up hardening, not the initial feature) 2153 - Fully compliant Compliant requirements: (This PR is a follow-up hardening, not documentation)

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🏅 Score: 95

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

PR Code Suggestions ✨

No code suggestions found for the PR.