diff --git a/crates/manager-core/migrations/0050_group_scripts.sql b/crates/manager-core/migrations/0050_group_scripts.sql new file mode 100644 index 0000000..61a02c7 --- /dev/null +++ b/crates/manager-core/migrations/0050_group_scripts.sql @@ -0,0 +1,40 @@ +-- Phase 4 (v1.2 Hierarchies): group-owned scripts. +-- +-- Until now every script was owned by exactly one app (`app_id NOT NULL`, +-- ON DELETE RESTRICT). Phase 4 lets a script be owned by a GROUP instead, so +-- it is shared by every descendant app — resolved by name, nearest-owner +-- wins (CoW: an app's own script of the same name shadows the inherited one), +-- exactly like `vars`/`secrets` (0048/0049). +-- +-- Phase 4-LITE scope: group-owned ENDPOINT scripts only. Group modules and +-- the origin-aware (lexical) import resolver are deferred to Phase 4b, so a +-- group-owned script must be self-contained (enforced in the service layer). +-- +-- Reshape (mirrors vars/secrets, but RESTRICT not CASCADE — code is not data, +-- a group can't be deleted out from under the scripts it owns, matching the +-- existing app_id RESTRICT and the §5.6 delete=RESTRICT rule): +-- * `group_id` — nullable FK→groups, ON DELETE RESTRICT. +-- * `app_id` — made nullable (was NOT NULL); the exactly-one CHECK now +-- enforces "owned by an app XOR a group". +-- * the per-app unique name index becomes two partial indexes, one per owner. + +ALTER TABLE scripts + ADD COLUMN group_id UUID REFERENCES groups(id) ON DELETE RESTRICT; + +ALTER TABLE scripts + ALTER COLUMN app_id DROP NOT NULL; + +ALTER TABLE scripts + ADD CONSTRAINT scripts_owner_exactly_one + CHECK ((group_id IS NULL) <> (app_id IS NULL)); + +-- Per-owner, case-insensitive name uniqueness. Partial so each owner column +-- only constrains its own rows; existing app rows (group_id NULL) keep the +-- exact same (app_id, LOWER(name)) uniqueness they had under the old index. +DROP INDEX scripts_name_uidx; +CREATE UNIQUE INDEX scripts_app_name_uidx + ON scripts (app_id, LOWER(name)) WHERE app_id IS NOT NULL; +CREATE UNIQUE INDEX scripts_group_name_uidx + ON scripts (group_id, LOWER(name)) WHERE group_id IS NOT NULL; + +CREATE INDEX scripts_group_id_idx ON scripts (group_id) WHERE group_id IS NOT NULL; diff --git a/crates/manager-core/src/api.rs b/crates/manager-core/src/api.rs index 275e63a..067c00b 100644 --- a/crates/manager-core/src/api.rs +++ b/crates/manager-core/src/api.rs @@ -181,6 +181,15 @@ async fn resolve_app_ident(apps: &dyn AppRepository, ident: &str) -> Result Result { + script.app_id.ok_or(ApiError::NotFound(script.id)) +} + async fn get_script( State(state): State>, Extension(principal): Extension, @@ -190,7 +199,7 @@ async fn get_script( require( state.authz.as_ref(), &principal, - Capability::AppRead(script.app_id), + Capability::AppRead(script_app(&script)?), ) .await?; Ok(Json(script)) @@ -234,7 +243,8 @@ async fn create_script( let created = state .repo .create(NewScript { - app_id: input.app_id, + app_id: Some(input.app_id), + group_id: None, name: input.name, description: input.description, source: input.source, @@ -291,7 +301,7 @@ async fn update_script( require( state.authz.as_ref(), &principal, - Capability::AppWriteScript(script.app_id), + Capability::AppWriteScript(script_app(&script)?), ) .await?; @@ -367,7 +377,7 @@ async fn delete_script( require( state.authz.as_ref(), &principal, - Capability::AppAdmin(script.app_id), + Capability::AppAdmin(script_app(&script)?), ) .await?; state.repo.delete(id).await?; @@ -408,7 +418,7 @@ async fn list_logs( require( state.authz.as_ref(), &principal, - Capability::AppLogRead(script.app_id), + Capability::AppLogRead(script_app(&script)?), ) .await?; // Cap to keep the dashboard responsive; the data plane writes are diff --git a/crates/manager-core/src/app_bootstrap.rs b/crates/manager-core/src/app_bootstrap.rs index 77f2dd5..4a2df42 100644 --- a/crates/manager-core/src/app_bootstrap.rs +++ b/crates/manager-core/src/app_bootstrap.rs @@ -60,7 +60,8 @@ async fn seed_into( ) -> Result<(), ScriptRepositoryError> { let script = scripts .create(NewScript { - app_id: default.id, + app_id: Some(default.id), + group_id: None, name: "hello".to_string(), description: Some("Reference example: returns a greeting at GET /hello.".to_string()), source: HELLO_RHAI_SOURCE.to_string(), diff --git a/crates/manager-core/src/apply_service.rs b/crates/manager-core/src/apply_service.rs index 5e16a48..7028f4b 100644 --- a/crates/manager-core/src/apply_service.rs +++ b/crates/manager-core/src/apply_service.rs @@ -519,7 +519,8 @@ impl ApplyService { Op::Create => { let bs = bundle_scripts[&key]; let new = NewScript { - app_id, + app_id: Some(app_id), + group_id: None, name: bs.name.clone(), description: bs.description.clone(), source: bs.source.clone(), @@ -1990,7 +1991,8 @@ mod tests { fn script(name: &str, source: &str) -> Script { Script { id: ScriptId::from(uuid::Uuid::new_v4()), - app_id: AppId::from(uuid::Uuid::nil()), + app_id: Some(AppId::from(uuid::Uuid::nil())), + group_id: None, name: name.to_string(), description: None, version: 1, diff --git a/crates/manager-core/src/dispatcher.rs b/crates/manager-core/src/dispatcher.rs index 96b0c52..ee64839 100644 --- a/crates/manager-core/src/dispatcher.rs +++ b/crates/manager-core/src/dispatcher.rs @@ -359,9 +359,14 @@ impl Dispatcher { // existing consumers. Dead-letter the message so the misfire is // observable rather than executing one app's script under // another app's SdkCallCx. - if script.app_id != claimed.app_id { + // Phase 4: a group-owned script (`app_id: None`) is not yet + // dispatchable here — the chain-membership check that authorizes + // inherited scripts lands with group-script binding. Until then + // `is_owned_by_app` is the exact app-owned guard as before and a + // group script fails closed (dead-lettered, not silently run). + if !script.is_owned_by_app(claimed.app_id) { tracing::error!( - script_app = %script.app_id, + script_app = ?script.app_id, claim_app = %claimed.app_id, script_id = %consumer.script_id, "queue consumer script belongs to a different app; dead-lettering" @@ -786,7 +791,9 @@ impl Dispatcher { // boundary. Not reachable via the trigger-create or `apply` paths // (both resolve the script within the app's own scope), so this is // the runtime backstop the other arms already carry. - if script.app_id != row.app_id { + // Phase 4: group scripts (`app_id: None`) fail closed here until the + // chain-membership check lands with group-script binding. + if !script.is_owned_by_app(row.app_id) { return Err(DispatcherError::ResolveTrigger( "trigger outbox target belongs to a different app".into(), )); @@ -876,7 +883,8 @@ impl Dispatcher { // build_invoke_request and dispatch_one_queue. A hand-edited // outbox row could otherwise execute one app's script under // another app's SdkCallCx. - if script.app_id != row.app_id { + // Phase 4: group scripts fail closed here (see queue/trigger arms). + if !script.is_owned_by_app(row.app_id) { return Err(DispatcherError::ResolveTrigger( "http outbox target belongs to a different app".into(), )); @@ -969,7 +977,8 @@ impl Dispatcher { })?; // Same-app guard — the script could have been re-assigned or // the row could have been hand-edited. Reject mismatches. - if script.app_id != row.app_id { + // Phase 4: group scripts fail closed here (see queue/trigger arms). + if !script.is_owned_by_app(row.app_id) { return Err(DispatcherError::ResolveTrigger( "invoke target belongs to a different app".into(), )); @@ -2097,7 +2106,8 @@ mod tests { pub(super) fn disabled_script(app_id: AppId) -> Script { Script { id: ScriptId::new(), - app_id, + app_id: Some(app_id), + group_id: None, name: "worker".into(), description: None, version: 1, diff --git a/crates/manager-core/src/invoke_service.rs b/crates/manager-core/src/invoke_service.rs index 68ee612..d518884 100644 --- a/crates/manager-core/src/invoke_service.rs +++ b/crates/manager-core/src/invoke_service.rs @@ -80,7 +80,11 @@ impl InvokeServiceImpl { .await .map_err(|e| InvokeError::Backend(e.to_string()))? .ok_or_else(|| InvokeError::NotFound(format!("id {script_id}")))?; - if script.app_id != cx.app_id { + // Phase 4: group scripts (`app_id: None`) fail closed here — invoke() + // by-id stays app-owned until chain-aware resolution lands. The + // execution context is the *caller's* app (`cx.app_id`), never read + // off the target script. + if !script.is_owned_by_app(cx.app_id) { return Err(InvokeError::CrossApp); } if !script.enabled { @@ -90,7 +94,7 @@ impl InvokeServiceImpl { } Ok(ResolvedScript { script_id: script.id, - app_id: script.app_id, + app_id: cx.app_id, source: script.source, updated_at: script.updated_at, name: script.name, @@ -113,7 +117,9 @@ impl InvokeServiceImpl { } Ok(ResolvedScript { script_id: script.id, - app_id: script.app_id, + // Resolved within `cx.app_id`'s own scope (get_by_name filters by + // it), so the execution-context app is the caller's app. + app_id: cx.app_id, source: script.source, updated_at: script.updated_at, name: script.name, @@ -227,7 +233,7 @@ mod tests { name: &str, ) -> Result, ScriptRepositoryError> { Ok( - if app_id == self.script.app_id && name == self.script.name { + if self.script.app_id == Some(app_id) && name == self.script.name { Some(self.script.clone()) } else { None @@ -312,7 +318,8 @@ mod tests { fn make_script(app_id: AppId, name: &str) -> Script { Script { id: ScriptId::new(), - app_id, + app_id: Some(app_id), + group_id: None, name: name.into(), description: None, version: 1, diff --git a/crates/manager-core/src/repo.rs b/crates/manager-core/src/repo.rs index b2c5f65..c10522f 100644 --- a/crates/manager-core/src/repo.rs +++ b/crates/manager-core/src/repo.rs @@ -3,7 +3,7 @@ use std::collections::BTreeMap; use async_trait::async_trait; use picloud_orchestrator_core::{ResolverError, ScriptResolver}; use picloud_shared::{ - AdminUserId, AppId, ExecutionLog, ExecutionSource, ExecutionStatus, RequestId, Script, + AdminUserId, AppId, ExecutionLog, ExecutionSource, ExecutionStatus, GroupId, RequestId, Script, ScriptId, ScriptKind, ScriptSandbox, }; use sqlx::PgPool; @@ -142,7 +142,13 @@ impl ScriptRepository for std::sync::Arc { /// constraints; the repo enforces them in the DB regardless. #[derive(Debug, Clone)] pub struct NewScript { - pub app_id: AppId, + /// App owner (the common case). Exactly one of `app_id`/`group_id` must + /// be `Some` — the DB CHECK is the backstop, but callers should uphold + /// it. App-owned creation continues to pass `Some(app_id)`, `group_id: + /// None`; group-owned creation (Phase 4) inverts that. + pub app_id: Option, + /// Group owner (Phase 4). See [`NewScript::app_id`]. + pub group_id: Option, pub name: String, pub description: Option, pub source: String, @@ -206,7 +212,7 @@ impl PostgresScriptRepository { /// Columns selected from `scripts` everywhere — kept in one constant so /// adding `kind` (v1.1.3) and future columns can't accidentally skip /// one query. -const SCRIPT_SELECT_COLS: &str = "id, app_id, name, description, version, source, kind, \ +const SCRIPT_SELECT_COLS: &str = "id, app_id, group_id, name, description, version, source, kind, \ timeout_seconds, memory_limit_mb, sandbox, enabled, \ created_at, updated_at"; @@ -397,12 +403,13 @@ pub(crate) async fn insert_script_tx( .unwrap_or_else(|_| serde_json::json!({})); let res = sqlx::query_as::<_, ScriptRow>(&format!( "INSERT INTO scripts ( \ - app_id, name, description, source, kind, \ + app_id, group_id, name, description, source, kind, \ timeout_seconds, memory_limit_mb, sandbox, enabled \ - ) VALUES ($1, $2, $3, $4, $5, COALESCE($6, 30), COALESCE($7, 256), $8, $9) \ + ) VALUES ($1, $2, $3, $4, $5, $6, COALESCE($7, 30), COALESCE($8, 256), $9, $10) \ RETURNING {SCRIPT_SELECT_COLS}" )) - .bind(input.app_id.into_inner()) + .bind(input.app_id.map(AppId::into_inner)) + .bind(input.group_id.map(GroupId::into_inner)) .bind(&input.name) .bind(input.description.as_deref()) .bind(&input.source) @@ -417,13 +424,18 @@ pub(crate) async fn insert_script_tx( Ok(row) => row.into(), Err(sqlx::Error::Database(e)) if e.is_unique_violation() => { return Err(ScriptRepositoryError::Conflict(format!( - "a script named {:?} already exists in this app", + "a script named {:?} already exists in this owner", input.name ))); } Err(e) => return Err(e.into()), }; - replace_imports_tx(tx, script.id, script.app_id, &input.imports).await?; + // Module imports are resolved within the owning app's script set. Group + // scripts must be self-contained in Phase 4-lite (the origin-aware import + // resolver is Phase 4b), so only app-owned scripts wire up import edges. + if let Some(app_id) = script.app_id { + replace_imports_tx(tx, script.id, app_id, &input.imports).await?; + } Ok(script) } @@ -476,7 +488,9 @@ pub(crate) async fn update_script_tx( Err(e) => return Err(e.into()), }; if let Some(imports) = patch.imports.as_deref() { - replace_imports_tx(tx, script.id, script.app_id, imports).await?; + if let Some(app_id) = script.app_id { + replace_imports_tx(tx, script.id, app_id, imports).await?; + } } Ok(script) } @@ -501,7 +515,10 @@ pub(crate) async fn delete_script_tx( #[derive(sqlx::FromRow)] struct ScriptRow { id: uuid::Uuid, - app_id: uuid::Uuid, + /// Polymorphic owner (Phase 4): exactly one of `app_id`/`group_id` is + /// non-NULL (DB CHECK). App-owned rows keep `app_id` set as before. + app_id: Option, + group_id: Option, name: String, description: Option, version: i32, @@ -531,7 +548,8 @@ impl From for Script { let kind = ScriptKind::parse_str(&r.kind).unwrap_or(ScriptKind::Endpoint); Self { id: r.id.into(), - app_id: r.app_id.into(), + app_id: r.app_id.map(Into::into), + group_id: r.group_id.map(Into::into), name: r.name, description: r.description, version: r.version, diff --git a/crates/manager-core/src/route_admin.rs b/crates/manager-core/src/route_admin.rs index f34cc93..b7f36f3 100644 --- a/crates/manager-core/src/route_admin.rs +++ b/crates/manager-core/src/route_admin.rs @@ -148,10 +148,16 @@ async fn list_routes( .get(script_id) .await? .ok_or(RouteApiError::ScriptNotFound(script_id))?; + // Phase 4: routes bind to app-owned scripts; a group-owned script + // (`app_id: None`) is not addressable here (binding lands in C3), so it + // reads as a missing script. + let app_id = script + .app_id + .ok_or(RouteApiError::ScriptNotFound(script_id))?; require( state.authz.as_ref(), &principal, - Capability::AppRead(script.app_id), + Capability::AppRead(app_id), ) .await?; Ok(Json(state.routes.list_for_script(script_id).await?)) @@ -176,7 +182,11 @@ async fn create_route( .get(script_id) .await? .ok_or(RouteApiError::ScriptNotFound(script_id))?; - let app_id = script.app_id; + // Phase 4: only app-owned scripts are route-bindable for now (group + // binding is C3); a group script reads as missing here. + let app_id = script + .app_id + .ok_or(RouteApiError::ScriptNotFound(script_id))?; require( state.authz.as_ref(), &principal, diff --git a/crates/manager-core/src/triggers_api.rs b/crates/manager-core/src/triggers_api.rs index 9b9da89..5194e89 100644 --- a/crates/manager-core/src/triggers_api.rs +++ b/crates/manager-core/src/triggers_api.rs @@ -289,7 +289,10 @@ async fn validate_trigger_target( .ok_or_else(|| { TriggersApiError::Invalid(format!("script {script_id} not found in this app")) })?; - if script.app_id != app_id { + // Phase 4: trigger targets are app-owned scripts; a group-owned script + // (`app_id: None`) fails closed here until group-script trigger binding + // lands in C3. + if !script.is_owned_by_app(app_id) { return Err(TriggersApiError::Invalid(format!( "script {script_id} does not belong to this app" ))); @@ -1348,7 +1351,8 @@ mod tests { script_id, picloud_shared::Script { id: script_id, - app_id, + app_id: Some(app_id), + group_id: None, name: format!( "{}_{}", match kind { @@ -1393,7 +1397,7 @@ mod tests { .lock() .await .values() - .find(|s| s.app_id == app_id && s.name == name) + .find(|s| s.app_id == Some(app_id) && s.name == name) .cloned()) } async fn list( diff --git a/crates/manager-core/tests/expected_schema.txt b/crates/manager-core/tests/expected_schema.txt index 7580be8..a1bdbba 100644 --- a/crates/manager-core/tests/expected_schema.txt +++ b/crates/manager-core/tests/expected_schema.txt @@ -132,6 +132,8 @@ table: apps description: text NULL created_at: timestamp with time zone NOT NULL default=now() updated_at: timestamp with time zone NOT NULL default=now() + group_id: uuid NOT NULL + environment: text NULL table: cron_trigger_details trigger_id: uuid NOT NULL @@ -212,6 +214,23 @@ table: files_trigger_details collection_glob: text NOT NULL ops: ARRAY NOT NULL +table: group_members + group_id: uuid NOT NULL + user_id: uuid NOT NULL + role: text NOT NULL + created_at: timestamp with time zone NOT NULL default=now() + +table: groups + id: uuid NOT NULL default=gen_random_uuid() + parent_id: uuid NULL + slug: text NOT NULL + name: text NOT NULL + description: text NULL + structure_version: bigint NOT NULL default=1 + owner_project: uuid NULL + created_at: timestamp with time zone NOT NULL default=now() + updated_at: timestamp with time zone NOT NULL default=now() + table: kv_entries app_id: uuid NOT NULL collection: text NOT NULL @@ -277,6 +296,7 @@ table: routes created_at: timestamp with time zone NOT NULL default=now() app_id: uuid NOT NULL dispatch_mode: text NOT NULL default='sync'::text + enabled: boolean NOT NULL default=true table: script_imports app_id: uuid NOT NULL @@ -295,17 +315,21 @@ table: scripts created_at: timestamp with time zone NOT NULL default=now() updated_at: timestamp with time zone NOT NULL default=now() sandbox: jsonb NOT NULL default='{}'::jsonb - app_id: uuid NOT NULL + app_id: uuid NULL kind: text NOT NULL default='endpoint'::text + enabled: boolean NOT NULL default=true + group_id: uuid NULL table: secrets - app_id: uuid NOT NULL + app_id: uuid NULL name: text NOT NULL encrypted_value: bytea NOT NULL nonce: bytea NOT NULL created_at: timestamp with time zone NOT NULL default=now() updated_at: timestamp with time zone NOT NULL default=now() version: smallint NOT NULL default=0 + group_id: uuid NULL + environment_scope: text NOT NULL default='*'::text table: topics app_id: uuid NOT NULL @@ -328,6 +352,18 @@ table: triggers registered_by_principal: uuid NOT NULL created_at: timestamp with time zone NOT NULL default=now() updated_at: timestamp with time zone NOT NULL default=now() + name: text NOT NULL default=(gen_random_uuid())::text + +table: vars + id: uuid NOT NULL default=gen_random_uuid() + group_id: uuid NULL + app_id: uuid NULL + environment_scope: text NOT NULL default='*'::text + key: text NOT NULL + value: jsonb NOT NULL + is_tombstone: boolean NOT NULL default=false + created_at: timestamp with time zone NOT NULL default=now() + updated_at: timestamp with time zone NOT NULL default=now() ## indexes @@ -395,6 +431,7 @@ indexes on app_users: idx_app_users_app_email_lower: public.app_users USING btree (app_id, lower(email)) indexes on apps: + apps_group_id_idx: public.apps USING btree (group_id) apps_pkey: public.apps USING btree (id) apps_slug_key: public.apps USING btree (slug) @@ -433,6 +470,15 @@ indexes on files: indexes on files_trigger_details: files_trigger_details_pkey: public.files_trigger_details USING btree (trigger_id) +indexes on group_members: + group_members_pkey: public.group_members USING btree (group_id, user_id) + group_members_user_id_idx: public.group_members USING btree (user_id) + +indexes on groups: + groups_parent_id_idx: public.groups USING btree (parent_id) + groups_pkey: public.groups USING btree (id) + groups_slug_key: public.groups USING btree (slug) + indexes on kv_entries: idx_kv_entries_app_collection: public.kv_entries USING btree (app_id, collection) kv_entries_pkey: public.kv_entries USING btree (app_id, collection, key) @@ -473,12 +519,16 @@ indexes on script_imports: indexes on scripts: idx_scripts_app_kind: public.scripts USING btree (app_id, kind) scripts_app_id_idx: public.scripts USING btree (app_id) - scripts_name_uidx: public.scripts USING btree (app_id, lower(name)) + scripts_app_name_uidx: public.scripts USING btree (app_id, lower(name)) WHERE (app_id IS NOT NULL) + scripts_group_id_idx: public.scripts USING btree (group_id) WHERE (group_id IS NOT NULL) + scripts_group_name_uidx: public.scripts USING btree (group_id, lower(name)) WHERE (group_id IS NOT NULL) scripts_pkey: public.scripts USING btree (id) indexes on secrets: idx_secrets_app: public.secrets USING btree (app_id) - secrets_pkey: public.secrets USING btree (app_id, name) + idx_secrets_group: public.secrets USING btree (group_id) WHERE (group_id IS NOT NULL) + secrets_app_uidx: public.secrets USING btree (app_id, environment_scope, name) WHERE (app_id IS NOT NULL) + secrets_group_uidx: public.secrets USING btree (group_id, environment_scope, name) WHERE (group_id IS NOT NULL) indexes on topics: topics_pkey: public.topics USING btree (app_id, name) @@ -487,8 +537,16 @@ indexes on triggers: idx_triggers_app_kind_enabled: public.triggers USING btree (app_id, kind) WHERE (enabled = true) idx_triggers_app_pubsub_enabled: public.triggers USING btree (app_id, kind) WHERE ((enabled = true) AND (kind = 'pubsub'::text)) idx_triggers_kind_enabled: public.triggers USING btree (kind) WHERE (enabled = true) + triggers_app_name_uniq: public.triggers USING btree (app_id, name) triggers_pkey: public.triggers USING btree (id) +indexes on vars: + vars_app_id_idx: public.vars USING btree (app_id) WHERE (app_id IS NOT NULL) + vars_app_uidx: public.vars USING btree (app_id, environment_scope, key) WHERE (app_id IS NOT NULL) + vars_group_id_idx: public.vars USING btree (group_id) WHERE (group_id IS NOT NULL) + vars_group_uidx: public.vars USING btree (group_id, environment_scope, key) WHERE (group_id IS NOT NULL) + vars_pkey: public.vars USING btree (id) + ## constraints constraints on abandoned_executions: @@ -561,6 +619,7 @@ constraints on app_users: [PRIMARY KEY] app_users_pkey: PRIMARY KEY (id) constraints on apps: + [FOREIGN KEY] apps_group_id_fk: FOREIGN KEY (group_id) REFERENCES groups(id) ON DELETE RESTRICT [PRIMARY KEY] apps_pkey: PRIMARY KEY (id) [UNIQUE] apps_slug_key: UNIQUE (slug) @@ -605,6 +664,17 @@ constraints on files_trigger_details: [FOREIGN KEY] files_trigger_details_trigger_id_fkey: FOREIGN KEY (trigger_id) REFERENCES triggers(id) ON DELETE CASCADE [PRIMARY KEY] files_trigger_details_pkey: PRIMARY KEY (trigger_id) +constraints on group_members: + [CHECK] group_members_role_check: CHECK ((role = ANY (ARRAY['app_admin'::text, 'editor'::text, 'viewer'::text]))) + [FOREIGN KEY] group_members_group_id_fkey: FOREIGN KEY (group_id) REFERENCES groups(id) ON DELETE CASCADE + [FOREIGN KEY] group_members_user_id_fkey: FOREIGN KEY (user_id) REFERENCES admin_users(id) ON DELETE CASCADE + [PRIMARY KEY] group_members_pkey: PRIMARY KEY (group_id, user_id) + +constraints on groups: + [FOREIGN KEY] groups_parent_id_fkey: FOREIGN KEY (parent_id) REFERENCES groups(id) ON DELETE RESTRICT + [PRIMARY KEY] groups_pkey: PRIMARY KEY (id) + [UNIQUE] groups_slug_key: UNIQUE (slug) + constraints on kv_entries: [FOREIGN KEY] kv_entries_app_id_fkey: FOREIGN KEY (app_id) REFERENCES apps(id) ON DELETE CASCADE [PRIMARY KEY] kv_entries_pkey: PRIMARY KEY (app_id, collection, key) @@ -648,13 +718,16 @@ constraints on scripts: [CHECK] scripts_kind_check: CHECK ((kind = ANY (ARRAY['endpoint'::text, 'module'::text]))) [CHECK] scripts_memory_limit_mb_check: CHECK (((memory_limit_mb > 0) AND (memory_limit_mb <= 2048))) [CHECK] scripts_module_name_shape: CHECK (((kind <> 'module'::text) OR (name ~ '^[a-zA-Z_][a-zA-Z0-9_]{0,63}$'::text))) + [CHECK] scripts_owner_exactly_one: CHECK (((group_id IS NULL) <> (app_id IS NULL))) [CHECK] scripts_timeout_seconds_check: CHECK (((timeout_seconds > 0) AND (timeout_seconds <= 300))) [FOREIGN KEY] scripts_app_id_fk: FOREIGN KEY (app_id) REFERENCES apps(id) ON DELETE RESTRICT + [FOREIGN KEY] scripts_group_id_fkey: FOREIGN KEY (group_id) REFERENCES groups(id) ON DELETE RESTRICT [PRIMARY KEY] scripts_pkey: PRIMARY KEY (id) constraints on secrets: + [CHECK] secrets_owner_exactly_one: CHECK (((group_id IS NULL) <> (app_id IS NULL))) [FOREIGN KEY] secrets_app_id_fkey: FOREIGN KEY (app_id) REFERENCES apps(id) ON DELETE CASCADE - [PRIMARY KEY] secrets_pkey: PRIMARY KEY (app_id, name) + [FOREIGN KEY] secrets_group_id_fkey: FOREIGN KEY (group_id) REFERENCES groups(id) ON DELETE CASCADE constraints on topics: [CHECK] topics_auth_mode_check: CHECK ((auth_mode = ANY (ARRAY['public'::text, 'token'::text, 'session'::text]))) @@ -670,6 +743,12 @@ constraints on triggers: [FOREIGN KEY] triggers_script_id_fkey: FOREIGN KEY (script_id) REFERENCES scripts(id) ON DELETE CASCADE [PRIMARY KEY] triggers_pkey: PRIMARY KEY (id) +constraints on vars: + [CHECK] vars_owner_exactly_one: CHECK (((group_id IS NULL) <> (app_id IS NULL))) + [FOREIGN KEY] vars_app_id_fkey: FOREIGN KEY (app_id) REFERENCES apps(id) ON DELETE CASCADE + [FOREIGN KEY] vars_group_id_fkey: FOREIGN KEY (group_id) REFERENCES groups(id) ON DELETE CASCADE + [PRIMARY KEY] vars_pkey: PRIMARY KEY (id) + ## applied migrations 0001: init 0002: sandbox @@ -715,3 +794,9 @@ constraints on triggers: 0042: secrets envelope version 0043: execution logs source 0044: delete reserved path routes + 0045: scripts routes enabled + 0046: trigger name + 0047: groups + 0048: vars + 0049: group secrets + 0050: group scripts diff --git a/crates/orchestrator-core/src/api.rs b/crates/orchestrator-core/src/api.rs index 4ae94a4..bf15cc4 100644 --- a/crates/orchestrator-core/src/api.rs +++ b/crates/orchestrator-core/src/api.rs @@ -126,8 +126,14 @@ where if !script.enabled { return Err(ApiError::NotFound(id)); } + // The direct-execute bypass runs a script under its *owning app's* + // context. A group-owned script (Phase 4) is a template with no single + // app, so it cannot be invoked through this id-addressed bypass — it + // runs only via a descendant app's route/trigger, which supplies the + // execution-context app. Fail closed (404) rather than guess an app. + let app_id = script.app_id.ok_or(ApiError::NotFound(id))?; - let mut req = build_exec_request(id, &script.name, &headers, &body, script.app_id, principal)?; + let mut req = build_exec_request(id, &script.name, &headers, &body, app_id, principal)?; req.sandbox_overrides = script.sandbox; let request_id = req.request_id; let request_path = req.path.clone(); @@ -151,7 +157,7 @@ where // audit-visible platform — but a sink failure must not mask the // user-facing result, so we only log a warning if it fails. let log = build_execution_log( - script.app_id, + app_id, id, request_id, request_path, diff --git a/crates/picloud-cli/src/cmds/scripts.rs b/crates/picloud-cli/src/cmds/scripts.rs index 245c7c5..bd3b222 100644 --- a/crates/picloud-cli/src/cmds/scripts.rs +++ b/crates/picloud-cli/src/cmds/scripts.rs @@ -40,10 +40,15 @@ pub async fn ls(app: Option<&str>, mode: OutputMode) -> Result<()> { let (apps, scripts) = tokio::try_join!(client.apps_list(), client.scripts_list_all())?; let slug_by_id: HashMap = apps.into_iter().map(|a| (a.id, a.slug)).collect(); for s in scripts { - let app_slug = slug_by_id - .get(&s.app_id) - .cloned() - .unwrap_or_else(|| "-".to_string()); + // Group-owned scripts (Phase 4) have no app; show a marker rather + // than a slug. App-owned scripts resolve their slug as before. + let app_slug = match s.app_id { + Some(app_id) => slug_by_id + .get(&app_id) + .cloned() + .unwrap_or_else(|| "-".to_string()), + None => "(group)".to_string(), + }; table.row([ s.id.to_string(), app_slug, diff --git a/crates/shared/src/script.rs b/crates/shared/src/script.rs index 7fd72d3..846bc4f 100644 --- a/crates/shared/src/script.rs +++ b/crates/shared/src/script.rs @@ -1,7 +1,7 @@ use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; -use crate::{AppId, ScriptId, ScriptSandbox}; +use crate::{AppId, GroupId, ScriptId, ScriptSandbox}; /// Semantic role of a script (v1.1.3). /// @@ -94,10 +94,26 @@ mod kind_tests { #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Script { pub id: ScriptId, - /// Owning app. Set on create, immutable thereafter — a "move to - /// another app" is a copy+delete, not an in-place edit (snapshot - /// semantics — see blueprint §11.5). - pub app_id: AppId, + /// Owning app, when app-owned. Set on create, immutable thereafter — a + /// "move to another app" is a copy+delete, not an in-place edit + /// (snapshot semantics — see blueprint §11.5). + /// + /// Phase 4 (v1.2 Hierarchies) made script ownership **polymorphic**: + /// exactly one of `app_id` / `group_id` is set (DB CHECK + [`ScriptOwner`]). + /// A group-owned script (`app_id: None`, `group_id: Some`) is a template + /// inherited by every descendant app — it has no single app, so the + /// **execution context** app is supplied by the route/trigger/caller that + /// invoked it, never read off the script. Reading `app_id` to mean "the + /// app this runs under" is therefore a bug for group scripts; use the + /// invoking surface's app_id instead. + #[serde(default)] + pub app_id: Option, + /// Owning group, when group-owned (Phase 4). Mutually exclusive with + /// `app_id`. The script is resolved by name, nearest-owner-wins, down the + /// `apps.group_id → groups.parent_id` chain (CoW: an app's own script of + /// the same name shadows the inherited one). + #[serde(default)] + pub group_id: Option, pub name: String, pub description: Option, pub version: i32, @@ -131,3 +147,36 @@ pub struct Script { pub created_at: DateTime, pub updated_at: DateTime, } + +/// Who owns a script (Phase 4). Exactly one owner — the DB enforces it with +/// a `CHECK ((group_id IS NULL) <> (app_id IS NULL))`; this enum is the +/// in-memory witness of that invariant so call sites can `match` exhaustively +/// instead of juggling two `Option`s. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ScriptOwner { + App(AppId), + Group(GroupId), +} + +impl Script { + /// The script's owner, reconstructed from the polymorphic columns. + /// A row that violates the exactly-one invariant (both/neither set — + /// impossible under the CHECK) is reported as whichever is present, + /// preferring the app, so a corrupt row never panics on the hot path. + #[must_use] + pub fn owner(&self) -> Option { + match (self.app_id, self.group_id) { + (Some(a), _) => Some(ScriptOwner::App(a)), + (None, Some(g)) => Some(ScriptOwner::Group(g)), + (None, None) => None, + } + } + + /// True iff this script is owned by `app` directly (not via a group). + /// The cheap, app-owned-only ownership check — group inheritance is + /// resolved separately (chain-membership), never by this method. + #[must_use] + pub fn is_owned_by_app(&self, app: AppId) -> bool { + self.app_id == Some(app) + } +}