Skip to content

fix(ingress): harden public platform exposure (#108)#109

Open
Agent-Hellboy wants to merge 1 commit intomainfrom
ingress/platform_security_hardening
Open

fix(ingress): harden public platform exposure (#108)#109
Agent-Hellboy wants to merge 1 commit intomainfrom
ingress/platform_security_hardening

Conversation

@Agent-Hellboy
Copy link
Copy Markdown
Owner

Addresses items 1, 2, 4, and 6 of the platform.mcpruntime.org security audit. Items 3 (unauth MCP tool execution) and 5 (public /api/runtime/servers) are intentionally not in this change.

  • UI service now sets baseline security headers (CSP with frame-ancestors 'none', X-Content-Type-Options, Referrer-Policy, Permissions-Policy, Cache-Control no-store on /api and /auth) and HSTS only when X-Forwarded-Proto is https.
  • New httpsRedirectMiddleware in the UI 308s HTTP -> HTTPS based on X-Forwarded-Proto. Behavior is gated by UI_REQUIRE_HTTPS (auto/true/false); auto skips localhost / IP-only hosts so the Kind dev stack is unaffected.
  • Platform Ingress no longer exposes /grafana or /prometheus on the public host (those tools have no built-in auth in this stack; operators should reach them via port-forward).
  • When TLS is configured, an additional HTTP-only Ingress is emitted for the same host so plain http://platform./ resolves to the UI (which then redirects), shadowing the host-less dev gateway Ingress that previously answered HTTP with 200.

Addresses items 1, 2, 4, and 6 of the platform.mcpruntime.org
security audit. Items 3 (unauth MCP tool execution) and 5 (public
/api/runtime/servers) are intentionally not in this change.

- UI service now sets baseline security headers (CSP with
  frame-ancestors 'none', X-Content-Type-Options, Referrer-Policy,
  Permissions-Policy, Cache-Control no-store on /api and /auth) and
  HSTS only when X-Forwarded-Proto is https.
- New httpsRedirectMiddleware in the UI 308s HTTP -> HTTPS based on
  X-Forwarded-Proto. Behavior is gated by UI_REQUIRE_HTTPS
  (auto/true/false); auto skips localhost / IP-only hosts so the
  Kind dev stack is unaffected.
- Platform Ingress no longer exposes /grafana or /prometheus on the
  public host (those tools have no built-in auth in this stack;
  operators should reach them via port-forward).
- When TLS is configured, an additional HTTP-only Ingress is emitted
  for the same host so plain http://platform.<domain>/ resolves to
  the UI (which then redirects), shadowing the host-less dev gateway
  Ingress that previously answered HTTP with 200.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.21%. Comparing base (92178fe) to head (1146e9c).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #109      +/-   ##
==========================================
+ Coverage   57.15%   57.21%   +0.05%     
==========================================
  Files          59       59              
  Lines       10448    10462      +14     
==========================================
+ Hits         5972     5986      +14     
  Misses       3905     3905              
  Partials      571      571              
Flag Coverage Δ
pre-merge 57.21% <100.00%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
internal/cli/platform_ingress.go 94.44% <100.00%> (+1.02%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request improves platform security and routing by restricting public access to Grafana and Prometheus and implementing automated HTTP-to-HTTPS redirection. It introduces middleware to manage security headers, including Content Security Policy (CSP), HSTS, and Cache-Control. Review feedback recommends further hardening the security configuration by refactoring the CSP to eliminate 'unsafe-inline', opting out of FLoC tracking via the Permissions-Policy header, and expanding Cache-Control directives for sensitive API and authentication endpoints.

Comment thread services/ui/main.go
// Google Sign-In needs accounts.google.com for scripts/iframes/connect.
h.Set("Content-Security-Policy",
"default-src 'self'; "+
"script-src 'self' 'unsafe-inline' https://accounts.google.com https://apis.google.com; "+
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.

security-medium medium

The use of 'unsafe-inline' in the script-src directive of the Content Security Policy (CSP) significantly weakens the protection against Cross-Site Scripting (XSS) attacks. While it may be necessary for some legacy or third-party scripts, it is highly recommended to refactor the frontend to use nonces or hashes for inline scripts, or move them to external files, to allow for a more restrictive policy.

Comment thread services/ui/main.go
h := w.Header()
h.Set("X-Content-Type-Options", "nosniff")
h.Set("Referrer-Policy", "strict-origin-when-cross-origin")
h.Set("Permissions-Policy", "camera=(), microphone=(), geolocation=(), payment=(), usb=()")
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.

security-medium medium

Consider adding interest-cohort=() to the Permissions-Policy header to explicitly opt-out of Google's FLoC (Federated Learning of Cohorts) tracking, which is a common privacy-hardening practice for modern web applications.

Comment thread services/ui/main.go
}
path := r.URL.Path
if strings.HasPrefix(path, "/api") || strings.HasPrefix(path, "/auth/") {
h.Set("Cache-Control", "no-store")
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.

security-medium medium

For maximum security on sensitive endpoints like /api and /auth/, consider expanding the Cache-Control header to include no-cache and must-revalidate (e.g., no-store, no-cache, must-revalidate). While no-store is the strongest directive, adding the others ensures that even if a browser or intermediary proxy doesn't fully respect no-store, the data is not served from cache without revalidation.

Suggested change
h.Set("Cache-Control", "no-store")
h.Set("Cache-Control", "no-store, no-cache, must-revalidate")

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1146e9c8c4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread services/ui/main.go
return true
}
// No proxy header. Only redirect in forced mode for non-local hosts.
return strings.EqualFold(strings.TrimSpace(mode), "true") && !isLocalHost(r.Host)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Treat on/1/yes as forced HTTPS mode

The forced-mode aliases declared in shouldRedirectToHTTPS are not handled consistently when X-Forwarded-Proto is missing: "on", "1", and "yes" enter the forced-mode branch in the switch, but the fallback path only checks mode == "true". In deployments where the upstream proxy does not set X-Forwarded-Proto, setting UI_REQUIRE_HTTPS=on (or 1/yes) silently disables the intended redirect and allows plain-HTTP responses for public hosts.

Useful? React with 👍 / 👎.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant