diff --git a/cmd/gateway.go b/cmd/gateway.go index e4136788..35efe0cc 100644 --- a/cmd/gateway.go +++ b/cmd/gateway.go @@ -3,6 +3,7 @@ package cmd import ( "context" "fmt" + "io" "log/slog" "os" "os/signal" @@ -49,6 +50,25 @@ import ( _ "github.com/nextlevelbuilder/goclaw/internal/workstation/backends" ) +func gatewayLogOutput() io.Writer { + logFile := strings.TrimSpace(os.Getenv("GOCLAW_LOG_FILE")) + if logFile == "" { + if st, err := os.Stat("/var/log/goclaw"); err == nil && st.IsDir() { + logFile = "/var/log/goclaw/goclaw.log" + } + } + if logFile == "" { + return os.Stdout + } + f, err := os.OpenFile(logFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o644) + if err != nil { + fmt.Fprintf(os.Stderr, "warning: cannot open GOCLAW_LOG_FILE=%q: %v\n", logFile, err) + return os.Stdout + } + fmt.Fprintf(os.Stderr, "logging to %s\n", logFile) + return io.MultiWriter(os.Stdout, f) +} + func runGateway() { // Setup structured logging logLevel := slog.LevelInfo @@ -70,9 +90,8 @@ func runGateway() { fmt.Fprintf(os.Stderr, "warning: unknown GOCLAW_LOG_LEVEL=%q, using info\n", lvl) } } - textHandler := slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ - Level: logLevel, - }) + logOutput := gatewayLogOutput() + textHandler := slog.NewTextHandler(logOutput, &slog.HandlerOptions{Level: logLevel}) logTee := gateway.NewLogTee(textHandler) slog.SetDefault(slog.New(logTee)) diff --git a/internal/agent/systemprompt.go b/internal/agent/systemprompt.go index 031bfc1a..04d1b698 100644 --- a/internal/agent/systemprompt.go +++ b/internal/agent/systemprompt.go @@ -592,7 +592,7 @@ func buildToolingSection(toolNames []string, hasSandbox bool, shellDenyGroups ma lines = append(lines, "", "### Media Files", - `When users send media (, , , ), use the corresponding read_* tool with the path/media_id.`, + `When users send media (, , , ), use the corresponding read_* tool with the path/media_id. For archives (.zip, .tar.gz, etc.), use exec with the document path to inspect/extract the archive.`, "You have full vision/audio/video capabilities. NEVER say you cannot see images or files.", ) } diff --git a/internal/channels/media/media_tags.go b/internal/channels/media/media_tags.go index ae276d1f..3651ea33 100644 --- a/internal/channels/media/media_tags.go +++ b/internal/channels/media/media_tags.go @@ -102,6 +102,9 @@ func ExtractDocumentContent(filePath, fileName string) (string, error) { if !isText { // Binary files (PDF, DOCX, etc.) are persisted via MediaRef and analyzed // by the read_document tool. Return a hint instead of "not supported" placeholder. + if isArchiveFileName(fileName) { + return fmt.Sprintf("[Archive: %s — use exec with the path from the tag to inspect or extract this archive]", fileName), nil + } return fmt.Sprintf("[File: %s — use read_document tool to analyze this file]", fileName), nil } @@ -122,3 +125,16 @@ func ExtractDocumentContent(filePath, fileName string) (string, error) { return fmt.Sprintf("\n%s\n", fileName, mime, escaped), nil } + +func isArchiveFileName(fileName string) bool { + lower := strings.ToLower(fileName) + return strings.HasSuffix(lower, ".zip") || + strings.HasSuffix(lower, ".tar") || + strings.HasSuffix(lower, ".tar.gz") || + strings.HasSuffix(lower, ".tgz") || + strings.HasSuffix(lower, ".tar.bz2") || + strings.HasSuffix(lower, ".tbz2") || + strings.HasSuffix(lower, ".tar.xz") || + strings.HasSuffix(lower, ".txz") || + strings.HasSuffix(lower, ".gz") +} diff --git a/internal/channels/telegram/channel.go b/internal/channels/telegram/channel.go index b667ed0b..ba81ed4e 100644 --- a/internal/channels/telegram/channel.go +++ b/internal/channels/telegram/channel.go @@ -2,6 +2,7 @@ package telegram import ( "context" + "errors" "fmt" "log/slog" "net" @@ -28,7 +29,7 @@ type Channel struct { config config.TelegramConfig httpClient *http.Client transport *http.Transport - ipv4Once sync.Once // guards enableIPv4Only to prevent data race + ipv4Once sync.Once // guards enableIPv4Only to prevent data race agentStore store.AgentStore // for agent key lookup (nil if not configured) configPermStore store.ConfigPermissionStore // for group file writer management (nil if not configured) teamStore store.TeamStore // for /tasks, /task_detail commands (nil if not configured) @@ -38,16 +39,16 @@ type Channel struct { typingCtrls sync.Map // localKey string → *typing.Controller reactions sync.Map // localKey string → *StatusReactionController threadIDs sync.Map // localKey string → messageThreadID int (for forum topic routing) - mentionMode string // "strict" (default) or "yield" - botDisplayName string // bot's first_name from GetMe (e.g. "ViệtBot"); captured once at Start - pollCancel context.CancelFunc // cancels the long polling context - pollDone chan struct{} // closed when polling goroutine exits - handlerWg sync.WaitGroup // tracks in-flight handler goroutines for graceful shutdown - handlerSem chan struct{} // bounded semaphore for concurrent handler goroutines - pendingDraftID sync.Map // localKey string → int (draftID) - audioMgr *audio.Manager // unified STT via audio.Manager (nil = no STT) - writerHealMu sync.Mutex // guards writerHealLastTry for /writers self-heal - writerHealLastTry map[string]time.Time // key "chatID|userID" → last attempt timestamp + mentionMode string // "strict" (default) or "yield" + botDisplayName string // bot's first_name from GetMe (e.g. "ViệtBot"); captured once at Start + pollCancel context.CancelFunc // cancels the long polling context + pollDone chan struct{} // closed when polling goroutine exits + handlerWg sync.WaitGroup // tracks in-flight handler goroutines for graceful shutdown + handlerSem chan struct{} // bounded semaphore for concurrent handler goroutines + pendingDraftID sync.Map // localKey string → int (draftID) + audioMgr *audio.Manager // unified STT via audio.Manager (nil = no STT) + writerHealMu sync.Mutex // guards writerHealLastTry for /writers self-heal + writerHealLastTry map[string]time.Time // key "chatID|userID" → last attempt timestamp // pairingService, approvedGroups, pairingDebounce, groupHistory, historyLimit, requireMention // are inherited from channels.BaseChannel. } @@ -185,7 +186,17 @@ func (c *Channel) Start(ctx context.Context) error { me, err := c.bot.GetMe(probeCtx) probeCancel() if err != nil { - return fmt.Errorf("validate telegram bot: %w", err) + if shouldRetryTelegramStartupWithIPv4(err) { + c.enableIPv4Only() + slog.Warn("telegram: startup probe retrying with IPv4 fallback", "error", sanitizeTelegramError(err, c.config.Token)) + + probeCtx, probeCancel = context.WithTimeout(ctx, probeOverallTimeout) + me, err = c.bot.GetMe(probeCtx) + probeCancel() + } + } + if err != nil { + return fmt.Errorf("validate telegram bot: %w", sanitizeTelegramError(err, c.config.Token)) } username := "" if me != nil { @@ -405,6 +416,51 @@ func connectedSummary(username string) string { return fmt.Sprintf("Connected as @%s", username) } +type redactedTelegramError struct { + err error + token string +} + +func (e redactedTelegramError) Error() string { + msg := e.err.Error() + if e.token != "" { + msg = strings.ReplaceAll(msg, e.token, "") + } + return msg +} + +func (e redactedTelegramError) Unwrap() error { return e.err } + +func sanitizeTelegramError(err error, token string) error { + if err == nil { + return nil + } + return redactedTelegramError{err: err, token: token} +} + +func shouldRetryTelegramStartupWithIPv4(err error) bool { + if err == nil { + return false + } + var dnsErr *net.DNSError + if errors.As(err, &dnsErr) { + return false + } + msg := strings.ToLower(err.Error()) + if strings.Contains(msg, "lookup") || strings.Contains(msg, "no such host") { + return false + } + if isRetryableNetworkErr(err) { + return true + } + if strings.Contains(msg, "network is unreachable") || + strings.Contains(msg, "no route to host") || + strings.Contains(msg, "deadline exceeded") { + return true + } + return false +} + // applyIPv4Dialer forces a transport to use IPv4 only by overriding DialContext. func applyIPv4Dialer(t *http.Transport) { dialer := &net.Dialer{ diff --git a/internal/channels/telegram/channel_network_test.go b/internal/channels/telegram/channel_network_test.go new file mode 100644 index 00000000..c9054880 --- /dev/null +++ b/internal/channels/telegram/channel_network_test.go @@ -0,0 +1,50 @@ +package telegram + +import ( + "context" + "errors" + "fmt" + "net" + "strings" + "testing" +) + +func TestShouldRetryTelegramStartupWithIPv4ForPostConnectNetworkError(t *testing.T) { + err := fmt.Errorf(`telego: getMe: internal execution: request call: http do request: Post "https://api.telegram.org/bot123:ABC/getMe": read tcp [2407::1]:39268->[2001:67c:4e8:f004::9]:443: read: connection reset by peer`) + + if !shouldRetryTelegramStartupWithIPv4(err) { + t.Fatal("expected IPv4 startup retry for connection reset after IPv6 connect") + } +} + +func TestShouldRetryTelegramStartupWithIPv4SkipsDNSFailures(t *testing.T) { + err := &net.DNSError{Err: "no such host", Name: "api.telegram.org", IsNotFound: true} + + if shouldRetryTelegramStartupWithIPv4(err) { + t.Fatal("expected DNS failures to avoid IPv4 retry") + } +} + +func TestShouldRetryTelegramStartupWithIPv4SkipsAuthFailures(t *testing.T) { + err := errors.New(`telego: getMe: api: 401 "Unauthorized"`) + + if shouldRetryTelegramStartupWithIPv4(err) { + t.Fatal("expected auth failures to avoid IPv4 retry") + } +} + +func TestSanitizeTelegramErrorRedactsTokenAndPreservesCause(t *testing.T) { + token := "123456:ABC" + raw := fmt.Errorf(`Post "https://api.telegram.org/bot%s/getMe": %w`, token, context.DeadlineExceeded) + err := sanitizeTelegramError(raw, token) + + if strings.Contains(err.Error(), token) { + t.Fatalf("expected token to be redacted, got %q", err.Error()) + } + if !strings.Contains(err.Error(), "") { + t.Fatalf("expected redaction marker, got %q", err.Error()) + } + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatal("expected sanitized error to preserve wrapped cause") + } +} diff --git a/internal/skills/system_package_installer.go b/internal/skills/system_package_installer.go index 7af05d69..e3feebec 100644 --- a/internal/skills/system_package_installer.go +++ b/internal/skills/system_package_installer.go @@ -13,7 +13,7 @@ var ( debPackageNameRE = regexp.MustCompile(`^[a-z0-9][a-z0-9+.-]*$`) systemLookPath = exec.LookPath systemCommandCombinedOutput = runSystemCommandCombinedOutput - aptSystemPackageAliases = map[string]string{"pip3": "python3-pip", "github-cli": "gh"} + aptSystemPackageAliases = map[string]string{"pip3": "python3-pip", "github-cli": "gh", "go": "golang-go", "golang": "golang-go"} errSystemPackageMgrUnavailable = "system package manager unavailable on this runtime" ) diff --git a/internal/skills/system_package_installer_test.go b/internal/skills/system_package_installer_test.go index cab7e24e..cce43208 100644 --- a/internal/skills/system_package_installer_test.go +++ b/internal/skills/system_package_installer_test.go @@ -32,6 +32,8 @@ func TestResolveDebianPackageNameAliases(t *testing.T) { tests := map[string]string{ "pip3": "python3-pip", "github-cli": "gh", + "go": "golang-go", + "golang": "golang-go", "ripgrep": "ripgrep", "libstdc++": "libstdc++", } diff --git a/internal/tools/read_document.go b/internal/tools/read_document.go index 9a7891f5..7b3952df 100644 --- a/internal/tools/read_document.go +++ b/internal/tools/read_document.go @@ -94,6 +94,10 @@ func (t *ReadDocumentTool) Parameters() map[string]any { "type": "string", "description": "Optional: specific media_id from tag. If omitted, uses most recent document.", }, + "path": map[string]any{ + "type": "string", + "description": "Optional file path from a tag. Use this when the tag provides a path or the file is an archive that should be inspected with exec.", + }, }, "required": []string{"prompt"}, } @@ -105,15 +109,23 @@ func (t *ReadDocumentTool) Execute(ctx context.Context, args map[string]any) *Re prompt = "Analyze this document and describe its contents." } mediaID, _ := args["media_id"].(string) + docPathArg, _ := args["path"].(string) // Resolve document file path from MediaRefs in context. - docPath, docMime, err := t.resolveDocumentFile(ctx, mediaID) + docPath, docMime, err := t.resolveDocumentFile(ctx, mediaID, docPathArg) if err != nil { return ErrorResult(err.Error()) } slog.Info("read_document: resolved file", "path", docPath, "mime", docMime, "media_id", mediaID) + if isArchiveDocumentPath(docPath) { + return NewResult(fmt.Sprintf( + "Archive file available at %s. read_document does not analyze archive containers directly. Use exec to inspect or extract it, for example: unzip -l %q or unzip -q %q -d , then use list_files/read_file on extracted files.", + docPath, docPath, docPath, + )) + } + // Read document file. data, err := os.ReadFile(docPath) if err != nil { diff --git a/internal/tools/read_document_resolve.go b/internal/tools/read_document_resolve.go index 92150431..47d72cf9 100644 --- a/internal/tools/read_document_resolve.go +++ b/internal/tools/read_document_resolve.go @@ -5,30 +5,46 @@ import ( "encoding/base64" "fmt" "log/slog" + "os" "path/filepath" "strings" "github.com/nextlevelbuilder/goclaw/internal/providers" ) -// resolveDocumentFile finds the document file path from context MediaRefs. -func (t *ReadDocumentTool) resolveDocumentFile(ctx context.Context, mediaID string) (path, mime string, err error) { +// resolveDocumentFile finds the document file path from an explicit workspace +// path or from context MediaRefs. +func (t *ReadDocumentTool) resolveDocumentFile(ctx context.Context, mediaID, docPath string) (path, mime string, err error) { + if docPath != "" { + p, err := resolveDocumentPathArg(ctx, docPath) + if err != nil { + return "", "", err + } + return p, mimeFromDocExt(filepath.Ext(p)), nil + } + refs := MediaDocRefsFromCtx(ctx) if len(refs) == 0 { return "", "", fmt.Errorf("no documents available in this conversation. The user may not have sent a document.") } + if strings.Contains(mediaID, "<") || strings.Contains(mediaID, "media:") { + slog.Debug("read_document: sanitizing tag-like media_id", "raw", mediaID) + mediaID = "" + } + // Find specific media_id or use most recent document. var ref *providers.MediaRef if mediaID != "" { for i := range refs { - if refs[i].ID == mediaID { + if documentRefMatches(refs[i], mediaID) { ref = &refs[i] break } } if ref == nil { - return "", "", fmt.Errorf("document with media_id %q not found in conversation", mediaID) + slog.Warn("read_document: media_id not found, falling back to most recent", "media_id", mediaID) + ref = &refs[len(refs)-1] } } else { // Use the last (most recent) document ref. @@ -57,6 +73,71 @@ func (t *ReadDocumentTool) resolveDocumentFile(ctx context.Context, mediaID stri return p, mime, nil } +func resolveDocumentPathArg(ctx context.Context, path string) (string, error) { + workspace := ToolWorkspaceFromCtx(ctx) + resolved, err := resolvePathWithAllowed(path, workspace, effectiveRestrict(ctx, true), allowedWithTeamWorkspace(ctx, nil)) + if err != nil { + return "", fmt.Errorf("invalid document path: %w", err) + } + if err := checkDeniedPath(resolved, workspace, nil); err != nil { + return "", err + } + if info, err := os.Stat(resolved); err != nil { + return "", fmt.Errorf("failed to stat document file: %w", err) + } else if info.IsDir() { + return "", fmt.Errorf("document path is a directory: %s", path) + } + return resolved, nil +} + +func documentRefMatches(ref providers.MediaRef, mediaID string) bool { + if ref.ID == mediaID { + return true + } + if ref.Path == "" { + return false + } + want := strings.ToLower(filepath.Base(mediaID)) + got := strings.ToLower(filepath.Base(ref.Path)) + if want == got { + return true + } + return strings.EqualFold(stripUploadShortID(got), want) +} + +func stripUploadShortID(name string) string { + ext := filepath.Ext(name) + stem := strings.TrimSuffix(name, ext) + idx := strings.LastIndexByte(stem, '-') + if idx < 0 || len(stem)-idx-1 != 8 { + return name + } + for _, r := range stem[idx+1:] { + if !((r >= '0' && r <= '9') || (r >= 'a' && r <= 'f') || (r >= 'A' && r <= 'F')) { + return name + } + } + return stem[:idx] + ext +} + +func isArchiveDocumentPath(path string) bool { + lower := strings.ToLower(path) + switch { + case strings.HasSuffix(lower, ".zip"), + strings.HasSuffix(lower, ".tar"), + strings.HasSuffix(lower, ".tar.gz"), + strings.HasSuffix(lower, ".tgz"), + strings.HasSuffix(lower, ".tar.bz2"), + strings.HasSuffix(lower, ".tbz2"), + strings.HasSuffix(lower, ".tar.xz"), + strings.HasSuffix(lower, ".txz"), + strings.HasSuffix(lower, ".gz"): + return true + default: + return false + } +} + // callProvider dispatches document analysis to the appropriate provider API. // For Gemini: uses native generateContent API (supports PDF natively). // For others: uses standard Chat API with base64 document. @@ -128,6 +209,12 @@ func mimeFromDocExt(ext string) string { return "application/vnd.openxmlformats-officedocument.presentationml.presentation" case ".csv": return "text/csv" + case ".zip": + return "application/zip" + case ".tar": + return "application/x-tar" + case ".gz", ".tgz": + return "application/gzip" default: return "application/octet-stream" } diff --git a/internal/tools/read_document_resolve_test.go b/internal/tools/read_document_resolve_test.go new file mode 100644 index 00000000..bc2ac937 --- /dev/null +++ b/internal/tools/read_document_resolve_test.go @@ -0,0 +1,110 @@ +package tools + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/google/uuid" + + "github.com/nextlevelbuilder/goclaw/internal/providers" + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +func TestResolveDocumentFileAcceptsWorkspacePath(t *testing.T) { + workspace := t.TempDir() + uploadDir := filepath.Join(workspace, ".uploads") + if err := os.MkdirAll(uploadDir, 0o755); err != nil { + t.Fatal(err) + } + docPath := filepath.Join(uploadDir, "codex-9c8914a5.zip") + if err := os.WriteFile(docPath, []byte("zip bytes"), 0o644); err != nil { + t.Fatal(err) + } + wantPath, err := filepath.EvalSymlinks(docPath) + if err != nil { + t.Fatal(err) + } + + tool := NewReadDocumentTool(nil, nil) + ctx := WithToolWorkspace(context.Background(), workspace) + gotPath, gotMime, err := tool.resolveDocumentFile(ctx, "", ".uploads/codex-9c8914a5.zip") + if err != nil { + t.Fatalf("resolveDocumentFile returned error: %v", err) + } + if gotPath != wantPath { + t.Fatalf("path = %q, want %q", gotPath, wantPath) + } + if gotMime != "application/zip" { + t.Fatalf("mime = %q, want application/zip", gotMime) + } +} + +func TestResolveDocumentFileMatchesUploadedFilenameAlias(t *testing.T) { + workspace := t.TempDir() + docPath := filepath.Join(workspace, ".uploads", "codex-9c8914a5.zip") + refs := []providers.MediaRef{{ + ID: uuid.NewString(), + Kind: "document", + MimeType: "application/zip", + Path: docPath, + }} + + tool := NewReadDocumentTool(nil, nil) + ctx := WithMediaDocRefs(context.Background(), refs) + gotPath, gotMime, err := tool.resolveDocumentFile(ctx, "codex.zip", "") + if err != nil { + t.Fatalf("resolveDocumentFile returned error: %v", err) + } + if gotPath != docPath { + t.Fatalf("path = %q, want %q", gotPath, docPath) + } + if gotMime != "application/zip" { + t.Fatalf("mime = %q, want application/zip", gotMime) + } +} + +func TestResolveDocumentFileInvalidMediaIDFallsBackToMostRecent(t *testing.T) { + refs := []providers.MediaRef{ + {ID: uuid.NewString(), Kind: "document", Path: "/workspace/.uploads/old.pdf", MimeType: "application/pdf"}, + {ID: uuid.NewString(), Kind: "document", Path: "/workspace/.uploads/latest.pdf", MimeType: "application/pdf"}, + } + + tool := NewReadDocumentTool(nil, nil) + ctx := WithMediaDocRefs(context.Background(), refs) + gotPath, _, err := tool.resolveDocumentFile(ctx, "not-a-real-media-id", "") + if err != nil { + t.Fatalf("resolveDocumentFile returned error: %v", err) + } + if gotPath != refs[1].Path { + t.Fatalf("path = %q, want most recent %q", gotPath, refs[1].Path) + } +} + +func TestReadDocumentArchiveReturnsExecHint(t *testing.T) { + workspace := t.TempDir() + docPath := filepath.Join(workspace, ".uploads", "codex-9c8914a5.zip") + if err := os.MkdirAll(filepath.Dir(docPath), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(docPath, []byte("zip bytes"), 0o644); err != nil { + t.Fatal(err) + } + + tool := NewReadDocumentTool(nil, nil) + ctx := WithToolWorkspace(context.Background(), workspace) + ctx = store.WithTenantID(store.WithAgentID(ctx, uuid.New()), uuid.New()) + result := tool.Execute(ctx, map[string]any{ + "prompt": "Inspect this archive", + "path": ".uploads/codex-9c8914a5.zip", + }) + + if result.IsError { + t.Fatalf("expected archive hint, got error: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "unzip -l") || !strings.Contains(result.ForLLM, docPath) { + t.Fatalf("expected unzip hint with path, got: %s", result.ForLLM) + } +} diff --git a/internal/tools/shell.go b/internal/tools/shell.go index ccc4de6f..6be167cd 100644 --- a/internal/tools/shell.go +++ b/internal/tools/shell.go @@ -206,6 +206,27 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *Result { // Unicode-based pattern bypass while preserving functional command content. normalizedCommand := normalizeCommand(command) + // Credentialed exec is argv-based, not shell-based. Route it before shell + // deny scanning so ordinary argument text cannot be mistaken for executable + // shell syntax. + if cred, binary, cmdArgs := t.lookupCredentialedBinary(ctx, command); cred != nil { + cwd := ToolWorkspaceFromCtx(ctx) + if cwd == "" { + cwd = t.workspace + } + if wd, _ := args["working_dir"].(string); wd != "" { + if effectiveRestrict(ctx, t.restrict) { + if resolved, err := resolvePath(wd, t.workspace, true); err == nil { + cwd = resolved + } + } else { + cwd = wd + } + } + sandboxKey := ToolSandboxKeyFromCtx(ctx) + return t.executeCredentialed(ctx, cred, binary, cmdArgs, cwd, sandboxKey, command) + } + // Resolve deny patterns: merge per-agent context overrides with global // config (per-key agent precedence), fallback to all registry defaults. denyOverrides := t.effectiveDenyGroups(ctx) @@ -288,26 +309,6 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *Result { return SilentResult(hint) } - // Credentialed exec: if command matches a configured binary, use Direct Exec Mode. - // This bypasses approval (admin trust) and shell (security). - if cred, binary, cmdArgs := t.lookupCredentialedBinary(ctx, command); cred != nil { - cwd := ToolWorkspaceFromCtx(ctx) - if cwd == "" { - cwd = t.workspace - } - if wd, _ := args["working_dir"].(string); wd != "" { - if effectiveRestrict(ctx, t.restrict) { - if resolved, err := resolvePath(wd, t.workspace, true); err == nil { - cwd = resolved - } - } else { - cwd = wd - } - } - sandboxKey := ToolSandboxKeyFromCtx(ctx) - return t.executeCredentialed(ctx, cred, binary, cmdArgs, cwd, sandboxKey, command) - } - // Secure CLI gate: registered-but-not-granted binaries MUST NOT fall through // to host exec with parent env. Works on the already-normalized command // (Red Team F6) and unwraps shell wrappers up to depth 3 (Red Team F1). @@ -414,6 +415,15 @@ func matchesAny(command string, patterns []*regexp.Regexp) bool { return false } +func posixShellPath() (string, error) { + for _, candidate := range []string{"/bin/sh", "/usr/bin/sh"} { + if st, err := os.Stat(candidate); err == nil && !st.IsDir() && st.Mode()&0o111 != 0 { + return candidate, nil + } + } + return "", errors.New("no executable POSIX shell found at /bin/sh or /usr/bin/sh") +} + // executeOnHost runs a command directly on the host (original behavior). // ctx cancellation (e.g. agent abort) triggers SIGTERM → 3s grace → SIGKILL on the // entire process group so forked children are also cleaned up (no orphans). @@ -428,7 +438,11 @@ func (t *ExecTool) executeOnHost(ctx context.Context, command, cwd string) *Resu if runtime.GOOS == "windows" { cmd = exec.Command("cmd", "/C", command) } else { - cmd = exec.Command("sh", "-c", command) + shellPath, err := posixShellPath() + if err != nil { + return ErrorResult(err.Error()) + } + cmd = exec.Command(shellPath, "-c", command) } cmd.Dir = cwd diff --git a/internal/tools/shell_credentialed_gate_test.go b/internal/tools/shell_credentialed_gate_test.go index 57bc36ed..c35841d5 100644 --- a/internal/tools/shell_credentialed_gate_test.go +++ b/internal/tools/shell_credentialed_gate_test.go @@ -4,6 +4,8 @@ import ( "context" "encoding/json" "errors" + "os" + "path/filepath" "runtime" "strings" "sync" @@ -184,6 +186,94 @@ func TestExec_GrantedBinary_UsesCredentialedPath(t *testing.T) { } } +func TestExec_CredentialedArgsDoNotTriggerPackageInstallApproval(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell script fixture is POSIX-only") + } + tool, ctx := newCredentialedZernioTestTool(t) + result := tool.Execute(ctx, map[string]any{ + "command": `zernio posts:create --accounts abc --text "install with npm install -g agent-browser"`, + }) + + if result.IsError { + t.Fatalf("credentialed command should treat quoted package install text as argv data, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "posts:create") || !strings.Contains(result.ForLLM, "npm install -g agent-browser") { + t.Fatalf("expected script to receive original args, got: %s", result.ForLLM) + } +} + +func TestExec_CredentialedRejectsUnquotedShellOperator(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell script fixture is POSIX-only") + } + tool, ctx := newCredentialedZernioTestTool(t) + result := tool.Execute(ctx, map[string]any{ + "command": `zernio posts:create --text ok ; echo leaked`, + }) + + if !result.IsError { + t.Fatalf("expected credentialed command with unquoted shell operator to fail") + } + if !strings.Contains(result.ForLLM, "Shell operators not supported") && !strings.Contains(result.ForLLM, "Detected: ;") { + t.Fatalf("expected shell-operator rejection, got: %s", result.ForLLM) + } +} + +func TestExec_NonCredentialedPackageInstallStillDenied(t *testing.T) { + tool := NewExecTool(t.TempDir(), false) + result := tool.Execute(context.Background(), map[string]any{ + "command": "npm install left-pad", + }) + + if !result.IsError { + t.Fatalf("expected package install command to be denied") + } + if !strings.Contains(result.ForLLM, "package_install") && !strings.Contains(result.ForLLM, "matches pattern") { + t.Fatalf("expected package-install policy denial, got: %s", result.ForLLM) + } +} + +func TestPosixShellPathPrefersAbsoluteShell(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("POSIX-only") + } + got, err := posixShellPath() + if err != nil { + t.Fatalf("expected absolute POSIX shell on test runtime: %v", err) + } + if got != "/bin/sh" && got != "/usr/bin/sh" { + t.Fatalf("unexpected shell path: %q", got) + } + if _, err := os.Stat(got); err != nil { + t.Fatalf("shell path %q must exist: %v", got, err) + } +} + +func newCredentialedZernioTestTool(t *testing.T) (*ExecTool, context.Context) { + t.Helper() + binDir := t.TempDir() + binPath := filepath.Join(binDir, "zernio") + if err := os.WriteFile(binPath, []byte("#!/bin/sh\nprintf '%s\\n' \"$@\"\n"), 0o755); err != nil { + t.Fatal(err) + } + + stub := newStubSecureCLIStore() + stub.byName["zernio"] = &store.SecureCLIBinary{ + BinaryName: "zernio", + BinaryPath: &binPath, + EncryptedEnv: []byte("{}"), + TimeoutSeconds: 10, + DenyArgs: json.RawMessage("[]"), + DenyVerbose: json.RawMessage("[]"), + } + + tool := NewExecTool(t.TempDir(), false) + tool.SetSecureCLIStore(stub) + ctx := store.WithTenantID(store.WithAgentID(context.Background(), uuid.New()), uuid.New()) + return tool, ctx +} + // --- Phase 3 gate-enforcement tests --- // helper: build a gate-wired ExecTool with a fresh stub + ctx.