Skip to content

Commit 1a0e1f1

Browse files
authored
Merge pull request #25 from A3S-Lab/fix/srt-workspace-scan-race-20260723
fix(srt): tolerate concurrent workspace removals
2 parents 7038d99 + d72d2d2 commit 1a0e1f1

4 files changed

Lines changed: 151 additions & 15 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Kept SRT workspace policy scans stable when another process removes an
13+
enumerated file or directory concurrently, while preserving fail-closed
14+
behavior for permission and other I/O failures.
15+
1016
## [6.4.0] - 2026-07-22
1117

1218
### Added

README.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -324,9 +324,11 @@ it exits; timeout or cancellation terminates the Unix process group. It never
324324
searches `PATH` for a sandbox runtime and never falls back to the host runner
325325
when its explicitly provisioned runtime is missing or fails. Write-deny paths
326326
already covered by a protected ancestor are collapsed before SRT startup,
327-
while more-specific credential read denies remain intact. The embedding host
328-
remains responsible for choosing whether an unavailable sandbox causes an
329-
interactive escalation or a deterministic denial.
327+
while more-specific credential read denies remain intact. Workspace policy
328+
scans treat an entry removed concurrently after enumeration as absent, but
329+
permission and other I/O failures remain fatal. The embedding host remains
330+
responsible for choosing whether an unavailable sandbox causes an interactive
331+
escalation or a deterministic denial.
330332

331333
Shell isolation does not automatically govern in-process workspace tools.
332334
Interactive hosts should construct `LocalWorkspaceBackend` or

core/src/sandbox/srt.rs

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -870,17 +870,27 @@ fn workspace_nested_env_paths(workspace: &Path) -> Result<Vec<PathBuf>> {
870870
let mut paths = Vec::new();
871871

872872
while let Some((directory, depth)) = pending.pop() {
873-
let entries = std::fs::read_dir(&directory)
874-
.with_context(|| format!("failed to scan SRT workspace {}", directory.display()))?;
873+
let Some(entries) = workspace_scan_result(std::fs::read_dir(&directory), || {
874+
format!("failed to scan SRT workspace {}", directory.display())
875+
})?
876+
else {
877+
continue;
878+
};
875879
for entry in entries {
876-
let entry = entry.with_context(|| {
880+
let Some(entry) = workspace_scan_result(entry, || {
877881
format!("failed to enumerate SRT workspace {}", directory.display())
878-
})?;
882+
})?
883+
else {
884+
continue;
885+
};
879886
scanned = next_workspace_scan_entry(scanned)?;
880887
let path = entry.path();
881-
let file_type = entry.file_type().with_context(|| {
888+
let Some(file_type) = workspace_scan_result(entry.file_type(), || {
882889
format!("failed to inspect SRT workspace path {}", path.display())
883-
})?;
890+
})?
891+
else {
892+
continue;
893+
};
884894
if entry
885895
.file_name()
886896
.to_str()
@@ -906,18 +916,28 @@ pub(crate) fn workspace_hardlink_paths(workspace: &Path) -> Result<Vec<PathBuf>>
906916
let mut hardlinks = Vec::new();
907917

908918
while let Some((directory, depth)) = pending.pop() {
909-
let entries = std::fs::read_dir(&directory)
910-
.with_context(|| format!("failed to scan SRT workspace {}", directory.display()))?;
919+
let Some(entries) = workspace_scan_result(std::fs::read_dir(&directory), || {
920+
format!("failed to scan SRT workspace {}", directory.display())
921+
})?
922+
else {
923+
continue;
924+
};
911925
for entry in entries {
912-
let entry = entry.with_context(|| {
926+
let Some(entry) = workspace_scan_result(entry, || {
913927
format!("failed to enumerate SRT workspace {}", directory.display())
914-
})?;
928+
})?
929+
else {
930+
continue;
931+
};
915932
scanned = next_workspace_scan_entry(scanned)?;
916933

917934
let path = entry.path();
918-
let metadata = std::fs::symlink_metadata(&path).with_context(|| {
935+
let Some(metadata) = workspace_scan_result(std::fs::symlink_metadata(&path), || {
919936
format!("failed to inspect SRT workspace path {}", path.display())
920-
})?;
937+
})?
938+
else {
939+
continue;
940+
};
921941
if metadata.file_type().is_symlink() {
922942
continue;
923943
}
@@ -945,6 +965,17 @@ pub(crate) fn workspace_hardlink_paths(workspace: &Path) -> Result<Vec<PathBuf>>
945965
Ok(hardlinks)
946966
}
947967

968+
fn workspace_scan_result<T>(
969+
result: std::io::Result<T>,
970+
context: impl FnOnce() -> String,
971+
) -> Result<Option<T>> {
972+
match result {
973+
Ok(value) => Ok(Some(value)),
974+
Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(None),
975+
Err(error) => Err(error).with_context(context),
976+
}
977+
}
978+
948979
fn next_workspace_scan_entry(scanned: usize) -> Result<usize> {
949980
let scanned = scanned
950981
.checked_add(1)

core/src/sandbox/srt/tests.rs

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,39 @@ async fn adapter_denies_every_preexisting_workspace_hardlink_alias() {
190190
}
191191
}
192192

193+
#[test]
194+
fn workspace_scan_treats_a_concurrently_removed_entry_as_absent() {
195+
let workspace = tempfile::tempdir().unwrap();
196+
let transient = workspace.path().join("transient");
197+
std::fs::write(&transient, "temporary").unwrap();
198+
std::fs::remove_file(&transient).unwrap();
199+
200+
let metadata = workspace_scan_result(std::fs::symlink_metadata(&transient), || {
201+
format!("failed to inspect {}", transient.display())
202+
})
203+
.unwrap();
204+
205+
assert!(metadata.is_none());
206+
}
207+
208+
#[test]
209+
fn workspace_scan_keeps_non_missing_io_failures_fatal() {
210+
let error = workspace_scan_result::<()>(
211+
Err(std::io::Error::from(std::io::ErrorKind::PermissionDenied)),
212+
|| "failed to inspect protected entry".to_string(),
213+
)
214+
.unwrap_err();
215+
216+
assert_eq!(error.to_string(), "failed to inspect protected entry");
217+
assert_eq!(
218+
error
219+
.source()
220+
.and_then(|source| source.downcast_ref::<std::io::Error>())
221+
.map(std::io::Error::kind),
222+
Some(std::io::ErrorKind::PermissionDenied)
223+
);
224+
}
225+
193226
#[cfg(unix)]
194227
#[tokio::test]
195228
async fn adapter_preserves_stdout_and_stderr_as_separate_streams() {
@@ -420,6 +453,70 @@ async fn real_srt_allows_workspace_writes_and_blocks_outside_writes() {
420453
}
421454
}
422455

456+
/// Run explicitly with `A3S_TEST_SRT_BIN=/absolute/path/to/cli.js` and
457+
/// `A3S_TEST_SRT_NODE=/absolute/path/to/node`.
458+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
459+
#[ignore = "requires an installed srt runtime and Node.js"]
460+
async fn real_srt_probe_survives_concurrent_workspace_churn() {
461+
use std::sync::{
462+
atomic::{AtomicBool, Ordering},
463+
Arc,
464+
};
465+
466+
let binary = std::env::var_os("A3S_TEST_SRT_BIN")
467+
.map(PathBuf::from)
468+
.expect("set A3S_TEST_SRT_BIN");
469+
let node = std::env::var_os("A3S_TEST_SRT_NODE")
470+
.map(PathBuf::from)
471+
.expect("set A3S_TEST_SRT_NODE");
472+
let workspace = tempfile::tempdir().unwrap();
473+
let churn_root = workspace.path().join("concurrent-writer");
474+
std::fs::create_dir_all(&churn_root).unwrap();
475+
476+
let running = Arc::new(AtomicBool::new(true));
477+
let writer_running = Arc::clone(&running);
478+
let writer = std::thread::spawn(move || {
479+
let mut generation = 0usize;
480+
while writer_running.load(Ordering::Acquire) {
481+
let batch = churn_root.join(format!("batch-{}", generation % 4));
482+
let nested = batch.join("nested");
483+
let _ = std::fs::remove_dir_all(&batch);
484+
if std::fs::create_dir_all(&nested).is_ok() {
485+
for index in 0..4 {
486+
let _ =
487+
std::fs::write(nested.join(format!(".env-{index}")), b"TRANSIENT=removed");
488+
}
489+
}
490+
let _ = std::fs::remove_dir_all(&batch);
491+
generation = generation.wrapping_add(1);
492+
}
493+
});
494+
495+
let result = async {
496+
for _ in 0..50 {
497+
let sandbox =
498+
SrtBashSandbox::from_verified_npm_with_node(&binary, &node, workspace.path())?;
499+
let output = sandbox
500+
.exec_command("printf a3s-managed-srt-ready", "/workspace")
501+
.await?;
502+
if output.exit_code != 0 || output.stdout != "a3s-managed-srt-ready" {
503+
bail!(
504+
"SRT capability probe failed with code {}: {}{}",
505+
output.exit_code,
506+
output.stdout,
507+
output.stderr
508+
);
509+
}
510+
}
511+
Ok::<(), anyhow::Error>(())
512+
}
513+
.await;
514+
515+
running.store(false, Ordering::Release);
516+
writer.join().unwrap();
517+
result.unwrap();
518+
}
519+
423520
/// Run explicitly with `A3S_TEST_SRT_BIN=/absolute/path/to/srt`.
424521
#[tokio::test]
425522
#[ignore = "requires an installed srt runtime"]

0 commit comments

Comments
 (0)