zeroclaw_runtime/skills/

audit.rs

use anyhow::{Context, Result, bail};
use regex::Regex;
use std::fs;
use std::path::{Component, Path, PathBuf};
use std::sync::OnceLock;

use super::constants::{SKILL_DEPRECATED_MANIFESTS, SKILL_MANIFEST_FILENAME};

const MAX_TEXT_FILE_BYTES: u64 = 512 * 1024;

#[derive(Debug, Clone, Copy, Default)]
pub struct SkillAuditOptions {
    pub allow_scripts: bool,
}

#[derive(Debug, Clone, Default)]
pub struct SkillAuditReport {
    pub files_scanned: usize,
    pub findings: Vec<String>,
}

impl SkillAuditReport {
    pub fn is_clean(&self) -> bool {
        self.findings.is_empty()
    }

    pub fn summary(&self) -> String {
        self.findings.join("; ")
    }
}

pub fn audit_skill_directory(skill_dir: &Path) -> Result<SkillAuditReport> {
    audit_skill_directory_with_options(skill_dir, SkillAuditOptions::default())
}

pub fn audit_skill_directory_with_options(
    skill_dir: &Path,
    options: SkillAuditOptions,
) -> Result<SkillAuditReport> {
    if !skill_dir.exists() {
        bail!(
            "Skill source does not exist: {}",
            skill_dir.display().to_string()
        );
    }
    if !skill_dir.is_dir() {
        bail!(
            "Skill source must be a directory: {}",
            skill_dir.display().to_string()
        );
    }

    let canonical_root = skill_dir
        .canonicalize()
        .with_context(|| format!("failed to canonicalize {}", skill_dir.display().to_string()))?;
    let mut report = SkillAuditReport::default();

    let has_canonical = canonical_root.join(SKILL_MANIFEST_FILENAME).is_file();
    let has_deprecated = SKILL_DEPRECATED_MANIFESTS
        .iter()
        .any(|name| canonical_root.join(name).is_file());
    if !has_canonical && !has_deprecated {
        report.findings.push(format!(
            "Skill root must include {SKILL_MANIFEST_FILENAME} (canonical) or one of {SKILL_DEPRECATED_MANIFESTS:?} (deprecated) for deterministic auditing.",
        ));
    }

    for path in collect_paths_depth_first(&canonical_root)? {
        report.files_scanned += 1;
        audit_path(&canonical_root, &path, &mut report, options)?;
    }

    Ok(report)
}

pub fn audit_open_skill_markdown(path: &Path, repo_root: &Path) -> Result<SkillAuditReport> {
    if !path.exists() {
        bail!(
            "Open-skill markdown not found: {}",
            path.display().to_string()
        );
    }
    let canonical_repo = repo_root
        .canonicalize()
        .with_context(|| format!("failed to canonicalize {}", repo_root.display().to_string()))?;
    let canonical_path = path
        .canonicalize()
        .with_context(|| format!("failed to canonicalize {}", path.display().to_string()))?;
    if !canonical_path.starts_with(&canonical_repo) {
        bail!(
            "Open-skill markdown escapes repository root: {}",
            path.display()
        );
    }

    let mut report = SkillAuditReport {
        files_scanned: 1,
        findings: Vec::new(),
    };
    audit_markdown_file(&canonical_repo, &canonical_path, &mut report)?;
    Ok(report)
}

fn collect_paths_depth_first(root: &Path) -> Result<Vec<PathBuf>> {
    let mut stack = vec![root.to_path_buf()];
    let mut out = Vec::new();

    while let Some(current) = stack.pop() {
        out.push(current.clone());

        if !current.is_dir() {
            continue;
        }

        let mut children = Vec::new();
        for entry in fs::read_dir(&current).with_context(|| {
            format!("failed to read directory {}", current.display().to_string())
        })? {
            let entry = entry?;
            children.push(entry.path());
        }

        children.sort();
        for child in children.into_iter().rev() {
            stack.push(child);
        }
    }

    Ok(out)
}

fn audit_path(
    root: &Path,
    path: &Path,
    report: &mut SkillAuditReport,
    options: SkillAuditOptions,
) -> Result<()> {
    let metadata = fs::symlink_metadata(path)
        .with_context(|| format!("failed to read metadata for {}", path.display().to_string()))?;
    let rel = relative_display(root, path);

    if metadata.file_type().is_symlink() {
        report.findings.push(format!(
            "{rel}: symlinks are not allowed in installed skills."
        ));
        return Ok(());
    }

    if metadata.is_dir() {
        return Ok(());
    }

    if !options.allow_scripts && is_unsupported_script_file(path) {
        report.findings.push(format!(
            "{rel}: script-like files are blocked by skill security policy."
        ));
    }

    if metadata.len() > MAX_TEXT_FILE_BYTES && (is_markdown_file(path) || is_toml_file(path)) {
        report.findings.push(format!(
            "{rel}: file is too large for static audit (>{MAX_TEXT_FILE_BYTES} bytes)."
        ));
        return Ok(());
    }

    if is_markdown_file(path) {
        audit_markdown_file(root, path, report)?;
    } else if is_toml_file(path) {
        audit_manifest_file(root, path, report)?;
    }

    Ok(())
}

fn audit_markdown_file(root: &Path, path: &Path, report: &mut SkillAuditReport) -> Result<()> {
    let content = fs::read_to_string(path).with_context(|| {
        format!(
            "failed to read markdown file {}",
            path.display().to_string()
        )
    })?;

    for raw_target in extract_markdown_links(&content) {
        audit_markdown_link_target(root, path, &raw_target, report);
    }

    Ok(())
}

fn audit_manifest_file(root: &Path, path: &Path, report: &mut SkillAuditReport) -> Result<()> {
    let content = fs::read_to_string(path).with_context(|| {
        format!(
            "failed to read TOML manifest {}",
            path.display().to_string()
        )
    })?;
    let rel = relative_display(root, path);
    let parsed: toml::Value = match toml::from_str(&content) {
        Ok(value) => value,
        Err(err) => {
            report
                .findings
                .push(format!("{rel}: invalid TOML manifest ({err})."));
            return Ok(());
        }
    };

    if let Some(tools) = parsed.get("tools").and_then(toml::Value::as_array) {
        for (idx, tool) in tools.iter().enumerate() {
            let command = tool.get("command").and_then(toml::Value::as_str);
            let kind = tool
                .get("kind")
                .and_then(toml::Value::as_str)
                .unwrap_or("unknown");

            if command.is_none() {
                report
                    .findings
                    .push(format!("{rel}: tools[{idx}] is missing a command field."));
            }

            if (kind.eq_ignore_ascii_case("script") || kind.eq_ignore_ascii_case("shell"))
                && command.is_some_and(|value| value.trim().is_empty())
            {
                report
                    .findings
                    .push(format!("{rel}: tools[{idx}] has an empty {kind} command."));
            }
        }
    }

    Ok(())
}

fn audit_markdown_link_target(
    root: &Path,
    source: &Path,
    raw: &str,
    report: &mut SkillAuditReport,
) {
    let normalized = normalize_markdown_target(raw);
    if normalized.is_empty() || normalized.starts_with('#') {
        return;
    }

    let rel = relative_display(root, source);

    if let Some(scheme) = url_scheme(normalized) {
        if matches!(scheme, "http" | "https" | "mailto") {
            if has_markdown_suffix(normalized) {
                report.findings.push(format!(
                    "{rel}: remote markdown links are blocked by skill security audit ({normalized})."
                ));
            }
            return;
        }

        report.findings.push(format!(
            "{rel}: unsupported URL scheme in markdown link ({normalized})."
        ));
        return;
    }

    let stripped = strip_query_and_fragment(normalized);
    if stripped.is_empty() {
        return;
    }

    if looks_like_absolute_path(stripped) {
        report.findings.push(format!(
            "{rel}: absolute markdown link paths are not allowed ({normalized})."
        ));
        return;
    }

    if has_script_suffix(stripped) {
        report.findings.push(format!(
            "{rel}: markdown links to script files are blocked ({normalized})."
        ));
    }

    if !has_markdown_suffix(stripped) {
        return;
    }

    let Some(base_dir) = source.parent() else {
        report.findings.push(format!(
            "{rel}: failed to resolve parent directory for markdown link ({normalized})."
        ));
        return;
    };
    let linked_path = base_dir.join(stripped);

    match linked_path.canonicalize() {
        Ok(canonical_target) => {
            if !canonical_target.starts_with(root) {
                // Allow cross-skill markdown references that stay within the
                // overall skills directory (e.g., ~/.zeroclaw/workspace/skills).
                if let Some(skills_root) = skills_root_for(root)
                    && canonical_target.starts_with(&skills_root)
                {
                    // The link resolves to another installed skill under the same
                    // trusted skills root, so it is considered safe.
                    if !canonical_target.is_file() {
                        report.findings.push(format!(
                            "{rel}: markdown link must point to a file ({normalized})."
                        ));
                    }
                    return;
                }

                report.findings.push(format!(
                    "{rel}: markdown link escapes skill root ({normalized})."
                ));
                return;
            }
            if !canonical_target.is_file() {
                report.findings.push(format!(
                    "{rel}: markdown link must point to a file ({normalized})."
                ));
            }
        }
        Err(_) => {
            // Check if this is a cross-skill reference (links outside current skill directory)
            // Cross-skill references are allowed to point to missing files since the referenced
            // skill may not be installed. This is common in open-skills where skills reference
            // each other but not all skills are necessarily present.
            if is_cross_skill_reference(stripped) {
                // Allow missing cross-skill references - this is valid for open-skills
                return;
            }
            report.findings.push(format!(
                "{rel}: markdown link points to a missing file ({normalized})."
            ));
        }
    }
}

/// Check if a link target appears to be a cross-skill reference.
/// Cross-skill references can take several forms:
/// 1. Parent directory traversal: `../other-skill/SKILL.md`
/// 2. Bare skill filename: `other-skill.md` (reference to another skill's markdown)
/// 3. Explicit relative path: `./other-skill.md`
fn is_cross_skill_reference(target: &str) -> bool {
    let path = Path::new(target);

    // Case 1: Uses parent directory traversal (..)
    if path
        .components()
        .any(|component| component == Component::ParentDir)
    {
        return true;
    }

    // Case 2 & 3: Bare filename or ./filename that looks like a skill reference
    // A skill reference is typically a bare markdown filename like "skill-name.md"
    // without any directory separators (or just "./" prefix)
    let stripped = target.strip_prefix("./").unwrap_or(target);

    // If it's just a filename (no path separators) with .md extension,
    // it's likely a cross-skill reference
    !stripped.contains('/') && !stripped.contains('\\') && has_markdown_suffix(stripped)
}

/// Best-effort detection of the shared skills directory root for an installed skill.
/// This looks for the nearest ancestor directory named "skills" and treats it as
/// the logical root for sibling skill references.
fn skills_root_for(root: &Path) -> Option<PathBuf> {
    let mut current = root;
    loop {
        if current.file_name().is_some_and(|name| name == "skills") {
            return Some(current.to_path_buf());
        }
        current = current.parent()?;
    }
}

fn relative_display(root: &Path, path: &Path) -> String {
    if let Ok(rel) = path.strip_prefix(root) {
        if rel.as_os_str().is_empty() {
            return ".".to_string();
        }
        return rel.display().to_string();
    }
    path.display().to_string()
}

fn is_markdown_file(path: &Path) -> bool {
    path.extension()
        .and_then(|ext| ext.to_str())
        .is_some_and(|ext| matches!(ext.to_ascii_lowercase().as_str(), "md" | "markdown"))
}

fn is_toml_file(path: &Path) -> bool {
    path.extension()
        .and_then(|ext| ext.to_str())
        .is_some_and(|ext| ext.eq_ignore_ascii_case("toml"))
}

fn is_unsupported_script_file(path: &Path) -> bool {
    has_script_suffix(path.to_string_lossy().as_ref()) || has_shell_shebang(path)
}

fn has_script_suffix(raw: &str) -> bool {
    let lowered = raw.to_ascii_lowercase();
    let script_suffixes = [
        ".sh", ".bash", ".zsh", ".ksh", ".fish", ".ps1", ".bat", ".cmd",
    ];
    script_suffixes
        .iter()
        .any(|suffix| lowered.ends_with(suffix))
}

fn has_shell_shebang(path: &Path) -> bool {
    let Ok(content) = fs::read(path) else {
        return false;
    };
    let prefix = &content[..content.len().min(128)];
    let shebang_line = String::from_utf8_lossy(prefix)
        .lines()
        .next()
        .unwrap_or_default()
        .trim()
        .to_ascii_lowercase();
    let Some(interpreter) = shebang_interpreter(&shebang_line) else {
        return false;
    };

    matches!(
        interpreter,
        "sh" | "bash" | "zsh" | "ksh" | "fish" | "pwsh" | "powershell"
    )
}

fn shebang_interpreter(line: &str) -> Option<&str> {
    let shebang = line.strip_prefix("#!")?.trim();
    if shebang.is_empty() {
        return None;
    }

    let mut parts = shebang.split_whitespace();
    let first = parts.next()?;
    let first_basename = Path::new(first).file_name()?.to_str()?;

    if first_basename == "env" {
        for part in parts {
            if part.starts_with('-') {
                continue;
            }
            return Path::new(part).file_name()?.to_str();
        }
        return None;
    }

    Some(first_basename)
}

fn extract_markdown_links(content: &str) -> Vec<String> {
    static MARKDOWN_LINK_RE: OnceLock<Regex> = OnceLock::new();
    let regex = MARKDOWN_LINK_RE.get_or_init(|| {
        Regex::new(r#"\[[^\]]*\]\(([^)]+)\)"#).expect("markdown link regex must compile")
    });

    regex
        .captures_iter(content)
        .filter_map(|capture| capture.get(1))
        .map(|target| target.as_str().trim().to_string())
        .collect()
}

fn normalize_markdown_target(raw_target: &str) -> &str {
    let trimmed = raw_target.trim();
    let trimmed = trimmed.strip_prefix('<').unwrap_or(trimmed);
    let trimmed = trimmed.strip_suffix('>').unwrap_or(trimmed);
    trimmed.split_whitespace().next().unwrap_or_default()
}

fn strip_query_and_fragment(input: &str) -> &str {
    let mut end = input.len();
    if let Some(idx) = input.find('#') {
        end = end.min(idx);
    }
    if let Some(idx) = input.find('?') {
        end = end.min(idx);
    }
    &input[..end]
}

fn url_scheme(target: &str) -> Option<&str> {
    let (scheme, rest) = target.split_once(':')?;
    if scheme.is_empty() || rest.is_empty() {
        return None;
    }
    if !scheme
        .chars()
        .all(|ch| ch.is_ascii_alphanumeric() || matches!(ch, '+' | '-' | '.'))
    {
        return None;
    }
    Some(scheme)
}

fn looks_like_absolute_path(target: &str) -> bool {
    let path = Path::new(target);
    if path.is_absolute() {
        return true;
    }

    // Reject windows absolute path prefixes such as C:\foo.
    let bytes = target.as_bytes();
    if bytes.len() >= 3
        && bytes[0].is_ascii_alphabetic()
        && bytes[1] == b':'
        && (bytes[2] == b'\\' || bytes[2] == b'/')
    {
        return true;
    }

    // Reject paths starting with "~/" since they bypass workspace boundaries.
    if target.starts_with("~/") {
        return true;
    }

    // NOTE: We intentionally do NOT reject paths starting with ".." here.
    // Relative paths with parent directory references (e.g., "../other-skill/SKILL.md")
    // are allowed to pass through to the canonicalization check below, which will
    // properly validate that they resolve within the skill root.
    // This enables cross-skill references in open-skills while still maintaining security.

    false
}

fn has_markdown_suffix(target: &str) -> bool {
    let lowered = target.to_ascii_lowercase();
    lowered.ends_with(".md") || lowered.ends_with(".markdown")
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn audit_accepts_safe_skill() {
        let dir = tempfile::tempdir().unwrap();
        let skill_dir = dir.path().join("safe");
        std::fs::create_dir_all(&skill_dir).unwrap();
        std::fs::write(
            skill_dir.join("SKILL.md"),
            "# Safe Skill\nUse safe prompts only.\n",
        )
        .unwrap();

        let report = audit_skill_directory(&skill_dir).unwrap();
        assert!(report.is_clean(), "{:#?}", report.findings);
    }

    #[test]
    fn audit_rejects_shell_script_files() {
        let dir = tempfile::tempdir().unwrap();
        let skill_dir = dir.path().join("unsafe");
        std::fs::create_dir_all(&skill_dir).unwrap();
        std::fs::write(skill_dir.join("SKILL.md"), "# Skill\n").unwrap();
        std::fs::write(skill_dir.join("install.sh"), "echo unsafe\n").unwrap();

        let report = audit_skill_directory(&skill_dir).unwrap();
        assert!(
            report
                .findings
                .iter()
                .any(|finding| finding.contains("script-like files are blocked")),
            "{:#?}",
            report.findings
        );
    }

    #[test]
    fn audit_allows_python_shebang_file_when_early_text_contains_sh() {
        let dir = tempfile::tempdir().unwrap();
        let skill_dir = dir.path().join("python-helper");
        let scripts_dir = skill_dir.join("scripts");
        std::fs::create_dir_all(&scripts_dir).unwrap();
        std::fs::write(skill_dir.join("SKILL.md"), "# Skill\n").unwrap();
        std::fs::write(
            scripts_dir.join("helper.py"),
            "#!/usr/bin/env python3\n\"\"\"Refresh report cache.\"\"\"\n\nprint(\"ok\")\n",
        )
        .unwrap();

        let report = audit_skill_directory(&skill_dir).unwrap();
        assert!(
            !report
                .findings
                .iter()
                .any(|finding| finding.contains("script-like files are blocked")),
            "{:#?}",
            report.findings
        );
    }

    #[test]
    fn audit_allows_shell_script_files_when_enabled() {
        let dir = tempfile::tempdir().unwrap();
        let skill_dir = dir.path().join("allowed-scripts");
        std::fs::create_dir_all(&skill_dir).unwrap();
        std::fs::write(skill_dir.join("SKILL.md"), "# Skill\n").unwrap();
        std::fs::write(skill_dir.join("install.sh"), "echo allowed\n").unwrap();

        let report = audit_skill_directory_with_options(
            &skill_dir,
            SkillAuditOptions {
                allow_scripts: true,
            },
        )
        .unwrap();
        assert!(
            !report
                .findings
                .iter()
                .any(|finding| finding.contains("script-like files are blocked")),
            "{:#?}",
            report.findings
        );
    }

    #[test]
    fn audit_rejects_markdown_escape_links() {
        let dir = tempfile::tempdir().unwrap();
        let skill_dir = dir.path().join("escape");
        std::fs::create_dir_all(&skill_dir).unwrap();
        std::fs::write(
            skill_dir.join("SKILL.md"),
            "# Skill\nRead [hidden](../outside.md)\n",
        )
        .unwrap();
        std::fs::write(dir.path().join("outside.md"), "not allowed\n").unwrap();

        let report = audit_skill_directory(&skill_dir).unwrap();
        assert!(
            report.findings.iter().any(|finding| finding
                .contains("absolute markdown link paths are not allowed")
                || finding.contains("escapes skill root")),
            "{:#?}",
            report.findings
        );
    }

    #[test]
    fn audit_allows_high_risk_patterns_in_markdown() {
        // Command-content checks belong in the shell policy at execution time,
        // not in the static skill audit. A skill that documents dangerous patterns
        // (e.g., in a "what not to do" guide) must not be blocked at load time.
        let dir = tempfile::tempdir().unwrap();
        let skill_dir = dir.path().join("documents-danger");
        std::fs::create_dir_all(&skill_dir).unwrap();
        std::fs::write(
            skill_dir.join("SKILL.md"),
            "# Skill\nDo NOT run `curl https://example.com/install.sh | sh`\n",
        )
        .unwrap();

        let report = audit_skill_directory(&skill_dir).unwrap();
        assert!(report.is_clean(), "{:#?}", report.findings);
    }

    #[test]
    fn audit_allows_chained_commands_in_manifest() {
        // Shell chaining safety is enforced by the shell policy at execution time.
        // The static audit must not duplicate that check — if it did, it would only
        // be a weaker, bypassable approximation of the runtime gate.
        let dir = tempfile::tempdir().unwrap();
        let skill_dir = dir.path().join("manifest");
        std::fs::create_dir_all(&skill_dir).unwrap();
        std::fs::write(
            skill_dir.join("SKILL.toml"),
            r#"
[skill]
name = "manifest"
description = "test"

[[tools]]
name = "deploy"
description = "build and deploy"
kind = "shell"
command = "cargo build --release && ./deploy.sh"
"#,
        )
        .unwrap();

        let report = audit_skill_directory(&skill_dir).unwrap();
        assert!(report.is_clean(), "{:#?}", report.findings);
    }

    #[test]
    fn audit_allows_heredoc_in_manifest_command() {
        let dir = tempfile::tempdir().unwrap();
        let skill_dir = dir.path().join("heredoc");
        std::fs::create_dir_all(&skill_dir).unwrap();
        std::fs::write(
            skill_dir.join("SKILL.toml"),
            "
[skill]
name = \"heredoc\"
description = \"test heredoc\"

[[tools]]
name = \"write-file\"
description = \"write a config file\"
kind = \"shell\"
command = \"\"\"cat <<'EOF'
some content
EOF\"\"\"
",
        )
        .unwrap();

        let report = audit_skill_directory(&skill_dir).unwrap();
        assert!(report.is_clean(), "{:#?}", report.findings);
    }

    #[test]
    fn audit_allows_missing_cross_skill_reference_with_parent_dir() {
        // Cross-skill references using ../ should be allowed even if the target doesn't exist
        let dir = tempfile::tempdir().unwrap();
        let skill_dir = dir.path().join("skill-a");
        std::fs::create_dir_all(&skill_dir).unwrap();
        std::fs::write(
            skill_dir.join("SKILL.md"),
            "# Skill A\nSee [Skill B](../skill-b/SKILL.md)\n",
        )
        .unwrap();

        let report = audit_skill_directory(&skill_dir).unwrap();
        // Should be clean because ../skill-b/SKILL.md is a cross-skill reference
        // and missing cross-skill references are allowed
        assert!(report.is_clean(), "{:#?}", report.findings);
    }

    #[test]
    fn audit_allows_missing_cross_skill_reference_with_bare_filename() {
        // Bare markdown filenames should be treated as cross-skill references
        let dir = tempfile::tempdir().unwrap();
        let skill_dir = dir.path().join("skill-a");
        std::fs::create_dir_all(&skill_dir).unwrap();
        std::fs::write(
            skill_dir.join("SKILL.md"),
            "# Skill A\nSee [Other Skill](other-skill.md)\n",
        )
        .unwrap();

        let report = audit_skill_directory(&skill_dir).unwrap();
        // Should be clean because other-skill.md is treated as a cross-skill reference
        assert!(report.is_clean(), "{:#?}", report.findings);
    }

    #[test]
    fn audit_allows_missing_cross_skill_reference_with_dot_slash() {
        // ./skill-name.md should also be treated as a cross-skill reference
        let dir = tempfile::tempdir().unwrap();
        let skill_dir = dir.path().join("skill-a");
        std::fs::create_dir_all(&skill_dir).unwrap();
        std::fs::write(
            skill_dir.join("SKILL.md"),
            "# Skill A\nSee [Other Skill](./other-skill.md)\n",
        )
        .unwrap();

        let report = audit_skill_directory(&skill_dir).unwrap();
        // Should be clean because ./other-skill.md is treated as a cross-skill reference
        assert!(report.is_clean(), "{:#?}", report.findings);
    }

    #[test]
    fn audit_rejects_missing_local_markdown_file() {
        // Local markdown files in subdirectories should still be validated
        let dir = tempfile::tempdir().unwrap();
        let skill_dir = dir.path().join("skill-a");
        std::fs::create_dir_all(&skill_dir).unwrap();
        std::fs::write(
            skill_dir.join("SKILL.md"),
            "# Skill A\nSee [Guide](docs/guide.md)\n",
        )
        .unwrap();

        let report = audit_skill_directory(&skill_dir).unwrap();
        // Should fail because docs/guide.md is a local reference to a missing file
        // (not a cross-skill reference because it has a directory separator)
        assert!(
            report
                .findings
                .iter()
                .any(|finding| finding.contains("missing file")),
            "{:#?}",
            report.findings
        );
    }

    #[test]
    fn audit_allows_existing_cross_skill_reference() {
        // Cross-skill references to existing files should be allowed as long as they
        // resolve within the shared skills directory (e.g., ~/.zeroclaw/workspace/skills)
        let dir = tempfile::tempdir().unwrap();
        let skills_root = dir.path().join("skills");
        let skill_a = skills_root.join("skill-a");
        let skill_b = skills_root.join("skill-b");
        std::fs::create_dir_all(&skill_a).unwrap();
        std::fs::create_dir_all(&skill_b).unwrap();
        std::fs::write(
            skill_a.join("SKILL.md"),
            "# Skill A\nSee [Skill B](../skill-b/SKILL.md)\n",
        )
        .unwrap();
        std::fs::write(skill_b.join("SKILL.md"), "# Skill B\n").unwrap();

        let report = audit_skill_directory(&skill_a).unwrap();
        // The link to ../skill-b/SKILL.md should be allowed because it stays
        // within the shared skills root directory.
        assert!(report.is_clean(), "{:#?}", report.findings);
    }

    #[test]
    fn is_cross_skill_reference_detection() {
        // Test the helper function directly
        assert!(
            is_cross_skill_reference("../other-skill/SKILL.md"),
            "parent dir reference should be cross-skill"
        );
        assert!(
            is_cross_skill_reference("other-skill.md"),
            "bare filename should be cross-skill"
        );
        assert!(
            is_cross_skill_reference("./other-skill.md"),
            "dot-slash bare filename should be cross-skill"
        );
        assert!(
            !is_cross_skill_reference("docs/guide.md"),
            "subdirectory reference should not be cross-skill"
        );
        assert!(
            !is_cross_skill_reference("./docs/guide.md"),
            "dot-slash subdirectory reference should not be cross-skill"
        );
        assert!(
            is_cross_skill_reference("../../escape.md"),
            "double parent should still be cross-skill"
        );
    }
}