rootandClaude Opus 4.7
6532938e85
Scrum iter 11 flagged crates/gateway/src/tools/service.rs with two
95%-confidence critical failures:
CF-1: "Direct SQL execution from model-provided parameters without
explicit validation or sanitization" (line 68, 95% conf)
CF-2: "No permission check performed before executing SQL query;
access control is bypassed entirely" (line 102, 90% conf)
CF-1 is the real one — same security gap as queryd /sql had before
P42-002 (9cc0ceb). Tool invocations build SQL from a template +
model-provided params, then state.query_fn.execute(&sql) runs it.
No truth-gate check between build and execute meant an adversarial
model could emit DROP TABLE / DELETE FROM / TRUNCATE inside a param
and bypass queryd's gate by routing through the tool surface instead.
Fix mirrors the queryd SQL gate exactly:
- ToolState grows an Arc<TruthStore> field
- main.rs constructs it via truth::sql_query_guard_store()
(shared default — same destructive-verb block as queryd)
- call_tool evaluates the built SQL against "sql_query" task class
BEFORE executing
- Any Reject/Block outcome → 403 FORBIDDEN + log_invocation row
marked success=false with the rule message
CF-2 (access control) is P13-001 territory — needs AccessControl
wiring into queryd first, still open. Flagged in memory.
Workspace warnings still at 0. Pattern is now:
queryd /sql → truth::sql_query_guard_store (9cc0ceb)
gateway /tools → truth::sql_query_guard_store (this commit)
execution_loop → truth::default_truth_store (51a1aa3)
All three surfaces that pipe SQL or spec-shaped data through to the
substrate now gate it. Any new SQL-executing surface should follow
the same pattern.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
54 lines
1.6 KiB
Rust
54 lines
1.6 KiB
Rust
pub mod registry;
|
|
pub mod service;
|
|
|
|
use queryd::context::QueryEngine;
|
|
use arrow::json::writer::{JsonArray, Writer as JsonWriter};
|
|
use std::sync::Arc;
|
|
use truth::TruthStore;
|
|
|
|
/// State for the tool system.
|
|
#[derive(Clone)]
|
|
pub struct ToolState {
|
|
pub registry: registry::ToolRegistry,
|
|
pub query_fn: QueryExecutor,
|
|
/// SQL guard (shared with queryd). Mirrors the queryd /sql truth
|
|
/// gate from P42-002 (9cc0ceb) — tools also execute model-
|
|
/// originated SQL, need the same destructive-verb block.
|
|
pub truth: Arc<TruthStore>,
|
|
}
|
|
|
|
/// Wraps QueryEngine to provide a simple execute interface for tools.
|
|
#[derive(Clone)]
|
|
pub struct QueryExecutor {
|
|
engine: QueryEngine,
|
|
}
|
|
|
|
impl QueryExecutor {
|
|
pub fn new(engine: QueryEngine) -> Self {
|
|
Self { engine }
|
|
}
|
|
|
|
/// Execute SQL and return (rows as JSON, row count).
|
|
pub async fn execute(&self, sql: &str) -> Result<(serde_json::Value, usize), String> {
|
|
let batches = self.engine.query(sql).await?;
|
|
|
|
if batches.is_empty() {
|
|
return Ok((serde_json::Value::Array(vec![]), 0));
|
|
}
|
|
|
|
let mut buf = Vec::new();
|
|
let mut writer = JsonWriter::<_, JsonArray>::new(&mut buf);
|
|
for batch in &batches {
|
|
writer.write(batch).map_err(|e| format!("JSON write: {e}"))?;
|
|
}
|
|
writer.finish().map_err(|e| format!("JSON finish: {e}"))?;
|
|
drop(writer);
|
|
|
|
let rows: serde_json::Value = serde_json::from_slice(&buf)
|
|
.map_err(|e| format!("JSON parse: {e}"))?;
|
|
let count = rows.as_array().map(|a| a.len()).unwrap_or(0);
|
|
|
|
Ok((rows, count))
|
|
}
|
|
}
|