From 7e89b43ef8c8b4d8f226a2615be43aea1bd1e008 Mon Sep 17 00:00:00 2001 From: Frank Song Date: Tue, 10 Nov 2026 10:30:26 +0800 Subject: [PATCH] fix(admin): backport native Windows project path validation Backport upstream #1413 with unchanged five-file patch and fork validation report. --- docs/RUNNER.md | 12 +++ docs/RUNNER.zh-CN.md | 9 +++ .../2026-11-10-windows-project-paths.md | 63 +++++++++++++++ src/admin_http.rs | 5 +- src/admin_http/tests/project_paths.rs | 81 +++++++++++++++++++ src/admin_project_lifecycle.rs | 4 +- 6 files changed, 171 insertions(+), 3 deletions(-) create mode 100644 docs/reviews/2026-11-10-windows-project-paths.md create mode 100644 src/admin_http/tests/project_paths.rs diff --git a/docs/RUNNER.md b/docs/RUNNER.md index 06fbc737a..b666cf000 100644 --- a/docs/RUNNER.md +++ b/docs/RUNNER.md @@ -132,6 +132,18 @@ The runtime tools `register_project` and `create_project` let a client register an existing directory or create a new one on an online Runner, subject to the Runner's `allowed_roots` policy. +The admin `POST /api/admin/projects/register` and +`POST /api/admin/projects/create` routes use the same cross-platform path +validation as these tools. Supply an absolute path on the selected Runner, +regardless of the Server's OS: for example `/srv/projects/demo`, +`D:\projects\demo` or `D:/projects/demo`. In JSON, escape backslashes: +`"path": "D:\\projects\\demo"`. Windows verbatim paths are not required. +Empty, relative, drive-relative (`D:demo`), NUL-containing and over-4096-byte +paths are rejected by the admin API. The Runner still owns native path +validation, canonicalization and `allowed_roots` enforcement. On Windows, +existing UNC/verbatim-UNC directories can be registered only under already +authorized roots; creating Projects on network shares remains unsupported. + ## Skill sources `list_skills` presents one catalog while preserving three distinct ownership and diff --git a/docs/RUNNER.zh-CN.md b/docs/RUNNER.zh-CN.md index 1e1b5c59a..a7b23d9ae 100644 --- a/docs/RUNNER.zh-CN.md +++ b/docs/RUNNER.zh-CN.md @@ -106,6 +106,15 @@ allowed_roots = ["/root/git"] runtime 工具 `register_project` 与 `create_project` 让客户端在在线 Runner 上 注册已有目录或创建新目录，受 Runner 的 `allowed_roots` policy 约束。 +管理员接口 `POST /api/admin/projects/register` 与 +`POST /api/admin/projects/create` 使用与这些工具相同的跨平台路径校验。 +无论 Server 运行在哪个操作系统，都应传入所选 Runner 上的绝对路径，例如 +`/srv/projects/demo`、`D:\projects\demo` 或 `D:/projects/demo`。 +JSON 中的反斜杠需要转义：`"path": "D:\\projects\\demo"`；不必改用 Windows verbatim 路径。 +管理员 API 会拒绝空路径、相对路径、盘符相对路径（`D:demo`）、含 NUL 的路径和 +超过 4096 字节的路径。原生路径校验、规范化及 `allowed_roots` 检查仍由 Runner 执行。 +Windows 上的已有 UNC/verbatim-UNC 目录只能在已授权根目录内注册；仍不支持在网络共享上创建 Project。 + ## Skill 来源 `list_skills` 继续只暴露一个 catalog，但其中保留三种彼此独立的 ownership / lifecycle： diff --git a/docs/reviews/2026-11-10-windows-project-paths.md b/docs/reviews/2026-11-10-windows-project-paths.md new file mode 100644 index 000000000..56cc6a01c --- /dev/null +++ b/docs/reviews/2026-11-10-windows-project-paths.md @@ -0,0 +1,63 @@ +# Windows admin Project path backport validation + +## Scope and immutable sources + +This candidate backports [upstream PR #1413](https://github.com/yyjeqhc/webcodex/pull/1413), +merge commit `991a7c3e0943131bac993c290de5d136205bb6d4`, onto the maintained fork +branch `integrate/reviewed-upstream-20261020` at +`3ca34f673f6e169c376c7e279eabd62a597ba666`. +The five upstream files applied without source adaptations: the admin adapter +uses the existing cross-platform path validator, retains the 4096-byte limit, +and adds HTTP regressions and matching English/Chinese Runner documentation. +The test fixture retains its temporary database directory for its service lifetime. + +The Runner still owns native path interpretation, canonicalization and allowed +roots. Authentication, same-origin checks, strict JSON admission and downstream +authorization are unchanged. This is not a native filesystem permission change. +Existing fork diagnostics, process fixtures and Session recovery patches remain +in the base. Fork `main` and any installed deployment are separate states. + +The five-file patch generated with `git diff --cached --binary` before this +report has SHA-256 +`574dd4d6d14bbf2ee132eb03e851a9ff72953e6fe2b114229e6cec7c35370143`. + +## Local validation + +Validation used an isolated Linux cloud checkout and existing isolated official +Rust/Cargo 1.99.1, locked dependencies and a new dedicated build directory. + +- Fail-first control: unchanged production base plus upstream test/fixture changes, + `cargo test --locked -p webcodex --lib admin_project_paths -- --nocapture`: + **0 passed, 2 failed**, exit 101. `D:\projects\demo` returned 400 instead of + reaching empty Runner lookup (503); `/srv/projects/\0demo` reached Runner lookup + instead of failing early with 400. These were executed assertions, not build failures. +- Final `cargo test --locked --offline -p webcodex --lib admin_`: + **41 passed, 0 failed**, including both new path tests and admin authorization, + same-origin and strict JSON coverage. +- Final `cargo test --locked --offline -p webcodex-core project_op_path`: + **1 passed, 0 failed**; the unrelated integration benchmark had zero matching tests. +- `cargo fmt --all -- --check`: passed. +- `bash scripts/workspace_boundary_check.sh`: passed for all 22 workspace packages. +- `python3 -m unittest scripts.tests.test_workspace_boundary_check`: 20 passed. +- `python3 scripts/check_markdown_links.py` and `git diff --check`: passed. + +The first offline build attempt could not download uncached `adler2 v2.0.1`. +Normal locked dependency retrieval succeeded; this initial environment failure +is not the fail-first evidence above. An initial Git fetch failed on unresolved +partial-clone deltas; ordinary refetch without common-commit negotiation succeeded. +Neither failure was an access denial. A direct invocation of the boundary Python +checker lacked its required arguments; its documented shell wrapper subsequently +passed. No test, deadline, permission or sandbox constraint was weakened. + +## CI, review and acceptance limits + +Upstream head `067011a186754feaad344abdc77f45591fbd1049` has 18 successful checks in +[upstream run 42643508654](https://github.com/yyjeqhc/webcodex/actions/runs/42643508654). +That is upstream evidence, not fork validation. Exact-head fork CI and final +independent review must be evaluated separately before merging this candidate. + +The local tests are Linux in-process HTTP/validator tests. Valid path fixtures +intentionally stop at empty Runner lookup; they do not establish native Windows +filesystem acceptance. Full workspace/Server tests, native Windows/macOS/ARM64, +real Host/MCP, installed Server/Runner compatibility and Windows E2E were not run. +No release, deployment, service restart, upstream PR or force-push is included. diff --git a/src/admin_http.rs b/src/admin_http.rs index 105f76a29..348f23f54 100644 --- a/src/admin_http.rs +++ b/src/admin_http.rs @@ -387,18 +387,21 @@ async fn dashboard(req: &mut Request, depot: &mut Depot, res: &mut Response) { #[cfg(test)] mod tests { + mod project_paths; + use super::*; use crate::auth::scopes::SCOPE_ADMIN; use salvo::test::{ResponseExt, TestClient}; use salvo::Service; fn service(auth: Option) -> Service { - let (_tmp, db) = crate::test_support::test_db(); + let (tmp, db) = crate::test_support::test_db(); let runtime = Arc::new(ToolRuntime::new( Arc::new(crate::RunnerRegistry::default()), Arc::new(crate::tool_runtime::RuntimeInfo::default()), )); let mut router = Router::new() + .hoop(affix_state::inject(Arc::new(tmp))) .hoop(affix_state::inject(db)) .hoop(affix_state::inject(runtime)); if let Some(auth) = auth { diff --git a/src/admin_http/tests/project_paths.rs b/src/admin_http/tests/project_paths.rs new file mode 100644 index 000000000..9857f3ef7 --- /dev/null +++ b/src/admin_http/tests/project_paths.rs @@ -0,0 +1,81 @@ +use super::*; + +async fn project_path_call(service: &Service, operation: &str, path: &str) -> (StatusCode, Value) { + let mut response = TestClient::post(format!("http://127.0.0.1/admin/projects/{operation}")) + .add_header("host", "127.0.0.1", true) + .add_header("origin", "http://127.0.0.1", true) + .add_header("content-type", "application/json", true) + .body( + json!({ + "client_id": "runner", + "project_id": "demo", + "name": "Demo", + "path": path, + "idempotency_key": "path-validation" + }) + .to_string(), + ) + .send(service) + .await; + let status = response.status_code.unwrap(); + (status, response.take_json().await.unwrap()) +} + +#[tokio::test] +async fn admin_project_paths_accept_cross_platform_absolute_shapes() { + let mut admin = AuthContext::new(AuthKind::ApiToken); + admin.scopes.push(SCOPE_ADMIN.to_string()); + let service = service(Some(admin)); + let max_length_path = format!("D:/{}", "a".repeat(4093)); + for operation in ["register", "create"] { + for path in [ + "/srv/projects/demo", + r"D:\projects\demo", + "d:/projects/demo", + r"\\server\share\demo", + r"\\?\D:\projects\demo", + r"\\?\UNC\server\share\demo", + "//?/D:/projects/demo", + &max_length_path, + ] { + let (status, body) = project_path_call(&service, operation, path).await; + // With no Runner connected, a valid request must reach Runner lookup. + // Acceptance here does not assert that its filesystem policy allows it. + assert_eq!( + status, + StatusCode::SERVICE_UNAVAILABLE, + "{operation}: {path:?}: {body}" + ); + assert_eq!(body, json!({"error": {"code": "agent_unavailable"}})); + } + } +} + +#[tokio::test] +async fn admin_project_paths_reject_relative_nul_and_oversized_inputs() { + let mut admin = AuthContext::new(AuthKind::ApiToken); + admin.scopes.push(SCOPE_ADMIN.to_string()); + let service = service(Some(admin)); + let oversized_path = format!("D:/{}", "a".repeat(4094)); + for operation in ["register", "create"] { + for path in [ + "", + "relative/path", + r"relative\path", + "D:", + r"D:projects\demo", + r"\projects\demo", + "/srv/projects/\0demo", + "D:\\projects\\\0demo", + &oversized_path, + ] { + let (status, body) = project_path_call(&service, operation, path).await; + assert_eq!( + status, + StatusCode::BAD_REQUEST, + "{operation}: {path:?}: {body}" + ); + assert_eq!(body, json!({"error": {"code": "invalid_request"}})); + } + } +} diff --git a/src/admin_project_lifecycle.rs b/src/admin_project_lifecycle.rs index b2393a8d7..9cf1632c8 100644 --- a/src/admin_project_lifecycle.rs +++ b/src/admin_project_lifecycle.rs @@ -12,6 +12,7 @@ use std::collections::HashMap; use std::sync::{Arc, OnceLock}; use std::time::Duration; use tokio::sync::Mutex; +use webcodex_core::runtime_contract::validate_project_op_path; const WAIT_SECS: u64 = 30; pub(crate) const IDEMPOTENCY_KEY_MAX: usize = 128; @@ -728,9 +729,8 @@ fn validate_common( } if name.trim().is_empty() || name.len() > 120 - || path.is_empty() || path.len() > 4096 - || !path.starts_with('/') + || validate_project_op_path(path).is_err() || description.is_some_and(|v| v.len() > 500) { return Err(api_error(400, "invalid_request"));