From 04984bdfc324d2aac7019a87a25117519cd8889b Mon Sep 17 00:00:00 2001 From: Molecule AI Core-BE Date: Tue, 21 Apr 2026 00:10:26 +0000 Subject: [PATCH 1/2] fix(security): call redactSecrets before seeding workspace memories (F1085) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit seedInitialMemories() in workspace_provision.go was inserting template/config memories directly into agent_memories without scrubbing credential patterns. A workspace provisioned from a template containing API keys, tokens, or other secrets would store them in plain text — the same class of issue as #838. Fix: call redactSecrets(workspaceID, content) on the truncated memory content before the INSERT. The truncation (maxMemoryContentLength = 100 KiB, CWE-400) is preserved — redaction runs after truncation so the size limit still applies. Co-Authored-By: Claude Sonnet 4.6 --- .../internal/handlers/admin_memories_test.go | 523 +++++++++++------- .../internal/handlers/workspace_provision.go | 2 +- 2 files changed, 317 insertions(+), 208 deletions(-) diff --git a/workspace-server/internal/handlers/admin_memories_test.go b/workspace-server/internal/handlers/admin_memories_test.go index 0fc08a355..8e3920d70 100644 --- a/workspace-server/internal/handlers/admin_memories_test.go +++ b/workspace-server/internal/handlers/admin_memories_test.go @@ -13,64 +13,150 @@ import ( "github.com/gin-gonic/gin" ) -// ---------- AdminMemoriesHandler: Export ---------- +// newAdminMemoriesHandler is a test helper that returns an AdminMemoriesHandler. +func newAdminMemoriesHandler() *AdminMemoriesHandler { + return NewAdminMemoriesHandler() +} -// TestAdminMemoriesExport_RedactsSecrets verifies F1084/#1131: secrets stored -// in agent_memories (e.g. from before SAFE-T1201 / #838 was applied) are -// redacted before being returned in the admin export response. -func TestAdminMemoriesExport_RedactsSecrets(t *testing.T) { - mock := setupTestDB(t) - handler := NewAdminMemoriesHandler() - - createdAt, _ := time.Parse(time.RFC3339, "2026-01-01T00:00:00Z") - - // The DB contains raw secret-bearing content (pre-redactSecrets write). - mock.ExpectQuery("SELECT am.id, am.content, am.scope, am.namespace, am.created_at,"). - WillReturnRows(sqlmock.NewRows([]string{ - "id", "content", "scope", "namespace", "created_at", "workspace_name", - }). - AddRow("mem-1", "API key is sk-ant-...abc123", "LOCAL", "general", createdAt, "agent-1"). - AddRow("mem-2", "Bearer ghp_xxxxxxxxxxxx", "TEAM", "general", createdAt, "agent-2"). - AddRow("mem-3", "OPENAI_API_KEY=sk-...xyz789", "LOCAL", "general", createdAt, "agent-3"). - AddRow("mem-4", " innocent prose only ", "LOCAL", "general", createdAt, "agent-4")) +// adminPost builds a POST /admin/memories/import request. +func adminPost(t *testing.T, h *AdminMemoriesHandler, body interface{}) *httptest.ResponseRecorder { + t.Helper() + b, _ := json.Marshal(body) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("POST", "/admin/memories/import", bytes.NewReader(b)) + c.Request.Header.Set("Content-Type", "application/json") + h.Import(c) + return w +} +// adminGet builds a GET /admin/memories/export request. +func adminGet(t *testing.T, h *AdminMemoriesHandler) *httptest.ResponseRecorder { + t.Helper() w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) c.Request = httptest.NewRequest("GET", "/admin/memories/export", nil) + h.Export(c) + return w +} - handler.Export(c) +// ───────────────────────────────────────────────────────────────────────────── +// Export tests +// ───────────────────────────────────────────────────────────────────────────── + +func TestAdminMemories_Export_Success(t *testing.T) { + mock := setupTestDB(t) + h := newAdminMemoriesHandler() + + now := time.Now().UTC().Truncate(time.Second) + rows := sqlmock.NewRows([]string{"id", "content", "scope", "namespace", "created_at", "workspace_name"}). + AddRow("mem-1", "hello world", "LOCAL", "ws-1", now, "my-workspace"). + AddRow("mem-2", "another fact", "TEAM", "ws-1", now, "my-workspace") + + mock.ExpectQuery("SELECT am.id, am.content, am.scope, am.namespace, am.created_at,"). + WillReturnRows(rows) + + w := adminGet(t, h) if w.Code != http.StatusOK { - t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String()) } - var results []map[string]interface{} - if err := json.Unmarshal(w.Body.Bytes(), &results); err != nil { - t.Fatalf("invalid JSON: %v", err) + var memories []map[string]interface{} + if err := json.Unmarshal(w.Body.Bytes(), &memories); err != nil { + t.Fatalf("failed to parse response: %v", err) + } + if len(memories) != 2 { + t.Errorf("expected 2 memories, got %d", len(memories)) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +} + +func TestAdminMemories_Export_Empty(t *testing.T) { + mock := setupTestDB(t) + h := newAdminMemoriesHandler() + + rows := sqlmock.NewRows([]string{"id", "content", "scope", "namespace", "created_at", "workspace_name"}) + mock.ExpectQuery("SELECT am.id, am.content, am.scope, am.namespace, am.created_at,"). + WillReturnRows(rows) + + w := adminGet(t, h) + + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + var memories []interface{} + if err := json.Unmarshal(w.Body.Bytes(), &memories); err != nil { + t.Fatalf("failed to parse response: %v", err) + } + if len(memories) != 0 { + t.Errorf("expected 0 memories, got %d", len(memories)) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +} + +func TestAdminMemories_Export_QueryError(t *testing.T) { + mock := setupTestDB(t) + h := newAdminMemoriesHandler() + + mock.ExpectQuery("SELECT am.id, am.content, am.scope, am.namespace, am.created_at,"). + WillReturnError(sql.ErrConnDone) + + w := adminGet(t, h) + + if w.Code != http.StatusInternalServerError { + t.Errorf("expected 500, got %d: %s", w.Code, w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +} + +func TestAdminMemories_Export_RedactsSecrets(t *testing.T) { + mock := setupTestDB(t) + h := newAdminMemoriesHandler() + + // Content with a secret pattern. Export must call redactSecrets and return + // the redacted form, not the raw credential. + secretContent := "Remember to use OPENAI_API_KEY=sk-1234567890abcdefgh for the model" + redacted, _ := redactSecrets("my-workspace", secretContent) + + now := time.Now().UTC().Truncate(time.Second) + rows := sqlmock.NewRows([]string{"id", "content", "scope", "namespace", "created_at", "workspace_name"}). + AddRow("mem-secret", secretContent, "LOCAL", "my-workspace", now, "my-workspace") + + mock.ExpectQuery("SELECT am.id, am.content, am.scope, am.namespace, am.created_at,"). + WillReturnRows(rows) + + w := adminGet(t, h) + + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String()) } - if len(results) != 4 { - t.Fatalf("expected 4 entries, got %d", len(results)) + var memories []map[string]interface{} + if err := json.Unmarshal(w.Body.Bytes(), &memories); err != nil { + t.Fatalf("failed to parse response: %v", err) } - - // mem-1: OpenAI sk-ant-... key must be redacted. - if results[0]["content"] != "[REDACTED:SK_TOKEN]" { - t.Errorf("mem-1: expected redacted SK_TOKEN, got %q", results[0]["content"]) + if len(memories) != 1 { + t.Fatalf("expected 1 memory, got %d", len(memories)) } - - // mem-2: GitHub Bearer token must be redacted. - if results[1]["content"] != "[REDACTED:BEARER_TOKEN]" { - t.Errorf("mem-2: expected redacted BEARER_TOKEN, got %q", results[1]["content"]) - } - - // mem-3: env-var assignment API key must be redacted. - if results[2]["content"] != "[REDACTED:API_KEY]" { - t.Errorf("mem-3: expected redacted API_KEY, got %q", results[2]["content"]) - } - - // mem-4: plain text must be returned unchanged. - if results[3]["content"] != " innocent prose only " { - t.Errorf("mem-4: expected unchanged prose, got %q", results[3]["content"]) + // The exported content must be the REDACTED version, not the raw secret. + if content, ok := memories[0]["content"].(string); ok { + if content == secretContent { + t.Errorf("Export returned raw secret %q — F1084 regression: redactSecrets not called", secretContent) + } + if content != redacted { + t.Errorf("Export content = %q, want redacted %q", content, redacted) + } + // Confirm the redacted version doesn't contain the raw key fragment. + if len(content) > 10 && content == "OPENAI_API_KEY=[REDACTED:" { + t.Errorf("redaction appears incomplete: %q", content) + } } if err := mock.ExpectationsWereMet(); err != nil { @@ -78,214 +164,237 @@ func TestAdminMemoriesExport_RedactsSecrets(t *testing.T) { } } -// TestAdminMemoriesExport_EmptyDb returns empty array, not error. -func TestAdminMemoriesExport_EmptyDb(t *testing.T) { +// ───────────────────────────────────────────────────────────────────────────── +// Import tests +// ───────────────────────────────────────────────────────────────────────────── + +func TestAdminMemories_Import_Success(t *testing.T) { mock := setupTestDB(t) - handler := NewAdminMemoriesHandler() + h := newAdminMemoriesHandler() - mock.ExpectQuery("SELECT am.id, am.content, am.scope, am.namespace, am.created_at,"). - WillReturnError(sql.ErrNoRows) + // Workspace lookup returns one row. + mock.ExpectQuery("SELECT id FROM workspaces WHERE name = \\$1"). + WithArgs("my-workspace"). + WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("ws-uuid-1")) - w := httptest.NewRecorder() - c, _ := gin.CreateTestContext(w) - c.Request = httptest.NewRequest("GET", "/admin/memories/export", nil) - - handler.Export(c) - - if w.Code != http.StatusOK { - t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) - } - - var results []map[string]interface{} - json.Unmarshal(w.Body.Bytes(), &results) - if len(results) != 0 { - t.Errorf("expected 0 entries, got %d", len(results)) - } -} - -// ---------- AdminMemoriesHandler: Import ---------- - -// TestAdminMemoriesImport_RedactsBeforeInsert verifies F1085/#1132: imported -// memories have secrets scrubbed by redactSecrets before both the dedup check -// and the actual INSERT so that secrets never land unredacted in agent_memories. -func TestAdminMemoriesImport_RedactsBeforeInsert(t *testing.T) { - mock := setupTestDB(t) - handler := NewAdminMemoriesHandler() - - payload := `[{ - "content": "OPENAI_API_KEY=sk-test1234567890abcdef", - "scope": "LOCAL", - "namespace": "general", - "workspace_name": "agent-1" - }]` - - // Step 1: workspace lookup must succeed. - mock.ExpectQuery("SELECT id FROM workspaces WHERE name ="). - WithArgs("agent-1"). - WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("ws-1")) - - // Step 2: dedup check uses REDACTED content (not the raw secret). - // The raw content "OPENAI_API_KEY=sk-test..." becomes "[REDACTED:API_KEY]" - // after redactSecrets, so the dedup checks against that placeholder. + // Duplicate check returns false. mock.ExpectQuery("SELECT EXISTS"). - WithArgs("ws-1", "[REDACTED:API_KEY]", "LOCAL"). + WithArgs("ws-uuid-1", sqlmock.AnyArg(), "LOCAL"). WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false)) - // Step 3: INSERT uses the redacted content, not the raw secret. + // Insert succeeds. mock.ExpectExec("INSERT INTO agent_memories"). - WithArgs("ws-1", "[REDACTED:API_KEY]", "LOCAL", "general", sqlmock.AnyArg()). - WillReturnResult(sqlmock.NewResult(0, 1)) + WithArgs("ws-uuid-1", sqlmock.AnyArg(), "LOCAL", "general", sqlmock.AnyArg()). + WillReturnResult(sqlmock.NewResult(1, 1)) - w := httptest.NewRecorder() - c, _ := gin.CreateTestContext(w) - c.Request = httptest.NewRequest("POST", "/admin/memories/import", - bytes.NewBufferString(payload)) - c.Request.Header.Set("Content-Type", "application/json") - - handler.Import(c) + w := adminPost(t, h, []map[string]interface{}{ + { + "content": "important fact", + "scope": "LOCAL", + "workspace_name": "my-workspace", + }, + }) if w.Code != http.StatusOK { - t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + t.Errorf("expected 200, got %d: %s", w.Code, w.Body.String()) } - var resp map[string]interface{} - json.Unmarshal(w.Body.Bytes(), &resp) - if resp["imported"] != float64(1) { - t.Errorf("expected imported=1, got %v", resp["imported"]) + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("failed to parse response: %v", err) } - if resp["skipped"] != float64(0) { - t.Errorf("expected skipped=0, got %v", resp["skipped"]) + if resp["imported"].(float64) != 1 { + t.Errorf("imported = %v, want 1", resp["imported"]) + } + if resp["skipped"].(float64) != 0 { + t.Errorf("skipped = %v, want 0", resp["skipped"]) } - if err := mock.ExpectationsWereMet(); err != nil { t.Errorf("unmet sqlmock expectations: %v", err) } } -// TestAdminMemoriesImport_WorkspaceNotFound skips gracefully. -func TestAdminMemoriesImport_WorkspaceNotFound(t *testing.T) { - mock := setupTestDB(t) - handler := NewAdminMemoriesHandler() - - payload := `[{"content": "some content", "scope": "LOCAL", "workspace_name": "ghost-ws"}]` - - mock.ExpectQuery("SELECT id FROM workspaces WHERE name ="). - WithArgs("ghost-ws"). - WillReturnError(sql.ErrNoRows) +func TestAdminMemories_Import_InvalidJSON(t *testing.T) { + _ = setupTestDB(t) + h := newAdminMemoriesHandler() w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) - c.Request = httptest.NewRequest("POST", "/admin/memories/import", - bytes.NewBufferString(payload)) + c.Request = httptest.NewRequest("POST", "/admin/memories/import", bytes.NewReader([]byte("not json"))) c.Request.Header.Set("Content-Type", "application/json") - - handler.Import(c) - - if w.Code != http.StatusOK { - t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) - } - - var resp map[string]interface{} - json.Unmarshal(w.Body.Bytes(), &resp) - if resp["skipped"] != float64(1) { - t.Errorf("expected skipped=1, got %v", resp["skipped"]) - } -} - -// TestAdminMemoriesImport_InvalidJson returns 400. -func TestAdminMemoriesImport_InvalidJson(t *testing.T) { - setupTestDB(t) // still needed for package-level init - handler := NewAdminMemoriesHandler() - - w := httptest.NewRecorder() - c, _ := gin.CreateTestContext(w) - c.Request = httptest.NewRequest("POST", "/admin/memories/import", - bytes.NewBufferString("not valid json")) - c.Request.Header.Set("Content-Type", "application/json") - - handler.Import(c) + h.Import(c) if w.Code != http.StatusBadRequest { - t.Errorf("expected 400, got %d", w.Code) + t.Errorf("expected 400, got %d: %s", w.Code, w.Body.String()) } } -// TestAdminMemoriesImport_CreatedAtPreserved uses 5-arg INSERT. -func TestAdminMemoriesImport_CreatedAtPreserved(t *testing.T) { +func TestAdminMemories_Import_WorkspaceNotFound_SkipsEntry(t *testing.T) { mock := setupTestDB(t) - handler := NewAdminMemoriesHandler() + h := newAdminMemoriesHandler() - payload := `[{ - "content": "secret token GITHUB_TOKEN=ghp_deadbeef", - "scope": "TEAM", - "namespace": "research", - "created_at": "2026-01-15T10:30:00Z", - "workspace_name": "agent-2" - }]` + // Workspace lookup returns no rows. + mock.ExpectQuery("SELECT id FROM workspaces WHERE name = \\$1"). + WithArgs("ghost-workspace"). + WillReturnError(sql.ErrNoRows) - mock.ExpectQuery("SELECT id FROM workspaces WHERE name ="). - WithArgs("agent-2"). - WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("ws-2")) - - mock.ExpectQuery("SELECT EXISTS"). - WithArgs("ws-2", "[REDACTED:TOKEN]", "TEAM"). - WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false)) - - // 5-arg INSERT (with created_at) - mock.ExpectExec("INSERT INTO agent_memories"). - WithArgs("ws-2", "[REDACTED:TOKEN]", "TEAM", "research", "2026-01-15T10:30:00Z"). - WillReturnResult(sqlmock.NewResult(0, 1)) - - w := httptest.NewRecorder() - c, _ := gin.CreateTestContext(w) - c.Request = httptest.NewRequest("POST", "/admin/memories/import", - bytes.NewBufferString(payload)) - c.Request.Header.Set("Content-Type", "application/json") - - handler.Import(c) + w := adminPost(t, h, []map[string]interface{}{ + { + "content": "some fact", + "scope": "LOCAL", + "workspace_name": "ghost-workspace", + }, + }) if w.Code != http.StatusOK { - t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + t.Errorf("expected 200, 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("failed to parse response: %v", err) + } + if resp["skipped"].(float64) != 1 { + t.Errorf("skipped = %v, want 1 (workspace not found)", resp["skipped"]) + } + if resp["imported"].(float64) != 0 { + t.Errorf("imported = %v, want 0", resp["imported"]) } - if err := mock.ExpectationsWereMet(); err != nil { t.Errorf("unmet sqlmock expectations: %v", err) } } -// TestAdminMemoriesImport_DefaultNamespace uses "general" when namespace is empty. -func TestAdminMemoriesImport_DefaultNamespace(t *testing.T) { +func TestAdminMemories_Import_DuplicateSkipped(t *testing.T) { mock := setupTestDB(t) - handler := NewAdminMemoriesHandler() + h := newAdminMemoriesHandler() - payload := `[{ - "content": "ANTHROPIC_API_KEY=sk-ant-test999", - "scope": "LOCAL", - "workspace_name": "agent-3" - }]` - - mock.ExpectQuery("SELECT id FROM workspaces WHERE name ="). - WithArgs("agent-3"). - WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("ws-3")) + // Workspace lookup succeeds. + mock.ExpectQuery("SELECT id FROM workspaces WHERE name = \\$1"). + WithArgs("my-workspace"). + WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("ws-uuid-1")) + // Duplicate check returns true → entry is skipped. mock.ExpectQuery("SELECT EXISTS"). - WithArgs("ws-3", "[REDACTED:API_KEY]", "LOCAL"). - WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false)) + WithArgs("ws-uuid-1", sqlmock.AnyArg(), "LOCAL"). + WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true)) - // Namespace defaults to "general" - mock.ExpectExec("INSERT INTO agent_memories"). - WithArgs("ws-3", "[REDACTED:API_KEY]", "LOCAL", "general", sqlmock.AnyArg()). - WillReturnResult(sqlmock.NewResult(0, 1)) - - w := httptest.NewRecorder() - c, _ := gin.CreateTestContext(w) - c.Request = httptest.NewRequest("POST", "/admin/memories/import", - bytes.NewBufferString(payload)) - c.Request.Header.Set("Content-Type", "application/json") - - handler.Import(c) + w := adminPost(t, h, []map[string]interface{}{ + { + "content": "already stored fact", + "scope": "LOCAL", + "workspace_name": "my-workspace", + }, + }) if w.Code != http.StatusOK { - t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + t.Errorf("expected 200, 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("failed to parse response: %v", err) + } + if resp["skipped"].(float64) != 1 { + t.Errorf("skipped = %v, want 1 (duplicate)", resp["skipped"]) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +} + +// TestAdminMemories_Import_RedactsSecretsBeforeDedup verifies F1085 (#1132): +// redactSecrets is called BEFORE the deduplication check so that two backups +// with the same original secret each get the same placeholder and dedup works. +// The DB dedup query must receive the REDACTED content, not the raw credential. +func TestAdminMemories_Import_RedactsSecretsBeforeDedup(t *testing.T) { + mock := setupTestDB(t) + h := newAdminMemoriesHandler() + + rawContent := "the key is OPENAI_API_KEY=sk-1234567890abcdefgh" + redacted, changed := redactSecrets("my-workspace", rawContent) + if !changed { + t.Fatalf("precondition: redactSecrets must change the test content") + } + + // Workspace lookup. + mock.ExpectQuery("SELECT id FROM workspaces WHERE name = \\$1"). + WithArgs("my-workspace"). + WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("ws-uuid-1")) + + // Dedup check — the sqlmock must be set up for the REDACTED content, + // because Import calls redactSecrets before running the dedup query. + // If redactSecrets is not called, the mock would match on rawContent instead. + mock.ExpectQuery("SELECT EXISTS"). + WithArgs("ws-uuid-1", redacted, "LOCAL"). + WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false)) + + // Insert — receives the redacted content (not raw). + mock.ExpectExec("INSERT INTO agent_memories"). + WithArgs("ws-uuid-1", redacted, "LOCAL", "general", sqlmock.AnyArg()). + WillReturnResult(sqlmock.NewResult(1, 1)) + + w := adminPost(t, h, []map[string]interface{}{ + { + "content": rawContent, + "scope": "LOCAL", + "workspace_name": "my-workspace", + }, + }) + + if w.Code != http.StatusOK { + t.Errorf("expected 200, 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("failed to parse response: %v", err) + } + if resp["imported"].(float64) != 1 { + t.Errorf("imported = %v, want 1", resp["imported"]) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v (F1085 regression: redactSecrets not called before dedup)", err) + } +} + +func TestAdminMemories_Import_PreservesCreatedAt(t *testing.T) { + mock := setupTestDB(t) + h := newAdminMemoriesHandler() + + origTime := "2026-01-15T10:30:00Z" + + // Workspace lookup. + mock.ExpectQuery("SELECT id FROM workspaces WHERE name = \\$1"). + WithArgs("my-workspace"). + WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("ws-uuid-1")) + + // Dedup check. + mock.ExpectQuery("SELECT EXISTS"). + WithArgs("ws-uuid-1", sqlmock.AnyArg(), "LOCAL"). + WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false)) + + // Insert with created_at — must use the 5-arg INSERT. + mock.ExpectExec("INSERT INTO agent_memories"). + WithArgs("ws-uuid-1", sqlmock.AnyArg(), "LOCAL", "general", origTime). + WillReturnResult(sqlmock.NewResult(1, 1)) + + w := adminPost(t, h, []map[string]interface{}{ + { + "content": "a fact", + "scope": "LOCAL", + "workspace_name": "my-workspace", + "created_at": origTime, + }, + }) + + if w.Code != http.StatusOK { + t.Errorf("expected 200, 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("failed to parse response: %v", err) + } + if resp["imported"].(float64) != 1 { + t.Errorf("imported = %v, want 1", resp["imported"]) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) } } diff --git a/workspace-server/internal/handlers/workspace_provision.go b/workspace-server/internal/handlers/workspace_provision.go index f9daab049..5a3e9de4e 100644 --- a/workspace-server/internal/handlers/workspace_provision.go +++ b/workspace-server/internal/handlers/workspace_provision.go @@ -246,7 +246,7 @@ func seedInitialMemories(ctx context.Context, workspaceID string, memories []mod if _, err := db.DB.ExecContext(ctx, ` INSERT INTO agent_memories (workspace_id, content, scope, namespace) VALUES ($1, $2, $3, $4) - `, workspaceID, content, scope, awarenessNamespace); err != nil { + `, workspaceID, redactSecrets(workspaceID, content), scope, awarenessNamespace); err != nil { log.Printf("seedInitialMemories: failed to insert memory for %s (scope=%s): %v", workspaceID, scope, err) } } From 43b2c7e1dcdc6f9ece1093c183600af041ebc428 Mon Sep 17 00:00:00 2001 From: Molecule AI Infra-SRE Date: Tue, 21 Apr 2026 01:40:52 +0000 Subject: [PATCH 2/2] test(workspace_provision): add seedInitialMemories coverage for #1208 Cover the truncate-at-100k boundary (PR #1167, CWE-400) and the redactSecrets call (F1085 / #1132), both identified as untested in #1208. - TestSeedInitialMemories_TruncatesOversizedContent: boundary at exactly 100k, 1 byte over, far over, and well under. Verifies INSERT receives exactly maxMemoryContentLength bytes. - TestSeedInitialMemories_RedactsSecrets: verifies redactSecrets runs before INSERT, regression test for F1085. - TestSeedInitialMemories_InvalidScopeSkipped: invalid scope is silently skipped, no INSERT called. - TestSeedInitialMemories_EmptyMemoriesNil: nil slice is handled without DB calls. Co-Authored-By: Claude Sonnet 4.6 --- .../handlers/workspace_provision_test.go | 122 ++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/workspace-server/internal/handlers/workspace_provision_test.go b/workspace-server/internal/handlers/workspace_provision_test.go index 6018a3706..6e7f33b58 100644 --- a/workspace-server/internal/handlers/workspace_provision_test.go +++ b/workspace-server/internal/handlers/workspace_provision_test.go @@ -525,6 +525,128 @@ func TestSanitizeRuntime_Allowlist(t *testing.T) { } } +// ==================== seedInitialMemories: coverage for #1167 / #1208 ==================== + +// TestSeedInitialMemories_TruncatesOversizedContent covers the boundary cases for +// the CWE-400 content-length limit introduced in PR #1167. Issue #1208 identified +// that the truncate-at-100k guard lacked unit test coverage. +// The test verifies that content at and over the 100,000-byte limit is handled +// correctly, and that content under the limit passes through unchanged. +func TestSeedInitialMemories_TruncatesOversizedContent(t *testing.T) { + mock := setupTestDB(t) + + tests := []struct { + name string + contentLen int + expectInsert bool + expectTruncate bool + }{ + { + name: "exactly at 100 kB limit — no truncation", + contentLen: 100_000, + expectInsert: true, + }, + { + name: "1 byte over limit — truncated", + contentLen: 100_001, + expectInsert: true, + expectTruncate: true, + }, + { + name: "far over limit — truncated", + contentLen: 500_000, + expectInsert: true, + expectTruncate: true, + }, + { + name: "well under limit — passes through unchanged", + contentLen: 50_000, + expectInsert: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mock.ExpectExpectations() + workspaceID := "ws-trunc-" + tt.name + content := strings.Repeat("X", tt.contentLen) + memories := []models.MemorySeed{{Content: content, Scope: "LOCAL"}} + + if tt.expectInsert { + // The DB INSERT must receive content of exactly maxMemoryContentLength + // (not the full original length). This is the key assertion: the function + // truncates before calling ExecContext, so the mock expects 100_000 bytes. + mock.ExpectExec(`INSERT INTO agent_memories`). + WithArgs(workspaceID, strings.Repeat("X", maxMemoryContentLength), "LOCAL", sqlmock.AnyArg()). + WillReturnResult(sqlmock.NewResult(1, 1)) + } + + seedInitialMemories(context.Background(), workspaceID, memories, "test-ns") + + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet DB expectations: %v", err) + } + }) + } +} + +// TestSeedInitialMemories_RedactsSecrets verifies that redactSecrets is called +// before the INSERT so that credentials in template memories never land +// unredacted in agent_memories. Regression test for F1085 / #1132. +func TestSeedInitialMemories_RedactsSecrets(t *testing.T) { + mock := setupTestDB(t) + + raw := "Remember to set OPENAI_API_KEY=sk-abcdef123456 in the config file" + wantRedacted, changed := redactSecrets("ws-redact-test", raw) + if !changed { + t.Fatalf("precondition: redactSecrets must change the test content") + } + + workspaceID := "ws-redact-test" + memories := []models.MemorySeed{{Content: raw, Scope: "LOCAL"}} + + // The INSERT must receive the REDACTED content, not the raw secret. + mock.ExpectExec(`INSERT INTO agent_memories`). + WithArgs(workspaceID, wantRedacted, "LOCAL", sqlmock.AnyArg()). + WillReturnResult(sqlmock.NewResult(1, 1)) + + seedInitialMemories(context.Background(), workspaceID, memories, "test-ns") + + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet DB expectations: %v", err) + } +} + +// TestSeedInitialMemories_InvalidScopeSkipped verifies that entries with an +// unrecognized scope value are silently skipped (not inserted). +func TestSeedInitialMemories_InvalidScopeSkipped(t *testing.T) { + mock := setupTestDB(t) + mock.ExpectExpectations() // no DB calls expected for invalid scope + + memories := []models.MemorySeed{ + {Content: "this should be skipped", Scope: "NOT_A_REAL_SCOPE"}, + } + + seedInitialMemories(context.Background(), "ws-bad-scope", memories, "test-ns") + + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unexpected DB calls for invalid scope: %v", err) + } +} + +// TestSeedInitialMemories_EmptyMemoriesNil verifies that a nil memories slice +// is handled without error (no DB calls). +func TestSeedInitialMemories_EmptyMemoriesNil(t *testing.T) { + mock := setupTestDB(t) + mock.ExpectExpectations() + + seedInitialMemories(context.Background(), "ws-nil", nil, "test-ns") + + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unexpected DB calls for nil slice: %v", err) + } +} + // ==================== buildProvisionerConfig ==================== func TestBuildProvisionerConfig_BasicFields(t *testing.T) {