feat(interceptors): per-interceptor wall-clock timeout (§9.4 M5)
A [[interceptors]] marker can set timeout_ms (migration 0075, CHECK > 0); a
runaway guard (loop {}) is interrupted and the op DENIED (fail closed) within
budget rather than hanging the write path. The effective deadline is
min(caller-remaining, now + timeout) — a hook can only tighten, never extend,
its caller's deadline. NULL uses PICLOUD_INTERCEPTOR_TIMEOUT_MS (default 5s).
Wiring: run_resolved_blocking splits into a core + a _with_timeout variant that
computes the effective deadline from engine::ambient_deadline() and runs via
execute_ast_with_deadline; run_one_hook passes the marker's timeout_ms (or the
env default). timeout_ms threaded end to end — manifest, plan, BundleInterceptor,
the reconcile diff (part of the mutable body: a timeout change is an Update),
insert_interceptor_tx, resolve_chain/list_for_owner/list_on_app_chain +
SealedInterceptor/InterceptorMarker, and interceptor_service → ResolvedInterceptor.
Schema snapshot re-blessed. Pinned by a journey: a loop{} guard with
timeout_ms=100 is denied and its write does not persist.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -126,6 +126,10 @@ pub struct BundleInterceptor {
|
||||
#[serde(default = "default_before_phase")]
|
||||
pub phase: String,
|
||||
pub script: String,
|
||||
/// §9.4 M5: per-hook timeout in ms (`None` = instance default). `default`
|
||||
/// so a pre-M5 CLI (which omits it) still deserializes.
|
||||
#[serde(default)]
|
||||
pub timeout_ms: Option<i32>,
|
||||
}
|
||||
|
||||
fn default_before_phase() -> String {
|
||||
@@ -828,8 +832,8 @@ pub struct CurrentState {
|
||||
/// v1.2 Workflows owned directly by this node (app-owned; empty for a group).
|
||||
pub workflows: Vec<crate::workflow_repo::Workflow>,
|
||||
/// §9.4 interceptor markers declared directly at this node, as
|
||||
/// `(service, op, phase, script)` tuples.
|
||||
pub interceptors: Vec<(String, String, String, String)>,
|
||||
/// `(service, op, phase, script, timeout_ms)` tuples.
|
||||
pub interceptors: Vec<(String, String, String, String, Option<i32>)>,
|
||||
}
|
||||
|
||||
/// One row of the read-only extension-point report (§5.5).
|
||||
@@ -1620,6 +1624,7 @@ impl ApplyService {
|
||||
&bi.op,
|
||||
&bi.phase,
|
||||
&bi.script,
|
||||
bi.timeout_ms,
|
||||
)
|
||||
.await
|
||||
.map_err(|e| ApplyError::Backend(e.to_string()))?;
|
||||
@@ -4263,7 +4268,7 @@ impl ApplyService {
|
||||
.await
|
||||
.map_err(|e| ApplyError::Backend(e.to_string()))?
|
||||
.into_iter()
|
||||
.map(|m| (m.service, m.op, m.phase, m.script))
|
||||
.map(|m| (m.service, m.op, m.phase, m.script, m.timeout_ms))
|
||||
.collect();
|
||||
Ok(CurrentState {
|
||||
scripts,
|
||||
@@ -4352,24 +4357,30 @@ fn compute_diff_with_names(
|
||||
/// declaring is a `Delete` (applied only under `--prune`).
|
||||
fn diff_interceptors(current: &CurrentState, bundle: &Bundle) -> Vec<ResourceChange> {
|
||||
let key = |service: &str, op: &str, phase: &str| format!("{service}/{op}/{phase}");
|
||||
let live: HashMap<String, &str> = current
|
||||
// The reconcile identity is `(service, op, phase)`; the script AND the
|
||||
// timeout are the mutable body, so a change to either is an Update.
|
||||
let live: HashMap<String, (&str, Option<i32>)> = current
|
||||
.interceptors
|
||||
.iter()
|
||||
.map(|(s, o, p, script)| (key(s, o, p), script.as_str()))
|
||||
.map(|(s, o, p, script, timeout)| (key(s, o, p), (script.as_str(), *timeout)))
|
||||
.collect();
|
||||
let mut out = Vec::new();
|
||||
for bi in &bundle.interceptors {
|
||||
let k = key(&bi.service, &bi.op, &bi.phase);
|
||||
match live.get(&k) {
|
||||
Some(cur) if *cur == bi.script => out.push(ResourceChange {
|
||||
op: Op::NoOp,
|
||||
key: k,
|
||||
detail: None,
|
||||
}),
|
||||
Some((cur_script, cur_timeout))
|
||||
if *cur_script == bi.script && *cur_timeout == bi.timeout_ms =>
|
||||
{
|
||||
out.push(ResourceChange {
|
||||
op: Op::NoOp,
|
||||
key: k,
|
||||
detail: None,
|
||||
});
|
||||
}
|
||||
Some(_) => out.push(ResourceChange {
|
||||
op: Op::Update,
|
||||
key: k,
|
||||
detail: Some("interceptor script changed".into()),
|
||||
detail: Some("interceptor script or timeout changed".into()),
|
||||
}),
|
||||
None => out.push(ResourceChange {
|
||||
op: Op::Create,
|
||||
@@ -4378,7 +4389,7 @@ fn diff_interceptors(current: &CurrentState, bundle: &Bundle) -> Vec<ResourceCha
|
||||
}),
|
||||
}
|
||||
}
|
||||
for (s, o, p, _) in ¤t.interceptors {
|
||||
for (s, o, p, _, _) in ¤t.interceptors {
|
||||
let present = bundle
|
||||
.interceptors
|
||||
.iter()
|
||||
|
||||
@@ -23,6 +23,8 @@ pub struct InterceptorMarker {
|
||||
/// `before` (allow/deny + transform) or `after` (observe; §9.4 M3).
|
||||
pub phase: String,
|
||||
pub script: String,
|
||||
/// §9.4 M5: per-hook timeout, or `None` for the instance default.
|
||||
pub timeout_ms: Option<i32>,
|
||||
}
|
||||
|
||||
/// List the markers declared **directly at** `owner` (not inherited), ordered
|
||||
@@ -31,10 +33,10 @@ pub async fn list_for_owner(
|
||||
pool: &PgPool,
|
||||
owner: ScriptOwner,
|
||||
) -> Result<Vec<InterceptorMarker>, sqlx::Error> {
|
||||
let rows: Vec<(String, String, String, String)> = match owner {
|
||||
let rows: Vec<(String, String, String, String, Option<i32>)> = match owner {
|
||||
ScriptOwner::App(a) => {
|
||||
sqlx::query_as(
|
||||
"SELECT service, op, phase, interceptor_script FROM interceptors \
|
||||
"SELECT service, op, phase, interceptor_script, timeout_ms FROM interceptors \
|
||||
WHERE app_id = $1 ORDER BY service, op, phase",
|
||||
)
|
||||
.bind(a.into_inner())
|
||||
@@ -43,7 +45,7 @@ pub async fn list_for_owner(
|
||||
}
|
||||
ScriptOwner::Group(g) => {
|
||||
sqlx::query_as(
|
||||
"SELECT service, op, phase, interceptor_script FROM interceptors \
|
||||
"SELECT service, op, phase, interceptor_script, timeout_ms FROM interceptors \
|
||||
WHERE group_id = $1 ORDER BY service, op, phase",
|
||||
)
|
||||
.bind(g.into_inner())
|
||||
@@ -53,12 +55,15 @@ pub async fn list_for_owner(
|
||||
};
|
||||
Ok(rows
|
||||
.into_iter()
|
||||
.map(|(service, op, phase, script)| InterceptorMarker {
|
||||
service,
|
||||
op,
|
||||
phase,
|
||||
script,
|
||||
})
|
||||
.map(
|
||||
|(service, op, phase, script, timeout_ms)| InterceptorMarker {
|
||||
service,
|
||||
op,
|
||||
phase,
|
||||
script,
|
||||
timeout_ms,
|
||||
},
|
||||
)
|
||||
.collect())
|
||||
}
|
||||
|
||||
@@ -69,10 +74,10 @@ pub async fn list_on_app_chain(
|
||||
pool: &PgPool,
|
||||
app_id: AppId,
|
||||
) -> Result<Vec<InterceptorMarker>, sqlx::Error> {
|
||||
let rows: Vec<(String, String, String, String)> = sqlx::query_as(&format!(
|
||||
let rows: Vec<(String, String, String, String, Option<i32>)> = sqlx::query_as(&format!(
|
||||
"{CHAIN_LEVELS_CTE} \
|
||||
SELECT DISTINCT ON (i.service, i.op, i.phase) \
|
||||
i.service, i.op, i.phase, i.interceptor_script \
|
||||
i.service, i.op, i.phase, i.interceptor_script, i.timeout_ms \
|
||||
FROM chain c \
|
||||
JOIN interceptors i ON (i.app_id = c.app_owner OR i.group_id = c.group_owner) \
|
||||
ORDER BY i.service, i.op, i.phase, c.depth ASC",
|
||||
@@ -82,12 +87,15 @@ pub async fn list_on_app_chain(
|
||||
.await?;
|
||||
Ok(rows
|
||||
.into_iter()
|
||||
.map(|(service, op, phase, script)| InterceptorMarker {
|
||||
service,
|
||||
op,
|
||||
phase,
|
||||
script,
|
||||
})
|
||||
.map(
|
||||
|(service, op, phase, script, timeout_ms)| InterceptorMarker {
|
||||
service,
|
||||
op,
|
||||
phase,
|
||||
script,
|
||||
timeout_ms,
|
||||
},
|
||||
)
|
||||
.collect())
|
||||
}
|
||||
|
||||
@@ -100,6 +108,8 @@ pub struct SealedInterceptor {
|
||||
pub marker_app: Option<Uuid>,
|
||||
pub marker_group: Option<Uuid>,
|
||||
pub script_name: String,
|
||||
/// §9.4 M5: per-hook wall-clock timeout, or `None` for the instance default.
|
||||
pub timeout_ms: Option<i32>,
|
||||
/// `(script_id, source, updated_at)` of the sealed script, or `None` if it
|
||||
/// is missing/disabled at the declaring owner.
|
||||
pub script: Option<(Uuid, String, DateTime<Utc>)>,
|
||||
@@ -157,6 +167,7 @@ pub async fn resolve_chain(
|
||||
Option<Uuid>, // marker_group
|
||||
String, // script_name
|
||||
String, // phase
|
||||
Option<i32>, // timeout_ms
|
||||
i32, // depth (0 = app, larger = ancestor)
|
||||
Option<Uuid>, // script id
|
||||
Option<String>, // script source
|
||||
@@ -166,12 +177,12 @@ pub async fn resolve_chain(
|
||||
markers AS ( \
|
||||
SELECT i.interceptor_script AS script_name, \
|
||||
i.app_id AS marker_app, i.group_id AS marker_group, \
|
||||
i.phase AS phase, c.depth AS depth \
|
||||
i.phase AS phase, i.timeout_ms AS timeout_ms, c.depth AS depth \
|
||||
FROM chain c \
|
||||
JOIN interceptors i ON (i.app_id = c.app_owner OR i.group_id = c.group_owner) \
|
||||
WHERE i.service = $2 AND i.op = $3 \
|
||||
) \
|
||||
SELECT m.marker_app, m.marker_group, m.script_name, m.phase, m.depth, \
|
||||
SELECT m.marker_app, m.marker_group, m.script_name, m.phase, m.timeout_ms, m.depth, \
|
||||
s.id, s.source, s.updated_at \
|
||||
FROM markers m \
|
||||
LEFT JOIN scripts s ON ( \
|
||||
@@ -191,11 +202,12 @@ pub async fn resolve_chain(
|
||||
// depth = ancestor runs first); `after` app→ancestor = depth ASC.
|
||||
let mut before: Vec<(i32, SealedInterceptor)> = Vec::new();
|
||||
let mut after: Vec<(i32, SealedInterceptor)> = Vec::new();
|
||||
for (marker_app, marker_group, script_name, phase, depth, sid, src, upd) in rows {
|
||||
for (marker_app, marker_group, script_name, phase, timeout_ms, depth, sid, src, upd) in rows {
|
||||
let sealed = SealedInterceptor {
|
||||
marker_app,
|
||||
marker_group,
|
||||
script_name,
|
||||
timeout_ms,
|
||||
script: match (sid, src, upd) {
|
||||
(Some(id), Some(source), Some(updated_at)) => Some((id, source, updated_at)),
|
||||
_ => None,
|
||||
@@ -225,37 +237,43 @@ pub async fn insert_interceptor_tx(
|
||||
op: &str,
|
||||
phase: &str,
|
||||
script: &str,
|
||||
timeout_ms: Option<i32>,
|
||||
) -> Result<(), sqlx::Error> {
|
||||
match owner {
|
||||
ScriptOwner::App(a) => {
|
||||
sqlx::query(
|
||||
// The conflict arbiter includes `phase` to match the
|
||||
// per-(owner, service, op, phase) index (§9.4 M3).
|
||||
"INSERT INTO interceptors (app_id, service, op, phase, interceptor_script) \
|
||||
VALUES ($1, $2, $3, $4, $5) \
|
||||
// per-(owner, service, op, phase) index (§9.4 M3). A re-apply
|
||||
// that changes only the script or timeout updates in place.
|
||||
"INSERT INTO interceptors (app_id, service, op, phase, interceptor_script, timeout_ms) \
|
||||
VALUES ($1, $2, $3, $4, $5, $6) \
|
||||
ON CONFLICT (app_id, service, op, phase) WHERE app_id IS NOT NULL \
|
||||
DO UPDATE SET interceptor_script = EXCLUDED.interceptor_script, updated_at = NOW()",
|
||||
DO UPDATE SET interceptor_script = EXCLUDED.interceptor_script, \
|
||||
timeout_ms = EXCLUDED.timeout_ms, updated_at = NOW()",
|
||||
)
|
||||
.bind(a.into_inner())
|
||||
.bind(service)
|
||||
.bind(op)
|
||||
.bind(phase)
|
||||
.bind(script)
|
||||
.bind(timeout_ms)
|
||||
.execute(&mut **tx)
|
||||
.await?;
|
||||
}
|
||||
ScriptOwner::Group(g) => {
|
||||
sqlx::query(
|
||||
"INSERT INTO interceptors (group_id, service, op, phase, interceptor_script) \
|
||||
VALUES ($1, $2, $3, $4, $5) \
|
||||
"INSERT INTO interceptors (group_id, service, op, phase, interceptor_script, timeout_ms) \
|
||||
VALUES ($1, $2, $3, $4, $5, $6) \
|
||||
ON CONFLICT (group_id, service, op, phase) WHERE group_id IS NOT NULL \
|
||||
DO UPDATE SET interceptor_script = EXCLUDED.interceptor_script, updated_at = NOW()",
|
||||
DO UPDATE SET interceptor_script = EXCLUDED.interceptor_script, \
|
||||
timeout_ms = EXCLUDED.timeout_ms, updated_at = NOW()",
|
||||
)
|
||||
.bind(g.into_inner())
|
||||
.bind(service)
|
||||
.bind(op)
|
||||
.bind(phase)
|
||||
.bind(script)
|
||||
.bind(timeout_ms)
|
||||
.execute(&mut **tx)
|
||||
.await?;
|
||||
}
|
||||
|
||||
@@ -33,6 +33,9 @@ impl InterceptorServiceImpl {
|
||||
/// (`cx.app_id`); only the `owner` is the sealing owner.
|
||||
fn map_entry(app_id: picloud_shared::AppId, sealed: SealedInterceptor) -> InterceptorEntry {
|
||||
let owner = sealed.sealing_owner();
|
||||
// A negative/oversized DB value is impossible (CHECK > 0), but clamp
|
||||
// defensively to `None` (→ instance default) rather than panic.
|
||||
let timeout_ms = sealed.timeout_ms.and_then(|v| u32::try_from(v).ok());
|
||||
match sealed.script {
|
||||
Some((sid, source, updated_at)) => InterceptorEntry::Run(ResolvedInterceptor {
|
||||
script: Box::new(ResolvedScript {
|
||||
@@ -43,7 +46,7 @@ impl InterceptorServiceImpl {
|
||||
updated_at,
|
||||
name: sealed.script_name,
|
||||
}),
|
||||
timeout_ms: None,
|
||||
timeout_ms,
|
||||
}),
|
||||
None => InterceptorEntry::Dangling(sealed.script_name),
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user