Skip to content

fix(server): contain API session working directories - #3916

Open
Sayt-0 wants to merge 2 commits into
mainfrom
fix/codeql-57-session-workingdir-root
Open

fix(server): contain API session working directories#3916
Sayt-0 wants to merge 2 commits into
mainfrom
fix/codeql-57-session-workingdir-root

Conversation

@Sayt-0

@Sayt-0 Sayt-0 commented Aug 5, 2026

Copy link
Copy Markdown
Member

Summary

  • add a dedicated opt-in --session-workingdir-root boundary for session working directories supplied through the HTTP API
  • canonicalize configured roots and candidate directories before rejecting traversal, sibling-directory, and symlink escapes
  • preserve unrestricted local-daemon behavior when no root is configured, without treating --working-dir or the process cwd as an implicit security boundary
  • document the deployment model and add server and CLI regression coverage

Security alert: https://github.com/docker/docker-agent/security/code-scanning/57

Alert expectations

Expectation Implementation
Prevent untrusted API callers from selecting arbitrary host paths WithSessionWorkingDirRoot constrains POST /api/sessions working directories when configured
Reject traversal and symlink escapes Both root and candidate are resolved with filepath.EvalSymlinks; containment uses filepath.Rel and filepath.IsLocal
Avoid the behavior regression reverted by #3788 The default remains unrestricted and neither runConfig.WorkingDir nor the process cwd becomes a boundary
Support API and attached-run control planes serve api exposes the flag; run --listen accepts the same hidden advanced flag
Keep the intentional unrestricted flow explicit to CodeQL The remaining unrestricted sink has a rule-specific local suppression with the compatibility rationale

Validation

  • task build
  • task lint
  • env -u HTTP_PROXY -u HTTPS_PROXY -u http_proxy -u https_proxy -u ALL_PROXY -u all_proxy task test
  • npx --yes markdownlint-cli2@0.22.1 features/api-server/index.md features/cli/index.md
  • ./scripts/docs-check-canonical.sh
  • ./scripts/docs-check-llms-txt.sh
  • offline documentation link check with Lychee

The proxy variables are removed for the test command because the local development proxy intercepts private-address SSRF fixtures; the same failing packages pass without that interception.

Security and compatibility notes

  • Containment is opt-in so long-lived local daemons can continue opening arbitrary user-selected workspaces. Multi-user or network-exposed deployments should configure the new root.
  • The configured root should be controlled by the operator. Filesystem namespace changes between canonicalization and later tool use remain subject to normal TOCTOU constraints when untrusted principals can mutate the root.
  • Existing HTTP error classification is unchanged.
  • Alert closure depends on the CodeQL analysis for this branch.

Add a dedicated opt-in root for session working directories created through the HTTP control plane. Resolve symlinks and reject paths outside the configured root while preserving unrestricted local-daemon behavior by default.\n\nDocument the deployment boundary and cover traversal, sibling, symlink, and compatibility cases.
@Sayt-0
Sayt-0 requested a review from a team as a code owner August 5, 2026 15:17
Comment thread pkg/server/session_manager.go
@aheritier aheritier added area/api For features/issues/fixes related to the usage of the cagent API area/cli CLI commands, flags, output formatting area/docs Documentation changes kind/fix PR fixes a bug (maps to fix:). Use on PRs only. labels Aug 5, 2026
…ed in path expression'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@Sayt-0
Sayt-0 enabled auto-merge August 5, 2026 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/api For features/issues/fixes related to the usage of the cagent API area/cli CLI commands, flags, output formatting area/docs Documentation changes kind/fix PR fixes a bug (maps to fix:). Use on PRs only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants