[Refactor/Chore] Constrain shellctl run cwd to the trusted sandbox workspace
- Dominant language
- TypeScript
- Stars
- 156k
- Forks
- 24.6k
- Avg merge
- 22h 9m
- Merged PRs (30d)
- 610
Description
**AI disclosure**: This issue was drafted and analyzed with Codex using GPT-5.4. I have reviewed the analysis, and I am responsible for the content.
## Description
The new agent shell feature is currently intended for trusted users. In the current Dify shell layer path, shell jobs are launched with a workspace cwd derived from the bound shell session. However, the lower-level shellctl API accepts `RunJobRequest.cwd` and resolves it directly as an existing directory.
CodeQL reports `py/path-injection` for `dify-agent/src/shellctl/server/service.py` around `_resolve_cwd()`. This is more meaningful than the job-id alerts because `cwd` is caller-provided input to `POST /v1/jobs/run`.
Current behavior:
- `RunJobRequest.cwd` is optional and can be supplied by the shellctl client.
- `_resolve_cwd()` expands and resolves the path, then only checks that it exists and is a directory.
- The Dify shell layer normally supplies a controlled workspace cwd.
- Direct shellctl API calls can bypass the Dify shell layer's workspace selection if shellctl is exposed to that caller.
This is acceptable only if shellctl is treated as a trusted control-plane API. For stronger sandbox semantics, shellctl should enforce an allowed cwd root itself.
## Motivation
The lower-level execution service should not silently expand the effective filesystem scope beyond the product-level shell workspace. Constraining cwd at the shellctl boundary would make the trusted-user assumption explicit and reduce the blast radius if shellctl is called directly from the same host or from future integrations.
Recommended scope:
- Add a configurable allowed cwd root or sandbox root to `ShellctlConfig`.
- Resolve both the requested cwd and allowed root with symlink-aware normalization.
- Reject requested cwd values outside the allowed root with a structured 400 error.
- Preserve the current Dify shell layer behavior by setting the allowed root to the shell home/workspace root used for new agent shell sessions.
- Add tests for absolute paths, `..`, symlink escape attempts, missing directories, and valid workspace subdirectories.
## Original CodeQL Annotation
- Alert 345: `dify-agent/src/shellctl/server/service.py:1047`
- Code: https://github.com/langgenius/dify/blob/7fabc39e97e4d54787384afdc35f7c021ff8e929/dify-agent/src/shellctl/server/service.py#L1047
- CodeQL alert: https://github.com/langgenius/dify/security/code-scanning/345
- Title: `Uncontrolled data used in path expression`
- Message: `This path depends on a user-provided value.`
## Additional Context
Relevant files in the current PR branch:
- `dify-agent/src/shellctl/shared/schemas.py`
- `dify-agent/src/shellctl/server/config.py`
- `dify-agent/src/shellctl/server/service.py`
- `dify-agent/src/dify_agent/layers/shell/layer.py`
- `dify-agent/tests/local/shellctl/test_shellctl_service.py`
This issue does not change the current positioning that new agent is for trusted users. It is a hardening item to keep shellctl's own execution boundary aligned with the Dify shell workspace boundary.
Contributor guide
Assessment
This issue has not been assessed yet.