fix(ws-server): fail-closed on unresolvable template runtime (controlplane#188) #1489

Closed
hongming wants to merge 1 commits from fix/ws-server-188-failclosed-template-runtime into main
2 changed files with 179 additions and 1 deletions
@@ -198,6 +198,17 @@ func (h *WorkspaceHandler) Create(c *gin.Context) {
// back to its compiled-in Anthropic default and 401s when the user's
// key is for a different provider. Non-hermes runtimes are unaffected
// (the server still passes model through, they just don't use it).
// runtimeExplicitlyRequested is true when the caller expressed intent for
// a SPECIFIC runtime — either by passing `runtime` directly, or by naming
// a `template` (a template encodes a runtime). When true, we must NOT
// silently fall back to langgraph if that intent can't be honored: that
// is the molecule-controlplane#188 / #184 contract violation (caller asks
// for codex/claude-code, gets a langgraph workspace, 201, no error — a
// false success). #188 mandates fail-closed (error+notify) on mismatch,
// not an advisory degrade. The legitimate "no template, no runtime →
// langgraph default" path (bare {"name":...}) is unaffected.
runtimeExplicitlyRequested := payload.Runtime != "" || payload.Template != ""
templateRuntimeResolved := payload.Runtime != ""
if payload.Template != "" && (payload.Runtime == "" || payload.Model == "") {
// #226: payload.Template is attacker-controllable. resolveInsideRoot
// rejects absolute paths and any ".." that escapes configsDir so the
@@ -230,6 +241,9 @@ func (h *WorkspaceHandler) Create(c *gin.Context) {
switch {
case payload.Runtime == "" && !indented && strings.HasPrefix(stripped, "runtime:") && !strings.HasPrefix(stripped, "runtime_config"):
payload.Runtime = strings.TrimSpace(strings.TrimPrefix(stripped, "runtime:"))
if payload.Runtime != "" {
templateRuntimeResolved = true
}
case payload.Model == "" && !indented && strings.HasPrefix(stripped, "model:"):
// Legacy top-level `model:` — pre-runtime_config templates.
payload.Model = strings.Trim(strings.TrimSpace(strings.TrimPrefix(stripped, "model:")), `"'`)
@@ -242,7 +256,27 @@ func (h *WorkspaceHandler) Create(c *gin.Context) {
}
}
}
// Fail-closed (molecule-controlplane#188 / #184): if the caller expressed
// intent for a specific runtime (passed `runtime`, or named a `template`)
// but we could NOT resolve a concrete runtime from it (template's
// config.yaml unreadable, or it has no `runtime:` key), DO NOT silently
// substitute langgraph and return 201 — that is the silent contract
// violation that produced 5/5 wrong workspaces and a false codex E2E pass.
// Return 422 so the caller learns the requested runtime was not honored.
// The platform-side CP fix (controlplane#188) is the sibling gate; this
// closes the ws-server `Create` boundary the product UI actually hits.
if payload.Runtime == "" && runtimeExplicitlyRequested && !templateRuntimeResolved {
log.Printf("Create: FAIL-CLOSED (controlplane#188) — template=%q requested but runtime could not be resolved; refusing silent langgraph fallback", payload.Template)
c.JSON(http.StatusUnprocessableEntity, gin.H{
"error": "runtime could not be resolved from the requested template; refusing to silently provision langgraph (controlplane#188). Pass an explicit \"runtime\", or use a template whose config.yaml declares one.",
"template": payload.Template,
"code": "RUNTIME_UNRESOLVED",
})
return
}
if payload.Runtime == "" {
// Legitimate default path: no template AND no runtime requested
// (bare {"name":...}) — langgraph is the intended default here.
payload.Runtime = "langgraph"
}
@@ -718,7 +718,7 @@ func TestWorkspaceList_Empty(t *testing.T) {
"parent_id", "active_tasks", "last_error_rate", "last_sample_error",
"uptime_seconds", "current_task", "runtime", "workspace_dir", "x", "y", "collapsed",
"budget_limit", "monthly_spend",
"broadcast_enabled", "talk_to_user_enabled",
"broadcast_enabled", "talk_to_user_enabled",
}))
w := httptest.NewRecorder()
@@ -1770,3 +1770,147 @@ runtime_config:
t.Fatalf("expected 201, got %d: %s", w.Code, w.Body.String())
}
}
// ==================== #188 fail-closed: template/runtime contract ====================
//
// molecule-controlplane#188 / #184: if a caller names a `template` (intent
// for a specific runtime) but the runtime cannot be resolved from it, the
// server MUST NOT silently provision langgraph and return 201 — that false
// success produced 5/5 wrong workspaces and a bogus codex E2E pass. These
// tests pin the fail-closed boundary at the ws-server `Create` handler (the
// path the product UI hits), and guard the legitimate default path against
// regression.
// Template requested but its dir/config.yaml is absent → 422, not silent
// langgraph 201.
func TestWorkspaceCreate_188_TemplateMissingRuntime_FailsClosed(t *testing.T) {
setupTestDB(t)
setupTestRedis(t)
broadcaster := newTestBroadcaster()
// configsDir is an empty temp dir → resolveInsideRoot succeeds (the path
// is inside root) but config.yaml read fails → runtime cannot be resolved.
configsDir := t.TempDir()
if err := os.MkdirAll(filepath.Join(configsDir, "ghost-template"), 0o755); err != nil {
t.Fatalf("mkdir: %v", err)
}
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", configsDir)
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
body := `{"name":"Ghost","template":"ghost-template"}`
c.Request = httptest.NewRequest("POST", "/workspaces", bytes.NewBufferString(body))
c.Request.Header.Set("Content-Type", "application/json")
handler.Create(c)
if w.Code != http.StatusUnprocessableEntity {
t.Fatalf("expected 422 (fail-closed, controlplane#188), 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("parse: %v", err)
}
if resp["code"] != "RUNTIME_UNRESOLVED" {
t.Errorf("expected code RUNTIME_UNRESOLVED, got %v", resp["code"])
}
}
// Template config.yaml has no `runtime:` key → 422, not silent langgraph.
func TestWorkspaceCreate_188_TemplateConfigNoRuntimeKey_FailsClosed(t *testing.T) {
setupTestDB(t)
setupTestRedis(t)
broadcaster := newTestBroadcaster()
configsDir := t.TempDir()
tdir := filepath.Join(configsDir, "noruntime-template")
if err := os.MkdirAll(tdir, 0o755); err != nil {
t.Fatalf("mkdir: %v", err)
}
// config.yaml exists but declares no runtime.
if err := os.WriteFile(filepath.Join(tdir, "config.yaml"), []byte("name: noruntime\n"), 0o644); err != nil {
t.Fatalf("write: %v", err)
}
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", configsDir)
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
body := `{"name":"NoRuntime","template":"noruntime-template"}`
c.Request = httptest.NewRequest("POST", "/workspaces", bytes.NewBufferString(body))
c.Request.Header.Set("Content-Type", "application/json")
handler.Create(c)
if w.Code != http.StatusUnprocessableEntity {
t.Fatalf("expected 422 (fail-closed), got %d: %s", w.Code, w.Body.String())
}
}
// Regression guard: the legitimate default path (no template, no runtime —
// bare {"name":...}) MUST still default to langgraph and return 201. The
// #188 fix must not break this.
func TestWorkspaceCreate_188_NoTemplateNoRuntime_StillDefaultsLanggraph(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").
WithArgs(sqlmock.AnyArg(), "Plain Default", nil, 3, "langgraph", sqlmock.AnyArg(), (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push").
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))
mock.ExpectExec("INSERT INTO structure_events").
WillReturnResult(sqlmock.NewResult(0, 1))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
body := `{"name":"Plain Default"}`
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.Fatalf("expected 201 (legitimate default path), got %d: %s", w.Code, w.Body.String())
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
// Explicit runtime, no template → honored, 201 (no template resolution
// needed; runtimeExplicitlyRequested true but already resolved).
func TestWorkspaceCreate_188_ExplicitRuntimeNoTemplate_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").
WithArgs(sqlmock.AnyArg(), "Explicit Codex", nil, 3, "codex", sqlmock.AnyArg(), (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push").
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))
mock.ExpectExec("INSERT INTO structure_events").
WillReturnResult(sqlmock.NewResult(0, 1))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
body := `{"name":"Explicit Codex","runtime":"codex"}`
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.Fatalf("expected 201, got %d: %s", w.Code, w.Body.String())
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}