diff --git a/.yoi/tickets/00001KX1JNJ2Y/item.md b/.yoi/tickets/00001KX1JNJ2Y/item.md index 7aff46ff..7bbd029f 100644 --- a/.yoi/tickets/00001KX1JNJ2Y/item.md +++ b/.yoi/tickets/00001KX1JNJ2Y/item.md @@ -2,7 +2,7 @@ title: 'Add ProfileSourceTree virtual filesystem and Decodal profile editor' state: 'inprogress' created_at: '2026-07-08T19:18:25Z' -updated_at: '2026-07-09T08:46:58Z' +updated_at: '2026-07-09T09:08:18Z' assignee: null queued_by: 'workspace-panel' queued_at: '2026-07-09T07:56:07Z' diff --git a/.yoi/tickets/00001KX1JNJ2Y/thread.md b/.yoi/tickets/00001KX1JNJ2Y/thread.md index 7c5b9542..eeb270f0 100644 --- a/.yoi/tickets/00001KX1JNJ2Y/thread.md +++ b/.yoi/tickets/00001KX1JNJ2Y/thread.md @@ -174,4 +174,28 @@ Validation: - `nix build .#yoi --no-link` +--- + + + +## Implementation report + +Follow-up fix for source-root symlink review blocker. + +Summary: +- Added a shared ProfileSourceRoot guard that rejects `.yoi/profiles` when it is a symlink and verifies the canonical profile source root remains under the canonical workspace root. +- Applied the guard to source tree listing, tree file read/delete checks, write preparation, source summaries, and archive construction through the existing tree scan/read path. +- Hardened writes so `.yoi` and `.yoi/profiles` are created one component at a time only after symlink/canonical containment checks, preventing outside mutations through a symlinked source root. +- Added regression coverage for `.yoi/profiles -> outside_dir` to assert writes fail before outside side effects and list/archive paths fail closed. + +Validation: +- `git diff --check` +- `cargo test -p yoi-workspace-server` +- `cargo test -p worker-runtime --features ws-server,fs-store` +- `cargo check -p yoi` +- `cd web/workspace && deno task check && deno task test` +- `yoi ticket doctor` +- `nix build .#yoi --no-link` + + --- diff --git a/crates/workspace-server/src/profile_settings.rs b/crates/workspace-server/src/profile_settings.rs index 9b3292a1..4f105c68 100644 --- a/crates/workspace-server/src/profile_settings.rs +++ b/crates/workspace-server/src/profile_settings.rs @@ -504,10 +504,7 @@ pub fn create_profile_source( .join("profiles") .join(format!("{name}.dcdl")); validate_source_content(workspace_root, &name, &relative_path, &request.content)?; - let full = checked_source_path(workspace_root, &relative_path)?; - if let Some(parent) = full.parent() { - fs::create_dir_all(parent)?; - } + let full = prepare_source_path_for_write(workspace_root, &relative_path)?; fs::write(&full, request.content)?; registry.profile.insert( name.clone(), @@ -1039,33 +1036,49 @@ fn ensure_profile_source_tree_id(source_tree_id: &str) -> Result<()> { fn list_profile_tree_files( workspace_root: &Path, ) -> Result> { + let Some(source_root) = existing_profile_source_root(workspace_root)? else { + return Ok(Vec::new()); + }; let mut files = Vec::new(); - collect_profile_tree_files( - workspace_root, - &workspace_root.join(PROFILE_SOURCE_ROOT_RELATIVE_PATH), - &mut files, - )?; + collect_profile_tree_files(workspace_root, &source_root, &source_root.path, &mut files)?; files.sort_by(|a, b| a.path.cmp(&b.path)); Ok(files) } fn collect_profile_tree_files( workspace_root: &Path, + source_root: &ProfileSourceRoot, dir: &Path, files: &mut Vec, ) -> Result<()> { - if !dir.exists() { - return Ok(()); + let canonical_dir = fs::canonicalize(dir)?; + if !canonical_dir.starts_with(&source_root.canonical_path) { + return Err(profile_source_symlink_escape( + "Profile source directory resolves outside the workspace profile source root", + )); } for entry in fs::read_dir(dir)? { let entry = entry?; let path = entry.path(); let file_type = entry.file_type()?; + if file_type.is_symlink() { + return Err(profile_source_symlink_escape( + "Profile source tree entries must not be symlinks", + )); + } if file_type.is_dir() { - collect_profile_tree_files(workspace_root, &path, files)?; + collect_profile_tree_files(workspace_root, source_root, &path, files)?; } else if file_type.is_file() && path.extension().and_then(|value| value.to_str()) == Some("dcdl") { + let canonical_file = fs::canonicalize(&path)?; + if !canonical_file.starts_with(&source_root.canonical_path) + || !canonical_file.starts_with(&source_root.canonical_workspace) + { + return Err(profile_source_symlink_escape( + "Profile source file resolves outside the workspace profile source root", + )); + } let relative = path .strip_prefix(workspace_root) .map_err(|_| { @@ -1398,7 +1411,7 @@ fn validate_source_content( message: "Profile source exceeds the browser editing size limit".to_string(), }); } - checked_source_path(workspace_root, relative_path)?; + validate_source_candidate_path(workspace_root, relative_path)?; let mut sources = read_profile_source_tree_contents(workspace_root)?; sources.insert(display_source_path(relative_path), content.to_string()); let mut registry = read_registry(workspace_root).map_err(profile_registry_error)?; @@ -1639,6 +1652,120 @@ fn sha256_hex(bytes: &[u8]) -> String { out } +#[derive(Debug, Clone)] +struct ProfileSourceRoot { + path: PathBuf, + canonical_path: PathBuf, + canonical_workspace: PathBuf, +} + +fn profile_source_symlink_escape(message: impl Into) -> Error { + Error::RuntimeOperationFailed { + runtime_id: "workspace-backend".to_string(), + code: "profile_source_symlink_escape".to_string(), + message: message.into(), + } +} + +fn existing_profile_source_root(workspace_root: &Path) -> Result> { + let source_root = workspace_root.join(PROFILE_SOURCE_ROOT_RELATIVE_PATH); + let metadata = match fs::symlink_metadata(&source_root) { + Ok(metadata) => metadata, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(err) => return Err(err.into()), + }; + validate_profile_source_root_metadata(workspace_root, source_root, metadata).map(Some) +} + +fn prepare_profile_source_root_for_write(workspace_root: &Path) -> Result { + let canonical_workspace = fs::canonicalize(workspace_root)?; + let yoi_dir = workspace_root.join(".yoi"); + match fs::symlink_metadata(&yoi_dir) { + Ok(metadata) => { + if metadata.file_type().is_symlink() { + return Err(profile_source_symlink_escape( + "Workspace .yoi directory must not be a symlink", + )); + } + if !metadata.is_dir() { + return Err(profile_validation_error( + "profile_source_path_invalid", + "Workspace .yoi path is not a directory", + )); + } + let canonical_yoi = fs::canonicalize(&yoi_dir)?; + if !canonical_yoi.starts_with(&canonical_workspace) { + return Err(profile_source_symlink_escape( + "Workspace .yoi directory resolves outside the workspace root", + )); + } + } + Err(err) if err.kind() == std::io::ErrorKind::NotFound => fs::create_dir(&yoi_dir)?, + Err(err) => return Err(err.into()), + } + + let source_root = workspace_root.join(PROFILE_SOURCE_ROOT_RELATIVE_PATH); + match fs::symlink_metadata(&source_root) { + Ok(metadata) => { + validate_profile_source_root_metadata(workspace_root, source_root, metadata) + } + Err(err) if err.kind() == std::io::ErrorKind::NotFound => { + fs::create_dir(&source_root)?; + let metadata = fs::symlink_metadata(&source_root)?; + validate_profile_source_root_metadata(workspace_root, source_root, metadata) + } + Err(err) => Err(err.into()), + } +} + +fn validate_profile_source_root_metadata( + workspace_root: &Path, + source_root: PathBuf, + metadata: std::fs::Metadata, +) -> Result { + if metadata.file_type().is_symlink() { + return Err(profile_source_symlink_escape( + "Workspace profile source root must not be a symlink", + )); + } + if !metadata.is_dir() { + return Err(profile_validation_error( + "profile_source_path_invalid", + "Workspace profile source root is not a directory", + )); + } + let canonical_workspace = fs::canonicalize(workspace_root)?; + let canonical_path = fs::canonicalize(&source_root)?; + if !canonical_path.starts_with(&canonical_workspace) { + return Err(profile_source_symlink_escape( + "Workspace profile source root resolves outside the workspace root", + )); + } + Ok(ProfileSourceRoot { + path: source_root, + canonical_path, + canonical_workspace, + }) +} + +fn validate_source_candidate_path(workspace_root: &Path, relative_path: &Path) -> Result<()> { + validate_relative_source_path(relative_path).map_err(|message| { + Error::RuntimeOperationFailed { + runtime_id: "workspace-backend".to_string(), + code: "profile_source_path_escape".to_string(), + message, + } + })?; + let Some(_) = existing_profile_source_root(workspace_root)? else { + return Ok(()); + }; + let full = workspace_root.join(relative_path); + if full.exists() || full.parent().is_some_and(Path::exists) { + checked_source_path(workspace_root, relative_path)?; + } + Ok(()) +} + fn prepare_source_path_for_write(workspace_root: &Path, relative_path: &Path) -> Result { validate_relative_source_path(relative_path).map_err(|message| { Error::RuntimeOperationFailed { @@ -1647,9 +1774,7 @@ fn prepare_source_path_for_write(workspace_root: &Path, relative_path: &Path) -> message, } })?; - let source_root = workspace_root.join(PROFILE_SOURCE_ROOT_RELATIVE_PATH); - fs::create_dir_all(&source_root)?; - let canonical_root = fs::canonicalize(&source_root)?; + let source_root = prepare_profile_source_root_for_write(workspace_root)?; let full = workspace_root.join(relative_path); let parent = full.parent().ok_or_else(|| { profile_validation_error( @@ -1657,13 +1782,13 @@ fn prepare_source_path_for_write(workspace_root: &Path, relative_path: &Path) -> "Profile source path has no parent directory", ) })?; - let parent_relative = parent.strip_prefix(&source_root).map_err(|_| { + let parent_relative = parent.strip_prefix(&source_root.path).map_err(|_| { profile_validation_error( "profile_source_path_escape", "Profile source parent must remain inside the source tree", ) })?; - let mut current = source_root.clone(); + let mut current = source_root.path.clone(); for component in parent_relative.components() { let Component::Normal(name) = component else { return Err(profile_validation_error( @@ -1675,11 +1800,9 @@ fn prepare_source_path_for_write(workspace_root: &Path, relative_path: &Path) -> match fs::symlink_metadata(&next) { Ok(metadata) => { if metadata.file_type().is_symlink() { - return Err(Error::RuntimeOperationFailed { - runtime_id: "workspace-backend".to_string(), - code: "profile_source_symlink_escape".to_string(), - message: "Profile source parent contains a symlink".to_string(), - }); + return Err(profile_source_symlink_escape( + "Profile source parent contains a symlink", + )); } if !metadata.is_dir() { return Err(profile_validation_error( @@ -1688,12 +1811,12 @@ fn prepare_source_path_for_write(workspace_root: &Path, relative_path: &Path) -> )); } let canonical_next = fs::canonicalize(&next)?; - if !canonical_next.starts_with(&canonical_root) { - return Err(Error::RuntimeOperationFailed { - runtime_id: "workspace-backend".to_string(), - code: "profile_source_symlink_escape".to_string(), - message: "Profile source parent resolves outside the workspace profile source root".to_string(), - }); + if !canonical_next.starts_with(&source_root.canonical_path) + || !canonical_next.starts_with(&source_root.canonical_workspace) + { + return Err(profile_source_symlink_escape( + "Profile source parent resolves outside the workspace profile source root", + )); } } Err(err) if err.kind() == std::io::ErrorKind::NotFound => fs::create_dir(&next)?, @@ -1712,22 +1835,26 @@ fn checked_source_path(workspace_root: &Path, relative_path: &Path) -> Result