What's Great
Reading through Backend/orchestrator.py around the _resolve_workspace_dir helper, I noticed the authors didn't take the lazy route. Instead of trusting user-supplied workspace names, the code uses a dedicated _path_is_relative_to check that compares the resolved absolute path against WORKSPACE_ROOT:
if not _path_is_relative_to(path, workspace_root):
raise ValueError("workspace 必须位于 backend/workspace 内")Code Reference
Backend/orchestrator.py:949
Why This Stands Out
Two things impressed me:
-
It resolves first, then validates. A lot of codebases do the check on the raw input, which gets bypassed by .. segments and symlinks. Resolving to an absolute path before checking is the right call.
-
The error message is intentional. It tells the user where their workspace should live without leaking the absolute filesystem path of the install. That's good security hygiene — most projects either leak the full path or give a useless "invalid input" error.
This is the kind of small, boring detail that prevents path-traversal CVEs. Easy to miss when reading the code casually, but it shows the authors thought about an adversarial caller. Nice work.
(One small follow-up I'd consider: a unit test asserting that ../etc/passwd, workspace/../../../etc, and a symlinked path all get rejected — would lock the behavior in against future refactors.)
What's Great
Reading through
Backend/orchestrator.pyaround the_resolve_workspace_dirhelper, I noticed the authors didn't take the lazy route. Instead of trusting user-supplied workspace names, the code uses a dedicated_path_is_relative_tocheck that compares the resolved absolute path againstWORKSPACE_ROOT:Code Reference
Backend/orchestrator.py:949Why This Stands Out
Two things impressed me:
It resolves first, then validates. A lot of codebases do the check on the raw input, which gets bypassed by
..segments and symlinks. Resolving to an absolute path before checking is the right call.The error message is intentional. It tells the user where their workspace should live without leaking the absolute filesystem path of the install. That's good security hygiene — most projects either leak the full path or give a useless "invalid input" error.
This is the kind of small, boring detail that prevents path-traversal CVEs. Easy to miss when reading the code casually, but it shows the authors thought about an adversarial caller. Nice work.
(One small follow-up I'd consider: a unit test asserting that
../etc/passwd,workspace/../../../etc, and a symlinked path all get rejected — would lock the behavior in against future refactors.)