forked from molecule-ai/molecule-core
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>
This commit is contained in:
parent
30d8f0cf36
commit
d7901bb831
@ -81,10 +81,23 @@ func resolveRestartTemplate(configsDir, wsName, dbRuntime string, body restartTe
|
||||
// Use case: Canvas Config tab changed the runtime; we need the new
|
||||
// runtime's base files (entry point, Dockerfile, skill scaffolding)
|
||||
// because the existing volume was written by the old runtime.
|
||||
//
|
||||
// SECURITY (CWE-22 / F1502): dbRuntime comes from the workspaces DB
|
||||
// column — set by the PATCH Update handler which only validates length
|
||||
// and newlines, not path-traversal characters. Without sanitisation an
|
||||
// attacker who holds a workspace token could set runtime to
|
||||
// "../../../etc" and, if a directory matching that path existed on the
|
||||
// host, load an arbitrary host directory as the workspace template.
|
||||
//
|
||||
// sanitizeRuntime applies an allowlist of known runtimes; any unknown
|
||||
// value (including traversal strings) is remapped to "langgraph". The
|
||||
// attacker cannot choose an arbitrary host path — they can at most
|
||||
// trigger application of the langgraph-default template.
|
||||
if body.ApplyTemplate && dbRuntime != "" {
|
||||
runtimeTemplate := filepath.Join(configsDir, dbRuntime+"-default")
|
||||
safeRuntime := sanitizeRuntime(dbRuntime)
|
||||
runtimeTemplate := filepath.Join(configsDir, safeRuntime+"-default")
|
||||
if _, err := os.Stat(runtimeTemplate); err == nil {
|
||||
label := dbRuntime + "-default"
|
||||
label := safeRuntime + "-default"
|
||||
log.Printf("Restart: applying template %s (runtime change)", label)
|
||||
return runtimeTemplate, label
|
||||
}
|
||||
|
||||
@ -176,3 +176,68 @@ func TestResolveRestartTemplate_Priority_ExplicitBeatsApplyTemplate(t *testing.T
|
||||
t.Errorf("expected path %q, got %q", expected, path)
|
||||
}
|
||||
}
|
||||
|
||||
// TestResolveRestartTemplate_CWE22_TraversalRuntime_FallsThrough is the
|
||||
// regression test for CWE-22 in Tier 4 of resolveRestartTemplate.
|
||||
//
|
||||
// An attacker who holds a workspace token can set the runtime field to a
|
||||
// path-traversal string (e.g. "../../../etc"). Before the fix, the code
|
||||
// did:
|
||||
// runtimeTemplate := filepath.Join(configsDir, dbRuntime+"-default")
|
||||
// which on a host with /configs/../../../etc-default would return /etc-default,
|
||||
// injecting arbitrary host files into the workspace container.
|
||||
//
|
||||
// After the fix, sanitizeRuntime is called first. Unknown runtimes
|
||||
// (including traversal strings) are remapped to "langgraph". The attacker
|
||||
// cannot choose an arbitrary host path — they can at most trigger
|
||||
// langgraph-default if that template happens to exist.
|
||||
//
|
||||
// This test verifies that a traversal string in dbRuntime falls through to
|
||||
// "existing-volume" when no langgraph-default template is present.
|
||||
func TestResolveRestartTemplate_CWE22_TraversalRuntime_FallsThrough(t *testing.T) {
|
||||
root := newTemplateDir(t) // no template dirs at all
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
dbRuntime string
|
||||
}{
|
||||
{"simple traversal", "../../../etc"},
|
||||
{"mid-path traversal", "langgraph/../../../etc"},
|
||||
{"absolute-path attempt", "/etc/passwd"},
|
||||
{"double-dot chain", "../.."},
|
||||
{"deep traversal", "a/b/c/../../../d"},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
path, label := resolveRestartTemplate(root, "Some Workspace", tc.dbRuntime, restartTemplateInput{
|
||||
ApplyTemplate: true,
|
||||
})
|
||||
// Must NOT return a path that escapes root
|
||||
if path != "" {
|
||||
t.Errorf("CWE-22: traversal runtime %q must not resolve; got path=%q", tc.dbRuntime, path)
|
||||
}
|
||||
if label != "existing-volume" {
|
||||
t.Errorf("CWE-22: traversal runtime %q must fall through to existing-volume; got label=%q", tc.dbRuntime, label)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestResolveRestartTemplate_CWE22_TraversalRuntime_CannotOverrideKnownRuntime
|
||||
// verifies that even if a langgraph-default template exists, a traversal
|
||||
// string in dbRuntime resolves langgraph-default (the safe default) rather
|
||||
// than any attacker-chosen path. The attacker gains no additional access.
|
||||
func TestResolveRestartTemplate_CWE22_TraversalRuntime_CannotOverrideKnownRuntime(t *testing.T) {
|
||||
root := newTemplateDir(t, "langgraph-default")
|
||||
|
||||
path, label := resolveRestartTemplate(root, "Some Workspace", "../../../etc", restartTemplateInput{
|
||||
ApplyTemplate: true,
|
||||
})
|
||||
// Must resolve to langgraph-default, not to an escaped path
|
||||
expected := filepath.Join(root, "langgraph-default")
|
||||
if path != expected {
|
||||
t.Errorf("traversal runtime must resolve to langgraph-default; got path=%q", path)
|
||||
}
|
||||
if label != "langgraph-default" {
|
||||
t.Errorf("label must be langgraph-default; got %q", label)
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user