mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 02:15:57 +00:00
Address issues identified in PR #137 with a cleaner approach: - Sandbox isolation: add SandboxCwd/ResolveSandboxPath helpers to map filesystem tool paths to agent-scoped container subdirectories, preventing cross-agent file access via read/write/edit/list tools - DooD volume mounting: add resolveHostWorkspacePath with multi-strategy container ID detection (/proc/self/mountinfo, HOSTNAME, os.Hostname) and 5s timeout on docker inspect - Sandbox hints: expand from 1 pattern (binary not found) to 6 patterns (permission denied, network disabled, read-only FS, missing file, resource limits) with MaybeFsBridgeHint for filesystem tools - Nginx DNS: add Docker resolver (127.0.0.11) with dynamic upstream variable to handle backend container IP changes - MCP args: switch from comma-separated to space-separated parsing with quote support for --flag="value with spaces" patterns - Refactor: rename ExecTool.workingDir to workspace for consistency, extract sandbox.DefaultContainerWorkdir constant
This commit is contained in:
1 parent
a8fecfb3b1
commit
a4a7a59b6a
18 files changed
+554
-72
No files matched your search
@@ -81,14 +81,15 @@ func newDockerSandbox(ctx context.Context, name string, cfg Config, workspace st
|
||||
args = append(args, "--network", "none")
|
||||
}
|
||||
|
||||
// Workspace mount
|
||||
// Workspace mount — resolve host path for DooD (Docker-out-of-Docker) setups.
|
||||
containerWorkdir := cfg.ContainerWorkdir()
|
||||
if workspace != "" && cfg.WorkspaceAccess != AccessNone {
|
||||
mountOpt := "rw"
|
||||
if cfg.WorkspaceAccess == AccessRO {
|
||||
mountOpt = "ro"
|
||||
}
|
||||
args = append(args, "-v", fmt.Sprintf("%s:%s:%s", workspace, containerWorkdir, mountOpt))
|
||||
hostPath := resolveHostWorkspacePath(ctx, workspace)
|
||||
args = append(args, "-v", fmt.Sprintf("%s:%s:%s", hostPath, containerWorkdir, mountOpt))
|
||||
}
|
||||
args = append(args, "-w", containerWorkdir)
|
||||
|
||||
|
||||
@@ -0,0 +1,136 @@
|
||||
package sandbox
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"log/slog"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"time"
|
||||
)
|
||||
|
||||
// resolveHostWorkspacePath maps a container-local path to its host-side
|
||||
// equivalent for Docker-out-of-Docker (DooD) sibling container mounts.
|
||||
// When GoClaw runs inside a container and spawns sandbox containers, the
|
||||
// workspace path (e.g., /app/workspace) only exists inside the GoClaw
|
||||
// container — the sandbox needs the corresponding host path or volume name
|
||||
// to mount it correctly.
|
||||
//
|
||||
// If not running in Docker (no /.dockerenv), returns localPath as-is.
|
||||
func resolveHostWorkspacePath(ctx context.Context, localPath string) string {
|
||||
// Not in a container — local path is the host path.
|
||||
if _, err := os.Stat("/.dockerenv"); err != nil {
|
||||
return localPath
|
||||
}
|
||||
|
||||
containerID := detectContainerID()
|
||||
if containerID == "" {
|
||||
slog.Error("sandbox.resolve: cannot determine container ID — DooD volume mounts will fail", "path", localPath)
|
||||
return localPath
|
||||
}
|
||||
|
||||
// Short timeout to avoid blocking on a stuck Docker daemon.
|
||||
inspectCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
|
||||
defer cancel()
|
||||
|
||||
out, err := exec.CommandContext(inspectCtx, "docker", "inspect", "--format", "{{json .Mounts}}", containerID).Output()
|
||||
if err != nil {
|
||||
slog.Warn("sandbox.resolve: docker inspect failed", "container", containerID, "error", err)
|
||||
return localPath
|
||||
}
|
||||
|
||||
var mounts []struct {
|
||||
Type string `json:"Type"`
|
||||
Source string `json:"Source"`
|
||||
Destination string `json:"Destination"`
|
||||
Name string `json:"Name"`
|
||||
}
|
||||
if err := json.Unmarshal(out, &mounts); err != nil {
|
||||
slog.Warn("sandbox.resolve: failed to parse mounts", "error", err)
|
||||
return localPath
|
||||
}
|
||||
|
||||
targetDir := filepath.Clean(localPath)
|
||||
var bestDest string
|
||||
var bestSource string
|
||||
var bestRel string
|
||||
var bestType string
|
||||
var bestName string
|
||||
|
||||
for _, m := range mounts {
|
||||
dest := filepath.Clean(m.Destination)
|
||||
if targetDir == dest || strings.HasPrefix(targetDir, dest+string(filepath.Separator)) {
|
||||
if len(dest) > len(bestDest) {
|
||||
bestDest = dest
|
||||
bestSource = m.Source
|
||||
bestType = m.Type
|
||||
bestName = m.Name
|
||||
bestRel, _ = filepath.Rel(dest, targetDir)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if bestDest == "" {
|
||||
slog.Warn("sandbox.resolve: no matching mount found", "path", localPath, "container", containerID)
|
||||
return localPath
|
||||
}
|
||||
|
||||
// Named volume: return volume name if it's the exact mount, otherwise
|
||||
// use the host source path (assumes local volume driver).
|
||||
if bestType == "volume" && bestName != "" {
|
||||
if bestRel == "." {
|
||||
slog.Debug("sandbox.resolve: resolved to named volume", "path", localPath, "volume", bestName)
|
||||
return bestName
|
||||
}
|
||||
if bestSource != "" {
|
||||
return filepath.Join(bestSource, bestRel)
|
||||
}
|
||||
}
|
||||
|
||||
// Bind mount: join source with relative path.
|
||||
if bestSource != "" {
|
||||
resolved := filepath.Join(bestSource, bestRel)
|
||||
slog.Debug("sandbox.resolve: resolved to host path", "path", localPath, "host", resolved)
|
||||
return resolved
|
||||
}
|
||||
|
||||
slog.Warn("sandbox.resolve: mount found but no source path", "path", localPath, "mount", bestDest)
|
||||
return localPath
|
||||
}
|
||||
|
||||
// detectContainerID returns the current Docker container ID using multiple
|
||||
// strategies for reliability:
|
||||
// 1. /proc/self/mountinfo — parse the container ID from cgroup mount paths
|
||||
// 2. HOSTNAME env var — usually the short container ID (but can be overridden)
|
||||
// 3. os.Hostname() — fallback
|
||||
func detectContainerID() string {
|
||||
// Strategy 1: Parse /proc/self/mountinfo for docker container ID.
|
||||
// Lines contain paths like /docker/containers/<id>/...
|
||||
if data, err := os.ReadFile("/proc/self/mountinfo"); err == nil {
|
||||
for _, line := range strings.Split(string(data), "\n") {
|
||||
if idx := strings.Index(line, "/docker/containers/"); idx != -1 {
|
||||
rest := line[idx+len("/docker/containers/"):]
|
||||
if slashIdx := strings.IndexByte(rest, '/'); slashIdx > 0 {
|
||||
id := rest[:slashIdx]
|
||||
if len(id) >= 12 { // Docker IDs are 64 hex chars, short form 12
|
||||
return id
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Strategy 2: HOSTNAME env var (common Docker default).
|
||||
if h := os.Getenv("HOSTNAME"); h != "" {
|
||||
return h
|
||||
}
|
||||
|
||||
// Strategy 3: os.Hostname() fallback.
|
||||
if h, err := os.Hostname(); err == nil && h != "" {
|
||||
return h
|
||||
}
|
||||
|
||||
return ""
|
||||
}
|
||||
@@ -117,12 +117,16 @@ func (c Config) ShouldSandbox(agentID string) bool {
|
||||
}
|
||||
}
|
||||
|
||||
// DefaultContainerWorkdir is the default container-side working directory
|
||||
// used when no custom Workdir is configured.
|
||||
const DefaultContainerWorkdir = "/workspace"
|
||||
|
||||
// ContainerWorkdir returns the container-side working directory.
|
||||
func (c Config) ContainerWorkdir() string {
|
||||
if c.Workdir != "" {
|
||||
return c.Workdir
|
||||
}
|
||||
return "/workspace"
|
||||
return DefaultContainerWorkdir
|
||||
}
|
||||
|
||||
// ResolveScopeKey maps a session key to a sandbox scope key.
|
||||
|
||||
@@ -114,7 +114,7 @@ func (t *ExecTool) executeCredentialed(ctx context.Context, cred *store.SecureCL
|
||||
if err != nil {
|
||||
r := credentialedPathError(binary, err)
|
||||
if t.sandboxMgr != nil && sandboxKey != "" {
|
||||
r.ForLLM += sandboxHint
|
||||
r.ForLLM += hintBinaryNotFound
|
||||
}
|
||||
return r
|
||||
}
|
||||
@@ -181,7 +181,7 @@ func (t *ExecTool) executeCredentialedHost(ctx context.Context, absPath string,
|
||||
func (t *ExecTool) executeCredentialedSandbox(ctx context.Context, absPath string, args []string,
|
||||
cwd string, sandboxKey string, envMap map[string]string, timeout time.Duration) *Result {
|
||||
|
||||
sb, err := t.sandboxMgr.Get(ctx, sandboxKey, t.workingDir, SandboxConfigFromCtx(ctx))
|
||||
sb, err := t.sandboxMgr.Get(ctx, sandboxKey, t.workspace, SandboxConfigFromCtx(ctx))
|
||||
if err != nil {
|
||||
slog.Warn("security.credentialed_exec_sandbox_unavailable",
|
||||
"binary", absPath, "error", err)
|
||||
|
||||
+11
-5
@@ -198,10 +198,16 @@ func (t *EditTool) executeInSandbox(ctx context.Context, path, oldStr, newStr st
|
||||
return ErrorResult(fmt.Sprintf("sandbox error: %v", err))
|
||||
}
|
||||
|
||||
bridge := sandbox.NewFsBridge(sb.ID(), "/workspace")
|
||||
content, err := bridge.ReadFile(ctx, path)
|
||||
containerCwd, cwdErr := SandboxCwd(ctx, t.workspace, sandbox.DefaultContainerWorkdir)
|
||||
if cwdErr != nil {
|
||||
return ErrorResult(fmt.Sprintf("sandbox path mapping: %v", cwdErr))
|
||||
}
|
||||
containerPath := ResolveSandboxPath(path, containerCwd)
|
||||
|
||||
bridge := sandbox.NewFsBridge(sb.ID(), sandbox.DefaultContainerWorkdir)
|
||||
content, err := bridge.ReadFile(ctx, containerPath)
|
||||
if err != nil {
|
||||
return ErrorResult(fmt.Sprintf("failed to read file: %v", err))
|
||||
return ErrorResult(fmt.Sprintf("failed to read file: %v", err) + MaybeFsBridgeHint(err))
|
||||
}
|
||||
|
||||
newContent, result := applyEdit(content, oldStr, newStr, replaceAll)
|
||||
@@ -209,8 +215,8 @@ func (t *EditTool) executeInSandbox(ctx context.Context, path, oldStr, newStr st
|
||||
return result
|
||||
}
|
||||
|
||||
if err := bridge.WriteFile(ctx, path, newContent); err != nil {
|
||||
return ErrorResult(fmt.Sprintf("failed to write file: %v", err))
|
||||
if err := bridge.WriteFile(ctx, containerPath, newContent); err != nil {
|
||||
return ErrorResult(fmt.Sprintf("failed to write file: %v", err) + MaybeFsBridgeHint(err))
|
||||
}
|
||||
|
||||
count := strings.Count(content, oldStr)
|
||||
|
||||
@@ -180,9 +180,15 @@ func (t *ReadFileTool) executeInSandbox(ctx context.Context, path, sandboxKey st
|
||||
return ErrorResult(fmt.Sprintf("sandbox error: %v", err))
|
||||
}
|
||||
|
||||
data, err := bridge.ReadFile(ctx, path)
|
||||
containerCwd, cwdErr := SandboxCwd(ctx, t.workspace, sandbox.DefaultContainerWorkdir)
|
||||
if cwdErr != nil {
|
||||
return ErrorResult(fmt.Sprintf("sandbox path mapping: %v", cwdErr))
|
||||
}
|
||||
containerPath := ResolveSandboxPath(path, containerCwd)
|
||||
|
||||
data, err := bridge.ReadFile(ctx, containerPath)
|
||||
if err != nil {
|
||||
return ErrorResult(fmt.Sprintf("failed to read file: %v", err))
|
||||
return ErrorResult(fmt.Sprintf("failed to read file: %v", err) + MaybeFsBridgeHint(err))
|
||||
}
|
||||
|
||||
return SilentResult(data)
|
||||
@@ -193,7 +199,7 @@ func (t *ReadFileTool) getFsBridge(ctx context.Context, sandboxKey string) (*san
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return sandbox.NewFsBridge(sb.ID(), "/workspace"), nil
|
||||
return sandbox.NewFsBridge(sb.ID(), sandbox.DefaultContainerWorkdir), nil
|
||||
}
|
||||
|
||||
// allowedWithTeamWorkspace returns the allowed prefixes with team workspace appended
|
||||
|
||||
@@ -138,9 +138,15 @@ func (t *ListFilesTool) executeInSandbox(ctx context.Context, path, sandboxKey s
|
||||
return ErrorResult(fmt.Sprintf("sandbox error: %v", err))
|
||||
}
|
||||
|
||||
output, err := bridge.ListDir(ctx, path)
|
||||
containerCwd, cwdErr := SandboxCwd(ctx, t.workspace, sandbox.DefaultContainerWorkdir)
|
||||
if cwdErr != nil {
|
||||
return ErrorResult(fmt.Sprintf("sandbox path mapping: %v", cwdErr))
|
||||
}
|
||||
containerPath := ResolveSandboxPath(path, containerCwd)
|
||||
|
||||
output, err := bridge.ListDir(ctx, containerPath)
|
||||
if err != nil {
|
||||
return ErrorResult(fmt.Sprintf("failed to list directory: %v", err))
|
||||
return ErrorResult(fmt.Sprintf("failed to list directory: %v", err) + MaybeFsBridgeHint(err))
|
||||
}
|
||||
|
||||
return SilentResult(output)
|
||||
@@ -151,5 +157,5 @@ func (t *ListFilesTool) getFsBridge(ctx context.Context, sandboxKey string) (*sa
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return sandbox.NewFsBridge(sb.ID(), "/workspace"), nil
|
||||
return sandbox.NewFsBridge(sb.ID(), sandbox.DefaultContainerWorkdir), nil
|
||||
}
|
||||
@@ -224,8 +224,14 @@ func (t *WriteFileTool) executeInSandbox(ctx context.Context, path, content, san
|
||||
return ErrorResult(fmt.Sprintf("sandbox error: %v", err))
|
||||
}
|
||||
|
||||
if err := bridge.WriteFile(ctx, path, content); err != nil {
|
||||
return ErrorResult(fmt.Sprintf("failed to write file: %v", err))
|
||||
containerCwd, cwdErr := SandboxCwd(ctx, t.workspace, sandbox.DefaultContainerWorkdir)
|
||||
if cwdErr != nil {
|
||||
return ErrorResult(fmt.Sprintf("sandbox path mapping: %v", cwdErr))
|
||||
}
|
||||
containerPath := ResolveSandboxPath(path, containerCwd)
|
||||
|
||||
if err := bridge.WriteFile(ctx, containerPath, content); err != nil {
|
||||
return ErrorResult(fmt.Sprintf("failed to write file: %v", err) + MaybeFsBridgeHint(err))
|
||||
}
|
||||
|
||||
msg := fmt.Sprintf("File written: %s (%d bytes)", path, len(content))
|
||||
@@ -251,5 +257,5 @@ func (t *WriteFileTool) getFsBridge(ctx context.Context, sandboxKey string) (*sa
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return sandbox.NewFsBridge(sb.ID(), "/workspace"), nil
|
||||
return sandbox.NewFsBridge(sb.ID(), sandbox.DefaultContainerWorkdir), nil
|
||||
}
|
||||
@@ -2,31 +2,114 @@ package tools
|
||||
|
||||
import "strings"
|
||||
|
||||
// sandboxHint is appended to tool error output when a command fails inside
|
||||
// a Docker sandbox container due to a missing binary/tool (exit code 127).
|
||||
const sandboxHint = "\n\n[SANDBOX] This command ran inside a Docker sandbox container. " +
|
||||
// --- Hint messages for LLM consumption ---
|
||||
|
||||
const hintBinaryNotFound = "\n\n[SANDBOX] This command ran inside a Docker sandbox container. " +
|
||||
"The required tool/binary is not installed in the sandbox image. " +
|
||||
"Tell the user this failed due to sandbox environment limitations — " +
|
||||
"they can install the binary in the sandbox image or disable sandbox mode for this agent."
|
||||
|
||||
// isBinaryNotFound returns true if the error output and exit code indicate
|
||||
// a missing binary. Exit code 127 is the POSIX convention for "command not found".
|
||||
const hintPermissionDenied = "\n\n[SANDBOX] Permission denied inside sandbox container. " +
|
||||
"The workspace may be mounted as read-only (workspace_access: ro). " +
|
||||
"Check the agent's sandbox configuration or tell the user to change workspace_access to rw."
|
||||
|
||||
const hintNetworkDisabled = "\n\n[SANDBOX] Network operation failed — sandbox networking is disabled (--network none). " +
|
||||
"If this agent needs internet access, tell the user to enable network_enabled in the agent's sandbox configuration."
|
||||
|
||||
const hintReadOnlyFS = "\n\n[SANDBOX] Write failed — target path is outside the mounted workspace volume. " +
|
||||
"The sandbox filesystem is read-only except for the workspace mount. " +
|
||||
"Ensure all file operations use paths within the workspace directory."
|
||||
|
||||
const hintNoSuchFile = "\n\n[SANDBOX] File or directory not found inside sandbox container. " +
|
||||
"The sandbox has a minimal filesystem — only the workspace mount and installed packages are available. " +
|
||||
"Verify the path exists within the workspace, or install required files in the sandbox image."
|
||||
|
||||
const hintResourceLimit = "\n\n[SANDBOX] Sandbox resource limit reached (disk space or memory). " +
|
||||
"The container has restricted resources. Tell the user to increase sandbox limits " +
|
||||
"or clean up temporary files in the workspace."
|
||||
|
||||
// --- Detection functions ---
|
||||
|
||||
// isBinaryNotFound returns true if the error indicates a missing binary.
|
||||
// Exit code 127 is the POSIX convention for "command not found".
|
||||
func isBinaryNotFound(exitCode int, output string) bool {
|
||||
if exitCode == 127 {
|
||||
return true
|
||||
}
|
||||
// Fallback: match stderr patterns from non-standard shells.
|
||||
lower := strings.ToLower(output)
|
||||
return strings.Contains(lower, "not found") &&
|
||||
(strings.Contains(lower, "command") || strings.Contains(lower, "sh:"))
|
||||
}
|
||||
|
||||
// MaybeSandboxHint returns the sandbox hint suffix if the error looks like
|
||||
// a missing binary. Returns empty string otherwise.
|
||||
// Only call this from sandbox execution paths — never from host execution.
|
||||
func isPermissionDenied(output string) bool {
|
||||
lower := strings.ToLower(output)
|
||||
return strings.Contains(lower, "permission denied") || strings.Contains(lower, "eacces")
|
||||
}
|
||||
|
||||
func isNetworkDisabled(output string) bool {
|
||||
lower := strings.ToLower(output)
|
||||
return strings.Contains(lower, "network is unreachable") ||
|
||||
strings.Contains(lower, "name resolution")
|
||||
// Note: "connection refused" is NOT matched here — it indicates a service
|
||||
// not running, not network disabled. --network none produces "network is
|
||||
// unreachable" or DNS resolution failures, not "connection refused".
|
||||
}
|
||||
|
||||
func isReadOnlyFS(output string) bool {
|
||||
lower := strings.ToLower(output)
|
||||
return strings.Contains(lower, "read-only file system") || strings.Contains(lower, "erofs")
|
||||
}
|
||||
|
||||
func isResourceLimit(output string) bool {
|
||||
lower := strings.ToLower(output)
|
||||
return strings.Contains(lower, "no space left") ||
|
||||
strings.Contains(lower, "enospc") ||
|
||||
strings.Contains(lower, "cannot allocate memory")
|
||||
}
|
||||
|
||||
// --- Public API ---
|
||||
|
||||
// MaybeSandboxHint returns an LLM-actionable hint suffix for sandbox exec errors.
|
||||
// Checks multiple error patterns in priority order. Returns empty string if no
|
||||
// pattern matches. Only call from sandbox execution paths.
|
||||
func MaybeSandboxHint(exitCode int, output string) string {
|
||||
if isBinaryNotFound(exitCode, output) {
|
||||
return sandboxHint
|
||||
return hintBinaryNotFound
|
||||
}
|
||||
if isPermissionDenied(output) {
|
||||
return hintPermissionDenied
|
||||
}
|
||||
if isNetworkDisabled(output) {
|
||||
return hintNetworkDisabled
|
||||
}
|
||||
if isReadOnlyFS(output) {
|
||||
return hintReadOnlyFS
|
||||
}
|
||||
if isResourceLimit(output) {
|
||||
return hintResourceLimit
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
// MaybeFsBridgeHint returns an LLM-actionable hint for FsBridge errors
|
||||
// (filesystem tool sandbox operations). FsBridge errors are Go errors,
|
||||
// not exit codes, so we match against err.Error() text.
|
||||
func MaybeFsBridgeHint(err error) string {
|
||||
if err == nil {
|
||||
return ""
|
||||
}
|
||||
msg := err.Error()
|
||||
if isPermissionDenied(msg) {
|
||||
return hintPermissionDenied
|
||||
}
|
||||
if isReadOnlyFS(msg) {
|
||||
return hintReadOnlyFS
|
||||
}
|
||||
if strings.Contains(strings.ToLower(msg), "no such file") {
|
||||
return hintNoSuchFile
|
||||
}
|
||||
if isResourceLimit(msg) {
|
||||
return hintResourceLimit
|
||||
}
|
||||
return ""
|
||||
}
|
||||
@@ -1,6 +1,7 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
@@ -35,28 +36,70 @@ func TestIsBinaryNotFound(t *testing.T) {
|
||||
|
||||
func TestMaybeSandboxHint(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
exitCode int
|
||||
output string
|
||||
wantHint bool
|
||||
name string
|
||||
exitCode int
|
||||
output string
|
||||
wantTag string // expected [SANDBOX] substring, or "" for no hint
|
||||
}{
|
||||
{"binary not found returns hint", 127, "sh: git: not found", true},
|
||||
{"success returns empty", 0, "ok", false},
|
||||
{"non-binary error returns empty", 1, "segfault", false},
|
||||
{"sh pattern returns hint", 1, "sh: python3: not found", true},
|
||||
{"binary not found", 127, "sh: git: not found", "not installed in the sandbox image"},
|
||||
{"success no hint", 0, "ok", ""},
|
||||
{"non-binary error no hint", 1, "segfault", ""},
|
||||
{"sh pattern", 1, "sh: python3: not found", "not installed in the sandbox image"},
|
||||
{"permission denied", 1, "bash: /workspace/file: Permission denied", "read-only"},
|
||||
{"network unreachable", 1, "connect: Network is unreachable", "networking is disabled"},
|
||||
{"dns failure", 1, "Temporary failure in name resolution", "networking is disabled"},
|
||||
{"read-only fs", 1, "mkdir: cannot create directory: Read-only file system", "outside the mounted workspace"},
|
||||
{"no space left", 1, "write: No space left on device", "resource limit"},
|
||||
{"cannot allocate memory", 1, "Cannot allocate memory", "resource limit"},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := MaybeSandboxHint(tt.exitCode, tt.output)
|
||||
if tt.wantHint {
|
||||
if !strings.Contains(got, "[SANDBOX]") {
|
||||
t.Errorf("MaybeSandboxHint(%d, %q) = %q, want hint containing [SANDBOX]", tt.exitCode, tt.output, got)
|
||||
}
|
||||
} else {
|
||||
if tt.wantTag == "" {
|
||||
if got != "" {
|
||||
t.Errorf("MaybeSandboxHint(%d, %q) = %q, want empty", tt.exitCode, tt.output, got)
|
||||
}
|
||||
} else {
|
||||
if !strings.Contains(got, "[SANDBOX]") {
|
||||
t.Errorf("MaybeSandboxHint(%d, %q) missing [SANDBOX] tag, got %q", tt.exitCode, tt.output, got)
|
||||
}
|
||||
if !strings.Contains(got, tt.wantTag) {
|
||||
t.Errorf("MaybeSandboxHint(%d, %q) missing %q, got %q", tt.exitCode, tt.output, tt.wantTag, got)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestMaybeFsBridgeHint(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
err error
|
||||
wantTag string
|
||||
}{
|
||||
{"nil error", nil, ""},
|
||||
{"permission denied", fmt.Errorf("docker exec: permission denied"), "read-only"},
|
||||
{"read-only filesystem", fmt.Errorf("write /workspace/file: read-only file system"), "outside the mounted workspace"},
|
||||
{"no such file", fmt.Errorf("stat /workspace/missing.txt: no such file or directory"), "not found inside sandbox"},
|
||||
{"no space", fmt.Errorf("write: no space left on device"), "resource limit"},
|
||||
{"generic error", fmt.Errorf("unexpected EOF"), ""},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := MaybeFsBridgeHint(tt.err)
|
||||
if tt.wantTag == "" {
|
||||
if got != "" {
|
||||
t.Errorf("MaybeFsBridgeHint(%v) = %q, want empty", tt.err, got)
|
||||
}
|
||||
} else {
|
||||
if !strings.Contains(got, "[SANDBOX]") {
|
||||
t.Errorf("MaybeFsBridgeHint(%v) missing [SANDBOX] tag, got %q", tt.err, got)
|
||||
}
|
||||
if !strings.Contains(got, tt.wantTag) {
|
||||
t.Errorf("MaybeFsBridgeHint(%v) missing %q, got %q", tt.err, tt.wantTag, got)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -0,0 +1,45 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// SandboxCwd maps the current effective workspace (from context) to its
|
||||
// corresponding path inside the sandbox container. The sandbox mounts the
|
||||
// global workspace root at containerBase (usually "/workspace"). This function
|
||||
// computes the relative path from globalWorkspace to the context workspace
|
||||
// and joins it with containerBase.
|
||||
//
|
||||
// Example: globalWorkspace="/app/workspace", ctx workspace="/app/workspace/agent-a/user-123"
|
||||
// → returns "/workspace/agent-a/user-123"
|
||||
func SandboxCwd(ctx context.Context, globalWorkspace, containerBase string) (string, error) {
|
||||
ws := ToolWorkspaceFromCtx(ctx)
|
||||
if ws == "" {
|
||||
// No per-request workspace — fall back to container root.
|
||||
return containerBase, nil
|
||||
}
|
||||
|
||||
rel, err := filepath.Rel(globalWorkspace, ws)
|
||||
if err != nil || strings.HasPrefix(filepath.Clean(rel), "..") {
|
||||
return "", fmt.Errorf("workspace %q is outside global mount %q", ws, globalWorkspace)
|
||||
}
|
||||
|
||||
if rel == "." {
|
||||
return containerBase, nil
|
||||
}
|
||||
return filepath.Join(containerBase, rel), nil
|
||||
}
|
||||
|
||||
// ResolveSandboxPath resolves a tool-provided path (relative or absolute)
|
||||
// against the sandbox container CWD. If the path is relative, it is joined
|
||||
// with containerCwd. Absolute paths are returned as-is (the sandbox
|
||||
// filesystem already restricts access to the mounted volume).
|
||||
func ResolveSandboxPath(path, containerCwd string) string {
|
||||
if filepath.IsAbs(path) {
|
||||
return path
|
||||
}
|
||||
return filepath.Join(containerCwd, path)
|
||||
}
|
||||
@@ -0,0 +1,133 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestSandboxCwd(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
ctxWorkspace string // empty = no workspace in context
|
||||
globalWorkspace string
|
||||
containerBase string
|
||||
want string
|
||||
wantErr bool
|
||||
}{
|
||||
{
|
||||
name: "no workspace in context — fallback to container base",
|
||||
ctxWorkspace: "",
|
||||
globalWorkspace: "/app/workspace",
|
||||
containerBase: "/workspace",
|
||||
want: "/workspace",
|
||||
},
|
||||
{
|
||||
name: "workspace equals global mount",
|
||||
ctxWorkspace: "/app/workspace",
|
||||
globalWorkspace: "/app/workspace",
|
||||
containerBase: "/workspace",
|
||||
want: "/workspace",
|
||||
},
|
||||
{
|
||||
name: "per-agent workspace",
|
||||
ctxWorkspace: "/app/workspace/agent-a-workspace",
|
||||
globalWorkspace: "/app/workspace",
|
||||
containerBase: "/workspace",
|
||||
want: "/workspace/agent-a-workspace",
|
||||
},
|
||||
{
|
||||
name: "per-user workspace",
|
||||
ctxWorkspace: "/app/workspace/agent-a/user-123",
|
||||
globalWorkspace: "/app/workspace",
|
||||
containerBase: "/workspace",
|
||||
want: "/workspace/agent-a/user-123",
|
||||
},
|
||||
{
|
||||
name: "team workspace",
|
||||
ctxWorkspace: "/app/workspace/teams/team-uuid/chat-123",
|
||||
globalWorkspace: "/app/workspace",
|
||||
containerBase: "/workspace",
|
||||
want: "/workspace/teams/team-uuid/chat-123",
|
||||
},
|
||||
{
|
||||
name: "workspace outside global mount — error",
|
||||
ctxWorkspace: "/other/path/agent-a",
|
||||
globalWorkspace: "/app/workspace",
|
||||
containerBase: "/workspace",
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "custom container base",
|
||||
ctxWorkspace: "/app/workspace/agent-a",
|
||||
globalWorkspace: "/app/workspace",
|
||||
containerBase: "/home/sandbox",
|
||||
want: "/home/sandbox/agent-a",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
if tt.ctxWorkspace != "" {
|
||||
ctx = WithToolWorkspace(ctx, tt.ctxWorkspace)
|
||||
}
|
||||
got, err := SandboxCwd(ctx, tt.globalWorkspace, tt.containerBase)
|
||||
if tt.wantErr {
|
||||
if err == nil {
|
||||
t.Errorf("SandboxCwd() = %q, want error", got)
|
||||
}
|
||||
return
|
||||
}
|
||||
if err != nil {
|
||||
t.Errorf("SandboxCwd() error = %v", err)
|
||||
return
|
||||
}
|
||||
if got != tt.want {
|
||||
t.Errorf("SandboxCwd() = %q, want %q", got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveSandboxPath(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
path string
|
||||
containerCwd string
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "relative path joined with cwd",
|
||||
path: "file.txt",
|
||||
containerCwd: "/workspace/agent-a",
|
||||
want: "/workspace/agent-a/file.txt",
|
||||
},
|
||||
{
|
||||
name: "relative subdirectory path",
|
||||
path: "subdir/file.txt",
|
||||
containerCwd: "/workspace/agent-a",
|
||||
want: "/workspace/agent-a/subdir/file.txt",
|
||||
},
|
||||
{
|
||||
name: "absolute path passed through",
|
||||
path: "/workspace/agent-a/file.txt",
|
||||
containerCwd: "/workspace/agent-b",
|
||||
want: "/workspace/agent-a/file.txt",
|
||||
},
|
||||
{
|
||||
name: "dot path",
|
||||
path: ".",
|
||||
containerCwd: "/workspace/agent-a",
|
||||
want: "/workspace/agent-a",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := ResolveSandboxPath(tt.path, tt.containerCwd)
|
||||
if got != tt.want {
|
||||
t.Errorf("ResolveSandboxPath(%q, %q) = %q, want %q", tt.path, tt.containerCwd, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
+14
-18
@@ -7,7 +7,6 @@ import (
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"strings"
|
||||
"time"
|
||||
@@ -31,7 +30,7 @@ func DefaultDenyPatterns() []*regexp.Regexp {
|
||||
|
||||
// ExecTool executes shell commands, optionally inside a sandbox container.
|
||||
type ExecTool struct {
|
||||
workingDir string
|
||||
workspace string
|
||||
timeout time.Duration
|
||||
pathDenyPatterns []*regexp.Regexp // always-on path-based denials (DenyPaths)
|
||||
denyExemptions []string // substrings that exempt a command from deny
|
||||
@@ -43,18 +42,18 @@ type ExecTool struct {
|
||||
}
|
||||
|
||||
// NewExecTool creates an exec tool that runs commands directly on the host.
|
||||
func NewExecTool(workingDir string, restrict bool) *ExecTool {
|
||||
func NewExecTool(workspace string, restrict bool) *ExecTool {
|
||||
return &ExecTool{
|
||||
workingDir: workingDir,
|
||||
workspace: workspace,
|
||||
timeout: 60 * time.Second,
|
||||
restrict: restrict,
|
||||
}
|
||||
}
|
||||
|
||||
// NewSandboxedExecTool creates an exec tool that routes commands through a sandbox container.
|
||||
func NewSandboxedExecTool(workingDir string, restrict bool, mgr sandbox.Manager) *ExecTool {
|
||||
func NewSandboxedExecTool(workspace string, restrict bool, mgr sandbox.Manager) *ExecTool {
|
||||
return &ExecTool{
|
||||
workingDir: workingDir,
|
||||
workspace: workspace,
|
||||
timeout: 300 * time.Second, // sandbox allows longer timeout
|
||||
restrict: restrict,
|
||||
sandboxMgr: mgr,
|
||||
@@ -168,11 +167,11 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *Result {
|
||||
if cred, binary, cmdArgs := t.lookupCredentialedBinary(ctx, command); cred != nil {
|
||||
cwd := ToolWorkspaceFromCtx(ctx)
|
||||
if cwd == "" {
|
||||
cwd = t.workingDir
|
||||
cwd = t.workspace
|
||||
}
|
||||
if wd, _ := args["working_dir"].(string); wd != "" {
|
||||
if effectiveRestrict(ctx, t.restrict) {
|
||||
if resolved, err := resolvePath(wd, t.workingDir, true); err == nil {
|
||||
if resolved, err := resolvePath(wd, t.workspace, true); err == nil {
|
||||
cwd = resolved
|
||||
}
|
||||
} else {
|
||||
@@ -202,11 +201,11 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *Result {
|
||||
// Use per-user workspace from context if available, fallback to struct field
|
||||
cwd := ToolWorkspaceFromCtx(ctx)
|
||||
if cwd == "" {
|
||||
cwd = t.workingDir
|
||||
cwd = t.workspace
|
||||
}
|
||||
if wd, _ := args["working_dir"].(string); wd != "" {
|
||||
if effectiveRestrict(ctx, t.restrict) {
|
||||
resolved, err := resolvePath(wd, t.workingDir, true)
|
||||
resolved, err := resolvePath(wd, t.workspace, true)
|
||||
if err != nil {
|
||||
return ErrorResult(err.Error())
|
||||
}
|
||||
@@ -282,7 +281,7 @@ func (t *ExecTool) executeOnHost(ctx context.Context, command, cwd string) *Resu
|
||||
|
||||
// executeInSandbox routes a command through a Docker sandbox container.
|
||||
func (t *ExecTool) executeInSandbox(ctx context.Context, command, cwd, sandboxKey string) *Result {
|
||||
sb, err := t.sandboxMgr.Get(ctx, sandboxKey, t.workingDir, SandboxConfigFromCtx(ctx))
|
||||
sb, err := t.sandboxMgr.Get(ctx, sandboxKey, t.workspace, SandboxConfigFromCtx(ctx))
|
||||
if err != nil {
|
||||
if errors.Is(err, sandbox.ErrSandboxDisabled) {
|
||||
return t.executeOnHost(ctx, command, cwd)
|
||||
@@ -296,13 +295,10 @@ func (t *ExecTool) executeInSandbox(ctx context.Context, command, cwd, sandboxKe
|
||||
return ErrorResult(fmt.Sprintf("sandbox unavailable: %v (will not fall back to unsandboxed host execution)", err))
|
||||
}
|
||||
|
||||
// Map host workdir to container workdir
|
||||
containerCwd := "/workspace"
|
||||
if cwd != t.workingDir {
|
||||
rel, relErr := filepath.Rel(t.workingDir, cwd)
|
||||
if relErr == nil {
|
||||
containerCwd = filepath.Join("/workspace", rel)
|
||||
}
|
||||
// Map host workdir to container workdir via SandboxCwd helper.
|
||||
containerCwd, cwdErr := SandboxCwd(ctx, t.workspace, sandbox.DefaultContainerWorkdir)
|
||||
if cwdErr != nil {
|
||||
return ErrorResult(fmt.Sprintf("sandbox path mapping: %v", cwdErr))
|
||||
}
|
||||
|
||||
result, err := sb.Exec(ctx, []string{"sh", "-c", command}, containerCwd) //nolint: no ExecOption for normal exec
|
||||
|
||||
+9
-3
@@ -10,6 +10,12 @@ server {
|
||||
gzip_types text/plain text/css application/json application/javascript text/xml application/xml text/javascript image/svg+xml;
|
||||
gzip_min_length 256;
|
||||
|
||||
# Docker internal DNS resolver — re-resolves upstream when backend
|
||||
# container restarts with a new IP (prevents stale DNS cache).
|
||||
# Note: valid=10s means up to 10s stale DNS on backend restart.
|
||||
resolver 127.0.0.11 valid=10s ipv6=off;
|
||||
set $upstream_backend "http://goclaw:18790";
|
||||
|
||||
# Cache static assets
|
||||
location /assets/ {
|
||||
expires 1y;
|
||||
@@ -18,7 +24,7 @@ server {
|
||||
|
||||
# WebSocket proxy
|
||||
location /ws {
|
||||
proxy_pass http://goclaw:18790;
|
||||
proxy_pass $upstream_backend;
|
||||
proxy_http_version 1.1;
|
||||
proxy_set_header Upgrade $http_upgrade;
|
||||
proxy_set_header Connection "upgrade";
|
||||
@@ -30,7 +36,7 @@ server {
|
||||
|
||||
# API proxy
|
||||
location /v1/ {
|
||||
proxy_pass http://goclaw:18790;
|
||||
proxy_pass $upstream_backend;
|
||||
proxy_set_header Host $host;
|
||||
proxy_set_header X-Real-IP $remote_addr;
|
||||
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
|
||||
@@ -38,7 +44,7 @@ server {
|
||||
|
||||
# Health check proxy
|
||||
location /health {
|
||||
proxy_pass http://goclaw:18790;
|
||||
proxy_pass $upstream_backend;
|
||||
}
|
||||
|
||||
# SPA fallback — serve index.html for all other routes
|
||||
|
||||
@@ -32,8 +32,8 @@
|
||||
"displayNamePlaceholder": "My MCP Server",
|
||||
"transport": "Transport *",
|
||||
"command": "Command *",
|
||||
"args": "Args (comma-separated)",
|
||||
"argsPlaceholder": "--flag1, --flag2",
|
||||
"args": "Args (space-separated)",
|
||||
"argsPlaceholder": "--flag1 --flag2=\"value with spaces\"",
|
||||
"url": "URL *",
|
||||
"headers": "Headers",
|
||||
"headerKeyPlaceholder": "Header name",
|
||||
|
||||
@@ -32,8 +32,8 @@
|
||||
"displayNamePlaceholder": "Máy chủ MCP của tôi",
|
||||
"transport": "Giao thức *",
|
||||
"command": "Lệnh *",
|
||||
"args": "Tham số (phân cách bằng dấu phẩy)",
|
||||
"argsPlaceholder": "--flag1, --flag2",
|
||||
"args": "Tham số (phân cách bằng dấu cách)",
|
||||
"argsPlaceholder": "--flag1 --flag2=\"giá trị có dấu cách\"",
|
||||
"url": "URL *",
|
||||
"headers": "Headers",
|
||||
"headerKeyPlaceholder": "Tên header",
|
||||
|
||||
@@ -32,8 +32,8 @@
|
||||
"displayNamePlaceholder": "我的 MCP 服务器",
|
||||
"transport": "传输协议 *",
|
||||
"command": "命令 *",
|
||||
"args": "参数(逗号分隔)",
|
||||
"argsPlaceholder": "--flag1, --flag2",
|
||||
"args": "参数(空格分隔)",
|
||||
"argsPlaceholder": "--flag1 --flag2=\"带空格的值\"",
|
||||
"url": "URL *",
|
||||
"headers": "请求头",
|
||||
"headerKeyPlaceholder": "请求头名称",
|
||||
|
||||
@@ -78,9 +78,20 @@ export function MCPFormDialog({ open, onOpenChange, server, onSubmit, onTest }:
|
||||
const isStdio = transport === "stdio";
|
||||
|
||||
const buildConnectionData = () => {
|
||||
const parsedArgs = isStdio && args.trim()
|
||||
? args.split(",").map((a) => a.trim()).filter(Boolean)
|
||||
: undefined;
|
||||
let parsedArgs: string[] | undefined = undefined;
|
||||
if (isStdio && args.trim()) {
|
||||
// Split by spaces respecting quoted strings, including --flag="value" patterns.
|
||||
const matches = args.match(/[^\s"']*"([^"]*)"|[^\s"']*'([^']*)'|[^\s]+/g);
|
||||
if (matches) {
|
||||
parsedArgs = matches.map((m) => {
|
||||
// Strip surrounding quotes from standalone quoted args: "foo bar" → foo bar
|
||||
if (m.startsWith('"') && m.endsWith('"')) return m.slice(1, -1);
|
||||
if (m.startsWith("'") && m.endsWith("'")) return m.slice(1, -1);
|
||||
// Strip quotes from --flag="value" patterns: --flag="foo bar" → --flag=foo bar
|
||||
return m.replace(/"([^"]*)"/g, "$1").replace(/'([^']*)'/g, "$1");
|
||||
});
|
||||
}
|
||||
}
|
||||
const parsedHeaders = !isStdio && Object.keys(headers).length > 0 ? headers : undefined;
|
||||
const parsedEnv = Object.keys(env).length > 0 ? env : undefined;
|
||||
return {
|
||||
|
||||
Reference in new issue
Block a user