feat: add zstd directory transfer endpoints and optimize chromium restart #122
Conversation
…tart This PR adds two major improvements: 1. New zstd directory transfer endpoints (2.45x faster than zip): - GET /fs/download_dir_zstd - Download directory as tar.zst archive - POST /fs/upload_zstd - Upload and extract tar.zst archive - Supports compression levels: fastest, default, better, best - Uses klauspost/compress library for high-performance compression 2. Chromium restart optimization (19x faster): - Changed startsecs=5 to startsecs=0 (API server already detects readiness) - Added stopsignal=KILL for immediate termination - Added stopwaitsecs=0 to eliminate shutdown grace period - Reduces restart time from ~8 seconds to ~0.4 seconds Includes benchmark tests for both improvements with documented results. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
rgarcia
force-pushed
the
feat/zstd-endpoints-and-chromium-restart-optimization
branch
from
eabaf62 to
a3b60f4
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Addresses security issue found by Cursor Bugbot: symlink targets were not validated, allowing malicious archives to write files outside the destination directory via symlink path traversal. Changes: - Reject absolute symlink targets - Validate that symlink targets resolve within destDir - Validate that hardlink targets are within destDir Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
When stripComponents > 0, hard link entries whose target paths have too few components should be skipped (like regular files), not processed with unchanged paths that would cause extraction failures. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| return oapi.UploadZstd400JSONResponse{BadRequestErrorJSONResponse: oapi.BadRequestErrorJSONResponse{Message: "invalid archive: path traversal detected"}}, nil | ||
| } | ||
| log.Error("failed to extract tar.zst archive", "err", err) | ||
| return oapi.UploadZstd500JSONResponse{InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{Message: "failed to extract archive"}}, nil |
There was a problem hiding this comment.
Symlink/hardlink path traversal errors return 500 instead of 400
Low Severity
The UploadZstd error handling only checks for "illegal file path" to return a 400 response, but UntarZstd can also return security errors with different text: "illegal symlink target (absolute path)", "illegal symlink target (escapes destination)", and "illegal hard link target". These path traversal attempts are correctly blocked, but incorrectly return HTTP 500 with message "failed to extract archive" instead of HTTP 400 with "invalid archive: path traversal detected". This miscategorizes client errors as server errors.
- Add lock file cleanup (SingletonLock, SingletonSocket, SingletonCookie) to handle stale files from SIGKILL termination - Add waitForPort function that checks port availability before starting chromium, with SO_REUSEADDR disabled to accurately match chromium's bind behavior - This allows keeping stopwaitsecs=0 for fast restarts while ensuring the port is actually free before chromium tries to bind Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| // Security check: prevent path traversal | ||
| if !strings.HasPrefix(filepath.Clean(destPath), filepath.Clean(destDir)+string(os.PathSeparator)) { | ||
| return fmt.Errorf("illegal file path: %s", header.Name) | ||
| } |
There was a problem hiding this comment.
Path traversal check fails when extracting to root directory
Low Severity
The path traversal security check incorrectly rejects all valid files when destDir is "/". The check uses strings.HasPrefix(filepath.Clean(destPath), filepath.Clean(destDir)+string(os.PathSeparator)), but when destDir is /, this becomes strings.HasPrefix(path, "//") since filepath.Clean("/") + "/" equals "//". Any valid path like /etc/passwd doesn't start with //, causing all extractions to the root directory to fail. The symlink check at lines 215-216 has different logic that handles the equality case, but regular files and hard links don't have this handling.
Additional Locations (1)
Chromium creates symlinks to /tmp (e.g., SingletonSocket -> /tmp/org.chromium.Chromium.xxx/SingletonSocket). The previous security check rejected all absolute symlinks, but absolute paths aren't a path traversal risk - only relative symlinks using ../ to escape the destination are. Now we only check relative symlinks for path traversal while allowing absolute symlinks. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
rgarcia
deleted the
feat/zstd-endpoints-and-chromium-restart-optimization
branch
Summary
Zstd Endpoints
Added new
DownloadDirZstdandUploadZstdendpoints using tar+zstd compression:Uses klauspost/compress library. Supports compression levels:
fastest,default,better,best.Chromium Restart Optimization
Updated supervisor config for chromium:
The
startsecs=5delay was unnecessary because the API server's UpstreamManager already detects readiness by watching for "DevTools listening on..." in the log output.Test plan
TestZipVsZstdComparison- verified zstd is 2.45x fasterTestChromiumRestartTiming- verified 19x speedup🤖 Generated with Claude Code
Note
Introduces fast tar.zst-based directory transfer and significantly speeds up Chromium restarts.
DownloadDirZstdandUploadZstdendpoints with selectable compression levels (fastest|default|better|best); updatesopenapi.yamland regeneratesoapiclient/server; newzstdutil(tar+zstd stream/untar viaklauspost/compress).startsecs=0,stopsignal=KILL,stopwaitsecs=0) and launcher improvements (cleanupSingleton*, wait-for-port without SO_REUSEADDR) to minimize stop/start latency.go.modaddsklauspost/compress.Written by Cursor Bugbot for commit 2316b9e. This will update automatically on new commits. Configure here.