diff --git a/platform/internal/handlers/activity.go b/platform/internal/handlers/activity.go index 4699a3a9..6eabf44a 100644 --- a/platform/internal/handlers/activity.go +++ b/platform/internal/handlers/activity.go @@ -338,10 +338,12 @@ func (h *ActivityHandler) Report(c *gin.Context) { // Empty source_id falls through to the default-to-self branch below. sourceID := body.SourceID if sourceID != "" && sourceID != workspaceID { - // Log the spoof attempt as a security event so an auditor cron can - // surface repeat probing. Keep the log line stable (greppable) and - // avoid echoing attacker-supplied data verbatim beyond the UUIDs. - log.Printf("security: source_id spoof attempt — authed_workspace=%s body_source_id=%s remote=%s", + // #234: sanitize attacker-controlled values before logging. + // body.SourceID comes from a JSON request, and json.Unmarshal + // decodes \n escapes into literal newlines — an authenticated + // workspace could otherwise inject fake log lines. Use %q which + // emits a Go-quoted string with all control characters escaped. + log.Printf("security: source_id spoof attempt — authed_workspace=%s body_source_id=%q remote=%q", workspaceID, sourceID, c.ClientIP()) c.JSON(http.StatusForbidden, gin.H{"error": "source_id must match authenticated workspace"}) return diff --git a/platform/internal/handlers/handlers_test.go b/platform/internal/handlers/handlers_test.go index 20897baf..d6cfca9f 100644 --- a/platform/internal/handlers/handlers_test.go +++ b/platform/internal/handlers/handlers_test.go @@ -1131,3 +1131,34 @@ func TestActivityHandler_Report_MatchingSourceIDAccepted(t *testing.T) { t.Errorf("matching source_id: got %d, want 200 (%s)", w.Code, w.Body.String()) } } + +// TestActivityHandler_Report_SourceIDLogInjection — #234 regression guard. +// The security log line must emit the attacker-supplied source_id through +// %q so control characters (\n, \r, \t) are escaped instead of splitting +// the log stream into fake entries. Harder to assert directly without a +// log capture, so we just exercise the code path with a payload containing +// newlines and confirm the handler still returns 403 cleanly (no panic, +// no accidental success). +func TestActivityHandler_Report_SourceIDLogInjection(t *testing.T) { + setupTestDB(t) + setupTestRedis(t) + broadcaster := newTestBroadcaster() + handler := NewActivityHandler(broadcaster) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Params = gin.Params{{Key: "id", Value: "ws-alice"}} + // JSON body with explicit \n escapes — json.Unmarshal decodes these + // into literal newline bytes before reaching the log call. + body := `{"activity_type":"agent_log","summary":"x","source_id":"ws-evil\ntimestamp=FORGED level=INFO msg=fake"}` + c.Request = httptest.NewRequest("POST", "/workspaces/ws-alice/activity", + bytes.NewBufferString(body)) + c.Request.Header.Set("Content-Type", "application/json") + + handler.Report(c) + + if w.Code != http.StatusForbidden { + t.Errorf("spoof with newline in source_id: got %d, want 403 (%s)", + w.Code, w.Body.String()) + } +}