Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 56 additions & 11 deletions pkg/auth/pages.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,49 +17,94 @@ limitations under the License.
package auth

import (
"fmt"
"encoding/json"
"html/template"
"log/slog"
"net/http"
)

const pageStyle = `font-family:system-ui,sans-serif;display:flex;` +
`justify-content:center;align-items:center;height:100vh;margin:0`

// JS redirect + clickable fallback is more reliable than meta-refresh for
// Cursor's cursor:// and http://localhost:8787/callback handoffs.
const redirectPageTmpl = `<!DOCTYPE html>
<html>
<head>
<title>%s</title>
<meta http-equiv="refresh" content="0; url=%s" />
<title>{{.Message}}</title>
</head>
<body style="` + pageStyle + `">
<div style="text-align:center"><p>%s</p></div>
<div style="text-align:center">
<p>{{.Message}}</p>
<p><a id="continue" href="{{.URL}}">Continue</a></p>
</div>
<script>window.location.replace({{.URLJS}});</script>
</body>
</html>`

const callbackPageTmpl = `<!DOCTYPE html>
<html>
<head>
<title>Authentication Successful</title>
<meta http-equiv="refresh" content="0; url=%s" />
</head>
<body style="` + pageStyle + `">
<div style="text-align:center">
<h2>Authentication complete</h2>
<p>You may now close this window.</p>
<p>Returning to Cursor&hellip;</p>
<p>If Cursor stays on &ldquo;waiting for callback&rdquo;, click:</p>
<p><a id="continue" href="{{.URL}}">Open Cursor callback</a></p>
</div>
<script>window.location.replace({{.URLJS}});</script>
</body>
</html>`

type redirectPageData struct {
Message string
// URL is typed template.URL (rather than string) because html/template
// otherwise sanitizes href values against a scheme allowlist that does
// NOT include custom schemes like cursor:// — an untyped string would
// silently render as the inert placeholder "#ZgotmplZ" instead of the
// real link. Callers must only pass already-validated redirect URIs
// (see isValidRedirectURI / HandleRegister's scheme checks).
URL template.URL
URLJS template.JS
}

func writeRedirectPage(w http.ResponseWriter, targetURL, message string) {
writeHTMLTemplate(w, redirectPageTmpl, redirectPageData{
Message: message,
URL: template.URL(targetURL),
URLJS: jsString(targetURL),
})
}

func writeCallbackPage(w http.ResponseWriter, redirectURL string) {
writeHTMLTemplate(w, callbackPageTmpl, redirectPageData{
URL: template.URL(redirectURL),
URLJS: jsString(redirectURL),
Comment on lines +61 to +84

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map authentication validation and handler symbols before inspecting them.
ast-grep outline pkg/auth --items all --type function

# Trace redirect URI validation, registration, storage, and tests.
rg -n -C 10 'isValidRedirectURI|HandleRegister|RedirectURI|redirect_uri|javascript:|cursor://|localhost:8787' pkg/auth

Repository: redhat-data-and-ai/unstructured-data-controller

Length of output: 22908


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- pkg/auth/server.go ---'
cat -n pkg/auth/server.go | sed -n '1,245p'
printf '%s\n' '--- pkg/auth/pages.go ---'
cat -n pkg/auth/pages.go | sed -n '1,125p'
printf '%s\n' '--- redirect-page call sites ---'
rg -n -C 8 'writeRedirectPage|writeCallbackPage|StoreCompletion|ConsumeCompletion|StorePending' pkg/auth
printf '%s\n' '--- scheme definitions and related tests ---'
rg -n -C 8 'dangerousSchemes|redirect.*URI|RedirectURIs|HandleRegister|HandleComplete' --glob '*.go' .

Repository: redhat-data-and-ai/unstructured-data-controller

Length of output: 50403


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from urllib.parse import urlsplit

dangerous = {"javascript", "data", "vbscript", "file"}
cases = [
    "cursor://callback",
    "http://localhost:8787/callback",
    "https://example.com/callback",
    "javascript:alert(1)",
    "data:text/html,<script>alert(1)</script>",
    "vbscript:msgbox(1)",
    "file:///tmp/callback",
    "ftp://example.com/callback",
    "custom://callback",
]

def accepted(uri):
    try:
        u = urlsplit(uri)
    except ValueError:
        return False
    if not u.scheme or not u.netloc or u.scheme in dangerous:
        return False
    hostname = u.hostname
    is_localhost = hostname in {"localhost", "127.0.0.1", "[::1]"}
    if u.scheme == "http" and not is_localhost:
        return False
    return True

for uri in cases:
    print(f"{uri!r}: {accepted(uri)}")
PY

Repository: redhat-data-and-ai/unstructured-data-controller

Length of output: 485


Restrict redirect URI schemes before storing clients.

HandleAuthorize stores only registered URIs, and registration rejects javascript, data, blob, and vbscript. However, registration still accepts arbitrary schemes such as ftp:// and custom://. template.URL then bypasses scheme filtering, and HandleComplete sends these URIs to both href and window.location.replace. Allow only cursor:// and loopback callback URIs.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/auth/pages.go` around lines 61 - 84, Restrict redirect URI validation in
the registration flow before clients are stored, using the existing validation
symbols such as isValidRedirectURI and HandleRegister. Accept only cursor://
URIs or approved loopback callback URIs, and reject ftp://, custom://, and all
other schemes so values later passed through writeRedirectPage and
writeCallbackPage cannot reach template.URL or window.location.replace.

})
}

func writeHTMLTemplate(w http.ResponseWriter, tmpl string, data redirectPageData) {
w.Header().Set("Content-Type", "text/html; charset=utf-8")
if _, err := fmt.Fprintf(w, redirectPageTmpl, message, targetURL, message); err != nil {
t, err := template.New("page").Parse(tmpl)
if err != nil {
slog.Error("failed to parse redirect page template", "error", err)
http.Error(w, "internal error", http.StatusInternalServerError)
return
}
if err := t.Execute(w, data); err != nil {
slog.Error("failed to write redirect page", "error", err)
}
}

func writeCallbackPage(w http.ResponseWriter, redirectURL string) {
w.Header().Set("Content-Type", "text/html; charset=utf-8")
if _, err := fmt.Fprintf(w, callbackPageTmpl, redirectURL); err != nil {
slog.Error("failed to write callback page", "error", err)
// jsString returns s as a safely-quoted JS string literal for embedding
// directly inside a <script> block (avoids the double-escaping bug that
// comes from mixing html/template auto-escaping with manual escaping).
func jsString(s string) template.JS {
b, err := json.Marshal(s)
if err != nil {
return template.JS(`""`)
}
return template.JS(b)
}
7 changes: 5 additions & 2 deletions pkg/auth/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,12 +229,15 @@ func (s *OAuthServer) HandleCallback(w http.ResponseWriter, r *http.Request) {
// navigates to a clean /auth/complete URL with no sensitive parameters.
completionToken := s.store.StoreCompletion(redirectURL.String())

s.logger.Info("authentication successful, redirecting to client", "client_id", safePrefix(pending.ClientID))
s.logger.Info("authentication successful, redirecting to client",
"client_id", safePrefix(pending.ClientID),
"client_redirect", pending.RedirectURI,
)
Comment on lines +232 to +235

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Redact the redirect URI in the Info log.

pending.RedirectURI can contain static query parameters, user identifiers, or tokens. Line 234 stores all of them in routine logs. Log the parsed scheme and host, or a URI with user info, query, and fragment removed.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/auth/server.go` around lines 232 - 235, Update the authentication success
log in the server handler to redact pending.RedirectURI before passing it as
client_redirect. Parse the URI and log only its scheme and host, or sanitize it
by removing user info, query, and fragment, while preserving the existing
authentication flow and safePrefix handling for client ID.

http.Redirect(w, r, "/auth/complete/"+completionToken, http.StatusFound)
}

// HandleComplete serves the post-authentication success page at a clean URL
// with no OAuth parameters exposed. It renders a meta-refresh redirect to the
// with no OAuth parameters exposed. It renders a JS-redirect page to the
// MCP client's redirect_uri.
// GET /auth/complete/{token}
func (s *OAuthServer) HandleComplete(w http.ResponseWriter, r *http.Request) {
Expand Down
Loading