tkmind/tkmind_go
1
0
Fork 0
Code Issues Pull Requests Actions 5 Packages Projects Releases Wiki Activity

fix(checks): reject symlinked check sources (#11489)

Browse Source
This commit is contained in:
Jasper
2026-08-24 16:51:01 +00:00
committed by GitHub
parent bcde80a63b
commit ce0bc7c567
2 changed files with 168 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/goose/src/checks/mod.rs
+137 -9
View File
@@ -74,7 +74,13 @@ pub struct Check {
impl Check {
/// Read and parse a check file from disk.
pub fn from_path(path: &Path) -> Result<Self> {
let content = fs::read_to_string(path)
let parent = path
.parent()
.ok_or_else(|| anyhow!("check {}: invalid path", path.display()))?;
let file_name = path
.file_name()
.ok_or_else(|| anyhow!("check {}: invalid filename", path.display()))?;
let content = crate::skills::read_source_file(parent, Path::new(file_name))
.with_context(|| format!("read check file: {}", path.display()))?;
Self::parse(&content, path)
}
@@ -345,19 +351,28 @@ pub fn discover_with_globals(
};
for dir in global_dirs {
for check in read_checks_dir(dir, "", LoadMode::Lenient)? {
let Ok(canonical_dir) = dir.canonicalize() else {
continue;
};
for mut check in read_checks_dir(&canonical_dir, &canonical_dir, "", LoadMode::Lenient)? {
if let Some(file_name) = check.path.file_name().map(ToOwned::to_owned) {
check.path = dir.join(file_name);
}
record(check, 0);
}
}
let root_dir = repo_root.join(".agents").join("checks");
for check in read_checks_dir(&root_dir, "", LoadMode::Strict)? {
let root_dir = canonical_repo_root.join(".agents").join("checks");
for check in read_checks_dir(&canonical_repo_root, &root_dir, "", LoadMode::Strict)? {
record(check, scope_priority(""));
}
for scope in &scope_dirs {
let dir = repo_root.join(scope).join(".agents").join("checks");
for check in read_checks_dir(&dir, scope, LoadMode::Strict)? {
let dir = canonical_repo_root
.join(scope)
.join(".agents")
.join("checks");
for check in read_checks_dir(&canonical_repo_root, &dir, scope, LoadMode::Strict)? {
let p = scope_priority(scope);
record(check, p);
}
@@ -393,17 +408,30 @@ enum LoadMode {
Lenient,
}
fn read_checks_dir(dir: &Path, scope_dir: &str, mode: LoadMode) -> Result<Vec<Check>> {
if !dir.is_dir() {
fn read_checks_dir(
scan_root: &Path,
dir: &Path,
scope_dir: &str,
mode: LoadMode,
) -> Result<Vec<Check>> {
if !checks_dir_is_unlinked(scan_root, dir)? {
return Ok(Vec::new());
}
match fs::symlink_metadata(dir) {
Ok(metadata) if metadata.is_dir() => {}
Ok(_) => return Ok(Vec::new()),
Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()),
Err(error) => {
return Err(error).with_context(|| format!("inspect checks dir {}", dir.display()))
}
}
let mut out = Vec::new();
let entries =
fs::read_dir(dir).with_context(|| format!("read checks dir {}", dir.display()))?;
for entry in entries {
let entry = entry?;
let path = entry.path();
if !path.is_file() {
if !entry.file_type()?.is_file() {
continue;
}
if path.extension().and_then(|s| s.to_str()) != Some("md") {
@@ -434,6 +462,30 @@ fn read_checks_dir(dir: &Path, scope_dir: &str, mode: LoadMode) -> Result<Vec<Ch
Ok(out)
}
fn checks_dir_is_unlinked(scan_root: &Path, dir: &Path) -> Result<bool> {
let relative = dir
.strip_prefix(scan_root)
.with_context(|| format!("checks dir {} is outside scan root", dir.display()))?;
let mut current = scan_root.to_path_buf();
for component in relative.components() {
let std::path::Component::Normal(component) = component else {
return Ok(false);
};
current.push(component);
match fs::symlink_metadata(&current) {
Ok(metadata) if metadata.file_type().is_symlink() => return Ok(false),
Ok(metadata) if metadata.is_dir() => {}
Ok(_) => return Ok(false),
Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(false),
Err(error) => {
return Err(error)
.with_context(|| format!("inspect checks dir component {}", current.display()))
}
}
}
Ok(true)
}
/// Priority of a scope for shadowing checks/overrides. Higher = closer.
fn scope_priority(scope_dir: &str) -> usize {
if scope_dir.is_empty() {
@@ -722,6 +774,29 @@ tools: [Bash, Read, Grep]
assert_eq!(result.checks[0].description.as_deref(), Some("repo"));
}
#[cfg(unix)]
#[test]
fn allows_symlinked_global_check_directories() {
use std::os::unix::fs::symlink;
let dir = tempdir().unwrap();
let root = dir.path().join("repo");
fs::create_dir_all(&root).unwrap();
let global = dir.path().join("global");
write(
&global.join("perf.md"),
"---\nname: perf\ndescription: global\n---\nglobal body",
);
let global_link = dir.path().join("global-link");
symlink(&global, &global_link).unwrap();
let result = discover_with_globals(&root, &[], std::slice::from_ref(&global_link)).unwrap();
assert_eq!(result.checks.len(), 1);
assert_eq!(result.checks[0].body, "global body");
assert_eq!(result.checks[0].path, global_link.join("perf.md"));
}
#[test]
fn user_check_named_repo_rules_is_not_overwritten_by_root_review_md() {
let dir = tempdir().unwrap();
@@ -754,6 +829,59 @@ tools: [Bash, Read, Grep]
assert_eq!(result.checks.len(), 1);
}
#[cfg(unix)]
#[test]
fn skips_symlinked_check_directories() {
use std::os::unix::fs::symlink;
let dir = tempdir().unwrap();
let root = dir.path().join("repo");
let external_checks = dir.path().join("external-checks");
write(
&external_checks.join("secret.md"),
"---\nname: secret\n---\nexternal body",
);
fs::create_dir_all(root.join(".agents")).unwrap();
symlink(&external_checks, root.join(".agents/checks")).unwrap();
let result = discover_with_globals(&root, &[], &[]).unwrap();
assert!(result.checks.is_empty());
}
#[cfg(unix)]
#[test]
fn skips_checks_behind_symlinked_agent_directory() {
use std::os::unix::fs::symlink;
let dir = tempdir().unwrap();
let root = dir.path().join("repo");
let external_agents = dir.path().join("external-agents");
write(
&external_agents.join("checks/secret.md"),
"---\nname: secret\n---\nexternal body",
);
fs::create_dir_all(&root).unwrap();
symlink(&external_agents, root.join(".agents")).unwrap();
let result = discover_with_globals(&root, &[], &[]).unwrap();
assert!(result.checks.is_empty());
}
#[test]
fn strict_project_check_read_failures_are_not_skipped() {
let dir = tempdir().unwrap();
let root = dir.path().join("repo");
let check = root.join(".agents/checks/oversized.md");
let oversized = "x".repeat(crate::scheduler::MAX_SCHEDULE_RECIPE_BYTES as usize + 1);
write(&check, &oversized);
let error = discover_with_globals(&root, &[], &[]).unwrap_err();
assert!(error.to_string().contains("read check file"));
}
#[cfg(unix)]
#[test]
fn skips_review_md_symlinks_outside_repo() {
crates/goose/src/sources.rs
+31
View File
@@ -1835,6 +1835,37 @@ mod tests {
);
}
#[cfg(unix)]
#[test]
fn list_agent_sources_rejects_symlinked_review_check_outside_project() {
use std::os::unix::fs::symlink;
let tmp = TempDir::new().unwrap();
let project = tmp.path().join("project");
let checks_dir = project.join(".agents").join("checks");
std::fs::create_dir_all(&checks_dir).unwrap();
let private_check = tmp.path().join("private").join("secret-check.md");
std::fs::create_dir_all(private_check.parent().unwrap()).unwrap();
std::fs::write(
&private_check,
"---\nname: secret-check\ndescription: hidden\n---\nTOP_SECRET_CHECK",
)
.unwrap();
symlink(&private_check, checks_dir.join("secret-check.md")).unwrap();
let listed = list_sources(
Some(SourceType::Agent),
Some(project.to_str().unwrap()),
false,
)
.unwrap();
assert!(!listed.iter().any(|source| {
source.name == "secret-check" && source.content.contains("TOP_SECRET_CHECK")
}));
}
#[test]
fn update_rejects_path_traversal() {
let tmp = TempDir::new().unwrap();