Skip to content

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

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

fix(server): contain API session working directories#3916
Sayt-0 wants to merge 4 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 Fixed
@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
Comment thread pkg/server/session_manager.go Fixed
@aheritier aheritier added the status/needs-signed-commits Some commits in the PR are signed with a valid SSH/GPG key label Aug 6, 2026
@aheritier

Copy link
Copy Markdown
Collaborator

👋 Some commits in this PR are not signed and verified by GitHub. Please sign your commits with a GPG or SSH key registered in your GitHub account, then force-push.

Commits that are not verified: a948b84

See GitHub's guide on signing commits for setup instructions. I've added status/needs-signed-commits; it will be removed automatically once every commit in this PR carries a valid GitHub-verified signature.

Sayt-0 added 2 commits August 6, 2026 13:55
Place the intentional unrestricted-path CodeQL suppression where the Go analyzer recognizes it, remove the over-restrictive autofix validation, and cover path-separated working directories through the HTTP API.
Validate raw working directory values before path normalization, classify client path failures as bad requests, and document the clean-path requirement.
@Sayt-0
Sayt-0 force-pushed the fix/codeql-57-session-workingdir-root branch from de71b00 to fb16299 Compare August 6, 2026 11:55
@aheritier aheritier removed the status/needs-signed-commits Some commits in the PR are signed with a valid SSH/GPG key label Aug 6, 2026
@aheritier
aheritier requested a review from docker-agent August 7, 2026 06:06
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