Skip to content

Accept user-owned symlinked source snapshot directory #789

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
devin-ai-integration wants to merge 2 commits into
base: main
Choose a base branch
from
+38 −2
Open

Accept user-owned symlinked source snapshot directory #789

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e8f64f1
Accept user-owned symlinked source snapshot directory (OR-617)
devin-ai-integration[bot] Oct 26, 2026
d2f0a17
Allow artifact cleanup through symlinked snapshot roots (OR-617)
devin-ai-integration[bot] Oct 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion src/compute.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ fn prepare_snapshot_dir(path: &Path) -> Result<()> {
{
use std::os::unix::fs::MetadataExt;

let metadata = std::fs::symlink_metadata(path)?;
// Follow symlinks: a user-owned link to a user-owned directory is fine.
let metadata = std::fs::metadata(path)?;

@greptile-apps greptile-apps Bot Oct 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Deletion still rejects directory links

Accepting a symlinked source-snapshots directory fixes launch, but cleanup still rejects that newly supported setup. Store::validate_run_artifact_roots uses symlink_metadata and returns “Refusing artifact cleanup through …” for the same directory. After a run finishes, users cannot delete its experiment or project, and orphan cleanup also fails.

Update the cleanup checks to safely accept the same owned directory targets, and add a test that covers launch through deletion.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/compute.rs
Line: 157

Comment:
**Deletion still rejects directory links**

Accepting a symlinked `source-snapshots` directory fixes launch, but cleanup still rejects that newly supported setup. `Store::validate_run_artifact_roots` uses `symlink_metadata` and returns “Refusing artifact cleanup through …” for the same directory. After a run finishes, users cannot delete its experiment or project, and orphan cleanup also fails.

Update the cleanup checks to safely accept the same owned directory targets, and add a test that covers launch through deletion.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@devin-ai-integration devin-ai-integration Bot Oct 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid. Fixed in d2f0a17: validate_run_artifact_roots now follows links for source-snapshots and submission-handles (the two roots prepare_snapshot_dir accepts), while other roots such as local-runs still refuse links. Added artifact_cleanup_follows_symlinked_snapshot_roots_only.

if !metadata.file_type().is_dir() || metadata.uid() != unsafe { libc::geteuid() } {
return Err(anyhow!(
"Source snapshot directory {} is not owned by the current user.",
Expand Down Expand Up @@ -1081,6 +1082,18 @@ fn not_ready(detail: impl Into<String>) -> Preflight {
pub(crate) mod tests {
use super::*;

#[cfg(unix)]
#[test]
fn snapshot_dir_accepts_a_user_owned_symlinked_directory() {
let root = std::env::temp_dir().join(format!("orx-snapshot-link-{}", uuid::Uuid::new_v4()));
let target = root.join("target");
let link = root.join("source-snapshots");
std::fs::create_dir_all(&target).unwrap();
std::os::unix::fs::symlink(&target, &link).unwrap();
prepare_snapshot_dir(&link).unwrap();
std::fs::remove_dir_all(&root).unwrap();
}

#[test]
fn run_reports_name_the_native_invoker_or_an_explicit_exception() {
let identity = |harness: &str| crate::store::InvocationIdentity {
Expand Down
25 changes: 24 additions & 1 deletion src/store.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1931,7 +1931,13 @@ impl Store {
.chain(["source-snapshots"])
{
let path = self.data_dir.join(name);
match std::fs::symlink_metadata(&path) {
// Launch accepts user-owned links for these roots (compute::prepare_snapshot_dir).
let metadata = if matches!(name, "source-snapshots" | "submission-handles") {
std::fs::metadata(&path)
} else {
std::fs::symlink_metadata(&path)
};
match metadata {
Ok(metadata) if !metadata.file_type().is_dir() => {
return Err(anyhow!(
"Refusing artifact cleanup through {}",
Expand Down Expand Up @@ -5058,6 +5064,23 @@ mod tests {
std::fs::remove_dir_all(dir).unwrap();
}

#[cfg(unix)]
#[test]
fn artifact_cleanup_follows_symlinked_snapshot_roots_only() {
let dir = std::env::temp_dir().join(format!("orx-linked-roots-{}", uuid::Uuid::new_v4()));
let store = Store::open_at(dir.clone()).unwrap();
for name in ["source-snapshots", "submission-handles"] {
std::fs::create_dir_all(dir.join(format!("outside-{name}"))).unwrap();
std::os::unix::fs::symlink(dir.join(format!("outside-{name}")), dir.join(name))
.unwrap();
}
store.validate_run_artifact_roots().unwrap();
std::fs::create_dir_all(dir.join("outside-local-runs")).unwrap();
std::os::unix::fs::symlink(dir.join("outside-local-runs"), dir.join("local-runs")).unwrap();
assert!(store.validate_run_artifact_roots().is_err());
std::fs::remove_dir_all(&dir).unwrap();
}

#[test]
fn refused_project_deletion_never_reaches_the_native_purge() {
let dir = std::env::temp_dir().join(format!("orx-delete-order-{}", uuid::Uuid::new_v4()));
Expand Down
Loading