4 Commits

Author	SHA1	Message	Date
claude-ceo-assistant	f7e2976324	chore: retire unmaintained workspace runtimes	2026-05-23 23:45:09 -07:00
hongming-codex-laptop	ad7acd30db	fix(platform): clear golangci-lint findings	2026-05-12 22:53:22 -07:00
core-offsec	d7901bb831	fix(handlers): apply sanitizeRuntime allowlist before Tier 4 filepath.Join (CWE-22) ... CWE-22 path traversal in restartTemplateInput Tier 4: dbRuntime was joined directly into the template path without sanitisation. runtimeTemplate := filepath.Join(configsDir, dbRuntime+"-default") An attacker holding a workspace token could set runtime to a path-traversal string (e.g. "../../../etc") via the PATCH /workspaces/:id Update handler, which only validates length and newlines. If a matching directory existed on the host (e.g. /configs/../../../etc-default), the restart would load files from an arbitrary host path into the workspace container. Fix: call sanitizeRuntime(dbRuntime) — the existing allowlist in workspace_provision.go — before filepath.Join. Unknown values are remapped to "langgraph", so the attacker cannot choose an arbitrary host path. Defense-in-depth: the path is still inside configsDir after sanitisation. Regression tests added: - CWE-22 traversal strings fall through to existing-volume - langgraph-default is used when traversal string is sanitised to langgraph Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-04-24 11:37:19 +00:00
hongming-personal	ba03fcfe2d	fix(restart): preserve user config volume on default restart (#1822 drift-risk-3) ... ### Repro On Canvas: create a workspace named "Hermes Agent" (runtime=langgraph, model=langgraph default). Open the Config tab, switch the model to a Minimax provider + Minimax token, hit Save and Restart. The model reverts to the default on every restart. ### Root cause `workspace_restart.go` called `findTemplateByName(configsDir, wsName)` unconditionally when the request body had no explicit `template`: template := body.Template if template == "" { template = findTemplateByName(h.configsDir, wsName) } `findTemplateByName` normalises the name ("Hermes Agent" → "hermes-agent") and ALSO scans every template's `config.yaml` for a matching `name:` field — a two-layer match that returns non-empty for any workspace whose name coincides with a template dir OR any template whose config.yaml claims the same display name. When the match returned non-empty, the restart handler set `templatePath = <template>` and the provisioner rewrote the workspace's config volume from the template on `Start`. The Canvas Save+Restart flow's `PUT /workspaces/:id/files/config.yaml` had already written the user's edits to the volume — those got clobbered. The comment immediately below (line 187) ALREADY said: // Apply runtime-default template ONLY when explicitly requested // via "apply_template": true. Use case: runtime was changed via // Config tab — need new runtime's base files. Normal restarts // preserve existing config volume (user's model, skills, prompts). The code contradicted the comment. The design intent was right; the implementation short-circuited it. Matches drift-risk #3 in #1822's Docker-vs-EC2 parity tracker ("Config-tab save must flush to DB before kicking off restart, not deferred"). ### Fix Extracted the template-resolution chain into a pure function `resolveRestartTemplate(configsDir, wsName, dbRuntime, body)` in a new `restart_template.go`. Gated the name-based auto-match on `body.ApplyTemplate`: 1. Explicit `body.Template` → always honoured (caller consent). 2. `ApplyTemplate=true` → name-based auto-match (prior behaviour). 3. `RebuildConfig=true` → org-templates recovery fallback (#239). 4. `ApplyTemplate=true` + dbRuntime → `<runtime>-default/`. 5. Fall through → empty path + "existing-volume" label. Provisioner reuses the volume. This is the path Canvas Save+Restart now hits. The handler now calls this helper and uses the returned path directly. Duplicate rebuild_config blocks at lines 167-186 were consolidated into the helper's single tier-3 case in passing. ### Abstraction win `resolveRestartTemplate` is a pure function — no gin context, no DB, no network. Takes a struct input, returns two strings. The whole priority chain is unit-testable in a temp dir, which is exactly what `restart_template_test.go` does. ### Tests `restart_template_test.go` — 8 table-style unit tests covering every branch of the priority chain: - DefaultRestart_PreservesVolume — the regression. Even when a template's config.yaml `name:` field matches the workspace name exactly (worst case), a default restart MUST return empty path. - ExplicitTemplate_AlwaysHonoured — caller-by-name, any mode. - ApplyTemplate_NameMatch — opt-in restores the auto-match. - ApplyTemplate_RuntimeDefault — runtime-change flow still works. - ApplyTemplate_NoMatch_NoRuntime — fallback to existing-volume. - InvalidExplicitTemplate_ProceedsWithout — traversal attempt stays inside root, falls through cleanly. - NonExistentExplicitTemplate — deleted/missing template falls through. - Priority_ExplicitBeatsApplyTemplate — explicit Template wins over name-match when both fire. Full handlers race suite (`go test -race ./internal/handlers/`) still passes — existing Restart-handler tests unchanged. ### Blast radius Any restart caller that omitted `apply_template: true` and relied on name-matching auto-applying a template is now a behaviour change. Identified call sites in this repo: - Canvas Save+Restart button (store/canvas.ts) — explicitly the flow this commit fixes, definitely wanted the fix. - Canvas Restart button (same file) — same semantics; user expects a restart, not a template reset. - Auto-restart sweeper (#1858) — never passes apply_template and depends on the existing volume having valid config. Separately, `workspace_provision.go`'s #1858 recovery path detects empty volumes and auto-applies `<runtime>-default` without going through findTemplateByName, so recovery is unaffected. - RestartByID — internal callers; audited, all intended "restart as-is", none relied on auto-template-match. No SaaS parity impact — this is a handler behaviour fix that applies equally to Docker and EC2 backends (both use the same Restart handler before dispatching to their respective provisioners). Refs #1822 drift-risk-3. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-04-23 13:57:42 -07:00