Skip to content

fix(bay): support workspace path aliases and UTF-8 download filenames - #31

Open
lzyqwr wants to merge 1 commit into
AstrBotDevs:mainfrom
lzyqwr:agent/fix-workspace-path-downloads
Open

fix(bay): support workspace path aliases and UTF-8 download filenames#31
lzyqwr wants to merge 1 commit into
AstrBotDevs:mainfrom
lzyqwr:agent/fix-workspace-path-downloads

Conversation

@lzyqwr

@lzyqwr lzyqwr commented Jul 27, 2026

Copy link
Copy Markdown

Summary

Fix file download failures for sandbox clients that use /workspace-style paths or Unicode filenames.

Changes

  • Accept /workspace/... and workspace/... as aliases for the sandbox workspace root.
  • Preserve path traversal protection after alias normalization.
  • Encode Content-Disposition filenames using RFC 5987 UTF-8 format.
  • Support downloading files with Chinese and other non-ASCII filenames.
  • Add unit and integration tests for workspace aliases, traversal protection, Unicode filenames, and header escaping.

Motivation

Some clients expose sandbox files using paths such as /workspace/file.txt or workspace/file.txt, while the Bay API expects paths relative to the workspace root. This causes valid files to be reported as missing.

The previous Content-Disposition header also embedded filenames directly and could not reliably handle non-ASCII filenames.

Security

Workspace aliases are normalized before traversal validation. Paths such as /workspace/../etc/passwd and workspace/../../etc/passwd remain rejected.

Filename values are percent-encoded before being placed in the response header.

Validation

  • Added unit tests for path normalization and traversal protection.
  • Added unit tests for UTF-8 Content-Disposition headers.
  • Added integration coverage for uploading and downloading a file with a Chinese filename.

Closes #21

Summary by Sourcery

Support workspace path aliases and Unicode-safe filenames when downloading sandbox files.

New Features:

  • Allow filesystem operations to accept /workspace and workspace path aliases for the sandbox workspace root.
  • Generate RFC 5987 UTF-8 encoded Content-Disposition headers for file downloads to support non-ASCII filenames.

Bug Fixes:

  • Prevent valid sandbox files addressed via /workspace-style paths from being incorrectly rejected as missing.
  • Ensure path traversal remains blocked after normalizing workspace aliases, including crafted /workspace/../ sequences.

Enhancements:

  • Refine relative path validation to normalize workspace aliases before applying absolute path and traversal checks.

Tests:

  • Add unit tests for workspace alias normalization and traversal rejection cases in the path validator.
  • Add unit tests verifying UTF-8, ASCII-only Content-Disposition headers and escaping of header metacharacters for file downloads.
  • Add integration coverage for uploading and downloading files with Unicode filenames via workspace path aliases.

@lzyqwr
lzyqwr marked this pull request as ready for review July 29, 2026 20:25

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • The validate_relative_path docstring and rules mention only /workspace as an allowed alias, but the implementation also accepts bare workspace and workspace/...; consider updating the docstring/comments to clearly reflect all supported alias forms.
  • Normalizing a bare workspace path to . changes the meaning of a legitimate workspace file/directory name into the workspace root; if this is intentional, it may be worth explicitly guarding or documenting this ambiguity to avoid surprising behavior for paths actually named workspace.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `validate_relative_path` docstring and rules mention only `/workspace` as an allowed alias, but the implementation also accepts bare `workspace` and `workspace/...`; consider updating the docstring/comments to clearly reflect all supported alias forms.
- Normalizing a bare `workspace` path to `.` changes the meaning of a legitimate `workspace` file/directory name into the workspace root; if this is intentional, it may be worth explicitly guarding or documenting this ambiguity to avoid surprising behavior for paths actually named `workspace`.

## Individual Comments

### Comment 1
<location path="pkgs/bay/app/validators/path.py" line_range="65-66" />
<code_context>
+    # workspace/....  Normalize both forms before applying the traversal checks.
+    if path == "/workspace" or path == "workspace":
+        path = "."
+    elif path.startswith("/workspace/"):
+        path = path.removeprefix("/workspace/")
+    elif path.startswith("workspace/"):
+        path = path.removeprefix("workspace/")
</code_context>
<issue_to_address>
**issue:** Double slashes after `/workspace/` may cause paths to be treated as absolute and rejected

For an input like `/workspace//foo`, `removeprefix` produces `//foo`, and `PurePosixPath("//foo")` is typically treated as absolute, so it will fail the later absolute-path checks. If everything under `/workspace` should be workspace-relative, consider normalizing redundant slashes (e.g., collapsing `//` to `/`) around the prefix stripping step.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +65 to +66
elif path.startswith("/workspace/"):
path = path.removeprefix("/workspace/")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: Double slashes after /workspace/ may cause paths to be treated as absolute and rejected

For an input like /workspace//foo, removeprefix produces //foo, and PurePosixPath("//foo") is typically treated as absolute, so it will fail the later absolute-path checks. If everything under /workspace should be workspace-relative, consider normalizing redundant slashes (e.g., collapsing // to /) around the prefix stripping step.

@mcxianyujun

Copy link
Copy Markdown

I reproduced the same issue in a real AstrBot + Shipyard Neo deployment.

For the Unicode filename case, the underlying Ship container successfully handled the file download request:

GET /fs/download?file_path=... HTTP/1.1" 200 OK

but Bay then returned HTTP 500.

The Bay traceback points directly to the Content-Disposition header construction:

File "/app/app/api/v1/capabilities.py", line 829, in download_file
    headers={"Content-Disposition": f'attachment; filename="{filename}"'},
UnicodeEncodeError: 'latin-1' codec can't encode characters

I also reproduced the failure with also_send_to_user=false, so the error occurs during the sandbox file download stage, before the file is sent through the messaging platform.

Control test:

  • test_ascii.txt → success
  • 测试中文文件名.txt → HTTP 500
  • Both files had identical contents and size
  • Both used relative paths

I also encountered the /workspace/... path behavior described in this PR.

So this PR appears to address the same two issues I encountered in my deployment.

I would be happy to test this PR in my environment if that would be helpful.

@mcxianyujun

Copy link
Copy Markdown

I tested this PR on a real AstrBot + Shipyard Neo deployment, and the Unicode filename / workspace alias case passed end-to-end.

What I verified:

  • Built Bay from this PR successfully.
  • Confirmed _attachment_content_disposition() produces an ASCII-only filename*=UTF-8''... header for Chinese filenames.
  • Confirmed Starlette can encode and emit that header without the previous UnicodeEncodeError.
  • Ran the PR's E2E test:
    tests/integration/filesystem/test_transfer.py::test_upload_and_download_unicode_filename_with_workspace_alias
  • The test created a real Ship sandbox, uploaded 中文测试文件.txt, downloaded it using workspace/中文测试文件.txt, and returned HTTP 200.
  • File contents matched and the Content-Disposition header used the UTF-8 filename form.

Result:

1 passed in 6.27s

This matches the failure I originally reproduced, where Ship returned 200 but Bay failed while building the Content-Disposition header for a Chinese filename. On this PR, that path now succeeds in my environment.

Thanks for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ai如果使用绝对路径的话,会显示没文件。文件名有中文也不行喵

2 participants