From c02cb0e1b628c5c8822eab3b36285281aa16d0fd Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Wed, 29 Apr 2026 14:33:41 -0700 Subject: [PATCH] review: defer forward-time URL re-validation to follow-up (#2316) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review found the original draft of this PR added forward-time validateAgentURL() as defense-in-depth — paranoia layer on top of the existing register-time gate. The validator unconditionally blocks loopback (127.0.0.1/8), which makes httptest-based proxy tests impossible without an env-var hatch I'd rather not add to a security- critical path on first pass. Trust note kept inline pointing at the upstream gate + tracking issue so the gap is explicit, not invisible. Refs #2312. --- workspace-server/internal/handlers/chat_files.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/workspace-server/internal/handlers/chat_files.go b/workspace-server/internal/handlers/chat_files.go index 7f5480d3..713a613a 100644 --- a/workspace-server/internal/handlers/chat_files.go +++ b/workspace-server/internal/handlers/chat_files.go @@ -177,6 +177,10 @@ func (h *ChatFilesHandler) Upload(c *gin.Context) { c.JSON(http.StatusServiceUnavailable, gin.H{"error": "workspace url not registered yet"}) return } + // Trust note: workspaces.url passes validateAgentURL at /registry/ + // register write time, blocking SSRF-shaped URLs. We rely on that + // upstream gate rather than re-validating here. Tracked at #2316 + // for follow-up: forward-time re-validation as defense-in-depth. secret, err := wsauth.ReadPlatformInboundSecret(ctx, db.DB, workspaceID) if err != nil {