diff --git a/.gitignore b/.gitignore index d95ab34..487c506 100644 --- a/.gitignore +++ b/.gitignore @@ -29,4 +29,8 @@ api-contracts .worktrees # GoReleaser output -dist/ \ No newline at end of file +dist/ + +# Connect log file (retask sandbox connect) +retask.log +retask.log.* diff --git a/go.mod b/go.mod index 1e2c3e1..57df382 100644 --- a/go.mod +++ b/go.mod @@ -1,12 +1,12 @@ module github.com/nwebxyz/retask-cli -go 1.26.4 +go 1.26.5 require ( connectrpc.com/connect v1.20.0 github.com/charmbracelet/lipgloss v1.1.0 github.com/coder/websocket v1.8.14 - github.com/hoaitan/agentfleet v0.6.27 + github.com/hoaitan/agentfleet v0.8.0 github.com/spf13/cobra v1.10.2 github.com/stretchr/testify v1.11.1 golang.org/x/term v0.43.0 diff --git a/go.sum b/go.sum index d686cc2..22d8e49 100644 --- a/go.sum +++ b/go.sum @@ -28,8 +28,8 @@ github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/hinshun/vt10x v0.0.0-20220301184237-5011da428d02 h1:AgcIVYPa6XJnU3phs104wLj8l5GEththEw6+F79YsIY= github.com/hinshun/vt10x v0.0.0-20220301184237-5011da428d02/go.mod h1:Q48J4R4DvxnHolD5P8pOtXigYlRuPLGl6moFx3ulM68= -github.com/hoaitan/agentfleet v0.6.27 h1:mAsBWhaZnwC6eUhxtZCgV4GXfcGgASUStHqRgBU3Gmk= -github.com/hoaitan/agentfleet v0.6.27/go.mod h1:nYrGTu9Xzn4fwzlYA9Na8g2PuBKBgYov5uy0mj1J1uM= +github.com/hoaitan/agentfleet v0.8.0 h1:Xj6C1KkzknM4t+TBFo6fZT1X9moOdRcY9xPLQPCdrtU= +github.com/hoaitan/agentfleet v0.8.0/go.mod h1:PtXaJtEF+1UfL2yJbkSll+Y5Vj0LKw8Ej96PqbhRdK4= github.com/inconshreveable/go-update v0.0.0-20160112193335-8152e7eb6ccf h1:WfD7VjIE6z8dIvMsI4/s+1qr5EL+zoIGev1BQj1eoJ8= github.com/inconshreveable/go-update v0.0.0-20160112193335-8152e7eb6ccf/go.mod h1:hyb9oH7vZsitZCiBt0ZvifOrB+qc8PS5IiilCIb87rg= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= diff --git a/internal/cmd/helpcmd/command.go b/internal/cmd/helpcmd/command.go index 481da99..5dde223 100644 --- a/internal/cmd/helpcmd/command.go +++ b/internal/cmd/helpcmd/command.go @@ -160,7 +160,7 @@ func buildManifest() manifest { {Command: "retask sandbox session update", Description: "Partial update a session", Flags: []string{"--name", "--seed-nrn", "--seed-prompt"}, Example: "retask sandbox session update --name \"My Session\""}, {Command: "retask sandbox session stop", Description: "Stop a session", Example: "retask sandbox session stop "}, {Command: "retask sandbox session delete", Description: "Delete a session", Example: "retask sandbox session delete "}, - {Command: "retask sandbox connect", Description: "Connect this machine as a Private VM sandbox (long-running)", Flags: []string{"--mode", "--auto-open", "--no-auto-respond", "--session-buffer"}, Example: "retask sandbox connect "}, + {Command: "retask sandbox connect", Description: "Connect this machine as a Private VM sandbox (long-running). Logs go to the TUI (stderr when headless) and to retask.log in the current folder, which rotates into retask.log.1 ... retask.log.N", Flags: []string{"--mode", "--auto-open", "--no-auto-respond", "--session-buffer", "--log-file", "--no-log-file", "--log-max-size", "--log-backups", "--no-log-path"}, Example: "retask sandbox connect "}, {Command: "retask sandbox attach", Description: "Attach terminal to a running local session", Example: "retask sandbox attach "}, {Command: "retask agent list", Description: "List agents", Flags: []string{"--role"}, Example: "retask agent list --role ROLE_TASK_PROCESSOR"}, {Command: "retask agent get", Description: "Get an agent by ID", Example: "retask agent get "}, diff --git a/internal/cmd/sandbox/connect.go b/internal/cmd/sandbox/connect.go index 47662e4..0ddcb7d 100644 --- a/internal/cmd/sandbox/connect.go +++ b/internal/cmd/sandbox/connect.go @@ -51,6 +51,7 @@ func newConnectCommand(gf *flags.Global) *cobra.Command { var autoOpen bool var noAutoRespond bool var sessionBuffer string + var logFlags logFileFlags cmd := &cobra.Command{ Use: "connect ", Short: "Connect this machine as a Private VM sandbox", @@ -70,18 +71,36 @@ session lane reconnect on their own with exponential backoff. While a session lane is down, its output is buffered (drop-oldest) and flushed to the proxy on reconnect, so the viewer keeps the most recent output across the gap. +Every log line goes to two places: the TUI log panel (stderr in headless mode) +and retask.log in the current folder. The log file rotates Unix-style — the live +file keeps its name and older generations shift down through retask.log.1, +retask.log.2, ... up to --log-backups. The TUI Logs divider shows the active path. + Flags: --mode string Running mode: auto, tui, headless (default: auto) --auto-open Auto-open a terminal tab for each new session (default: false) --no-auto-respond Disable auto-accepting known agent startup prompts (default: false) --session-buffer Per-session output retained across a session-lane drop, flushed on reconnect (default: 10MB). 0 disables buffering. Accepts 512KB, 10MB, ... + --log-file string Log file written alongside the TUI/stderr output, relative to the + current folder (default: retask.log) + --no-log-file Do not write a log file; log to the TUI/stderr only (default: false) + --log-max-size Rotate the log file once it exceeds this size (default: 10MB). + 0 disables rotation. Accepts 512KB, 10MB, ... + --log-backups int Rotated log generations kept, retask.log.1 ... retask.log.N + (default: 5). 0 keeps none + --no-log-path Hide the log file path from the TUI Logs divider (default: false) Environment: SANDBOX_PROXY_ENDPOINT Proxy base URL (default: https://sandbox-proxy.prd.nweb.app/) RETASK_SANDBOX_AUTO_OPEN_SESSION=1 Enable auto-open without the flag RETASK_SANDBOX_NO_AUTO_RESPOND=1 Disable prompt auto-response without the flag - RETASK_SANDBOX_SESSION_BUFFER Session output buffer size (overridden by --session-buffer)`, + RETASK_SANDBOX_SESSION_BUFFER Session output buffer size (overridden by --session-buffer) + RETASK_SANDBOX_LOG_FILE Log file path (overridden by --log-file) + RETASK_SANDBOX_NO_LOG_FILE=1 Disable the log file without the flag + RETASK_SANDBOX_LOG_MAX_SIZE Log rotation threshold (overridden by --log-max-size) + RETASK_SANDBOX_LOG_BACKUPS Rotated generations kept (overridden by --log-backups) + RETASK_SANDBOX_NO_LOG_PATH=1 Hide the log path from the TUI without the flag`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if mode != "auto" && mode != "tui" && mode != "headless" { @@ -131,14 +150,38 @@ Environment: useTUI := mode == "tui" || (mode == "auto" && term.IsTerminal(int(os.Stdout.Fd()))) + // agentfleet config. + fleetCfg := agentfleet.DefaultConfig() + fleetCfg.Agent = sessionAgentConfig() + fleetCfg.TUI.Title = makeTitleFunc(sbResp.Msg.Name, sbResp.Msg.SandboxId) + fleetCfg.TUI.TitleRight = makeConnStatusFunc(&rawConnState) + fleetCfg.TUI.AutoOpen = autoOpen || os.Getenv("RETASK_SANDBOX_AUTO_OPEN_SESSION") == "1" + fleetCfg.TUI.FilterLines = filterLines + + logCfg, showLogPath, err := logFlags.resolve(cmd) + if err != nil { + return err + } + // A disabled log file yields a nil *LogFile, which discards writes, + // so the tee below needs no branch for it. + logFile, err := agentfleet.OpenLogFile(logCfg) + if err != nil { + return err + } + defer logFile.Close() //nolint:errcheck + // LogBuffer captures all events; in TUI mode it feeds the log panel, - // in headless mode it drains to stderr so output is identical. + // in headless mode it drains to stderr so output is identical. Either + // way the same lines are appended to the log file. logBuf := agentfleet.NewLogBuffer(500) - var logOut io.Writer = os.Stderr + var termOut io.Writer = os.Stderr if useTUI { - logOut = logBuf + termOut = logBuf + fleetCfg.TUI.Log = logBuf + fleetCfg.TUI.LogPath = logFile.Path() + fleetCfg.TUI.ShowLogPath = showLogPath } - logger := slog.New(slog.NewTextHandler(logOut, &slog.HandlerOptions{ + logger := slog.New(slog.NewTextHandler(io.MultiWriter(termOut, logFile), &slog.HandlerOptions{ Level: slog.LevelInfo, })) @@ -150,18 +193,9 @@ Environment: "mode", mode, "tui", useTUI, "proxy", wsBase, + "log_file", logFile.Path(), ) - // agentfleet config. - fleetCfg := agentfleet.DefaultConfig() - fleetCfg.Agent = sessionAgentConfig() - fleetCfg.TUI.Title = makeTitleFunc(sbResp.Msg.Name, sbResp.Msg.SandboxId) - fleetCfg.TUI.TitleRight = makeConnStatusFunc(&rawConnState) - fleetCfg.TUI.AutoOpen = autoOpen || os.Getenv("RETASK_SANDBOX_AUTO_OPEN_SESSION") == "1" - fleetCfg.TUI.FilterLines = filterLines - if useTUI { - fleetCfg.TUI.Log = logBuf - } fleet := agentfleet.NewFleet(fleetCfg.Fleet) baseDir, err := os.Getwd() @@ -219,6 +253,7 @@ Environment: cmd.Flags().BoolVar(&autoOpen, "auto-open", false, "Auto-open a terminal tab for each new session") cmd.Flags().BoolVar(&noAutoRespond, "no-auto-respond", false, "Disable auto-accepting known agent startup prompts (e.g. folder-trust)") cmd.Flags().StringVar(&sessionBuffer, "session-buffer", "10MB", "Per-session output buffered across a session-lane drop (drop-oldest), flushed on reconnect. 0 disables. Accepts 512KB, 10MB, ...") + logFlags.register(cmd) return cmd } diff --git a/internal/cmd/sandbox/datalane.go b/internal/cmd/sandbox/datalane.go index 88a1d37..69d114a 100644 --- a/internal/cmd/sandbox/datalane.go +++ b/internal/cmd/sandbox/datalane.go @@ -142,7 +142,9 @@ func (dl *DataLane) connectOnce(ctx context.Context) (established bool, err erro conn, _, err := websocket.Dial(ctx, dialURL, nil) if err != nil { - return false, err + // The dial error quotes dialURL, JWT and all — scrub it before it + // reaches the log panel or retask.log. + return false, redactErr(err) } established = true defer conn.CloseNow() //nolint:errcheck diff --git a/internal/cmd/sandbox/logfileflags.go b/internal/cmd/sandbox/logfileflags.go new file mode 100644 index 0000000..5e37881 --- /dev/null +++ b/internal/cmd/sandbox/logfileflags.go @@ -0,0 +1,92 @@ +// internal/cmd/sandbox/logfileflags.go +package sandbox + +import ( + "fmt" + "os" + "strconv" + + agentfleet "github.com/hoaitan/agentfleet" + "github.com/spf13/cobra" +) + +// Defaults for the on-disk connect log. The file lives in the folder the +// command was started from — for a sandbox that is the session folder — so the +// log sits next to the work it describes. +const ( + defaultLogFileName = "retask.log" + defaultLogMaxSize = "10MB" + defaultLogBackups = 5 +) + +// logFileFlags holds the connect flags controlling the on-disk log file and +// whether the TUI shows its path. +type logFileFlags struct { + path string + disabled bool + maxSize string + backups int + hidePath bool +} + +// register binds the log flags to cmd. +func (f *logFileFlags) register(cmd *cobra.Command) { + cmd.Flags().StringVar(&f.path, "log-file", defaultLogFileName, + "Log file written alongside the TUI/stderr output, relative to the current folder") + cmd.Flags().BoolVar(&f.disabled, "no-log-file", false, + "Do not write a log file; log to the TUI/stderr only") + cmd.Flags().StringVar(&f.maxSize, "log-max-size", defaultLogMaxSize, + "Rotate the log file once it exceeds this size. 0 disables rotation. Accepts 512KB, 10MB, ...") + cmd.Flags().IntVar(&f.backups, "log-backups", defaultLogBackups, + "Rotated log generations kept (retask.log.1 ... retask.log.N). 0 keeps none") + cmd.Flags().BoolVar(&f.hidePath, "no-log-path", false, + "Hide the log file path from the TUI Logs divider") +} + +// resolve applies the environment fallbacks and returns the agentfleet log file +// config plus whether the TUI should show the path. A flag always wins over its +// environment variable. +func (f *logFileFlags) resolve(cmd *cobra.Command) (cfg agentfleet.LogFileConfig, showPath bool, err error) { + path := f.path + if v, ok := os.LookupEnv("RETASK_SANDBOX_LOG_FILE"); ok && !cmd.Flags().Changed("log-file") { + path = v + } + disabled := f.disabled + if os.Getenv("RETASK_SANDBOX_NO_LOG_FILE") == "1" && !cmd.Flags().Changed("no-log-file") { + disabled = true + } + + maxSize := f.maxSize + if v := os.Getenv("RETASK_SANDBOX_LOG_MAX_SIZE"); v != "" && !cmd.Flags().Changed("log-max-size") { + maxSize = v + } + maxBytes, err := parseByteSize(maxSize) + if err != nil { + return cfg, false, fmt.Errorf("--log-max-size: %w", err) + } + + backups := f.backups + if v := os.Getenv("RETASK_SANDBOX_LOG_BACKUPS"); v != "" && !cmd.Flags().Changed("log-backups") { + backups, err = strconv.Atoi(v) + if err != nil { + return cfg, false, fmt.Errorf("RETASK_SANDBOX_LOG_BACKUPS: invalid count %q", v) + } + } + if backups < 0 { + return cfg, false, fmt.Errorf("--log-backups: must not be negative") + } + + showPath = !f.hidePath + if os.Getenv("RETASK_SANDBOX_NO_LOG_PATH") == "1" && !cmd.Flags().Changed("no-log-path") { + showPath = false + } + + // An empty path is the same intent as --no-log-file, so both spellings work. + cfg = agentfleet.LogFileConfig{ + Enabled: !disabled && path != "", + Path: path, + MaxBytes: int64(maxBytes), + Backups: backups, + } + return cfg, showPath, nil +} diff --git a/internal/cmd/sandbox/logfileflags_test.go b/internal/cmd/sandbox/logfileflags_test.go new file mode 100644 index 0000000..22a24a6 --- /dev/null +++ b/internal/cmd/sandbox/logfileflags_test.go @@ -0,0 +1,234 @@ +package sandbox + +import ( + "io" + "log/slog" + "os" + "path/filepath" + "strings" + "testing" + + agentfleet "github.com/hoaitan/agentfleet" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// newLogFlagsCmd builds a command with the log flags registered and the given +// argv applied, mirroring how cobra hands them to connect's RunE. +func newLogFlagsCmd(t *testing.T, argv ...string) (*cobra.Command, *logFileFlags) { + t.Helper() + f := &logFileFlags{} + cmd := &cobra.Command{Use: "connect", RunE: func(*cobra.Command, []string) error { return nil }} + f.register(cmd) + require.NoError(t, cmd.ParseFlags(argv)) + return cmd, f +} + +func TestLogFileFlagsDefaults(t *testing.T) { + cmd, f := newLogFlagsCmd(t) + + cfg, showPath, err := f.resolve(cmd) + require.NoError(t, err) + + assert.True(t, cfg.Enabled, "the log file is written by default") + assert.Equal(t, defaultLogFileName, cfg.Path) + assert.Equal(t, int64(10*1024*1024), cfg.MaxBytes) + assert.Equal(t, defaultLogBackups, cfg.Backups) + assert.True(t, showPath, "the TUI shows the log path by default") +} + +func TestLogFileFlagsOverrides(t *testing.T) { + cmd, f := newLogFlagsCmd(t, + "--log-file", "/var/log/retask.log", + "--log-max-size", "512KB", + "--log-backups", "2", + "--no-log-path", + ) + + cfg, showPath, err := f.resolve(cmd) + require.NoError(t, err) + + assert.True(t, cfg.Enabled) + assert.Equal(t, "/var/log/retask.log", cfg.Path) + assert.Equal(t, int64(512*1024), cfg.MaxBytes) + assert.Equal(t, 2, cfg.Backups) + assert.False(t, showPath) +} + +func TestLogFileFlagsNoLogFile(t *testing.T) { + cmd, f := newLogFlagsCmd(t, "--no-log-file") + + cfg, _, err := f.resolve(cmd) + require.NoError(t, err) + assert.False(t, cfg.Enabled) +} + +func TestLogFileFlagsEmptyPathDisables(t *testing.T) { + cmd, f := newLogFlagsCmd(t, "--log-file", "") + + cfg, _, err := f.resolve(cmd) + require.NoError(t, err) + assert.False(t, cfg.Enabled, "an empty path means the same as --no-log-file") +} + +func TestLogFileFlagsZeroMaxSizeDisablesRotation(t *testing.T) { + cmd, f := newLogFlagsCmd(t, "--log-max-size", "0") + + cfg, _, err := f.resolve(cmd) + require.NoError(t, err) + assert.Equal(t, int64(0), cfg.MaxBytes) +} + +func TestLogFileFlagsEnvFallbacks(t *testing.T) { + t.Setenv("RETASK_SANDBOX_LOG_FILE", "/env/retask.log") + t.Setenv("RETASK_SANDBOX_LOG_MAX_SIZE", "1MB") + t.Setenv("RETASK_SANDBOX_LOG_BACKUPS", "9") + t.Setenv("RETASK_SANDBOX_NO_LOG_PATH", "1") + + cmd, f := newLogFlagsCmd(t) + + cfg, showPath, err := f.resolve(cmd) + require.NoError(t, err) + + assert.Equal(t, "/env/retask.log", cfg.Path) + assert.Equal(t, int64(1024*1024), cfg.MaxBytes) + assert.Equal(t, 9, cfg.Backups) + assert.False(t, showPath) +} + +func TestLogFileFlagsEnvNoLogFile(t *testing.T) { + t.Setenv("RETASK_SANDBOX_NO_LOG_FILE", "1") + + cmd, f := newLogFlagsCmd(t) + + cfg, _, err := f.resolve(cmd) + require.NoError(t, err) + assert.False(t, cfg.Enabled) +} + +func TestLogFileFlagsFlagBeatsEnv(t *testing.T) { + t.Setenv("RETASK_SANDBOX_LOG_FILE", "/env/retask.log") + t.Setenv("RETASK_SANDBOX_LOG_MAX_SIZE", "1MB") + t.Setenv("RETASK_SANDBOX_LOG_BACKUPS", "9") + t.Setenv("RETASK_SANDBOX_NO_LOG_PATH", "1") + + cmd, f := newLogFlagsCmd(t, + "--log-file", "/flag/retask.log", + "--log-max-size", "2MB", + "--log-backups", "1", + ) + + cfg, showPath, err := f.resolve(cmd) + require.NoError(t, err) + + assert.Equal(t, "/flag/retask.log", cfg.Path) + assert.Equal(t, int64(2*1024*1024), cfg.MaxBytes) + assert.Equal(t, 1, cfg.Backups) + assert.False(t, showPath, "--no-log-path was not passed, so the env var still applies") +} + +func TestLogFileFlagsEmptyEnvPathDisables(t *testing.T) { + t.Setenv("RETASK_SANDBOX_LOG_FILE", "") + + cmd, f := newLogFlagsCmd(t) + + cfg, _, err := f.resolve(cmd) + require.NoError(t, err) + assert.False(t, cfg.Enabled, "an explicitly empty env path disables the file") +} + +func TestLogFileFlagsInvalidValues(t *testing.T) { + cmd, f := newLogFlagsCmd(t, "--log-max-size", "10XB") + _, _, err := f.resolve(cmd) + require.Error(t, err) + assert.Contains(t, err.Error(), "--log-max-size") + + cmd, f = newLogFlagsCmd(t, "--log-backups=-1") + _, _, err = f.resolve(cmd) + require.Error(t, err) + assert.Contains(t, err.Error(), "--log-backups") + + t.Setenv("RETASK_SANDBOX_LOG_BACKUPS", "many") + cmd, f = newLogFlagsCmd(t) + _, _, err = f.resolve(cmd) + require.Error(t, err) + assert.Contains(t, err.Error(), "RETASK_SANDBOX_LOG_BACKUPS") +} + +// TestConnectLogTeeWritesBothPlaces covers the wiring connect performs: the +// same slog handler feeds the TUI buffer and the rotating file. +func TestConnectLogTeeWritesBothPlaces(t *testing.T) { + dir := t.TempDir() + cmd, f := newLogFlagsCmd(t, "--log-file", filepath.Join(dir, "retask.log")) + + cfg, showPath, err := f.resolve(cmd) + require.NoError(t, err) + assert.True(t, showPath) + + logFile, err := agentfleet.OpenLogFile(cfg) + require.NoError(t, err) + defer logFile.Close() //nolint:errcheck + + logBuf := agentfleet.NewLogBuffer(500) + logger := slog.New(slog.NewTextHandler(io.MultiWriter(logBuf, logFile), &slog.HandlerOptions{Level: slog.LevelInfo})) + logger.Info("retask sandbox connect", "sandbox_id", "sandbox_abc123") + + lines := logBuf.Lines() + require.Len(t, lines, 1) + assert.Contains(t, lines[0], "sandbox_abc123") + + onDisk, err := os.ReadFile(filepath.Join(dir, "retask.log")) + require.NoError(t, err) + assert.Contains(t, string(onDisk), "sandbox_abc123") + assert.Equal(t, strings.TrimRight(string(onDisk), "\n"), lines[0]) +} + +// TestRotationProducesNumberedGenerations pins the file names an operator sees +// in the sandbox folder once the log outgrows --log-max-size. +func TestRotationProducesNumberedGenerations(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + + cmd, f := newLogFlagsCmd(t, "--log-max-size", "1KB", "--log-backups", "2") + cfg, _, err := f.resolve(cmd) + require.NoError(t, err) + + logFile, err := agentfleet.OpenLogFile(cfg) + require.NoError(t, err) + defer logFile.Close() //nolint:errcheck + + logger := slog.New(slog.NewTextHandler(logFile, &slog.HandlerOptions{Level: slog.LevelInfo})) + for i := 0; i < 200; i++ { + logger.Info("session output flushed", "session_id", "session_0123456789abcdef", "bytes", i) + } + + assert.FileExists(t, filepath.Join(dir, "retask.log")) + assert.FileExists(t, filepath.Join(dir, "retask.log.1")) + assert.FileExists(t, filepath.Join(dir, "retask.log.2")) + assert.NoFileExists(t, filepath.Join(dir, "retask.log.3"), "--log-backups caps the generations kept") +} + +// TestDisabledLogFileWritesNowhere confirms the nil *LogFile stays a safe tee +// target, so connect needs no branch when the file is turned off. +func TestDisabledLogFileWritesNowhere(t *testing.T) { + dir := t.TempDir() + t.Chdir(dir) + + cmd, f := newLogFlagsCmd(t, "--no-log-file") + cfg, _, err := f.resolve(cmd) + require.NoError(t, err) + + logFile, err := agentfleet.OpenLogFile(cfg) + require.NoError(t, err) + assert.Equal(t, "", logFile.Path()) + + logBuf := agentfleet.NewLogBuffer(500) + logger := slog.New(slog.NewTextHandler(io.MultiWriter(logBuf, logFile), &slog.HandlerOptions{Level: slog.LevelInfo})) + logger.Info("still reaches the panel") + + assert.Len(t, logBuf.Lines(), 1) + entries, err := os.ReadDir(dir) + require.NoError(t, err) + assert.Empty(t, entries, "no log file is created") +} diff --git a/internal/cmd/sandbox/redact.go b/internal/cmd/sandbox/redact.go new file mode 100644 index 0000000..3e9b76f --- /dev/null +++ b/internal/cmd/sandbox/redact.go @@ -0,0 +1,41 @@ +// internal/cmd/sandbox/redact.go +package sandbox + +import "regexp" + +// tokenParamRe matches a token query parameter and its value. Both lane URLs +// carry the JWT that way, and a failed dial reports the URL it tried — so the +// raw token would otherwise reach the log panel and retask.log. +var tokenParamRe = regexp.MustCompile(`([?&]token=)[^&"\s]*`) + +// redactedToken replaces the token value in log output. +const redactedToken = "REDACTED" + +// redactToken blanks the value of every token query parameter in s. +func redactToken(s string) string { + return tokenParamRe.ReplaceAllString(s, "${1}"+redactedToken) +} + +// redactErr returns err with any token query parameter in its message redacted. +// The original error stays reachable through Unwrap, so errors.Is and errors.As +// keep working on the wrapped chain. +func redactErr(err error) error { + if err == nil { + return nil + } + msg := err.Error() + redacted := redactToken(msg) + if redacted == msg { + return err + } + return &redactedError{msg: redacted, err: err} +} + +// redactedError presents a scrubbed message while preserving the error chain. +type redactedError struct { + msg string + err error +} + +func (e *redactedError) Error() string { return e.msg } +func (e *redactedError) Unwrap() error { return e.err } diff --git a/internal/cmd/sandbox/redact_test.go b/internal/cmd/sandbox/redact_test.go new file mode 100644 index 0000000..a064622 --- /dev/null +++ b/internal/cmd/sandbox/redact_test.go @@ -0,0 +1,82 @@ +package sandbox + +import ( + "errors" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const fakeJWT = "eyJhbGciOiJSUzI1NiJ9.eyJzdWIiOiJ1c2VyIn0.c2lnbmF0dXJl-_x" + +func TestRedactTokenInLaneURLs(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + { + name: "data lane url", + in: "wss://proxy/ws/data-lane?sandbox_id=sb_1&token=" + fakeJWT + "&client_version=0.23.0", + want: "wss://proxy/ws/data-lane?sandbox_id=sb_1&token=REDACTED&client_version=0.23.0", + }, + { + name: "session lane url, token last", + in: "wss://proxy/ws/session-lane?sandbox_id=sb_1&session_id=s_1&token=" + fakeJWT, + want: "wss://proxy/ws/session-lane?sandbox_id=sb_1&session_id=s_1&token=REDACTED", + }, + { + name: "token as the first parameter", + in: "wss://proxy/ws?token=" + fakeJWT + "&sandbox_id=sb_1", + want: "wss://proxy/ws?token=REDACTED&sandbox_id=sb_1", + }, + { + name: "quoted url inside an error message", + in: `failed to dial: Get "wss://proxy/ws?token=` + fakeJWT + `": connection refused`, + want: `failed to dial: Get "wss://proxy/ws?token=REDACTED": connection refused`, + }, + { + name: "two urls in one message", + in: "a?token=" + fakeJWT + " and b?token=" + fakeJWT, + want: "a?token=REDACTED and b?token=REDACTED", + }, + { + name: "nothing to redact", + in: "dial tcp 127.0.0.1:59999: connect: connection refused", + want: "dial tcp 127.0.0.1:59999: connect: connection refused", + }, + { + name: "leaves lookalike parameters alone", + in: "wss://proxy/ws?refresh_token_hint=keep&sandbox_id=sb_1", + want: "wss://proxy/ws?refresh_token_hint=keep&sandbox_id=sb_1", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := redactToken(tc.in) + assert.Equal(t, tc.want, got) + assert.NotContains(t, got, fakeJWT) + }) + } +} + +func TestRedactErrScrubsMessageAndKeepsChain(t *testing.T) { + sentinel := errors.New("connection refused") + wrapped := fmt.Errorf(`Get "wss://proxy/ws?token=%s": %w`, fakeJWT, sentinel) + + got := redactErr(wrapped) + + require.NotNil(t, got) + assert.NotContains(t, got.Error(), fakeJWT) + assert.Contains(t, got.Error(), "token=REDACTED") + assert.ErrorIs(t, got, sentinel, "the original chain stays reachable") +} + +func TestRedactErrPassesThroughCleanErrors(t *testing.T) { + assert.Nil(t, redactErr(nil)) + + clean := errors.New("connection refused") + assert.Same(t, clean, redactErr(clean), "an error with no token is returned unchanged") +} diff --git a/internal/cmd/sandbox/sessionlane.go b/internal/cmd/sandbox/sessionlane.go index e8678ae..76c26e7 100644 --- a/internal/cmd/sandbox/sessionlane.go +++ b/internal/cmd/sandbox/sessionlane.go @@ -24,7 +24,9 @@ type wsDialer func(ctx context.Context, url string) (*websocket.Conn, error) func defaultWSDial(ctx context.Context, url string) (*websocket.Conn, error) { c, _, err := websocket.Dial(ctx, url, nil) - return c, err + // A dial error quotes the URL it tried, JWT and all. Scrub it here so no + // caller can leak the token into the log panel or retask.log. + return c, redactErr(err) } // SessionManager creates and tracks one agentfleet Runner per active sandbox session.