security: confine sandbox volume mounts to working directory

- Add sanitizeVolumeMount in internal/sandbox/sandbox.go.
- Resolve host paths to absolute, reject .. components and symlinks.
- Require extra volume host paths to be inside the working directory.
- Expand ForbiddenMountPrefixes with /var, /run, /root, /home, and
  /var/run/docker.sock.
- Rewrite mounts with canonical absolute host paths so Docker does not
  interpret relative paths relative to the daemon's cwd.
- Update affected tests in cmd/odek/main_test.go and internal/sandbox tests.
- Document the sandbox_volumes restriction and fix examples in
  docs/SANDBOXING.md.

Full suite: go test ./... -count=1 passes.

Author: jkyberneees

cmd/odek/main_test.go

@@ -1151,7 +1151,7 @@ func TestBuildSandboxArgs_EnvAndVolumes(t *testing.T) {
 			"GOCACHE":  "/tmp/gocache",
 			"NODE_ENV": "test",
 		},
-		Volumes: []string{"/host/cache:/container/cache", "/host/data:/data:ro"},
+		Volumes: []string{"/tmp/workdir/cache:/container/cache", "/tmp/workdir/data:/data:ro"},
 	}
 	args := sandbox.BuildRunArgs(cfg, "odek-test", "/tmp/workdir", cfg.Image)
 
@@ -1163,12 +1163,13 @@ func TestBuildSandboxArgs_EnvAndVolumes(t *testing.T) {
 		t.Error("missing env var NODE_ENV=test in docker args")
 	}
 
-	// Must contain volume mounts as "-v HOST:CONTAINER" pairs
-	if !hasArgPair(args, "-v", "/host/cache:/container/cache") {
-		t.Error("missing volume /host/cache:/container/cache in docker args")
+	// Must contain volume mounts as "-v HOST:CONTAINER" pairs.
+	// With the security fix, extra volume host paths must stay inside workdir.
+	if !hasArgPair(args, "-v", "/tmp/workdir/cache:/container/cache") {
+		t.Error("missing volume /tmp/workdir/cache:/container/cache in docker args")
 	}
-	if !hasArgPair(args, "-v", "/host/data:/data:ro") {
-		t.Error("missing volume /host/data:/data:ro in docker args")
+	if !hasArgPair(args, "-v", "/tmp/workdir/data:/data:ro") {
+		t.Error("missing volume /tmp/workdir/data:/data:ro in docker args")
 	}
 }
 
@@ -1724,7 +1725,7 @@ func TestBuildSandboxArgs_WithResources(t *testing.T) {
 		CPUs:    "0.5",
 		User:    "1000:1000",
 		Env:     map[string]string{"FOO": "bar"},
-		Volumes: []string{"/data:/data"},
+		Volumes: []string{"/workspace/data:/data"},
 	}, "odek-test", "/workspace", "alpine:latest")
 	full := strings.Join(args, " ")
 	if !strings.Contains(full, "--memory") || !strings.Contains(full, "512m") {
@@ -1739,7 +1740,7 @@ func TestBuildSandboxArgs_WithResources(t *testing.T) {
 	if !strings.Contains(full, "FOO=bar") {
 		t.Error("should include env var")
 	}
-	if !strings.Contains(full, "/data:/data") {
+	if !strings.Contains(full, "/workspace/data:/data") {
 		t.Error("should include extra volume")
 	}
 }
@@ -1868,15 +1869,16 @@ func TestBuildSandboxArgs_AllForbiddenPrefixes(t *testing.T) {
 	}
 }
 
-// TestBuildSandboxArgs_ValidVolume verifies a non-forbidden volume IS included.
+// TestBuildSandboxArgs_ValidVolume verifies a non-forbidden volume under the
+// working directory IS included.
 func TestBuildSandboxArgs_ValidVolume(t *testing.T) {
 	cfg := sandboxConfig{
 		Network: "bridge",
-		Volumes: []string{"/data:/data"},
+		Volumes: []string{"/workspace/data:/data"},
 	}
 	args := sandbox.BuildRunArgs(cfg, "odek-test", "/workspace", "alpine:latest")
-	if !hasArgPair(args, "-v", "/data:/data") {
-		t.Error("valid volume /data:/data should be included in docker args")
+	if !hasArgPair(args, "-v", "/workspace/data:/data") {
+		t.Error("valid volume /workspace/data:/data should be included in docker args")
 	}
 }
 

docs/SANDBOXING.md

@@ -41,7 +41,7 @@ All sandbox settings are available in `~/.odek/config.json`, `./odek.json`, `ODE
     "NODE_ENV": "development"
   },
   "sandbox_volumes": [
-    "/home/user/.npm:/root/.npm"
+    "./.npm:/root/.npm"
   ]
 }
 ```
@@ -61,6 +61,12 @@ All sandbox settings are available in `~/.odek/config.json`, `./odek.json`, `ODE
 | `sandbox_volumes` | — | — | array | `[]` | Extra volume mounts (`host:container`) |
 
 > **Note:** `sandbox_env` and `sandbox_volumes` are config-file-only — they're too complex for flat env vars or CLI flags. For all other fields, env vars and CLI flags follow the standard `ODEK_*` pattern.
+>
+> **Security restriction on `sandbox_volumes`:** Extra volume host paths must be
+> inside the working directory. Absolute paths outside the project (e.g.
+> `/var/run/docker.sock`, `/etc`, `/home/user/...`) and paths containing `..`
+> or symlinks are rejected. Relative paths are resolved relative to the working
+> directory and must stay inside it.
 
 ### Env var examples
 
@@ -232,7 +238,7 @@ odek's sandbox follows the principle of **least privilege with progressive opt-i
     "NPM_CONFIG_CACHE": "/tmp/.npm"
   },
   "sandbox_volumes": [
-    "/root/.npm:/root/.npm"
+    "./.npm:/root/.npm"
   ]
 }
 ```

internal/sandbox/sandbox.go

@@ -29,7 +29,11 @@ const DockerfileName = "Dockerfile.odek"
 // touch. A user could accidentally (or, in --task scenarios, be coaxed
 // into) requesting one of these and undo the whole point of sandboxing.
 // Mounts to these paths are dropped with a stderr warning.
-var ForbiddenMountPrefixes = []string{"/", "/etc", "/proc", "/sys", "/boot", "/dev"}
+var ForbiddenMountPrefixes = []string{
+	"/", "/etc", "/proc", "/sys", "/boot", "/dev",
+	"/var", "/run", "/root", "/home",
+	"/var/run/docker.sock",
+}
 
 // Config is the resolved sandbox configuration for one agent run. All
 // fields come from the merged config (files → env → CLI) — this struct
@@ -135,27 +139,122 @@ func BuildRunArgs(cfg Config, containerName, workdir, image string) []string {
 	}
 
 	for _, vol := range cfg.Volumes {
-		reject := false
-		parts := strings.SplitN(vol, ":", 2)
-		if len(parts) > 0 {
-			hostPath := filepath.Clean(parts[0])
-			for _, forbidden := range ForbiddenMountPrefixes {
-				if hostPath == forbidden || strings.HasPrefix(hostPath, forbidden+"/") {
-					fmt.Fprintf(os.Stderr, "odek: WARNING: rejecting forbidden volume mount %q (host path %s)\n", vol, hostPath)
-					reject = true
-					break
-				}
-			}
-		}
-		if !reject {
-			args = append(args, "-v", vol)
+		sanitized, ok := sanitizeVolumeMount(vol, workdir)
+		if !ok {
+			continue
 		}
+		args = append(args, "-v", sanitized)
 	}
 
 	args = append(args, image, "sleep", "infinity")
 	return args
 }
 
+// sanitizeVolumeMount validates and canonicalises a user-supplied docker -v
+// string. It returns the canonical mount string and true if the mount is
+// allowed, or an empty string and false if it should be dropped.
+//
+// Security rules:
+//   - The host path must be absolute or a local path under workdir.
+//   - Any ".." component is rejected.
+//   - The resolved host path must be inside workdir.
+//   - The resolved host path must not match ForbiddenMountPrefixes.
+//   - Symlinks are rejected (they could point outside workdir).
+//
+// The returned string uses the resolved absolute host path so Docker does not
+// interpret a relative path relative to the daemon's working directory.
+func sanitizeVolumeMount(vol, workdir string) (string, bool) {
+	// Docker volume format: host[:container[:options]]. We only need the host.
+	parts := strings.SplitN(vol, ":", 2)
+	host := strings.TrimSpace(parts[0])
+	if host == "" {
+		fmt.Fprintf(os.Stderr, "odek: WARNING: rejecting malformed volume mount %q (empty host path)\n", vol)
+		return "", false
+	}
+
+	// Reject paths with traversal attempts before resolving them.
+	if hasDotDotComponent(host) {
+		fmt.Fprintf(os.Stderr, "odek: WARNING: rejecting volume mount %q (contains ..)\n", vol)
+		return "", false
+	}
+
+	// Resolve to an absolute path. Relative paths are interpreted relative to
+	// the current working directory, which should be the project root.
+	absHost := host
+	if !filepath.IsAbs(absHost) {
+		absHost = filepath.Join(workdir, absHost)
+	}
+	absHost = filepath.Clean(absHost)
+
+	absWorkdir, err := filepath.Abs(workdir)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "odek: WARNING: rejecting volume mount %q (cannot resolve workdir: %v)\n", vol, err)
+		return "", false
+	}
+	absWorkdir = filepath.Clean(absWorkdir)
+
+	// The host path must stay inside the working directory.
+	if !isPathUnder(absHost, absWorkdir) {
+		fmt.Fprintf(os.Stderr, "odek: WARNING: rejecting volume mount %q (host path %s is outside working directory %s)\n", vol, absHost, absWorkdir)
+		return "", false
+	}
+
+	// Reject symlinks — they could escape the working directory even if the
+	// link itself is inside it.
+	if info, err := os.Lstat(absHost); err == nil {
+		if info.Mode()&os.ModeSymlink != 0 {
+			fmt.Fprintf(os.Stderr, "odek: WARNING: rejecting volume mount %q (symlinks are not allowed)\n", vol)
+			return "", false
+		}
+	}
+
+	// Reject forbidden host paths.
+	for _, forbidden := range ForbiddenMountPrefixes {
+		if absHost == forbidden {
+			fmt.Fprintf(os.Stderr, "odek: WARNING: rejecting forbidden volume mount %q (host path %s)\n", vol, absHost)
+			return "", false
+		}
+		if strings.HasPrefix(absHost, forbidden+string(filepath.Separator)) {
+			fmt.Fprintf(os.Stderr, "odek: WARNING: rejecting forbidden volume mount %q (host path %s)\n", vol, absHost)
+			return "", false
+		}
+	}
+
+	// Rebuild the mount with the canonical absolute host path.
+	rest := ""
+	if len(parts) == 2 {
+		rest = ":" + parts[1]
+	}
+	return absHost + rest, true
+}
+
+// hasDotDotComponent reports whether p contains a ".." path component after
+// cleaning. It allows names that merely contain ".." as a substring (e.g.
+// "foo..bar") but rejects any traversal attempt.
+func hasDotDotComponent(p string) bool {
+	clean := filepath.Clean(p)
+	for _, part := range strings.Split(clean, string(filepath.Separator)) {
+		if part == ".." {
+			return true
+		}
+	}
+	return false
+}
+
+// isPathUnder reports whether path is inside root. Both paths must be clean
+// absolute paths. A path equal to root is considered under root.
+func isPathUnder(path, root string) bool {
+	rel, err := filepath.Rel(root, path)
+	if err != nil {
+		return false
+	}
+	if rel == "." {
+		return true
+	}
+	// filepath.Rel returns paths starting with ".." when path is outside root.
+	return !strings.HasPrefix(rel, "..") && !strings.Contains(filepath.ToSlash(rel), "/../")
+}
+
 // InjectFiles copies each file under cwd into a running container via
 // `docker cp`. Files inside cwd preserve their relative path; absolute
 // paths outside cwd are placed by basename at /workspace/. Nested paths

internal/sandbox/sandbox_test.go

@@ -134,15 +134,57 @@ func TestBuildRunArgs_ReadonlyAppendsRoSuffix(t *testing.T) {
 
 func TestBuildRunArgs_ForbiddenVolumeMountRejected(t *testing.T) {
 	args := BuildRunArgs(Config{
-		Volumes: []string{"/etc:/container/etc", "/safe:/safe"},
+		Volumes: []string{"/etc:/container/etc", "/workspace/extra:/container/extra"},
 	}, "odek-test", "/workspace", "alpine:latest")
 	for _, a := range args {
 		if strings.HasPrefix(a, "/etc:") {
 			t.Errorf("forbidden /etc mount should have been rejected, found %q", a)
 		}
 	}
-	if !contains(args, "/safe:/safe") {
-		t.Errorf("safe mount %q should have been preserved\nargs: %v", "/safe:/safe", args)
+	if !contains(args, "/workspace/extra:/container/extra") {
+		t.Errorf("safe mount %q should have been preserved\nargs: %v", "/workspace/extra:/container/extra", args)
+	}
+}
+
+func TestBuildRunArgs_VolumeOutsideWorkdirRejected(t *testing.T) {
+	args := BuildRunArgs(Config{
+		Volumes: []string{"/tmp:/container/tmp"},
+	}, "odek-test", "/workspace", "alpine:latest")
+	for i, a := range args {
+		if a == "-v" && i+1 < len(args) && strings.HasPrefix(args[i+1], "/tmp:") {
+			t.Errorf("volume outside workdir should have been rejected, found %q", args[i+1])
+		}
+	}
+}
+
+func TestBuildRunArgs_RelativeTraversalRejected(t *testing.T) {
+	args := BuildRunArgs(Config{
+		Volumes: []string{"../../etc:/container/etc"},
+	}, "odek-test", "/workspace", "alpine:latest")
+	for i, a := range args {
+		if a == "-v" && i+1 < len(args) && strings.Contains(args[i+1], "etc") {
+			t.Errorf("relative traversal volume should have been rejected, found %q", args[i+1])
+		}
+	}
+}
+
+func TestBuildRunArgs_DockerSocketRejected(t *testing.T) {
+	args := BuildRunArgs(Config{
+		Volumes: []string{"/var/run/docker.sock:/var/run/docker.sock"},
+	}, "odek-test", "/workspace", "alpine:latest")
+	for i, a := range args {
+		if a == "-v" && i+1 < len(args) && strings.Contains(args[i+1], "docker.sock") {
+			t.Errorf("docker socket mount should have been rejected, found %q", args[i+1])
+		}
+	}
+}
+
+func TestBuildRunArgs_RelativeVolumeResolvedUnderWorkdir(t *testing.T) {
+	args := BuildRunArgs(Config{
+		Volumes: []string{"extra:/container/extra"},
+	}, "odek-test", "/workspace", "alpine:latest")
+	if !contains(args, "/workspace/extra:/container/extra") {
+		t.Errorf("relative volume should be resolved to absolute under workdir\nargs: %v", args)
 	}
 }