Skip to content

Commit b755c65

Browse files
committed
fix(tools): harden rss dispatch ownership
Compile from_source as supplied bytes, cache programs by module-tree digest, keep a single production worker path through tools::dispatch, and fail closed on process shutdown races and malformed JSON payloads.
1 parent 0df5647 commit b755c65

22 files changed

Lines changed: 1307 additions & 586 deletions

‎docs/configuration.md‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -193,12 +193,11 @@ page bounds.
193193
## Coding tools and serial loop
194194

195195
The library `AgentService` worker compiles bundled `rss/agent/main.rss` and
196-
drives a **serial** native tool loop. RSS builds canonical provider requests
197-
and dispatches tools only through the native host bridges
198-
(`agent::provider_call`, `agent::tool_dispatch`). This is not an
199-
OpenAI-compatible inference path.
196+
drives a **serial** RSS `tools::dispatch` loop over the generic capability
197+
host (`agent::provider_call`, filesystem/process/artifact adapters). This is
198+
not an OpenAI-compatible inference path.
200199

201-
Built-in native tools, in registry order:
200+
Built-in RSS registry tools, in registry order:
202201

203202
| Name | Toolset | Risk | Notes |
204203
| --- | --- | --- | --- |

‎rss/tools/dispatch.rss‎

Lines changed: 74 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -59,21 +59,6 @@ fn call_id(call: map) -> string {
5959
id
6060
}
6161

62-
fn json_object_text(text: string) -> bool {
63-
let mut ok: bool = false;
64-
if text.length >= 2 {
65-
if text[0:1] == "{" {
66-
let second: string = text[1:2];
67-
if second == "\"" || second == "}" || second == " " || second == "\n" || second == "\t" {
68-
if text[(text.length - 1):text.length] == "}" {
69-
ok = true;
70-
}
71-
}
72-
}
73-
}
74-
ok
75-
}
76-
7762
fn parse_call_arguments(call: map) -> map {
7863
let mut out: map = {
7964
ok: true,
@@ -93,19 +78,24 @@ fn parse_call_arguments(call: map) -> map {
9378
if call.has("arguments_json") {
9479
if type(call.arguments_json) == "string" {
9580
let text: string = call.arguments_json;
96-
if text.length > 0 {
97-
if json_object_text(text) {
98-
let decoded: map = json::decode(text);
99-
out.arguments = decoded;
100-
} else {
101-
out = {
102-
ok: false,
103-
arguments: {},
104-
code: "malformed_payload",
105-
message: "tool arguments must be a JSON object"
106-
};
107-
}
81+
let parsed: map = agent::parse_json_object(text);
82+
if map_bool(parsed, "ok", false) {
83+
out.arguments = types::map_map(parsed, "value");
84+
} else {
85+
out = {
86+
ok: false,
87+
arguments: {},
88+
code: "malformed_payload",
89+
message: "tool arguments must be a JSON object"
90+
};
10891
}
92+
} else {
93+
out = {
94+
ok: false,
95+
arguments: {},
96+
code: "malformed_payload",
97+
message: "tool arguments must be a JSON object"
98+
};
10999
}
110100
}
111101
}
@@ -128,7 +118,18 @@ fn find_named(registry: array, name: string) -> map {
128118
let mut found: map = {};
129119
let mut matches: int = 0;
130120
let mut i: int = 0;
121+
let mut scans: int = 0;
131122
while i < registry.length {
123+
if i < 0 {
124+
break;
125+
}
126+
scans = scans + 1;
127+
if scans > 64 {
128+
break;
129+
}
130+
if registry.has(i) == false {
131+
break;
132+
}
132133
let mut entry: map = {};
133134
if type(registry[i]) == "map" {
134135
let coerced: map = registry[i];
@@ -146,23 +147,57 @@ fn find_named(registry: array, name: string) -> map {
146147
}
147148
}
148149

150+
// Duplicate scan is O(n^2) with a hard n<=64 bound. The comparison
151+
// counter is n*(n-1)/2 worst case; 64 is exact-max, 65 fails closed.
149152
fn has_duplicate_names(registry: array) -> bool {
150153
let mut dup: bool = false;
151-
let mut i: int = 0;
152-
while i < registry.length {
153-
let mut entry: map = {};
154-
if type(registry[i]) == "map" {
155-
let coerced: map = registry[i];
156-
entry = coerced;
157-
}
158-
let name: string = types::map_string(entry, "name", "");
159-
if name.length > 0 {
160-
let counted: map = find_named(registry, name);
161-
if counted.matches > 1 {
154+
if registry.length > 64 {
155+
dup = true;
156+
} else {
157+
let mut comparisons: int = 0;
158+
let mut i: int = 0;
159+
while i < registry.length {
160+
if i < 0 {
162161
dup = true;
162+
break;
163+
}
164+
if registry.has(i) == false {
165+
break;
163166
}
167+
let mut left: map = {};
168+
if type(registry[i]) == "map" {
169+
let coerced: map = registry[i];
170+
left = coerced;
171+
}
172+
let name: string = types::map_string(left, "name", "");
173+
if name.length > 0 {
174+
let mut j: int = i + 1;
175+
while j < registry.length {
176+
if j < 0 {
177+
dup = true;
178+
break;
179+
}
180+
comparisons = comparisons + 1;
181+
if comparisons > 64 * 64 {
182+
dup = true;
183+
break;
184+
}
185+
if registry.has(j) == false {
186+
break;
187+
}
188+
let mut right: map = {};
189+
if type(registry[j]) == "map" {
190+
let other: map = registry[j];
191+
right = other;
192+
}
193+
if types::map_string(right, "name", "") == name {
194+
dup = true;
195+
}
196+
j = j + 1;
197+
}
198+
}
199+
i = i + 1;
164200
}
165-
i = i + 1;
166201
}
167202
dup
168203
}

‎src/capabilities/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ pub mod types;
1010
mod confined_io;
1111
mod hash;
1212

13+
pub(crate) use hash::sha256_hex;
14+
1315
pub use artifacts::{ArtifactCapability, ArtifactLimits, ArtifactRef};
1416
pub use filesystem::{
1517
FilesystemCapability, FilesystemLimits, FsDirEntry, FsList, FsMetadata, FsRead, FsWrite,

‎src/capabilities/process.rs‎

Lines changed: 70 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use std::{
77
collections::HashMap,
88
sync::{
99
Arc, Mutex, Weak,
10-
atomic::{AtomicBool, AtomicUsize, Ordering},
10+
atomic::{AtomicBool, AtomicU64, AtomicUsize, Ordering},
1111
mpsc::{self, RecvTimeoutError},
1212
},
1313
thread,
@@ -146,9 +146,13 @@ struct ProcessInner {
146146
host_limits: ProcessLimits,
147147
root: ConfinedFsRoot,
148148
table: Mutex<HashMap<String, OwnedProcess>>,
149+
closing: AtomicBool,
150+
generation: AtomicU64,
149151
after_running_poll_hook: Mutex<Option<ProcessOpHook>>,
150152
write_blocked_hook: Mutex<Option<ProcessOpHook>>,
151153
before_write_cycle_hook: Mutex<Option<ProcessOpHook>>,
154+
before_os_spawn_hook: Mutex<Option<ProcessOpHook>>,
155+
after_os_spawn_hook: Mutex<Option<ProcessOpHook>>,
152156
stdin_workers: AtomicUsize,
153157
}
154158

@@ -208,9 +212,13 @@ impl ProcessCapability {
208212
host_limits,
209213
root,
210214
table: Mutex::new(HashMap::new()),
215+
closing: AtomicBool::new(false),
216+
generation: AtomicU64::new(1),
211217
after_running_poll_hook: Mutex::new(None),
212218
write_blocked_hook: Mutex::new(None),
213219
before_write_cycle_hook: Mutex::new(None),
220+
before_os_spawn_hook: Mutex::new(None),
221+
after_os_spawn_hook: Mutex::new(None),
214222
stdin_workers: AtomicUsize::new(0),
215223
}),
216224
})
@@ -239,6 +247,9 @@ impl ProcessCapability {
239247
stdin: Option<&[u8]>,
240248
) -> Result<ProcessSpawn, CapabilityError> {
241249
let claims = self.authorize(token, CapabilityRisk::Execute)?;
250+
if self.is_closing() {
251+
return Err(closing_error());
252+
}
242253
if argv.is_empty() {
243254
return Err(CapabilityError::new(
244255
"invalid_request",
@@ -281,16 +292,32 @@ impl ProcessCapability {
281292
{
282293
request = request.with_stdin(stdin.to_vec());
283294
}
295+
if self.is_closing() {
296+
return Err(closing_error());
297+
}
298+
fire_hook(&self.inner.before_os_spawn_hook);
299+
if self.is_closing() {
300+
return Err(closing_error());
301+
}
302+
let fence = self.inner.generation.load(Ordering::SeqCst);
284303
let process = BoundedProcess::spawn(request).map_err(map_process_error)?;
304+
fire_hook(&self.inner.after_os_spawn_hook);
285305
let handle = process.lifecycle_handle();
286306
let write_handle = handle.clone();
287307
let pid = handle.pid();
288308
let id = uuid::Uuid::new_v4().simple().to_string();
289-
self.inner
290-
.table
291-
.lock()
292-
.unwrap_or_else(|poisoned| poisoned.into_inner())
293-
.insert(
309+
{
310+
let mut table = self
311+
.inner
312+
.table
313+
.lock()
314+
.unwrap_or_else(|poisoned| poisoned.into_inner());
315+
if self.is_closing() || self.inner.generation.load(Ordering::SeqCst) != fence {
316+
drop(table);
317+
let _ = handle.shutdown();
318+
return Err(closing_error());
319+
}
320+
table.insert(
294321
id.clone(),
295322
OwnedProcess {
296323
owner_key: claims.owner.key(),
@@ -299,6 +326,7 @@ impl ProcessCapability {
299326
cancel: cancel.clone(),
300327
},
301328
);
329+
}
302330
let reaper = Arc::new(ProcessReaper {
303331
inner: Arc::downgrade(&self.inner),
304332
id: id.clone(),
@@ -466,6 +494,24 @@ impl ProcessCapability {
466494
.unwrap_or_else(|poisoned| poisoned.into_inner()) = Some(hook);
467495
}
468496

497+
/// Test barrier: fires immediately before the OS spawn syscall.
498+
pub fn set_before_os_spawn_hook(&self, hook: Arc<dyn Fn() + Send + Sync>) {
499+
*self
500+
.inner
501+
.before_os_spawn_hook
502+
.lock()
503+
.unwrap_or_else(|poisoned| poisoned.into_inner()) = Some(hook);
504+
}
505+
506+
/// Test barrier: fires after OS spawn and before table insert.
507+
pub fn set_after_os_spawn_hook(&self, hook: Arc<dyn Fn() + Send + Sync>) {
508+
*self
509+
.inner
510+
.after_os_spawn_hook
511+
.lock()
512+
.unwrap_or_else(|poisoned| poisoned.into_inner()) = Some(hook);
513+
}
514+
469515
/// Returns a bounded log window.
470516
pub fn log(
471517
&self,
@@ -550,9 +596,13 @@ impl ProcessCapability {
550596

551597
/// Terminates every owned child and drops table entries.
552598
///
553-
/// Run cleanup must drain committed background residue; `cancel_all`
554-
/// kills the process tree but leaves handles observable in the table.
599+
/// Closing is irreversible: later spawns refuse insert and terminate any
600+
/// OS process created after the fence. Run cleanup must drain committed
601+
/// background residue; `cancel_all` kills the process tree but leaves
602+
/// handles observable in the table.
555603
pub fn shutdown_all(&self) {
604+
self.inner.closing.store(true, Ordering::SeqCst);
605+
self.inner.generation.fetch_add(1, Ordering::SeqCst);
556606
let owned: Vec<OwnedProcess> = {
557607
let mut table = self
558608
.inner
@@ -566,6 +616,11 @@ impl ProcessCapability {
566616
}
567617
}
568618

619+
/// True after [`Self::shutdown_all`] has started. The fence is irreversible.
620+
pub fn is_closing(&self) -> bool {
621+
self.inner.closing.load(Ordering::SeqCst)
622+
}
623+
569624
fn authorize(&self, token: &str, risk: CapabilityRisk) -> Result<TokenClaims, CapabilityError> {
570625
match self
571626
.inner
@@ -578,6 +633,9 @@ impl ProcessCapability {
578633
}
579634

580635
fn lookup(&self, token: &str, handle: &str) -> Result<OwnedProcess, CapabilityError> {
636+
if self.is_closing() {
637+
return Err(closing_error());
638+
}
581639
let claims = self.authorize(token, CapabilityRisk::Execute)?;
582640
let table = self
583641
.inner
@@ -712,6 +770,10 @@ fn fire_hook(slot: &Mutex<Option<ProcessOpHook>>) {
712770
}
713771
}
714772

773+
fn closing_error() -> CapabilityError {
774+
CapabilityError::new("capability_unavailable", "process capability is closing")
775+
}
776+
715777
fn interrupt_write_worker(
716778
handle: &BoundedProcessHandle,
717779
worker: thread::JoinHandle<()>,

‎src/gateway/api_server.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,7 @@ async fn delete_session_handler(
691691
{
692692
state
693693
.service()
694-
.cleanup_session_native_dispatch(&session_id_for_cleanup);
694+
.cleanup_session_capability_host(&session_id_for_cleanup);
695695
}
696696
response
697697
}

0 commit comments

Comments
 (0)