From b857efbf54e3885e489a0a523f028ccfebb3e80d Mon Sep 17 00:00:00 2001 From: vincent Date: Wed, 28 Oct 2026 08:05:47 +1000 Subject: [PATCH 1/9] feat(auth): carry explicit project-share parent-root grants Add an optional canonical parent-root grant to project-share OAuth subjects. The grant is carried on AuthContext, populated from the Server's WEBCODEX_OAUTH2_PROJECT_SHARE_ROOTS environment for project-share OAuth tokens only, and is bounded to eight roots. Roots are absolute, canonicalized, existing directories; relative, missing, non-directory and symlinked spellings resolve to the real target (or are dropped, which only removes authority). Every other credential class keeps an empty list, preserving the exact-path fence. Test fixtures construct the new field as empty. --- src/admin_project_lifecycle.rs | 2 + src/auth/context.rs | 8 ++ src/auth/mod.rs | 6 +- src/auth/project_share.rs | 96 +++++++++++++++++++ src/auth/tokens.rs | 1 + src/config.rs | 21 ++++ src/mcp_tests/result_app.rs | 2 + src/runner_http/mod_tests.rs | 6 ++ .../mod_tests/shared_key_isolation.rs | 2 + src/runtime_http/tests/import_http_tests.rs | 4 + src/tool_runtime/tests/artifact_transfer.rs | 2 + src/tool_runtime/tests/metadata.rs | 3 + src/tool_runtime/tests/support/auth.rs | 10 ++ 13 files changed, 160 insertions(+), 3 deletions(-) diff --git a/src/admin_project_lifecycle.rs b/src/admin_project_lifecycle.rs index b7f8f0ec1..8b6d76118 100644 --- a/src/admin_project_lifecycle.rs +++ b/src/admin_project_lifecycle.rs @@ -873,6 +873,8 @@ mod tests { allowed_client_id: None, shared_key_hash: None, project_grant_id: None, + + project_root_scopes: Vec::new(), } } diff --git a/src/auth/context.rs b/src/auth/context.rs index 54556561e..791a4da5f 100644 --- a/src/auth/context.rs +++ b/src/auth/context.rs @@ -1,5 +1,7 @@ //! Depot-injected authentication identity and credential classification. +use std::path::PathBuf; + use crate::auth::scopes::SCOPE_ADMIN; #[derive(Debug, Clone, PartialEq, Eq)] @@ -45,6 +47,11 @@ pub struct AuthContext { pub shared_key_hash: Option, /// Stable, non-secret identity of the configured project credential. pub project_grant_id: Option, + /// Canonical parent directories an explicitly authorized project-share + /// subject may admit new Projects from. Populated only for OAuth + /// project-share subjects whose share was started with `--project-roots`; + /// every other credential class keeps this empty. + pub project_root_scopes: Vec, } impl AuthContext { @@ -61,6 +68,7 @@ impl AuthContext { allowed_client_id: None, shared_key_hash: None, project_grant_id: None, + project_root_scopes: Vec::new(), } } diff --git a/src/auth/mod.rs b/src/auth/mod.rs index bd5877f34..86910fcac 100644 --- a/src/auth/mod.rs +++ b/src/auth/mod.rs @@ -67,9 +67,9 @@ pub(crate) use project_credential::{ }; pub(crate) use project_share::{ configured_project_share_subject, generate_project_share_session_id, - parse_project_share_subject_id, project_share_scopes_are_bounded, - validate_project_share_grant_subject, PROJECT_SHARE_OAUTH_SCOPES, - PROJECT_SHARE_OAUTH_SUBJECT_KIND, PROJECT_SHARE_OAUTH_TOKEN_KIND, + parse_project_share_subject_id, project_share_root_scopes_from_config, + project_share_scopes_are_bounded, validate_project_share_grant_subject, + PROJECT_SHARE_OAUTH_SCOPES, PROJECT_SHARE_OAUTH_SUBJECT_KIND, PROJECT_SHARE_OAUTH_TOKEN_KIND, }; pub use scopes::{ diff --git a/src/auth/project_share.rs b/src/auth/project_share.rs index 4e70f7aeb..229c1e87e 100644 --- a/src/auth/project_share.rs +++ b/src/auth/project_share.rs @@ -5,6 +5,8 @@ //! old authorization codes/access tokens/refresh tokens unusable without //! changing ownership of existing project tasks. +use std::path::PathBuf; + use super::{ SCOPE_JOB_RUN, SCOPE_PROJECT_READ, SCOPE_PROJECT_WRITE, SCOPE_RUNTIME_READ, SCOPE_SESSION_COLLABORATE, @@ -63,8 +65,102 @@ pub(crate) fn validate_project_share_grant_subject( Ok(()) } +/// Canonical parent-root grants for project-share OAuth subjects. +/// +/// Roots are canonicalized so later containment checks compare real directory +/// components, not textual prefixes. A configured root that no longer resolves +/// (deleted or unreadable since server start) is dropped rather than failing +/// authentication: dropping it only removes authority, never grants more. +/// Relative configured roots are dropped for the same reason: `canonicalize` +/// would resolve them against the Server process cwd and silently grant +/// whatever that lands on. Only the `--project-roots` CLI flag and the +/// persisted grant file reach this function through +/// `validate_share_project_root`, which already requires absolute paths; this +/// check closes the remaining `WEBCODEX_OAUTH2_PROJECT_SHARE_ROOTS` env path. +/// Symlinks inside the root are resolved by canonicalization, so a link that +/// escapes the root cannot pass the fence through its link name. +pub(crate) fn project_share_root_scopes_from_config(config: &crate::Config) -> Vec { + config + .oauth2 + .project_share_roots + .iter() + .filter_map(|root| { + if !root.is_absolute() { + tracing::warn!( + root = %root.display(), + "configured project-share root is relative; ignored" + ); + return None; + } + match root.canonicalize() { + Ok(canonical) if canonical.is_dir() => Some(canonical), + Ok(_) => { + tracing::warn!("configured project-share root is not a directory; ignored"); + None + } + Err(error) => { + tracing::warn!(%error, "configured project-share root did not resolve; ignored"); + None + } + } + }) + .collect() +} + pub(crate) fn project_share_scopes_are_bounded(scopes: &str) -> bool { scopes .split_ascii_whitespace() .all(|scope| PROJECT_SHARE_OAUTH_SCOPES.contains(&scope) || scope == "offline_access") } + +#[cfg(test)] +mod tests { + use super::*; + + fn config_with_roots(roots: Vec) -> crate::Config { + crate::Config { + addr: "127.0.0.1:0".to_string(), + data_dir: PathBuf::from("./data"), + token: Some("bootstrap-token".to_string()), + max_text_size: 2 * 1024 * 1024, + oauth2: crate::OAuth2Config { + project_share_roots: roots, + ..crate::OAuth2Config::default() + }, + } + } + + #[test] + fn project_share_root_scopes_drop_relative_and_unresolvable_roots() { + let dir = tempfile::tempdir().unwrap(); + let granted = dir.path().join("granted"); + std::fs::create_dir(&granted).unwrap(); + + // Core policy, platform-independent: one admitted absolute directory, + // one relative spelling that would resolve from any cwd, one missing + // absolute path (dropped as before). + let config = config_with_roots(vec![ + granted.clone(), + PathBuf::from(".."), + dir.path().join("missing"), + ]); + let scopes = project_share_root_scopes_from_config(&config); + assert_eq!(scopes, vec![granted.canonicalize().unwrap()]); + } + + #[cfg(unix)] + #[test] + fn project_share_root_scopes_resolve_a_symlinked_root_to_its_target() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("granted"); + std::fs::create_dir(&target).unwrap(); + // A symlinked spelling of a configured root resolves to its target: + // the authority is the real directory, never the link name. + let link = dir.path().join("link-to-granted"); + std::os::unix::fs::symlink(&target, &link).unwrap(); + + let config = config_with_roots(vec![link]); + let scopes = project_share_root_scopes_from_config(&config); + assert_eq!(scopes, vec![target.canonicalize().unwrap()]); + } +} diff --git a/src/auth/tokens.rs b/src/auth/tokens.rs index c99017e6d..fa403db18 100644 --- a/src/auth/tokens.rs +++ b/src/auth/tokens.rs @@ -375,6 +375,7 @@ impl OAuth2Verifier { token_kind: Some(crate::auth::PROJECT_SHARE_OAUTH_TOKEN_KIND.to_string()), allowed_client_id: Some(at_record.client_id.clone()), project_grant_id: Some(grant_id.to_string()), + project_root_scopes: crate::auth::project_share_root_scopes_from_config(config), ..AuthContext::new(AuthKind::OAuth2Token) } } diff --git a/src/config.rs b/src/config.rs index f9cd5b263..8d855bcd9 100644 --- a/src/config.rs +++ b/src/config.rs @@ -457,8 +457,17 @@ pub struct OAuth2Config { /// Ephemeral share-session fence. A new `webcodex share --auth oauth` /// process uses a new value, invalidating every older project-share grant. pub project_share_session_id: Option, + /// Parent directories this project share may admit new Projects from, on + /// demand, for its own project-share OAuth subjects. Empty by default: + /// without an explicit grant the exact-path project fence stays in force. + pub project_share_roots: Vec, } +/// Upper bound on parent-root grants per share. Keeps the per-token attach and +/// the path fence bounded; one share with eight parent roots is already far +/// beyond any real working setup. +pub const MAX_PROJECT_SHARE_ROOTS: usize = 8; + impl fmt::Debug for OAuth2Config { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("OAuth2Config") @@ -488,6 +497,7 @@ impl fmt::Debug for OAuth2Config { "project_share_session_id", &self.project_share_session_id.as_ref().map(|_| "[REDACTED]"), ) + .field("project_share_roots_count", &self.project_share_roots.len()) .finish() } } @@ -506,6 +516,7 @@ impl Default for OAuth2Config { trust_loopback_api_token_mcp_file_import: false, project_share_grant_id: None, project_share_session_id: None, + project_share_roots: Vec::new(), } } } @@ -579,6 +590,15 @@ impl OAuth2Config { .ok() .map(|value| value.trim().to_string()) .filter(|value| !value.is_empty()); + let project_share_roots = std::env::var("WEBCODEX_OAUTH2_PROJECT_SHARE_ROOTS") + .ok() + .map(|value| { + std::env::split_paths(&value) + .filter(|path| !path.as_os_str().is_empty()) + .take(MAX_PROJECT_SHARE_ROOTS) + .collect::>() + }) + .unwrap_or_default(); Self { issuer, enabled, @@ -591,6 +611,7 @@ impl OAuth2Config { trust_loopback_api_token_mcp_file_import, project_share_grant_id, project_share_session_id, + project_share_roots, } } } diff --git a/src/mcp_tests/result_app.rs b/src/mcp_tests/result_app.rs index 400e22972..9aed6f044 100644 --- a/src/mcp_tests/result_app.rs +++ b/src/mcp_tests/result_app.rs @@ -1725,6 +1725,8 @@ fn result_app_auth() -> crate::auth::AuthContext { allowed_client_id: None, shared_key_hash: None, project_grant_id: None, + + project_root_scopes: Vec::new(), } } diff --git a/src/runner_http/mod_tests.rs b/src/runner_http/mod_tests.rs index 42f113065..d9577c5e0 100644 --- a/src/runner_http/mod_tests.rs +++ b/src/runner_http/mod_tests.rs @@ -35,6 +35,8 @@ fn auth_context(username: Option<&str>, is_bootstrap: bool) -> crate::auth::Auth allowed_client_id: None, shared_key_hash: None, project_grant_id: None, + + project_root_scopes: Vec::new(), } } @@ -57,6 +59,8 @@ fn agent_auth_context( allowed_client_id: Some(allowed_client_id.to_string()), shared_key_hash: None, project_grant_id: None, + + project_root_scopes: Vec::new(), } } @@ -77,6 +81,8 @@ fn oauth_bridge_auth_context(hash: &str, scopes: Vec<&str>) -> crate::auth::Auth allowed_client_id: Some("oauth-client".to_string()), shared_key_hash: Some(hash.to_string()), project_grant_id: None, + + project_root_scopes: Vec::new(), } } diff --git a/src/runner_http/mod_tests/shared_key_isolation.rs b/src/runner_http/mod_tests/shared_key_isolation.rs index acc3c035c..bc429ff60 100644 --- a/src/runner_http/mod_tests/shared_key_isolation.rs +++ b/src/runner_http/mod_tests/shared_key_isolation.rs @@ -16,6 +16,8 @@ fn managed_oauth_auth_context( allowed_client_id: Some("oauth-client".to_string()), shared_key_hash: shared_key_hash.map(str::to_string), project_grant_id: None, + + project_root_scopes: Vec::new(), } } diff --git a/src/runtime_http/tests/import_http_tests.rs b/src/runtime_http/tests/import_http_tests.rs index 89c041324..5187e235c 100644 --- a/src/runtime_http/tests/import_http_tests.rs +++ b/src/runtime_http/tests/import_http_tests.rs @@ -527,6 +527,8 @@ async fn runtime_conversation_import_host_ref_saves_pptx_through_artifact_path_b allowed_client_id: None, shared_key_hash: None, project_grant_id: None, + + project_root_scopes: Vec::new(), }; let outcome = tokio::time::timeout( Duration::from_secs(5), @@ -607,6 +609,8 @@ async fn runtime_conversation_import_rejects_untrusted_api_transport() { allowed_client_id: None, shared_key_hash: None, project_grant_id: None, + + project_root_scopes: Vec::new(), }; let call = ToolCall::from_tool_name( "import_host_files", diff --git a/src/tool_runtime/tests/artifact_transfer.rs b/src/tool_runtime/tests/artifact_transfer.rs index 4e7f9122b..1a68bfcc1 100644 --- a/src/tool_runtime/tests/artifact_transfer.rs +++ b/src/tool_runtime/tests/artifact_transfer.rs @@ -43,6 +43,8 @@ fn transfer_auth(username: &str) -> AuthContext { allowed_client_id: Some("transfer-test-client".to_string()), shared_key_hash: None, project_grant_id: None, + + project_root_scopes: Vec::new(), } } diff --git a/src/tool_runtime/tests/metadata.rs b/src/tool_runtime/tests/metadata.rs index cc1db3f22..53936c062 100644 --- a/src/tool_runtime/tests/metadata.rs +++ b/src/tool_runtime/tests/metadata.rs @@ -136,6 +136,7 @@ fn shared_key_auth(hash: &str) -> crate::auth::AuthContext { allowed_client_id: None, shared_key_hash: Some(hash.to_string()), project_grant_id: None, + project_root_scopes: Vec::new(), } } @@ -158,6 +159,7 @@ fn open_auth() -> crate::auth::AuthContext { allowed_client_id: None, shared_key_hash: None, project_grant_id: None, + project_root_scopes: Vec::new(), } } @@ -174,6 +176,7 @@ fn bootstrap_auth() -> crate::auth::AuthContext { allowed_client_id: None, shared_key_hash: None, project_grant_id: None, + project_root_scopes: Vec::new(), } } diff --git a/src/tool_runtime/tests/support/auth.rs b/src/tool_runtime/tests/support/auth.rs index bed3f2103..d4bf4cc8d 100644 --- a/src/tool_runtime/tests/support/auth.rs +++ b/src/tool_runtime/tests/support/auth.rs @@ -27,6 +27,8 @@ pub(in crate::tool_runtime::tests) fn auth_context( allowed_client_id: None, shared_key_hash: None, project_grant_id: None, + + project_root_scopes: Vec::new(), } } @@ -54,6 +56,8 @@ pub(in crate::tool_runtime::tests) fn shared_key_auth_context( allowed_client_id: None, shared_key_hash: Some(hash.to_string()), project_grant_id: None, + + project_root_scopes: Vec::new(), } } @@ -73,6 +77,8 @@ pub(in crate::tool_runtime::tests) fn oauth_bridge_auth_context( allowed_client_id: Some("oauth-client".to_string()), shared_key_hash: Some(hash.to_string()), project_grant_id: None, + + project_root_scopes: Vec::new(), } } @@ -96,6 +102,8 @@ pub(in crate::tool_runtime::tests) fn managed_oauth_auth_context( allowed_client_id: Some("oauth-client".to_string()), shared_key_hash: shared_key_hash.map(str::to_string), project_grant_id: None, + + project_root_scopes: Vec::new(), } } @@ -118,6 +126,8 @@ pub(in crate::tool_runtime::tests) fn open_auth_context() -> crate::auth::AuthCo allowed_client_id: None, shared_key_hash: None, project_grant_id: None, + + project_root_scopes: Vec::new(), } } From 89ea872dedd27ae1aa593d523f2ec120ec4df36a Mon Sep 17 00:00:00 2001 From: vincent Date: Wed, 28 Oct 2026 08:05:47 +1000 Subject: [PATCH 2/9] feat(runner): revalidate share grants at the project registry write boundary Accept an optional project_root_scopes field on project resolution and managed-worktree preparation. Malformed, empty or relative scopes fail closed; ordinary requests keep their previous exact payload shapes. Before any registration or worktree side effect, the Runner re-derives containment from its own canonical resolution of the committed target against the granted roots. A source that resolved differently at Server-fence time than at the Runner write boundary is rejected. The managed-worktree grant binds the source only; the derived sibling destination remains governed by allowed_roots. Move resume handling after source identity validation so resume is anchored to the registered source identity as well as its stored path. --- crates/webcodex-runner/src/main_tests.rs | 476 +++++++++++++++++- .../src/main_tests/project_registration.rs | 204 ++++++++ .../projects/managed_worktree.rs | 94 +++- .../webcodex_runner/projects/registration.rs | 75 ++- 4 files changed, 821 insertions(+), 28 deletions(-) diff --git a/crates/webcodex-runner/src/main_tests.rs b/crates/webcodex-runner/src/main_tests.rs index 84292c9da..08417cf8c 100644 --- a/crates/webcodex-runner/src/main_tests.rs +++ b/crates/webcodex-runner/src/main_tests.rs @@ -1234,10 +1234,14 @@ fn seed_managed_worktree_repo(source: &Path) -> (String, String) { } fn register_managed_source_project(registry: &Path, source: &Path) { + register_managed_source_project_as(registry, source, "source"); +} + +fn register_managed_source_project_as(registry: &Path, source: &Path, id: &str) { std::fs::create_dir_all(registry).unwrap(); let source = source.canonicalize().unwrap(); let project = RunnerProjectFile { - id: "source".to_string(), + id: id.to_string(), path: source.to_string_lossy().into_owned(), shell_profile: None, allow_patch: true, @@ -1256,7 +1260,7 @@ fn register_managed_source_project(registry: &Path, source: &Path) { managed_operation_id: None, }; std::fs::write( - registry.join("source.toml"), + registry.join(format!("{id}.toml")), toml::to_string(&project).unwrap(), ) .unwrap(); @@ -1279,10 +1283,11 @@ fn managed_worktree_request( ) } -fn managed_worktree_request_with_source_identity( +fn managed_worktree_request_with_expected_identity( source: &Path, base_ref: serde_json::Value, operation_id: &str, + resume_project_id: Option<&str>, source_project_id: &str, source_root_fingerprint: &str, ) -> RunnerRequest { @@ -1292,13 +1297,30 @@ fn managed_worktree_request_with_source_identity( "path": source.to_string_lossy(), "base_ref": base_ref, "operation_id": operation_id, - "resume_project_id": null, + "resume_project_id": resume_project_id, "expected_source_project_id": source_project_id, "expected_source_root_fingerprint": source_root_fingerprint, }), ) } +fn managed_worktree_request_with_source_identity( + source: &Path, + base_ref: serde_json::Value, + operation_id: &str, + source_project_id: &str, + source_root_fingerprint: &str, +) -> RunnerRequest { + managed_worktree_request_with_expected_identity( + source, + base_ref, + operation_id, + None, + source_project_id, + source_root_fingerprint, + ) +} + #[test] fn project_root_fingerprint_uses_platform_path_identity_rules() { #[cfg(windows)] @@ -1452,6 +1474,171 @@ fn managed_worktree_expected_source_identity_is_revalidated_before_creation() { assert!(!tmp.path().join(".webcodex-worktrees").exists()); } +#[cfg(unix)] +#[test] +fn managed_worktree_expected_source_identity_denies_symlink_source_switched_to_other_project() { + let tmp = tempfile::tempdir().unwrap(); + let source = tmp.path().join("source"); + let other = tmp.path().join("other"); + let registry = tmp.path().join("project-registry"); + seed_managed_worktree_repo(&source); + seed_managed_worktree_repo(&other); + register_managed_source_project_as(®istry, &source, "source"); + register_managed_source_project_as(®istry, &other, "other"); + // A symlink is exact-path aliasing only: the request path resolves to the + // other registered project's canonical root, and the root fingerprint is a + // hash of that canonical path (not an inode identity), so id and + // fingerprint both stop matching the expected source. + let alias = tmp.path().join("alias"); + std::os::unix::fs::symlink(&other, &alias).unwrap(); + let policy = project_policy(tmp.path()); + + let error = project_error_value(handle_prepare_managed_worktree( + &policy, + ®istry, + &managed_worktree_request_with_expected_identity( + &alias, + serde_json::Value::Null, + "22222222-2222-4222-8222-222222222222", + None, + "source", + &project_root_fingerprint(&source.canonicalize().unwrap()), + ), + )); + assert_eq!( + error["error_code"], + "managed_worktree_source_identity_changed" + ); + assert_eq!(error["state_changed"], false); + assert_eq!( + error["source_project_id"], "other", + "the symlink must resolve to the registered project it points at" + ); + assert!( + !tmp.path().join(".webcodex-worktrees").exists(), + "an identity mismatch must deny before any worktree side effect" + ); + assert_eq!( + load_runner_project_summaries_from_dir(®istry).len(), + 2, + "a denied identity switch must not add Project records" + ); +} + +#[cfg(unix)] +#[test] +fn managed_worktree_expected_source_identity_resume_denies_symlink_source_switched_to_other_project( +) { + let tmp = tempfile::tempdir().unwrap(); + let source = tmp.path().join("source"); + let other = tmp.path().join("other"); + let registry = tmp.path().join("project-registry"); + seed_managed_worktree_repo(&source); + seed_managed_worktree_repo(&other); + register_managed_source_project_as(®istry, &source, "source"); + register_managed_source_project_as(®istry, &other, "other"); + let policy = project_policy(tmp.path()); + let operation = "23232323-2323-4323-8323-232323232323"; + let created = project_ok(handle_prepare_managed_worktree( + &policy, + ®istry, + &managed_worktree_request_with_source_identity( + &source, + serde_json::Value::Null, + operation, + "source", + &project_root_fingerprint(&source.canonicalize().unwrap()), + ), + )); + let managed_id = created["agent_project_id"].as_str().unwrap(); + let worktree_path = created["path"].as_str().unwrap().to_string(); + + // The caller's source path is now a symlink to a different registered + // project; the expected identity still names the original source. + let alias = tmp.path().join("alias"); + std::os::unix::fs::symlink(&other, &alias).unwrap(); + let resume = managed_worktree_request_with_expected_identity( + &alias, + serde_json::Value::Null, + operation, + Some(managed_id), + "source", + &project_root_fingerprint(&source.canonicalize().unwrap()), + ); + let error = project_error_value(handle_prepare_managed_worktree(&policy, ®istry, &resume)); + assert_eq!(error["state_changed"], false); + assert!( + error["error_code"] == "managed_worktree_source_identity_changed" + || error["error_code"] == "managed_worktree_resume_mismatch", + "expected identity must fail closed on resume, got: {error}" + ); + assert!( + Path::new(&worktree_path).exists(), + "the existing managed worktree must remain untouched" + ); + assert_eq!( + std::fs::read_dir(tmp.path().join(".webcodex-worktrees")) + .unwrap() + .count(), + 1, + "a denied resume must not create additional worktrees" + ); + assert_eq!( + load_runner_project_summaries_from_dir(®istry).len(), + 3, + "a denied resume must not add Project records" + ); +} + +#[test] +fn managed_worktree_expected_source_identity_allows_resume_without_duplicate() { + let tmp = tempfile::tempdir().unwrap(); + let source = tmp.path().join("source"); + let registry = tmp.path().join("project-registry"); + seed_managed_worktree_repo(&source); + register_managed_source_project(®istry, &source); + let policy = project_policy(tmp.path()); + let operation = "21212121-2121-4121-8121-212121212121"; + let fingerprint = project_root_fingerprint(&source.canonicalize().unwrap()); + let created = project_ok(handle_prepare_managed_worktree( + &policy, + ®istry, + &managed_worktree_request_with_source_identity( + &source, + serde_json::Value::Null, + operation, + "source", + &fingerprint, + ), + )); + let resume = managed_worktree_request_with_expected_identity( + &source, + serde_json::Value::Null, + operation, + created["agent_project_id"].as_str(), + "source", + &fingerprint, + ); + let resumed = project_ok(handle_prepare_managed_worktree(&policy, ®istry, &resume)); + assert_eq!(resumed["agent_project_id"], created["agent_project_id"]); + assert_eq!(resumed["path"], created["path"]); + assert_eq!(resumed["outcome"], "managed_worktree_recovered"); + assert_eq!(resumed["registered"], false); + assert_eq!(resumed["changed"], false); + assert_eq!( + load_runner_project_summaries_from_dir(®istry).len(), + 2, + "a matching identity resume must reuse the existing managed record" + ); + assert_eq!( + std::fs::read_dir(tmp.path().join(".webcodex-worktrees")) + .unwrap() + .count(), + 1, + "a matching identity resume must not create a second worktree" + ); +} + #[test] fn managed_worktree_bootstrap_is_detached_registered_and_same_operation_recovers() { let tmp = tempfile::tempdir().unwrap(); @@ -2138,3 +2325,284 @@ async fn websocket_session_accepts_pong_without_error_or_disconnect() { server_task.await.unwrap(); } + +/// Reachability fixture for the share-grant managed-worktree boundary. +/// +/// The spelling the Server authorizes lives inside a granted parent; by the +/// time the Runner resolves it, the link points at an *existing registered* +/// project outside the grant that is still inside the Runner allowed_roots. +/// Without a forwarded share grant the Runner has no way to tell the +/// difference, which is exactly the boundary under test. +#[cfg(unix)] +fn seed_managed_source_flip( + tmp: &Path, +) -> (std::path::PathBuf, std::path::PathBuf, std::path::PathBuf) { + let source_root = tmp.join("outside").join("repo"); + seed_managed_worktree_repo(&source_root); + let registry = tmp.join("project-registry"); + register_managed_source_project(®istry, &source_root); + let granted = tmp.join("granted"); + std::fs::create_dir(&granted).unwrap(); + std::fs::create_dir(granted.join("placeholder")).unwrap(); + std::os::unix::fs::symlink(&source_root, granted.join("link")).unwrap(); + (granted, registry, source_root) +} + +#[test] +#[cfg(unix)] +fn managed_worktree_without_share_grant_keeps_pre_fix_behavior() { + let tmp = tempfile::tempdir().unwrap(); + let (granted, registry, source_root) = seed_managed_source_flip(tmp.path()); + let policy = project_policy(tmp.path()); + // Ordinary credentials forward no grant: the derived worktree from the + // flipped, outside-grant source still succeeds, which proves this branch + // had no share-grant enforcement before the fix. + let result = project_ok(handle_prepare_managed_worktree( + &policy, + ®istry, + &managed_worktree_request( + &granted.join("link"), + serde_json::Value::Null, + "22222222-2222-4222-8222-222222222222", + None, + ), + )); + assert_eq!(result["outcome"], "managed_worktree_created"); + assert_eq!(result["registered"], true); + let worktree = std::path::PathBuf::from(result["path"].as_str().unwrap()); + assert_ne!( + worktree.canonicalize().unwrap(), + source_root.canonicalize().unwrap() + ); + let records = std::fs::read_dir(®istry) + .unwrap() + .filter(|entry| entry.as_ref().unwrap().path().extension() == Some("toml".as_ref())) + .count(); + assert_eq!(records, 2, "fixture proof: a second project was registered"); +} + +fn managed_worktree_request_with_grant( + source: &Path, + base_ref: serde_json::Value, + operation_id: &str, + resume_project_id: Option<&str>, + granted_roots: &[&Path], +) -> RunnerRequest { + project_request( + "prepare_managed_worktree", + serde_json::json!({ + "path": source.to_string_lossy(), + "base_ref": base_ref, + "operation_id": operation_id, + "resume_project_id": resume_project_id, + "project_root_scopes": granted_roots + .iter() + .map(|root| root.to_string_lossy()) + .collect::>(), + }), + ) +} + +fn managed_registry_records(registry: &Path) -> usize { + std::fs::read_dir(registry) + .unwrap() + .filter(|entry| entry.as_ref().unwrap().path().extension() == Some("toml".as_ref())) + .count() +} + +#[test] +fn managed_worktree_share_grant_source_inside_grant_succeeds() { + let tmp = tempfile::tempdir().unwrap(); + let granted = tmp.path().join("granted"); + let source = granted.join("repo"); + let registry = tmp.path().join("project-registry"); + seed_managed_worktree_repo(&source); + register_managed_source_project(®istry, &source); + let policy = project_policy(tmp.path()); + let granted_root = granted.canonicalize().unwrap(); + + let result = project_ok(handle_prepare_managed_worktree( + &policy, + ®istry, + &managed_worktree_request_with_grant( + &source, + serde_json::Value::Null, + "33333333-3333-4333-8333-333333333333", + None, + &[granted_root.as_path()], + ), + )); + assert_eq!(result["outcome"], "managed_worktree_created"); + assert_eq!(result["registered"], true); + let worktree = std::path::PathBuf::from(result["path"].as_str().unwrap()) + .canonicalize() + .unwrap(); + // The derived sibling layout is Runner-owned and deliberately outside the + // source grant while still inside allowed_roots. + assert!( + !webcodex_runner_config::paths::path_is_within(&worktree, &granted_root), + "{worktree:?}" + ); + assert!(webcodex_runner_config::paths::path_is_within( + &worktree, + &tmp.path().canonicalize().unwrap() + )); + assert_eq!(managed_registry_records(®istry), 2); +} + +#[test] +#[cfg(unix)] +fn managed_worktree_share_grant_rejects_flipped_source_outside_grant() { + let tmp = tempfile::tempdir().unwrap(); + let (granted, registry, _source_root) = seed_managed_source_flip(tmp.path()); + let policy = project_policy(tmp.path()); + let granted_root = granted.canonicalize().unwrap(); + + let error = project_error_value(handle_prepare_managed_worktree( + &policy, + ®istry, + &managed_worktree_request_with_grant( + &granted.join("link"), + serde_json::Value::Null, + "44444444-4444-4444-8444-444444444444", + None, + &[granted_root.as_path()], + ), + )); + assert_eq!(error["error_code"], "project_path_outside_granted_roots"); + assert_eq!(error["state_changed"], false); + // No side effects: no extra registration and no managed-worktree namespace. + assert_eq!(managed_registry_records(®istry), 1); + assert!(!tmp.path().join(".webcodex-worktrees").exists()); +} + +#[test] +fn managed_worktree_share_grant_retry_and_resume_do_not_widen_or_duplicate() { + let tmp = tempfile::tempdir().unwrap(); + let granted = tmp.path().join("granted"); + let source = granted.join("repo"); + let registry = tmp.path().join("project-registry"); + seed_managed_worktree_repo(&source); + register_managed_source_project(®istry, &source); + let policy = project_policy(tmp.path()); + let granted_root = granted.canonicalize().unwrap(); + let operation_id = "55555555-5555-4555-8555-555555555555"; + + let first = project_ok(handle_prepare_managed_worktree( + &policy, + ®istry, + &managed_worktree_request_with_grant( + &source, + serde_json::Value::Null, + operation_id, + None, + &[granted_root.as_path()], + ), + )); + assert_eq!(first["outcome"], "managed_worktree_created"); + let worktree_id = first["agent_project_id"].as_str().unwrap().to_string(); + assert_eq!(managed_registry_records(®istry), 2); + + // Same operation_id: re-observe the deterministic target, never a second + // worktree, never an extra grant. + let retry = project_ok(handle_prepare_managed_worktree( + &policy, + ®istry, + &managed_worktree_request_with_grant( + &source, + serde_json::Value::Null, + operation_id, + None, + &[granted_root.as_path()], + ), + )); + assert_eq!(retry["outcome"], "managed_worktree_recovered"); + assert_eq!(retry["agent_project_id"], worktree_id); + assert_eq!(managed_registry_records(®istry), 2); + + // Resume that points at the existing managed project but whose source is + // outside the grant is refused before resolution: resuming never widens + // the grant. + let outside = tmp.path().join("outside").join("repo"); + seed_managed_worktree_repo(&outside); + let error = project_error_value(handle_prepare_managed_worktree( + &policy, + ®istry, + &managed_worktree_request_with_grant( + &outside, + serde_json::Value::Null, + operation_id, + Some(&worktree_id), + &[granted_root.as_path()], + ), + )); + assert_eq!(error["error_code"], "project_path_outside_granted_roots"); + assert_eq!(managed_registry_records(®istry), 2); + + // The legitimate resume still converges on the same identity. + let resumed = project_ok(handle_prepare_managed_worktree( + &policy, + ®istry, + &managed_worktree_request_with_grant( + &source, + serde_json::Value::Null, + operation_id, + Some(&worktree_id), + &[granted_root.as_path()], + ), + )); + assert_eq!(resumed["agent_project_id"], worktree_id); + assert_eq!(managed_registry_records(®istry), 2); +} + +#[test] +fn managed_worktree_rejects_malformed_grant_scopes_fail_closed() { + let tmp = tempfile::tempdir().unwrap(); + let granted = tmp.path().join("granted"); + let source = granted.join("repo"); + let registry = tmp.path().join("project-registry"); + seed_managed_worktree_repo(&source); + register_managed_source_project(®istry, &source); + let policy = project_policy(tmp.path()); + let granted_root = granted.canonicalize().unwrap(); + + let mut base = |extra: serde_json::Value| { + let mut payload = serde_json::json!({ + "path": source.to_string_lossy(), + "base_ref": serde_json::Value::Null, + "operation_id": "66666666-6666-4666-8666-666666666666", + "resume_project_id": serde_json::Value::Null, + }); + for (key, value) in extra.as_object().unwrap() { + payload[key] = value.clone(); + } + project_request("prepare_managed_worktree", payload) + }; + + for (label, payload) in [ + ( + "relative root entry", + serde_json::json!({"project_root_scopes": ["relative/root"]}), + ), + ( + "empty scope set", + serde_json::json!({"project_root_scopes": []}), + ), + ( + "non-array scopes", + serde_json::json!({"project_root_scopes": granted_root.to_string_lossy()}), + ), + ( + "unknown fifth field", + serde_json::json!({"unexpected": true}), + ), + ] { + let error = project_error_value(handle_prepare_managed_worktree( + &policy, + ®istry, + &base(payload), + )); + assert_eq!(error["error_code"], "invalid_request", "{label}"); + assert_eq!(managed_registry_records(®istry), 1, "{label}"); + } +} diff --git a/crates/webcodex-runner/src/main_tests/project_registration.rs b/crates/webcodex-runner/src/main_tests/project_registration.rs index 09b91d065..350fe1a84 100644 --- a/crates/webcodex-runner/src/main_tests/project_registration.rs +++ b/crates/webcodex-runner/src/main_tests/project_registration.rs @@ -724,3 +724,207 @@ fn register_project_overwrite_semantics_are_accurate() { assert_eq!(overwritten["created_config"], false); assert_eq!(overwritten["overwritten"], true); } + +#[test] +fn resolve_or_register_project_rejects_target_outside_granted_roots_before_any_registration() { + let tmp = tempfile::tempdir().unwrap(); + let granted = tmp.path().join("granted"); + let inner = granted.join("inner"); + let outside = tmp.path().join("outside"); + std::fs::create_dir_all(&inner).unwrap(); + std::fs::create_dir(&outside).unwrap(); + let project_registry_dir = tmp.path().join("project-registry"); + std::fs::create_dir(&project_registry_dir).unwrap(); + let policy = project_policy(tmp.path()); + + let err = project_err(handle_resolve_or_register_project( + &policy, + &project_registry_dir, + &project_request( + "resolve_or_register_project", + serde_json::json!({ + "path": outside.to_string_lossy(), + "project_root_scopes": [granted.to_string_lossy()], + }), + ), + )); + assert_eq!(err, "project_path_outside_granted_roots"); + let records = std::fs::read_dir(&project_registry_dir) + .unwrap() + .filter(|entry| entry.as_ref().unwrap().path().extension() == Some("toml".as_ref())) + .count(); + assert_eq!( + records, 0, + "a rejected target must not create any registration" + ); +} + +#[test] +fn resolve_or_register_project_grant_gate_covers_existing_registrations() { + let tmp = tempfile::tempdir().unwrap(); + let granted = tmp.path().join("granted"); + std::fs::create_dir_all(granted.join("inner")).unwrap(); + let outside = tmp.path().join("outside"); + std::fs::create_dir(&outside).unwrap(); + let project_registry_dir = tmp.path().join("project-registry"); + std::fs::create_dir(&project_registry_dir).unwrap(); + let policy = project_policy(tmp.path()); + + // Pre-existing registration created without any grant: an outside target + // that is already registered must not be resolvable through a grant + // scoped to a different root either. + project_ok(handle_project_op( + &policy, + &project_registry_dir, + &project_request( + "register_project", + serde_json::json!({ + "id": "outside-project", + "name": "Outside", + "path": outside.to_string_lossy(), + "allow_patch": false + }), + ), + )); + + let err = project_err(handle_resolve_or_register_project( + &policy, + &project_registry_dir, + &project_request( + "resolve_or_register_project", + serde_json::json!({ + "path": outside.to_string_lossy(), + "project_root_scopes": [granted.to_string_lossy()], + }), + ), + )); + assert_eq!(err, "project_path_outside_granted_roots"); + let records = std::fs::read_dir(&project_registry_dir) + .unwrap() + .filter(|entry| entry.as_ref().unwrap().path().extension() == Some("toml".as_ref())) + .count(); + assert_eq!(records, 1, "no second registration may appear"); +} + +#[test] +fn resolve_or_register_project_accepts_target_inside_granted_roots_idempotently() { + let tmp = tempfile::tempdir().unwrap(); + let granted = tmp.path().join("granted"); + let inner = granted.join("inner"); + std::fs::create_dir_all(&inner).unwrap(); + let project_registry_dir = tmp.path().join("project-registry"); + std::fs::create_dir(&project_registry_dir).unwrap(); + let policy = project_policy(tmp.path()); + let payload = serde_json::json!({ + "path": inner.to_string_lossy(), + "project_root_scopes": [granted.to_string_lossy()], + }); + + let first = project_ok(handle_resolve_or_register_project( + &policy, + &project_registry_dir, + &project_request("resolve_or_register_project", payload.clone()), + )); + assert_eq!(first["outcome"], "auto_registered"); + assert_eq!(first["registered"], true); + let second = project_ok(handle_resolve_or_register_project( + &policy, + &project_registry_dir, + &project_request("resolve_or_register_project", payload), + )); + assert_eq!(second["outcome"], "reused_existing_registration"); + assert_eq!(second["registered"], false); + assert_eq!(second["agent_project_id"], first["agent_project_id"]); + let records = std::fs::read_dir(&project_registry_dir) + .unwrap() + .filter(|entry| entry.as_ref().unwrap().path().extension() == Some("toml".as_ref())) + .count(); + assert_eq!(records, 1, "repeated calls keep one stable identity"); +} + +#[test] +fn resolve_or_register_project_rejects_malformed_grant_scopes_fail_closed() { + let tmp = tempfile::tempdir().unwrap(); + let granted = tmp.path().join("granted"); + std::fs::create_dir_all(granted.join("inner")).unwrap(); + let project_registry_dir = tmp.path().join("project-registry"); + std::fs::create_dir(&project_registry_dir).unwrap(); + let policy = project_policy(tmp.path()); + let granted = granted.to_string_lossy().to_string(); + + for (label, payload) in [ + ( + "relative root entry", + serde_json::json!({"path": tmp.path().join("granted/inner").to_string_lossy(), "project_root_scopes": ["relative/root"]}), + ), + ( + "empty scope set", + serde_json::json!({"path": tmp.path().join("granted/inner").to_string_lossy(), "project_root_scopes": []}), + ), + ( + "non-array scopes", + serde_json::json!({"path": tmp.path().join("granted/inner").to_string_lossy(), "project_root_scopes": granted}), + ), + ( + "unknown second field", + serde_json::json!({"path": tmp.path().join("granted/inner").to_string_lossy(), "unrelated": true}), + ), + ] { + let err = project_err(handle_resolve_or_register_project( + &policy, + &project_registry_dir, + &project_request("resolve_or_register_project", payload), + )); + assert_eq!(err, "invalid_request", "{label}"); + let records = std::fs::read_dir(&project_registry_dir) + .unwrap() + .filter(|entry| entry.as_ref().unwrap().path().extension() == Some("toml".as_ref())) + .count(); + assert_eq!(records, 0, "{label}: no registration may be created"); + } +} + +#[test] +#[cfg(unix)] +fn resolve_or_register_project_rejects_target_flip_after_server_authorization() { + // Deterministic boundary test for the server-authorized-then-runner-commits + // window: the Server fence authorized this exact canonical target, then the + // directory component was replaced by a symlink before the Runner resolved + // it. The Runner-side grant check must refuse the commit: no registration, + // no reuse. + let tmp = tempfile::tempdir().unwrap(); + let granted = tmp.path().join("granted"); + let inner = granted.join("inner"); + std::fs::create_dir_all(&inner).unwrap(); + let outside = tmp.path().join("outside"); + std::fs::create_dir_all(outside.join("secret")).unwrap(); + let project_registry_dir = tmp.path().join("project-registry"); + std::fs::create_dir(&project_registry_dir).unwrap(); + let policy = project_policy(tmp.path()); + let canonical_target = inner.canonicalize().unwrap(); + + // The flip: the authorized target becomes a symlink that escapes the grant. + std::fs::remove_dir(&inner).unwrap(); + std::os::unix::fs::symlink(outside.join("secret"), &inner).expect("symlink"); + + let err = project_err(handle_resolve_or_register_project( + &policy, + &project_registry_dir, + &project_request( + "resolve_or_register_project", + serde_json::json!({ + "path": canonical_target.to_string_lossy(), + "project_root_scopes": [granted.canonicalize().unwrap().to_string_lossy()], + }), + ), + )); + assert_eq!(err, "project_path_outside_granted_roots"); + let records = std::fs::read_dir(&project_registry_dir) + .unwrap() + .filter(|entry| entry.as_ref().unwrap().path().extension() == Some("toml".as_ref())) + .count(); + assert_eq!( + records, 0, + "a flipped target must not commit a registration" + ); +} diff --git a/crates/webcodex-runner/src/webcodex_runner/projects/managed_worktree.rs b/crates/webcodex-runner/src/webcodex_runner/projects/managed_worktree.rs index 425f037c7..4ab23ac9b 100644 --- a/crates/webcodex-runner/src/webcodex_runner/projects/managed_worktree.rs +++ b/crates/webcodex-runner/src/webcodex_runner/projects/managed_worktree.rs @@ -543,7 +543,46 @@ pub(crate) fn handle_prepare_managed_worktree_operation( else { return managed_worktree_error(start, "invalid_request", false, None, None, None); }; - if !matches!(payload.len(), 4 | 6) { + // Optional project-share parent-root grant forwarded by the Server. It + // authorizes the *source* the worktree derives from, judged on the + // Runner's own canonical resolution before any side effect; the derived + // sibling destination is a Runner-owned namespace and is not required to + // live inside the grant. Ordinary credentials omit the field and keep the + // exact previous payload. Malformed scopes fail closed. + let granted_roots = match payload.get("project_root_scopes") { + None => None, + Some(serde_json::Value::Array(items)) => { + if items.is_empty() { + return managed_worktree_error(start, "invalid_request", false, None, None, None); + } + let mut roots = Vec::with_capacity(items.len()); + for item in items { + let Some(root) = item.as_str().filter(|root| { + !root.is_empty() && !root.contains('\0') && Path::new(root).is_absolute() + }) else { + return managed_worktree_error( + start, + "invalid_request", + false, + None, + None, + None, + ); + }; + roots.push(PathBuf::from(root)); + } + Some(roots) + } + Some(_) => { + return managed_worktree_error(start, "invalid_request", false, None, None, None) + } + }; + let expected_field_counts = if granted_roots.is_some() { + [5usize, 7] + } else { + [4usize, 6] + }; + if !expected_field_counts.contains(&payload.len()) { return managed_worktree_error(start, "invalid_request", false, None, None, None); } let Some(path) = payload @@ -617,16 +656,6 @@ pub(crate) fn handle_prepare_managed_worktree_operation( ) } }; - if resume_project_id.is_some() && expected_source_identity.is_some() { - return managed_worktree_error( - start, - "invalid_request", - false, - Some(&base_ref), - None, - None, - ); - } if validate_windows_project_root(Path::new(path)).is_err() { return managed_worktree_error( start, @@ -667,6 +696,23 @@ pub(crate) fn handle_prepare_managed_worktree_operation( None, ); } + if let Some(granted_roots) = &granted_roots { + // Share-grant admission is judged on the Runner's own canonical + // resolution of the source before any git invocation or registry + // write: a source that resolved inside the grant at Server-fence time + // but elsewhere here fails closed. The derived worktree destination is + // not part of this check; allowed_roots above still governs it. + if !granted_roots.iter().any(|root| source.starts_with(root)) { + return managed_worktree_error( + start, + "project_path_outside_granted_roots", + false, + Some(&base_ref), + None, + None, + ); + } + } let source_root = match managed_worktree_git_text(&source, &["rev-parse", "--show-toplevel"]) .ok() .and_then(|root| canonicalize_existing(Path::new(&root)).ok()) @@ -696,18 +742,6 @@ pub(crate) fn handle_prepare_managed_worktree_operation( ) } }; - if let Some(resume_project_id) = resume_project_id { - return resume_managed_worktree( - start, - policy, - project_registry_dir, - client_id, - &source_root, - source_dirty, - requested_base_ref.as_deref(), - resume_project_id, - ); - } let source_projects = match load_project_files_for_path_resolution(project_registry_dir) { Ok(projects) => projects, Err(error) => { @@ -735,6 +769,8 @@ pub(crate) fn handle_prepare_managed_worktree_operation( ) } }; + // Validated before the resume short-circuit so a resume is anchored to the + // registered source identity too, not only to the stored source path. if expected_source_identity.is_some_and(|(expected_project_id, expected_root_fingerprint)| { expected_project_id != source_project_id || expected_root_fingerprint != source_root_fingerprint @@ -751,6 +787,18 @@ pub(crate) fn handle_prepare_managed_worktree_operation( }), ); } + if let Some(resume_project_id) = resume_project_id { + return resume_managed_worktree( + start, + policy, + project_registry_dir, + client_id, + &source_root, + source_dirty, + requested_base_ref.as_deref(), + resume_project_id, + ); + } let base_sha = match exact_git_commit(&source_root, &base_ref) { Ok(sha) => sha, Err(error) => { diff --git a/crates/webcodex-runner/src/webcodex_runner/projects/registration.rs b/crates/webcodex-runner/src/webcodex_runner/projects/registration.rs index 6eacffd03..2e382a5bb 100644 --- a/crates/webcodex-runner/src/webcodex_runner/projects/registration.rs +++ b/crates/webcodex-runner/src/webcodex_runner/projects/registration.rs @@ -606,7 +606,7 @@ pub(crate) fn handle_resolve_or_register_project_operation( ) } }; - if payload.len() != 1 { + if payload.is_empty() || payload.len() > 2 { return structured_project_error_cmd( start, "invalid_request", @@ -627,6 +627,59 @@ pub(crate) fn handle_resolve_or_register_project_operation( ) } }; + // Optional project-share parent-root grant forwarded by the Server. The + // Server owns the grant; the Runner re-derives the admission decision + // against its own filesystem view of the committed target before its + // first side effect (the registry write below), so a path that resolves + // differently here than it did at the Server fence fails closed. + // Ordinary credentials omit the field and keep the exact previous + // behavior. Empty or non-absolute entries fail closed. + let granted_roots = match payload.get("project_root_scopes") { + None => { + if payload.len() != 1 { + return structured_project_error_cmd( + start, + "invalid_request", + false, + serde_json::json!({}), + ); + } + None + } + Some(serde_json::Value::Array(items)) => { + if items.is_empty() { + return structured_project_error_cmd( + start, + "invalid_request", + false, + serde_json::json!({}), + ); + } + let mut roots = Vec::with_capacity(items.len()); + for item in items { + let Some(root) = item.as_str().filter(|root| { + !root.is_empty() && !root.contains('\0') && Path::new(root).is_absolute() + }) else { + return structured_project_error_cmd( + start, + "invalid_request", + false, + serde_json::json!({"field": "project_root_scopes"}), + ); + }; + roots.push(PathBuf::from(root)); + } + Some(roots) + } + Some(_) => { + return structured_project_error_cmd( + start, + "invalid_request", + false, + serde_json::json!({}), + ) + } + }; // Reject unsupported namespaces before filesystem access. Raw UNC/VerbatimUNC // inputs must also be covered by pre-existing Runner authority before they may // trigger SMB I/O; local CLI/Desktop onboarding owns any authority extension. @@ -693,6 +746,26 @@ pub(crate) fn handle_resolve_or_register_project_operation( serde_json::json!({"field": "path"}), ); } + if let Some(granted_roots) = &granted_roots { + // Share-grant admission is judged on the Runner's own canonical + // resolution of the committed target, component-wise against the + // Server-granted canonical roots. This is the last gate before any + // resolution outcome below: neither an existing registration nor a + // fresh registration may be resolved outside the grant, and the + // Runner filesystem allowed_roots above is a coarser boundary that + // never substitutes for it. + if !granted_roots + .iter() + .any(|root| canonical_path.starts_with(root)) + { + return structured_project_error_cmd( + start, + "project_path_outside_granted_roots", + false, + serde_json::json!({"field": "path"}), + ); + } + } let projects = match load_project_files_for_path_resolution(project_registry_dir) { Ok(projects) => projects, From 424207278a15c693d56068ddde7064d03c4a6a73 Mon Sep 17 00:00:00 2001 From: vincent Date: Wed, 28 Oct 2026 08:05:47 +1000 Subject: [PATCH 3/9] feat(share): accept and persist explicit project roots Add repeatable --project-roots and --no-project-roots to share --auth oauth, pass the grant to the spawned Server, and persist it next to the protected share OAuth client state. An explicit list replaces the stored grant, re-sending the same list is a no-op, omitting the flag restores the last grant, and --no-project-roots removes it so removed roots cannot revive on a later restart. The grant file is replaced atomically in the same protected directory with owner-only permissions; failures never destroy the previous value or write through a planted symlink. --project-roots requires OAuth and an existing absolute directory. --- src/project_entry.rs | 20 +- src/project_entry_setup.rs | 97 ++++++ src/project_entry_share.rs | 560 ++++++++++++++++++++++++++++++++++- src/project_entry_windows.rs | 28 ++ 4 files changed, 703 insertions(+), 2 deletions(-) diff --git a/src/project_entry.rs b/src/project_entry.rs index fddc632ef..5dae1ce7f 100644 --- a/src/project_entry.rs +++ b/src/project_entry.rs @@ -674,6 +674,9 @@ pub(crate) fn render_error(error: &ProductError, json: bool) -> String { pub(super) struct ProjectShareOAuthRuntimeOptions { pub(super) project_grant_id: String, pub(super) session_id: String, + /// Canonical parent-root grant handed to the spawned Server through + /// `WEBCODEX_OAUTH2_PROJECT_SHARE_ROOTS`. Empty keeps the exact-path fence. + pub(super) project_roots: Vec, } #[derive(Debug, Clone)] @@ -859,12 +862,27 @@ pub(super) async fn start_local_runtime( oauth.project_grant_id, ) .env("WEBCODEX_OAUTH2_PROJECT_SHARE_SESSION_ID", oauth.session_id); + if oauth.project_roots.is_empty() { + server_command.env_remove("WEBCODEX_OAUTH2_PROJECT_SHARE_ROOTS"); + } else { + // One canonical path per share; the OS path-list separator keeps + // multi-root grants unambiguous on every supported platform. + let roots = std::env::join_paths(&oauth.project_roots).map_err(|error| { + ProductError::new( + "project_registration_invalid", + format!("could not encode project share roots: {error}"), + Some("Use parent directories without the platform path separator."), + ) + })?; + server_command.env("WEBCODEX_OAUTH2_PROJECT_SHARE_ROOTS", roots); + } } else { server_command .env("WEBCODEX_OAUTH2_ENABLED", "false") .env_remove("WEBCODEX_OAUTH2_ISSUER") .env_remove("WEBCODEX_OAUTH2_PROJECT_SHARE_GRANT_ID") - .env_remove("WEBCODEX_OAUTH2_PROJECT_SHARE_SESSION_ID"); + .env_remove("WEBCODEX_OAUTH2_PROJECT_SHARE_SESSION_ID") + .env_remove("WEBCODEX_OAUTH2_PROJECT_SHARE_ROOTS"); } configure_console_assets_environment(&mut server_command, console_assets_dir.as_deref()); let mut server = server_command.spawn().map_err(|_| { diff --git a/src/project_entry_setup.rs b/src/project_entry_setup.rs index 14fb78d8b..3c83411e1 100644 --- a/src/project_entry_setup.rs +++ b/src/project_entry_setup.rs @@ -1188,3 +1188,100 @@ pub(super) fn write_new_private(path: &Path, content: &[u8]) -> Result<(), Produ }) } } + +/// Atomically create or replace an updatable private state file. +/// +/// Unlike [`write_new_private`], which deliberately refuses to touch an +/// existing file, this is for state a later share launch must be able to +/// update or clear in place (currently the project-share parent-root grant). +/// The replacement file is created fresh in the SAME protected directory as +/// the target, fully written and synced, and then renamed over the target, so +/// a failure never leaves a half-written configuration and never destroys the +/// previous value. Renaming replaces the target name itself: a symlink planted +/// at the target path is replaced rather than written through, and the +/// replacement file never inherits a broad pre-existing permission. +pub(super) fn replace_private_file(path: &Path, content: &[u8]) -> Result<(), ProductError> { + let Some(parent) = path.parent() else { + return Err(ProductError::new( + "project_registration_invalid", + "WebCodex private state path has no parent directory", + Some("Check the project state location, then retry."), + )); + }; + create_private_dir(parent)?; + let file_name = path + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or("state"); + let temporary = parent.join(format!("{file_name}.replace-{}", Uuid::new_v4().simple())); + let write_result = (|| -> Result<(), ProductError> { + #[cfg(windows)] + { + super::windows_private_state::write_replace_private_segment(&temporary, content) + .map_err(|message| { + ProductError::new( + "project_registration_invalid", + message, + Some("Check local filesystem permissions and reparse points, then retry."), + ) + }) + } + #[cfg(not(windows))] + { + let mut options = OpenOptions::new(); + options.write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + let mut file = options.open(&temporary).map_err(|_| { + ProductError::new( + "project_registration_invalid", + "WebCodex could not create the private state replacement file", + Some("Check local filesystem permissions, then retry."), + ) + })?; + file.write_all(content).map_err(|_| { + ProductError::new( + "project_registration_invalid", + "WebCodex could not write the private state replacement file", + Some("Check local filesystem permissions, then retry."), + ) + })?; + file.sync_all().map_err(|_| { + ProductError::new( + "project_registration_invalid", + "WebCodex could not sync the private state replacement file", + Some("Check local filesystem permissions, then retry."), + ) + }) + } + })(); + if let Err(error) = write_result { + let _ = std::fs::remove_file(&temporary); + return Err(error); + } + std::fs::rename(&temporary, path).map_err(|error| { + let _ = std::fs::remove_file(&temporary); + ProductError::new( + "project_registration_invalid", + format!("WebCodex could not replace the private state file: {error}"), + Some("Check local filesystem permissions, then retry."), + ) + }) +} + +/// Remove an updatable private state file. A missing file is already the +/// cleared state, so removal is idempotent by construction. +pub(super) fn remove_private_state_file(path: &Path) -> Result<(), ProductError> { + match std::fs::remove_file(path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(ProductError::new( + "project_registration_invalid", + format!("WebCodex could not remove the private state file: {error}"), + Some("Check local filesystem permissions, then retry."), + )), + } +} diff --git a/src/project_entry_share.rs b/src/project_entry_share.rs index 6de7a10a5..87da9d422 100644 --- a/src/project_entry_share.rs +++ b/src/project_entry_share.rs @@ -4,7 +4,8 @@ use super::client_handoff_service::{ }; use super::setup_service::{ create_private_dir, generate_project_credential, read_private_value, read_project_credential, - write_new_private, ProjectConfig, ProjectPaths, + remove_private_state_file, replace_private_file, write_new_private, ProjectConfig, + ProjectPaths, }; use super::{ configured_project, ensure_local_runtime_port_available, parse_options, @@ -52,6 +53,14 @@ pub(crate) struct ShareCommandOptions { pub(crate) public_url: Option, pub(crate) copy_url: bool, pub(crate) stop_on_stdin_eof: bool, + /// Explicit parent-root grant for this OAuth share's project-share + /// subjects. Canonicalized and validated at parse time; persisted next to + /// the share OAuth client state so restarts keep the same grant. + pub(crate) oauth_project_roots: Vec, + /// Whether the invocation expressed a parent-root choice at all + /// (`--project-roots` or `--no-project-roots`). Explicit invocations + /// replace the persisted grant; plain restarts restore it. + pub(crate) oauth_project_roots_explicit: bool, } pub(crate) fn parse_share_options(args: &[String]) -> Result { @@ -61,6 +70,8 @@ pub(crate) fn parse_share_options(args: &[String]) -> Result = Vec::new(); + let mut oauth_project_roots_explicit = false; let mut project_args = Vec::new(); let mut index = 0; while index < args.len() { @@ -123,6 +134,27 @@ pub(crate) fn parse_share_options(args: &[String]) -> Result copy_url = false, "--stop-on-stdin-eof" => stop_on_stdin_eof = true, + "--project-roots" => { + index += 1; + let value = args + .get(index) + .ok_or_else(|| "--project-roots requires a directory".to_string())?; + let root = validate_share_project_root(value)?; + if oauth_project_roots.len() >= crate::config::MAX_PROJECT_SHARE_ROOTS { + return Err(format!( + "at most {} --project-roots directories are supported", + crate::config::MAX_PROJECT_SHARE_ROOTS + )); + } + if !oauth_project_roots.contains(&root) { + oauth_project_roots.push(root); + } + oauth_project_roots_explicit = true; + } + "--no-project-roots" => { + oauth_project_roots.clear(); + oauth_project_roots_explicit = true; + } flag => project_args.push(flag.to_string()), } index += 1; @@ -152,6 +184,12 @@ pub(crate) fn parse_share_options(args: &[String]) -> Result Result Result { + let trimmed = value.trim(); + let path = Path::new(trimmed); + if trimmed.is_empty() || !path.is_absolute() { + return Err(format!( + "--project-roots requires an absolute directory path, got '{trimmed}'" + )); + } + let canonical = path + .canonicalize() + .map_err(|error| format!("--project-roots directory '{trimmed}' is not usable: {error}"))?; + if !canonical.is_dir() { + return Err(format!("--project-roots '{trimmed}' is not a directory")); + } + Ok(canonical) +} + +const SHARE_PROJECT_ROOTS_FILE: &str = "project-roots"; + +/// Read the persisted parent-root grant from the share OAuth state directory. +/// One canonical absolute path per line; unreadable or malformed entries are +/// skipped, and an absent file means no grant (the exact-path fence stays). +fn read_persisted_share_project_roots(directory: &Path) -> Vec { + let Ok(content) = std::fs::read_to_string(directory.join(SHARE_PROJECT_ROOTS_FILE)) else { + return Vec::new(); + }; + content + .lines() + .map(str::trim) + .filter(|line| !line.is_empty()) + .filter_map(|line| validate_share_project_root(line).ok()) + .take(crate::config::MAX_PROJECT_SHARE_ROOTS) + .collect() +} + +/// Whether the stored roots file may serve as the no-op fast path: a regular +/// file (never through a symlink) at owner-only permissions holding exactly +/// the requested bytes. Anything else — a symlink with equal content, a +/// wide-permission file, or a different value — falls through to the safe +/// same-directory replacement, which repairs the target type and permissions +/// instead of returning success while leaving a non-conforming target. +fn stored_roots_is_owner_only_file_with(path: &Path, bytes: &[u8]) -> bool { + let Ok(metadata) = std::fs::symlink_metadata(path) else { + return false; + }; + if metadata.file_type().is_symlink() || !metadata.is_file() { + return false; + } + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + if metadata.permissions().mode() & 0o777 != 0o600 { + return false; + } + } + std::fs::read(path).ok().as_deref() == Some(bytes) +} + +/// Persist the parent-root grant next to the share OAuth client state. +/// +/// Explicit invocations must be idempotent and updatable: the same list may be +/// re-sent by an unchanging systemd unit on every restart, a changed list +/// replaces the previous grant, and an empty list clears it. An identical +/// stored grant is left untouched; a different one is replaced through an +/// atomic same-directory rename that never destroys the previous value on +/// failure. Failure is fatal for an explicit grant: a share that silently +/// drops its roots would report success while denying the intended +/// directories. +fn persist_share_project_roots(directory: &Path, roots: &[PathBuf]) -> Result<(), ProductError> { + create_private_dir(directory)?; + let path = directory.join(SHARE_PROJECT_ROOTS_FILE); + if roots.is_empty() { + return remove_private_state_file(&path); + } + let mut content = String::new(); + for root in roots { + content.push_str(&root.to_string_lossy()); + content.push('\n'); + } + let bytes = content.as_bytes(); + if stored_roots_is_owner_only_file_with(&path, bytes) { + return Ok(()); + } + replace_private_file(&path, bytes) +} + fn mcp_query_token_url(public_url: &str, credential: &str) -> String { let mut query = url::form_urlencoded::Serializer::new(String::new()); query.append_pair("token", credential); @@ -298,6 +428,9 @@ struct ShareOAuthClient { client_id: String, client_secret: String, redirect_uri: String, + /// Protected per-redirect share OAuth state directory; also holds the + /// persisted parent-root grant. + state_directory: PathBuf, } fn valid_generated_oauth_value(value: &str, prefix: &str) -> bool { @@ -418,6 +551,7 @@ fn prepare_share_oauth_client( client_id, client_secret, redirect_uri, + state_directory: directory, }); } @@ -456,6 +590,7 @@ fn prepare_share_oauth_client( client_id, client_secret, redirect_uri, + state_directory: directory, }) } @@ -632,11 +767,23 @@ pub(crate) async fn share(options: &ShareCommandOptions) -> Result<(), ProductEr .ok_or_else(|| share_oauth_state_error("OAuth redirect URI is missing"))?, )?), }; + // Parent-root grant: an explicit flag replaces the persisted list; without + // the flag the persisted grant is restored so restarts keep the same + // authorization without retyping it. + let oauth_project_roots = match oauth_client.as_ref() { + Some(client) if options.oauth_project_roots_explicit => { + persist_share_project_roots(&client.state_directory, &options.oauth_project_roots)?; + options.oauth_project_roots.clone() + } + Some(client) => read_persisted_share_project_roots(&client.state_directory), + None => Vec::new(), + }; let project_share_oauth = oauth_client .as_ref() .map(|_| ProjectShareOAuthRuntimeOptions { project_grant_id: config.project_grant_id(&paths), session_id: crate::auth::generate_project_share_session_id(), + project_roots: oauth_project_roots.clone(), }); // All process/network readiness for this share uses one absolute budget. @@ -1569,6 +1716,7 @@ mod tests { client_id: "wc_client_test".to_string(), client_secret: "wc_csec_test".to_string(), redirect_uri: "https://client.example/callback".to_string(), + state_directory: PathBuf::new(), }; let output = render_share_oauth_ready( "demo", @@ -1906,4 +2054,414 @@ mod tests { let deadline = Instant::now() + Duration::from_secs(5); assert_pid_gone(descendant_pid, deadline).await; } + + fn share_root_fixture() -> tempfile::TempDir { + tempfile::tempdir().unwrap() + } + + fn share_args(extra: &[&str]) -> Vec { + let mut args: Vec = vec![ + "--auth".into(), + "oauth".into(), + "--oauth-redirect-uri".into(), + "https://client.example/callback".into(), + ]; + args.extend(extra.iter().map(|s| s.to_string())); + args + } + + #[test] + fn parse_share_project_roots_canonicalize_and_dedupe() { + let temp = share_root_fixture(); + let root = temp.path().join("Projects"); + std::fs::create_dir_all(&root).unwrap(); + let canonical = root.canonicalize().unwrap(); + + let parsed = parse_share_options(&share_args(&[ + "--project-roots", + &root.to_string_lossy(), + "--project-roots", + &canonical.to_string_lossy(), + ])) + .unwrap(); + assert_eq!(parsed.oauth_project_roots, vec![canonical.clone()]); + assert!(parsed.oauth_project_roots_explicit); + } + + #[test] + fn parse_share_project_roots_validate_inputs() { + let temp = share_root_fixture(); + let root = temp.path().join("Projects"); + std::fs::create_dir_all(&root).unwrap(); + + // Relative path rejected. + assert!(parse_share_options(&share_args(&["--project-roots", "relative/dir"])).is_err()); + // Missing directory rejected. + let missing = temp.path().join("missing").to_string_lossy().into_owned(); + assert!(parse_share_options(&share_args(&["--project-roots", &missing])).is_err()); + // A file is not a directory root. + let file = temp.path().join("file"); + std::fs::write(&file, b"x").unwrap(); + let file = file.to_string_lossy().into_owned(); + assert!(parse_share_options(&share_args(&["--project-roots", &file])).is_err()); + // Missing value rejected. + assert!(parse_share_options(&share_args(&["--project-roots"])).is_err()); + // Non-OAuth auth rejects the option. + let bearer_args: Vec = vec![ + "--auth".into(), + "bearer".into(), + "--project-roots".into(), + root.to_string_lossy().into_owned(), + ]; + assert!(parse_share_options(&bearer_args).is_err()); + // --no-project-roots is explicit but empty. + let cleared = parse_share_options(&share_args(&["--no-project-roots"])).unwrap(); + assert!(cleared.oauth_project_roots.is_empty()); + assert!(cleared.oauth_project_roots_explicit); + } + + #[test] + fn share_project_roots_explicit_updates_are_idempotent_and_replaceable() { + let temp = share_root_fixture(); + let root_a = temp.path().join("a"); + let root_b = temp.path().join("b"); + std::fs::create_dir_all(&root_a).unwrap(); + std::fs::create_dir_all(&root_b).unwrap(); + let roots_a = vec![root_a.canonicalize().unwrap()]; + let roots_b = vec![root_b.canonicalize().unwrap()]; + let directory = temp.path().join("share-oauth"); + + // First explicit grant persists. + persist_share_project_roots(&directory, &roots_a).unwrap(); + let path = directory.join(SHARE_PROJECT_ROOTS_FILE); + let first = std::fs::read(&path).unwrap(); + + // Same explicit grant again: idempotent success, identical content. + persist_share_project_roots(&directory, &roots_a).unwrap(); + assert_eq!( + std::fs::read(&path).unwrap(), + first, + "same-value update churned the file" + ); + + // A different explicit list replaces the previous one. + persist_share_project_roots(&directory, &roots_b).unwrap(); + assert_eq!( + read_persisted_share_project_roots(&directory), + roots_b, + "replacement grant not stored" + ); + + // Replacing an existing file never leaves temp segments behind. + let leftovers: Vec<_> = std::fs::read_dir(&directory) + .unwrap() + .filter_map(Result::ok) + .map(|entry| entry.file_name().to_string_lossy().into_owned()) + .filter(|name| name.contains(".replace-")) + .collect(); + assert!( + leftovers.is_empty(), + "temp segments left behind: {leftovers:?}" + ); + } + + #[test] + fn share_project_roots_clear_is_idempotent_and_goes_fully_away() { + let temp = share_root_fixture(); + let root = temp.path().join("a"); + std::fs::create_dir_all(&root).unwrap(); + let directory = temp.path().join("share-oauth"); + let path = directory.join(SHARE_PROJECT_ROOTS_FILE); + + // Clearing an absent grant is already the cleared state. + persist_share_project_roots(&directory, &[]).unwrap(); + assert!(!path.exists()); + + persist_share_project_roots(&directory, &[root.canonicalize().unwrap()]).unwrap(); + assert!(path.exists()); + persist_share_project_roots(&directory, &[]).unwrap(); + assert!(!path.exists(), "cleared grant left the roots file behind"); + // Repeated clear stays successful. + persist_share_project_roots(&directory, &[]).unwrap(); + // And a later no-flag launch has nothing to restore. + assert!(read_persisted_share_project_roots(&directory).is_empty()); + } + + #[test] + fn share_project_roots_failed_update_leaves_previous_value_intact() { + let temp = share_root_fixture(); + let root_a = temp.path().join("a"); + let root_b = temp.path().join("b"); + std::fs::create_dir_all(&root_a).unwrap(); + std::fs::create_dir_all(&root_b).unwrap(); + let directory = temp.path().join("share-oauth"); + let roots_a = vec![root_a.canonicalize().unwrap()]; + let roots_b = vec![root_b.canonicalize().unwrap()]; + persist_share_project_roots(&directory, &roots_a).unwrap(); + let path = directory.join(SHARE_PROJECT_ROOTS_FILE); + let previous = std::fs::read(&path).unwrap(); + + // Failure injection without a framework: occupy the target path with a + // directory holding a sentinel, so the final same-directory rename + // cannot replace it. (Making the parent read-only is not injectable: + // create_private_dir deliberately re-asserts owner-only permissions, + // which the self-healing test below pins instead.) + std::fs::remove_file(&path).unwrap(); + std::fs::create_dir(&path).unwrap(); + std::fs::write(path.join("sentinel"), b"occupied").unwrap(); + + let update = persist_share_project_roots(&directory, &roots_b); + assert!( + update.is_err(), + "update over a directory unexpectedly succeeded" + ); + assert!( + std::fs::read(path.join("sentinel")).unwrap() == b"occupied", + "failed update disturbed the occupied target" + ); + + // Clearing against the same occupied path must also fail explicitly + // instead of pretending the grant is gone. + let clear = persist_share_project_roots(&directory, &[]); + assert!( + clear.is_err(), + "clear over a directory unexpectedly succeeded" + ); + assert!(path.is_dir(), "failed clear removed the occupied target"); + + // Restore the regular file and prove the previous grant survives the + // whole failed sequence. + std::fs::remove_dir_all(&path).unwrap(); + std::fs::write(&path, &previous).unwrap(); + assert_eq!( + read_persisted_share_project_roots(&directory), + roots_a, + "the previous grant was not preserved across the failed operations" + ); + let leftovers: Vec<_> = std::fs::read_dir(&directory) + .unwrap() + .filter_map(Result::ok) + .map(|entry| entry.file_name().to_string_lossy().into_owned()) + .filter(|name| name.contains(".replace-")) + .collect(); + assert!( + leftovers.is_empty(), + "failed update left temp segments: {leftovers:?}" + ); + } + + #[cfg(unix)] + #[test] + fn share_project_roots_state_directory_self_heals_owner_only_permissions() { + use std::os::unix::fs::PermissionsExt; + let temp = share_root_fixture(); + let root = temp.path().join("a"); + std::fs::create_dir_all(&root).unwrap(); + let directory = temp.path().join("share-oauth"); + persist_share_project_roots(&directory, &[root.canonicalize().unwrap()]).unwrap(); + + // A loosened directory must not stay loosened: the private-state + // helpers re-assert owner-only protection on every write. + let mut permissions = std::fs::metadata(&directory).unwrap().permissions(); + permissions.set_mode(0o755); + std::fs::set_permissions(&directory, permissions).unwrap(); + persist_share_project_roots(&directory, &[root.canonicalize().unwrap()]).unwrap(); + let dir_mode = std::fs::metadata(&directory).unwrap().permissions().mode() & 0o777; + assert_eq!( + dir_mode, 0o700, + "loosened state directory was not re-protected: {dir_mode:o}" + ); + } + + #[test] + fn share_project_roots_replacement_replaces_a_planted_symlink_not_its_target() { + let temp = share_root_fixture(); + let root_a = temp.path().join("a"); + let root_b = temp.path().join("b"); + std::fs::create_dir_all(&root_a).unwrap(); + std::fs::create_dir_all(&root_b).unwrap(); + let directory = temp.path().join("share-oauth"); + std::fs::create_dir_all(&directory).unwrap(); + let path = directory.join(SHARE_PROJECT_ROOTS_FILE); + + // Plant a symlink at the exact target path pointing outside the state + // directory; the replacement must overwrite the link, never write + // through it. + let decoy = temp.path().join("decoy-roots"); + std::fs::write(&decoy, b"planted\n").unwrap(); + #[cfg(unix)] + std::os::unix::fs::symlink(&decoy, &path).unwrap(); + #[cfg(not(unix))] + std::fs::write(&path, b"planted\n").unwrap(); + + let roots_b = vec![root_b.canonicalize().unwrap()]; + persist_share_project_roots(&directory, &roots_b).unwrap(); + + assert_eq!( + read_persisted_share_project_roots(&directory), + roots_b, + "replacement did not take effect over the planted link" + ); + #[cfg(unix)] + { + let metadata = std::fs::symlink_metadata(&path).unwrap(); + assert!( + !metadata.file_type().is_symlink(), + "target is still a symlink" + ); + assert_eq!( + std::fs::read(&decoy).unwrap(), + b"planted\n", + "decoy was written through" + ); + } + } + + #[cfg(unix)] + #[test] + fn share_project_roots_same_value_symlink_is_replaced_not_written_through() { + use std::os::unix::fs::PermissionsExt; + let temp = share_root_fixture(); + let root_a = temp.path().join("a"); + std::fs::create_dir_all(&root_a).unwrap(); + let directory = temp.path().join("share-oauth"); + std::fs::create_dir_all(&directory).unwrap(); + let path = directory.join(SHARE_PROJECT_ROOTS_FILE); + let roots = vec![root_a.canonicalize().unwrap()]; + let mut content = String::new(); + for root in &roots { + content.push_str(&root.to_string_lossy()); + content.push('\n'); + } + + // Plant a symlink at the target pointing OUTSIDE the state directory, + // holding exactly the bytes that would be stored. + let outside = temp.path().join("outside-roots"); + std::fs::write(&outside, &content).unwrap(); + std::os::unix::fs::symlink(&outside, &path).unwrap(); + + // Re-sending the identical grant must repair the target, not accept + // the link (and never write through it). + persist_share_project_roots(&directory, &roots).unwrap(); + + let metadata = std::fs::symlink_metadata(&path).unwrap(); + assert!( + !metadata.file_type().is_symlink(), + "symlink survived a same-value update" + ); + assert!( + metadata.is_file(), + "target is not a regular file after repair" + ); + let mode = metadata.permissions().mode() & 0o777; + assert_eq!( + mode, 0o600, + "repaired target is not owner-private: {mode:o}" + ); + assert_eq!(std::fs::read(&path).unwrap(), content.as_bytes()); + // The external target was never written through: it still holds the + // planted bytes and remains a plain file outside the state directory. + assert_eq!(std::fs::read(&outside).unwrap(), content.as_bytes()); + assert!(std::fs::symlink_metadata(&outside).unwrap().is_file()); + } + + #[cfg(unix)] + #[test] + fn share_project_roots_same_value_wide_permissions_are_repaired() { + use std::os::unix::fs::PermissionsExt; + let temp = share_root_fixture(); + let root_a = temp.path().join("a"); + std::fs::create_dir_all(&root_a).unwrap(); + let directory = temp.path().join("share-oauth"); + std::fs::create_dir_all(&directory).unwrap(); + let path = directory.join(SHARE_PROJECT_ROOTS_FILE); + let roots = vec![root_a.canonicalize().unwrap()]; + let mut content = String::new(); + for root in &roots { + content.push_str(&root.to_string_lossy()); + content.push('\n'); + } + + // A same-content file with group/other read bits must not qualify for + // the no-op fast path: the update repairs it to owner-only. + std::fs::write(&path, &content).unwrap(); + let mut wide = std::fs::metadata(&path).unwrap().permissions(); + wide.set_mode(0o644); + std::fs::set_permissions(&path, wide).unwrap(); + + persist_share_project_roots(&directory, &roots).unwrap(); + + let metadata = std::fs::symlink_metadata(&path).unwrap(); + assert!(metadata.is_file()); + let mode = metadata.permissions().mode() & 0o777; + assert_eq!( + mode, 0o600, + "wide permissions survived a same-value update: {mode:o}" + ); + assert_eq!(std::fs::read(&path).unwrap(), content.as_bytes()); + // A second same-value update now takes the no-op path and stays + // owner-only (idempotence preserved after the repair). + persist_share_project_roots(&directory, &roots).unwrap(); + let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600); + } + + #[cfg(unix)] + #[test] + fn share_project_roots_replacement_keeps_private_permissions() { + use std::os::unix::fs::PermissionsExt; + let temp = share_root_fixture(); + let root = temp.path().join("a"); + std::fs::create_dir_all(&root).unwrap(); + let directory = temp.path().join("share-oauth"); + persist_share_project_roots(&directory, &[root.canonicalize().unwrap()]).unwrap(); + let path = directory.join(SHARE_PROJECT_ROOTS_FILE); + let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600, "roots file is not owner-private: {mode:o}"); + let dir_mode = std::fs::metadata(&directory).unwrap().permissions().mode() & 0o777; + assert_eq!( + dir_mode, 0o700, + "state directory is not owner-private: {dir_mode:o}" + ); + } + + #[test] + fn share_project_roots_persist_and_restore_round_trip() { + let temp = share_root_fixture(); + let root_a = temp.path().join("a"); + let root_b = temp.path().join("b"); + std::fs::create_dir_all(&root_a).unwrap(); + std::fs::create_dir_all(&root_b).unwrap(); + let roots = vec![ + root_a.canonicalize().unwrap(), + root_b.canonicalize().unwrap(), + ]; + + let directory = temp.path().join("share-oauth"); + persist_share_project_roots(&directory, &roots).unwrap(); + assert_eq!( + read_persisted_share_project_roots(&directory), + roots, + "persisted roots must restore verbatim" + ); + + // An absent state directory means no grant. + assert!(read_persisted_share_project_roots(&temp.path().join("absent")).is_empty()); + + // A malformed line is skipped instead of poisoning the whole grant. + let mut broken = root_a + .canonicalize() + .unwrap() + .to_string_lossy() + .into_owned(); + broken.push('\n'); + broken.push_str("not-a-directory\n../escape\n"); + std::fs::create_dir_all(&directory).unwrap(); + std::fs::write(directory.join(SHARE_PROJECT_ROOTS_FILE), broken).unwrap(); + assert_eq!( + read_persisted_share_project_roots(&directory), + vec![root_a.canonicalize().unwrap()], + "only still-valid roots survive restore" + ); + } } diff --git a/src/project_entry_windows.rs b/src/project_entry_windows.rs index 685307e0a..7a51f1875 100644 --- a/src/project_entry_windows.rs +++ b/src/project_entry_windows.rs @@ -89,6 +89,34 @@ pub(super) fn write_new_private_file(path: &Path, content: &[u8]) -> Result<(), result } +/// Windows half of [`super::setup_service::replace_private_file`]: create the +/// replacement segment under the target's name plus a fresh suffix, protect it +/// with the same current-user DACL machinery, sync it, and rename it over the +/// target. The rename replaces the target name itself (including a planted +/// reparse point) instead of writing through it. +pub(super) fn write_replace_private_segment( + temporary: &Path, + content: &[u8], +) -> Result<(), String> { + let mut file = OpenOptions::new() + .write(true) + .create_new(true) + .access_mode(GENERIC_WRITE | READ_CONTROL | WRITE_DAC) + .custom_flags(FILE_FLAG_OPEN_REPARSE_POINT) + .open(temporary) + .map_err(|_| "could not create the private state replacement file".to_string())?; + let result = (|| { + reject_reparse(&file, "private Windows replacement file")?; + protect_handle(file.as_raw_handle() as _, false)?; + file.write_all(content) + .map_err(|_| "could not write private Windows state".to_string())?; + file.sync_all() + .map_err(|_| "could not sync private Windows state".to_string()) + })(); + drop(file); + result +} + fn reject_reparse(file: &File, label: &str) -> Result<(), String> { let mut info: BY_HANDLE_FILE_INFORMATION = unsafe { std::mem::zeroed() }; if unsafe { GetFileInformationByHandle(file.as_raw_handle() as _, &mut info) } == 0 { From 54de03bebd9c22cbebbe26147f85f341ea297298 Mon Sep 17 00:00:00 2001 From: vincent Date: Wed, 28 Oct 2026 08:05:47 +1000 Subject: [PATCH 4/9] feat(projects): admit granted parent-root targets through the exact-path fence For project-scoped credentials, an exact registered path keeps its previous behavior. When the share carries parent-root grants, an existing canonical directory inside one of them may now resolve or register through the ordinary idempotent project-op path instead of being denied before the Runner sees it. The Server forwards the canonical target plus the granted roots, and re-verifies the Runner's returned path against the grant, reporting state_changed from the Runner's registered flag. Missing or malformed effect metadata stays operation_indeterminate rather than claiming no change. Symlink escapes, textual-prefix siblings, relative paths and missing directories remain denied and are never created. Credentials without a grant keep the exact previous payload and fence. --- src/mcp_tests/tools.rs | 479 +++++++++++++++++++++++++++ src/tool_runtime/projects.rs | 618 ++++++++++++++++++++++++++++++++--- 2 files changed, 1058 insertions(+), 39 deletions(-) diff --git a/src/mcp_tests/tools.rs b/src/mcp_tests/tools.rs index 093b47542..9e487a721 100644 --- a/src/mcp_tests/tools.rs +++ b/src/mcp_tests/tools.rs @@ -3996,6 +3996,485 @@ async fn mcp_show_changes_distinguishes_recording_session_id_from_query_session_ .any(|event| event.tool_name == "read_workspace_changes")); } +#[tokio::test] +async fn managed_worktree_project_grant_admission_preserves_exact_and_root_authority() { + const GRANT: &str = "wc_pgrant_r4aaaaaaaaaaaaaa"; + const CLIENT: &str = "r4-grant-runner"; + const INSTANCE: &str = "inst-r4-grant"; + const PROJECT: &str = "source-a"; + let fixture = tempfile::tempdir().unwrap(); + let source = fixture.path().join("source-a"); + let granted_root = fixture.path().join("granted"); + let inside = granted_root.join("source-b"); + let outside = fixture.path().join("outside"); + for path in [&source, &inside, &outside] { + std::fs::create_dir_all(path).unwrap(); + } + let source = source.canonicalize().unwrap(); + let granted_root = granted_root.canonicalize().unwrap(); + let inside = inside.canonicalize().unwrap(); + let outside = outside.canonicalize().unwrap(); + let fingerprint = format!("wc_projroot_{}", "1".repeat(64)); + let runtime = test_runtime(); + let runner_auth = crate::auth::AuthContext { + username: Some("local-owner".into()), + role: Some("agent".into()), + token_kind: Some("agent".into()), + allowed_client_id: Some(CLIENT.into()), + project_grant_id: Some(GRANT.into()), + ..crate::auth::AuthContext::new(crate::auth::AuthKind::AgentToken) + }; + let access = crate::runner_http::runner_access_from_auth(Some(&runner_auth)).unwrap(); + let mut caps = RunnerCapabilities::default(); + caps.set( + crate::runner_protocol::RunnerCapabilityId::ManagedWorktree, + true, + ); + runtime + .runner_registry + .register_with_auth( + crate::test_support::current_runner_registration(RunnerRegisterRequest { + computer_session_availability: None, + process_started_at: None, + build: None, + job_concurrency_limit: None, + job_inventory: None, + coding_agent_providers: None, + coding_agent_inventory: None, + client_id: CLIENT.into(), + runner_instance_id: INSTANCE.into(), + runner_protocol_generation: crate::runner_protocol::RUNNER_PROTOCOL_GENERATION_V2, + display_name: Some(CLIENT.into()), + owner: Some("local-owner".into()), + hostname: None, + host_context: None, + capabilities: caps, + policy: None, + }), + Some(&access), + ) + .await + .unwrap(); + crate::test_support::apply_project_inventory_snapshot( + &runtime.runner_registry, + CLIENT, + INSTANCE, + vec![RunnerProjectSummary { + id: PROJECT.into(), + name: Some(PROJECT.into()), + path: source.to_string_lossy().into_owned(), + allow_patch: true, + kind: Some("repo".into()), + registration_source: None, + description: None, + hooks: Vec::new(), + disabled: false, + revision: None, + root_fingerprint: Some(fingerprint.clone()), + lineage: None, + git_branch: None, + git_head: None, + git_dirty: None, + updated_at: 1, + shell_profile: None, + }], + ) + .await; + let auth = crate::auth::AuthContext { + role: Some("project".into()), + scopes: crate::auth::PROJECT_SHARE_OAUTH_SCOPES + .iter() + .map(|scope| (*scope).into()) + .collect(), + token_kind: Some(crate::auth::PROJECT_SHARE_OAUTH_TOKEN_KIND.into()), + allowed_client_id: Some("project-share-oauth-client".into()), + project_grant_id: Some(GRANT.into()), + project_root_scopes: vec![granted_root.clone()], + ..crate::auth::AuthContext::new(crate::auth::AuthKind::OAuth2Token) + }; + + for (path, expected_roots) in [(&source, false), (&inside, true)] { + let call = runtime.prepare_managed_worktree( + CLIENT.into(), + path.to_string_lossy().into_owned(), + None, + "r4-operation".into(), + None, + Some(PROJECT.into()), + Some(fingerprint.clone()), + Some(&auth), + ); + tokio::pin!(call); + let request = tokio::select! { + result = &mut call => panic!("authorized path must dispatch to Runner: {result:?}"), + request = wait_for_mcp_agent_request(&runtime.runner_registry, CLIENT, INSTANCE, "managed worktree") => request, + }; + assert_eq!(request.kind, "prepare_managed_worktree"); + let payload: Value = serde_json::from_str(request.stdin.as_deref().unwrap()).unwrap(); + assert_eq!(payload["path"], path.to_string_lossy().as_ref()); + assert_eq!(payload["expected_source_project_id"], PROJECT); + assert_eq!(payload["expected_source_root_fingerprint"], fingerprint); + if expected_roots { + assert_eq!( + payload["project_root_scopes"], + json!([granted_root.to_string_lossy()]) + ); + } else { + assert!(payload.get("project_root_scopes").is_none(), "{payload}"); + } + runtime + .runner_registry + .complete(RunnerResultRequest { + client_id: CLIENT.into(), + runner_instance_id: INSTANCE.into(), + request_id: request.request_id, + exit_code: Some(0), + stdout: Some( + json!({ + "outcome": "managed_worktree_created", + "registered": true, + "agent_project_id": "managed-1", + "path": "/tmp/worktree", + "name": "managed-1", + "allow_patch": true, + "disabled": false, + "kind": "repo", + "registration_source": "managed_worktree", + "description": "", + }) + .to_string(), + ), + stderr: Some(String::new()), + stdout_truncated: false, + stderr_truncated: false, + duration_ms: Some(1), + error: None, + }) + .await + .unwrap(); + let result = tokio::time::timeout(std::time::Duration::from_secs(10), &mut call) + .await + .unwrap(); + assert!(result.success, "{result:?}"); + } + let denied = runtime + .prepare_managed_worktree( + CLIENT.into(), + outside.to_string_lossy().into_owned(), + None, + "r4-denied".into(), + None, + None, + None, + Some(&auth), + ) + .await; + assert!(!denied.success); + assert_eq!(denied.output["error_kind"], "project_registry_scope_denied"); + assert!(runtime + .runner_registry + .poll(RunnerPollRequest { + client_id: CLIENT.into(), + runner_instance_id: INSTANCE.into(), + }) + .await + .unwrap() + .is_none()); + + // The Runner republishes root_fingerprint=None while its project directory + // is temporarily unresolvable. An exact-path authorized source then has + // neither grant roots nor a usable identity anchor, so the request must + // fail closed on the existing Reobserve contract instead of going out + // unanchored. + crate::test_support::apply_project_inventory_snapshot( + &runtime.runner_registry, + CLIENT, + INSTANCE, + vec![RunnerProjectSummary { + id: PROJECT.into(), + name: Some(PROJECT.into()), + path: source.to_string_lossy().into_owned(), + allow_patch: true, + kind: Some("repo".into()), + registration_source: None, + description: None, + hooks: Vec::new(), + disabled: false, + revision: None, + root_fingerprint: None, + lineage: None, + git_branch: None, + git_head: None, + git_dirty: None, + updated_at: 1, + shell_profile: None, + }], + ) + .await; + let unanchored = runtime + .prepare_managed_worktree( + CLIENT.into(), + source.to_string_lossy().into_owned(), + None, + "r5-unanchored".into(), + None, + None, + None, + Some(&auth), + ) + .await; + assert!(!unanchored.success, "{unanchored:?}"); + assert_eq!( + unanchored.output["error_kind"], + "managed_worktree_source_identity_unavailable" + ); + assert_eq!(unanchored.output["state_changed"], false); + assert!(runtime + .runner_registry + .poll(RunnerPollRequest { + client_id: CLIENT.into(), + runner_instance_id: INSTANCE.into(), + }) + .await + .unwrap() + .is_none()); + + // An ordinary non-project-scoped credential (the Runner's own AgentToken: + // a transport credential, not a model subject) never hits the exact-path + // fence and carries no grant roots. Its fresh managed-worktree request + // without expected identity keeps the pre-existing dispatch behavior — + // the Runner resolves the source identity itself. + let ordinary = runtime.prepare_managed_worktree( + CLIENT.into(), + source.to_string_lossy().into_owned(), + None, + "r6-ordinary".into(), + None, + None, + None, + Some(&runner_auth), + ); + tokio::pin!(ordinary); + let request = tokio::select! { + result = &mut ordinary => { + panic!("ordinary credential must keep dispatching without identity: {result:?}") + } + request = wait_for_mcp_agent_request(&runtime.runner_registry, CLIENT, INSTANCE, "managed worktree") => request, + }; + let payload: Value = serde_json::from_str(request.stdin.as_deref().unwrap()).unwrap(); + assert!( + payload.get("expected_source_project_id").is_none(), + "{payload}" + ); + assert!( + payload.get("expected_source_root_fingerprint").is_none(), + "{payload}" + ); + assert!(payload.get("project_root_scopes").is_none(), "{payload}"); + runtime + .runner_registry + .complete(RunnerResultRequest { + client_id: CLIENT.into(), + runner_instance_id: INSTANCE.into(), + request_id: request.request_id, + exit_code: Some(0), + stdout: Some( + json!({ + "outcome": "managed_worktree_created", + "registered": true, + "agent_project_id": "managed-1", + "path": "/tmp/worktree", + "name": "managed-1", + "allow_patch": true, + "disabled": false, + "kind": "repo", + "registration_source": "managed_worktree", + "description": "", + }) + .to_string(), + ), + stderr: Some(String::new()), + stdout_truncated: false, + stderr_truncated: false, + duration_ms: Some(1), + error: None, + }) + .await + .unwrap(); + let result = tokio::time::timeout(std::time::Duration::from_secs(10), &mut ordinary) + .await + .unwrap(); + assert!(result.success, "{result:?}"); + + // A caller-supplied complete identity keeps precedence over the missing + // inventory fingerprint: the request goes out anchored by that identity + // (the Runner revalidates it) and without grant roots. + let anchored = runtime.prepare_managed_worktree( + CLIENT.into(), + source.to_string_lossy().into_owned(), + None, + "r5-caller-anchored".into(), + None, + Some(PROJECT.into()), + Some(fingerprint.clone()), + Some(&auth), + ); + tokio::pin!(anchored); + let request = tokio::select! { + result = &mut anchored => { + panic!("caller-anchored identity must still dispatch to Runner: {result:?}") + } + request = wait_for_mcp_agent_request(&runtime.runner_registry, CLIENT, INSTANCE, "managed worktree") => request, + }; + let payload: Value = serde_json::from_str(request.stdin.as_deref().unwrap()).unwrap(); + assert_eq!(payload["expected_source_project_id"], PROJECT); + assert_eq!(payload["expected_source_root_fingerprint"], fingerprint); + assert!(payload.get("project_root_scopes").is_none(), "{payload}"); + runtime + .runner_registry + .complete(RunnerResultRequest { + client_id: CLIENT.into(), + runner_instance_id: INSTANCE.into(), + request_id: request.request_id, + exit_code: Some(0), + stdout: Some( + json!({ + "outcome": "managed_worktree_created", + "registered": true, + "agent_project_id": "managed-1", + "path": "/tmp/worktree", + "name": "managed-1", + "allow_patch": true, + "disabled": false, + "kind": "repo", + "registration_source": "managed_worktree", + "description": "", + }) + .to_string(), + ), + stderr: Some(String::new()), + stdout_truncated: false, + stderr_truncated: false, + duration_ms: Some(1), + error: None, + }) + .await + .unwrap(); + let result = tokio::time::timeout(std::time::Duration::from_secs(10), &mut anchored) + .await + .unwrap(); + assert!(result.success, "{result:?}"); +} + +#[tokio::test] +async fn project_share_root_scope_admits_new_directories_without_weakening_other_credentials() { + const GRANT_A: &str = "wc_pgrant_rootsaaaaaaaaaaaa"; + const CLIENT_A: &str = "project-roots-runner"; + const INSTANCE_A: &str = "inst-project-roots"; + + let base = + std::env::temp_dir().join(format!("wc-share-roots-{}", uuid::Uuid::new_v4().simple())); + std::fs::create_dir_all(base.join("fresh-dir")).expect("fixture directories"); + let sibling = format!("{}-extra", base.display()); + std::fs::create_dir_all(&sibling).expect("sibling fixture"); + let root_canonical = base.canonicalize().expect("canonical fixture root"); + + let runtime = test_runtime(); + let runner_auth = crate::auth::AuthContext { + username: Some("local-owner".to_string()), + role: Some("agent".to_string()), + token_kind: Some("agent".to_string()), + allowed_client_id: Some(CLIENT_A.to_string()), + project_grant_id: Some(GRANT_A.to_string()), + ..crate::auth::AuthContext::new(crate::auth::AuthKind::AgentToken) + }; + let access = crate::runner_http::runner_access_from_auth(Some(&runner_auth)).unwrap(); + runtime + .runner_registry + .register_with_auth( + crate::test_support::current_runner_registration(RunnerRegisterRequest { + computer_session_availability: None, + process_started_at: None, + build: None, + job_concurrency_limit: None, + job_inventory: None, + coding_agent_providers: None, + coding_agent_inventory: None, + client_id: CLIENT_A.to_string(), + runner_instance_id: INSTANCE_A.to_string(), + runner_protocol_generation: crate::runner_protocol::RUNNER_PROTOCOL_GENERATION_V2, + display_name: Some(CLIENT_A.to_string()), + owner: Some("local-owner".to_string()), + hostname: None, + host_context: None, + capabilities: RunnerCapabilities::default(), + policy: None, + }), + Some(&access), + ) + .await + .unwrap(); + + let share_oauth = crate::auth::AuthContext { + role: Some("project".to_string()), + scopes: crate::auth::PROJECT_SHARE_OAUTH_SCOPES + .iter() + .map(|scope| (*scope).to_string()) + .collect(), + token_kind: Some(crate::auth::PROJECT_SHARE_OAUTH_TOKEN_KIND.to_string()), + allowed_client_id: Some("project-share-oauth-client".to_string()), + project_grant_id: Some(GRANT_A.to_string()), + project_root_scopes: vec![root_canonical.clone()], + ..crate::auth::AuthContext::new(crate::auth::AuthKind::OAuth2Token) + }; + assert!(share_oauth.is_oauth_project_subject()); + + // Inside the granted root, an unregistered directory opens the on-demand + // path instead of denying: the fence returns None and the caller proceeds + // to the ordinary idempotent project-op. + let admitted = runtime + .project_scoped_visible_project_for_exact_path( + CLIENT_A, + &base.join("fresh-dir").to_string_lossy(), + Some(&share_oauth), + ) + .await + .expect("in-root path must not be scope-denied"); + assert!(admitted.is_none(), "no registration exists yet"); + + // Outside the granted root the fence stays closed. + let outside = runtime + .project_scoped_visible_project_for_exact_path(CLIENT_A, &sibling, Some(&share_oauth)) + .await; + assert!(outside.is_err(), "prefix sibling must stay denied"); + let parent = runtime + .project_scoped_visible_project_for_exact_path( + CLIENT_A, + root_canonical.parent().unwrap().to_str().unwrap(), + Some(&share_oauth), + ) + .await; + assert!(parent.is_err(), "root parent must stay denied"); + + // A project credential (non-OAuth share subject) never inherits the grant: + // its root scope list is empty by construction, so the same path denies. + let project_credential = crate::auth::shared_key::project_credential_context(GRANT_A); + let denied = runtime + .project_scoped_visible_project_for_exact_path( + CLIENT_A, + &base.join("fresh-dir").to_string_lossy(), + Some(&project_credential), + ) + .await; + assert!( + denied.is_err(), + "project credential must not inherit the parent-root grant" + ); + + std::fs::remove_dir_all(&base).ok(); + std::fs::remove_dir_all(&sibling).ok(); +} + #[tokio::test] async fn project_grant_authority_is_identical_for_project_credential_and_share_oauth_across_direct_and_gateway( ) { diff --git a/src/tool_runtime/projects.rs b/src/tool_runtime/projects.rs index fb59ffdf5..b657028da 100644 --- a/src/tool_runtime/projects.rs +++ b/src/tool_runtime/projects.rs @@ -14,6 +14,7 @@ //! no workspace abstraction. use serde_json::{json, Value}; +use std::path::Path; use std::time::Duration; use super::tool_result::{RecoveryKind, ToolResult}; @@ -274,6 +275,190 @@ fn project_registry_mutation_denied( .then(|| project_registry_scope_denied(operation)) } +/// Whether an explicitly authorized parent-root grant admits `path`. +/// +/// The grant lives on the project-share OAuth subject +/// (`AuthContext::project_root_scopes`), never on Runner filesystem policy, so +/// Runner cwd authority still cannot become Project-registry authority. The +/// request path must already exist and is canonicalized before the +/// component-wise containment check, which rejects `..` climbs, textual-prefix +/// siblings such as `-extra`, and symlinks that resolve outside every +/// granted root. A missing directory stays denied: admission never creates +/// directories, and the Runner-side `allowed_roots` policy remains the real +/// execution boundary. +fn authorized_project_root_scope_admits(auth: Option<&AuthContext>, path: &str) -> bool { + let Some(auth) = auth else { + return false; + }; + if auth.project_root_scopes.is_empty() { + return false; + } + let requested = Path::new(path); + if !requested.is_absolute() { + return false; + } + let Ok(canonical) = requested.canonicalize() else { + return false; + }; + auth.project_root_scopes + .iter() + .any(|root| canonical.starts_with(root)) +} + +/// Canonical target plus forwarded grant roots for one fence-checked path. +/// Root-scope credentials send the canonical target the fence authorized and +/// the grant's canonical roots, so the Runner re-derives the admission +/// decision against its own resolution before its first side effect; a +/// symlink or component flipped between the fence and the Runner's own +/// resolution then fails closed at the Runner instead of committing. +/// Credentials without parent-root grants get the raw path and no roots, the +/// exact previous behavior. An unresolvable or no-longer-admitted target +/// fails closed here: nothing is submitted. +fn grant_bound_project_path( + path: &str, + auth: Option<&AuthContext>, +) -> Result<(String, Option>), ToolResult> { + let Some(auth) = auth.filter(|auth| !auth.project_root_scopes.is_empty()) else { + return Ok((path.to_string(), None)); + }; + let canonical = match std::path::Path::new(path).canonicalize() { + Ok(canonical) => canonical, + Err(_) => { + return Err(project_registry_scope_denied("project path resolution")); + } + }; + let Some(canonical) = canonical.to_str() else { + return Err(project_registry_scope_denied("project path resolution")); + }; + if !authorized_project_root_scope_admits(Some(auth), canonical) { + return Err(project_registry_scope_denied("project path resolution")); + } + Ok(( + canonical.to_string(), + Some( + auth.project_root_scopes + .iter() + .map(|root| root.to_string_lossy().into_owned()) + .collect(), + ), + )) +} + +/// Build the resolve-or-register project-op payload from the bound path. +fn resolve_or_register_op_payload( + path: &str, + auth: Option<&AuthContext>, +) -> Result { + let (path, granted_roots) = grant_bound_project_path(path, auth)?; + Ok(match granted_roots { + None => json!({"path": path}), + Some(granted_roots) => json!({ + "path": path, + "project_root_scopes": granted_roots, + }), + }) +} + +/// Assemble one managed-worktree project-op payload. The Runner validates the +/// exact field count (4 | 6 ordinary, 5 | 7 with a forwarded share grant), so +/// this shape is a contract: source identity and grant roots are only ever +/// added together with their validated values, and the optional grant rides +/// along unchanged through the indeterminate-retry re-submission. +fn managed_worktree_op_payload( + source_path: String, + base_ref: Option, + operation_id: String, + resume_project_id: Option, + expected_source_identity: Option<(String, String)>, + granted_roots: Option>, +) -> serde_json::Value { + let mut payload = json!({ + "path": source_path, + "base_ref": base_ref, + "operation_id": operation_id, + "resume_project_id": resume_project_id, + }); + if let Some((project_id, root_fingerprint)) = expected_source_identity { + payload["expected_source_project_id"] = json!(project_id); + payload["expected_source_root_fingerprint"] = json!(root_fingerprint); + } + if let Some(granted_roots) = granted_roots { + payload["project_root_scopes"] = json!(granted_roots); + } + payload +} + +/// Re-verify the Runner's committed canonical path against the grant. The +/// Runner-side grant check is the commit-time authority; this catches view +/// drift between the Runner's resolution and the Server's grant after the +/// fact and reports the honest effect state. When the response does not carry +/// usable effect metadata the existing indeterminate contract applies: +/// the effect may have happened, so it is never reported as no-change. +fn verify_runner_path_within_granted_roots( + result: ToolResult, + auth: Option<&AuthContext>, +) -> ToolResult { + let Some(auth) = auth.filter(|auth| !auth.project_root_scopes.is_empty()) else { + return result; + }; + if !result.success { + return result; + } + let registered = result + .output + .get("registered") + .and_then(serde_json::Value::as_bool); + let Some(returned) = result + .output + .get("path") + .and_then(serde_json::Value::as_str) + else { + let message = "Runner returned a project resolution without a path; the granted project roots could not be re-verified"; + return match registered { + Some(registered) => ToolResult::err_with_output( + message, + json!({ + "error_kind": "project_registry_scope_denied", + "failure_kind": "authorization_denied", + "state_changed": registered, + }), + ) + .with_recovery(RecoveryKind::UserAction), + None => indeterminate_grant_verification_result(message), + }; + }; + if authorized_project_root_scope_admits(Some(auth), returned) { + return result; + } + let message = "Runner returned a project path outside the granted project roots"; + match registered { + Some(registered) => ToolResult::err_with_output( + message, + json!({ + "error_kind": "project_registry_scope_denied", + "failure_kind": "authorization_denied", + "state_changed": registered, + }), + ) + .with_recovery(RecoveryKind::UserAction), + None => indeterminate_grant_verification_result(message), + } +} + +/// The Runner reported success but not whether its write landed: the shared +/// project-op indeterminate contract, not a claim of no effect. +fn indeterminate_grant_verification_result(message: &str) -> ToolResult { + ToolResult::err_with_output( + message, + json!({ + "error_kind": "operation_indeterminate", + "failure_kind": "operation_indeterminate", + "state_changed": true, + }), + ) + .with_recovery(RecoveryKind::Reobserve) +} + fn existing_project_path_result(client_id: &str, project: &RunnerProjectSummary) -> ToolResult { ToolResult::ok(json!({ "id": runner_project_runtime_id(client_id, &project.id), @@ -304,9 +489,13 @@ impl ToolRuntime { /// For project-scoped model/API credentials, path-based coding may only use /// an exact path already present in that ProjectGrant's caller-visible - /// Runner inventory. This prevents broad Runner filesystem authority (needed - /// for canonical managed-worktree siblings) from becoming Project-registry - /// authority. Other credential classes keep the ordinary path-registration + /// Runner inventory, or — when the share was explicitly started with + /// `--project-roots` — an existing directory inside one of those granted + /// canonical parent roots, which the caller then admits on demand through + /// the ordinary idempotent project-op path. This prevents broad Runner + /// filesystem authority (needed for canonical managed-worktree siblings) + /// from becoming Project-registry authority without an explicit share-level + /// grant. Other credential classes keep the ordinary path-registration /// behavior. pub(crate) async fn project_scoped_visible_project_for_exact_path( &self, @@ -325,14 +514,22 @@ impl ToolRuntime { else { return Err(project_registry_scope_denied("project path resolution")); }; - let project = client + if let Some(project) = client .view .projects .iter() .find(|project| !project.disabled && project.path == path) .cloned() - .ok_or_else(|| project_registry_scope_denied("project path resolution"))?; - Ok(Some(project)) + { + return Ok(Some(project)); + } + if authorized_project_root_scope_admits(auth, path) { + // No exact registration yet: let the caller's project-op resolve or + // register it under the Runner registry write lock. Repeated calls + // keep the same stable project identity. + return Ok(None); + } + Err(project_registry_scope_denied("project path resolution")) } pub(crate) async fn list_projects_with_options( @@ -686,13 +883,14 @@ impl ToolRuntime { .with_recovery(RecoveryKind::NoAction); } } - self.submit_project_op( - "resolve_or_register_project", - client_id, - json!({"path": path}), - auth, - ) - .await + let payload = match resolve_or_register_op_payload(&path, auth) { + Ok(payload) => payload, + Err(denied) => return denied, + }; + let result = self + .submit_project_op("resolve_or_register_project", client_id, payload, auth) + .await; + verify_runner_path_within_granted_roots(result, auth) } /// Ask the exact Runner to prepare or re-observe one managed detached @@ -720,13 +918,43 @@ impl ToolRuntime { }), ); } - match self + let exact_path_project = match self .project_scoped_visible_project_for_exact_path(&client_id, &path, auth) .await { - Ok(Some(_)) | Ok(None) => {} + Ok(exact) => exact, Err(result) => return result, - } + }; + // The exact-path fence hit is the authorization fact itself; absence of + // grant roots is NOT (ordinary credentials also carry none). + let exact_path_authorized = exact_path_project.is_some(); + // Two independent, separately valid authorization sources bind the source + // the Runner will actually derive from: an exact-path registered Project + // (admitted above by registration, not by roots) or the credential's + // parent-root share grant. Exact-path sources keep their registered + // project identity and root fingerprint as the replay/retarget anchor and + // carry no grant roots; parent-root sources carry the grant. The Runner + // re-checks either against its own canonical resolution before any side + // effect. Only the source is grant-scoped — the derived sibling worktree + // destination is a Runner-owned namespace whose boundary remains + // allowed_roots, and the Runner result reports the destination path, so + // there is nothing to re-verify here. + let (source_path, granted_roots, fence_identity) = match exact_path_project { + Some(project) => ( + path.clone(), + None, + project + .root_fingerprint + .map(|root_fingerprint| (project.id.clone(), root_fingerprint)), + ), + None => { + let (source_path, granted_roots) = match grant_bound_project_path(&path, auth) { + Ok(bound) => bound, + Err(denied) => return denied, + }; + (source_path, granted_roots, None) + } + }; let access = crate::runner_http::runner_access_from_auth(auth); if let Some(client) = self .runner_registry @@ -754,30 +982,56 @@ impl ToolRuntime { } } let fresh_managed_bootstrap = resume_project_id.is_none(); - let mut payload = json!({ - "path": path, - "base_ref": base_ref, - "operation_id": operation_id, - "resume_project_id": resume_project_id, - }); - match (expected_source_project_id, expected_source_root_fingerprint) { - (Some(project_id), Some(root_fingerprint)) => { - payload["expected_source_project_id"] = json!(project_id); - payload["expected_source_root_fingerprint"] = json!(root_fingerprint); - } - (None, None) => {} - _ => { - return ToolResult::err_with_output( - "managed worktree source identity must be complete", - json!({ - "error_kind": "managed_worktree_source_identity_unavailable", - "failure_kind": "operation_failed", - "state_changed": false, - }), - ) - .with_recovery(RecoveryKind::Reobserve); - } + // Caller-supplied identity anchors replay/retarget and wins; the + // exact-path fence fills it in when the model did not supply one. + let expected_source_identity = + match (expected_source_project_id, expected_source_root_fingerprint) { + (Some(project_id), Some(root_fingerprint)) => Some((project_id, root_fingerprint)), + (None, None) => fence_identity, + _ => { + return ToolResult::err_with_output( + "managed worktree source identity must be complete", + json!({ + "error_kind": "managed_worktree_source_identity_unavailable", + "failure_kind": "operation_failed", + "state_changed": false, + }), + ) + .with_recovery(RecoveryKind::Reobserve); + } + }; + // Only sources actually admitted through the exact-path fence must have + // a complete, usable expected source identity to anchor the operation. + // When the Runner's inventory temporarily lacks a root_fingerprint and + // the caller did not supply one, the request must fail closed on the + // existing Reobserve contract instead of going out unanchored. + // Parent-root and ordinary credentials keep the pre-existing behavior: + // their sources are bounded by the forwarded grant or by ordinary + // admission, not by a fence identity. Resume requests (resume_project_id + // set) are exempt from THIS server-side completeness check only — the + // Runner still anchors them to its stored source path and lineage. + if exact_path_authorized + && expected_source_identity.is_none() + && resume_project_id.is_none() + { + return ToolResult::err_with_output( + "managed worktree source identity must be complete", + json!({ + "error_kind": "managed_worktree_source_identity_unavailable", + "failure_kind": "operation_failed", + "state_changed": false, + }), + ) + .with_recovery(RecoveryKind::Reobserve); } + let payload = managed_worktree_op_payload( + source_path, + base_ref, + operation_id, + resume_project_id, + expected_source_identity, + granted_roots, + ); let first = self .submit_project_op( "prepare_managed_worktree", @@ -1517,4 +1771,290 @@ mod tests { assert!(validate_project_op_name("My Project").is_ok()); assert!(validate_project_op_name("A").is_ok()); } + + fn root_scoped_auth(roots: &[&std::path::Path]) -> AuthContext { + let mut auth = AuthContext::new(crate::auth::AuthKind::OAuth2Token); + auth.token_kind = Some(crate::auth::PROJECT_SHARE_OAUTH_TOKEN_KIND.to_string()); + auth.project_grant_id = Some("wc_pgrant_test".to_string()); + auth.project_root_scopes = roots.iter().map(|root| root.to_path_buf()).collect(); + auth + } + + #[test] + fn project_root_scope_admits_canonical_children_and_root_itself() { + let base = + std::env::temp_dir().join(format!("wc-root-scope-{}", uuid::Uuid::new_v4().simple())); + let inner = base.join("magpie"); + std::fs::create_dir_all(&inner).expect("fixture directories"); + // An intermediate directory that really exists, so the `..` spelling + // below resolves through it. + std::fs::create_dir_all(base.join("nested")).expect("fixture intermediate"); + let root_canonical = base.canonicalize().expect("canonical fixture root"); + let auth = root_scoped_auth(&[root_canonical.as_path()]); + + assert!(authorized_project_root_scope_admits( + Some(&auth), + &inner.to_string_lossy() + )); + assert!(authorized_project_root_scope_admits( + Some(&auth), + &root_canonical.to_string_lossy() + )); + // A `..`-laden spelling resolves to the same canonical directory. + assert!(authorized_project_root_scope_admits( + Some(&auth), + &format!("{}/nested/../magpie", root_canonical.display()) + )); + + std::fs::remove_dir_all(&base).ok(); + } + + #[test] + fn project_root_scope_rejects_outside_and_prefix_siblings() { + let base = + std::env::temp_dir().join(format!("wc-root-scope-{}", uuid::Uuid::new_v4().simple())); + let inner = base.join("magpie"); + std::fs::create_dir_all(&inner).expect("fixture directories"); + let sibling = format!("{}-extra", base.display()); + std::fs::create_dir_all(&sibling).expect("sibling fixture directory"); + let root_canonical = base.canonicalize().expect("canonical fixture root"); + let auth = root_scoped_auth(&[root_canonical.as_path()]); + + // Textual-prefix sibling directory is not inside the root. + assert!(!authorized_project_root_scope_admits(Some(&auth), &sibling)); + // Parent of the root is outside. + let parent = root_canonical.parent().unwrap().to_path_buf(); + assert!(!authorized_project_root_scope_admits( + Some(&auth), + &parent.to_string_lossy() + )); + // Missing directory stays denied (admission never creates directories). + assert!(!authorized_project_root_scope_admits( + Some(&auth), + &base.join("not-created-yet").to_string_lossy() + )); + // Relative paths are rejected outright. + assert!(!authorized_project_root_scope_admits(Some(&auth), "magpie")); + // A subject without granted roots keeps the fence closed. + assert!(!authorized_project_root_scope_admits( + Some(&root_scoped_auth(&[] as &[&std::path::Path])), + &inner.to_string_lossy() + )); + // No auth context at all is rejected. + assert!(!authorized_project_root_scope_admits( + None, + &inner.to_string_lossy() + )); + + std::fs::remove_dir_all(&base).ok(); + std::fs::remove_dir_all(&sibling).ok(); + } + + #[test] + fn project_root_scope_rejects_symlink_escape() { + let base = + std::env::temp_dir().join(format!("wc-root-scope-{}", uuid::Uuid::new_v4().simple())); + let outside = std::env::temp_dir().join(format!( + "wc-root-scope-outside-{}", + uuid::Uuid::new_v4().simple() + )); + std::fs::create_dir_all(base.join("real")).expect("fixture root"); + std::fs::create_dir_all(outside.join("secret")).expect("outside fixture"); + let root_canonical = base.canonicalize().expect("canonical fixture root"); + #[cfg(unix)] + std::os::unix::fs::symlink(&outside, base.join("escape-link")).expect("symlink"); + let auth = root_scoped_auth(&[root_canonical.as_path()]); + + // The link name is inside the root, but it resolves outside: denied. + assert!(!authorized_project_root_scope_admits( + Some(&auth), + &base.join("escape-link").join("secret").to_string_lossy() + )); + + std::fs::remove_dir_all(&base).ok(); + std::fs::remove_dir_all(&outside).ok(); + } + + #[test] + fn managed_worktree_op_payload_field_counts_match_the_runner_contract() { + let canonical = "/tmp/granted/repo".to_string(); + // Ordinary credentials: the exact previous four-field payload. + let plain = + managed_worktree_op_payload(canonical.clone(), None, "op".into(), None, None, None); + assert_eq!(plain.as_object().unwrap().len(), 4); + assert_eq!(plain["path"], canonical); + assert!(plain.get("project_root_scopes").is_none()); + + // Source identity retains its two-field addition. + let identities = managed_worktree_op_payload( + canonical.clone(), + Some("HEAD".into()), + "op".into(), + Some("wt".into()), + Some(("source".into(), "wc_projroot_1".into())), + None, + ); + assert_eq!(identities.as_object().unwrap().len(), 6); + assert_eq!(identities["expected_source_project_id"], "source"); + + // A forwarded share grant adds exactly one field, alone or with the + // source identity, so the Runner's 5 | 7 acceptance applies. + let granted = managed_worktree_op_payload( + canonical.clone(), + None, + "op".into(), + None, + None, + Some(vec!["/tmp/granted".into()]), + ); + assert_eq!(granted.as_object().unwrap().len(), 5); + assert_eq!( + granted["project_root_scopes"], + serde_json::json!(["/tmp/granted"]) + ); + let both = managed_worktree_op_payload( + canonical, + Some("HEAD".into()), + "op".into(), + None, + Some(("source".into(), "wc_projroot_1".into())), + Some(vec!["/tmp/granted".into()]), + ); + assert_eq!(both.as_object().unwrap().len(), 7); + } + + #[test] + fn resolve_or_register_project_op_payload_binds_canonical_target_and_roots() { + let base = + std::env::temp_dir().join(format!("wc-root-scope-{}", uuid::Uuid::new_v4().simple())); + std::fs::create_dir_all(base.join("granted/inner")).expect("fixture"); + let root_canonical = base.join("granted").canonicalize().unwrap(); + let auth = root_scoped_auth(&[root_canonical.as_path()]); + + // A non-canonical spelling of an admitted target: the payload carries + // the canonical form the fence authorized plus the grant's roots. + let payload = resolve_or_register_op_payload( + &base.join("granted/inner/.").to_string_lossy(), + Some(&auth), + ) + .unwrap(); + assert_eq!( + payload["path"], + base.join("granted/inner") + .canonicalize() + .unwrap() + .to_string_lossy() + .as_ref() + ); + assert_eq!( + payload["project_root_scopes"], + serde_json::json!([root_canonical.to_string_lossy()]) + ); + + // Credentials without parent-root grants keep the exact previous + // payload shape. + let plain = + resolve_or_register_op_payload(&base.join("granted/inner/.").to_string_lossy(), None) + .unwrap(); + assert_eq!( + plain, + serde_json::json!({"path": base.join("granted/inner/.").to_string_lossy()}) + ); + + std::fs::remove_dir_all(&base).ok(); + } + + #[test] + #[cfg(unix)] + fn resolve_or_register_project_op_payload_fails_closed_when_target_stops_resolving() { + let base = + std::env::temp_dir().join(format!("wc-root-scope-{}", uuid::Uuid::new_v4().simple())); + std::fs::create_dir_all(base.join("granted/inner")).expect("fixture"); + let root_canonical = base.join("granted").canonicalize().unwrap(); + std::os::unix::fs::symlink(base.join("granted/inner"), base.join("granted/link")) + .expect("symlink"); + let auth = root_scoped_auth(&[root_canonical.as_path()]); + + // The fence admits the link spelling; then the target disappears, so + // nothing resolvable can be submitted: deny with no state change. + std::fs::remove_dir_all(base.join("granted/inner")).expect("remove target"); + let denied = resolve_or_register_op_payload( + &base.join("granted/link").to_string_lossy(), + Some(&auth), + ) + .unwrap_err(); + assert!(!denied.success); + assert_eq!(denied.output["state_changed"], false); + + std::fs::remove_dir_all(&base).ok(); + } + + #[test] + fn verify_runner_path_rejects_returned_paths_outside_the_grant() { + let base = + std::env::temp_dir().join(format!("wc-root-scope-{}", uuid::Uuid::new_v4().simple())); + std::fs::create_dir_all(base.join("granted/inner")).expect("fixture"); + std::fs::create_dir_all(base.join("outside")).expect("fixture"); + let root_canonical = base.join("granted").canonicalize().unwrap(); + let auth = root_scoped_auth(&[root_canonical.as_path()]); + + // The Runner committed a fresh registration outside the grant: deny, + // and report the effect honestly (state_changed follows `registered`). + let rejected = verify_runner_path_within_granted_roots( + ToolResult::ok(json!({ + "path": base.join("outside").canonicalize().unwrap().to_string_lossy(), + "registered": true, + })), + Some(&auth), + ); + assert!(!rejected.success); + assert_eq!(rejected.output["state_changed"], true); + + // An inside target passes through unchanged. + let inside_path = base.join("granted/inner").canonicalize().unwrap(); + let passed = verify_runner_path_within_granted_roots( + ToolResult::ok(json!({ + "path": inside_path.to_string_lossy(), + "registered": true, + })), + Some(&auth), + ); + assert!(passed.success); + + // A success without a resolvable path cannot be verified: deny, but + // keep whatever effect state the Runner did report. + let denied = verify_runner_path_within_granted_roots( + ToolResult::ok(json!({"registered": false})), + Some(&auth), + ); + assert!(!denied.success); + assert_eq!(denied.output["state_changed"], false); + let denied = verify_runner_path_within_granted_roots( + ToolResult::ok(json!({"registered": true})), + Some(&auth), + ); + assert!(!denied.success); + assert_eq!(denied.output["state_changed"], true); + + // Effect metadata missing or malformed uses the shared indeterminate + // contract: the write may have landed, so it is never claimed as + // no-change. + for node in [json!({}), json!({"registered": "yes"})] { + let denied = verify_runner_path_within_granted_roots(ToolResult::ok(node), Some(&auth)); + assert!(!denied.success); + assert_eq!(denied.output["error_kind"], "operation_indeterminate"); + assert_eq!(denied.output["state_changed"], true); + } + + // Failing results and credentials without grants pass through. + let failed = ToolResult::err("runner unavailable"); + assert!(!verify_runner_path_within_granted_roots(failed, Some(&auth)).success); + let unscoped = verify_runner_path_within_granted_roots( + ToolResult::ok(json!({"path": "anything"})), + None, + ); + assert!(unscoped.success); + + std::fs::remove_dir_all(&base).ok(); + } } From aaa6e8e74964846a88af6b890f2e569c7d0c130d Mon Sep 17 00:00:00 2001 From: vincent Date: Wed, 28 Oct 2026 08:05:47 +1000 Subject: [PATCH 5/9] test/docs: cover project-share root admission end to end Document --project-roots and --no-project-roots in both CLI peers. Add a loopback-only E2E that runs real dogfood Server and Runner processes behind a real share: OAuth authorization, on-demand admission, concurrency, symlink and prefix boundaries, grant replacement, restoration and clearing across nine launches. Because this branch intentionally excludes OAuth restart-persistence, each launch re-authorizes and exercises only its own fresh token. --- docs/CLI.md | 2 +- docs/CLI.zh-CN.md | 2 +- scripts/e2e_project_share_roots.py | 720 +++++++++++++++++++++++++++++ 3 files changed, 722 insertions(+), 2 deletions(-) create mode 100644 scripts/e2e_project_share_roots.py diff --git a/docs/CLI.md b/docs/CLI.md index 5553e4350..f8a05de76 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -119,7 +119,7 @@ These commands work on the current Git project. `webcodex share --auth query-token` is an explicit temporary-share compatibility mode for MCP clients that cannot configure a Bearer header. It accepts the exact share Project Credential only on `/mcp?token=...`, prints a URL-encoded sensitive MCP URL, and tells the client to use No authentication. The mode is disabled for ordinary Server/runtime requests, does not accept PAT/OAuth/shared-key/Runner credentials through the query, and is rejected with `--tunnel openai`. Treat the complete URL as a credential because URL queries may be retained by clients, proxies, clipboards, or access logs. The default remains `--auth bearer`. -`webcodex share --auth oauth --oauth-redirect-uri ` uses OAuth 2.0 Authorization Code with PKCE S256. The OAuth client ID/secret are persisted in protected project state for that project + callback, while the temporary OAuth grants are valid only for the current `share` run. Restarting `share` therefore invalidates old OAuth grants without changing the project. OAuth access tokens are never accepted on Runner transport. +`webcodex share --auth oauth --oauth-redirect-uri ` uses OAuth 2.0 Authorization Code with PKCE S256. The OAuth client ID/secret are persisted in protected project state for that project + callback, while the temporary OAuth grants are valid only for the current `share` run. Restarting `share` therefore invalidates old OAuth grants without changing the project. OAuth access tokens are never accepted on Runner transport. `--project-roots ` (repeatable, up to 8) explicitly authorizes this share's own project-share OAuth subjects to admit new Projects on demand from existing directories under the given canonical parent roots, so newly cloned repositories and new plain directories start working through the same MCP entry without per-project registration. The grant is validated and canonicalized at share start, persisted next to the share OAuth client state with durable update semantics: re-sending the same list is a no-op, a changed list atomically replaces the previous one (a failed update never destroys it), omitting the flag restores the last saved list, and `--no-project-roots` removes the persisted grant so removed roots never revive on a later restart. The grant is scoped to this identity on this Runner: paths outside the granted roots — including textual-prefix siblings, symlink escapes and missing directories — keep the `project_registry_scope_denied` fence, and the Runner `allowed_roots` policy remains the real execution boundary. Quick Tunnel origins remain temporary. For an operator-managed stable HTTPS origin, use `--tunnel none --public-url https://share.example` and route that origin to the loopback WebCodex Server yourself; `--public-url` advertises the external origin/issuer and does not create a proxy or tunnel. diff --git a/docs/CLI.zh-CN.md b/docs/CLI.zh-CN.md index 6d41ffbb7..8b0fb3d3e 100644 --- a/docs/CLI.zh-CN.md +++ b/docs/CLI.zh-CN.md @@ -68,7 +68,7 @@ LaunchAgent 与 Windows 用户计划任务需要该用户保持登录。旧服 `webcodex share --auth query-token` 是给无法配置 Bearer header 的 MCP client 使用的显式临时 share 兼容模式。它只在 `/mcp?token=...` 接受当前 share 的精确 Project Credential，输出经过 URL 编码的敏感 MCP URL，并让 client 选择 No authentication。普通 Server/runtime 请求不会因此启用 query auth，PAT/OAuth/shared-key/Runner credential 也不能通过这个 query 路径回退认证；`--tunnel openai` 会拒绝该模式。整条 URL 都必须当作 credential，因为 query 可能被 client、proxy、剪贴板或 access log 留存。默认仍是 `--auth bearer`。 -`webcodex share --auth oauth --oauth-redirect-uri <精确回调地址>` 使用 OAuth 2.0 Authorization Code + PKCE S256。OAuth client ID/secret 会按“项目 + 回调地址”保存在受保护的 project state 中；临时 OAuth grant 只在当前 `share` 运行期间有效。重启 `share` 会让旧 OAuth grant 失效，但不会改变项目。OAuth access token 永远不能用于 Runner transport。 +`webcodex share --auth oauth --oauth-redirect-uri <精确回调地址>` 使用 OAuth 2.0 Authorization Code + PKCE S256。OAuth client ID/secret 会按“项目 + 回调地址”保存在受保护的 project state 中；临时 OAuth grant 只在当前 `share` 运行期间有效。重启 `share` 会让旧 OAuth grant 失效，但不会改变项目。OAuth access token 永远不能用于 Runner transport。 `--project-roots <目录>`（可重复，最多 8 个）显式授权本 share 的 project-share OAuth 主体按需接入给定规范化父目录下已存在的目录，使新 clone 的仓库和新建的普通目录无需逐项目注册即可通过同一 MCP 入口开始工作。授权在 share 启动时校验并规范化，与 share OAuth client 状态一同持久化，并具备持久更新语义：重复传入相同列表是幂等空操作；列表变化时原子替换旧列表（更新失败绝不破坏旧值）；省略参数恢复最后保存的列表；`--no-project-roots` 删除持久化授权，被移除的根不会在之后的重启中复活。授权仅绑定本身份、本 Runner：授权根之外的路径——包括前缀相似目录、符号链接逃逸与不存在的目录——仍维持 `project_registry_scope_denied` 栅栏，Runner `allowed_roots` 策略仍是真实执行边界。 Cloudflare Quick Tunnel 的公网 origin 仍然是临时的。如需稳定 HTTPS origin，可使用 `--tunnel none --public-url https://share.example`，并由 operator 自己把该 origin 反向代理/隧道到 loopback WebCodex Server；`--public-url` 只声明外部 origin/issuer，不会创建代理或 tunnel。 diff --git a/scripts/e2e_project_share_roots.py b/scripts/e2e_project_share_roots.py new file mode 100644 index 000000000..57d75717c --- /dev/null +++ b/scripts/e2e_project_share_roots.py @@ -0,0 +1,720 @@ +#!/usr/bin/env python3 +"""Loopback-only project-share parent-root acceptance with real Server/Runner. + +Build the fork binaries first: + cargo build --locked --profile dogfood -p webcodex -p webcodex-cli -p webcodex-runner + python3 scripts/e2e_project_share_roots.py --bin-dir target/dogfood + +Requires Unix, Python 3, git. Everything runs on 127.0.0.1: a real +`webcodex share --tunnel none --auth oauth --project-roots ...` process owns a +real Server and WebSocket Runner, the harness completes the real OAuth +authorization-code + PKCE S256 flow with the share's own client, and every +project admission is driven through the MCP `work_on_project` tool surface. +No external service, production state or TLS verification bypass is used. +""" +from __future__ import annotations + +import argparse +import base64 +import concurrent.futures +import hashlib +import http.client +import json +import os +from pathlib import Path +import re +import secrets +import signal +import socket +import subprocess +import sys +import tempfile +import time +import urllib.parse + +from e2e_session_continuity_ws import check + +MCP_PROTOCOL = "2026-07-28" + + +def reserve_port() -> int: + with socket.socket() as sock: + sock.bind(("127.0.0.1", 0)) + return sock.getsockname()[1] + + +def git(repo: Path, *args: str) -> str: + result = subprocess.run( + ["git", *args], cwd=repo, capture_output=True, text=True, check=True + ) + return result.stdout.strip() + + +def make_git_repo(path: Path, branch: str = "main") -> None: + path.mkdir(parents=True, exist_ok=True) + git(path, "init", "-b", branch) + git(path, "config", "user.email", "fixture@example.invalid") + git(path, "config", "user.name", "Share Roots Fixture") + (path / "README.md").write_text("share roots fixture\n") + git(path, "add", "-A") + git(path, "commit", "-m", "fixture root commit") + + +class ShareRuntime: + def __init__(self, bin_dir: Path, root: Path): + self.bin_dir = bin_dir + self.root = root + self.public_port = reserve_port() + self.origin = f"http://127.0.0.1:{self.public_port}" + # The Server itself listens on the project's derived local port; the + # public origin above is only the OAuth issuer/MCP URL identity. It is + # discovered from the generated Runner config once the share is ready. + self.port: int | None = None + self.process: subprocess.Popen | None = None + self.rpc_id = 0 + + def launch(self, project_dir: Path, extra_args: list[str], tag: str = "a") -> None: + # One log per launch: wait_ready must only ever match the fresh ready + # message, never a previous phase's. + log_path = self.root / f"share-{self.public_port}-{tag}.log" + log_path.write_bytes(b"") + self.log_path = log_path + log = open(log_path, "ab") + command = [ + str(self.bin_dir / "webcodex"), "share", + "--tunnel", "none", + "--public-url", self.origin, + "--auth", "oauth", + "--oauth-redirect-uri", f"{self.origin}/callback", + "--no-copy-url", + "--state-dir", str(self.root / "state"), + *extra_args, + ] + self.process = subprocess.Popen( + command, cwd=project_dir, stdout=log, stderr=log, + start_new_session=True, + ) + + def discover_server_port(self) -> None: + configs = list((self.root / "state").glob("**/runner.toml")) + check(bool(configs), "generated Runner config not found under state dir") + match = re.search(r'server_url\s*=\s*"http://127\.0\.0\.1:(\d+)"', + configs[0].read_text()) + check(bool(match), f"server_url missing from {configs[0]}") + self.port = int(match.group(1)) + + def wait_ready(self, deadline_secs: float = 90.0) -> dict: + log_path = getattr(self, "log_path", + self.root / f"share-{self.public_port}-a.log") + deadline = time.monotonic() + deadline_secs + ready_re = re.compile( + r"4\. Client ID: (?P\S+)\n.*?5\. Client secret: (?P\S+)\n" + r".*?Project share credential \(this share only\): (?P\S+)", + re.S, + ) + while time.monotonic() < deadline: + if self.process is not None and self.process.poll() is not None: + raise AssertionError( + f"share exited early with {self.process.returncode}; see {log_path}" + ) + text = log_path.read_text(errors="replace") + match = ready_re.search(text) + if match: + return { + "client_id": match.group("client"), + "client_secret": match.group("secret"), + "credential": match.group("credential"), + } + time.sleep(0.3) + raise AssertionError(f"share readiness deadline exceeded; see {log_path}") + + def stop(self) -> None: + if self.process is None: + return + try: + os.killpg(self.process.pid, signal.SIGTERM) + except ProcessLookupError: + pass + try: + self.process.wait(timeout=20) + except subprocess.TimeoutExpired: + try: + os.killpg(self.process.pid, signal.SIGKILL) + except ProcessLookupError: + pass + self.process = None + + # --- plain HTTP ------------------------------------------------------- + def request(self, path: str, method: str = "GET", body: bytes | None = None, + headers: dict | None = None) -> tuple[int, dict, bytes]: + headers = dict(headers or {}) + if body is not None and "Content-Type" not in headers: + headers["Content-Type"] = "application/x-www-form-urlencoded" + check(self.port is not None, "server port not discovered yet") + connection = http.client.HTTPConnection("127.0.0.1", self.port, timeout=30) + try: + connection.request(method, path, body=body, headers=headers) + response = connection.getresponse() + data = response.read(2 * 1024 * 1024 + 1) + check(len(data) <= 2 * 1024 * 1024, "response exceeded bound") + return response.status, {k.lower(): v for k, v in response.getheaders()}, data + finally: + connection.close() + + # --- OAuth ------------------------------------------------------------ + def authorize(self, client: dict) -> dict: + verifier = secrets.token_urlsafe(48) + challenge = base64.urlsafe_b64encode( + hashlib.sha256(verifier.encode()).digest() + ).decode().rstrip("=") + parameters = { + "response_type": "code", + "client_id": client["client_id"], + "redirect_uri": f"{self.origin}/callback", + "scope": "runtime:read session:collaborate project:read project:write job:run", + "state": secrets.token_urlsafe(16), + "resource": self.origin + "/mcp", + "code_challenge": challenge, + "code_challenge_method": "S256", + } + authorize_path = "/oauth/authorize?" + urllib.parse.urlencode(parameters) + status, _, _ = self.request(authorize_path) + check(status == 200, "project share authorize form unavailable") + form = dict(parameters) + form["project_credential"] = client["credential"] + status, headers, _ = self.request( + "/oauth/authorize/project", "POST", urllib.parse.urlencode(form).encode(), + headers={"Origin": self.origin}, + ) + check(status == 302, f"project share authorize rejected: {status}") + query = urllib.parse.parse_qs(urllib.parse.urlsplit(headers.get("location", "")).query) + check(query.get("state") == [parameters["state"]] and "code" in query, + "OAuth redirect/code mismatch") + status, _, raw = self.request("/oauth/token", "POST", urllib.parse.urlencode({ + "grant_type": "authorization_code", + "code": query["code"][0], + "client_id": client["client_id"], + "client_secret": client["client_secret"], + "redirect_uri": f"{self.origin}/callback", + "resource": self.origin + "/mcp", + "code_verifier": verifier, + }).encode()) + check(status == 200, "S256 code exchange denied") + tokens = json.loads(raw) + check(isinstance(tokens.get("access_token"), str) + and isinstance(tokens.get("refresh_token"), str), "OAuth tokens missing") + return tokens + + # --- MCP -------------------------------------------------------------- + def mcp(self, token: str, method: str, params: dict | None = None) -> dict: + self.rpc_id += 1 + params = dict(params or {}) + params["_meta"] = { + "io.modelcontextprotocol/protocolVersion": MCP_PROTOCOL, + "io.modelcontextprotocol/clientCapabilities": {}, + } + headers = { + "Accept": "application/json, text/event-stream", + "Content-Type": "application/json", + "MCP-Protocol-Version": MCP_PROTOCOL, + "MCP-Method": method, + "Authorization": "Bearer " + token, + } + if "name" in params: + headers["MCP-Name"] = params["name"] + status, _, raw = self.request( + "/mcp", "POST", + json.dumps({"jsonrpc": "2.0", "id": self.rpc_id, "method": method, + "params": params}).encode(), + headers=headers, + ) + body = raw.decode(errors="replace") + if raw.lstrip().startswith(b"event:") or raw.lstrip().startswith(b"data:"): + data_lines = [line[5:].strip() for line in body.splitlines() + if line.startswith("data:")] + check(bool(data_lines), "empty SSE MCP response") + body = data_lines[-1] + reply = json.loads(body) + check(status == 200, f"MCP {method} failed: {status} {reply}") + return reply + + def call_tool(self, token: str, name: str, arguments: dict) -> dict: + reply = self.mcp(token, "tools/call", {"name": name, "arguments": arguments}) + check("error" not in reply, f"tools/call {name} JSON-RPC error: {reply.get('error')}") + result = reply["result"] + structured = result.get("structuredContent") + if isinstance(structured, dict): + payload = structured + else: + content = result.get("content", []) + text = "".join(part.get("text", "") for part in content + if part.get("type") == "text") + try: + payload = json.loads(text) + except json.JSONDecodeError: + payload = {"raw": text} + success = result.get("isError", False) is False and payload.get("success") is not False + return {"success": success, "output": payload} + + +def work_on_project(runtime: ShareRuntime, token: str, client_id: str, path: Path) -> dict: + return runtime.call_tool(token, "work_on_project", { + "client_id": client_id, + "path": str(path), + "mode": "checkout", + "instruction": "share roots fixture acceptance", + }) + + +def error_kind_of(result: dict) -> str | None: + output = result["output"] + if isinstance(output, dict): + if isinstance(output.get("error_kind"), str): + return output["error_kind"] + nested = output.get("output") + if isinstance(nested, dict) and isinstance(nested.get("error_kind"), str): + return nested["error_kind"] + return None + + +def tool_payload(result: dict) -> dict: + """Unwrap the nested tool-result text payload to its effective output.""" + output = result["output"] + if isinstance(output, dict) and isinstance(output.get("output"), dict): + return output["output"] + return output if isinstance(output, dict) else {} + + +def runner_client_id(runtime: ShareRuntime, token: str) -> str: + runners = runtime.call_tool(token, "list_runners", {"summary_only": True}) + check(runners["success"], f"list_runners failed: {runners}") + ids = tool_payload(runners).get("runners") + check(isinstance(ids, list) and len(ids) >= 1, f"no visible runners: {runners}") + first = ids[0] + client_id = first.get("client_id") if isinstance(first, dict) else None + check(isinstance(client_id, str) and client_id, f"runner client id missing: {first}") + return client_id + + +def list_project_paths(runtime: ShareRuntime, token: str) -> list[str]: + # list_projects is console-only on Adaptive Runtime: it is reachable only + # through the canonical gateway tool. + projects = runtime.call_tool(token, "call_runtime_tool", { + "tool": "list_projects", + "arguments": {"summary_only": True}, + }) + check(projects["success"], f"list_projects failed: {projects}") + rows = tool_payload(projects).get("projects", []) + return [row.get("path") for row in rows if isinstance(row, dict)] + + +def project_id_of(result: dict) -> str | None: + output = tool_payload(result) + for key in ("project", "resolved_project", "id", "agent_project_id"): + value = output.get(key) + if isinstance(value, str) and value: + return value + return None + + +def read_file(runtime: ShareRuntime, token: str, project_ref: str, path: str) -> str: + result = runtime.call_tool(token, "read_files", { + "project": project_ref, + "items": [{"path": path}], + }) + check(result["success"], f"read_files {path} failed: {result}") + output = tool_payload(result) + items = output.get("items") or (output.get("output", {}) or {}).get("items") + check(isinstance(items, list) and items, f"read_files returned no items: {output}") + first = items[0] + body = first.get("output", first) + text = body.get("text") if isinstance(body, dict) else body + check(isinstance(text, str), f"read_files produced no text: {first}") + return text + + +def create_file(runtime: ShareRuntime, token: str, project_ref: str, path: str, + content: str) -> None: + result = runtime.call_tool(token, "edit_project_files", { + "project": project_ref, + "changes": [{"kind": "create", "path": path, "content": content}], + }) + check(result["success"], f"edit_project_files create {path} failed: {result}") + + +def fresh_dir(path: Path) -> Path: + path.mkdir(parents=True, exist_ok=True) + return path + + +def deny_check(runtime: ShareRuntime, token: str, client_id: str, label: str, + path: Path, token_label: str = "fresh") -> None: + result = work_on_project(runtime, token, client_id, path) + check(not result["success"] and error_kind_of(result) == "project_registry_scope_denied", + f"[{token_label}] {label} was admitted: {result}") + + +def admit_check(runtime: ShareRuntime, token: str, client_id: str, label: str, + path: Path, expected_id: str | None = None) -> str: + result = work_on_project(runtime, token, client_id, path) + check(result["success"], f"[{label}] was not admitted: {result}") + identity = project_id_of(result) + check(identity, f"[{label}] admitted without a project identity: {result}") + if expected_id is not None: + check(identity == expected_id, + f"[{label}] identity drifted: {identity} != {expected_id}") + return identity + + +def managed_worktree_check(runtime: ShareRuntime, token: str, client_id: str, + label: str, source: Path, source_id: str) -> None: + before = git(source, "worktree", "list", "--porcelain") + result = runtime.call_tool(token, "work_on_project", { + "project": source_id, + "mode": "worktree", + "base_ref": "HEAD", + "instruction": "share roots managed worktree fixture", + }) + check(result["success"], f"[{label}] managed worktree failed: {result}") + output = tool_payload(result) + project = project_id_of(result) + session_id = output.get("session_id") + worktree = output.get("worktree", {}) + resolution = output.get("project_resolution", {}) + check(isinstance(project, str) and project.startswith("agent:") and project != source_id, + f"[{label}] missing distinct managed project: {output}") + check(isinstance(output.get("project_ref"), str) and output["project_ref"].startswith("~p"), + f"[{label}] missing managed project ref: {output}") + check(isinstance(session_id, str) and session_id.startswith("wc_sess_"), + f"[{label}] missing session: {output}") + check(worktree.get("managed") is True and worktree.get("base_sha") == git(source, "rev-parse", "HEAD"), + f"[{label}] incorrect managed base: {worktree}") + check(resolution.get("source") == "managed_worktree", + f"[{label}] incorrect project resolution: {resolution}") + after = git(source, "worktree", "list", "--porcelain") + check(after != before, f"[{label}] managed worktree was not created") + check("share roots fixture" in read_file(runtime, token, project, "README.md"), + f"[{label}] managed project contents missing") + + # A repeated worktree request without a Session starts fresh work. Resume + # explicitly through the source path to recover this exact managed project. + resumed = runtime.call_tool(token, "work_on_project", { + "client_id": client_id, + "path": str(source), + "mode": "worktree", + "session_id": session_id, + "instruction": "resume share roots managed worktree fixture", + }) + check(resumed["success"], f"[{label}] managed worktree resume failed: {resumed}") + resumed_output = tool_payload(resumed) + check(project_id_of(resumed) == project and resumed_output.get("session_id") == session_id + and resumed_output.get("continuation") == "resumed_explicitly", + f"[{label}] managed worktree resume changed identity: {resumed_output}") + check(git(source, "worktree", "list", "--porcelain") == after, + f"[{label}] resume created another worktree") + + +class Lifecycle: + """One launched share phase: client credentials and fresh OAuth tokens.""" + + def __init__(self, tag: str): + self.tag = tag + self.client: dict | None = None + self.tokens: dict | None = None + + +def revocation_check(runtime: ShareRuntime, token: str, kind: str, client_id: str, + label: str, denied_path: Path, valid_path: Path, + valid_id: str) -> None: + """One matrix cell: prove this token class still reaches its registered + project, THEN prove it cannot admit a real, never-registered directory + that sits inside the removed/cleared root. A missing directory would be + denied regardless of authorization, so every denied target is created + first and asserted to have never been admitted before.""" + admit_check(runtime, token, client_id, f"{label}: valid project stays reachable", + valid_path, expected_id=valid_id) + deny_check(runtime, token, client_id, + f"{label}: new dir under removed root", denied_path, + token_label=kind) + + +def main() -> None: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--bin-dir", type=Path, required=True) + args = parser.parse_args() + bin_dir = args.bin_dir.resolve() + for binary in ("webcodex", "webcodex-server", "webcodex-runner"): + check((bin_dir / binary).is_file(), f"missing binary {binary}") + + base = Path(tempfile.mkdtemp(prefix="wc-share-roots-")) + projects_root = base / "Projects" + share_proj = projects_root / "share-proj" + existing_repo = projects_root / "existing-repo" + root_a = projects_root / "roots-a" + existing_a = root_a / "existing-a" + root_b = projects_root / "roots-b" + sibling = base / "Projects-extra" + outside = base / "outside" + make_git_repo(share_proj) + make_git_repo(existing_repo) + (existing_repo / "notes.txt").write_text("existing repo notes\n") + root_a.mkdir(parents=True) + make_git_repo(existing_a) + root_b.mkdir(parents=True) # exists before its L4 grant + (outside / "deeper").mkdir(parents=True) # real outside target for the symlink case + sibling.mkdir(parents=True) + + runtime = ShareRuntime(bin_dir, base) + # OAuth transport grants stay fenced to one share launch (this branch + # intentionally excludes restart-persistent grants), so every phase + # re-authorizes and only that phase's fresh access token is exercised. + # Stable share client identity and the persisted parent-root grant are the + # properties under test across restarts. + state = { + "fresh_access": None, "first_client_id": None, + } + tokens_log: list[str] = [] + ids: dict[str, str] = {} + client_id: str | None = None + + def token_for(kind: str) -> str: + check(kind == "fresh", f"unsupported token class {kind}") + value = state["fresh_access"] + check(isinstance(value, str) and value, "fresh access token not minted yet") + return value # type: ignore[return-value] + + def restart(tag: str, extra_args: list[str]) -> None: + phase = Lifecycle(tag) + runtime.stop() + time.sleep(1.0) # let the previous phase's children release the port + runtime.launch(share_proj, extra_args, tag=tag) + phase.client = runtime.wait_ready() + runtime.discover_server_port() + if state["first_client_id"] is None: + state["first_client_id"] = phase.client["client_id"] + check(phase.client["client_id"] == state["first_client_id"], + f"[{tag}] persisted share OAuth client identity changed: " + f"{phase.client['client_id']} != {state['first_client_id']}") + phase.tokens = runtime.authorize(phase.client) + state["fresh_access"] = phase.tokens["access_token"] + tokens_log.append(f"{tag}: fresh=minted (fenced to this launch)") + + try: + # ---- L1: first explicit grant of root A --------------------------- + restart("l1", ["--project-roots", str(root_a)]) + client_id = runner_client_id(runtime, token_for("fresh")) + + # The share host project is pre-registered; everything else must come + # through the granted root. existing-repo sits OUTSIDE root A: it stays + # denied until it would be explicitly granted or registered. + ids["existing_a"] = admit_check( + runtime, token_for("fresh"), client_id, "existing dir under A", existing_a) + admit_check(runtime, token_for("fresh"), client_id, + "repeat A dir", existing_a, expected_id=ids["existing_a"]) + deny_check(runtime, token_for("fresh"), client_id, + "git repo outside any granted root", existing_repo) + + # Access regression on a project admitted through the parent root: + # MCP read of known content, MCP create, re-read, Git status. + marker = read_file(runtime, token_for("fresh"), ids["existing_a"], "README.md") + check("share roots fixture" in marker, f"known content mismatch: {marker!r}") + created = "roots-fixture-writes/evidence.txt" + create_file(runtime, token_for("fresh"), ids["existing_a"], created, + "written through MCP\n") + after = read_file(runtime, token_for("fresh"), ids["existing_a"], created) + check(after.strip() == "written through MCP", f"created content mismatch: {after!r}") + reentered = work_on_project(runtime, token_for("fresh"), client_id, existing_a) + check(reentered["success"], f"re-entry failed: {reentered}") + workspace = tool_payload(reentered).get("workspace", {}) + changed = [str(p) for p in (workspace.get("changed_paths") or [])] + # git collapses a wholly-untracked directory to its name; accept both + # the collapsed directory projection and the file-level one. + reflected = any(p.rstrip("/") == "roots-fixture-writes" + or p.endswith("evidence.txt") for p in changed) + check(reflected, + f"git status did not reflect the created file: {workspace}") + check(workspace.get("branch") == "main", f"unexpected branch: {workspace}") + + # A Git repository cloned/created AFTER the OAuth session exists is + # admitted on demand and recognized as Git (solidifies the reviewer's + # independently-verified case into the committed script). + late_repo = root_a / "late-git-repo" + make_git_repo(late_repo, branch="trunk") + late = work_on_project(runtime, token_for("fresh"), client_id, late_repo) + check(late["success"], f"post-session git repo not admitted: {late}") + late_ws = tool_payload(late).get("workspace", {}) + check(late_ws.get("branch") == "trunk", f"post-session repo branch wrong: {late_ws}") + late_id = project_id_of(late) + admit_check(runtime, token_for("fresh"), client_id, + "post-session git repo repeat", late_repo, + expected_id=late_id) + # A plain (non-Git) directory created after the session, already + # covered by fresh_dir targets below, is asserted explicitly too. + late_plain = fresh_dir(root_a / "late-plain") + admit_check(runtime, token_for("fresh"), client_id, + "post-session plain dir", late_plain) + + # Four concurrent opens of ONE first-admission directory: one identity. + fresh_same = root_a / "concurrent-same" + fresh_same.mkdir() + with concurrent.futures.ThreadPoolExecutor(max_workers=4) as pool: + futures = [pool.submit(work_on_project, runtime, token_for("fresh"), + client_id, fresh_same) for _ in range(4)] + results = [future.result() for future in futures] + check(all(result["success"] for result in results), + f"concurrent open failures: {results}") + identities = [project_id_of(result) for result in results] + check(all(isinstance(identity, str) and identity for identity in identities), + f"concurrent opens returned empty identities: {identities}") + check(len(set(identities)) == 1, + f"concurrent opens produced identities {set(identities)}") + + # Boundaries: outside root, prefix sibling, real-target symlink escape, + # missing path (and the missing path stays missing). + deny_check(runtime, token_for("fresh"), client_id, "outside dir", outside) + deny_check(runtime, token_for("fresh"), client_id, "prefix sibling", sibling) + escape_link = root_a / "escape-link" + os.symlink(outside, escape_link) + deny_check(runtime, token_for("fresh"), client_id, + "symlink escape (real target)", escape_link / "deeper") + missing = root_a / "never-created" + deny_check(runtime, token_for("fresh"), client_id, "missing dir", missing) + check(not missing.exists(), "denied admission created the missing directory") + + # Distinct directories keep distinct identities (regression retained). + distinct_a = root_a / "distinct-a" + distinct_b = root_a / "distinct-b" + distinct_a.mkdir(); distinct_b.mkdir() + id_a = admit_check(runtime, token_for("fresh"), client_id, + "distinct A", distinct_a) + id_b = admit_check(runtime, token_for("fresh"), client_id, + "distinct B", distinct_b) + check(id_a != id_b, "distinct directories collided on one identity") + + # ---- L2: two more launches carrying the SAME explicit flag -------- + # This is the systemd reality: an unchanging ExecStart re-sends the + # same --project-roots value on every restart (the P1-01 defect path). + restart("l2a", ["--project-roots", str(root_a)]) + admit_check(runtime, token_for("fresh"), client_id, + "A dir after same-flag restart 1", existing_a, + expected_id=ids["existing_a"]) + admit_check(runtime, token_for("fresh"), client_id, + "A new dir after same-flag restart 1", fresh_dir(root_a / "l2a-new")) + restart("l2b", ["--project-roots", str(root_a)]) + admit_check(runtime, token_for("fresh"), client_id, + "A dir after same-flag restart 2", existing_a, + expected_id=ids["existing_a"]) + admit_check(runtime, token_for("fresh"), client_id, + "A new dir after same-flag restart 2", fresh_dir(root_a / "l2b-new")) + + # ---- L3: omitting the flag restores A ------------------------------ + restart("l3", []) + admit_check(runtime, token_for("fresh"), client_id, + "A restored on omission", fresh_dir(root_a / "l3-restored")) + + # ---- L4: replace A with B ------------------------------------------ + restart("l4", ["--project-roots", str(root_b)]) + # The share project is an exact-path grant outside B. A new Git source + # under B first enters the Runner registry through ordinary checkout. + share_id = admit_check(runtime, token_for("fresh"), client_id, + "exact share project", share_proj) + managed_worktree_check(runtime, token_for("fresh"), client_id, + "exact share project outside B", share_proj, share_id) + b_source = root_b / "l4-worktree-source" + make_git_repo(b_source) + b_source_id = admit_check(runtime, token_for("fresh"), client_id, + "new Git source under B", b_source) + managed_worktree_check(runtime, token_for("fresh"), client_id, + "parent-root Git source B", b_source, b_source_id) + + # An unregistered Git source outside both grants must fail before + # registration or Git worktree creation. + paths_before = sorted(list_project_paths(runtime, token_for("fresh"))) + worktrees_before = git(existing_repo, "worktree", "list", "--porcelain") + denied = runtime.call_tool(token_for("fresh"), "work_on_project", { + "client_id": client_id, + "path": str(existing_repo), + "mode": "worktree", + "instruction": "outside roots managed worktree fixture", + }) + check(not denied["success"] and error_kind_of(denied) == "project_registry_scope_denied", + f"outside roots worktree was admitted: {denied}") + check(sorted(list_project_paths(runtime, token_for("fresh"))) == paths_before, + "denied worktree changed project registrations") + check(git(existing_repo, "worktree", "list", "--porcelain") == worktrees_before, + "denied worktree changed Git worktrees") + admit_check(runtime, token_for("fresh"), client_id, + "B admitted after replacement", fresh_dir(root_b / "l4-b-new")) + # Root A was removed: this launch's fresh token keeps registered-project + # access but cannot admit a REAL, never-registered directory under A. + revocation_check(runtime, token_for("fresh"), "fresh", client_id, + "A→B removed A (fresh)", + fresh_dir(root_a / "l4-a-fresh"), existing_a, + ids["existing_a"]) + # The top-level Projects directory is outside ANY grant. + deny_check(runtime, token_for("fresh"), client_id, + "top-level outside any grant", fresh_dir(projects_root / "l4-top-new")) + # The removed-root symlink-escape boundary stays enforced. + deny_check(runtime, token_for("fresh"), client_id, + "removed A symlink escape", escape_link / "deeper") + + # ---- L5: omission restores B, not A -------------------------------- + restart("l5", []) + admit_check(runtime, token_for("fresh"), client_id, + "B restored on omission", fresh_dir(root_b / "l5-b-new")) + deny_check(runtime, token_for("fresh"), client_id, + "A stays removed after omission", + fresh_dir(root_a / "l5-a-new")) + + # ---- L6: explicit clear -------------------------------------------- + # The grant in force immediately before the clear is B, so the + # never-registered revocation target must be a fresh directory under B + # (a directory under the already-removed root A would be denied + # regardless of the clear and prove nothing). + restart("l6", ["--no-project-roots"]) + revocation_check(runtime, token_for("fresh"), "fresh", client_id, + "cleared (fresh)", + fresh_dir(root_b / "l6-b-fresh"), existing_a, + ids["existing_a"]) + # Cleared grant must not resurrect through the old escape link either. + deny_check(runtime, token_for("fresh"), client_id, + "cleared symlink escape", escape_link / "deeper") + + # ---- L7: repeated clear is idempotent ------------------------------- + restart("l7", ["--no-project-roots"]) + deny_check(runtime, token_for("fresh"), client_id, + "still cleared after repeat", fresh_dir(root_b / "l7-b-new")) + + # ---- L8: omission after clear never revives old roots --------------- + restart("l8", []) + deny_check(runtime, token_for("fresh"), client_id, + "A did not revive", fresh_dir(root_a / "l8-a-new")) + deny_check(runtime, token_for("fresh"), client_id, + "B did not revive", fresh_dir(root_b / "l8-b-new")) + admit_check(runtime, token_for("fresh"), client_id, + "registered project access intact at the end", existing_a, + expected_id=ids["existing_a"]) + # Registered-project access survives the whole walk even though every + # new-directory admission it once had is gone. + admit_check(runtime, token_for("fresh"), client_id, + "fresh token registered access at the end", existing_a, + expected_id=ids["existing_a"]) + + print("token_provenance:") + for line in tokens_log: + print(f" {line}") + print("grant_revocation=A→B and cleared; fresh-launch grants denied for new " + "directories with registered-project access proven first") + print("E2E_PROJECT_SHARE_ROOTS=PASS") + print(f"fixture_root={base}") + print("lifecycle_launches=9 client_id_stable=true") + finally: + runtime.stop() + + +if __name__ == "__main__": + try: + main() + except AssertionError as error: + print(f"E2E_PROJECT_SHARE_ROOTS=FAIL: {error}", file=sys.stderr) + sys.exit(1) From 1e062b4846354e1498543d344cb867607eed3130 Mon Sep 17 00:00:00 2001 From: vincent Date: Wed, 28 Oct 2026 08:19:59 +1000 Subject: [PATCH 6/9] fix(runner): compare granted roots in canonical form The Windows runner job rejected a target inside an explicitly granted root: the grant check compared the Runner's canonical target against the root text it received, and a Windows verbatim namespace prefix (or any unresolved component) made the component comparison fail, so admission was denied with project_path_outside_granted_roots. Resolve each granted root on the Runner before comparing, using the same component-wise, case-insensitive path_is_within helper the existing allowed_roots boundary uses. A root this filesystem cannot resolve stays denied, and the target is still resolved locally, so the commit-boundary binding is unchanged. Both the exact-path and the managed-worktree grant checks use it. Regression: resolve_or_register_project_accepts_unresolved_granted_root grants "/../" (accepted) and a missing root (denied), independent of the platform's canonical text form. --- .../src/main_tests/project_registration.rs | 41 +++++++++++++++++++ .../projects/managed_worktree.rs | 10 ++++- .../webcodex_runner/projects/registration.rs | 23 +++++++---- 3 files changed, 63 insertions(+), 11 deletions(-) diff --git a/crates/webcodex-runner/src/main_tests/project_registration.rs b/crates/webcodex-runner/src/main_tests/project_registration.rs index 350fe1a84..01e3b2682 100644 --- a/crates/webcodex-runner/src/main_tests/project_registration.rs +++ b/crates/webcodex-runner/src/main_tests/project_registration.rs @@ -842,6 +842,47 @@ fn resolve_or_register_project_accepts_target_inside_granted_roots_idempotently( assert_eq!(records, 1, "repeated calls keep one stable identity"); } +#[test] +fn resolve_or_register_project_accepts_unresolved_granted_root() { + let tmp = tempfile::tempdir().unwrap(); + let granted = tmp.path().join("granted"); + let inner = granted.join("inner"); + std::fs::create_dir_all(&inner).unwrap(); + let project_registry_dir = tmp.path().join("project-registry"); + std::fs::create_dir(&project_registry_dir).unwrap(); + let policy = project_policy(tmp.path()); + + // The Server canonicalizes granted roots, but admission must not depend on + // that text form: the Runner resolves each root itself before comparing. + let unresolved_granted = granted.join("..").join("granted"); + let resolved = project_ok(handle_resolve_or_register_project( + &policy, + &project_registry_dir, + &project_request( + "resolve_or_register_project", + serde_json::json!({ + "path": inner.to_string_lossy(), + "project_root_scopes": [unresolved_granted.to_string_lossy()], + }), + ), + )); + assert_eq!(resolved["outcome"], "auto_registered"); + + // A root this filesystem cannot resolve stays denied. + let missing = project_err(handle_resolve_or_register_project( + &policy, + &project_registry_dir, + &project_request( + "resolve_or_register_project", + serde_json::json!({ + "path": inner.to_string_lossy(), + "project_root_scopes": [tmp.path().join("missing-root").to_string_lossy()], + }), + ), + )); + assert_eq!(missing, "project_path_outside_granted_roots"); +} + #[test] fn resolve_or_register_project_rejects_malformed_grant_scopes_fail_closed() { let tmp = tempfile::tempdir().unwrap(); diff --git a/crates/webcodex-runner/src/webcodex_runner/projects/managed_worktree.rs b/crates/webcodex-runner/src/webcodex_runner/projects/managed_worktree.rs index 4ab23ac9b..d37948f86 100644 --- a/crates/webcodex-runner/src/webcodex_runner/projects/managed_worktree.rs +++ b/crates/webcodex-runner/src/webcodex_runner/projects/managed_worktree.rs @@ -700,9 +700,15 @@ pub(crate) fn handle_prepare_managed_worktree_operation( // Share-grant admission is judged on the Runner's own canonical // resolution of the source before any git invocation or registry // write: a source that resolved inside the grant at Server-fence time - // but elsewhere here fails closed. The derived worktree destination is + // but elsewhere here fails closed. Roots are compared in canonical form + // too, so unresolved text or a Windows verbatim prefix cannot decide it; + // an unresolvable root stays denied. The derived worktree destination is // not part of this check; allowed_roots above still governs it. - if !granted_roots.iter().any(|root| source.starts_with(root)) { + let admitted = granted_roots.iter().any(|root| { + canonicalize_existing(root) + .is_ok_and(|root| webcodex_runner_config::paths::path_is_within(&source, &root)) + }); + if !admitted { return managed_worktree_error( start, "project_path_outside_granted_roots", diff --git a/crates/webcodex-runner/src/webcodex_runner/projects/registration.rs b/crates/webcodex-runner/src/webcodex_runner/projects/registration.rs index 2e382a5bb..9e23d54ce 100644 --- a/crates/webcodex-runner/src/webcodex_runner/projects/registration.rs +++ b/crates/webcodex-runner/src/webcodex_runner/projects/registration.rs @@ -749,15 +749,20 @@ pub(crate) fn handle_resolve_or_register_project_operation( if let Some(granted_roots) = &granted_roots { // Share-grant admission is judged on the Runner's own canonical // resolution of the committed target, component-wise against the - // Server-granted canonical roots. This is the last gate before any - // resolution outcome below: neither an existing registration nor a - // fresh registration may be resolved outside the grant, and the - // Runner filesystem allowed_roots above is a coarser boundary that - // never substitutes for it. - if !granted_roots - .iter() - .any(|root| canonical_path.starts_with(root)) - { + // Server-granted roots. Both sides are compared in canonical form so a + // text difference (an unresolved component, or a Windows verbatim + // namespace prefix) cannot decide admission; a root this Runner cannot + // resolve stays denied. + // This is the last gate before any resolution outcome below: neither an + // existing registration nor a fresh registration may be resolved outside + // the grant, and the Runner filesystem allowed_roots above is a coarser + // boundary that never substitutes for it. + let admitted = granted_roots.iter().any(|root| { + canonicalize_existing(root).is_ok_and(|root| { + webcodex_runner_config::paths::path_is_within(&canonical_path, &root) + }) + }); + if !admitted { return structured_project_error_cmd( start, "project_path_outside_granted_roots", From 1e3ad6c55a90507f5fc833b9dcdecf911fb7fe75 Mon Sep 17 00:00:00 2001 From: vincent Date: Thu, 29 Oct 2026 04:44:47 +1000 Subject: [PATCH 7/9] test(runner): drop redundant mut in share-grant fixture --- crates/webcodex-runner/src/main_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/webcodex-runner/src/main_tests.rs b/crates/webcodex-runner/src/main_tests.rs index 08417cf8c..c135f666c 100644 --- a/crates/webcodex-runner/src/main_tests.rs +++ b/crates/webcodex-runner/src/main_tests.rs @@ -2566,7 +2566,7 @@ fn managed_worktree_rejects_malformed_grant_scopes_fail_closed() { let policy = project_policy(tmp.path()); let granted_root = granted.canonicalize().unwrap(); - let mut base = |extra: serde_json::Value| { + let base = |extra: serde_json::Value| { let mut payload = serde_json::json!({ "path": source.to_string_lossy(), "base_ref": serde_json::Value::Null, From 4eb075c5a258c1363921e1970be642afbbdf6e63 Mon Sep 17 00:00:00 2001 From: vincent Date: Thu, 29 Oct 2026 04:58:25 +1000 Subject: [PATCH 8/9] docs(cli): advertise --project-roots and --no-project-roots in share usage --- scripts/e2e_project_share_roots.py | 2 +- src/project_entry.rs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/e2e_project_share_roots.py b/scripts/e2e_project_share_roots.py index 57d75717c..5101f0179 100644 --- a/scripts/e2e_project_share_roots.py +++ b/scripts/e2e_project_share_roots.py @@ -1,7 +1,7 @@ #!/usr/bin/env python3 """Loopback-only project-share parent-root acceptance with real Server/Runner. -Build the fork binaries first: +Build the binaries first: cargo build --locked --profile dogfood -p webcodex -p webcodex-cli -p webcodex-runner python3 scripts/e2e_project_share_roots.py --bin-dir target/dogfood diff --git a/src/project_entry.rs b/src/project_entry.rs index 5dae1ce7f..cc3a1055e 100644 --- a/src/project_entry.rs +++ b/src/project_entry.rs @@ -237,6 +237,7 @@ pub(crate) fn usage() -> &'static str { "Usage: webcodex share [--root PATH] [--profile NAME] [--state-dir PATH] [--json]\n\ [--tunnel cloudflare|openai|none] [--auth bearer|query-token|oauth]\n\ [--oauth-redirect-uri URL] [--public-url URL] [--no-copy-url]\n\ + [--project-roots DIR]... [--no-project-roots]\n\ [--stop-on-stdin-eof]\n\ webcodex status [--root PATH] [--profile NAME] [--state-dir PATH] [--json]\n\ webcodex doctor [--root PATH] [--profile NAME] [--state-dir PATH] [--json]\n\ @@ -258,6 +259,9 @@ starting services. `run` is the explicit foreground local runtime step. Its opti `--console-assets-dir` enables loopback-only development assets for that run.\n\ `--auth query-token` is a temporary share-only convenience for MCP clients that\n\ cannot configure a Bearer header; `--auth oauth` adds project-bound OAuth.\n\ +Repeatable `--project-roots DIR` (up to 8) lets this share's own project-share\n\ +OAuth subjects admit new Projects from existing directories under those\n\ +canonical parent roots; `--no-project-roots` clears the saved list.\n\ On Windows, explicit `webcodex share` is supported. Managed Cloudflare acquisition is available on Windows x64; Windows ARM64 requires a trusted explicit/PATH cloudflared because the pinned upstream release has no official ARM64 artifact. Native OpenAI Tunnel supports Windows x64/arm64.\n" } From e87057f9460f80b5d0ef603dd15799be78451a4e Mon Sep 17 00:00:00 2001 From: vincent Date: Thu, 29 Oct 2026 06:04:58 +1000 Subject: [PATCH 9/9] docs(testing): list the project-share parent-root acceptance harness --- docs/TESTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/TESTING.md b/docs/TESTING.md index c2cb2c0b6..21e645433 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -88,6 +88,7 @@ adopt the CLI text ordering policy. | real-process hosted-connect harness | Build the real Server, Runner, and CLI; start a shared-key-enabled loopback Server; run `webcodex connect`; verify same-key project visibility and a read, cross-key isolation, detached Runner survival, repeated-connect PID reuse, hosted `runner status`, explicit stop, secret-safe output/log/state, and an untouched Git checkout. | Local processes, isolated XDG config/state roots, a temp Git project, bounded curl and outer timeout, trap cleanup; never production. | `bash scripts/e2e_hosted_connect.sh` | | real-process job reconciliation harness | Boot a real server plus a WebSocket runner that advertises `job_state_reconciliation`. Scenario A keeps a raw async Job running across a SERVER-only restart and asserts the SAME runner instance, original `job_id`, preserved ownership/project/session, non-regressing sequence/log cursors, `recovered_after_server_restart`, original-process stop, and one side-effect set. Scenario B lets a Job complete while the Server is offline and reconciles the terminal result without duplicate logs or execution. Scenario C forces `run_process` past its synchronous grace window, then proves the handed-off structured Job survives a Server restart and an old Server-epoch observation token refreshes immediately for the same `job_id`. Scenario D uses a delayed Cargo fixture to force a real `cargo_check` validation handoff past its sync window, then proves the same restart/token-refresh/stop contract with the validation command started exactly once. Ordinary Runner-owned Jobs keep the Runner process alive for these scenarios; `run_detached_process` restart survival is a separate supervisor-ownership contract covered by its focused Runner suites and production dogfood. | Local processes, temp dirs/ports/tokens, and a temp project; no production services or QUIC certs. Scenario D intentionally takes roughly the validation sync window plus restart time. | `bash scripts/e2e_job_reconciliation_ws.sh` | | real-process job recovery failure/non-reconciliation harness | Cover the failure and non-reconciliation paths the happy-path reconciliation harness omits, using `WEBCODEX_JOB_RECOVERY_GRACE_SECS=10` (clamped, above the 5s floor) so the deadline is bounded without waiting the 120s default. Scenario C: kill the runner only (server stays up), let the job enter `recovering`, and assert the non-request-triggered recovery-timeout sweep transitions it to `lost` with `runner_recovery_deadline_exceeded`, `ended_at` set once, one list record, stop-on-lost stable, and the command never re-executes. Scenario D: instance B replaces instance A (same client_id, new `agent_instance_id`); A's job becomes `lost` with `runner_instance_replaced`, B starts its own new job, A's late update is rejected, first `ended_at`/reason preserved. Scenario E: a generation-2 Runner registered with `WEBCODEX_RUNNER_DISABLE_JOB_STATE_RECONCILIATION=1` (no capability, no inventory) dispatches a job and, on disconnect, deterministically fences it to `lost` with `runner_disconnected_without_reconciliation` (never `recovering`); after a server restart its public lost receipt remains observable within retention, and a same-client new no-reconciliation instance cannot revive execution. Scenario F: a long job across three server restarts keeps the same `job_id`, runs the command once, keeps `last_update_seq`/log cursors non-regressing and markers non-duplicating, and reaches a terminal `stopped` that survives a third restart with `ended_at` unchanged by terminal inventory replay. | Local processes, temp dirs/ports/tokens, and a temp project; no production services or QUIC certs. | `bash scripts/e2e_job_recovery_failures_ws.sh` | +| project-scoped parent-root admission | Real `webcodex share --auth oauth --project-roots DIR` process with a real Server and WebSocket Runner: authorize with PKCE S256, then admit an already-registered project, a new Git directory and a new non-Git directory through the MCP `work_on_project` entry point, create and resume a managed worktree, collapse concurrent admissions to one project identity, and assert denials for out-of-root, prefix-sibling, symlink-escape and missing roots plus exact-path credentials keeping their fence. Also covers root replacement, omitted-flag restore and `--no-project-roots` clearing without revival. | Temporary state dir, loopback ports, a temp fixture root; re-authorizes each share launch instead of reusing a token. Unix only. | `python3 scripts/e2e_project_share_roots.py --bin-dir ` | | security auth matrix | Cover OAuth, scope policy, shared-key behavior, token classes, read-only session guards, and denied mutations. | No external identity provider by default; use local fixtures and synthetic tokens. | `cargo test -p webcodex --lib oauth -- --nocapture`; `cargo test -p webcodex --lib scope -- --nocapture`; `cargo test -p webcodex --lib metadata -- --nocapture` | | MCP conformance baseline | Exercise the real Salvo `/mcp` handlers through a disposable loopback fixture against a pinned upstream referee, then gate on check-level coverage/classification rather than process exit alone. | Loopback product fixture; immutable external referee source/dependencies fetched only by the explicit script/CI lane; no production credentials. | `bash scripts/mcp_conformance.sh`; see [`MCP_CONFORMANCE.md`](MCP_CONFORMANCE.md). |