fix(terminal): validate fnm PATH resolution against symlink escape

[JanTvrdik]

Summary

• Canonicalize the fnm alias target and verify it stays within the fnm directory before adding it to PATH
• Prevents a symlink at ~/.local/share/fnm/aliases/default from injecting an attacker-controlled directory into PATH
• Logs a warning when a symlink escape attempt is detected

Closes #77

Test plan

• cargo build succeeds
• cargo test passes (32 tests)
• Manual: verify fnm Node resolution still works with a valid default alias
• Manual: verify a symlink pointing outside fnm dir is rejected with a log warning

Co-Authored-By: Claude Code

[Copilot]

Pull request overview

Hardens get_extended_path()’s fnm integration by preventing ~/.local/share/fnm/aliases/default from resolving to a path outside the fnm directory (mitigating PATH-hijack via symlink escape, per #77).

Changes:

• Canonicalizes fnm_dir and the computed Node installation/bin path before adding it to PATH.
• Skips adding the path (and logs a warning) if the canonicalized bin path is outside the canonical fnm directory.

Comments suppressed due to low confidence (2)

crates/okena-terminal/src/session_backend.rs:706

• node_bin.canonicalize() succeeds for non-directory paths too, so this can add a file path to PATH (regression vs the previous node_bin.is_dir() guard). Add an explicit directory check (e.g., canonical_bin.is_dir()) before inserting into result/seen.

        // Validate the resolved path stays within fnm directory to prevent symlink escape
        if let Ok(canonical_bin) = node_bin.canonicalize() {
            if !canonical_bin.starts_with(&fnm_canonical) {
                log::warn!("fnm alias points outside fnm directory, skipping: {:?}", node_bin);
                return;
            }
            if let Some(s) = canonical_bin.to_str() {
                if seen.insert(s.to_string()) {
                    result.push(s.to_string());
                }
            }
        }

crates/okena-terminal/src/session_backend.rs:706

• This change adds security-critical behavior (accept valid alias paths within fnm_dir, reject symlink escapes, and emit a warning). There are existing unit tests in this file, but none cover resolve_fnm_path; please add Unix-only tests using a temp dir + symlinks to assert (1) a normal alias adds the expected bin dir and (2) an alias that resolves outside fnm_dir is skipped.

    let fnm_canonical = match fnm_dir.canonicalize() {
        Ok(p) => p,
        Err(_) => return,
    };
    // fnm aliases: default → specific version
    let default_alias = fnm_dir.join("aliases/default");
    if let Ok(version) = std::fs::read_link(&default_alias)
        .or_else(|_| std::fs::read_to_string(&default_alias).map(std::path::PathBuf::from))
    {
        // version is either an absolute path or just a version string like "v22.14.0"
        let node_bin = if version.is_absolute() {
            version.join("installation/bin")
        } else {
            fnm_dir.join("node-versions").join(version.to_string_lossy().trim()).join("installation/bin")
        };
        // Validate the resolved path stays within fnm directory to prevent symlink escape
        if let Ok(canonical_bin) = node_bin.canonicalize() {
            if !canonical_bin.starts_with(&fnm_canonical) {
                log::warn!("fnm alias points outside fnm directory, skipping: {:?}", node_bin);
                return;
            }
            if let Some(s) = canonical_bin.to_str() {
                if seen.insert(s.to_string()) {
                    result.push(s.to_string());
                }
            }
        }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The warning logs node_bin (pre-canonicalization), which may be relative and make it hard to see where it actually escaped to. Consider logging the canonicalized path (and/or both) so the warning is actionable when diagnosing PATH issues.

Suggested change

	log::warn!("fnm alias points outside fnm directory, skipping: {:?}", node_bin);
	log::warn!(
	"fnm alias points outside fnm directory, skipping: canonical={:?}, original={:?}",
	canonical_bin,
	node_bin
	);