fix(dashboard): apply allowed_roots check to file:// URLs in add_remote_skill (CWE-22)

[sebastiondev]

Summary

add_remote_skill() in dashboard/server.py accepts skill source URLs in several forms (http(s)://, file://, absolute/relative path). The bare-path branch correctly checks the resolved path against allowed_roots (OCLAW_HOME and the project root), but the file:// branch had no such check. As a result, a file:///etc/passwd-style URL was read directly and the contents were imported as a "skill" into the workspace.

• CWE: CWE-22 (Improper Limitation of a Pathname to a Restricted Directory)
• File / function: dashboard/server.py → add_remote_skill() (the elif source_url.startswith('file://') branch, around line 350)
• Severity: High — arbitrary local file read for any file matching the SKILL.md frontmatter shape; result is persisted into the workspace where the attacker can retrieve it via the existing skill-listing/read APIs.
• Data flow: HTTP request body → source_url → pathlib.Path(source_url[7:]) → .read_text() with no allow-list check.

Fix

Mirror the guard already used by the sibling bare-path branch onto the file:// branch:

local_path = pathlib.Path(source_url[7:]).resolve()
if not local_path.exists():
    return {'ok': False, 'error': f'本地文件不存在: {local_path}'}
allowed_roots = (OCLAW_HOME.resolve(), BASE.parent.resolve())
if not any(str(local_path).startswith(str(root)) for root in allowed_roots):
    return {'ok': False, 'error': '路径不在允许的目录范围内'}
content = local_path.read_text()

Rationale: keeps both branches consistent, uses the same allowed_roots already trusted elsewhere in the file, and resolve() collapses .. segments before the prefix check so traversal payloads like file:///tmp/../etc/passwd are normalized first.

The diff is minimal (4 added lines plus the .resolve() call), no behavior change for legitimate file:// URLs that point inside OCLAW_HOME or the project tree.

Tests

Added tests/test_cwe22_file_url.py with three regression tests:

1. test_file_url_path_traversal_blocked — file:// URL pointing outside allowed roots is rejected.
2. test_file_url_within_allowed_roots_works — legitimate file:// URL inside OCLAW_HOME still works.
3. test_file_url_etc_passwd_blocked — file:///etc/passwd is rejected.

Result locally:

tests/test_cwe22_file_url.py::test_file_url_path_traversal_blocked PASSED
tests/test_cwe22_file_url.py::test_file_url_within_allowed_roots_works PASSED
tests/test_cwe22_file_url.py::test_file_url_etc_passwd_blocked PASSED
3 passed in 0.11s

Exploitability

The dashboard server currently prints 认证未配置，所有 API 公开访问 when no auth is configured, which is the default. In that mode, anyone who can reach the dashboard port (default 127.0.0.1, but operators commonly bind to 0.0.0.0 or expose it via tunnels) can call add_remote_skill with a file:// URL and read any local file whose contents parse as SKILL.md frontmatter — /etc/passwd, SSH keys, env files, application configs, etc. Even with auth enabled, the same call can be reached via CSRF against an authenticated user if CORS/SameSite settings are permissive, or by any local process running as a different user able to talk to the loopback port.

Adversarial review

Before submitting, we tried to disprove this. We checked whether the bare-path branch's existing allowed_roots check would catch file:// inputs first — it doesn't, dispatch is by URL scheme so file:// skips it entirely. We checked whether auth is required by default — it isn't; the server explicitly logs that all APIs are public when no auth is configured. We checked whether the read is sandboxed at the OS level — there's no such sandbox; pathlib.Path.read_text() runs with full server privileges. The fix is the smallest change that closes the gap and matches the pattern already used in the same function.

cc @lewiswigmore

[lewiswigmore]

Closing this to reduce the open-PR pile-up — we have multiple outstanding security contributions to this repo and that volume is not fair on your review queue. Keeping #318 (CWE-918: block SSRF via generic webhook URL (CWE-918)) as the primary one to focus attention on.

Happy to revisit this finding separately later if it is still relevant. Apologies for the noise.