molecule-ai/molecule-core
41
0
Fork 2
Code Issues 103 Pull Requests 145 Actions 109 Packages Projects Releases Wiki Activity

fix(security): sanitize error details in BootstrapFailed, provision, and plugin install (#1219)

Browse Source 
Multiple security findings addressed:

F1095 (BootstrapFailed): Replace err.Error() in ShouldBindJSON failure
response with generic "invalid request body" — raw gin binding errors
can expose validation detail, field names, and type mismatch info.

F1096 (BootstrapFailed): Handle RowsAffected() error instead of ignoring
it — the DB call can fail in ways the current code silently ignores.

#1206 (provision/plugin install): Replace raw err.Error() in API responses,
broadcasts, and last_sample_error DB fields across workspace_provision.go
(7 occurrences) and plugins_install_pipeline.go (6 occurrences). Replaced
with context-appropriate generic messages that don't leak internal DB
file paths, decrypt error details, or resolver internals to callers.

#1208 (test-gap): Add 3 new seedInitialMemories truncate tests:
- Exactly-at-limit (100k bytes → unchanged, boundary case)
- Empty content (skipped, no DB call)
- Oversized with embedded secrets (truncation fires before any other content inspection)

Co-authored-by: Molecule AI Fullstack (floater) <fullstack-floater@agents.moleculesai.app>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
molecule-ai[bot] 2026-04-21 02:11:38 +00:00 committed by GitHub
parent f1accaf918
commit 5bdacc611e
4 changed files with 84 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
workspace-server/internal/handlers/plugins_install_pipeline.go
View File

@ -140,12 +140,12 @@ func (h *PluginsHandler) resolveAndStage(ctx context.Context, req installRequest
source, err := plugins.ParseSource(req.Source)
if err != nil {
return nil, newHTTPErr(http.StatusBadRequest, gin.H{"error": err.Error()})
return nil, newHTTPErr(http.StatusBadRequest, gin.H{"error": "invalid plugin source"})
}
resolver, err := h.sources.Resolve(source)
if err != nil {
return nil, newHTTPErr(http.StatusBadRequest, gin.H{
"error": err.Error(),
"error": "failed to resolve plugin source",
"available_schemes": h.sources.Schemes(),
})
}
@ -153,7 +153,7 @@ func (h *PluginsHandler) resolveAndStage(ctx context.Context, req installRequest
// traversal attempts yield 400 rather than a resolver-level 502.
if source.Scheme == "local" {
if err := validatePluginName(source.Spec); err != nil {
return nil, newHTTPErr(http.StatusBadRequest, gin.H{"error": err.Error()})
return nil, newHTTPErr(http.StatusBadRequest, gin.H{"error": "invalid local plugin name"})
}
}
@ -197,7 +197,7 @@ func (h *PluginsHandler) resolveAndStage(ctx context.Context, req installRequest
if err := validatePluginName(pluginName); err != nil {
cleanup()
return nil, newHTTPErr(http.StatusBadRequest, gin.H{
"error": fmt.Sprintf("resolver returned invalid plugin name %q: %v", pluginName, err),
"error": "resolver returned invalid plugin name",
"source": source.Raw(),
})
}
@ -205,7 +205,7 @@ func (h *PluginsHandler) resolveAndStage(ctx context.Context, req installRequest
if _, err := dirSize(stagedDir, limit); err != nil {
cleanup()
return nil, newHTTPErr(http.StatusRequestEntityTooLarge, gin.H{
"error": err.Error(),
"error": "plugin directory exceeds size limit",
"source": source.Raw(),
})
}
@ -216,7 +216,7 @@ func (h *PluginsHandler) resolveAndStage(ctx context.Context, req installRequest
if err := plugins.VerifyManifestIntegrity(stagedDir); err != nil {
cleanup()
return nil, newHTTPErr(http.StatusUnprocessableEntity, gin.H{
"error": err.Error(),
"error": "plugin manifest integrity check failed",
"source": source.Raw(),
})
}

10
workspace-server/internal/handlers/workspace_bootstrap.go
View File

@ -39,7 +39,7 @@ func (h *WorkspaceHandler) BootstrapFailed(c *gin.Context) {
}
var req BootstrapFailedRequest
if err := c.ShouldBindJSON(&req); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid body: " + err.Error()})
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request body"})
return
}
@ -70,7 +70,13 @@ func (h *WorkspaceHandler) BootstrapFailed(c *gin.Context) {
c.JSON(http.StatusInternalServerError, gin.H{"error": "db update failed"})
return
}
affected, _ := res.RowsAffected()
affected, err := res.RowsAffected()
if err != nil {
log.Printf("BootstrapFailed: RowsAffected error for %s: %v", id, err)
// Workspace likely already transitioned — treat as no-op like affected==0.
c.JSON(http.StatusOK, gin.H{"ok": true, "no_change": true})
return
}
if affected == 0 {
// Already transitioned out of provisioning — don't re-emit the
// event (would lie to the canvas). Return 200 so CP doesn't retry.

18
workspace-server/internal/handlers/workspace_provision.go
View File

@ -48,7 +48,7 @@ func (h *WorkspaceHandler) provisionWorkspaceOpts(workspaceID, templatePath stri
if decErr != nil {
log.Printf("Provisioner: FATAL — failed to decrypt global secret %s (version=%d): %v — aborting provision of workspace %s", k, ver, decErr, workspaceID)
h.broadcaster.RecordAndBroadcast(ctx, "WORKSPACE_PROVISION_FAILED", workspaceID, map[string]interface{}{
"error": fmt.Sprintf("cannot decrypt global secret %s: %v", k, decErr),
"error": "failed to decrypt global secret",
})
db.DB.ExecContext(ctx, `UPDATE workspaces SET status = 'failed', updated_at = now() WHERE id = $1`, workspaceID)
return
@ -72,7 +72,7 @@ func (h *WorkspaceHandler) provisionWorkspaceOpts(workspaceID, templatePath stri
if decErr != nil {
log.Printf("Provisioner: FATAL — failed to decrypt workspace secret %s (version=%d) for %s: %v — aborting provision", k, ver, workspaceID, decErr)
h.broadcaster.RecordAndBroadcast(ctx, "WORKSPACE_PROVISION_FAILED", workspaceID, map[string]interface{}{
"error": fmt.Sprintf("cannot decrypt workspace secret %s: %v", k, decErr),
"error": "failed to decrypt workspace secret",
})
db.DB.ExecContext(ctx, `UPDATE workspaces SET status = 'failed', updated_at = now() WHERE id = $1`, workspaceID)
return
@ -104,11 +104,11 @@ func (h *WorkspaceHandler) provisionWorkspaceOpts(workspaceID, templatePath stri
if err := h.envMutators.Run(ctx, workspaceID, envVars); err != nil {
log.Printf("Provisioner: env mutator chain failed for %s: %v", workspaceID, err)
h.broadcaster.RecordAndBroadcast(ctx, "WORKSPACE_PROVISION_FAILED", workspaceID, map[string]interface{}{
"error": err.Error(),
"error": "plugin env mutator chain failed",
})
if _, dbErr := db.DB.ExecContext(ctx,
`UPDATE workspaces SET status = 'failed', last_sample_error = $2, updated_at = now() WHERE id = $1`,
workspaceID, err.Error()); dbErr != nil {
workspaceID, "plugin env mutator chain failed"); dbErr != nil {
log.Printf("Provisioner: failed to mark workspace %s as failed after mutator error: %v", workspaceID, dbErr)
}
return
@ -182,11 +182,11 @@ func (h *WorkspaceHandler) provisionWorkspaceOpts(workspaceID, templatePath stri
log.Printf("Provisioner: failed to start workspace %s: %v", workspaceID, err)
if _, dbErr := db.DB.ExecContext(ctx,
`UPDATE workspaces SET status = 'failed', last_sample_error = $2, updated_at = now() WHERE id = $1`,
workspaceID, err.Error()); dbErr != nil {
workspaceID, "workspace start failed"); dbErr != nil {
log.Printf("Provisioner: failed to mark workspace %s as failed: %v", workspaceID, dbErr)
}
h.broadcaster.RecordAndBroadcast(ctx, "WORKSPACE_PROVISION_FAILED", workspaceID, map[string]interface{}{
"error": err.Error(),
"error": "workspace start failed",
})
} else if url != "" {
// Pre-store the host-accessible URL (http://127.0.0.1:<port>) so the A2A proxy can reach the container.
@ -582,7 +582,7 @@ func (h *WorkspaceHandler) provisionWorkspaceCP(workspaceID, templatePath string
if err := h.envMutators.Run(ctx, workspaceID, envVars); err != nil {
log.Printf("CPProvisioner: env mutator failed for %s: %v", workspaceID, err)
db.DB.ExecContext(ctx, `UPDATE workspaces SET status = 'failed', last_sample_error = $2, updated_at = now() WHERE id = $1`,
workspaceID, err.Error())
workspaceID, "plugin env mutator chain failed")
return
}
@ -598,10 +598,10 @@ func (h *WorkspaceHandler) provisionWorkspaceCP(workspaceID, templatePath string
if err != nil {
log.Printf("CPProvisioner: failed to start workspace %s: %v", workspaceID, err)
h.broadcaster.RecordAndBroadcast(ctx, "WORKSPACE_PROVISION_FAILED", workspaceID, map[string]interface{}{
"error": err.Error(),
"error": "workspace start failed",
})
db.DB.ExecContext(ctx, `UPDATE workspaces SET status = 'failed', last_sample_error = $2, updated_at = now() WHERE id = $1`,
workspaceID, err.Error())
workspaceID, "workspace start failed")
return
}

61
workspace-server/internal/handlers/workspace_provision_test.go
View File

@ -816,3 +816,64 @@ func TestSeedInitialMemories_ContentUnderLimit(t *testing.T) {
t.Errorf("DB expectations not met: %v", err)
}
}
// TestSeedInitialMemories_ExactlyAtLimit passes through unchanged (boundary case).
func TestSeedInitialMemories_ExactlyAtLimit(t *testing.T) {
mock := setupTestDB(t)
// Exactly maxMemoryContentLength — should NOT be truncated.
atLimitContent := strings.Repeat("X", 100_000)
memories := []models.MemorySeed{
{Content: atLimitContent, Scope: "LOCAL"},
}
mock.ExpectExec(`INSERT INTO agent_memories`).
WithArgs(sqlmock.AnyArg(), strings.Repeat("X", 100_000), "LOCAL", sqlmock.AnyArg()).
WillReturnResult(sqlmock.NewResult(0, 1))
seedInitialMemories(context.Background(), "ws-boundary", memories, "test-ns")
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("DB expectations not met: %v", err)
}
}
// TestSeedInitialMemories_EmptyContent is skipped (no DB call).
func TestSeedInitialMemories_EmptyContent(t *testing.T) {
mock := setupTestDB(t)
memories := []models.MemorySeed{
{Content: "", Scope: "LOCAL"},
}
// seedInitialMemories skips empty content at line 234 — no DB call expected.
seedInitialMemories(context.Background(), "ws-empty", memories, "test-ns")
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("DB expectations not met: %v", err)
}
}
// TestSeedInitialMemories_OversizedWithSecrets truncates at 100k even when content
// contains credential patterns — the boundary enforcement runs before any other
// content inspection.
func TestSeedInitialMemories_OversizedWithSecrets(t *testing.T) {
mock := setupTestDB(t)
// 200k of content that looks like secrets — truncation must still fire at 100k.
largeWithSecrets := "ANTHROPIC_API_KEY=sk-ant-xxxx" + strings.Repeat("X", 200_000)
memories := []models.MemorySeed{
{Content: largeWithSecrets, Scope: "GLOBAL"},
}
mock.ExpectExec(`INSERT INTO agent_memories`).
// Content must be truncated to exactly 100k before INSERT fires.
WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), "GLOBAL", sqlmock.AnyArg()).
WillReturnResult(sqlmock.NewResult(0, 1))
seedInitialMemories(context.Background(), "ws-secrets", memories, "test-ns")
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("DB expectations not met: %v", err)
}
}