fix(platform): persist secrets envelope from POST /workspaces payload (#568)
`CreateWorkspacePayload` was missing a `Secrets` field, so any
`secrets: { KEY: value }` included in a POST /workspaces body was
silently dropped by ShouldBindJSON.
Changes:
- Add `Secrets map[string]string` field to `CreateWorkspacePayload`
- Wrap workspace INSERT in a DB transaction; iterate over secrets,
encrypt each value via `crypto.Encrypt`, and upsert into
`workspace_secrets` within the same tx — rollback both on any failure
- Add `mock.ExpectBegin()`/`mock.ExpectCommit()`/`mock.ExpectRollback()`
to all existing Create tests that were missing transaction expectations
- Add 3 new tests: WithSecrets_Persists, SecretPersistFails_RollsBack,
EmptySecrets_OK
Closes #545
Co-authored-by: Molecule AI Backend Engineer <backend-engineer@agents.moleculesai.app>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
38b72e149d
commit
a73de2451c
@ -28,9 +28,11 @@ func TestWorkspaceCreate_WithParentID(t *testing.T) {
|
||||
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
|
||||
|
||||
parentID := "parent-ws-123"
|
||||
mock.ExpectBegin()
|
||||
mock.ExpectExec("INSERT INTO workspaces").
|
||||
WithArgs(sqlmock.AnyArg(), "Child Agent", nil, 1, "langgraph", sqlmock.AnyArg(), &parentID, nil, "none").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
mock.ExpectCommit()
|
||||
mock.ExpectExec("INSERT INTO canvas_layouts").
|
||||
WithArgs(sqlmock.AnyArg(), float64(0), float64(0)).
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
@ -61,9 +63,11 @@ func TestWorkspaceCreate_ExplicitClaudeCodeRuntime(t *testing.T) {
|
||||
broadcaster := newTestBroadcaster()
|
||||
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
|
||||
|
||||
mock.ExpectBegin()
|
||||
mock.ExpectExec("INSERT INTO workspaces").
|
||||
WithArgs(sqlmock.AnyArg(), "CC Agent", nil, 2, "claude-code", sqlmock.AnyArg(), (*string)(nil), nil, "none").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
mock.ExpectCommit()
|
||||
mock.ExpectExec("INSERT INTO canvas_layouts").
|
||||
WithArgs(sqlmock.AnyArg(), float64(10), float64(20)).
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
@ -248,11 +248,17 @@ func TestWorkspaceCreate(t *testing.T) {
|
||||
broadcaster := newTestBroadcaster()
|
||||
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", "/tmp/configs")
|
||||
|
||||
// Expect transaction begin for atomic workspace+secrets creation
|
||||
mock.ExpectBegin()
|
||||
|
||||
// Expect workspace INSERT (uuid is dynamic, use AnyArg for id, runtime, awareness_namespace)
|
||||
mock.ExpectExec("INSERT INTO workspaces").
|
||||
WithArgs(sqlmock.AnyArg(), "Test Agent", nil, 1, "langgraph", sqlmock.AnyArg(), (*string)(nil), nil, "none").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
// Expect transaction commit (no secrets in this payload)
|
||||
mock.ExpectCommit()
|
||||
|
||||
// Expect canvas_layouts INSERT
|
||||
mock.ExpectExec("INSERT INTO canvas_layouts").
|
||||
WithArgs(sqlmock.AnyArg(), float64(100), float64(200)).
|
||||
|
||||
@ -10,6 +10,7 @@ import (
|
||||
"path/filepath"
|
||||
"strings"
|
||||
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/crypto"
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/db"
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/events"
|
||||
"github.com/Molecule-AI/molecule-monorepo/platform/internal/middleware"
|
||||
@ -129,17 +130,59 @@ func (h *WorkspaceHandler) Create(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
// Insert workspace with runtime persisted in DB
|
||||
_, err := db.DB.ExecContext(ctx, `
|
||||
// Begin a transaction so the workspace row and any initial secrets are
|
||||
// committed atomically. A secret-encrypt or DB error rolls back the
|
||||
// workspace insert so we never leave a workspace row with missing secrets.
|
||||
tx, txErr := db.DB.BeginTx(ctx, nil)
|
||||
if txErr != nil {
|
||||
log.Printf("Create workspace: begin tx error: %v", txErr)
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create workspace"})
|
||||
return
|
||||
}
|
||||
|
||||
// Insert workspace with runtime persisted in DB (inside transaction)
|
||||
_, err := tx.ExecContext(ctx, `
|
||||
INSERT INTO workspaces (id, name, role, tier, runtime, awareness_namespace, status, parent_id, workspace_dir, workspace_access)
|
||||
VALUES ($1, $2, $3, $4, $5, $6, 'provisioning', $7, $8, $9)
|
||||
`, id, payload.Name, role, payload.Tier, payload.Runtime, awarenessNamespace, payload.ParentID, workspaceDir, workspaceAccess)
|
||||
if err != nil {
|
||||
tx.Rollback() //nolint:errcheck
|
||||
log.Printf("Create workspace error: %v", err)
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create workspace"})
|
||||
return
|
||||
}
|
||||
|
||||
// Persist initial secrets from the create payload (inside same transaction).
|
||||
// nil/empty map is a no-op. Any failure rolls back the workspace insert
|
||||
// so we never have a workspace row without its intended secrets.
|
||||
for k, v := range payload.Secrets {
|
||||
encrypted, encErr := crypto.Encrypt([]byte(v))
|
||||
if encErr != nil {
|
||||
tx.Rollback() //nolint:errcheck
|
||||
log.Printf("Create workspace %s: failed to encrypt secret %q: %v", id, k, encErr)
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to encrypt secret: " + k})
|
||||
return
|
||||
}
|
||||
version := crypto.CurrentEncryptionVersion()
|
||||
if _, dbErr := tx.ExecContext(ctx, `
|
||||
INSERT INTO workspace_secrets (workspace_id, key, encrypted_value, encryption_version)
|
||||
VALUES ($1, $2, $3, $4)
|
||||
ON CONFLICT (workspace_id, key) DO UPDATE
|
||||
SET encrypted_value = $3, encryption_version = $4, updated_at = now()
|
||||
`, id, k, encrypted, version); dbErr != nil {
|
||||
tx.Rollback() //nolint:errcheck
|
||||
log.Printf("Create workspace %s: failed to persist secret %q: %v", id, k, dbErr)
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to save secret: " + k})
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
if commitErr := tx.Commit(); commitErr != nil {
|
||||
log.Printf("Create workspace %s: transaction commit failed: %v", id, commitErr)
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to create workspace"})
|
||||
return
|
||||
}
|
||||
|
||||
// Insert canvas layout — non-fatal: workspace can be dragged into position later
|
||||
if _, err := db.DB.ExecContext(ctx, `
|
||||
INSERT INTO canvas_layouts (workspace_id, x, y) VALUES ($1, $2, $3)
|
||||
|
||||
@ -146,10 +146,12 @@ func TestWorkspaceCreate_DBInsertError(t *testing.T) {
|
||||
broadcaster := newTestBroadcaster()
|
||||
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
|
||||
|
||||
// Workspace INSERT fails
|
||||
// Transaction begins, workspace INSERT fails, transaction is rolled back.
|
||||
mock.ExpectBegin()
|
||||
mock.ExpectExec("INSERT INTO workspaces").
|
||||
WithArgs(sqlmock.AnyArg(), "Failing Agent", nil, 1, "langgraph", sqlmock.AnyArg(), (*string)(nil), nil, "none").
|
||||
WillReturnError(sql.ErrConnDone)
|
||||
mock.ExpectRollback()
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
@ -175,10 +177,13 @@ func TestWorkspaceCreate_DefaultsApplied(t *testing.T) {
|
||||
broadcaster := newTestBroadcaster()
|
||||
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
|
||||
|
||||
// Transaction wraps the workspace INSERT (no secrets in this request).
|
||||
mock.ExpectBegin()
|
||||
// Expect workspace INSERT with defaulted tier=1, runtime="langgraph"
|
||||
mock.ExpectExec("INSERT INTO workspaces").
|
||||
WithArgs(sqlmock.AnyArg(), "Default Agent", nil, 1, "langgraph", sqlmock.AnyArg(), (*string)(nil), nil, "none").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
mock.ExpectCommit()
|
||||
|
||||
// Expect canvas_layouts INSERT (x=0, y=0 — defaults)
|
||||
mock.ExpectExec("INSERT INTO canvas_layouts").
|
||||
@ -215,6 +220,117 @@ func TestWorkspaceCreate_DefaultsApplied(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestWorkspaceCreate_WithSecrets_Persists asserts that secrets in the create
|
||||
// payload are written to workspace_secrets inside the same transaction as the
|
||||
// workspace row, and that the handler returns 201.
|
||||
func TestWorkspaceCreate_WithSecrets_Persists(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
broadcaster := newTestBroadcaster()
|
||||
// External workspace: simplest code path — no provisioner goroutine.
|
||||
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
|
||||
|
||||
mock.ExpectBegin()
|
||||
mock.ExpectExec("INSERT INTO workspaces").
|
||||
WithArgs(sqlmock.AnyArg(), "Hermes Agent", nil, 1, "hermes", sqlmock.AnyArg(), (*string)(nil), nil, "none").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
// Secret inserted inside the same transaction.
|
||||
mock.ExpectExec("INSERT INTO workspace_secrets").
|
||||
WithArgs(sqlmock.AnyArg(), "HERMES_API_KEY", sqlmock.AnyArg(), sqlmock.AnyArg()).
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
mock.ExpectCommit()
|
||||
|
||||
// canvas_layouts (non-fatal, outside tx)
|
||||
mock.ExpectExec("INSERT INTO canvas_layouts").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
|
||||
body := `{"name":"Hermes Agent","runtime":"hermes","external":true,"secrets":{"HERMES_API_KEY":"sk-test-123"}}`
|
||||
c.Request = httptest.NewRequest("POST", "/workspaces", bytes.NewBufferString(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.Create(c)
|
||||
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Errorf("expected status 201, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet sqlmock expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestWorkspaceCreate_SecretPersistFails_RollsBack asserts that a DB error
|
||||
// while persisting a secret causes the entire transaction to roll back and
|
||||
// the handler to return 500. The workspace row must NOT be committed.
|
||||
func TestWorkspaceCreate_SecretPersistFails_RollsBack(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
broadcaster := newTestBroadcaster()
|
||||
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
|
||||
|
||||
mock.ExpectBegin()
|
||||
mock.ExpectExec("INSERT INTO workspaces").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
mock.ExpectExec("INSERT INTO workspace_secrets").
|
||||
WillReturnError(sql.ErrConnDone) // DB failure while writing secret
|
||||
mock.ExpectRollback() // workspace insert must be rolled back
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
|
||||
body := `{"name":"Rollback Agent","secrets":{"OPENAI_API_KEY":"sk-fail"}}`
|
||||
c.Request = httptest.NewRequest("POST", "/workspaces", bytes.NewBufferString(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.Create(c)
|
||||
|
||||
if w.Code != http.StatusInternalServerError {
|
||||
t.Errorf("expected status 500, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet sqlmock expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestWorkspaceCreate_EmptySecrets_OK asserts that an empty secrets map (or
|
||||
// no secrets key at all) creates the workspace normally without touching
|
||||
// workspace_secrets.
|
||||
func TestWorkspaceCreate_EmptySecrets_OK(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
broadcaster := newTestBroadcaster()
|
||||
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
|
||||
|
||||
mock.ExpectBegin()
|
||||
mock.ExpectExec("INSERT INTO workspaces").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
// No ExpectExec for workspace_secrets — empty map must be a no-op.
|
||||
mock.ExpectCommit()
|
||||
mock.ExpectExec("INSERT INTO canvas_layouts").
|
||||
WillReturnResult(sqlmock.NewResult(0, 1))
|
||||
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
|
||||
body := `{"name":"No Secrets Agent","external":true,"secrets":{}}`
|
||||
c.Request = httptest.NewRequest("POST", "/workspaces", bytes.NewBufferString(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.Create(c)
|
||||
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Errorf("expected status 201, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet sqlmock expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// ==================== GET /workspaces (List) ====================
|
||||
|
||||
func TestWorkspaceList_Empty(t *testing.T) {
|
||||
|
||||
@ -63,6 +63,10 @@ type CreateWorkspacePayload struct {
|
||||
WorkspaceDir string `json:"workspace_dir"` // host path to mount as /workspace (empty = isolated volume)
|
||||
WorkspaceAccess string `json:"workspace_access"` // "none" (default), "read_only", or "read_write" — see #65
|
||||
ParentID *string `json:"parent_id"`
|
||||
// Secrets is an optional map of key→plaintext-value pairs to persist as
|
||||
// workspace secrets at creation time. Stored encrypted (same path as
|
||||
// POST /workspaces/:id/secrets). Nil/empty map is a no-op.
|
||||
Secrets map[string]string `json:"secrets"`
|
||||
Canvas struct {
|
||||
X float64 `json:"x"`
|
||||
Y float64 `json:"y"`
|
||||
|
||||
Loading…
Reference in New Issue
Block a user