Merge pull request #446 from Molecule-AI/fix/issue-435-registry-error-leak

fix(security): suppress raw DB error from /registry/register response
This commit is contained in:
Hongming Wang 2026-04-16 05:16:57 -07:00 committed by GitHub
commit 1184232d86
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 149 additions and 1 deletions

View File

@ -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
}

View File

@ -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)
}
}

View File

@ -259,6 +259,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/<dir>"). 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 {

View File

@ -121,6 +121,66 @@ func TestFindTemplateByName_InvalidDir(t *testing.T) {
}
}
// ==================== resolveOrgTemplate ====================
// TestResolveOrgTemplate_HitByDirName verifies the happy path: org-templates/<role>
// 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) {

View File

@ -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 {