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
17 changes: 16 additions & 1 deletion backend/internal/httpd/controllers/sessions.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,19 @@ func (c *SessionsController) preview(w http.ResponseWriter, r *http.Request) {
envelope.WriteJSON(w, http.StatusOK, res)
}

// previewMarkdownCSP hardens rendered-Markdown previews: no scripts and no
// network access (a rendered document needs neither), while still allowing the
// inline stylesheet and images/badges so the document renders normally. This
// severs the "previewed file runs a script that calls the loopback API" chain
// (issue #2771) for the Markdown path at zero cost to document rendering.
const previewMarkdownCSP = "default-src 'none'; img-src * data:; style-src 'unsafe-inline'; font-src * data:; connect-src 'none'; base-uri 'none'; form-action 'none'"

// previewFileCSP applies to raw workspace files served verbatim (including
// .html, which cannot be sanitized). Scripts/styles/images still load so a
// built static app renders, but connect-src 'none' blocks fetch/XHR/WebSocket
// so page script cannot reach the loopback control API (issue #2771).
const previewFileCSP = "connect-src 'none'; form-action 'none'"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This raw-file policy is bypassable on the legacy same-origin /preview/files/ route, so it does not fully deliver "page script cannot reach the loopback control API" for raw .html.

connect-src 'none' only constrains fetch/XHR issued from this document's own realm. It has no frame-src/child-src/object-src/worker-src, and there is no default-src to backstop them. On the legacy route the preview is same-origin with the API (http://127.0.0.1:<port>), and the daemon sets no global X-Frame-Options/frame-ancestors (nothing in internal/httpd sets them). So a malicious index.html served under this policy (scripts run) can reach the sink through a nested same-origin iframe:

const f = document.createElement('iframe');
f.src = '/api/v1/health';        // any same-origin API doc; its response carries NO CSP
f.onload = () => f.contentWindow.fetch('/api/v1/projects/ID/config', {
  method: 'PUT',
  headers: { 'Content-Type': 'application/json' },
  body: '{"postCreate":"..."}',
});
document.body.appendChild(f);

The fetch executes in the child realm, whose response has no CSP, so the parent's connect-src 'none' does not apply. (An about:blank iframe inherits the parent policy and would be blocked, but a navigated same-origin document gets its own empty policy, which is the hole.) The Markdown policy is unaffected because default-src 'none' implies frame-src 'none'.

Suggested fix: add frame-src 'none'; child-src 'none'; object-src 'none'; worker-src 'none' to previewFileCSP (or use default-src 'none' and re-allow only script-src/style-src/img-src/font-src). Note this may restrict a built app that legitimately uses iframes/workers, so it is a real trade-off worth calling out.


func (c *SessionsController) previewFile(w http.ResponseWriter, r *http.Request) {
if c.Svc == nil {
apispec.NotImplemented(w, r, "GET", "/api/v1/sessions/{sessionId}/preview/files/*")
Expand Down Expand Up @@ -270,6 +283,7 @@ func (c *SessionsController) serveWorkspacePreviewFile(w http.ResponseWriter, r
}
defer func() { _ = file.Close() }()
if !previewutil.IsMarkdownPath(clean) {
w.Header().Set("Content-Security-Policy", previewFileCSP)
http.ServeContent(w, r, info.Name(), info.ModTime(), file)
return
}
Expand All @@ -284,9 +298,10 @@ func (c *SessionsController) serveWorkspacePreviewFile(w http.ResponseWriter, r
envelope.WriteError(w, r, err)
return
}
w.Header().Set("Content-Security-Policy", previewMarkdownCSP)
w.Header().Set("Content-Type", "text/html; charset=utf-8")
if r.Method != http.MethodHead {
_, _ = w.Write(rendered) //nolint:gosec // G705: preview content is workspace-local and agent-trusted
_, _ = w.Write(rendered) //nolint:gosec // G705: raw-HTML passthrough is contained by the restrictive CSP set above (default-src 'none'; connect-src 'none'), so previewed content cannot reach the loopback API — see issue #2771
}
}

Expand Down
36 changes: 36 additions & 0 deletions backend/internal/httpd/controllers/sessions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,42 @@ func TestSessionsAPI_PreviewRejectsSessionIDTooLongForHostname(t *testing.T) {
assertErrorCode(t, body, status, http.StatusUnprocessableEntity, "PREVIEW_SESSION_ID_UNSUPPORTED")
}

func TestSessionsAPI_PreviewSendsCSP(t *testing.T) {
svc := newFakeSessionService()
workspace := t.TempDir()
if err := os.WriteFile(filepath.Join(workspace, "README.md"),
[]byte("# Hi\n\n<script>fetch('/api/v1/projects')</script>"), 0o644); err != nil {
t.Fatalf("write md: %v", err)
}
if err := os.WriteFile(filepath.Join(workspace, "page.html"),
[]byte("<h1>hi</h1>"), 0o644); err != nil {
t.Fatalf("write html: %v", err)
}
s := svc.sessions["ao-1"]
s.Metadata = domain.SessionMetadata{WorkspacePath: workspace}
svc.sessions["ao-1"] = s
srv := newSessionTestServer(t, svc)

// Markdown path: strict CSP (no scripts, no network).
_, status, headers := doRequest(t, srv, "GET", "/api/v1/sessions/ao-1/preview/files/README.md", "")
if status != http.StatusOK {
t.Fatalf("md preview = %d, want 200", status)
}
if csp := headers.Get("Content-Security-Policy"); !strings.Contains(csp, "default-src 'none'") ||
!strings.Contains(csp, "connect-src 'none'") {
t.Fatalf("markdown CSP = %q, want default-src 'none' + connect-src 'none'", csp)
}

// Raw file path: connect-src 'none' cuts the API-reach vector.
_, status, headers = doRequest(t, srv, "GET", "/api/v1/sessions/ao-1/preview/files/page.html", "")
if status != http.StatusOK {
t.Fatalf("html preview = %d, want 200", status)
}
if csp := headers.Get("Content-Security-Policy"); !strings.Contains(csp, "connect-src 'none'") {
t.Fatalf("html CSP = %q, want connect-src 'none'", csp)
}
}

func TestSessionsAPI_SetPreviewExplicitURLPersists(t *testing.T) {
svc := newFakeSessionService()
srv := newSessionTestServer(t, svc)
Expand Down