Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions cmd/root/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,15 @@ import (
)

type apiFlags struct {
listenAddr string
sessionDB string
pullIntervalMins int
fakeResponses string
recordPath string
authToken string
pprofAddr string
runConfig config.RuntimeConfig
listenAddr string
sessionDB string
pullIntervalMins int
fakeResponses string
recordPath string
authToken string
pprofAddr string
sessionWorkingDirRoot string
runConfig config.RuntimeConfig
}

func newAPICmd() *cobra.Command {
Expand All @@ -47,6 +48,7 @@ func newAPICmd() *cobra.Command {
cmd.PersistentFlags().StringVar(&flags.fakeResponses, "fake", "", "Replay AI responses from cassette file (for testing)")
cmd.PersistentFlags().StringVar(&flags.recordPath, "record", "", "Record AI API interactions to cassette file")
cmd.PersistentFlags().StringVar(&flags.authToken, "auth-token", "", "Bearer token required for API requests (empty = no authentication)")
cmd.PersistentFlags().StringVar(&flags.sessionWorkingDirRoot, "session-workingdir-root", "", "Restrict the working_dir of sessions created via POST /api/sessions to this directory and its descendants (empty = no restriction; recommended for multi-user deployments)")
cmd.PersistentFlags().StringVar(&flags.pprofAddr, "pprof-addr", "", "TCP host:port to expose Go pprof endpoints at /debug/pprof/ (e.g. 127.0.0.1:6060); also set via CAGENT_PPROF_ADDR")
_ = cmd.PersistentFlags().MarkHidden("pprof-addr")
cmd.MarkFlagsMutuallyExclusive("fake", "record")
Expand All @@ -65,6 +67,10 @@ func (f *apiFlags) runAPICommand(cmd *cobra.Command, args []string) (commandErr
out := cli.NewPrinter(cmd.OutOrStdout())
agentsPath := args[0]

if err := validateSessionWorkingDirRoot(f.sessionWorkingDirRoot); err != nil {
return err
}

// Make sure no question is ever asked to the user in api mode.
os.Stdin = nil

Expand Down Expand Up @@ -132,7 +138,8 @@ func (f *apiFlags) runAPICommand(cmd *cobra.Command, args []string) (commandErr
return fmt.Errorf("resolving agent sources: %w", err)
}

s, err := server.New(ctx, sessionStore, &f.runConfig, time.Duration(f.pullIntervalMins)*time.Minute, sources, f.authToken)
s, err := server.New(ctx, sessionStore, &f.runConfig, time.Duration(f.pullIntervalMins)*time.Minute, sources, f.authToken,
server.WithSessionWorkingDirRoot(f.sessionWorkingDirRoot))
if err != nil {
return fmt.Errorf("creating server: %w", err)
}
Expand Down
12 changes: 12 additions & 0 deletions cmd/root/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package root

import (
"context"
"errors"
"fmt"
"log/slog"
"net"
Expand Down Expand Up @@ -83,6 +84,17 @@ func setupWorkingDirectory(workingDir string) error {
return nil
}

// validateSessionWorkingDirRoot rejects a --session-workingdir-root value
// that trims to empty (e.g. an unresolved shell variable): the operator
// asked for containment, so silently disabling it would be invisible and
// dangerous. An empty value (flag unset) means unrestricted and is valid.
func validateSessionWorkingDirRoot(root string) error {
if root != "" && strings.TrimSpace(root) == "" {
return errors.New("--session-workingdir-root: value is empty after trimming whitespace")
}
return nil
}

func canonize(endpoint string) string {
return strings.TrimSuffix(strings.TrimSpace(endpoint), "/")
}
Expand Down
31 changes: 22 additions & 9 deletions cmd/root/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,19 @@ type runExecFlags struct {
outputJSON bool

// Run only
hideToolResults bool
lean bool
leanChanged bool
appName string
sidebar bool
listenAddr string
onEventSpecs []string
disabledCommands []string
theme string
hideToolResults bool
lean bool
leanChanged bool
appName string
sidebar bool
listenAddr string
// sessionWorkingDirRoot confines the working_dir of sessions created
// through the --listen control plane; empty means unrestricted (see
// server.WithSessionWorkingDirRoot).
sessionWorkingDirRoot string
onEventSpecs []string
disabledCommands []string
theme string

// globalPermissions holds the user-level global permission checker built
// from user config settings. Nil when no global permissions are configured.
Expand Down Expand Up @@ -182,6 +186,8 @@ func addRunOrExecFlags(cmd *cobra.Command, flags *runExecFlags) {
_ = cmd.PersistentFlags().MarkHidden("exit-after-response")
cmd.PersistentFlags().StringVar(&flags.listenAddr, "listen", "", "Expose this run's control plane on the given address (e.g. 127.0.0.1:0)")
_ = cmd.PersistentFlags().MarkHidden("listen")
cmd.PersistentFlags().StringVar(&flags.sessionWorkingDirRoot, "session-workingdir-root", "", "Restrict the working_dir of sessions created via the --listen control plane to this directory and its descendants (empty = no restriction)")
_ = cmd.PersistentFlags().MarkHidden("session-workingdir-root")
cmd.PersistentFlags().StringArrayVar(&flags.onEventSpecs, "on-event", nil, "Run shell command on event: --on-event <type>=<cmd> (or *=<cmd> for any). Repeatable.")
cmd.PersistentFlags().StringVar(&flags.cpuProfile, "cpuprofile", "", "Write CPU profile to file")
_ = cmd.PersistentFlags().MarkHidden("cpuprofile")
Expand Down Expand Up @@ -269,6 +275,13 @@ func (f *runExecFlags) runRunCommand(cmd *cobra.Command, args []string) (command
f.safetyChanged = cmd.Flags().Changed("safety")
f.yoloChanged = cmd.Flags().Changed("yolo")

// A --session-workingdir-root that trims to empty (e.g. an unresolved
// shell variable) must fail loudly instead of silently disabling the
// containment the operator asked for.
if err := validateSessionWorkingDirRoot(f.sessionWorkingDirRoot); err != nil {
return err
}

useTUI := !f.exec && (f.forceTUI || isatty.IsTerminal(os.Stdout.Fd()))
f.leanChanged = cmd.Flags().Changed("lean")

Expand Down
4 changes: 2 additions & 2 deletions cmd/root/run_listen.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (f *runExecFlags) recallCoordinatorOpt(ctx context.Context, rt runtime.Runt
sm := f.listenSM
exposed := sm != nil
if !exposed {
sm = server.NewSessionManager(ctx, nil, rt.SessionStore(), 0, &f.runConfig)
sm = server.NewSessionManager(ctx, nil, rt.SessionStore(), 0, &f.runConfig, server.WithSessionWorkingDirRoot(f.sessionWorkingDirRoot))
}
guard := sm.AttachRuntime(ctx, sess.ID, rt, sess)
return func(a *app.App) {
Expand Down Expand Up @@ -71,7 +71,7 @@ func (f *runExecFlags) startSessionCoordinator(ctx context.Context, out *cli.Pri
return f.recallCoordinatorOpt(ctx, rt, sess), nil
}

sm := server.NewSessionManager(ctx, nil, rt.SessionStore(), 0, &f.runConfig)
sm := server.NewSessionManager(ctx, nil, rt.SessionStore(), 0, &f.runConfig, server.WithSessionWorkingDirRoot(f.sessionWorkingDirRoot))
guard := sm.AttachRuntime(ctx, sess.ID, rt, sess)
// Publish the serving manager so sessions spawned later (TUI tabs) attach
// to it too (see recallCoordinatorOpt). Set before the TUI starts, so no
Expand Down
54 changes: 54 additions & 0 deletions cmd/root/session_workingdir_root_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package root

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// TestAPISessionWorkingDirRootFlag: serve api exposes the opt-in containment
// root for session working directories, unrestricted by default.
func TestAPISessionWorkingDirRootFlag(t *testing.T) {
t.Parallel()

cmd := newAPICmd()

flag := cmd.PersistentFlags().Lookup("session-workingdir-root")
require.NotNil(t, flag, "serve api must expose --session-workingdir-root")
assert.Empty(t, flag.DefValue, "default must be unrestricted")
assert.False(t, flag.Hidden)

require.NoError(t, cmd.PersistentFlags().Parse([]string{"--session-workingdir-root", "/srv/workspaces"}))
value, err := cmd.PersistentFlags().GetString("session-workingdir-root")
require.NoError(t, err)
assert.Equal(t, "/srv/workspaces", value)
}

// TestRunSessionWorkingDirRootFlagIsHidden: run exposes the same containment
// root for its --listen control plane, hidden like --listen itself.
func TestRunSessionWorkingDirRootFlagIsHidden(t *testing.T) {
t.Parallel()

cmd := newRunCmd()

flag := cmd.PersistentFlags().Lookup("session-workingdir-root")
require.NotNil(t, flag, "run must expose --session-workingdir-root for --listen")
assert.Empty(t, flag.DefValue, "default must be unrestricted")
assert.True(t, flag.Hidden, "advanced/automation flag stays hidden like --listen")
}

// TestValidateSessionWorkingDirRoot: a value that trims to empty must fail
// loudly instead of silently disabling containment.
func TestValidateSessionWorkingDirRoot(t *testing.T) {
t.Parallel()

require.NoError(t, validateSessionWorkingDirRoot(""))
require.NoError(t, validateSessionWorkingDirRoot("/srv/workspaces"))

err := validateSessionWorkingDirRoot(" ")
require.Error(t, err)
assert.Contains(t, err.Error(), "--session-workingdir-root")

require.Error(t, validateSessionWorkingDirRoot("\t\n"))
}
1 change: 1 addition & 0 deletions docs/features/api-server/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ docker agent serve api <agent-file>|<agents-dir> [flags]
| ------------------ | ---------------- | ------------------------------------------------ |
| `-l, --listen` | `127.0.0.1:8080` | Address to listen on |
| `--auth-token` | (none) | Bearer token required for all API requests. Leave empty to disable authentication (safe when listening on loopback interfaces only). Recommended when `--listen` binds to a network-reachable interface. |
| `--session-workingdir-root` | (none — unrestricted) | Confine the `working_dir` accepted by `POST /api/sessions` to this directory: after resolving symlinks, the requested directory must be the root or one of its descendants. By default any clean host directory is accepted — the intended behaviour for local single-user daemons that open arbitrary workspaces — but raw values containing `..` are always rejected. Set a root whenever the API serves callers that must not reach arbitrary host paths (multi-user or network-exposed deployments). |
| `-s, --session-db` | `session.db` | Path to the SQLite session database |
| `--pull-interval` | `0` (disabled) | Auto-pull OCI reference every N minutes |
| `--fake` | (none) | Replay AI responses from cassette file (testing) |
Expand Down
2 changes: 2 additions & 0 deletions docs/features/cli/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ $ docker agent run [config] [message...] [flags]
| `--dry-run` | Initialize the agent without executing anything (useful for validating a config) |
| `--remote <addr>` | Use a remote runtime at the given address instead of running the agent locally. Mutually exclusive with `--sandbox`, `--worktree`, `--worktree-pr`, `--worktree-base`, `--session`, `--session-db`, `--record`, and `--fake` — a remote runtime owns its own session storage and execution environment, so these local-only concerns don't apply. |
| `--listen <addr>` | Expose this run's control plane over HTTP so an external process can drive the running TUI (send follow-ups, stream events, read the title). Accepts `host:port` or `unix://`, `npipe://`, `fd://`. Hidden from `docker agent run --help` — like `debug`, it's a stable but advanced/automation-oriented flag rather than a day-to-day one. See the [API Server](../api-server/index.md#listen) guide for the full walkthrough. |
| `--session-workingdir-root <path>` | Confine the `working_dir` of sessions created through the `--listen` control plane to this directory and its descendants (default: no restriction — any clean host directory is accepted, though raw values containing `..` are rejected). Recommended when the control plane is reachable by other users. Hidden from `--help`, like `--listen`. |
| `--lean` | Use a simplified, non-alternate-screen TUI. Unlike the default full-screen TUI, this renders inline in the normal terminal buffer — useful in environments where an alternate screen is unwanted (e.g. inside tmux panes, CI with a tty, or log-friendly pipelines). Displays an ASCII art banner on startup. |
| `--app-name <name>` | Override the application name label shown in the TUI (status bar, window title, "/exit" notifications). |
| `--sidebar` | Control sidebar visibility. Set to `--sidebar=false` to hide the sidebar and disable the Ctrl+B toggle (default: `true`). |
Expand Down Expand Up @@ -283,6 +284,7 @@ $ docker agent serve api <agent-file>|<agents-dir>|<registry-ref> [flags]
| -------------------------- | ------------------ | ---------------------------------------------------------------------------------------------------------- |
| `-l, --listen <addr>` | `127.0.0.1:8080` | Address to listen on. |
| `--auth-token <token>` | (none) | Bearer token required for all API requests. When set, every request must include `Authorization: Bearer <token>`. Leave empty to disable authentication (safe when listening on loopback interfaces only). |
| `--session-workingdir-root <path>` | (none) | Confine the `working_dir` of sessions created via `POST /api/sessions` to this directory and its descendants (symlinks are resolved before the check). Unrestricted by default — any clean host directory is accepted (raw values containing `..` are rejected), as local single-user daemons rely on. Recommended for multi-user or network-exposed deployments. |
| `-s, --session-db <path>` | `session.db` | Path to the SQLite session database (relative paths resolve against the working directory). |
| `--pull-interval <minutes>`| `0` | Periodically re-pull OCI/URL references and refresh the agent definition. `0` disables auto-pull. |
| `--fake <path>` | (none) | Replay AI responses from a cassette file (for testing). Mutually exclusive with `--record`. |
Expand Down
7 changes: 5 additions & 2 deletions pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ type Server struct {
heartbeatInterval time.Duration
}

func New(ctx context.Context, sessionStore session.Store, runConfig *config.RuntimeConfig, refreshInterval time.Duration, agentSources config.Sources, authToken string) (*Server, error) {
return NewWithManager(NewSessionManager(ctx, agentSources, sessionStore, refreshInterval, runConfig), authToken), nil
func New(ctx context.Context, sessionStore session.Store, runConfig *config.RuntimeConfig, refreshInterval time.Duration, agentSources config.Sources, authToken string, opts ...SessionManagerOpt) (*Server, error) {
return NewWithManager(NewSessionManager(ctx, agentSources, sessionStore, refreshInterval, runConfig, opts...), authToken), nil
}

const defaultMaxRequestBytes int64 = 1 << 20 // 1 MiB
Expand Down Expand Up @@ -267,6 +267,9 @@ func (s *Server) createSession(c echo.Context) error {

sess, err := s.sm.CreateSession(c.Request().Context(), &sessionTemplate)
if err != nil {
if errors.Is(err, ErrInvalidWorkingDir) {
return echo.NewHTTPError(http.StatusBadRequest, err.Error())
}
return echo.NewHTTPError(http.StatusInternalServerError, fmt.Sprintf("failed to create session: %v", err))
}

Expand Down
68 changes: 68 additions & 0 deletions pkg/server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,74 @@ func TestServer_UpdateSessionTitle(t *testing.T) {
assert.Equal(t, newTitle, sessionResp.Title)
}

// TestServer_CreateSessionWorkingDirWithSeparators pins the removal of a
// Copilot-autofix validation that rejected any working_dir containing path
// separators or an absolute path: POST /api/sessions must accept a real
// host directory (which always contains separators) and store it on the
// session. Only a raw working_dir containing ".." is rejected up front.
// Containment stays opt-in via WithSessionWorkingDirRoot (CodeQL alert
// #57); the unrestricted default is intentional (#3788).
func TestServer_CreateSessionWorkingDirWithSeparators(t *testing.T) {
t.Parallel()

ctx := t.Context()
sm := NewSessionManager(ctx, config.Sources{}, session.NewInMemorySessionStore(), 0, &config.RuntimeConfig{})
srv := NewWithManager(sm, "")

wd := t.TempDir()
require.True(t, strings.ContainsAny(wd, `/\`))

body, err := json.Marshal(map[string]any{"working_dir": wd})
require.NoError(t, err)
req := httptest.NewRequestWithContext(ctx, http.MethodPost, "/api/sessions", bytes.NewReader(body))
req.Header.Set("Content-Type", "application/json")
rec := httptest.NewRecorder()

srv.e.ServeHTTP(rec, req)

require.Equal(t, http.StatusOK, rec.Code, rec.Body.String())
var created session.Session
unmarshal(t, rec.Body.Bytes(), &created)
require.NotEmpty(t, created.ID)
assert.Equal(t, wd, created.WorkingDir)

stored, err := sm.GetSession(ctx, created.ID)
require.NoError(t, err)
assert.Equal(t, wd, stored.WorkingDir)
}

// TestServer_CreateSessionWorkingDirDotDotRejected pins the HTTP mapping of
// the raw ".." rejection: POST /api/sessions with a traversal-carrying
// working_dir must answer 400 (ErrInvalidWorkingDir), not 500, and create
// no session.
func TestServer_CreateSessionWorkingDirDotDotRejected(t *testing.T) {
t.Parallel()

ctx := t.Context()
sm := NewSessionManager(ctx, config.Sources{}, session.NewInMemorySessionStore(), 0, &config.RuntimeConfig{})
srv := NewWithManager(sm, "")

wd := t.TempDir() + string(filepath.Separator) + ".."
body, err := json.Marshal(map[string]any{"working_dir": wd})
require.NoError(t, err)
req := httptest.NewRequestWithContext(ctx, http.MethodPost, "/api/sessions", bytes.NewReader(body))
req.Header.Set("Content-Type", "application/json")
rec := httptest.NewRecorder()

srv.e.ServeHTTP(rec, req)

require.Equal(t, http.StatusBadRequest, rec.Code, rec.Body.String())
var errResp struct {
Message string `json:"message"`
}
unmarshal(t, rec.Body.Bytes(), &errResp)
assert.Contains(t, errResp.Message, `must not contain ".."`)

sessions, err := sm.GetSessions(ctx)
require.NoError(t, err)
assert.Empty(t, sessions)
}

// TestServer_GetSessionsRace pins the data-race fix for the GET
// /api/sessions and GET /api/sessions/:id handlers (#3591): the in-memory
// store hands them live *session.Session pointers, so reading
Expand Down
Loading
Loading