From c1e0654672a8782206f0d55c18d5fceace31aa59 Mon Sep 17 00:00:00 2001 From: Peter Dedene Date: Wed, 7 Oct 2026 11:18:12 +0200 Subject: [PATCH 1/6] feat(credentials): add file: backend for mounted secret files Reads Docker/Kubernetes secret mounts. Follows symlinks (Secret volumes use ..data links), requires owner root or daemon uid, rejects group-writable or other-accessible files, caps size at 64 KiB, trims trailing newlines, supports jq, and is never cached so rotation applies immediately. --- internal/credentials/credentials.go | 12 +++ internal/credentials/file.go | 58 ++++++++++++ internal/credentials/file_test.go | 132 ++++++++++++++++++++++++++++ internal/credentials/parser.go | 6 ++ 4 files changed, 208 insertions(+) create mode 100644 internal/credentials/file.go create mode 100644 internal/credentials/file_test.go diff --git a/internal/credentials/credentials.go b/internal/credentials/credentials.go index 20323d1..529ddb7 100644 --- a/internal/credentials/credentials.go +++ b/internal/credentials/credentials.go @@ -138,6 +138,18 @@ func fetchCredentialFromBackend(parsed *ParsedSource, options *FetchOptions) (Cr } } + case BackendFile: + result, err = fetchFromFile(parsed.Path) + if err != nil { + return Credential{}, err + } + if parsed.HasJQ() { + result, err = ApplyJQ(ctx, []byte(result), parsed.JQExpr) + if err != nil { + return Credential{}, err + } + } + case BackendPass: result, err = fetchFromPass(options.PassBinary, parsed.Path) if err != nil { diff --git a/internal/credentials/file.go b/internal/credentials/file.go new file mode 100644 index 0000000..edb674f --- /dev/null +++ b/internal/credentials/file.go @@ -0,0 +1,58 @@ +package credentials + +import ( + "fmt" + "io" + "os" + "path/filepath" + "strings" + "syscall" +) + +// maxSecretFileSize bounds how much of a file: credential is read. +const maxSecretFileSize = 64 * 1024 + +// fetchFromFile reads a mounted secret file (Docker/Kubernetes secret +// convention). Symlinks are followed because Kubernetes secret volumes are +// built from ..data symlinks. Trust comes from the file itself: it must be a +// regular file owned by root or the daemon user, and not group-writable or +// accessible by others. Trailing newlines are trimmed. +func fetchFromFile(path string) (string, error) { + if !filepath.IsAbs(path) { + return "", fmt.Errorf("secret file path must be absolute: %s", path) + } + + f, err := os.Open(path) + if err != nil { + return "", fmt.Errorf("open secret file: %w", err) + } + defer f.Close() + + info, err := f.Stat() + if err != nil { + return "", fmt.Errorf("stat secret file: %w", err) + } + if !info.Mode().IsRegular() { + return "", fmt.Errorf("secret file must be a regular file: %s", path) + } + if perm := info.Mode().Perm(); perm&0o027 != 0 { + return "", fmt.Errorf("secret file must not be group-writable or accessible by others: %s has %04o", path, perm) + } + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return "", fmt.Errorf("secret file: unsupported file stat type") + } + if owner := int(stat.Uid); owner != 0 && owner != currentEUIDFunc() { + return "", fmt.Errorf("secret file owner must be root or the daemon uid: %s owner=%d daemon=%d", path, owner, currentEUIDFunc()) + } + + data, err := io.ReadAll(io.LimitReader(f, maxSecretFileSize+1)) + if err != nil { + return "", fmt.Errorf("read secret file: %w", err) + } + if len(data) > maxSecretFileSize { + return "", fmt.Errorf("secret file exceeds %d bytes: %s", maxSecretFileSize, path) + } + + return strings.TrimRight(string(data), "\r\n"), nil +} diff --git a/internal/credentials/file_test.go b/internal/credentials/file_test.go new file mode 100644 index 0000000..29317b8 --- /dev/null +++ b/internal/credentials/file_test.go @@ -0,0 +1,132 @@ +package credentials + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func writeSecretFile(t *testing.T, content string, mode os.FileMode) string { + t.Helper() + path := filepath.Join(t.TempDir(), "secret") + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Chmod(path, mode); err != nil { + t.Fatal(err) + } + return path +} + +func TestParseSource_File(t *testing.T) { + p, err := ParseSource("file:/etc/claw-wrap/secrets/token | .value") + if err != nil { + t.Fatal(err) + } + if p.Backend != BackendFile || p.Path != "/etc/claw-wrap/secrets/token" || p.JQExpr != ".value" { + t.Fatalf("parsed = %+v", p) + } +} + +func TestFetch_File(t *testing.T) { + t.Run("reads value and trims trailing newline", func(t *testing.T) { + path := writeSecretFile(t, "s3cr3t-value\r\n\n", 0o600) + got, err := Fetch("file:" + path) + if err != nil { + t.Fatalf("Fetch() error = %v", err) + } + if got != "s3cr3t-value" { + t.Fatalf("Fetch() = %q", got) + } + }) + + t.Run("group-readable is allowed (k8s fsGroup + defaultMode 0440)", func(t *testing.T) { + path := writeSecretFile(t, "v", 0o440) + if _, err := Fetch("file:" + path); err != nil { + t.Fatalf("Fetch() error = %v", err) + } + }) + + t.Run("follows symlinks (k8s secret volumes use ..data links)", func(t *testing.T) { + target := writeSecretFile(t, "linked", 0o400) + link := filepath.Join(t.TempDir(), "link") + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + got, err := Fetch("file:" + link) + if err != nil || got != "linked" { + t.Fatalf("Fetch() = %q, %v", got, err) + } + }) + + t.Run("jq extraction", func(t *testing.T) { + path := writeSecretFile(t, `{"client_secret":"abc"}`, 0o600) + got, err := Fetch("file:" + path + " | .client_secret") + if err != nil || got != "abc" { + t.Fatalf("Fetch() = %q, %v", got, err) + } + }) + + t.Run("re-reads on every fetch so rotation applies", func(t *testing.T) { + path := writeSecretFile(t, "one", 0o600) + if got, _ := Fetch("file:" + path); got != "one" { + t.Fatalf("first fetch = %q", got) + } + if err := os.WriteFile(path, []byte("two"), 0o600); err != nil { + t.Fatal(err) + } + if got, _ := Fetch("file:" + path); got != "two" { + t.Fatalf("second fetch = %q, want rotated value", got) + } + }) + + errorCases := []struct { + name string + source func(t *testing.T) string + wantErr string + }{ + {"relative path", func(t *testing.T) string { return "file:secrets/token" }, "must be absolute"}, + {"world-readable", func(t *testing.T) string { return "file:" + writeSecretFile(t, "v", 0o644) }, "must not be group-writable or accessible by others"}, + {"group-writable", func(t *testing.T) string { return "file:" + writeSecretFile(t, "v", 0o660) }, "must not be group-writable or accessible by others"}, + {"directory", func(t *testing.T) string { return "file:" + t.TempDir() }, "must be a regular file"}, + {"missing", func(t *testing.T) string { return "file:/nonexistent/claw-wrap/secret" }, "open secret file"}, + {"too large", func(t *testing.T) string { + return "file:" + writeSecretFile(t, strings.Repeat("x", maxSecretFileSize+1), 0o600) + }, "exceeds"}, + } + for _, tc := range errorCases { + t.Run(tc.name, func(t *testing.T) { + _, err := Fetch(tc.source(t)) + if err == nil || !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("Fetch() error = %v, want containing %q", err, tc.wantErr) + } + }) + } + + t.Run("owner must be daemon or root", func(t *testing.T) { + path := writeSecretFile(t, "v", 0o600) + orig := currentEUIDFunc + currentEUIDFunc = func() int { return os.Geteuid() + 4242 } + t.Cleanup(func() { currentEUIDFunc = orig }) + + _, err := Fetch("file:" + path) + if os.Geteuid() == 0 { + if err != nil { + t.Fatalf("root-owned file should be accepted: %v", err) + } + return + } + if err == nil || !strings.Contains(err.Error(), "owner must be root or the daemon uid") { + t.Fatalf("Fetch() error = %v", err) + } + }) + + t.Run("error never contains the secret", func(t *testing.T) { + path := writeSecretFile(t, "TOPSECRETVALUE", 0o644) + _, err := Fetch("file:" + path) + if err == nil || strings.Contains(err.Error(), "TOPSECRETVALUE") { + t.Fatalf("Fetch() error = %v", err) + } + }) +} diff --git a/internal/credentials/parser.go b/internal/credentials/parser.go index a1fd609..2a418ab 100644 --- a/internal/credentials/parser.go +++ b/internal/credentials/parser.go @@ -18,6 +18,7 @@ const ( BackendBitwarden Backend = "bw" BackendVault Backend = "vault" BackendExecJSON Backend = "exec-json" + BackendFile Backend = "file" ) // ParsedSource represents a parsed credential source URI. @@ -42,6 +43,8 @@ type ParsedSource struct { // - age:/path/to/file.age | .jq_expr // - vault:secret/myapp/api-key // - vault:secret/myapp/api-key | .password +// - file:/etc/claw-wrap/secrets/token +// - file:/etc/claw-wrap/secrets/creds.json | .client_secret // - path/in/store (legacy, assumed pass) func ParseSource(source string) (*ParsedSource, error) { if source == "" { @@ -86,6 +89,9 @@ func ParseSource(source string) (*ParsedSource, error) { case strings.HasPrefix(source, "vault:"): backend = BackendVault path = strings.TrimPrefix(source, "vault:") + case strings.HasPrefix(source, "file:"): + backend = BackendFile + path = strings.TrimPrefix(source, "file:") case strings.HasPrefix(source, "exec-json:"): backend = BackendExecJSON path = strings.TrimPrefix(source, "exec-json:") From 43f6afd6d2be3c043ed5b864cf4cecb7e5fcb2eb Mon Sep 17 00:00:00 2001 From: Peter Dedene Date: Wed, 7 Oct 2026 11:18:29 +0200 Subject: [PATCH 2/6] feat: support running claw-wrap as a container sidecar Tool options: - match: argv in allowed_args: one anchored pattern per argument, exact arity. Arguments are never joined, so spaces/newlines cannot widen a match. - working_dir pins the tool cwd (the caller's cwd may not exist daemon-side). - use_stdin: false gives the tool /dev/null stdin and disables PTY. - request_env allowlists which env vars the caller may set. - Injected credential values (>= 8 bytes) are redacted from tool output. Runtime: - --runtime-gid, --auth-mode, --socket-mode (ported from feature/docker-compose) and strict --uid parsing. - CLAW_WRAP_RUNTIME_DIR moves socket/auth/proxy token for daemon and wrapper; the env credential file stays put. - Reload config on Kubernetes ConfigMap ..data symlink swaps. - audit.stdout writes JSONL audit entries to stdout; audit cwd is the effective working directory. - Warn when no tools are configured; log once when caller exe verification is unavailable (separate PID namespace). Fixes: - Tool output could be truncated: Wait() closed StdoutPipe readers before the pumpers drained them. Use owned pipes with an idle-based drain deadline, which also keeps detached children from holding a call open. - Data race between handleTimeout and cmd.Wait on ProcessState. - finalizeOutput no longer drops stderr when stdout finalization fails. examples/mcparcel/wrappers.yaml is covered by regression tests. --- cmd/claw-wrap/main.go | 43 +++- cmd/claw-wrap/main_test.go | 48 +++- examples/mcparcel/wrappers.yaml | 62 +++++ internal/audit/audit.go | 32 +++ internal/audit/stdout_test.go | 45 ++++ internal/auth/auth.go | 9 +- internal/auth/auth_test.go | 26 ++ internal/config/args_rules.go | 66 ++++++ internal/config/args_rules_test.go | 151 ++++++++++++ internal/config/config.go | 100 ++++---- internal/config/tool_options_test.go | 128 ++++++++++ internal/daemon/args_policy.go | 27 +++ .../daemon/configwatcher_configmap_test.go | 75 ++++++ internal/daemon/daemon.go | 124 +++++++++- internal/daemon/executor.go | 223 ++++++++++++------ internal/daemon/executor_lifecycle_test.go | 216 +++++++++++++++++ internal/daemon/executor_options_test.go | 121 ++++++++++ internal/daemon/executor_secrets_test.go | 108 +++++++++ internal/daemon/helper_process_test.go | 94 ++++++++ internal/daemon/mcparcel_policy_test.go | 124 ++++++++++ internal/daemon/output_redactor.go | 49 ++++ internal/daemon/runtime_modes.go | 71 ++++++ internal/daemon/runtime_modes_test.go | 89 +++++++ internal/paths/paths.go | 30 ++- internal/paths/runtime_dir_test.go | 41 ++++ 25 files changed, 1974 insertions(+), 128 deletions(-) create mode 100644 examples/mcparcel/wrappers.yaml create mode 100644 internal/audit/stdout_test.go create mode 100644 internal/config/args_rules.go create mode 100644 internal/config/args_rules_test.go create mode 100644 internal/config/tool_options_test.go create mode 100644 internal/daemon/configwatcher_configmap_test.go create mode 100644 internal/daemon/executor_lifecycle_test.go create mode 100644 internal/daemon/executor_options_test.go create mode 100644 internal/daemon/executor_secrets_test.go create mode 100644 internal/daemon/helper_process_test.go create mode 100644 internal/daemon/mcparcel_policy_test.go create mode 100644 internal/daemon/runtime_modes.go create mode 100644 internal/daemon/runtime_modes_test.go create mode 100644 internal/paths/runtime_dir_test.go diff --git a/cmd/claw-wrap/main.go b/cmd/claw-wrap/main.go index ac06b79..06c7aa7 100644 --- a/cmd/claw-wrap/main.go +++ b/cmd/claw-wrap/main.go @@ -19,6 +19,7 @@ import ( "os" "path/filepath" "regexp" + "strconv" "claw-wrap/internal/config" "claw-wrap/internal/daemon" @@ -99,10 +100,34 @@ func runDaemon() error { opts = append(opts, daemon.WithConfigPath(os.Args[i+1])) i++ case arg == "--uid" && i+1 < len(os.Args): - var uid uint32 - fmt.Sscanf(os.Args[i+1], "%d", &uid) + uidValue, err := strconv.ParseUint(os.Args[i+1], 10, 32) + if err != nil { + return fmt.Errorf("invalid --uid value %q: %w", os.Args[i+1], err) + } + uid := uint32(uidValue) opts = append(opts, daemon.WithAllowedUID(uid)) i++ + case arg == "--runtime-gid" && i+1 < len(os.Args): + gid, err := strconv.Atoi(os.Args[i+1]) + if err != nil || gid < 0 { + return fmt.Errorf("invalid --runtime-gid value %q (must be >= 0)", os.Args[i+1]) + } + opts = append(opts, daemon.WithRuntimeGID(gid)) + i++ + case arg == "--auth-mode" && i+1 < len(os.Args): + mode, err := daemon.ParseAuthFileMode(os.Args[i+1]) + if err != nil { + return fmt.Errorf("invalid --auth-mode: %w", err) + } + opts = append(opts, daemon.WithAuthFileMode(mode)) + i++ + case arg == "--socket-mode" && i+1 < len(os.Args): + mode, err := daemon.ParseSocketFileMode(os.Args[i+1]) + if err != nil { + return fmt.Errorf("invalid --socket-mode: %w", err) + } + opts = append(opts, daemon.WithSocketFileMode(mode)) + i++ case arg == "-h" || arg == "--help": fmt.Printf(`claw-wrap daemon - Start the secrets daemon @@ -110,10 +135,16 @@ Usage: claw-wrap daemon [options] Options: - --socket PATH Socket path (default: %s) - --config PATH Config path (default: %s) - --uid UID Allowed UID (default: %d) - -h, --help Show this help + --socket PATH Socket path (default: %s) + --config PATH Config path (default: %s) + --uid UID Allowed UID (default: %d) + --runtime-gid GID Shared group for runtime dir, socket and auth file (optional) + --auth-mode MODE Auth file mode: 0600 or 0640 (default: 0600) + --socket-mode MODE Socket mode: 0600 or 0660 (default: 0600) + -h, --help Show this help + +Environment: + CLAW_WRAP_RUNTIME_DIR Runtime dir for socket and auth file (set it for the wrapper too) `, paths.SocketPath(), paths.ConfigPath(), os.Getuid()) return nil } diff --git a/cmd/claw-wrap/main_test.go b/cmd/claw-wrap/main_test.go index 8f8c348..4adccfe 100644 --- a/cmd/claw-wrap/main_test.go +++ b/cmd/claw-wrap/main_test.go @@ -1,6 +1,10 @@ package main -import "testing" +import ( + "os" + "strings" + "testing" +) func TestSafeToolNameRegex(t *testing.T) { valid := []string{"gh", "my-tool", "tool.v2", "tool_name"} @@ -17,3 +21,45 @@ func TestSafeToolNameRegex(t *testing.T) { } } } + +func TestRunDaemon_InvalidFlags(t *testing.T) { + t.Run("invalid uid", func(t *testing.T) { + orig := os.Args + defer func() { os.Args = orig }() + os.Args = []string{"claw-wrap", "daemon", "--uid", "nope"} + err := runDaemon() + if err == nil || !strings.Contains(err.Error(), "invalid --uid") { + t.Fatalf("runDaemon() error = %v, want invalid --uid", err) + } + }) + + t.Run("invalid auth mode", func(t *testing.T) { + orig := os.Args + defer func() { os.Args = orig }() + os.Args = []string{"claw-wrap", "daemon", "--auth-mode", "0666"} + err := runDaemon() + if err == nil || !strings.Contains(err.Error(), "invalid --auth-mode") { + t.Fatalf("runDaemon() error = %v, want invalid --auth-mode", err) + } + }) + + t.Run("invalid socket mode", func(t *testing.T) { + orig := os.Args + defer func() { os.Args = orig }() + os.Args = []string{"claw-wrap", "daemon", "--socket-mode", "0640"} + err := runDaemon() + if err == nil || !strings.Contains(err.Error(), "invalid --socket-mode") { + t.Fatalf("runDaemon() error = %v, want invalid --socket-mode", err) + } + }) + + t.Run("invalid runtime gid", func(t *testing.T) { + orig := os.Args + defer func() { os.Args = orig }() + os.Args = []string{"claw-wrap", "daemon", "--runtime-gid", "-1"} + err := runDaemon() + if err == nil || !strings.Contains(err.Error(), "invalid --runtime-gid") { + t.Fatalf("runDaemon() error = %v, want invalid --runtime-gid", err) + } + }) +} diff --git a/examples/mcparcel/wrappers.yaml b/examples/mcparcel/wrappers.yaml new file mode 100644 index 0000000..2de2500 --- /dev/null +++ b/examples/mcparcel/wrappers.yaml @@ -0,0 +1,62 @@ +# claw-wrap sidecar config: broker a CLI that talks to an MCP server. +# +# Example: an agent may call a handful of Front tools through `mcparcel call`. +# The sidecar holds the OAuth client credentials; mcparcel exchanges them for +# short-lived tokens and enforces its own tool policy on top of this one. +# +# This file is loaded by internal/daemon/mcparcel_policy_test.go, so every +# change here is covered by the regression tests. + +security: + # Caller exe verification cannot work across containers (separate PID + # namespaces: the peer PID is 0). UID check + HMAC are the boundary. + deny_unverified_caller_exe: false + +credentials: + front_client_id: + source: file:/etc/claw-wrap/secrets/front-client-id + front_client_secret: + source: file:/etc/claw-wrap/secrets/front-client-secret + +audit: + enabled: true + stdout: true # JSONL on stdout; daemon logs go to stderr + include_args: true + +tools: + mcparcel: + binary: /usr/local/bin/mcparcel + timeout: 120s + # The caller's cwd does not exist in the sidecar; run here instead. + working_dir: /var/lib/claw-wrap + # Non-interactive: stdin is /dev/null (implies no PTY), so a prompt fails + # immediately instead of hanging until the timeout. + use_stdin: false + # The agent signs its own requests, so any env it sends is untrusted. + # Accept none: a tool-specific variable (say, a server URL) could + # otherwise redirect the credentials. + request_env: [] + env: + FRONT_CLIENT_ID: front_client_id + FRONT_CLIENT_SECRET: front_client_secret + mode: allowlist + # Checked first. Defense in depth only: the argv rules below already pin + # every position, so no flag can sneak in anywhere. + blocked_args: + - pattern: '^--?config(=|$)' + message: "config overrides are not allowed" + # match: argv = one full-match regex per argument, exact argument count. + # Arguments are never joined, so spaces or newlines inside an argument + # cannot shift or widen a match. + allowed_args: + - match: argv + argv: + - 'call' + - 'front\.(read_conversation|create_draft|update_draft|add_comment|tag_conversation|list_tags|list_teammates|assign_conversation)' + - '--args' + - '(?s)\{.*\}' + message: "only allowlisted Front tools via: mcparcel call front. [--args '']" + - match: argv + argv: + - 'call' + - 'front\.(read_conversation|create_draft|update_draft|add_comment|tag_conversation|list_tags|list_teammates|assign_conversation)' diff --git a/internal/audit/audit.go b/internal/audit/audit.go index 6713370..96c5d71 100644 --- a/internal/audit/audit.go +++ b/internal/audit/audit.go @@ -4,6 +4,7 @@ package audit import ( "encoding/json" "fmt" + "io" "log" "os" "path/filepath" @@ -89,6 +90,33 @@ func (l *FileLogger) Close() error { return l.file.Close() } +// stdoutWriter is the destination of the stdout logger (swapped in tests). +var stdoutWriter io.Writer = os.Stdout + +// StreamLogger writes JSONL entries to a stream it does not own, such as the +// daemon's stdout in a container where logs are collected from the stream. +type StreamLogger struct { + mu sync.Mutex + enc *json.Encoder +} + +// NewStreamLogger returns a logger that writes JSONL to w. +func NewStreamLogger(w io.Writer) *StreamLogger { + enc := json.NewEncoder(w) + enc.SetEscapeHTML(false) + return &StreamLogger{enc: enc} +} + +// Log writes an entry as a single JSONL line. +func (l *StreamLogger) Log(entry Entry) error { + l.mu.Lock() + defer l.mu.Unlock() + return l.enc.Encode(entry) +} + +// Close is a no-op: the stream belongs to the caller. +func (l *StreamLogger) Close() error { return nil } + // MultiLogger fans out to multiple loggers. type MultiLogger struct { loggers []Logger @@ -132,6 +160,10 @@ func New(cfg *config.AuditConfig) (Logger, error) { loggers = append(loggers, fl) } + if cfg.Stdout { + loggers = append(loggers, NewStreamLogger(stdoutWriter)) + } + if cfg.Syslog { sl, err := newSyslogLogger(cfg.SyslogFacility) if err != nil { diff --git a/internal/audit/stdout_test.go b/internal/audit/stdout_test.go new file mode 100644 index 0000000..f66c152 --- /dev/null +++ b/internal/audit/stdout_test.go @@ -0,0 +1,45 @@ +package audit + +import ( + "bytes" + "encoding/json" + "strings" + "testing" + + "claw-wrap/internal/config" +) + +func TestNew_StdoutLoggerWritesJSONL(t *testing.T) { + var buf bytes.Buffer + orig := stdoutWriter + stdoutWriter = &buf + t.Cleanup(func() { stdoutWriter = orig }) + + l, err := New(&config.AuditConfig{Enabled: true, Stdout: true}) + if err != nil { + t.Fatalf("New() error = %v", err) + } + if err := l.Log(Entry{Tool: "gh", Args: []string{""}}); err != nil { + t.Fatalf("Log() error = %v", err) + } + // Closing must not close the process's stdout. + if err := l.Close(); err != nil { + t.Fatalf("Close() error = %v", err) + } + + line := strings.TrimSpace(buf.String()) + var got Entry + if err := json.Unmarshal([]byte(line), &got); err != nil { + t.Fatalf("stdout is not one JSON line: %q: %v", line, err) + } + if got.Tool != "gh" || got.Args[0] != "" { + t.Fatalf("entry = %+v", got) + } +} + +func TestValidateAudit_StdoutIsAnOutput(t *testing.T) { + cfg := &config.Config{Audit: &config.AuditConfig{Enabled: true, Stdout: true}} + if err := cfg.Validate(); err != nil { + t.Fatalf("Validate() error = %v", err) + } +} diff --git a/internal/auth/auth.go b/internal/auth/auth.go index f685ac3..8547a71 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -55,6 +55,13 @@ func GenerateSecret() ([]byte, error) { // It uses an atomic write via a temporary file to prevent partial writes. // It refuses to write if the target path is a symlink (prevents symlink attacks). func WriteSecret(path string, secret []byte) error { + return WriteSecretWithMode(path, secret, 0o600) +} + +// WriteSecretWithMode writes the secret to the specified path with the provided mode. +// It uses an atomic write via a temporary file to prevent partial writes. +// It refuses to write if the target path is a symlink (prevents symlink attacks). +func WriteSecretWithMode(path string, secret []byte, mode os.FileMode) error { dir := filepath.Dir(path) if err := os.MkdirAll(dir, 0700); err != nil { return fmt.Errorf("failed to create directory %s: %w", dir, err) @@ -82,7 +89,7 @@ func WriteSecret(path string, secret []byte) error { }() // Set permissions before writing content - if err := tmpFile.Chmod(0600); err != nil { + if err := tmpFile.Chmod(mode); err != nil { tmpFile.Close() return fmt.Errorf("failed to set permissions: %w", err) } diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 3e26ebf..0b67d5f 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -359,6 +359,32 @@ func TestWriteSecret_LoadSecret_Roundtrip(t *testing.T) { } } +func TestWriteSecretWithMode_CustomMode(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "auth-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + secretPath := filepath.Join(tmpDir, "auth-secret") + secret, err := GenerateSecret() + if err != nil { + t.Fatalf("GenerateSecret() error = %v", err) + } + + if err := WriteSecretWithMode(secretPath, secret, 0o640); err != nil { + t.Fatalf("WriteSecretWithMode() error = %v", err) + } + + info, err := os.Stat(secretPath) + if err != nil { + t.Fatalf("Failed to stat secret file: %v", err) + } + if got := info.Mode().Perm(); got != 0o640 { + t.Fatalf("WriteSecretWithMode() file mode = %o, want 0640", got) + } +} + func TestLoadSecret_NotFound(t *testing.T) { _, err := LoadSecret("/nonexistent/path/to/secret") if err != ErrSecretNotFound { diff --git a/internal/config/args_rules.go b/internal/config/args_rules.go new file mode 100644 index 0000000..ce0d9be --- /dev/null +++ b/internal/config/args_rules.go @@ -0,0 +1,66 @@ +package config + +import ( + "fmt" + "regexp" + "strings" +) + +// compileArgRule normalizes the match mode of a blocked_args/allowed_args rule +// and compiles its patterns. field names the list for error messages. +func compileArgRule(rule *BlockedArg, field string, allowArgv bool) error { + matchMode := strings.TrimSpace(rule.Match) + if matchMode == "" { + matchMode = BlockedArgMatchArg + } + + switch matchMode { + case BlockedArgMatchArg, BlockedArgMatchCommand: + if len(rule.Argv) > 0 { + return fmt.Errorf("%s: argv is only valid with match: argv", field) + } + re, err := regexp.Compile(rule.Pattern) + if err != nil { + return fmt.Errorf("invalid %s pattern %q: %w", field, rule.Pattern, err) + } + rule.Compiled = re + + case BlockedArgMatchArgv: + if !allowArgv { + return fmt.Errorf("%s: match %q is only supported in allowed_args", field, BlockedArgMatchArgv) + } + if rule.Pattern != "" { + return fmt.Errorf("%s: use either pattern or argv, not both", field) + } + if len(rule.Argv) == 0 { + return fmt.Errorf("%s: match %q requires a non-empty argv list", field, BlockedArgMatchArgv) + } + compiled := make([]*regexp.Regexp, len(rule.Argv)) + for i, p := range rule.Argv { + // Compile the bare pattern first: only a pattern that is valid on + // its own is a closed group, so wrapping cannot be escaped with + // unbalanced parens (e.g. "a)|(.*"). + if _, err := regexp.Compile(p); err != nil { + return fmt.Errorf("invalid %s argv[%d] %q: %w", field, i, p, err) + } + // Anchor every position so a pattern can never match a prefix, + // suffix, or an argument that smuggles extra words or newlines. + re, err := regexp.Compile(`^(?:` + p + `)$`) + if err != nil { + return fmt.Errorf("invalid %s argv[%d] %q: %w", field, i, p, err) + } + compiled[i] = re + } + rule.CompiledArgv = compiled + + default: + valid := fmt.Sprintf("%q or %q", BlockedArgMatchArg, BlockedArgMatchCommand) + if allowArgv { + valid = fmt.Sprintf("%q, %q or %q", BlockedArgMatchArg, BlockedArgMatchCommand, BlockedArgMatchArgv) + } + return fmt.Errorf("invalid %s match %q (must be %s)", field, rule.Match, valid) + } + + rule.Match = matchMode + return nil +} diff --git a/internal/config/args_rules_test.go b/internal/config/args_rules_test.go new file mode 100644 index 0000000..7badad8 --- /dev/null +++ b/internal/config/args_rules_test.go @@ -0,0 +1,151 @@ +package config + +import ( + "strings" + "testing" +) + +func TestLoad_AllowedArgsArgv(t *testing.T) { + cfg, err := loadYAML(t, ` +tools: + t: + binary: /usr/bin/true + mode: allowlist + allowed_args: + - match: argv + argv: ['call', 'front\.(read|list)', '--args', '(?s)\{.*\}'] +`) + if err != nil { + t.Fatalf("Load() error = %v", err) + } + + rule := cfg.Tools["t"].AllowedArgs[0] + if rule.Match != BlockedArgMatchArgv { + t.Fatalf("Match = %q, want %q", rule.Match, BlockedArgMatchArgv) + } + if len(rule.CompiledArgv) != 4 { + t.Fatalf("CompiledArgv has %d entries, want 4", len(rule.CompiledArgv)) + } + + // Each position is a full match: no prefix/suffix slack. + if !rule.CompiledArgv[1].MatchString("front.read") { + t.Error("position 1 should match front.read") + } + for _, s := range []string{"front.readX", "xfront.read", "front.read\n", "front.read --config"} { + if rule.CompiledArgv[1].MatchString(s) { + t.Errorf("position 1 must not match %q", s) + } + } + if !rule.CompiledArgv[3].MatchString("{\n \"a\": 1\n}") { + t.Error("(?s) JSON pattern should match multi-line JSON") + } +} + +func TestValidate_ArgvRuleErrors(t *testing.T) { + tests := []struct { + name string + yaml string + wantErr string + }{ + { + name: "argv without patterns", + yaml: ` +tools: + t: + binary: /usr/bin/true + mode: allowlist + allowed_args: + - match: argv +`, + wantErr: "match \"argv\" requires a non-empty argv list", + }, + { + name: "argv with pattern", + yaml: ` +tools: + t: + binary: /usr/bin/true + mode: allowlist + allowed_args: + - match: argv + pattern: call + argv: [call] +`, + wantErr: "use either pattern or argv", + }, + { + name: "argv list on non-argv rule", + yaml: ` +tools: + t: + binary: /usr/bin/true + mode: allowlist + allowed_args: + - match: command + argv: [call] +`, + wantErr: "argv is only valid with match: argv", + }, + { + name: "invalid argv regex", + yaml: ` +tools: + t: + binary: /usr/bin/true + mode: allowlist + allowed_args: + - match: argv + argv: ['call', '('] +`, + wantErr: "invalid allowed_args argv[1]", + }, + { + // Wrapped naively as ^(?:call)|(.*)$ this would match anything. + name: "unbalanced parens cannot escape the anchors", + yaml: ` +tools: + t: + binary: /usr/bin/true + mode: allowlist + allowed_args: + - match: argv + argv: ['call)|(.*'] +`, + wantErr: "invalid allowed_args argv[0]", + }, + { + name: "argv in blocked_args", + yaml: ` +tools: + t: + binary: /usr/bin/true + blocked_args: + - match: argv + argv: [call] +`, + wantErr: "match \"argv\" is only supported in allowed_args", + }, + { + name: "unknown match mode lists argv", + yaml: ` +tools: + t: + binary: /usr/bin/true + mode: allowlist + allowed_args: + - match: positional + pattern: x +`, + wantErr: "\"argv\"", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := loadYAML(t, tt.yaml) + if err == nil || !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("Load() error = %v, want containing %q", err, tt.wantErr) + } + }) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index dae99c2..fa06349 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -133,6 +133,7 @@ type InjectSpec struct { type AuditConfig struct { Enabled bool `yaml:"enabled"` File string `yaml:"file"` + Stdout bool `yaml:"stdout"` // JSONL to the daemon's stdout (containers) IncludeArgs *bool `yaml:"include_args"` IncludeOutputHash *bool `yaml:"include_output_hash"` IncludeDuration *bool `yaml:"include_duration"` @@ -248,19 +249,34 @@ type ToolDef struct { AllowedArgs []BlockedArg `yaml:"allowed_args,omitempty"` RedactOutput []ToolRedactRule `yaml:"redact_output,omitempty"` ConfigFile *ConfigFileDef `yaml:"config_file,omitempty"` - UseProxy bool `yaml:"use_proxy,omitempty"` // Enable HTTP proxy for this tool - UsePTY *bool `yaml:"use_pty,omitempty"` // PTY mode: nil=default on, false=opt out + UseProxy bool `yaml:"use_proxy,omitempty"` // Enable HTTP proxy for this tool + UsePTY *bool `yaml:"use_pty,omitempty"` // PTY mode: nil=default on, false=opt out + UseStdin *bool `yaml:"use_stdin,omitempty"` // nil=default on; false=stdin is /dev/null + WorkingDir string `yaml:"working_dir,omitempty"` // fixed cwd; caller cwd is ignored when set + // RequestEnv allowlists env vars the caller may set. nil = any var not on + // the deny list (legacy); empty = none. + RequestEnv []string `yaml:"request_env,omitempty"` } // GetUsePTY returns whether PTY mode is enabled for this tool. -// Defaults to true when not explicitly configured. +// Defaults to true when not explicitly configured, unless stdin is disabled: +// a PTY is an interactive terminal, which contradicts use_stdin: false. func (t ToolDef) GetUsePTY() bool { if t.UsePTY == nil { - return true + return t.GetUseStdin() } return *t.UsePTY } +// GetUseStdin returns whether the caller's stdin is forwarded to the tool. +// Defaults to true. When false the tool reads from /dev/null. +func (t ToolDef) GetUseStdin() bool { + if t.UseStdin == nil { + return true + } + return *t.UseStdin +} + // ToolRedactRule defines an output redaction rule for tool stdout/stderr. type ToolRedactRule struct { Pattern string `yaml:"pattern"` @@ -268,12 +284,14 @@ type ToolRedactRule struct { Compiled *regexp.Regexp `yaml:"-"` } -// BlockedArg defines a blocked argument pattern. +// BlockedArg defines an argument rule, used by both blocked_args and allowed_args. type BlockedArg struct { - Pattern string `yaml:"pattern"` - Message string `yaml:"message"` - Match string `yaml:"match,omitempty"` // arg (default) or command - Compiled *regexp.Regexp `yaml:"-"` // compiled at validation time + Pattern string `yaml:"pattern"` + Message string `yaml:"message"` + Match string `yaml:"match,omitempty"` // arg (default), command, or argv (allowed_args only) + Argv []string `yaml:"argv,omitempty"` // match: argv — one full-match pattern per position + Compiled *regexp.Regexp `yaml:"-"` // compiled at validation time + CompiledArgv []*regexp.Regexp `yaml:"-"` // compiled argv patterns, anchored ^(?:p)$ } // ConfigFileDef defines a temporary config file to generate. @@ -289,6 +307,9 @@ const ( BlockedArgMatchArg = "arg" // BlockedArgMatchCommand matches against strings.Join(args, " "). BlockedArgMatchCommand = "command" + // BlockedArgMatchArgv matches args positionally: exactly one argument per + // argv pattern, each pattern a full match. Only valid in allowed_args. + BlockedArgMatchArgv = "argv" // ToolModeBlocklist rejects commands matching blocked_args (default). ToolModeBlocklist = "blocklist" @@ -360,6 +381,25 @@ func (c *Config) Validate() error { log.Printf("[WARN] tool %q: binary %q not found on disk: %v", toolName, tool.Binary, err) } + if tool.WorkingDir != "" { + if !filepath.IsAbs(tool.WorkingDir) { + return fmt.Errorf("tool %q: working_dir must be absolute: %q", toolName, tool.WorkingDir) + } + if info, err := os.Stat(tool.WorkingDir); err != nil || !info.IsDir() { + log.Printf("[WARN] tool %q: working_dir %q is not an existing directory", toolName, tool.WorkingDir) + } + } + + for _, name := range tool.RequestEnv { + if !envVarNameRegex.MatchString(name) { + return fmt.Errorf("tool %q: invalid request_env name %q", toolName, name) + } + } + + if tool.UsePTY != nil && *tool.UsePTY && !tool.GetUseStdin() { + return fmt.Errorf("tool %q: use_pty: true conflicts with use_stdin: false", toolName) + } + // Build credential names set for validation credNames := CredentialNamesSet(c.Credentials) @@ -397,23 +437,10 @@ func (c *Config) Validate() error { } // Compile blocked_args regex patterns. - for i, b := range tool.BlockedArgs { - matchMode := strings.TrimSpace(b.Match) - if matchMode == "" { - matchMode = BlockedArgMatchArg + for i := range tool.BlockedArgs { + if err := compileArgRule(&c.Tools[toolName].BlockedArgs[i], "blocked_args", false); err != nil { + return fmt.Errorf("tool %q: %w", toolName, err) } - switch matchMode { - case BlockedArgMatchArg, BlockedArgMatchCommand: - default: - return fmt.Errorf("tool %q: invalid blocked_args match %q (must be %q or %q)", toolName, b.Match, BlockedArgMatchArg, BlockedArgMatchCommand) - } - - re, err := regexp.Compile(b.Pattern) - if err != nil { - return fmt.Errorf("tool %q: invalid blocked_args pattern %q: %w", toolName, b.Pattern, err) - } - c.Tools[toolName].BlockedArgs[i].Match = matchMode - c.Tools[toolName].BlockedArgs[i].Compiled = re } // Validate and normalize mode. @@ -438,23 +465,10 @@ func (c *Config) Validate() error { } // Compile allowed_args regex patterns. - for i, a := range tool.AllowedArgs { - matchMode := strings.TrimSpace(a.Match) - if matchMode == "" { - matchMode = BlockedArgMatchArg - } - switch matchMode { - case BlockedArgMatchArg, BlockedArgMatchCommand: - default: - return fmt.Errorf("tool %q: invalid allowed_args match %q (must be %q or %q)", toolName, a.Match, BlockedArgMatchArg, BlockedArgMatchCommand) - } - - re, err := regexp.Compile(a.Pattern) - if err != nil { - return fmt.Errorf("tool %q: invalid allowed_args pattern %q: %w", toolName, a.Pattern, err) + for i := range tool.AllowedArgs { + if err := compileArgRule(&c.Tools[toolName].AllowedArgs[i], "allowed_args", true); err != nil { + return fmt.Errorf("tool %q: %w", toolName, err) } - c.Tools[toolName].AllowedArgs[i].Match = matchMode - c.Tools[toolName].AllowedArgs[i].Compiled = re } // Store normalized mode. @@ -1184,8 +1198,8 @@ var validSyslogFacilities = map[string]bool{ func (c *Config) validateAudit() error { a := c.Audit - if a.File == "" && !a.Syslog { - return fmt.Errorf("at least one output (file or syslog) must be configured") + if a.File == "" && !a.Syslog && !a.Stdout { + return fmt.Errorf("at least one output (file, stdout or syslog) must be configured") } if a.File != "" && !filepath.IsAbs(a.File) { return fmt.Errorf("file path must be absolute: %q", a.File) diff --git a/internal/config/tool_options_test.go b/internal/config/tool_options_test.go new file mode 100644 index 0000000..69c846a --- /dev/null +++ b/internal/config/tool_options_test.go @@ -0,0 +1,128 @@ +package config + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func loadYAML(t *testing.T, content string) (*Config, error) { + t.Helper() + path := filepath.Join(t.TempDir(), "wrappers.yaml") + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatalf("write config: %v", err) + } + return Load(path) +} + +func TestLoad_WorkingDirAndUseStdin(t *testing.T) { + cfg, err := loadYAML(t, ` +tools: + batch: + binary: /usr/bin/true + working_dir: /tmp + use_stdin: false + plain: + binary: /usr/bin/true +`) + if err != nil { + t.Fatalf("Load() error = %v", err) + } + + batch := cfg.Tools["batch"] + if batch.WorkingDir != "/tmp" { + t.Errorf("WorkingDir = %q, want /tmp", batch.WorkingDir) + } + if batch.GetUseStdin() { + t.Error("GetUseStdin() = true, want false") + } + if batch.GetUsePTY() { + t.Error("use_stdin: false must imply no PTY when use_pty is unset") + } + + plain := cfg.Tools["plain"] + if !plain.GetUseStdin() || !plain.GetUsePTY() { + t.Error("defaults must keep stdin and PTY enabled") + } +} + +func TestLoad_RequestEnv(t *testing.T) { + cfg, err := loadYAML(t, ` +tools: + none: + binary: /usr/bin/true + request_env: [] + term: + binary: /usr/bin/true + request_env: [TERM, COLORTERM] + legacy: + binary: /usr/bin/true +`) + if err != nil { + t.Fatalf("Load() error = %v", err) + } + if got := cfg.Tools["none"].RequestEnv; got == nil || len(got) != 0 { + t.Errorf("request_env: [] must load as an empty, non-nil allowlist, got %#v", got) + } + if got := cfg.Tools["term"].RequestEnv; len(got) != 2 { + t.Errorf("request_env = %#v", got) + } + if cfg.Tools["legacy"].RequestEnv != nil { + t.Error("unset request_env must stay nil (legacy behaviour)") + } +} + +func TestValidate_ToolOptionErrors(t *testing.T) { + tests := []struct { + name string + yaml string + wantErr string + }{ + { + name: "relative working_dir", + yaml: ` +tools: + t: + binary: /usr/bin/true + working_dir: tmp +`, + wantErr: "working_dir must be absolute", + }, + { + name: "use_pty true with use_stdin false", + yaml: ` +tools: + t: + binary: /usr/bin/true + use_pty: true + use_stdin: false +`, + wantErr: "use_pty: true conflicts with use_stdin: false", + }, + } + + tests = append(tests, struct { + name string + yaml string + wantErr string + }{ + name: "invalid request_env name", + yaml: ` +tools: + t: + binary: /usr/bin/true + request_env: ["BAD-NAME"] +`, + wantErr: "invalid request_env name", + }) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := loadYAML(t, tt.yaml) + if err == nil || !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("Load() error = %v, want containing %q", err, tt.wantErr) + } + }) + } +} diff --git a/internal/daemon/args_policy.go b/internal/daemon/args_policy.go index 07d06e9..580b3ce 100644 --- a/internal/daemon/args_policy.go +++ b/internal/daemon/args_policy.go @@ -2,6 +2,7 @@ package daemon import ( "log" + "regexp" "strings" "claw-wrap/internal/config" @@ -74,6 +75,17 @@ func checkAllowedArgs(args []string, allowed []config.BlockedArg) (bool, string) joinedArgs := strings.Join(args, " ") for _, a := range allowed { + if a.Match == config.BlockedArgMatchArgv { + if len(a.CompiledArgv) == 0 || len(a.CompiledArgv) != len(a.Argv) { + log.Printf("[ERROR] uncompiled argv rule in allowed_args %q - fail-closed", a.Argv) + return false, "internal error: invalid security pattern" + } + if matchArgv(args, a.CompiledArgv) { + return true, "" + } + continue + } + if a.Compiled == nil { log.Printf("[ERROR] nil compiled pattern for allowed_args %q - fail-closed", a.Pattern) return false, "internal error: invalid security pattern" @@ -100,6 +112,21 @@ func checkAllowedArgs(args []string, allowed []config.BlockedArg) (bool, string) return false, allowedArgMessage(allowed) } +// matchArgv reports whether args has exactly one argument per pattern and +// every argument fully matches the pattern at its position. Patterns are +// anchored at config load time. +func matchArgv(args []string, patterns []*regexp.Regexp) bool { + if len(args) != len(patterns) { + return false + } + for i, re := range patterns { + if !re.MatchString(args[i]) { + return false + } + } + return true +} + func allowedArgMessage(allowed []config.BlockedArg) string { for _, a := range allowed { if a.Message != "" { diff --git a/internal/daemon/configwatcher_configmap_test.go b/internal/daemon/configwatcher_configmap_test.go new file mode 100644 index 0000000..5659012 --- /dev/null +++ b/internal/daemon/configwatcher_configmap_test.go @@ -0,0 +1,75 @@ +package daemon + +import ( + "os" + "path/filepath" + "testing" + "time" +) + +// Kubernetes mounts a ConfigMap as symlinks into a timestamped directory: +// +// wrappers.yaml -> ..data/wrappers.yaml +// ..data -> ..2026_10_07_10_00_00.000000001 +// +// An update writes a new timestamped dir and atomically renames a fresh +// ..data_tmp symlink over ..data. No event names wrappers.yaml itself. +func TestConfigWatcher_ReloadsOnConfigMapSymlinkSwap(t *testing.T) { + dir := t.TempDir() + + writeVersion := func(name, content string) { + t.Helper() + if err := os.Mkdir(filepath.Join(dir, name), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, name, "wrappers.yaml"), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + + writeVersion("..v1", "tools:\n gh:\n binary: /usr/bin/gh\n") + if err := os.Symlink("..v1", filepath.Join(dir, "..data")); err != nil { + t.Fatal(err) + } + configPath := filepath.Join(dir, "wrappers.yaml") + if err := os.Symlink("..data/wrappers.yaml", configPath); err != nil { + t.Fatal(err) + } + + d := New(WithConfigPath(configPath)) + if err := d.reloadConfig(); err != nil { + t.Fatalf("initial reloadConfig: %v", err) + } + if err := d.startConfigWatcher(); err != nil { + t.Fatalf("startConfigWatcher: %v", err) + } + defer d.stopConfigWatcher() + + // Same sequence as kubelet's AtomicWriter. + writeVersion("..v2", "tools:\n gh:\n binary: /usr/bin/gh\n mcparcel:\n binary: /usr/local/bin/mcparcel\n") + if err := os.Symlink("..v2", filepath.Join(dir, "..data_tmp")); err != nil { + t.Fatal(err) + } + if err := os.Rename(filepath.Join(dir, "..data_tmp"), filepath.Join(dir, "..data")); err != nil { + t.Fatal(err) + } + if err := os.RemoveAll(filepath.Join(dir, "..v1")); err != nil { + t.Fatal(err) + } + + deadline := time.After(3 * time.Second) + tick := time.NewTicker(50 * time.Millisecond) + defer tick.Stop() + for { + select { + case <-deadline: + t.Fatal("ConfigMap-style update did not trigger a reload") + case <-tick.C: + if cfg := d.getConfig(); cfg != nil { + if _, ok := cfg.Tools["mcparcel"]; ok { + return + } + } + } + } +} diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index cae982c..5d6d3d0 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -34,6 +34,13 @@ import ( // DefaultSocketPath is the default Unix socket path. var DefaultSocketPath = paths.SocketPath() +const ( + defaultAuthFileMode = os.FileMode(0o600) + defaultSocketFileMode = os.FileMode(0o600) + defaultRuntimeDirMode = os.FileMode(0o700) + sharedRuntimeDirMode = os.FileMode(0o750) +) + // Ucred holds Unix peer credentials. type Ucred struct { PID int32 @@ -46,6 +53,9 @@ type Daemon struct { socketPath string configPath string allowedUID uint32 + runtimeGID int + authFileMode os.FileMode + socketFileMode os.FileMode allowedBinaries []string version string listener net.Listener @@ -66,6 +76,8 @@ type Daemon struct { configWatcherWg sync.WaitGroup configWatcherMu sync.Mutex configWatcherInit bool + + unverifiedExeOnce sync.Once configStopCh chan struct{} } @@ -112,6 +124,21 @@ func WithAllowedUID(uid uint32) Option { return func(d *Daemon) { d.allowedUID = uid } } +// WithRuntimeGID sets an optional shared GID for runtime artifacts. +func WithRuntimeGID(gid int) Option { + return func(d *Daemon) { d.runtimeGID = gid } +} + +// WithAuthFileMode sets the mode used for the HMAC auth file. +func WithAuthFileMode(mode os.FileMode) Option { + return func(d *Daemon) { d.authFileMode = mode } +} + +// WithSocketFileMode sets the mode used for the Unix socket. +func WithSocketFileMode(mode os.FileMode) Option { + return func(d *Daemon) { d.socketFileMode = mode } +} + // WithAllowedBinaries sets the allowed binary paths. func WithAllowedBinaries(binaries []string) Option { return func(d *Daemon) { d.allowedBinaries = binaries } @@ -151,6 +178,9 @@ func New(opts ...Option) *Daemon { socketPath: DefaultSocketPath, configPath: config.DefaultConfigPath, allowedUID: uint32(os.Getuid()), + runtimeGID: -1, + authFileMode: defaultAuthFileMode, + socketFileMode: defaultSocketFileMode, allowedBinaries: allowed, metrics: newSecurityMetrics(), proxyAuthTokenPath: paths.ProxyAuthTokenPath(), @@ -163,11 +193,25 @@ func New(opts ...Option) *Daemon { // Run starts the daemon and blocks until shutdown. func (d *Daemon) Run() error { + if err := paths.ValidateRuntimeDirEnv(); err != nil { + return err + } + if d.runtimeGID < -1 { + return fmt.Errorf("runtime gid must be >= 0 when set") + } + if !IsAllowedAuthFileMode(d.authFileMode) { + return fmt.Errorf("invalid auth file mode %04o", d.authFileMode) + } + if !IsAllowedSocketFileMode(d.socketFileMode) { + return fmt.Errorf("invalid socket file mode %04o", d.socketFileMode) + } + cfg, err := config.Load(d.configPath) if err != nil { return fmt.Errorf("load config: %w", err) } log.Printf("[INFO] Loaded %d credentials from config", len(cfg.Credentials)) + warnIfNoTools(cfg) setAgeIdentityFileFunc(cfg.GetAgeIdentityFile()) setOPTokenFileFunc(cfg.GetOPTokenFile()) setCredentialCacheTTLFunc(cfg.GetCredentialCacheTTL()) @@ -192,16 +236,19 @@ func (d *Daemon) Run() error { d.secret = secret secretPath := cfg.GetHMACSecretFile() - if err := auth.WriteSecret(secretPath, secret); err != nil { + if err := auth.WriteSecretWithMode(secretPath, secret, d.authFileMode); err != nil { return fmt.Errorf("write HMAC secret: %w", err) } + if err := d.applyRuntimePathOwnership(secretPath); err != nil { + return fmt.Errorf("chgrp auth file: %w", err) + } log.Printf("[INFO] HMAC secret written to %s", secretPath) runtimeDir := paths.RuntimeDir() - if err := os.MkdirAll(runtimeDir, 0o700); err != nil { + if err := os.MkdirAll(runtimeDir, d.runtimeDirMode()); err != nil { return fmt.Errorf("create runtime dir: %w", err) } - if err := os.Chmod(runtimeDir, 0o700); err != nil { + if err := d.applyRuntimeDirPermissions(runtimeDir); err != nil { return fmt.Errorf("chmod runtime dir: %w", err) } @@ -217,9 +264,12 @@ func (d *Daemon) Run() error { } d.listener = listener - if err := os.Chmod(d.socketPath, 0o600); err != nil { + if err := os.Chmod(d.socketPath, d.socketFileMode); err != nil { return fmt.Errorf("chmod socket: %w", err) } + if err := d.applyRuntimePathOwnership(d.socketPath); err != nil { + return fmt.Errorf("chgrp socket: %w", err) + } d.cfgMu.Lock() d.cfg = cfg @@ -292,6 +342,51 @@ func (d *Daemon) Run() error { } } +func (d *Daemon) runtimeDirMode() os.FileMode { + if d.runtimeGID >= 0 { + return sharedRuntimeDirMode + } + return defaultRuntimeDirMode +} + +func (d *Daemon) applyRuntimeDirPermissions(path string) error { + if err := d.applyRuntimePathOwnership(path); err != nil { + return err + } + info, err := os.Stat(path) + if err != nil { + return err + } + if info.Mode().Perm() == d.runtimeDirMode() { + return nil + } + if err := os.Chmod(path, d.runtimeDirMode()); err != nil { + return err + } + return nil +} + +func (d *Daemon) applyRuntimePathOwnership(path string) error { + if d.runtimeGID < 0 { + return nil + } + + info, err := os.Stat(path) + if err != nil { + return err + } + if stat, ok := info.Sys().(*syscall.Stat_t); ok { + if int(stat.Gid) == d.runtimeGID { + return nil + } + } + + if err := os.Chown(path, -1, d.runtimeGID); err != nil { + return err + } + return nil +} + func (d *Daemon) acquireConnSlot() bool { select { case d.connSem <- struct{}{}: @@ -313,6 +408,7 @@ func (d *Daemon) reloadConfig() error { if err != nil { return err } + warnIfNoTools(newCfg) d.cfgMu.Lock() defer d.cfgMu.Unlock() @@ -383,6 +479,18 @@ func (d *Daemon) reloadConfig() error { // Editors often emit multiple events per save (rename+create, write+chmod). const configWatchDebounce = 500 * time.Millisecond +// warnIfNoTools flags a config that makes every wrapped call fail with +// "unknown tool". +func warnIfNoTools(cfg *config.Config) { + if len(cfg.Tools) == 0 { + log.Printf("[WARN] no tools configured: every wrapped call will be rejected as an unknown tool") + } +} + +// kubernetesDataLink is the symlink kubelet swaps when a ConfigMap or Secret +// volume changes. +const kubernetesDataLink = "..data" + // startConfigWatcher starts an fsnotify watcher on the config file directory. // It calls reloadConfig when the config file changes. Non-fatal if it fails. func (d *Daemon) startConfigWatcher() error { @@ -442,7 +550,9 @@ func (d *Daemon) configWatchLoop(stopCh <-chan struct{}, events <-chan fsnotify. return } - if filepath.Base(event.Name) != configFile { + // Kubernetes ConfigMap volumes update by atomically swapping the + // ..data symlink; no event names the config file itself. + if base := filepath.Base(event.Name); base != configFile && base != kubernetesDataLink { continue } if event.Op&(fsnotify.Write|fsnotify.Create|fsnotify.Chmod|fsnotify.Rename) == 0 { @@ -641,6 +751,9 @@ func (d *Daemon) handleConnection(conn net.Conn, cfg *config.Config) { log.Printf("[DEBUG] peer pid=%d uid=%d exe=%s", ucred.PID, ucred.UID, callerExe) } else { log.Printf("[DEBUG] peer pid=%d uid=%d exe=unverified", ucred.PID, ucred.UID) + d.unverifiedExeOnce.Do(func() { + log.Printf("[INFO] caller executable cannot be verified (peer pid=%d, e.g. a separate PID namespace); requests are authorized by UID + HMAC only", ucred.PID) + }) } switch { @@ -1128,6 +1241,7 @@ func auditConfigChanged(old, new *config.Config) bool { } return oa.Enabled != na.Enabled || oa.File != na.File || + oa.Stdout != na.Stdout || oa.Syslog != na.Syslog || oa.SyslogFacility != na.SyslogFacility } diff --git a/internal/daemon/executor.go b/internal/daemon/executor.go index d1bae7f..50fc114 100644 --- a/internal/daemon/executor.go +++ b/internal/daemon/executor.go @@ -16,8 +16,10 @@ import ( "os/exec" "os/user" "path/filepath" + "slices" "strings" "sync" + "sync/atomic" "syscall" "time" @@ -119,6 +121,9 @@ type ToolExecutor struct { cmd *exec.Cmd pgid int stdinPipe io.WriteCloser + draining atomic.Bool // set once the tool has exited; pumpers then read under a deadline + stdoutR *os.File // read end of the stdout pipe (pipe mode) + stderrR *os.File // read end of the stderr pipe (pipe mode) ctx context.Context cancel context.CancelFunc @@ -133,6 +138,9 @@ type ToolExecutor struct { sendMu sync.Mutex configDir string // temp dir for config file injection + workDir string // effective cwd: tool.WorkingDir or req.Cwd + + secretValues []string // resolved credential values, redacted from output stdoutBuf *OutputBuffer stderrBuf *OutputBuffer @@ -193,16 +201,23 @@ func (e *ToolExecutor) Run() error { defer e.cleanup() e.startTime = time.Now() + // A configured working_dir wins over the caller's cwd, which may not exist + // on the daemon side (e.g. when the daemon runs in a sidecar container). + e.workDir = e.req.Cwd + if e.tool.WorkingDir != "" { + e.workDir = e.tool.WorkingDir + } + // Validate working directory is absolute - if !filepath.IsAbs(e.req.Cwd) { - log.Printf("[ERROR] working directory is not absolute: %s", e.req.Cwd) + if !filepath.IsAbs(e.workDir) { + log.Printf("[ERROR] working directory is not absolute: %s", e.workDir) e.sendError("invalid working directory") - return fmt.Errorf("relative working directory: %s", e.req.Cwd) + return fmt.Errorf("relative working directory: %s", e.workDir) } // Verify working directory exists - if _, err := os.Stat(e.req.Cwd); os.IsNotExist(err) { - log.Printf("[ERROR] working directory does not exist: %s", e.req.Cwd) + if _, err := os.Stat(e.workDir); os.IsNotExist(err) { + log.Printf("[ERROR] working directory does not exist: %s", e.workDir) e.sendError("invalid working directory") return err } @@ -288,6 +303,7 @@ func (e *ToolExecutor) buildEnvironment() ([]string, error) { if value == "" { return "", fmt.Errorf("credential %s returned empty value", name) } + e.secretValues = append(e.secretValues, value) return value, nil } @@ -321,6 +337,10 @@ func (e *ToolExecutor) buildEnvironment() ([]string, error) { log.Printf("[WARN] Request attempted to override forced_env key %q, ignoring", k) continue } + if e.tool.RequestEnv != nil && !slices.Contains(e.tool.RequestEnv, k) { + log.Printf("[DEBUG] request env %q not in request_env allowlist, ignoring", k) + continue + } envMap[k] = v if k == "TERM" { termFromReq = true @@ -355,6 +375,7 @@ func (e *ToolExecutor) buildEnvironment() ([]string, error) { if err != nil { return nil, fmt.Errorf("build proxy URL: %w", err) } + e.secretValues = append(e.secretValues, e.proxyAuthToken) } else { // No auth required - use simple URL without credentials proxyURL = "http://" + e.cfg.GetHTTPProxyListen() @@ -447,6 +468,7 @@ func (e *ToolExecutor) setupConfigFile() error { return fmt.Errorf("empty credential: %s", credName) } credValues[credName] = value + e.secretValues = append(e.secretValues, value) } // Render template @@ -471,7 +493,7 @@ func (e *ToolExecutor) startProcess(env []string) error { } e.cmd = exec.CommandContext(e.ctx, e.tool.Binary, e.req.Args...) - e.cmd.Dir = e.req.Cwd + e.cmd.Dir = e.workDir e.cmd.Env = env // Create new process group so we can kill all children @@ -480,29 +502,43 @@ func (e *ToolExecutor) startProcess(env []string) error { Pgid: 0, } - // Set up pipes - stdout, err := e.cmd.StdoutPipe() + // Own the output pipes instead of using StdoutPipe/StderrPipe: Wait() + // closes those read ends as soon as the process exits, dropping output the + // pumpers have not consumed yet. With our own pipes the pumpers read to EOF + // (bounded by outputDrainGrace once the process has exited). + stdout, stdoutW, err := os.Pipe() if err != nil { return fmt.Errorf("stdout pipe: %w", err) } - - stderr, err := e.cmd.StderrPipe() + stderr, stderrW, err := os.Pipe() if err != nil { + stdout.Close() + stdoutW.Close() return fmt.Errorf("stderr pipe: %w", err) } + e.stdoutR, e.stderrR = stdout, stderr + e.cmd.Stdout = stdoutW + e.cmd.Stderr = stderrW - stdin, err := e.cmd.StdinPipe() - if err != nil { - return fmt.Errorf("stdin pipe: %w", err) + // use_stdin: false leaves cmd.Stdin nil, so the tool reads /dev/null and a + // prompt fails fast instead of waiting for input that never comes. Stdin + // messages from the wrapper are then dropped; signals still pass through. + if e.tool.GetUseStdin() { + stdin, err := e.cmd.StdinPipe() + if err != nil { + stdoutW.Close() + stderrW.Close() + return fmt.Errorf("stdin pipe: %w", err) + } + e.stdinPipe = stdin } - e.stdinPipe = stdin // Create output buffers e.stdoutBuf = NewOutputBuffer("stdout", e.threshold, e.maxOutSz, e.sendMessage) e.stderrBuf = NewOutputBuffer("stderr", e.threshold, e.maxOutSz, e.sendMessage) - if len(e.tool.RedactOutput) > 0 { - e.stdoutBuf.SetRedactor(NewOutputRedactor(e.tool.RedactOutput)) - e.stderrBuf.SetRedactor(NewOutputRedactor(e.tool.RedactOutput)) + if r := NewOutputRedactorWithSecrets(e.tool.RedactOutput, e.secretValues); r != nil { + e.stdoutBuf.SetRedactor(r) + e.stderrBuf.SetRedactor(NewOutputRedactorWithSecrets(e.tool.RedactOutput, e.secretValues)) } // Wire up SHA256 hashers for audit output hash @@ -514,7 +550,12 @@ func (e *ToolExecutor) startProcess(env []string) error { } // Start the process - if err := e.cmd.Start(); err != nil { + err = e.cmd.Start() + // The child holds its own copies of the write ends; close ours so the + // pumpers see EOF once every writer is gone. + stdoutW.Close() + stderrW.Close() + if err != nil { return fmt.Errorf("start: %w", err) } @@ -535,7 +576,7 @@ func (e *ToolExecutor) startProcess(env []string) error { // This enables interactive TUI applications to work correctly with colors and cursor control. func (e *ToolExecutor) startProcessWithPTY(env []string) error { e.cmd = exec.CommandContext(e.ctx, e.tool.Binary, e.req.Args...) - e.cmd.Dir = e.req.Cwd + e.cmd.Dir = e.workDir e.cmd.Env = env // Start with PTY, using initial window size if provided so the child @@ -564,8 +605,8 @@ func (e *ToolExecutor) startProcessWithPTY(env []string) error { // Create single output buffer for PTY (stdout and stderr are merged) // PTY mode always streams inline - no file buffering threshold e.ptyBuf = NewOutputBuffer("stdout", 0, e.maxOutSz, e.sendMessage) - if len(e.tool.RedactOutput) > 0 { - e.ptyBuf.SetRedactor(NewOutputRedactor(e.tool.RedactOutput)) + if r := NewOutputRedactorWithSecrets(e.tool.RedactOutput, e.secretValues); r != nil { + e.ptyBuf.SetRedactor(r) } // Wire up SHA256 hasher for audit output hash @@ -702,6 +743,10 @@ func (e *ToolExecutor) runIOLoop() error { } } + // Bound the drain: a detached descendant (e.g. a daemon the tool + // started) may still hold the pipe write ends, so EOF might never come. + e.setOutputDrainDeadline() + // Wait for output pumpers to finish draining pipes // Use a channel with timeout to avoid blocking forever pumpersDone := make(chan struct{}) @@ -729,13 +774,13 @@ func (e *ToolExecutor) runIOLoop() error { case <-e.ctx.Done(): // Timeout or cancellation - e.handleTimeout() + e.handleTimeout(waitDone) return e.ctx.Err() } } // stdoutPumper reads from stdout and writes to the output buffer. -func (e *ToolExecutor) stdoutPumper(r io.Reader) { +func (e *ToolExecutor) stdoutPumper(r *os.File) { defer e.pumperWg.Done() buf := make([]byte, 32*1024) // 32KB buffer @@ -750,9 +795,12 @@ func (e *ToolExecutor) stdoutPumper(r io.Reader) { } log.Printf("[WARN] stdout write: %v", writeErr) } + e.extendOutputDeadline(r) } if err != nil { - if err != io.EOF { + if errors.Is(err, os.ErrDeadlineExceeded) { + log.Printf("[WARN] tool %s exited but a descendant still holds its stdout; detach background processes from stdio", e.req.Tool) + } else if err != io.EOF { log.Printf("[DEBUG] stdout read: %v", err) } return @@ -761,7 +809,7 @@ func (e *ToolExecutor) stdoutPumper(r io.Reader) { } // stderrPumper reads from stderr and writes to the output buffer. -func (e *ToolExecutor) stderrPumper(r io.Reader) { +func (e *ToolExecutor) stderrPumper(r *os.File) { defer e.pumperWg.Done() buf := make([]byte, 32*1024) // 32KB buffer @@ -776,9 +824,12 @@ func (e *ToolExecutor) stderrPumper(r io.Reader) { } log.Printf("[WARN] stderr write: %v", writeErr) } + e.extendOutputDeadline(r) } if err != nil { - if err != io.EOF { + if errors.Is(err, os.ErrDeadlineExceeded) { + log.Printf("[WARN] tool %s exited but a descendant still holds its stderr; detach background processes from stdio", e.req.Tool) + } else if err != io.EOF { log.Printf("[DEBUG] stderr read: %v", err) } return @@ -937,35 +988,32 @@ func (e *ToolExecutor) finalizeOutput() error { return nil } - // Pipe mode uses separate stdout/stderr buffers - // Finalize stdout buffer - if e.stdoutBuf != nil { - if stdoutPath, err := e.stdoutBuf.Finalize(); err != nil { - return fmt.Errorf("finalize stdout: %w", err) - } else if stdoutPath != "" { - if err := e.streamFile(stdoutPath, protocol.MsgTypeStdout); err != nil { - return fmt.Errorf("stream stdout file: %w", err) - } - if err := os.Remove(stdoutPath); err != nil && !os.IsNotExist(err) { - log.Printf("[WARN] cleanup stdout temp file %s: %v", stdoutPath, err) - } - } - } + // Pipe mode uses separate stdout/stderr buffers. Finalize both even if one + // fails, so a stdout problem never swallows stderr. + return errors.Join( + e.finalizeStream(e.stdoutBuf, protocol.MsgTypeStdout), + e.finalizeStream(e.stderrBuf, protocol.MsgTypeStderr), + ) +} - // Finalize stderr buffer - if e.stderrBuf != nil { - if stderrPath, err := e.stderrBuf.Finalize(); err != nil { - return fmt.Errorf("finalize stderr: %w", err) - } else if stderrPath != "" { - if err := e.streamFile(stderrPath, protocol.MsgTypeStderr); err != nil { - return fmt.Errorf("stream stderr file: %w", err) - } - if err := os.Remove(stderrPath); err != nil && !os.IsNotExist(err) { - log.Printf("[WARN] cleanup stderr temp file %s: %v", stderrPath, err) - } - } +// finalizeStream flushes one pipe-mode buffer and streams its spill file. +func (e *ToolExecutor) finalizeStream(buf *OutputBuffer, streamType string) error { + if buf == nil { + return nil + } + path, err := buf.Finalize() + if err != nil { + return fmt.Errorf("finalize %s: %w", streamType, err) + } + if path == "" { + return nil + } + if err := e.streamFile(path, streamType); err != nil { + return fmt.Errorf("stream %s file: %w", streamType, err) + } + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + log.Printf("[WARN] cleanup %s temp file %s: %v", streamType, path, err) } - return nil } @@ -997,6 +1045,39 @@ func (e *ToolExecutor) streamFile(path string, streamType string) error { } } +// outputDrainGrace bounds how long output is still read after the tool has +// exited. Output the tool wrote before exiting is already buffered in the pipe +// and drains immediately; the grace only matters when a detached descendant +// inherited stdout/stderr and keeps the pipe open. +const outputDrainGrace = time.Second + +// The deadline is idle-based: each pumper pushes it out again after handling a +// chunk, so a slow client delaying sendMessage cannot cut off output that is +// still buffered in the pipe. +func (e *ToolExecutor) setOutputDrainDeadline() { + e.draining.Store(true) + for _, f := range []*os.File{e.stdoutR, e.stderrR} { + extendDrainDeadline(f) + } +} + +// extendOutputDeadline re-arms the drain deadline after a pumper handled a +// chunk, once the tool has exited. +func (e *ToolExecutor) extendOutputDeadline(f *os.File) { + if e.draining.Load() { + extendDrainDeadline(f) + } +} + +func extendDrainDeadline(f *os.File) { + if f == nil { + return + } + if err := f.SetReadDeadline(time.Now().Add(outputDrainGrace)); err != nil { + log.Printf("[DEBUG] set output drain deadline: %v", err) + } +} + // killProcessGroup sends a signal to the entire process group. func (e *ToolExecutor) killProcessGroup(sig syscall.Signal) error { if e.pgid == 0 { @@ -1009,7 +1090,9 @@ func (e *ToolExecutor) killProcessGroup(sig syscall.Signal) error { // handleTimeout gracefully terminates the process on timeout. // Sends SIGTERM first, waits 5 seconds, then SIGKILL if still running. -func (e *ToolExecutor) handleTimeout() { +// waitDone delivers the result of cmd.Wait(); it is the only safe way to +// observe process exit while Wait runs in another goroutine. +func (e *ToolExecutor) handleTimeout(waitDone <-chan error) { log.Printf("[INFO] Tool timeout after %v", e.timeout) // Send SIGTERM to process group @@ -1017,26 +1100,20 @@ func (e *ToolExecutor) handleTimeout() { log.Printf("[WARN] SIGTERM to process group: %v", err) } - // Wait up to 5 seconds for graceful shutdown - gracePeriod := 5 * time.Second - deadline := time.Now().Add(gracePeriod) - - for time.Now().Before(deadline) { - // Check if process has exited - if e.cmd.ProcessState != nil { - break - } - time.Sleep(100 * time.Millisecond) - } - - // If still running, send SIGKILL - if e.cmd.ProcessState == nil { + // Wait up to 5 seconds for graceful shutdown, then SIGKILL + select { + case <-waitDone: + case <-time.After(5 * time.Second): log.Printf("[INFO] Process still running after SIGTERM, sending SIGKILL") if err := e.killProcessGroup(syscall.SIGKILL); err != nil { log.Printf("[WARN] SIGKILL to process group: %v", err) } } + // Output written before the kill is drained; a descendant holding the + // pipes open must not stall the timeout path. + e.setOutputDrainDeadline() + // Wait for output pumpers to finish (with timeout) pumpersDone := make(chan struct{}) go func() { @@ -1073,6 +1150,14 @@ func (e *ToolExecutor) cleanup() { e.stdinPipe = nil } + // Close output pipe read ends (pipe mode) + if e.stdoutR != nil { + e.stdoutR.Close() + } + if e.stderrR != nil { + e.stderrR.Close() + } + // Close PTY master if still open (PTY mode) if e.ptyMaster != nil { e.ptyMaster.Close() @@ -1131,7 +1216,7 @@ func (e *ToolExecutor) emitAuditEntry(exitCode int, timeout bool) { entry := audit.Entry{ Timestamp: e.startTime.UTC().Format(time.RFC3339), Tool: e.req.Tool, - Cwd: e.req.Cwd, + Cwd: e.workDir, CallerPID: e.callerPID, CallerExe: e.callerExe, ExitCode: exitCode, diff --git a/internal/daemon/executor_lifecycle_test.go b/internal/daemon/executor_lifecycle_test.go new file mode 100644 index 0000000..9c5c3f3 --- /dev/null +++ b/internal/daemon/executor_lifecycle_test.go @@ -0,0 +1,216 @@ +package daemon + +import ( + "encoding/base64" + "net" + "os" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + "time" + + "claw-wrap/internal/audit" + "claw-wrap/internal/config" + "claw-wrap/internal/framing" + "claw-wrap/internal/protocol" +) + +// clientReadDelay makes the test client stall once, after the first message, +// to simulate a slow consumer. +var clientReadDelay time.Duration + +type execResult struct { + stdout string + stderr string + exitCode int + timeout bool + errMsg string + elapsed time.Duration +} + +// runHelperTool executes the test binary as a wrapped tool through a real +// ToolExecutor and collects everything the wrapper would receive. +func runHelperTool(t *testing.T, tool config.ToolDef, req protocol.ProxyRequest) execResult { + t.Helper() + return runHelperToolWith(t, tool, req, &config.Config{}, nil) +} + +func runHelperToolWith(t *testing.T, tool config.ToolDef, req protocol.ProxyRequest, cfg *config.Config, auditLogger audit.Logger) execResult { + t.Helper() + + exe, err := os.Executable() + if err != nil { + t.Fatalf("os.Executable: %v", err) + } + tool.Binary = exe + if req.Tool == "" { + req.Tool = "helper" + } + if req.Cwd == "" { + req.Cwd = t.TempDir() + } + + daemonSide, clientSide := net.Pipe() + defer clientSide.Close() + + ex, err := NewToolExecutor(daemonSide, &req, &tool, cfg, "", auditLogger, 0, "") + if err != nil { + t.Fatalf("NewToolExecutor: %v", err) + } + + start := time.Now() + runDone := make(chan struct{}) + go func() { + defer close(runDone) + _ = ex.Run() + daemonSide.Close() + }() + + var res execResult + var stdout, stderr strings.Builder + dec := framing.NewDecoder(clientSide) + readDone := make(chan struct{}) + go func() { + defer close(readDone) + stalled := false + for { + var msg protocol.ResponseMessage + if err := dec.Decode(&msg); err != nil { + return + } + if clientReadDelay > 0 && !stalled { + stalled = true + time.Sleep(clientReadDelay) + } + switch msg.Type { + case protocol.MsgTypeStdout, protocol.MsgTypeStderr: + data, err := base64.StdEncoding.DecodeString(msg.Data) + if err != nil { + t.Errorf("decode %s: %v", msg.Type, err) + return + } + if msg.Type == protocol.MsgTypeStdout { + stdout.Write(data) + } else { + stderr.Write(data) + } + case protocol.MsgTypeDone: + res.exitCode = msg.ExitCode + res.timeout = msg.Timeout + case protocol.MsgTypeError: + res.errMsg = msg.Message + } + } + }() + + select { + case <-runDone: + case <-time.After(30 * time.Second): + t.Fatal("executor did not finish within 30s") + } + res.elapsed = time.Since(start) + <-readDone + res.stdout = stdout.String() + res.stderr = stderr.String() + return res +} + +func helperTool(mode string) config.ToolDef { + noPTY := false + return config.ToolDef{ + Timeout: "20s", + Env: map[string]string{helperEnv: mode}, + UsePTY: &noPTY, + } +} + +func readPID(t *testing.T, path string) int { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read pidfile: %v", err) + } + pid, err := strconv.Atoi(string(data)) + if err != nil { + t.Fatalf("parse pid: %v", err) + } + t.Cleanup(func() { _ = syscall.Kill(pid, syscall.SIGKILL) }) + return pid +} + +// processAlive reports whether pid still exists after giving a just-signalled +// process a moment to be reaped. +func processAlive(pid int) bool { + deadline := time.Now().Add(2 * time.Second) + for { + if err := syscall.Kill(pid, 0); err != nil { + return false + } + if time.Now().After(deadline) { + return true + } + time.Sleep(50 * time.Millisecond) + } +} + +func TestExecutor_DetachedDaemonChild(t *testing.T) { + tests := []struct { + name string + session string + stdio string + wantAlive bool + }{ + // Well-behaved: new session, stdio detached. The contract we document. + {name: "setsid devnull", session: "setsid", stdio: "devnull", wantAlive: true}, + // Sloppy: new session but stdout/stderr inherited. Must not hang claw-wrap. + {name: "setsid inherited stdio", session: "setsid", stdio: "inherit", wantAlive: true}, + // Background child left in the tool's process group: cleanup kills it. + {name: "same process group", session: "same-pgrp", stdio: "devnull", wantAlive: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pidFile := filepath.Join(t.TempDir(), "daemon.pid") + res := runHelperTool(t, helperTool("spawn-daemon"), protocol.ProxyRequest{ + Args: []string{tt.session, tt.stdio, pidFile}, + }) + + if res.errMsg != "" { + t.Fatalf("executor error: %s", res.errMsg) + } + if res.exitCode != 0 || res.timeout { + t.Fatalf("exit=%d timeout=%v stderr=%q", res.exitCode, res.timeout, res.stderr) + } + if res.stdout != "daemon started" { + t.Fatalf("stdout = %q, want %q", res.stdout, "daemon started") + } + if res.elapsed > 3*time.Second { + t.Fatalf("executor took %v; a detached child must not hold the call open", res.elapsed) + } + + pid := readPID(t, pidFile) + if alive := processAlive(pid); alive != tt.wantAlive { + t.Fatalf("daemon alive = %v, want %v", alive, tt.wantAlive) + } + }) + } +} + +// Wait() on an exec.Cmd closes StdoutPipe readers as soon as the process +// exits, which can drop output the pumpers have not read yet. +func TestExecutor_NoOutputTruncationOnExit(t *testing.T) { + const size = 200 * 1024 + for i := 0; i < 25; i++ { + res := runHelperTool(t, helperTool("big-output"), protocol.ProxyRequest{ + Args: []string{strconv.Itoa(size)}, + }) + if res.exitCode != 0 { + t.Fatalf("run %d: exit=%d stderr=%q", i, res.exitCode, res.stderr) + } + if len(res.stdout) != size { + t.Fatalf("run %d: got %d stdout bytes, want %d", i, len(res.stdout), size) + } + } +} diff --git a/internal/daemon/executor_options_test.go b/internal/daemon/executor_options_test.go new file mode 100644 index 0000000..cd646cf --- /dev/null +++ b/internal/daemon/executor_options_test.go @@ -0,0 +1,121 @@ +package daemon + +import ( + "path/filepath" + "strconv" + "testing" + "time" + + "claw-wrap/internal/config" + "claw-wrap/internal/protocol" +) + +// In a sidecar the caller's cwd usually does not exist in the daemon's +// filesystem; working_dir pins the tool's cwd instead. +func TestExecutor_WorkingDirOverridesRequestCwd(t *testing.T) { + workDir, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + tool := helperTool("pwd") + tool.WorkingDir = workDir + + cfg := &config.Config{Audit: &config.AuditConfig{Enabled: true, Stdout: true}} + logger := &captureAudit{} + res := runHelperToolWith(t, tool, protocol.ProxyRequest{Cwd: "/nonexistent/caller/cwd"}, cfg, logger) + if res.errMsg != "" { + t.Fatalf("executor error: %s", res.errMsg) + } + if res.stdout != workDir { + t.Fatalf("tool cwd = %q, want %q", res.stdout, workDir) + } + // The audit log records where the tool actually ran. + if len(logger.entries) != 1 || logger.entries[0].Cwd != workDir { + t.Fatalf("audit entries = %+v, want cwd %q", logger.entries, workDir) + } +} + +func TestExecutor_MissingRequestCwdRejectedWithoutWorkingDir(t *testing.T) { + res := runHelperTool(t, helperTool("pwd"), protocol.ProxyRequest{Cwd: "/nonexistent/caller/cwd"}) + if res.errMsg != "invalid working directory" { + t.Fatalf("errMsg = %q, want %q", res.errMsg, "invalid working directory") + } +} + +// With use_stdin: false a tool that prompts gets EOF immediately, even though +// the wrapper connection (and thus the stdin stream) stays open. +func TestExecutor_UseStdinFalseFailsFastOnPrompt(t *testing.T) { + tool := helperTool("read-stdin") + noStdin := false + tool.UseStdin = &noStdin + tool.UsePTY = nil + + res := runHelperTool(t, tool, protocol.ProxyRequest{UsePTY: true}) + if res.timeout { + t.Fatal("tool timed out; stdin should have been /dev/null") + } + if res.exitCode != 3 || res.stderr != "no input available" { + t.Fatalf("exit=%d stderr=%q, want exit 3 with EOF message", res.exitCode, res.stderr) + } + if res.elapsed > 2*time.Second { + t.Fatalf("took %v, want immediate failure", res.elapsed) + } +} + +// Without use_stdin: false a prompting tool waits for input, but the tool +// timeout still bounds it. +func TestExecutor_PromptingToolHitsTimeout(t *testing.T) { + tool := helperTool("read-stdin") + tool.Timeout = "1s" + + res := runHelperTool(t, tool, protocol.ProxyRequest{}) + if !res.timeout { + t.Fatalf("expected timeout, got exit=%d stderr=%q", res.exitCode, res.stderr) + } + if res.elapsed > 8*time.Second { + t.Fatalf("timeout handling took %v", res.elapsed) + } +} + +// The caller signs its own requests, so request env is caller-controlled. +// request_env limits which variables it may set. +func TestExecutor_RequestEnvAllowlist(t *testing.T) { + req := protocol.ProxyRequest{ + Args: []string{"MCP_SERVER_URL"}, + Env: map[string]string{"MCP_SERVER_URL": "https://evil.example", "TERM": "xterm"}, + } + + legacy := runHelperTool(t, helperTool("env"), req) + if legacy.stdout != "https://evil.example" { + t.Fatalf("without request_env the caller's env passes through (legacy), got %q", legacy.stdout) + } + + tool := helperTool("env") + tool.RequestEnv = []string{"TERM"} + res := runHelperTool(t, tool, req) + if res.stdout != "" { + t.Fatalf("MCP_SERVER_URL leaked into the tool: %q", res.stdout) + } + + req.Args = []string{"TERM"} + res = runHelperTool(t, tool, req) + if res.stdout != "xterm" { + t.Fatalf("allowlisted TERM = %q, want xterm", res.stdout) + } +} + +// Output still buffered in the pipe when the tool exits must survive a client +// that reads slower than the post-exit drain grace. +func TestExecutor_SlowClientDoesNotTruncateOutput(t *testing.T) { + const size = 90 * 1024 + clientReadDelay = 2 * outputDrainGrace + t.Cleanup(func() { clientReadDelay = 0 }) + + res := runHelperTool(t, helperTool("big-output"), protocol.ProxyRequest{Args: []string{strconv.Itoa(size)}}) + if res.exitCode != 0 { + t.Fatalf("exit=%d stderr=%q err=%q", res.exitCode, res.stderr, res.errMsg) + } + if len(res.stdout) != size { + t.Fatalf("got %d stdout bytes, want %d", len(res.stdout), size) + } +} diff --git a/internal/daemon/executor_secrets_test.go b/internal/daemon/executor_secrets_test.go new file mode 100644 index 0000000..ada07ee --- /dev/null +++ b/internal/daemon/executor_secrets_test.go @@ -0,0 +1,108 @@ +package daemon + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "claw-wrap/internal/audit" + "claw-wrap/internal/config" + "claw-wrap/internal/protocol" +) + +type captureAudit struct { + mu sync.Mutex + entries []audit.Entry +} + +func (c *captureAudit) Log(e audit.Entry) error { + c.mu.Lock() + defer c.mu.Unlock() + c.entries = append(c.entries, e) + return nil +} + +func (c *captureAudit) Close() error { return nil } + +// A credential mapped into the tool env reaches the tool, but never the +// wrapper's output or the audit log, even when the tool prints it. +func TestExecutor_CredentialEnvIsRedactedAndNotAudited(t *testing.T) { + const secret = "front-client-secret-0123456789" + secretPath := filepath.Join(t.TempDir(), "front-client-secret") + if err := os.WriteFile(secretPath, []byte(secret+"\n"), 0o600); err != nil { + t.Fatal(err) + } + + cfg := &config.Config{ + Credentials: map[string]config.CredentialDef{ + "front_client_secret": {Source: "file:" + secretPath}, + }, + Audit: &config.AuditConfig{Enabled: true, Stdout: true}, + } + tool := helperTool("env") + tool.Env["FRONT_CLIENT_SECRET"] = "front_client_secret" + + logger := &captureAudit{} + res := runHelperToolWith(t, tool, protocol.ProxyRequest{Args: []string{"FRONT_CLIENT_SECRET"}}, cfg, logger) + + if res.errMsg != "" || res.exitCode != 0 { + t.Fatalf("exit=%d err=%q stderr=%q", res.exitCode, res.errMsg, res.stderr) + } + if res.stdout != config.DefaultRedactReplacement { + t.Fatalf("stdout = %q, want the secret redacted", res.stdout) + } + + if len(logger.entries) != 1 { + t.Fatalf("audit entries = %d, want 1", len(logger.entries)) + } + raw, _ := json.Marshal(logger.entries[0]) + if strings.Contains(string(raw), secret) || strings.Contains(string(raw), "FRONT_CLIENT_SECRET=") { + t.Fatalf("audit entry leaks the credential: %s", raw) + } +} + +func TestOutputRedactor_SecretAcrossChunkBoundary(t *testing.T) { + secret := strings.Repeat("s3cr3t", 50) // 300 bytes, longer than the default overlap + r := NewOutputRedactorWithSecrets(nil, []string{secret}) + + input := "head " + secret + " tail" + var out strings.Builder + // Feed in small chunks so the secret is split across many of them. + for i := 0; i < len(input); i += 7 { + end := i + 7 + if end > len(input) { + end = len(input) + } + out.Write(r.RedactChunk([]byte(input[i:end]), false)) + } + out.Write(r.RedactChunk(nil, true)) + + if got := out.String(); got != "head [REDACTED] tail" { + t.Fatalf("redacted output = %q", got) + } +} + +func TestOutputRedactor_IgnoresShortSecrets(t *testing.T) { + // Very short values (e.g. "1", "true") would mangle unrelated output. + if r := NewOutputRedactorWithSecrets(nil, []string{"abc", ""}); r != nil { + t.Fatal("expected no redactor for values below the minimum length") + } +} + +// A tool rule that rewrites part of a secret must not leave a fragment behind. +func TestOutputRedactor_SecretsBeforeToolRules(t *testing.T) { + cfg := &config.Config{Tools: map[string]config.ToolDef{ + "t": {Binary: "/usr/bin/true", RedactOutput: []config.ToolRedactRule{{Pattern: "tok", Replace: "***"}}}, + }} + if err := cfg.Validate(); err != nil { + t.Fatal(err) + } + r := NewOutputRedactorWithSecrets(cfg.Tools["t"].RedactOutput, []string{"tok_live_123456"}) + got := string(r.RedactChunk([]byte("key=tok_live_123456"), true)) + if got != "key=[REDACTED]" { + t.Fatalf("redacted = %q, want key=[REDACTED]", got) + } +} diff --git a/internal/daemon/helper_process_test.go b/internal/daemon/helper_process_test.go new file mode 100644 index 0000000..161d8d2 --- /dev/null +++ b/internal/daemon/helper_process_test.go @@ -0,0 +1,94 @@ +package daemon + +import ( + "bufio" + "fmt" + "os" + "os/exec" + "strconv" + "strings" + "syscall" + "testing" + "time" +) + +// helperEnv selects a helper mode when the test binary is re-executed as a +// wrapped tool. Executor tests point ToolDef.Binary at os.Executable() and set +// this variable through ToolDef.Env, so the "tool" is this very binary. +const helperEnv = "CLAW_WRAP_TEST_HELPER" + +func TestMain(m *testing.M) { + mode := os.Getenv(helperEnv) + if mode == "" { + os.Exit(m.Run()) + } + os.Exit(runHelper(mode, os.Args[1:])) +} + +func runHelper(mode string, args []string) int { + switch mode { + case "echo": + fmt.Fprint(os.Stdout, strings.Join(args, " ")) + return 0 + + case "pwd": + wd, _ := os.Getwd() + fmt.Fprint(os.Stdout, wd) + return 0 + + case "env": + fmt.Fprint(os.Stdout, os.Getenv(args[0])) + return 0 + + case "big-output": + // args: + n, _ := strconv.Atoi(args[0]) + w := bufio.NewWriterSize(os.Stdout, 64*1024) + for i := 0; i < n; i++ { + _ = w.WriteByte('a' + byte(i%26)) + } + _ = w.Flush() + return 0 + + case "read-stdin": + // Mimics a CLI that prompts for input; EOF means "non-interactive". + line, err := bufio.NewReader(os.Stdin).ReadString('\n') + if err != nil { + fmt.Fprint(os.Stderr, "no input available") + return 3 + } + fmt.Fprint(os.Stdout, "got:"+line) + return 0 + + case "spawn-daemon": + // args: + // Mimics a CLI whose first invocation starts a long-lived background + // daemon and then exits. + cmd := exec.Command(os.Args[0]) + cmd.Env = append(os.Environ(), helperEnv+"=daemon") + if args[0] == "setsid" { + cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true} + } + if args[1] == "inherit" { + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + } + if err := cmd.Start(); err != nil { + fmt.Fprintf(os.Stderr, "start daemon: %v", err) + return 1 + } + if err := os.WriteFile(args[2], []byte(strconv.Itoa(cmd.Process.Pid)), 0o600); err != nil { + fmt.Fprintf(os.Stderr, "write pidfile: %v", err) + return 1 + } + fmt.Fprint(os.Stdout, "daemon started") + return 0 + + case "daemon": + time.Sleep(60 * time.Second) + return 0 + } + + fmt.Fprintf(os.Stderr, "unknown helper mode %q", mode) + return 2 +} diff --git a/internal/daemon/mcparcel_policy_test.go b/internal/daemon/mcparcel_policy_test.go new file mode 100644 index 0000000..1b57a7c --- /dev/null +++ b/internal/daemon/mcparcel_policy_test.go @@ -0,0 +1,124 @@ +package daemon + +import ( + "testing" + + "claw-wrap/internal/config" +) + +// Regression suite for the shipped sidecar example (examples/mcparcel). It +// pins down which `mcparcel` invocations an agent can get through claw-wrap. +func TestMcparcelExamplePolicy(t *testing.T) { + cfg, err := config.Load("../../examples/mcparcel/wrappers.yaml") + if err != nil { + t.Fatalf("load example config: %v", err) + } + tool, ok := cfg.Tools["mcparcel"] + if !ok { + t.Fatal("example config has no mcparcel tool") + } + + // The agent signs its own requests: it must not control env or cwd. + if tool.RequestEnv == nil || len(tool.RequestEnv) != 0 { + t.Errorf("request_env = %#v, want an empty allowlist", tool.RequestEnv) + } + if tool.WorkingDir == "" || tool.GetUseStdin() { + t.Errorf("example must pin working_dir and disable stdin (working_dir=%q use_stdin=%v)", tool.WorkingDir, tool.GetUseStdin()) + } + + allowed := [][]string{ + {"call", "front.read_conversation", "--args", `{"conversation_id":"cnv_123"}`}, + {"call", "front.create_draft", "--args", `{"conversation_id":"cnv_123","body":"Hi"}`}, + {"call", "front.update_draft", "--args", `{}`}, + {"call", "front.add_comment", "--args", `{"body":"note"}`}, + {"call", "front.tag_conversation", "--args", `{"tag_id":"tag_1"}`}, + {"call", "front.assign_conversation", "--args", `{"assignee_id":"tea_1"}`}, + {"call", "front.list_tags"}, + {"call", "front.list_teammates"}, + // JSON values may carry spaces, newlines and flag-looking text: they + // are one argument and stay one argument. + {"call", "front.create_draft", "--args", "{\n \"body\": \"line 1\nline 2 --config /tmp/x\"\n}"}, + } + for _, args := range allowed { + if ok, msg := checkToolArgs(args, &tool); !ok { + t.Errorf("expected allowed: %q (msg=%q)", args, msg) + } + } + + blocked := map[string][]string{ + "send_message is not allowlisted": {"call", "front.send_message", "--args", `{}`}, + "other server": {"call", "nimbu.anything", "--args", `{}`}, + "other server, no args": {"call", "nimbu.anything"}, + "list": {"list"}, + "auth": {"auth", "login"}, + "runtime": {"runtime", "status"}, + "import": {"import", "claude"}, + "add": {"add", "evil", "https://evil.example"}, + "no args": {}, + "global flag before subcommand": {"--config", "/tmp/x", "call", "front.read_conversation", "--args", `{}`}, + "global flag=value before": {"--config=/tmp/x", "call", "front.read_conversation"}, + "flag after subcommand": {"call", "front.read_conversation", "--args", `{}`, "--config", "/tmp/x"}, + "flag instead of --args": {"call", "front.read_conversation", "--config", "/tmp/x"}, + "flag=value instead of --args": {"call", "front.read_conversation", "--config=/tmp/x"}, + "unknown flag after tool": {"call", "front.read_conversation", "--server", "https://evil.example"}, + "--args=json form": {"call", "front.read_conversation", `--args={}`}, + "--args without value": {"call", "front.read_conversation", "--args"}, + "--args with non-JSON value": {"call", "front.read_conversation", "--args", "x"}, + "extra positional": {"call", "front.read_conversation", "--args", `{}`, `{}`}, + "subcommand+tool in one arg": {"call front.read_conversation"}, + "tool with smuggled second tool": {"call", "front.create_draft front.send_message"}, + "tool with smuggled flag": {"call", "front.read_conversation --config /tmp/x"}, + "tool with trailing newline": {"call", "front.read_conversation\n"}, + "tool with newline and second tool": {"call", "front.list_tags\nfront.send_message"}, + "tool name suffix": {"call", "front.read_conversationX"}, + "tool name prefix": {"call", "xfront.read_conversation"}, + "case variant": {"call", "FRONT.read_conversation"}, + "call with leading space": {" call", "front.list_tags"}, + "newline-joined subcommand": {"call\nfront.list_tags"}, + "json arg missing closing brace": {"call", "front.read_conversation", "--args", `{"a":1`}, + "json followed by newline and flag": {"call", "front.read_conversation", "--args", "{}\n--config"}, + "subcommand looks like call": {"calls", "front.list_tags"}, + "dotted path traversal in tool name": {"call", "front.list_tags/../send_message"}, + } + for name, args := range blocked { + if ok, _ := checkToolArgs(args, &tool); ok { + t.Errorf("%s: expected blocked: %q", name, args) + } + } +} + +// Documents why the example uses match: argv. With match: command the args +// are joined on spaces before matching, so argument boundaries disappear and +// anything after the matched prefix rides along unless blocked_args happens to +// enumerate it. +func TestMatchCommandLosesArgumentBoundaries(t *testing.T) { + cfg := &config.Config{Tools: map[string]config.ToolDef{ + "mcparcel": { + Binary: "/usr/local/bin/mcparcel", + Mode: config.ToolModeAllowlist, + AllowedArgs: []config.BlockedArg{{ + Pattern: `^call\s+front\.(read_conversation|create_draft)(\s|$)`, + Match: config.BlockedArgMatchCommand, + }}, + }, + }} + if err := cfg.Validate(); err != nil { + t.Fatal(err) + } + tool := cfg.Tools["mcparcel"] + + widened := [][]string{ + // Unlisted flags after the tool name pass. + {"call", "front.read_conversation", "--server", "https://evil.example"}, + // Two tool names in one argument pass; safety now depends on how the + // CLI parses that argument. + {"call", "front.create_draft front.send_message"}, + // Subcommand and tool fused into one argument pass. + {"call front.read_conversation"}, + } + for _, args := range widened { + if ok, _ := checkToolArgs(args, &tool); !ok { + t.Errorf("expected match: command to (unsafely) allow %q", args) + } + } +} diff --git a/internal/daemon/output_redactor.go b/internal/daemon/output_redactor.go index a6e49ca..462dc83 100644 --- a/internal/daemon/output_redactor.go +++ b/internal/daemon/output_redactor.go @@ -2,6 +2,7 @@ package daemon import ( "regexp" + "sort" "claw-wrap/internal/config" ) @@ -48,6 +49,54 @@ func NewOutputRedactor(rules []config.ToolRedactRule) *OutputRedactor { } } +// minSecretRedactLength is the shortest injected credential value that is +// redacted literally. Shorter values ("1", "true") would mangle unrelated output. +const minSecretRedactLength = 8 + +// NewOutputRedactorWithSecrets builds a redactor from the tool's rules plus a +// literal rule for every injected credential value, so a tool that echoes its +// own secret (debug output, error messages) cannot hand it to the caller. +func NewOutputRedactorWithSecrets(rules []config.ToolRedactRule, secrets []string) *OutputRedactor { + r := NewOutputRedactor(rules) + + seen := make(map[string]bool, len(secrets)) + literals := make([]string, 0, len(secrets)) + for _, s := range secrets { + if len(s) < minSecretRedactLength || seen[s] { + continue + } + seen[s] = true + literals = append(literals, s) + } + if len(literals) == 0 { + return r + } + // Longest first, so a secret containing another is replaced whole. + sort.Slice(literals, func(i, j int) bool { return len(literals[i]) > len(literals[j]) }) + + if r == nil { + r = &OutputRedactor{overlap: defaultRedactionOverlapBytes} + } + // Literal secrets run before the tool's own rules: a user regex that + // rewrites part of a secret would otherwise leave a fragment the literal + // no longer matches. + replace := []byte(config.DefaultRedactReplacement) + secretRules := make([]outputRedactionRule, 0, len(literals)) + for _, s := range literals { + secretRules = append(secretRules, outputRedactionRule{ + re: regexp.MustCompile(regexp.QuoteMeta(s)), + replace: replace, + }) + // The carry must hold all but the last byte of a secret for matches + // that span chunk boundaries. + if len(s) > r.overlap { + r.overlap = len(s) + } + } + r.rules = append(secretRules, r.rules...) + return r +} + // RedactChunk redacts one output chunk. When finalize is false it keeps a // bounded carry to catch matches that span chunk boundaries. func (r *OutputRedactor) RedactChunk(data []byte, finalize bool) []byte { diff --git a/internal/daemon/runtime_modes.go b/internal/daemon/runtime_modes.go new file mode 100644 index 0000000..280b09c --- /dev/null +++ b/internal/daemon/runtime_modes.go @@ -0,0 +1,71 @@ +package daemon + +import ( + "fmt" + "os" + "strconv" + "strings" +) + +var ( + allowedAuthFileModes = map[os.FileMode]struct{}{ + 0o600: {}, + 0o640: {}, + } + allowedSocketFileModes = map[os.FileMode]struct{}{ + 0o600: {}, + 0o660: {}, + } +) + +// ParseAuthFileMode parses and validates auth file mode values. +func ParseAuthFileMode(raw string) (os.FileMode, error) { + mode, err := parsePermMode(raw) + if err != nil { + return 0, err + } + if !IsAllowedAuthFileMode(mode) { + return 0, fmt.Errorf("invalid auth mode %04o (allowed: 0600, 0640)", mode) + } + return mode, nil +} + +// ParseSocketFileMode parses and validates socket file mode values. +func ParseSocketFileMode(raw string) (os.FileMode, error) { + mode, err := parsePermMode(raw) + if err != nil { + return 0, err + } + if !IsAllowedSocketFileMode(mode) { + return 0, fmt.Errorf("invalid socket mode %04o (allowed: 0600, 0660)", mode) + } + return mode, nil +} + +// IsAllowedAuthFileMode returns true for supported auth file modes. +func IsAllowedAuthFileMode(mode os.FileMode) bool { + _, ok := allowedAuthFileModes[mode.Perm()] + return ok +} + +// IsAllowedSocketFileMode returns true for supported socket file modes. +func IsAllowedSocketFileMode(mode os.FileMode) bool { + _, ok := allowedSocketFileModes[mode.Perm()] + return ok +} + +func parsePermMode(raw string) (os.FileMode, error) { + modeStr := strings.TrimSpace(raw) + if modeStr == "" { + return 0, fmt.Errorf("mode cannot be empty") + } + modeStr = strings.TrimPrefix(modeStr, "0o") + value, err := strconv.ParseUint(modeStr, 8, 32) + if err != nil { + return 0, fmt.Errorf("invalid mode %q: %w", raw, err) + } + if value&^0o777 != 0 { + return 0, fmt.Errorf("invalid mode %q: only permission bits are allowed", raw) + } + return os.FileMode(value).Perm(), nil +} diff --git a/internal/daemon/runtime_modes_test.go b/internal/daemon/runtime_modes_test.go new file mode 100644 index 0000000..772b2c0 --- /dev/null +++ b/internal/daemon/runtime_modes_test.go @@ -0,0 +1,89 @@ +package daemon + +import ( + "os" + "testing" +) + +func TestParseAuthFileMode(t *testing.T) { + tests := []struct { + name string + input string + want os.FileMode + wantErr bool + }{ + {name: "0600", input: "0600", want: 0o600}, + {name: "0640", input: "0640", want: 0o640}, + {name: "invalid", input: "0666", wantErr: true}, + {name: "setgid bit", input: "2640", wantErr: true}, + {name: "empty", input: "", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ParseAuthFileMode(tt.input) + if (err != nil) != tt.wantErr { + t.Fatalf("ParseAuthFileMode() error = %v, wantErr %v", err, tt.wantErr) + } + if err == nil && got.Perm() != tt.want { + t.Fatalf("ParseAuthFileMode() = %04o, want %04o", got.Perm(), tt.want) + } + }) + } +} + +func TestParseSocketFileMode(t *testing.T) { + tests := []struct { + name string + input string + want os.FileMode + wantErr bool + }{ + {name: "0600", input: "0600", want: 0o600}, + {name: "0660", input: "0660", want: 0o660}, + {name: "invalid", input: "0640", wantErr: true}, + {name: "setuid bit", input: "4660", wantErr: true}, + {name: "empty", input: "", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ParseSocketFileMode(tt.input) + if (err != nil) != tt.wantErr { + t.Fatalf("ParseSocketFileMode() error = %v, wantErr %v", err, tt.wantErr) + } + if err == nil && got.Perm() != tt.want { + t.Fatalf("ParseSocketFileMode() = %04o, want %04o", got.Perm(), tt.want) + } + }) + } +} + +func TestDaemonRunRejectsInvalidRuntimeOptions(t *testing.T) { + d := New(WithRuntimeGID(-2)) + if err := d.Run(); err == nil { + t.Fatal("Run() should fail for invalid runtime gid") + } + + d = New(WithAuthFileMode(0o666)) + if err := d.Run(); err == nil { + t.Fatal("Run() should fail for invalid auth mode") + } + + d = New(WithSocketFileMode(0o640)) + if err := d.Run(); err == nil { + t.Fatal("Run() should fail for invalid socket mode") + } +} + +func TestRuntimeDirMode(t *testing.T) { + d := New() + if got := d.runtimeDirMode(); got != 0o700 { + t.Fatalf("runtimeDirMode() = %04o, want 0700", got) + } + + d = New(WithRuntimeGID(2000)) + if got := d.runtimeDirMode(); got != 0o750 { + t.Fatalf("runtimeDirMode() = %04o, want 0750", got) + } +} diff --git a/internal/paths/paths.go b/internal/paths/paths.go index 31473ca..453d664 100644 --- a/internal/paths/paths.go +++ b/internal/paths/paths.go @@ -9,10 +9,32 @@ import ( "strings" ) -// RuntimeDir returns the platform-appropriate runtime directory. +// RuntimeDirEnv overrides the runtime directory (socket, HMAC auth file, +// proxy auth token) for both the daemon and the wrapper. Containers use it to +// point both sides at a directory inside a shared volume. Must be absolute. +const RuntimeDirEnv = "CLAW_WRAP_RUNTIME_DIR" + +// RuntimeDir returns the runtime directory: $CLAW_WRAP_RUNTIME_DIR when set to +// an absolute path, otherwise the platform default. // - Linux: /run/openclaw (created by systemd RuntimeDirectory=) // - macOS: $TMPDIR/openclaw (per-user, permission-restricted) func RuntimeDir() string { + if dir := os.Getenv(RuntimeDirEnv); dir != "" && filepath.IsAbs(dir) { + return filepath.Clean(dir) + } + return defaultRuntimeDir() +} + +// ValidateRuntimeDirEnv reports a set but unusable $CLAW_WRAP_RUNTIME_DIR, which +// RuntimeDir would otherwise silently ignore. +func ValidateRuntimeDirEnv() error { + if dir := os.Getenv(RuntimeDirEnv); dir != "" && !filepath.IsAbs(dir) { + return fmt.Errorf("%s must be an absolute path, got %q", RuntimeDirEnv, dir) + } + return nil +} + +func defaultRuntimeDir() string { if runtime.GOOS == "darwin" { return filepath.Join(os.TempDir(), "openclaw") } @@ -29,9 +51,11 @@ func AuthPath() string { return filepath.Join(RuntimeDir(), "auth") } -// EnvFile returns the default env credential file path. +// EnvFile returns the default env credential file path. It deliberately ignores +// $CLAW_WRAP_RUNTIME_DIR: an overridden runtime dir may be shared with the +// client, and credentials must never live there. func EnvFile() string { - return filepath.Join(RuntimeDir(), "env") + return filepath.Join(defaultRuntimeDir(), "env") } // ProxyAuthTokenPath returns the default HTTP proxy auth token file path. diff --git a/internal/paths/runtime_dir_test.go b/internal/paths/runtime_dir_test.go new file mode 100644 index 0000000..db1965f --- /dev/null +++ b/internal/paths/runtime_dir_test.go @@ -0,0 +1,41 @@ +package paths + +import ( + "path/filepath" + "testing" +) + +func TestRuntimeDir_EnvOverride(t *testing.T) { + defaultEnv := EnvFile() + t.Setenv(RuntimeDirEnv, "/run/claw-wrap/rt/") + + if got := RuntimeDir(); got != "/run/claw-wrap/rt" { + t.Fatalf("RuntimeDir() = %q", got) + } + for name, got := range map[string]string{ + "SocketPath": SocketPath(), + "AuthPath": AuthPath(), + "ProxyAuthTokenPath": ProxyAuthTokenPath(), + } { + if filepath.Dir(got) != "/run/claw-wrap/rt" { + t.Errorf("%s() = %q, want it under the override", name, got) + } + } + // The env credential file must never follow the override: the runtime + // dir may be shared with the (untrusted) client container. + if got := EnvFile(); got != defaultEnv { + t.Errorf("EnvFile() = %q, want unchanged %q", got, defaultEnv) + } +} + +func TestRuntimeDir_IgnoresRelativeOverride(t *testing.T) { + t.Setenv(RuntimeDirEnv, "") + want := RuntimeDir() + t.Setenv(RuntimeDirEnv, "relative/dir") + if got := RuntimeDir(); got != want { + t.Fatalf("RuntimeDir() = %q, want default %q", got, want) + } + if err := ValidateRuntimeDirEnv(); err == nil { + t.Fatal("ValidateRuntimeDirEnv() should reject a relative path") + } +} From b810e2cd03a6235ed4bef76dd1b61828f74ffa0a Mon Sep 17 00:00:00 2001 From: Peter Dedene Date: Wed, 7 Oct 2026 11:18:29 +0200 Subject: [PATCH 3/6] build: add minimal sidecar image and multi-arch image workflow Debian slim with tini as PID 1 (reaps detached tool daemons) and a non-root claw user. The workflow builds linux/amd64 and linux/arm64, pushes to ghcr.io on v* tags and only builds on PRs touching the image. --- .dockerignore | 11 ++++++++ .github/workflows/image.yml | 56 +++++++++++++++++++++++++++++++++++++ Dockerfile | 52 ++++++++++++++++++++++++++++++++++ 3 files changed, 119 insertions(+) create mode 100644 .dockerignore create mode 100644 .github/workflows/image.yml create mode 100644 Dockerfile diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 0000000..2a5cb3a --- /dev/null +++ b/.dockerignore @@ -0,0 +1,11 @@ +.git +.github +.claude +.pi +.env +build +dist +tmp +docs +examples +*.md diff --git a/.github/workflows/image.yml b/.github/workflows/image.yml new file mode 100644 index 0000000..404401a --- /dev/null +++ b/.github/workflows/image.yml @@ -0,0 +1,56 @@ +name: Image + +on: + push: + tags: + - "v*" + pull_request: + branches: [main] + paths: + - Dockerfile + - .dockerignore + - .github/workflows/image.yml + +permissions: + contents: read + packages: write + +env: + IMAGE: ghcr.io/${{ github.repository }} + +jobs: + image: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + + - uses: docker/setup-qemu-action@99012661954931238ded8c8b007157a8430204e1 # v4.4.0 + + - uses: docker/setup-buildx-action@f87e5991a6d7451dcb8d9637bfbc97413f497069 # v4.4.1 + + - if: github.event_name == 'push' + uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4.6.0 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - id: meta + uses: docker/metadata-action@dc802804100637a589fabce1cb79ff13a1411302 # v6.2.0 + with: + images: ${{ env.IMAGE }} + tags: | + type=semver,pattern={{version}} + type=semver,pattern={{major}}.{{minor}} + + - uses: docker/build-push-action@c3c9e263c25d99ce0380d002d59b67737d91b0dc # v7.4.0 + with: + context: . + platforms: linux/amd64,linux/arm64 + push: ${{ github.event_name == 'push' }} + tags: ${{ steps.meta.outputs.tags }} + labels: ${{ steps.meta.outputs.labels }} + build-args: | + VERSION=${{ steps.meta.outputs.version }} + cache-from: type=gha + cache-to: type=gha,mode=max diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 0000000..74a3f28 --- /dev/null +++ b/Dockerfile @@ -0,0 +1,52 @@ +# syntax=docker/dockerfile:1 +# +# Minimal claw-wrap image: the daemon for a sidecar container, and the source +# of the client binary for the agent container. +# +# docker build -t claw-wrap . +# +# Add the tools you want to broker in a derived image: +# +# FROM ghcr.io/dedene/claw-wrap: +# COPY --from= /out/mytool /usr/local/bin/mytool +# +# See docs/KUBERNETES.md. + +ARG GO_VERSION=1.26 +ARG DEBIAN_RELEASE=trixie + +FROM --platform=$BUILDPLATFORM golang:${GO_VERSION}-${DEBIAN_RELEASE} AS build +WORKDIR /src +COPY go.mod go.sum ./ +RUN --mount=type=cache,target=/go/pkg/mod go mod download +COPY . . +ARG TARGETOS +ARG TARGETARCH +ARG VERSION=dev +RUN --mount=type=cache,target=/go/pkg/mod \ + --mount=type=cache,target=/root/.cache/go-build \ + CGO_ENABLED=0 GOOS=${TARGETOS} GOARCH=${TARGETARCH} \ + go build -trimpath -ldflags "-s -w -X main.version=${VERSION}" -o /out/claw-wrap ./cmd/claw-wrap + +FROM debian:${DEBIAN_RELEASE}-slim + +# tini runs as PID 1 so detached daemons started by wrapped tools (which get +# reparented to PID 1 when their parent exits) are reaped instead of lingering +# as zombies. +RUN apt-get update \ + && apt-get install -y --no-install-recommends ca-certificates tini \ + && rm -rf /var/lib/apt/lists/* + +# A real passwd entry gives wrapped tools a sane HOME and USER. +ARG UID=10001 +ARG GID=10001 +RUN groupadd --gid ${GID} claw \ + && useradd --uid ${UID} --gid ${GID} --home-dir /var/lib/claw-wrap --create-home --shell /usr/sbin/nologin claw \ + && install -d -o claw -g claw -m 0700 /run/openclaw + +COPY --from=build /out/claw-wrap /usr/local/bin/claw-wrap + +USER ${UID}:${GID} +WORKDIR /var/lib/claw-wrap +ENTRYPOINT ["/usr/bin/tini", "--", "/usr/local/bin/claw-wrap"] +CMD ["daemon"] From 185ad02f2f054cdc25682e1919548a85335c3c97 Mon Sep 17 00:00:00 2001 From: Peter Dedene Date: Wed, 7 Oct 2026 11:18:29 +0200 Subject: [PATCH 4/6] docs: add Kubernetes sidecar guide and document new options Covers the shared runtime dir, UID/GID modes, security model without exe verification, Secrets via file:, ConfigMap reload, probes, audit to stdout and the contract for tools that start background daemons. CONFIG.md gains file:, working_dir, use_pty, use_stdin, request_env, match: argv, audit and daemon flags. --- README.md | 9 +- docs/CONFIG.md | 129 ++++++++++++++++++ docs/KUBERNETES.md | 203 ++++++++++++++++++++++++++++ examples/kubernetes/deployment.yaml | 136 +++++++++++++++++++ 4 files changed, 476 insertions(+), 1 deletion(-) create mode 100644 docs/KUBERNETES.md create mode 100644 examples/kubernetes/deployment.yaml diff --git a/README.md b/README.md index 8201cd4..d9f2f41 100644 --- a/README.md +++ b/README.md @@ -92,7 +92,8 @@ claw-wrap supports two approaches for credential injection: | Backend | Prefix | Example | Notes | | ------- | ------ | ------- | ----- | | [pass](https://www.passwordstore.org/) | `pass:` | `pass:cli/github/token` | Default when no prefix given | -| Environment | `env:` | `env:MY_TOKEN` | Reads from daemon environment | +| Environment file | `env:` | `env:MY_TOKEN` | Reads `KEY=value` lines from `/run/openclaw/env` | +| Secret file | `file:` | `file:/etc/claw-wrap/secrets/token` | Docker/Kubernetes secret mounts; re-read on every use | | [1Password](https://1password.com/) | `op://` | `op://Vault/Item/field` | Requires `op` CLI, session auth | | [Bitwarden](https://bitwarden.com/) | `bw:` | `bw:item-uuid` | Requires `bw` CLI, session managed | | [macOS Keychain](https://support.apple.com/guide/keychain-access/) | `keychain:` | `keychain:service-name` | macOS only | @@ -341,6 +342,10 @@ claw-wrap works with deny-by-default sandboxes where credentials directories (`~ See [docs/SANDBOX.md](docs/SANDBOX.md) for the full guide — firejail profile, nono setup, self-restart mechanism, and verification steps. +## Containers and Kubernetes + +claw-wrap can run as a sidecar: the agent container gets only the client binary, and the sidecar holds the credentials and runs the real tools. The repository `Dockerfile` builds a small base image to extend with your tools. See [docs/KUBERNETES.md](docs/KUBERNETES.md) for the pod layout, security model and a tested manifest. + ## Documentation - [Installation Guide](docs/INSTALL.md) — full setup with `pass`, systemd, and troubleshooting @@ -348,6 +353,7 @@ self-restart mechanism, and verification steps. args, output redaction, config file injection - [HTTP Proxy Setup](docs/CONFIG.md#http-proxy-settings) — MITM proxy for API credential injection - [Sandbox Setup](docs/SANDBOX.md) — firejail (Linux) and nono (macOS) with verification steps +- [Kubernetes Sidecar](docs/KUBERNETES.md) — shared socket volume, UID/GID setup, Secrets, probes, audit - [Protocol Specification](docs/SPEC.md) — HMAC authentication, message framing, proxy protocol ## Usage @@ -355,6 +361,7 @@ self-restart mechanism, and verification steps. ```bash # Daemon mode (usually via systemd) claw-wrap daemon +claw-wrap daemon --uid 1000 --runtime-gid 2000 --socket-mode 0660 --auth-mode 0640 # shared with another UID # Admin commands claw-wrap list # List configured tools diff --git a/docs/CONFIG.md b/docs/CONFIG.md index c7714a4..db3c587 100644 --- a/docs/CONFIG.md +++ b/docs/CONFIG.md @@ -13,6 +13,21 @@ The runtime directory varies by platform: Examples in this document use Linux paths. On macOS, substitute `$TMPDIR/openclaw/` for `/run/openclaw/`. +Set `CLAW_WRAP_RUNTIME_DIR` (absolute path) to move the socket, auth file and proxy auth token, for example onto a volume shared between containers. The daemon and the wrapper both read it, so set it on both sides. The `env:` credential file does not move. + +### Daemon flags + +| Flag | Default | Purpose | +| --- | --- | --- | +| `--config PATH` | `/etc/openclaw/wrappers.yaml` | Config file | +| `--socket PATH` | `/secrets.sock` | Socket path | +| `--uid UID` | daemon's UID | Only this peer UID may connect | +| `--runtime-gid GID` | unset | Group for runtime dir (`0750`), socket and auth file | +| `--socket-mode MODE` | `0600` | `0600` or `0660` | +| `--auth-mode MODE` | `0600` | `0600` or `0640` | + +The last three let a client with a different UID reach the daemon through a shared group. See [KUBERNETES.md](KUBERNETES.md). + ## Minimal Example A single tool (`gh`) with one credential: @@ -525,6 +540,24 @@ Set to `true` to reject connections when `/proc//exe` is unreadable (opt-in In firejail deployments this can fail closed unexpectedly depending on host `/proc` policy (`hidepid`, ptrace restrictions), so test carefully before enabling. +## Audit Log + +```yaml +audit: + enabled: true + file: /var/log/claw-wrap/audit.jsonl # JSONL file, created 0600 + stdout: false # JSONL on the daemon's stdout (containers) + syslog: false + syslog_facility: local0 + include_args: true # default true + include_output_hash: true # default true + include_duration: true # default true +``` + +At least one of `file`, `stdout` or `syslog` is required. Each completed call produces one entry with the tool, arguments, working directory, caller PID and executable (when verifiable), exit code, output size and hash. Environment variables and credential values are never logged. + +`file` must not be a symlink, so use `stdout: true` rather than `file: /dev/stdout`. Daemon logs go to stderr, which keeps the two streams separate. + ## Credential Sources ### Password Store (`pass`) @@ -552,6 +585,29 @@ Security requirements for `/run/openclaw/env`: - Must be owned by the daemon user - Mode must be `0600` or `0640` +### Mounted secret file (`file:`) + +```yaml +credentials: + api-token: + source: file:/etc/claw-wrap/secrets/api-token + oauth-client: + source: file:/etc/claw-wrap/secrets/client.json | .client_secret +``` + +Reads a whole file, the convention for Docker and Kubernetes secret mounts. Docker and Compose secrets default to mode `0444`, which is rejected; set `mode: 0400` or `0440` on the secret. Trailing newlines are trimmed. jq extraction works on JSON files. + +Rules, checked on every read: +- Path must be absolute +- Symlinks are followed (Kubernetes secret volumes are `..data` symlinks) +- Must resolve to a regular file of at most 64 KiB +- Owner must be root or the daemon user +- Must not be group-writable or have any permission bits for others (`0400`, `0440`, `0600` and `0640` are fine) + +The value is not cached, so a rotated secret applies on the next call. + +Prefer `file:` over `env:` in containers: the `env:` file must be a non-symlink owned by the daemon user, which a Kubernetes Secret volume cannot provide. + ### 1Password (`op://`) ```yaml @@ -896,6 +952,8 @@ tools: API_SECRET: my-api-secret ``` +Every credential value injected through `env` or `config_file` (8 bytes or longer), and the HTTP proxy auth token, is redacted from the tool's stdout and stderr as `[REDACTED]`, before any `redact_output` rules run. This catches a tool that prints its token verbatim in debug output or an error message. It only matches the exact value: base64, URL-encoded or otherwise transformed copies pass through, so it is a safety net, not a guarantee. Environment variables never appear in the audit log. + ### `forced_env` (optional) Environment variables that are always set. The agent cannot override these. @@ -956,6 +1014,33 @@ tools: message: "Only informational commands allowed" ``` +`allowed_args` also supports `match: argv`, which matches arguments by position: + +```yaml +tools: + mcparcel: + binary: /usr/local/bin/mcparcel + mode: allowlist + allowed_args: + - match: argv + argv: + - 'call' + - 'front\.(read_conversation|create_draft)' + - '--args' + - '(?s)\{.*\}' + message: "only allowlisted tools" + - match: argv + argv: ['call', 'front\.list_tags'] +``` + +- The command must have exactly as many arguments as `argv` has patterns +- Each pattern must match its whole argument; patterns are anchored as `^(?:pattern)$` +- `.` does not match newlines unless the pattern starts with `(?s)` +- List several rules for optional arguments; any matching rule allows the call +- `pattern` and `argv` are mutually exclusive; `argv` is not available in `blocked_args` + +Prefer `argv` over `match: command` for tools with subcommands and flags. `command` joins the arguments with spaces before matching, so argument boundaries disappear: with `^call\s+front\.read(\s|$)`, the calls `call front.read --server https://evil.example` and `call "front.read front.delete"` both pass, and only `blocked_args` stands between them and the tool. With `argv` every position is pinned, so flags before or after the subcommand, extra arguments, and arguments containing spaces or newlines are rejected. + When both `blocked_args` and `allowed_args` are present (with `mode: allowlist`): 1. `blocked_args` are checked first (any match = deny) 2. `allowed_args` are checked next (at least one must match) @@ -987,6 +1072,50 @@ Behavior notes: - Rules are applied in order - Invalid regex patterns are rejected during config validation +### `working_dir` (optional) + +Absolute directory the tool runs in. By default the tool runs in the caller's working directory, and the call fails with `invalid working directory` when that directory does not exist on the daemon's side. Set `working_dir` when the daemon runs in another container or on another filesystem. + +```yaml +tools: + mcparcel: + binary: /usr/local/bin/mcparcel + working_dir: /var/lib/claw-wrap +``` + +### `use_pty` (optional) + +Default `true`: when the caller's stdin is a terminal, the tool gets a pseudo-terminal (colors, interactive prompts, TUIs). Set `false` to always use plain pipes. + +### `use_stdin` (optional) + +Default `true`: the caller's stdin is forwarded to the tool. With `false` the tool reads from `/dev/null`, so a tool that prompts for input fails immediately instead of waiting until `timeout`. Use it for tools that agents call non-interactively. `use_stdin: false` turns PTY mode off as well; combining it with `use_pty: true` is a config error. + +### `request_env` (optional) + +Allowlist of environment variables the caller may set for this tool. By default the caller may set any variable that is not on the built-in deny list (`BASH_ENV`, `LD_*`, proxy variables and similar), but tool-specific variables are not on that list. When the caller is untrusted, an unexpected variable such as a config path or server URL can change where the tool sends its credentials. + +```yaml +tools: + mcparcel: + binary: /usr/local/bin/mcparcel + request_env: [] # accept no caller env at all + gh: + binary: /usr/bin/gh + request_env: [TERM, COLORTERM] # what the wrapper sends for terminals +``` + +Unset keeps the default behaviour. `env` entries can never be overridden by the caller either way. + +### Tools that start background processes + +Each call runs in its own process group. When the call ends, claw-wrap kills that group, and it stops reading output one second after the tool exits. A tool that starts a long-lived daemon on first use must therefore: + +- start the daemon in a new session (`setsid`), so the process group kill does not reach it +- redirect the daemon's stdin, stdout and stderr to `/dev/null` or a log file + +A daemon that keeps the tool's stdout or stderr open does not block the call, but claw-wrap logs a warning and the daemon's next write to that pipe fails (usually with `SIGPIPE`). In containers, run claw-wrap under an init process such as `tini` so detached daemons are reaped. + ### `config_file` (optional) For tools that read config from a file instead of environment variables. diff --git a/docs/KUBERNETES.md b/docs/KUBERNETES.md new file mode 100644 index 0000000..ab89fab --- /dev/null +++ b/docs/KUBERNETES.md @@ -0,0 +1,203 @@ +# Running claw-wrap as a Kubernetes sidecar + +This guide runs claw-wrap next to an AI agent in one pod. The agent container holds no secrets. It calls wrapped tools through the claw-wrap client, and the sidecar runs the real binaries with credentials injected. + +``` +Pod +├─ agent /usr/local/bin/ -> claw-wrap (client only, no secrets) +│ │ Unix socket + HMAC on a shared emptyDir +└─ claw-wrap daemon: UID check, HMAC, argument allowlist, audit + (sidecar) └─ runs the real with credentials from a Secret +``` + +Only CLI brokering is used here. The HTTP proxy stays off. + +A working manifest is in [`examples/kubernetes/deployment.yaml`](../examples/kubernetes/deployment.yaml), with a matching tool config in [`examples/mcparcel/wrappers.yaml`](../examples/mcparcel/wrappers.yaml). Both were deployed to a kind cluster (Kubernetes 1.35) and exercised end to end while writing this guide. + +## Images + +### Sidecar + +The repository `Dockerfile` builds a small Debian image with `claw-wrap`, `tini` and CA certificates. It runs as UID/GID 10001 with `claw-wrap daemon` as the default command and `tini` as PID 1. + +Add the tools you want to broker in a derived image: + +```dockerfile +FROM ghcr.io/dedene/claw-wrap:0.6.0 +COPY --from=mytool-build /out/mcparcel /usr/local/bin/mcparcel +``` + +`tini` matters when a wrapped tool starts a background daemon: once the tool exits, the daemon is reparented to PID 1, and `tini` reaps it when it dies. Without an init process it lingers as a zombie, and a liveness check like `kill -0 ` keeps reporting it as alive. + +### Agent + +The agent image needs the `claw-wrap` binary and one symlink per tool. The symlink name is the tool name claw-wrap asks the daemon for. + +```dockerfile +COPY --from=ghcr.io/dedene/claw-wrap:0.6.0 /usr/local/bin/claw-wrap /usr/local/bin/claw-wrap +RUN ln -s claw-wrap /usr/local/bin/mcparcel +``` + +Release archives with linux/amd64 and linux/arm64 binaries work too. + +## Shared runtime directory + +The client and daemon find each other through the runtime directory: `secrets.sock` (the Unix socket) and `auth` (the HMAC secret the client signs requests with). Both containers mount the same `emptyDir` and set the same `CLAW_WRAP_RUNTIME_DIR`: + +```yaml +env: + - name: CLAW_WRAP_RUNTIME_DIR + value: /run/claw-wrap/ipc +volumeMounts: + - name: ipc + mountPath: /run/claw-wrap +``` + +Point `CLAW_WRAP_RUNTIME_DIR` at a subdirectory of the mount, not at the mount itself. The kubelet creates `emptyDir` mounts owned by root, so the non-root daemon cannot chmod them. It creates the subdirectory itself and controls its mode. + +`CLAW_WRAP_RUNTIME_DIR` only moves the socket, the auth file and the proxy auth token. The `env:` credential file stays at its default location, because the runtime directory here is shared with the agent. + +## Users, groups and modes + +The example runs the sidecar as UID 10001 and the agent as UID 1000, with `fsGroup: 2000` on the pod. Every container gets GID 2000 as a supplementary group, and the kubelet gives that group to the `emptyDir` and Secret volumes. + +```yaml +args: [daemon, --uid, "1000", --runtime-gid, "2000", --socket-mode, "0660", --auth-mode, "0640"] +``` + +| Flag | Effect | +| --- | --- | +| `--uid 1000` | Only peers with UID 1000 (the agent) are accepted. Everything else is rejected before HMAC is checked. | +| `--runtime-gid 2000` | Runtime dir becomes `0750`, socket and auth file are chgrp'd to 2000. | +| `--socket-mode 0660` | Group members can connect. Allowed: `0600`, `0660`. | +| `--auth-mode 0640` | Group members can read the HMAC secret. Allowed: `0600`, `0640`. | + +Resulting layout as seen from the agent: + +``` +drwxr-x--- 10001 2000 /run/claw-wrap/ipc +-rw-r----- 10001 2000 /run/claw-wrap/ipc/auth +srw-rw---- 10001 2000 /run/claw-wrap/ipc/secrets.sock +``` + +Running both containers as the same UID also works, with the default `0600` modes and no `--runtime-gid`. Separate UIDs are the safer default: if someone later turns on `shareProcessNamespace`, the agent still cannot read the sidecar's `/proc//environ`. + +## Security model + +Keep `shareProcessNamespace: false`. The agent must not see the sidecar's processes. + +The consequence is that claw-wrap cannot verify the caller's executable. Across PID namespaces `SO_PEERCRED` reports PID 0, so `/proc//exe` is not available. The daemon logs this once: + +``` +[INFO] caller executable cannot be verified (peer pid=0, e.g. a separate PID namespace); requests are authorized by UID + HMAC only +``` + +Leave `security.deny_unverified_caller_exe` at `false`; with `true` every request is rejected. + +What protects the tools then: + +1. Only containers that mount the `ipc` volume can reach the socket at all. +2. The peer UID must equal `--uid`. +3. Requests must be signed with the HMAC secret from the auth file and are replay-protected. +4. Arguments must pass the tool's `blocked_args` and `allowed_args`. + The caller's environment and working directory are ignored when the tool sets `request_env: []` and `working_dir`. +5. The tool itself may enforce its own policy (mcparcel's `toolPolicy`, for example). + +Points 2 and 3 identify "a process in the agent container", nothing finer. The agent holds the HMAC secret, so it can sign any request it likes: arguments, environment, working directory. For every tool in a sidecar: + +- Use `mode: allowlist` with `match: argv` rules; see [CONFIG.md](CONFIG.md#allowed_args-optional-requires-mode-allowlist). +- Set `request_env: []` (or a short list like `[TERM]`). Otherwise the agent can pass tool-specific variables, for example a config path or server URL that redirects the credentials. +- Set `working_dir`. Otherwise the tool runs in a directory the agent picks, which matters as soon as the sidecar shares a volume with the agent (think `.git/config` or `.npmrc` in an agent-writable workspace). + +Also set `automountServiceAccountToken: false` unless the agent needs the Kubernetes API. + +## Secrets + +Mount the Secret into the sidecar only, and read it with the `file:` credential backend: + +```yaml +credentials: + front_client_secret: + source: file:/etc/claw-wrap/secrets/front-client-secret +``` + +```yaml +volumes: + - name: secrets + secret: + secretName: claw-wrap-secrets + defaultMode: 0440 +``` + +Secret volume files are root-owned symlinks into a `..data` directory. `file:` follows symlinks and accepts files owned by root or the daemon UID, as long as they are not group-writable and not accessible by others. With `fsGroup` and `defaultMode: 0440` the files are `root:2000 0440`, readable by the sidecar. The agent is also in group 2000 but does not mount the volume. + +`file:` is read on every use, so rotated Secrets apply on the next call after the kubelet updates the volume. + +Map credentials to fixed env names with `env:`. claw-wrap redacts every injected credential value of 8 bytes or more from the tool's stdout and stderr. That only catches the exact value; an encoded copy gets through, so do not rely on it as the only control. The audit log never contains environment variables. + +## Config and hot reload + +Mount `wrappers.yaml` from a ConfigMap at `/etc/openclaw` (the default config path). Mount the whole ConfigMap, not a `subPath`: subPath mounts never receive updates. + +The kubelet updates ConfigMap volumes by swapping a `..data` symlink. claw-wrap watches for that swap and reloads, typically within a minute of `kubectl apply`. An invalid config is rejected and the previous one stays active. + +## Probes + +Use a startup probe and a readiness probe that check for the socket and auth file: + +```yaml +startupProbe: + exec: + command: ["sh", "-c", "test -S /run/claw-wrap/ipc/secrets.sock && test -s /run/claw-wrap/ipc/auth"] + periodSeconds: 1 + failureThreshold: 30 +``` + +As a native sidecar (`initContainers` with `restartPolicy: Always`, Kubernetes 1.29+), the agent container only starts after the sidecar's startup probe passes. On older clusters run claw-wrap as a regular container and let the agent retry until the socket exists. + +## Audit log + +```yaml +audit: + enabled: true + stdout: true + include_args: true +``` + +`stdout: true` writes one JSON object per call to the daemon's stdout. Daemon logs go to stderr, so a log pipeline can tell them apart. To keep a file instead, set `file:` to a path on a volume. `/dev/stdout` does not work there because the file logger refuses symlinks. + +## Writable paths + +With `readOnlyRootFilesystem: true` the sidecar needs: + +| Path | Why | +| --- | --- | +| `/run/claw-wrap` | Shared `ipc` emptyDir | +| `/tmp` | Large-output buffering and `config_file` temp dirs | +| `/var/lib/claw-wrap` | `HOME` and the tool's `working_dir`: tool state such as a daemon socket or token cache | + +## Tools that start a background daemon + +Some CLIs start a long-lived daemon on first use and talk to it afterwards. claw-wrap supports this when the daemon detaches properly: + +- Start it in a new session (`setsid`). claw-wrap runs each tool in its own process group and kills that group when the call ends, so a daemon left in the group dies with it. +- Point its stdin, stdout and stderr at `/dev/null` or a log file. If it keeps the tool's stdout or stderr open, claw-wrap stops reading one second after the tool exits, logs a warning and returns. The daemon then writes into a closed pipe, which kills most runtimes with `SIGPIPE`. + +The daemon inherits the tool's environment, including credentials from `env:`. Its state lives in the sidecar's filesystem (`HOME`, `working_dir`) and is never visible to the agent. + +## Non-interactive tools + +Set `use_stdin: false` on tools the agent calls. The tool reads from `/dev/null`, so a prompt fails immediately instead of waiting for input until `timeout` expires. It also turns off PTY mode. Without it, a tool that waits on stdin is killed at its `timeout` (SIGTERM, then SIGKILL after 5 seconds). + +## Troubleshooting + +| Symptom | Cause | +| --- | --- | +| `connect: no such file or directory` in the agent | `CLAW_WRAP_RUNTIME_DIR` differs between containers, or the sidecar is not up yet | +| `load secret: ... permission denied` | Agent not in the runtime group: check `fsGroup`, `--runtime-gid`, `--auth-mode 0640` | +| `unauthorized caller` | Agent UID does not match `--uid` | +| `invalid working directory` | The caller's cwd does not exist in the sidecar. Set `working_dir` on the tool. | +| Tool ignores a variable the agent sets | Expected with `request_env: []`; list the variable if the tool really needs it | +| `unknown tool` | No `tools:` entry with that name; the daemon also warns at startup when `tools` is empty | +| `chmod runtime dir: operation not permitted` | `CLAW_WRAP_RUNTIME_DIR` points at the emptyDir mount itself; use a subdirectory | +| Config changes not picked up | ConfigMap mounted with `subPath` | diff --git a/examples/kubernetes/deployment.yaml b/examples/kubernetes/deployment.yaml new file mode 100644 index 0000000..4d529f8 --- /dev/null +++ b/examples/kubernetes/deployment.yaml @@ -0,0 +1,136 @@ +# claw-wrap as a sidecar: the agent container holds no secrets and reaches +# wrapped tools only through the claw-wrap socket on a shared emptyDir. +# +# Prerequisites (see docs/KUBERNETES.md): +# kubectl create configmap claw-wrap-config --from-file=wrappers.yaml=examples/mcparcel/wrappers.yaml +# kubectl create secret generic claw-wrap-secrets \ +# --from-file=front-client-id=./front-client-id \ +# --from-file=front-client-secret=./front-client-secret +# +# Replace the two images: the sidecar image is FROM ghcr.io/dedene/claw-wrap +# plus the real tool binary; the agent image contains the claw-wrap binary and +# a symlink named after the tool. +apiVersion: apps/v1 +kind: Deployment +metadata: + name: agent +spec: + replicas: 1 + strategy: + type: Recreate + selector: + matchLabels: + app: agent + template: + metadata: + labels: + app: agent + spec: + automountServiceAccountToken: false + # Keep PID namespaces separate: the agent must not see the sidecar's + # processes (or their environment). Consequence: claw-wrap cannot verify + # the caller's executable (peer PID is 0); the boundary is the UID check + # plus the HMAC secret, and only containers that mount the ipc volume can + # reach the socket at all. + shareProcessNamespace: false + securityContext: + runAsNonRoot: true + # Shared group for the ipc volume and the Secret files. Every container + # in the pod gets it as a supplementary group. + fsGroup: 2000 + seccompProfile: + type: RuntimeDefault + initContainers: + # Native sidecar (Kubernetes >= 1.29): starts before the agent and keeps + # running. On older clusters, move it to `containers` and drop + # restartPolicy; the agent then has to retry until the socket exists. + - name: claw-wrap + image: registry.example.com/claw-wrap-mcparcel:1.0.0 + restartPolicy: Always + args: + - daemon + - --uid + - "1000" # the agent's UID; every other UID is rejected + - --runtime-gid + - "2000" # = fsGroup + - --socket-mode + - "0660" + - --auth-mode + - "0640" + env: + # Both containers must agree on this path. The daemon creates the + # directory itself (it cannot chmod the root-owned emptyDir mount). + - name: CLAW_WRAP_RUNTIME_DIR + value: /run/claw-wrap/ipc + securityContext: + runAsUser: 10001 + runAsGroup: 10001 + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + capabilities: + drop: ["ALL"] + startupProbe: + exec: + command: ["sh", "-c", "test -S /run/claw-wrap/ipc/secrets.sock && test -s /run/claw-wrap/ipc/auth"] + periodSeconds: 1 + failureThreshold: 30 + readinessProbe: + exec: + command: ["sh", "-c", "test -S /run/claw-wrap/ipc/secrets.sock && test -s /run/claw-wrap/ipc/auth"] + periodSeconds: 10 + resources: + requests: + cpu: 10m + memory: 32Mi + limits: + memory: 256Mi + volumeMounts: + - name: ipc + mountPath: /run/claw-wrap + # Mount the whole ConfigMap (no subPath) so updates reach the pod; + # claw-wrap reloads on the ..data symlink swap. + - name: config + mountPath: /etc/openclaw + readOnly: true + # Secrets are mounted into the sidecar only. + - name: secrets + mountPath: /etc/claw-wrap/secrets + readOnly: true + # HOME and tool working_dir: tool state (e.g. a daemon's socket or + # token cache) lives here, invisible to the agent. + - name: state + mountPath: /var/lib/claw-wrap + # Output buffering and config_file temp dirs. + - name: tmp + mountPath: /tmp + containers: + - name: agent + image: registry.example.com/agent:latest + env: + - name: CLAW_WRAP_RUNTIME_DIR + value: /run/claw-wrap/ipc + securityContext: + runAsUser: 1000 + runAsGroup: 1000 + allowPrivilegeEscalation: false + capabilities: + drop: ["ALL"] + volumeMounts: + - name: ipc + mountPath: /run/claw-wrap + volumes: + - name: ipc + emptyDir: + medium: Memory + sizeLimit: 1Mi + - name: config + configMap: + name: claw-wrap-config + - name: secrets + secret: + secretName: claw-wrap-secrets + defaultMode: 0440 # root:fsGroup, readable by the sidecar only (only it mounts this) + - name: state + emptyDir: {} + - name: tmp + emptyDir: {} From 386c3ef1af5b13022f5e3d3943006a2a4c7f7e12 Mon Sep 17 00:00:00 2001 From: Peter Dedene Date: Wed, 7 Oct 2026 11:23:06 +0200 Subject: [PATCH 5/6] build: bump toolchain to go1.26.8 and x/net to v0.55.0 govulncheck flags GO-2026-6218, -6090, -6089, -5972, -5856 (stdlib, fixed in go1.26.5/6) and GO-2026-5026 (x/net, fixed in v0.55.0). --- go.mod | 12 ++++++------ go.sum | 20 ++++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/go.mod b/go.mod index eaedb02..261c29f 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module claw-wrap go 1.26.1 -toolchain go1.26.4 +toolchain go1.26.8 require ( filippo.io/age v1.3.1 @@ -11,15 +11,15 @@ require ( github.com/fsnotify/fsnotify v1.9.0 github.com/itchyny/gojq v0.12.18 golang.org/x/sync v0.21.0 - golang.org/x/sys v0.43.0 - golang.org/x/term v0.42.0 + golang.org/x/sys v0.45.0 + golang.org/x/term v0.43.0 gopkg.in/yaml.v3 v3.0.1 ) require ( filippo.io/hpke v0.4.0 // indirect github.com/itchyny/timefmt-go v0.1.7 // indirect - golang.org/x/crypto v0.50.0 // indirect - golang.org/x/net v0.53.0 // indirect - golang.org/x/text v0.36.0 // indirect + golang.org/x/crypto v0.51.0 // indirect + golang.org/x/net v0.55.0 // indirect + golang.org/x/text v0.37.0 // indirect ) diff --git a/go.sum b/go.sum index 7db7b05..895f666 100644 --- a/go.sum +++ b/go.sum @@ -22,18 +22,18 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= -golang.org/x/crypto v0.50.0 h1:zO47/JPrL6vsNkINmLoo/PH1gcxpls50DNogFvB5ZGI= -golang.org/x/crypto v0.50.0/go.mod h1:3muZ7vA7PBCE6xgPX7nkzzjiUq87kRItoJQM1Yo8S+Q= -golang.org/x/net v0.53.0 h1:d+qAbo5L0orcWAr0a9JweQpjXF19LMXJE8Ey7hwOdUA= -golang.org/x/net v0.53.0/go.mod h1:JvMuJH7rrdiCfbeHoo3fCQU24Lf5JJwT9W3sJFulfgs= +golang.org/x/crypto v0.51.0 h1:IBPXwPfKxY7cWQZ38ZCIRPI50YLeevDLlLnyC5wRGTI= +golang.org/x/crypto v0.51.0/go.mod h1:8AdwkbraGNABw2kOX6YFPs3WM22XqI4EXEd8g+x7Oc8= +golang.org/x/net v0.55.0 h1:bcvxaJn3e1U6InsFWt1JUq1aSjnRxLzT2rtD2KfkDF8= +golang.org/x/net v0.55.0/go.mod h1:L5U2KuzuOe1lY7Z+aWVIKK6qEeJXnXV9yzGA+WCHJww= golang.org/x/sync v0.21.0 h1:HLII4xRRTtCRkxYp4HNFF0Js/Og6q2i++KXbg0gHCwM= golang.org/x/sync v0.21.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= -golang.org/x/sys v0.43.0 h1:Rlag2XtaFTxp19wS8MXlJwTvoh8ArU6ezoyFsMyCTNI= -golang.org/x/sys v0.43.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/term v0.42.0 h1:UiKe+zDFmJobeJ5ggPwOshJIVt6/Ft0rcfrXZDLWAWY= -golang.org/x/term v0.42.0/go.mod h1:Dq/D+snpsbazcBG5+F9Q1n2rXV8Ma+71xEjTRufARgY= -golang.org/x/text v0.36.0 h1:JfKh3XmcRPqZPKevfXVpI1wXPTqbkE5f7JA92a55Yxg= -golang.org/x/text v0.36.0/go.mod h1:NIdBknypM8iqVmPiuco0Dh6P5Jcdk8lJL0CUebqK164= +golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= +golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/term v0.43.0 h1:S4RLU2sB31O/NCl+zFN9Aru9A/Cq2aqKpTZJ6B+DwT4= +golang.org/x/term v0.43.0/go.mod h1:lrhlHNdQJHO+1qVYiHfFKVuVioJIheAc3fBSMFYEIsk= +golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= +golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= From f3b7ac4dcc105220c256bc81229d845b23989d82 Mon Sep 17 00:00:00 2001 From: Peter Dedene Date: Wed, 7 Oct 2026 12:19:04 +0200 Subject: [PATCH 6/6] fix(daemon): stop secret redaction from hiding prompts or leaking split secrets Split the output redactor into two stages. Literal secrets are replaced first and only a trailing proper prefix of a secret is held back, so output such as an interactive "Continue? [y/N] " prompt is released immediately instead of sitting in a 128-byte carry while the tool waits for input. redact_output regex rules then run on that output only, so they can no longer rewrite the start of a secret that is still arriving ("key=tok_" + "live_123456" with a tok -> *** rule leaked "_live_123456"). When one secret is a prefix of another, the longer one is awaited. --- docs/CONFIG.md | 2 + internal/daemon/executor_lifecycle_test.go | 15 ++++ internal/daemon/executor_secrets_test.go | 85 +++++++++++++++++++ internal/daemon/helper_process_test.go | 10 +++ internal/daemon/output_redactor.go | 99 +++++++++++++++++----- 5 files changed, 190 insertions(+), 21 deletions(-) diff --git a/docs/CONFIG.md b/docs/CONFIG.md index db3c587..42ba4e5 100644 --- a/docs/CONFIG.md +++ b/docs/CONFIG.md @@ -1071,6 +1071,8 @@ Behavior notes: - Works for both inline and file-backed output responses - Rules are applied in order - Invalid regex patterns are rejected during config validation +- To catch matches across chunk boundaries, the last 128 bytes are held back until more output arrives or the tool exits. A prompt without a trailing newline can stay invisible until then, so avoid `redact_output` on interactive tools +- Injected credential values are redacted before these rules run and hold back only a trailing partial match, so they do not delay prompts ### `working_dir` (optional) diff --git a/internal/daemon/executor_lifecycle_test.go b/internal/daemon/executor_lifecycle_test.go index 9c5c3f3..0a08c43 100644 --- a/internal/daemon/executor_lifecycle_test.go +++ b/internal/daemon/executor_lifecycle_test.go @@ -21,6 +21,10 @@ import ( // to simulate a slow consumer. var clientReadDelay time.Duration +// clientAnswer, when set, makes the test client send "y\n" on stdin as soon as +// the given prompt text has arrived on stdout. +var clientAnswer string + type execResult struct { stdout string stderr string @@ -71,6 +75,13 @@ func runHelperToolWith(t *testing.T, tool config.ToolDef, req protocol.ProxyRequ var res execResult var stdout, stderr strings.Builder dec := framing.NewDecoder(clientSide) + answered := false + answer := func() { + msg := protocol.WrapperMessage{Type: protocol.MsgTypeStdin, Data: base64.StdEncoding.EncodeToString([]byte("y\n"))} + if err := framing.NewNDJSONWriter(clientSide).Write(&msg); err != nil { + t.Errorf("send stdin: %v", err) + } + } readDone := make(chan struct{}) go func() { defer close(readDone) @@ -93,6 +104,10 @@ func runHelperToolWith(t *testing.T, tool config.ToolDef, req protocol.ProxyRequ } if msg.Type == protocol.MsgTypeStdout { stdout.Write(data) + if clientAnswer != "" && !answered && strings.Contains(stdout.String(), clientAnswer) { + answered = true + go answer() + } } else { stderr.Write(data) } diff --git a/internal/daemon/executor_secrets_test.go b/internal/daemon/executor_secrets_test.go index ada07ee..6c8b07b 100644 --- a/internal/daemon/executor_secrets_test.go +++ b/internal/daemon/executor_secrets_test.go @@ -106,3 +106,88 @@ func TestOutputRedactor_SecretsBeforeToolRules(t *testing.T) { t.Fatalf("redacted = %q, want key=[REDACTED]", got) } } + +// A prompt without a trailing newline must reach the caller while the tool +// waits for input, even when the tool has credentials to redact. +func TestExecutor_PromptVisibleWithCredentialRedaction(t *testing.T) { + secretPath := filepath.Join(t.TempDir(), "token") + if err := os.WriteFile(secretPath, []byte("tok_live_123456"), 0o600); err != nil { + t.Fatal(err) + } + cfg := &config.Config{Credentials: map[string]config.CredentialDef{ + "token": {Source: "file:" + secretPath}, + }} + tool := helperTool("prompt") + tool.Timeout = "3s" + tool.Env["API_TOKEN"] = "token" + + clientAnswer = "Continue? [y/N] " + t.Cleanup(func() { clientAnswer = "" }) + + res := runHelperToolWith(t, tool, protocol.ProxyRequest{}, cfg, nil) + if res.timeout { + t.Fatalf("prompt was held back by the redactor; tool timed out (stdout=%q)", res.stdout) + } + if res.stdout != "Continue? [y/N] answer:y\n" { + t.Fatalf("stdout = %q", res.stdout) + } +} + +func redactAll(r *OutputRedactor, chunks ...string) (emitted []string, total string) { + var b strings.Builder + for _, c := range chunks { + out := string(r.RedactChunk([]byte(c), false)) + emitted = append(emitted, out) + b.WriteString(out) + } + b.Write(r.RedactChunk(nil, true)) + return emitted, b.String() +} + +func TestOutputRedactor_ReleasesBytesThatCannotBeASecret(t *testing.T) { + r := NewOutputRedactorWithSecrets(nil, []string{"tok_live_123456"}) + + emitted, total := redactAll(r, "Continue? [y/N] ", "key=tok_", "live_123456 end", " and tok_x") + if emitted[0] != "Continue? [y/N] " { + t.Errorf("prompt held back: %q", emitted[0]) + } + if emitted[1] != "key=" { + t.Errorf("chunk 2 = %q, want only the part that cannot start the secret", emitted[1]) + } + if emitted[2] != "[REDACTED] end" { + t.Errorf("chunk 3 = %q", emitted[2]) + } + if total != "Continue? [y/N] key=[REDACTED] end and tok_x" { + t.Errorf("total = %q", total) + } +} + +// Custom rules must never see a secret that is still arriving. +func TestOutputRedactor_ToolRulesCannotBreakSplitSecret(t *testing.T) { + cfg := &config.Config{Tools: map[string]config.ToolDef{ + "t": {Binary: "/usr/bin/true", RedactOutput: []config.ToolRedactRule{{Pattern: "tok", Replace: "***"}}}, + }} + if err := cfg.Validate(); err != nil { + t.Fatal(err) + } + r := NewOutputRedactorWithSecrets(cfg.Tools["t"].RedactOutput, []string{"tok_live_123456"}) + if _, total := redactAll(r, "key=tok_", "live_123456"); total != "key=[REDACTED]" { + t.Fatalf("total = %q, want key=[REDACTED]", total) + } + r = NewOutputRedactorWithSecrets(cfg.Tools["t"].RedactOutput, []string{"tok_live_123456"}) + if _, total := redactAll(r, "a tok b"); total != "a *** b" { + t.Fatalf("tool rule on non-secret text: %q", total) + } +} + +// When one secret is a prefix of another, wait for the longer one. +func TestOutputRedactor_PrefixSecrets(t *testing.T) { + r := NewOutputRedactorWithSecrets(nil, []string{"abcdefgh", "abcdefgh12"}) + if _, total := redactAll(r, "x abcdefgh", "12 y"); total != "x [REDACTED] y" { + t.Fatalf("total = %q", total) + } + r = NewOutputRedactorWithSecrets(nil, []string{"abcdefgh", "abcdefgh12"}) + if _, total := redactAll(r, "x abcdefgh", "1"); total != "x [REDACTED]1" { + t.Fatalf("shorter secret must still be redacted once the longer one is ruled out: %q", total) + } +} diff --git a/internal/daemon/helper_process_test.go b/internal/daemon/helper_process_test.go index 161d8d2..3f33168 100644 --- a/internal/daemon/helper_process_test.go +++ b/internal/daemon/helper_process_test.go @@ -50,6 +50,16 @@ func runHelper(mode string, args []string) int { _ = w.Flush() return 0 + case "prompt": + // Interactive confirmation: no trailing newline, then wait for input. + fmt.Fprint(os.Stdout, "Continue? [y/N] ") + line, err := bufio.NewReader(os.Stdin).ReadString('\n') + if err != nil { + return 3 + } + fmt.Fprint(os.Stdout, "answer:"+line) + return 0 + case "read-stdin": // Mimics a CLI that prompts for input; EOF means "non-interactive". line, err := bufio.NewReader(os.Stdin).ReadString('\n') diff --git a/internal/daemon/output_redactor.go b/internal/daemon/output_redactor.go index 462dc83..a5c043c 100644 --- a/internal/daemon/output_redactor.go +++ b/internal/daemon/output_redactor.go @@ -1,6 +1,7 @@ package daemon import ( + "bytes" "regexp" "sort" @@ -14,9 +15,18 @@ type outputRedactionRule struct { re *regexp.Regexp } -// OutputRedactor applies regex-based output redaction with bounded overlap -// so matches can be detected across chunk boundaries. +// OutputRedactor redacts streamed tool output in two stages: +// +// 1. Literal secrets (injected credential values). Only the trailing bytes +// that could still be the start of a secret are held back; everything +// else is released at once, so prompts without a newline stay visible. +// 2. Regex rules from redact_output, with a bounded overlap carry so matches +// spanning chunks are found. They only see stage-1 output, which never +// contains part of a secret that is still arriving. type OutputRedactor struct { + secrets [][]byte // longest first + secretCarry []byte + rules []outputRedactionRule carry []byte overlap int @@ -75,35 +85,82 @@ func NewOutputRedactorWithSecrets(rules []config.ToolRedactRule, secrets []strin sort.Slice(literals, func(i, j int) bool { return len(literals[i]) > len(literals[j]) }) if r == nil { - r = &OutputRedactor{overlap: defaultRedactionOverlapBytes} + r = &OutputRedactor{} } - // Literal secrets run before the tool's own rules: a user regex that - // rewrites part of a secret would otherwise leave a fragment the literal - // no longer matches. - replace := []byte(config.DefaultRedactReplacement) - secretRules := make([]outputRedactionRule, 0, len(literals)) - for _, s := range literals { - secretRules = append(secretRules, outputRedactionRule{ - re: regexp.MustCompile(regexp.QuoteMeta(s)), - replace: replace, - }) - // The carry must hold all but the last byte of a secret for matches - // that span chunk boundaries. - if len(s) > r.overlap { - r.overlap = len(s) - } + for _, lit := range literals { + r.secrets = append(r.secrets, []byte(lit)) } - r.rules = append(secretRules, r.rules...) return r } -// RedactChunk redacts one output chunk. When finalize is false it keeps a -// bounded carry to catch matches that span chunk boundaries. +// RedactChunk redacts one output chunk. When finalize is false it may keep +// a carry to catch matches that span chunk boundaries. func (r *OutputRedactor) RedactChunk(data []byte, finalize bool) []byte { if r == nil { return data } + if len(r.secrets) > 0 { + data = r.redactSecrets(data, finalize) + } + if len(r.rules) == 0 { + return data + } + return r.redactRules(data, finalize) +} + +// redactSecrets replaces literal secrets and holds back only a trailing +// partial match. +func (r *OutputRedactor) redactSecrets(data []byte, finalize bool) []byte { + buf := append(r.secretCarry, data...) + r.secretCarry = nil + if len(buf) == 0 { + return nil + } + + replace := []byte(config.DefaultRedactReplacement) + out := make([]byte, 0, len(buf)) + for i := 0; i < len(buf); { + rest := buf[i:] + // A tail that is a proper prefix of a secret might complete in the + // next chunk. Wait for it, even if a shorter secret already matches. + if !finalize && r.isSecretPrefix(rest) { + r.secretCarry = append([]byte(nil), rest...) + break + } + if n := r.matchSecret(rest); n > 0 { + out = append(out, replace...) + i += n + continue + } + out = append(out, buf[i]) + i++ + } + return out +} + +// isSecretPrefix reports whether b is a proper prefix of any secret. +func (r *OutputRedactor) isSecretPrefix(b []byte) bool { + for _, s := range r.secrets { + if len(b) < len(s) && bytes.HasPrefix(s, b) { + return true + } + } + return false +} + +// matchSecret returns the length of the longest secret b starts with, or 0. +func (r *OutputRedactor) matchSecret(b []byte) int { + for _, s := range r.secrets { // longest first + if bytes.HasPrefix(b, s) { + return len(s) + } + } + return 0 +} + +// redactRules applies the regex rules with a bounded overlap carry. +func (r *OutputRedactor) redactRules(data []byte, finalize bool) []byte { if len(data) == 0 && !finalize { return nil }