Skip to content

Commit 74191cd

Browse files
committed
fix(tools): preserve rss scan accounting
Charge fs_list examination slots for non-UTF8 and vanished dirents, increment dirs_visited at walk entry before file-cap checks, and decode nested clock overflow codes instead of collapsing them to cancelled.
1 parent 1ca8b23 commit 74191cd

4 files changed

Lines changed: 260 additions & 8 deletions

File tree

rss/tools/search_files.rss

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -698,8 +698,8 @@ fn observe_limits(state: map) -> map {
698698
let tick: map = cap::clock_monotonic_ms(types::map_string(state, "token", ""));
699699
if map_bool(tick, "ok", false) == false {
700700
state.fatal = true;
701-
state.fatal_code = types::map_string(tick, "code", "cancelled");
702-
state.fatal_message = types::map_string(tick, "message", "clock failed");
701+
state.fatal_code = types::map_string(types::map_map(tick, "error"), "code", "internal_error");
702+
state.fatal_message = types::map_string(types::map_map(tick, "error"), "message", "capability failed");
703703
state.stop = true;
704704
} else {
705705
let now: int = map_int(tick, "ms", 0);
@@ -734,6 +734,7 @@ fn observe_limits(state: map) -> map {
734734
}
735735

736736
fn walk_search(state: map, path: string, depth: int) -> map {
737+
state.dirs_visited = map_int(state, "dirs_visited", 0) + 1;
737738
state = observe_limits(state);
738739
if map_bool(state, "stop", false) == false {
739740
if depth > map_int(state, "max_search_depth", 32) {
@@ -745,7 +746,6 @@ fn walk_search(state: map, path: string, depth: int) -> map {
745746
state.stop = true;
746747
} else {
747748
if remaining <= 1 {
748-
state.dirs_visited = map_int(state, "dirs_visited", 0) + 1;
749749
state.truncated = true;
750750
state.stop = true;
751751
} else {
@@ -758,7 +758,6 @@ fn walk_search(state: map, path: string, depth: int) -> map {
758758
let error: map = types::map_map(listed, "error");
759759
let code: string = types::map_string(error, "code", "internal_error");
760760
if code == "budget_exceeded" {
761-
state.dirs_visited = map_int(state, "dirs_visited", 0) + 1;
762761
state.truncated = true;
763762
state.stop = true;
764763
} else {
@@ -778,7 +777,6 @@ fn walk_search(state: map, path: string, depth: int) -> map {
778777
state.stop = true;
779778
}
780779
} else {
781-
state.dirs_visited = map_int(state, "dirs_visited", 0) + 1;
782780
let mut drop_page: bool = false;
783781
if remaining <= 2 {
784782
let probe_entries: array = types::map_array(listed, "entries");
@@ -1030,7 +1028,10 @@ pub fn execute(context: map, arguments: map) -> map {
10301028
result = fail_host(start_clock);
10311029
} else {
10321030
state.start_ms = map_int(start_clock, "ms", 0);
1033-
state = walk_search(state, path, 0);
1031+
state = observe_limits(state);
1032+
if map_bool(state, "stop", false) == false {
1033+
state = walk_search(state, path, 0);
1034+
}
10341035
if map_bool(state, "fatal", false) {
10351036
result = fail(types::map_string(state, "fatal_code", "internal_error"), types::map_string(state, "fatal_message", ""), {});
10361037
} else {

src/capabilities/confined_io.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ pub(crate) struct ListEntry {
3535
pub len: u64,
3636
}
3737

38-
/// One cursor page. Only `limit` entries are retained, plus constant lookahead.
38+
/// One cursor page. `limit` bounds physical dirents examined (not only emitted
39+
/// valid names), plus constant one-entry lookahead for `truncated`.
3940
pub(crate) struct ListPage {
4041
pub entries: Vec<ListEntry>,
4142
pub next_cursor: u64,
@@ -316,7 +317,7 @@ mod unix {
316317
skipped += 1;
317318
continue;
318319
}
319-
if entries.len() >= limit {
320+
if consumed >= u64::try_from(limit).unwrap_or(u64::MAX) {
320321
truncated = true;
321322
break;
322323
}

tests/capability_tests.rs

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1877,3 +1877,114 @@ fn list_omits_non_utf8_names() {
18771877
.collect::<Vec<_>>()
18781878
);
18791879
}
1880+
1881+
#[cfg(unix)]
1882+
#[test]
1883+
fn list_examination_budget_counts_non_utf8_slots() {
1884+
use std::ffi::OsString;
1885+
use std::os::unix::ffi::OsStringExt;
1886+
1887+
let fixture = Fixture::new("list-exam-slots");
1888+
fs::create_dir(fixture.root.join("dir")).expect("dir");
1889+
fs::write(
1890+
fixture
1891+
.root
1892+
.join("dir")
1893+
.join(OsString::from_vec(vec![0xff, 0x80])),
1894+
"secret",
1895+
)
1896+
.expect("invalid-a");
1897+
fs::write(
1898+
fixture
1899+
.root
1900+
.join("dir")
1901+
.join(OsString::from_vec(vec![0xff, 0x81])),
1902+
"secret",
1903+
)
1904+
.expect("invalid-b");
1905+
fs::write(fixture.root.join("dir").join("keep.txt"), "ok").expect("keep");
1906+
1907+
let fs_cap = fixture.filesystem();
1908+
let token = fixture.token(CapabilityRisk::Read);
1909+
let mut cursor = 0_u64;
1910+
let mut pages = 0_usize;
1911+
let mut seen_keep = false;
1912+
loop {
1913+
pages += 1;
1914+
assert!(pages <= 8, "pagination must not loop");
1915+
let page = fs_cap.list(&token, "dir", cursor, 1).expect("page");
1916+
let examined = page.next_cursor.saturating_sub(page.cursor);
1917+
assert!(
1918+
examined <= 1,
1919+
"limit must bound physical dirents examined, got examined={examined} page={page:?}"
1920+
);
1921+
assert!(
1922+
page.entries.len() <= 1,
1923+
"page must not emit more names than the examination budget"
1924+
);
1925+
assert!(
1926+
page.entries
1927+
.iter()
1928+
.all(|entry| !entry.name.contains('\u{FFFD}')),
1929+
"lossy names must not be listed: {:?}",
1930+
page.entries
1931+
.iter()
1932+
.map(|entry| &entry.name)
1933+
.collect::<Vec<_>>()
1934+
);
1935+
assert!(
1936+
!page.entries.iter().any(|entry| entry.name == "secret"),
1937+
"invalid-byte contents must not leak through the name slot"
1938+
);
1939+
if page.entries.iter().any(|entry| entry.name == "keep.txt") {
1940+
seen_keep = true;
1941+
}
1942+
if page.truncated {
1943+
assert_ne!(
1944+
page.next_cursor, cursor,
1945+
"truncated pages must advance next_cursor"
1946+
);
1947+
cursor = page.next_cursor;
1948+
continue;
1949+
}
1950+
break;
1951+
}
1952+
assert!(seen_keep, "valid keep.txt must remain reachable by cursor");
1953+
1954+
let host_fs = Arc::new(fixture.filesystem());
1955+
let source = format!(
1956+
r#"
1957+
pub fn run(input: map) -> map {{
1958+
cap::fs_list("{token}", "dir", 0, 1)
1959+
}}
1960+
"#
1961+
);
1962+
let result = run_cap_source(&fixture, Some(host_fs), None, None, &source);
1963+
let VmValue::Map(fields) = &result else {
1964+
panic!("expected list envelope, got {result:?}");
1965+
};
1966+
assert_eq!(
1967+
fields.get(&VmValue::string("ok")),
1968+
Some(&VmValue::Bool(true))
1969+
);
1970+
let Some(VmValue::Int(next_cursor)) = fields.get(&VmValue::string("next_cursor")) else {
1971+
panic!("expected next_cursor, got {result:?}");
1972+
};
1973+
assert!(
1974+
*next_cursor <= 1,
1975+
"host list must charge examined slots, got {result:?}"
1976+
);
1977+
if let Some(VmValue::Array(entries)) = fields.get(&VmValue::string("entries")) {
1978+
for entry in entries.iter() {
1979+
let VmValue::Map(entry) = entry else {
1980+
panic!("expected entry map, got {entry:?}");
1981+
};
1982+
if let Some(VmValue::String(name)) = entry.get(&VmValue::string("name")) {
1983+
assert!(
1984+
!name.contains('\u{FFFD}'),
1985+
"host list must not expose lossy names: {name}"
1986+
);
1987+
}
1988+
}
1989+
}
1990+
}

tests/rss_file_tool_tests.rs

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1253,6 +1253,21 @@ fn search_enumeration_budget_counts_dot_slots_like_native() {
12531253
"exam-nested-rem-2",
12541254
5,
12551255
&["adir/f0.txt", "adir/f1.txt", "adir/f2.txt", "zdir/late.txt"],
1256+
content.clone(),
1257+
);
1258+
1259+
// After the first subtree consumes the file budget, entering the next
1260+
// sibling increments dirs_visited before truncation (native walk-entry order).
1261+
assert_search_exam_budget_eq(
1262+
"exam-nested-rem-0",
1263+
6,
1264+
&[
1265+
"adir/f0.txt",
1266+
"adir/f1.txt",
1267+
"adir/f2.txt",
1268+
"adir/f3.txt",
1269+
"zdir/late.txt",
1270+
],
12561271
content,
12571272
);
12581273
}
@@ -1808,6 +1823,48 @@ fn search_skips_non_utf8_names_like_native() {
18081823
assert_search_eq(&fixture, json!({"pattern": "*", "target": "files"}));
18091824
}
18101825

1826+
#[cfg(unix)]
1827+
#[test]
1828+
fn search_non_utf8_name_consumes_exam_slot_and_does_not_leak_secret() {
1829+
use std::ffi::OsString;
1830+
use std::os::unix::ffi::OsStringExt;
1831+
1832+
let fixture = Fixture::new("search-non-utf8-cap");
1833+
fs::write(
1834+
fixture
1835+
.root
1836+
.join(OsString::from_vec(vec![0xff, b'x', 0x80])),
1837+
"secret needle\n",
1838+
)
1839+
.unwrap();
1840+
fs::write(fixture.root.join("secret.txt"), "secret needle\n").unwrap();
1841+
fs::write(fixture.root.join("other.txt"), "other\n").unwrap();
1842+
let mut config = fixture.config();
1843+
config.max_search_files = 4;
1844+
config.artifact_store.root = fixture.parent.join("artifacts-non-utf8-cap");
1845+
let arguments = json!({"pattern": "secret"});
1846+
let native = native_execute(
1847+
&fixture.tools_with_config(config.clone()),
1848+
NativeToolExecutor::SearchFiles,
1849+
&arguments,
1850+
);
1851+
let rss = run_rss_search(&fixture, &config, arguments);
1852+
assert_exact_envelope(&native, &rss.result);
1853+
assert!(native.ok, "native={native:?}");
1854+
assert!(native.truncated, "native must truncate at the exam cap");
1855+
assert!(
1856+
!native.content.contains("secret.txt"),
1857+
"secret.txt must not leak after a non-UTF8 exam slot: native={native:?}"
1858+
);
1859+
assert!(
1860+
!rss.result["content"]
1861+
.as_str()
1862+
.is_some_and(|content| content.contains("secret.txt")),
1863+
"secret.txt must not leak after a non-UTF8 exam slot: rss={}",
1864+
rss.result
1865+
);
1866+
}
1867+
18111868
#[test]
18121869
fn search_directory_order_is_byte_lexicographic_including_multibyte() {
18131870
let fixture = Fixture::new("sort-multi");
@@ -1858,6 +1915,88 @@ fn search_fake_clock_backward_jump_does_not_extend_budget() {
18581915
assert_eq!(rss.result["error"], Value::Null);
18591916
}
18601917

1918+
struct OverflowAfterClock {
1919+
ok_ticks: AtomicU64,
1920+
overflow_after: u64,
1921+
instant: Instant,
1922+
}
1923+
1924+
impl OverflowAfterClock {
1925+
fn after_ok_ticks(ok_ticks: u64) -> Arc<Self> {
1926+
Arc::new(Self {
1927+
ok_ticks: AtomicU64::new(0),
1928+
overflow_after: ok_ticks,
1929+
instant: Instant::now(),
1930+
})
1931+
}
1932+
}
1933+
1934+
impl LifecycleClock for OverflowAfterClock {
1935+
fn now_ms(&self) -> u64 {
1936+
1_000
1937+
}
1938+
1939+
fn now(&self) -> Instant {
1940+
self.instant
1941+
}
1942+
1943+
fn monotonic_ms(&self) -> Option<u64> {
1944+
let seen = self.ok_ticks.fetch_add(1, Ordering::SeqCst);
1945+
if seen >= self.overflow_after {
1946+
None
1947+
} else {
1948+
Some(1_000)
1949+
}
1950+
}
1951+
}
1952+
1953+
#[test]
1954+
fn search_fake_clock_overflow_uses_nested_capability_error_envelope() {
1955+
let fixture = Fixture::new("search-clock-overflow");
1956+
fs::write(fixture.root.join("a.txt"), "alpha\n").unwrap();
1957+
let mut config = fixture.config();
1958+
config.max_search_wall_time = Duration::from_millis(2_000);
1959+
let rss = run_rss_exec(
1960+
&fixture,
1961+
&config,
1962+
RssExec {
1963+
module: "search_files.rss",
1964+
tool_name: "search_files",
1965+
arguments: json!({"pattern": "alpha"}),
1966+
durable: MemoryDurable::new(),
1967+
approval: Arc::new(AllowAll),
1968+
cancellation: Arc::new(NeverCancelled),
1969+
clock: OverflowAfterClock::after_ok_ticks(1),
1970+
deadline_ms: 1_000_000,
1971+
install_artifacts: false,
1972+
artifact_limits: default_artifact_limits(),
1973+
call_id: "call-clock-overflow".to_string(),
1974+
},
1975+
);
1976+
assert_eq!(rss.result["ok"], json!(false), "rss={}", rss.result);
1977+
assert_eq!(
1978+
rss.result["error"]["code"],
1979+
json!("internal_error"),
1980+
"overflow must preserve the nested capability code, rss={}",
1981+
rss.result
1982+
);
1983+
assert_eq!(
1984+
rss.result["error"]["message"],
1985+
json!("monotonic clock overflow"),
1986+
"overflow must preserve the nested capability message, rss={}",
1987+
rss.result
1988+
);
1989+
assert_ne!(
1990+
rss.result["error"]["code"],
1991+
json!("cancelled"),
1992+
"top-level code must not collapse overflow to cancelled"
1993+
);
1994+
assert_eq!(rss.result["truncated"], json!(false), "rss={}", rss.result);
1995+
assert_eq!(rss.result["content"], json!(""));
1996+
assert_eq!(rss.result["data"], json!({}));
1997+
assert_eq!(rss.result["artifacts"], json!([]));
1998+
}
1999+
18612000
#[test]
18622001
fn published_result_artifact_is_retracted_when_commit_fails() {
18632002
let fixture = Fixture::new("artifact-rollback");

0 commit comments

Comments
 (0)