diff --git a/crates/manager-core/src/route_admin.rs b/crates/manager-core/src/route_admin.rs index b6b1a5b..492945b 100644 --- a/crates/manager-core/src/route_admin.rs +++ b/crates/manager-core/src/route_admin.rs @@ -184,6 +184,17 @@ async fn create_route( ) .await?; + // v1.1.3: module scripts have no executable entry point — they're + // libraries imported by other scripts. Reject route bindings here + // before we touch the routes table. + if script.kind == picloud_shared::ScriptKind::Module { + return Err(RouteApiError::BadRequest(format!( + "script {script_id} has kind=module; modules are imported, \ + not bound to routes — switch the script to kind=endpoint \ + or attach this route to a different script" + ))); + } + // Validate the route's host is consistent with one of the app's // domain claims. `HostKind::Any` is always permitted (catches every // host the app already owns). Specific hosts must match a claim. diff --git a/crates/manager-core/src/triggers_api.rs b/crates/manager-core/src/triggers_api.rs index b73fdb2..20981d2 100644 --- a/crates/manager-core/src/triggers_api.rs +++ b/crates/manager-core/src/triggers_api.rs @@ -16,12 +16,13 @@ use axum::http::StatusCode; use axum::response::{IntoResponse, Json, Response}; use axum::routing::{delete, get, post}; use axum::{Extension, Router}; -use picloud_shared::{AppId, DocsEventOp, KvEventOp, Principal, ScriptId, TriggerId}; +use picloud_shared::{AppId, DocsEventOp, KvEventOp, Principal, ScriptId, ScriptKind, TriggerId}; use serde::{Deserialize, Serialize}; use serde_json::json; use crate::app_repo::AppRepository; use crate::authz::{require, AuthzDenied, AuthzError, AuthzRepo, Capability}; +use crate::repo::{ScriptRepository, ScriptRepositoryError}; use crate::trigger_config::{BackoffShape, TriggerConfig}; use crate::trigger_repo::{ CreateDeadLetterTrigger, CreateDocsTrigger, CreateKvTrigger, Trigger, TriggerDispatchMode, @@ -33,6 +34,11 @@ pub struct TriggersState { pub triggers: Arc, pub apps: Arc, pub authz: Arc, + /// v1.1.3: trigger creation must verify the target script (1) exists, + /// (2) belongs to this app, and (3) is `kind = endpoint` — modules + /// cannot be invoked. The script-load lives in the handler, so the + /// state needs a repo handle. + pub scripts: Arc, /// Defaults applied to created triggers when the request omits /// retry settings. Kept on the state struct so tests can swap /// in a stricter / looser config without env tinkering. @@ -146,6 +152,44 @@ async fn list_triggers( Ok(Json(TriggerListResponse { triggers })) } +/// v1.1.3: shared check used by every trigger-create handler. Returns +/// `Ok(())` when the target script exists, lives in the same app, and +/// is `kind = endpoint`. Wrong app surfaces as 422 (not 404) so we +/// don't leak whether a script id exists in some other app. +async fn validate_trigger_target( + scripts: &dyn ScriptRepository, + app_id: AppId, + script_id: ScriptId, +) -> Result<(), TriggersApiError> { + let script = scripts + .get(script_id) + .await + .map_err(map_script_repo_err)? + .ok_or_else(|| { + TriggersApiError::Invalid(format!("script {script_id} not found in this app")) + })?; + if script.app_id != app_id { + return Err(TriggersApiError::Invalid(format!( + "script {script_id} does not belong to this app" + ))); + } + if script.kind == ScriptKind::Module { + return Err(TriggersApiError::Invalid(format!( + "script {script_id} has kind=module; modules cannot be trigger targets — \ + switch the script to kind=endpoint or attach this trigger to a different script" + ))); + } + Ok(()) +} + +fn map_script_repo_err(e: ScriptRepositoryError) -> TriggersApiError { + // Surface as Invalid so the wire shape (422 with `error` field) + // stays consistent with the other trigger-validation failures. + // The underlying DB error is still logged through the manager's + // tracing instrumentation. + TriggersApiError::Invalid(format!("script lookup failed: {e}")) +} + async fn create_kv_trigger( State(s): State, Extension(principal): Extension, @@ -165,6 +209,7 @@ async fn create_kv_trigger( "collection_glob must not be empty".into(), )); } + validate_trigger_target(&*s.scripts, app_id, input.script_id).await?; let req = CreateKvTrigger { script_id: input.script_id, @@ -201,6 +246,7 @@ async fn create_docs_trigger( "collection_glob must not be empty".into(), )); } + validate_trigger_target(&*s.scripts, app_id, input.script_id).await?; let req = CreateDocsTrigger { script_id: input.script_id, @@ -231,6 +277,7 @@ async fn create_dl_trigger( Capability::AppManageTriggers(app_id), ) .await?; + validate_trigger_target(&*s.scripts, app_id, input.script_id).await?; let req = CreateDeadLetterTrigger { script_id: input.script_id, source_filter: input.source_filter, @@ -628,6 +675,123 @@ mod tests { } } + /// Minimal `ScriptRepository` impl backing the trigger-create + /// handler's `validate_trigger_target` check. Tests insert one or + /// more scripts via [`InMemoryScriptRepo::with_endpoint`] / + /// [`with_module`] and pass it into `TriggersState`. + struct InMemoryScriptRepo { + existing: Mutex>, + } + + impl InMemoryScriptRepo { + fn empty() -> Arc { + Arc::new(Self { + existing: Mutex::new(HashMap::new()), + }) + } + fn with_endpoint(app_id: AppId, script_id: ScriptId) -> Arc { + Self::with(app_id, script_id, ScriptKind::Endpoint) + } + fn with_module(app_id: AppId, script_id: ScriptId) -> Arc { + Self::with(app_id, script_id, ScriptKind::Module) + } + fn with(app_id: AppId, script_id: ScriptId, kind: ScriptKind) -> Arc { + let now = Utc::now(); + let mut existing = HashMap::new(); + existing.insert( + script_id, + picloud_shared::Script { + id: script_id, + app_id, + name: format!( + "{}_{}", + match kind { + ScriptKind::Endpoint => "endpoint", + ScriptKind::Module => "module", + }, + script_id + ), + description: None, + version: 1, + source: String::new(), + kind, + timeout_seconds: 30, + sandbox: Default::default(), + memory_limit_mb: 256, + created_at: now, + updated_at: now, + }, + ); + Arc::new(Self { + existing: Mutex::new(existing), + }) + } + } + + #[async_trait] + impl ScriptRepository for InMemoryScriptRepo { + async fn get( + &self, + id: ScriptId, + ) -> Result, crate::repo::ScriptRepositoryError> { + Ok(self.existing.lock().await.get(&id).cloned()) + } + async fn list( + &self, + ) -> Result, crate::repo::ScriptRepositoryError> { + Ok(self.existing.lock().await.values().cloned().collect()) + } + async fn list_for_app( + &self, + _app_id: AppId, + ) -> Result, crate::repo::ScriptRepositoryError> { + unimplemented!() + } + async fn list_for_user( + &self, + _user_id: AdminUserId, + ) -> Result, crate::repo::ScriptRepositoryError> { + unimplemented!() + } + async fn create( + &self, + _input: crate::repo::NewScript, + ) -> Result { + unimplemented!() + } + async fn update( + &self, + _id: ScriptId, + _patch: crate::repo::ScriptPatch, + ) -> Result { + unimplemented!() + } + async fn delete( + &self, + _id: ScriptId, + ) -> Result<(), crate::repo::ScriptRepositoryError> { + unimplemented!() + } + async fn count_routes_for_script( + &self, + _script_id: ScriptId, + ) -> Result { + Ok(0) + } + async fn count_triggers_for_script( + &self, + _script_id: ScriptId, + ) -> Result { + Ok(0) + } + async fn list_imports( + &self, + _script_id: ScriptId, + ) -> Result, crate::repo::ScriptRepositoryError> { + Ok(vec![]) + } + } + struct AlwaysAllowAuthzRepo; #[async_trait] impl AuthzRepo for AlwaysAllowAuthzRepo { @@ -666,6 +830,24 @@ mod tests { triggers: Arc::new(InMemoryTriggerRepo::default()), apps: InMemoryAppRepo::with(app_id), authz, + scripts: InMemoryScriptRepo::empty(), + config: TriggerConfig::conservative(), + } + } + + /// Like [`state_with`] but pre-populates the script repo with a + /// single endpoint script (so the v1.1.3 `validate_trigger_target` + /// check passes and tests can exercise downstream behavior). + fn state_with_endpoint( + authz: Arc, + app_id: AppId, + script_id: ScriptId, + ) -> TriggersState { + TriggersState { + triggers: Arc::new(InMemoryTriggerRepo::default()), + apps: InMemoryAppRepo::with(app_id), + authz, + scripts: InMemoryScriptRepo::with_endpoint(app_id, script_id), config: TriggerConfig::conservative(), } } @@ -718,7 +900,8 @@ mod tests { #[tokio::test] async fn kv_trigger_uses_env_defaults_when_omitted() { let app_id = AppId::new(); - let mut state = state_with(Arc::new(AlwaysAllowAuthzRepo), app_id); + let script_id = ScriptId::new(); + let mut state = state_with_endpoint(Arc::new(AlwaysAllowAuthzRepo), app_id, script_id); // Tweak the config so we can detect that defaults were used. state.config.retry_max_attempts = 7; state.config.retry_base_ms = 12_345; @@ -727,7 +910,7 @@ mod tests { Extension(member_principal()), Path(app_id), Json(CreateKvTriggerRequest { - script_id: ScriptId::new(), + script_id, collection_glob: "widgets".into(), ops: vec![KvEventOp::Insert], dispatch_mode: TriggerDispatchMode::Async, @@ -769,13 +952,14 @@ mod tests { #[tokio::test] async fn docs_trigger_create_succeeds() { let app_id = AppId::new(); - let state = state_with(Arc::new(AlwaysAllowAuthzRepo), app_id); + let script_id = ScriptId::new(); + let state = state_with_endpoint(Arc::new(AlwaysAllowAuthzRepo), app_id, script_id); let (status, Json(trigger)) = create_docs_trigger( State(state), Extension(member_principal()), Path(app_id), Json(CreateDocsTriggerRequest { - script_id: ScriptId::new(), + script_id, collection_glob: "users".into(), ops: vec![DocsEventOp::Create, DocsEventOp::Update], dispatch_mode: TriggerDispatchMode::Async, diff --git a/crates/picloud/src/lib.rs b/crates/picloud/src/lib.rs index 99dac87..72aa451 100644 --- a/crates/picloud/src/lib.rs +++ b/crates/picloud/src/lib.rs @@ -217,7 +217,7 @@ pub async fn build_app(pool: PgPool, auth: AuthDeps) -> anyhow::Result { }; let route_admin = RouteAdminState { routes: route_repo.clone(), - scripts: Arc::new(PostgresScriptRepoHandle(script_repo)), + scripts: Arc::new(PostgresScriptRepoHandle(script_repo.clone())), domains: domains_repo.clone(), table: route_table.clone(), authz: authz.clone(), @@ -245,6 +245,7 @@ pub async fn build_app(pool: PgPool, auth: AuthDeps) -> anyhow::Result { triggers: trigger_repo, apps: apps_repo.clone(), authz: authz.clone(), + scripts: Arc::new(PostgresScriptRepoHandle(script_repo.clone())), config: trigger_config, }; let dead_letters_state = DeadLettersState {