feat(scripts): polymorphic script owner (app XOR group) — Phase 4 foundation
Phase 4-lite C1. Make script ownership polymorphic so a script can be owned
by a GROUP (a template inherited by descendant apps) instead of an app —
mirroring vars/secrets (0048/0049), but ON DELETE RESTRICT (code is not data).
Schema (0050): `scripts.group_id` (nullable FK→groups RESTRICT), `app_id` made
nullable, `scripts_owner_exactly_one` CHECK, and the per-app name index split
into two per-owner partial-unique indexes. Existing app-owned rows keep their
exact `(app_id, lower(name))` uniqueness.
Type: `Script.app_id` becomes `Option<AppId>`; add `Script.group_id` +
`ScriptOwner` / `is_owned_by_app()`. `NewScript` gains the same polymorphic
owner. The execution-context app (what a script runs *under*) is supplied by
the invoking route/trigger/caller, never read off the script — group scripts
have no single app.
Behavior is fully preserved for app-owned scripts (the only kind creatable
today): every isolation backstop and authz site now uses `is_owned_by_app`,
which is byte-identical for app owners and **fails closed** for group owners.
A group script therefore can't yet be run, route/trigger-bound, invoked, or
managed via the app-script API — those land in C2 (group-script creation) and
C3 (chain-membership resolution + binding). The `/execute/{id}` bypass 404s a
group script (no app context to run under).
Re-blesses expected_schema.txt; note the golden was last blessed at migration
0044, so this also captures the already-committed 0045–0049 schema.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -181,6 +181,15 @@ async fn resolve_app_ident(apps: &dyn AppRepository, ident: &str) -> Result<AppI
|
||||
Ok(lookup.app.id)
|
||||
}
|
||||
|
||||
/// The owning app of a script, for app-scoped admin authz. Every script
|
||||
/// before Phase 4 is app-owned and yields its app. A group-owned script
|
||||
/// (Phase 4) is not addressable through the app-script admin API — it is
|
||||
/// managed via the group-script API — so it 404s here, indistinguishable
|
||||
/// from an absent id.
|
||||
fn script_app(script: &Script) -> Result<AppId, ApiError> {
|
||||
script.app_id.ok_or(ApiError::NotFound(script.id))
|
||||
}
|
||||
|
||||
async fn get_script<R: ScriptRepository, L: ExecutionLogRepository>(
|
||||
State(state): State<AdminState<R, L>>,
|
||||
Extension(principal): Extension<Principal>,
|
||||
@@ -190,7 +199,7 @@ async fn get_script<R: ScriptRepository, L: ExecutionLogRepository>(
|
||||
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<R: ScriptRepository, L: ExecutionLogRepository>(
|
||||
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<R: ScriptRepository, L: ExecutionLogRepository>(
|
||||
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<R: ScriptRepository, L: ExecutionLogRepository>(
|
||||
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<R: ScriptRepository, L: ExecutionLogRepository>(
|
||||
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
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<Option<Script>, 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,
|
||||
|
||||
@@ -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<T: ScriptRepository + ?Sized> ScriptRepository for std::sync::Arc<T> {
|
||||
/// 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<AppId>,
|
||||
/// Group owner (Phase 4). See [`NewScript::app_id`].
|
||||
pub group_id: Option<GroupId>,
|
||||
pub name: String,
|
||||
pub description: Option<String>,
|
||||
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<uuid::Uuid>,
|
||||
group_id: Option<uuid::Uuid>,
|
||||
name: String,
|
||||
description: Option<String>,
|
||||
version: i32,
|
||||
@@ -531,7 +548,8 @@ impl From<ScriptRow> 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,
|
||||
|
||||
@@ -148,10 +148,16 @@ async fn list_routes<RR: RouteRepository, SR: ScriptRepository>(
|
||||
.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<RR: RouteRepository, SR: ScriptRepository>(
|
||||
.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,
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user