langgenius / langgenius/dify

[Refactor/Chore] Constrain shellctl run cwd to the trusted sandbox workspace

Open
#38,611 1 comment 1 reaction 1 assignee Claimed by @BeautyyuYanli View on GitHub
🤖 feat:agent project#dify python refactor
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.