From 2451b1acc04e7982ec2baf62a6411dc610dc69fd Mon Sep 17 00:00:00 2001 From: Backend Engineer Date: Thu, 16 Apr 2026 10:34:25 +0000 Subject: [PATCH 1/2] fix(provisioner): rebuild_config flag on restart recovers from destroyed config volume (closes #239) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a workspace container AND its /configs Docker volume are both destroyed, the restart handler previously had no recovery path — findTemplateByName searched only the top-level configsDir, which holds workspace-instance dirs (ws-{id[:12]}/), not the role-named org-template source directories. Fix: add `rebuild_config: true` to the POST /workspaces/:id/restart body struct. When set, the handler falls back to searching configsDir/org-templates/ via the existing findTemplateByName logic (which already handles name normalisation and config.yaml name-field matching). The workspace can then self-recover with its own bearer token — no admin intervention required. New helper: resolveOrgTemplate(configsDir, wsName) — pure function, independently tested (4 cases: hit-by-dir, hit-by-config-yaml, no org-templates dir, no match). Usage: curl -X POST -H "Authorization: Bearer $(cat /configs/.auth_token)" \ -d '{"rebuild_config": true}' \ http://platform:8080/workspaces/$WORKSPACE_ID/restart Co-Authored-By: Claude Sonnet 4.6 --- .../internal/handlers/workspace_provision.go | 19 ++++++ .../handlers/workspace_provision_test.go | 60 +++++++++++++++++++ .../internal/handlers/workspace_restart.go | 12 ++++ 3 files changed, 91 insertions(+) diff --git a/platform/internal/handlers/workspace_provision.go b/platform/internal/handlers/workspace_provision.go index bdfe6af5..99930029 100644 --- a/platform/internal/handlers/workspace_provision.go +++ b/platform/internal/handlers/workspace_provision.go @@ -248,6 +248,25 @@ func findTemplateByName(configsDir, name string) string { return "" } +// resolveOrgTemplate looks for a matching role directory under +// configsDir/org-templates/ and returns the absolute path and a short label +// ("org-templates/"). Used by the restart handler's rebuild_config path +// (#239) so a workspace can recover from a destroyed config volume without +// admin intervention. +// Returns ("", "") when no match is found. +func resolveOrgTemplate(configsDir, wsName string) (path, label string) { + orgDir := filepath.Join(configsDir, "org-templates") + match := findTemplateByName(orgDir, wsName) + if match == "" { + return "", "" + } + full := filepath.Join(orgDir, match) + if _, err := os.Stat(full); err != nil { + return "", "" + } + return full, "org-templates/" + match +} + // configDirName returns the standard config directory name for a workspace ID. // Used by resolveConfigDir in templates.go for host-side template resolution. func configDirName(workspaceID string) string { diff --git a/platform/internal/handlers/workspace_provision_test.go b/platform/internal/handlers/workspace_provision_test.go index 9232b67a..d983a9ed 100644 --- a/platform/internal/handlers/workspace_provision_test.go +++ b/platform/internal/handlers/workspace_provision_test.go @@ -121,6 +121,66 @@ func TestFindTemplateByName_InvalidDir(t *testing.T) { } } +// ==================== resolveOrgTemplate ==================== + +// TestResolveOrgTemplate_HitByDirName verifies the happy path: org-templates/ +// dir exists with a normalized name match. +func TestResolveOrgTemplate_HitByDirName(t *testing.T) { + configsDir := t.TempDir() + orgDir := filepath.Join(configsDir, "org-templates") + roleDir := filepath.Join(orgDir, "technical-researcher") + os.MkdirAll(roleDir, 0755) + + path, label := resolveOrgTemplate(configsDir, "Technical Researcher") + if path != roleDir { + t.Errorf("expected path %q, got %q", roleDir, path) + } + if label != "org-templates/technical-researcher" { + t.Errorf("expected label %q, got %q", "org-templates/technical-researcher", label) + } +} + +// TestResolveOrgTemplate_HitByConfigYAML verifies the config.yaml name-field +// fallback works when the dir name doesn't match the workspace name directly. +func TestResolveOrgTemplate_HitByConfigYAML(t *testing.T) { + configsDir := t.TempDir() + orgDir := filepath.Join(configsDir, "org-templates") + roleDir := filepath.Join(orgDir, "org-backend") + os.MkdirAll(roleDir, 0755) + os.WriteFile(filepath.Join(roleDir, "config.yaml"), []byte("name: Backend Engineer\n"), 0644) + + path, label := resolveOrgTemplate(configsDir, "Backend Engineer") + if path != roleDir { + t.Errorf("expected path %q, got %q", roleDir, path) + } + if label != "org-templates/org-backend" { + t.Errorf("expected label %q, got %q", "org-templates/org-backend", label) + } +} + +// TestResolveOrgTemplate_NoOrgTemplatesDir returns empty when the org-templates +// directory does not exist. +func TestResolveOrgTemplate_NoOrgTemplatesDir(t *testing.T) { + configsDir := t.TempDir() // no org-templates subdir created + + path, label := resolveOrgTemplate(configsDir, "Technical Researcher") + if path != "" || label != "" { + t.Errorf("expected empty, got path=%q label=%q", path, label) + } +} + +// TestResolveOrgTemplate_NoMatchInOrgTemplates returns empty when org-templates +// exists but has no entry matching the workspace name. +func TestResolveOrgTemplate_NoMatchInOrgTemplates(t *testing.T) { + configsDir := t.TempDir() + os.MkdirAll(filepath.Join(configsDir, "org-templates", "seo-agent"), 0755) + + path, label := resolveOrgTemplate(configsDir, "Backend Engineer") + if path != "" || label != "" { + t.Errorf("expected empty, got path=%q label=%q", path, label) + } +} + // ==================== ensureDefaultConfig ==================== func TestEnsureDefaultConfig_LangGraph(t *testing.T) { diff --git a/platform/internal/handlers/workspace_restart.go b/platform/internal/handlers/workspace_restart.go index 503c6d68..3a263d39 100644 --- a/platform/internal/handlers/workspace_restart.go +++ b/platform/internal/handlers/workspace_restart.go @@ -107,6 +107,7 @@ func (h *WorkspaceHandler) Restart(c *gin.Context) { Template string `json:"template"` ApplyTemplate bool `json:"apply_template"` // force re-apply runtime-default template (e.g. after runtime change) Reset bool `json:"reset"` // #12: discard claude-sessions volume before restart + RebuildConfig bool `json:"rebuild_config"` // #239: re-render config volume from org-template source (recovery path when volume was destroyed) } c.ShouldBindJSON(&body) @@ -131,6 +132,17 @@ func (h *WorkspaceHandler) Restart(c *gin.Context) { } } + // #239: rebuild_config=true — try org-templates as last-resort source so a + // workspace with a destroyed config volume can self-recover without admin + // intervention. Only fires when no other template was resolved above. + if templatePath == "" && body.RebuildConfig { + if p, label := resolveOrgTemplate(h.configsDir, wsName); p != "" { + templatePath = p + configLabel = label + log.Printf("Restart: rebuild_config — using org-template %s for %s (%s)", label, wsName, id) + } + } + if templatePath == "" { log.Printf("Restart: reusing existing config volume for %s (%s)", wsName, id) } else { From b0381d656c9f1f6ff417d94e3317194a1b8fe604 Mon Sep 17 00:00:00 2001 From: Backend Engineer Date: Thu, 16 Apr 2026 10:34:35 +0000 Subject: [PATCH 2/2] fix(security): registry DB errors must not leak raw driver messages (closes #435) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Register handler was serialising the raw Go error into the HTTP response: c.JSON(500, gin.H{"error": fmt.Sprintf("failed to register: %v", err)}) PostgreSQL errors wrapped by lib/pq contain table names, constraint names, and driver-version strings — enough for a caller to fingerprint the schema and craft targeted attacks. The error is already logged at full detail with Printf before this line, so callers only need the generic message. Fix: replace the Sprintf with a static "registration failed" string (same pattern the heartbeat and update-card handlers already used). New test: TestRegister_DBErrorResponseIsOpaque verifies the response body is the opaque string and that "sql:", "pq:", and "connection" substrings are absent. Co-Authored-By: Claude Sonnet 4.6 --- platform/internal/handlers/registry.go | 2 +- platform/internal/handlers/registry_test.go | 57 +++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/platform/internal/handlers/registry.go b/platform/internal/handlers/registry.go index bb4aecfb..445d6903 100644 --- a/platform/internal/handlers/registry.go +++ b/platform/internal/handlers/registry.go @@ -144,7 +144,7 @@ func (h *RegistryHandler) Register(c *gin.Context) { `, payload.ID, payload.ID, payload.URL, agentCardStr) if err != nil { log.Printf("Registry register error: %v (id=%s)", err, payload.ID) - c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("failed to register: %v", err)}) + c.JSON(http.StatusInternalServerError, gin.H{"error": "registration failed"}) return } diff --git a/platform/internal/handlers/registry_test.go b/platform/internal/handlers/registry_test.go index d38e3679..2420c3ed 100644 --- a/platform/internal/handlers/registry_test.go +++ b/platform/internal/handlers/registry_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "strings" "testing" "github.com/DATA-DOG/go-sqlmock" @@ -597,3 +598,59 @@ func TestRegister_C18_HijackBlockedNoBearer(t *testing.T) { t.Errorf("C18 hijack: unmet expectations: %v", err) } } + +// ==================== Issue #435 — DB error must not leak raw message ==================== + +// TestRegister_DBErrorResponseIsOpaque verifies that when the DB upsert fails, +// the HTTP response body contains only the generic "registration failed" message +// and never the raw Go/PostgreSQL error string (issue #435). +func TestRegister_DBErrorResponseIsOpaque(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + broadcaster := newTestBroadcaster() + handler := NewRegistryHandler(broadcaster) + + // C18 pre-check — no live tokens (bootstrap path). + mock.ExpectQuery("SELECT COUNT\\(\\*\\) FROM workspace_auth_tokens"). + WithArgs("ws-errtest"). + WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0)) + + // DB upsert fails with a descriptive internal error. + mock.ExpectExec("INSERT INTO workspaces"). + WithArgs("ws-errtest", "ws-errtest", "http://localhost:9200", `{"name":"err-agent"}`). + WillReturnError(sql.ErrConnDone) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("POST", "/registry/register", + bytes.NewBufferString(`{"id":"ws-errtest","url":"http://localhost:9200","agent_card":{"name":"err-agent"}}`)) + c.Request.Header.Set("Content-Type", "application/json") + + handler.Register(c) + + if w.Code != http.StatusInternalServerError { + t.Errorf("expected 500, got %d: %s", w.Code, w.Body.String()) + } + + var resp map[string]interface{} + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("response is not valid JSON: %v — body: %s", err, w.Body.String()) + } + + errMsg, ok := resp["error"].(string) + if !ok { + t.Fatalf("expected string 'error' field, got %T: %v", resp["error"], resp["error"]) + } + if errMsg != "registration failed" { + t.Errorf("expected opaque 'registration failed', got %q (raw error leaked)", errMsg) + } + // Confirm the raw driver error string is absent. + rawBody := w.Body.String() + if strings.Contains(rawBody, "sql:") || strings.Contains(rawBody, "pq:") || strings.Contains(rawBody, "connection") { + t.Errorf("raw DB error leaked into response body: %s", rawBody) + } + + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +}