fix(cms-poc): remediate findings surfaced by the CMS proof-of-concept

Additive fixes surfaced by building a full CMS on PiCloud
(examples/cms-poc/). One migration (0077_apps_cors.sql); no destructive
changes.

Security:
- Close the 502 info-leak on user routes: an uncaught script error (incl.
  a throw) leaked the app UUID + script fn names + source line/col. The
  user-route (inbox) path now shares scrub_runtime_detail with the
  execute-by-id path — raw detail is logged under a correlation id and the
  client sees only "script execution error (ref: <uuid>)".

Added:
- docs::find $contains operator (array membership via JSONB @>), per-app
  and group-shared — the inverse of $in.
- App-user role management from API/CLI: pic users add-role / rm-role,
  backed by the apps/{id}/users/{user_id}/roles endpoints (AppUsersAdmin).
- Per-app CORS: pic apps cors set, apps.cors_allowed_origins; the
  orchestrator echoes an allowed Origin and answers OPTIONS preflight.
- Non-JSON request bodies: form-urlencoded -> object, text/* -> string,
  other -> base64; ctx.request.content_type added. Malformed JSON still 400s.
- Binary responses via #{ body_base64 } with a script-set Content-Type.
- workflow::run_status(run_id) SDK (F-038): the starting script can poll a
  run — #{ status, output, error, steps } or () when no such run belongs to
  the app (app_id is the isolation boundary); same AppInvoke gate as start.

Fixed:
- pic apply "app not found" is now actionable (points at pic apps create).
- Interceptor- (F-021) and workflow-step-bound (F-037) scripts no longer
  warn "no route or trigger" at plan time — both count as reachability.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-07-19 20:15:17 +02:00
parent dbdd398d90
commit 5682be366c
32 changed files with 1361 additions and 92 deletions

View File

@@ -0,0 +1,11 @@
-- F-030 per-app CORS. A decoupled browser SPA on a different origin cannot
-- call an app's user routes today: the orchestrator emits no
-- `Access-Control-Allow-Origin` header and never answers an OPTIONS preflight.
--
-- This adds a per-app allow-list of origins. The orchestrator (which has no DB)
-- reads it into its in-memory cache on each domain-cache rebuild, echoes the
-- request `Origin` when it matches, and short-circuits preflights. Empty array
-- (the default) = CORS off, preserving today's behaviour. `["*"]` allows any
-- origin.
ALTER TABLE apps
ADD COLUMN cors_allowed_origins JSONB NOT NULL DEFAULT '[]'::jsonb;

View File

@@ -103,6 +103,25 @@ pub trait AppRepository: Send + Sync {
/// single transaction so a partial delete cannot be observed.
async fn delete_cascade(&self, id: AppId) -> Result<(), ScriptRepositoryError>;
async fn count_scripts_in_app(&self, id: AppId) -> Result<i64, ScriptRepositoryError>;
/// F-030 per-app CORS. The allow-list of origins each app permits on its
/// user routes, keyed by app_id — read once per domain-cache rebuild to
/// push into the orchestrator (which has no DB). Default: none (CORS off).
async fn cors_all(&self) -> Result<Vec<(AppId, Vec<String>)>, ScriptRepositoryError> {
Ok(Vec::new())
}
/// The CORS allow-list for one app (`[]` = CORS disabled).
async fn get_cors(&self, _id: AppId) -> Result<Vec<String>, ScriptRepositoryError> {
Ok(Vec::new())
}
/// Replace an app's CORS allow-list. `["*"]` allows any origin.
async fn set_cors(
&self,
_id: AppId,
_origins: Vec<String>,
) -> Result<(), ScriptRepositoryError> {
Ok(())
}
}
pub struct PostgresAppRepository {
@@ -443,6 +462,32 @@ impl AppRepository for PostgresAppRepository {
.await?;
Ok(count.0)
}
async fn cors_all(&self) -> Result<Vec<(AppId, Vec<String>)>, ScriptRepositoryError> {
let rows: Vec<(uuid::Uuid, sqlx::types::Json<Vec<String>>)> =
sqlx::query_as("SELECT id, cors_allowed_origins FROM apps")
.fetch_all(&self.pool)
.await?;
Ok(rows.into_iter().map(|(id, j)| (id.into(), j.0)).collect())
}
async fn get_cors(&self, id: AppId) -> Result<Vec<String>, ScriptRepositoryError> {
let row: Option<(sqlx::types::Json<Vec<String>>,)> =
sqlx::query_as("SELECT cors_allowed_origins FROM apps WHERE id = $1")
.bind(id.into_inner())
.fetch_optional(&self.pool)
.await?;
Ok(row.map(|(j,)| j.0).unwrap_or_default())
}
async fn set_cors(&self, id: AppId, origins: Vec<String>) -> Result<(), ScriptRepositoryError> {
sqlx::query("UPDATE apps SET cors_allowed_origins = $2, updated_at = NOW() WHERE id = $1")
.bind(id.into_inner())
.bind(sqlx::types::Json(origins))
.execute(&self.pool)
.await?;
Ok(())
}
}
#[derive(sqlx::FromRow)]

View File

@@ -705,7 +705,14 @@ async fn resolve_app_id(apps: &dyn AppRepository, ident: &str) -> Result<AppId,
.await
.map_err(|e| ApplyError::Backend(e.to_string()))?
.map(|l| l.app.id)
.ok_or_else(|| ApplyError::AppNotFound(ident.to_string()))
// `pic apply` reconciles the group tree but never creates an app — make
// the miss actionable instead of a bare "app not found: cms" (F-006).
.ok_or_else(|| {
ApplyError::AppNotFound(format!(
"{ident} — apply does not create apps; create it first with \
`pic apps create {ident}`, then re-run apply"
))
})
}
fn map_authz(denied: AuthzDenied) -> ApplyError {

View File

@@ -3362,7 +3362,16 @@ impl ApplyService {
.await
.map_err(|e| ApplyError::Backend(e.to_string()))?
.map(|l| l.app)
.ok_or_else(|| ApplyError::AppNotFound(ident.to_string()))
// `pic apply` reconciles the group tree (groups are created
// declaratively) but never creates an app — unlike a group, an
// app node must pre-exist. Make the failure actionable instead of
// a bare "app not found: cms" (F-006).
.ok_or_else(|| {
ApplyError::AppNotFound(format!(
"{ident} — apply does not create apps; create it first with \
`pic apps create {ident}`, then re-run apply"
))
})
}
async fn resolve_group(&self, ident: &str) -> Result<GroupId, ApplyError> {
@@ -5573,11 +5582,25 @@ fn reject_reserved_path(path: &str) -> Result<(), ApplyError> {
/// execute-by-id bypass / `invoke()`. Modules are exempt (never invoked
/// directly); disabled endpoints are intentionally inert, so skip them.
fn unreachable_endpoint_warnings(bundle: &Bundle) -> Vec<String> {
// A script is reachable if a route, a trigger, an interceptor, OR a
// workflow step binds it. Interceptors were the first missing binding-type
// (F-021); workflow-step `function`s were the second (F-037) — a step
// script reached only via `workflow::start` was wrongly flagged "no route
// or trigger". Interceptors reach a script via the `invoke()` re-entry
// path; workflow steps via the durable orchestrator.
let bound: HashSet<&str> = bundle
.routes
.iter()
.map(|r| r.script.as_str())
.chain(bundle.triggers.iter().map(BundleTrigger::script))
.chain(bundle.interceptors.iter().map(|i| i.script.as_str()))
.chain(
bundle
.workflows
.iter()
.flat_map(|w| w.definition.steps.iter())
.filter_map(|s| s.function.as_deref()),
)
.collect();
bundle
.scripts
@@ -6698,6 +6721,31 @@ mod tests {
lib.kind = ScriptKind::Module;
m.scripts = vec![lib];
assert!(unreachable_endpoint_warnings(&m).is_empty());
// F-021: bound only by an interceptor → reachable, no warning.
let mut h = empty_bundle();
h.scripts = vec![bundle_script("guard", "x")];
h.interceptors = vec![BundleInterceptor {
service: "docs".into(),
op: "create".into(),
phase: "before".into(),
script: "guard".into(),
timeout_ms: None,
}];
assert!(
unreachable_endpoint_warnings(&h).is_empty(),
"an interceptor-bound endpoint must not be flagged unreachable"
);
// F-037: bound only as a workflow step's `function` → reachable.
let mut w = empty_bundle();
w.scripts = vec![bundle_script("stepfn", "x")];
w.workflows = vec![bundle_workflow(
"pipeline",
serde_json::json!([{ "name": "s1", "function": "stepfn" }]),
)];
assert!(
unreachable_endpoint_warnings(&w).is_empty(),
"a workflow-step-bound endpoint must not be flagged unreachable"
);
}
#[test]

View File

@@ -74,6 +74,10 @@ pub fn apps_router(state: AppsState) -> Router {
"/apps/{id_or_slug}/domains/{domain_id}",
delete(delete_domain),
)
.route(
"/apps/{id_or_slug}/cors",
get(get_cors_handler).put(set_cors_handler),
)
.with_state(state)
}
@@ -134,6 +138,13 @@ pub struct SlugCheckResponse {
pub reason: Option<String>,
}
/// F-030 per-app CORS config: the set of browser origins allowed to call this
/// app's user routes. `["*"]` = any origin. Empty = CORS off.
#[derive(Debug, Serialize, Deserialize)]
pub struct CorsConfig {
pub allowed_origins: Vec<String>,
}
#[derive(Debug, Deserialize)]
pub struct CreateDomainRequest {
pub pattern: String,
@@ -513,6 +524,44 @@ async fn delete_domain(
Ok(StatusCode::NO_CONTENT)
}
/// F-030: read an app's CORS allow-list. `AppRead` — same gate as viewing
/// domain claims (CORS is host-adjacent config).
async fn get_cors_handler(
State(s): State<AppsState>,
Extension(principal): Extension<Principal>,
Path(id_or_slug): Path<String>,
) -> Result<Json<CorsConfig>, AppsApiError> {
let app = resolve_app(&*s.apps, &id_or_slug).await?.app;
require(s.authz.as_ref(), &principal, Capability::AppRead(app.id)).await?;
let allowed_origins = s.apps.get_cors(app.id).await?;
Ok(Json(CorsConfig { allowed_origins }))
}
/// F-030: replace an app's CORS allow-list, then refresh the domain cache so
/// the orchestrator serves the new policy immediately. `AppManageDomains` —
/// the same gate as domain-claim CRUD.
async fn set_cors_handler(
State(s): State<AppsState>,
Extension(principal): Extension<Principal>,
Path(id_or_slug): Path<String>,
Json(input): Json<CorsConfig>,
) -> Result<Json<CorsConfig>, AppsApiError> {
let app = resolve_app(&*s.apps, &id_or_slug).await?.app;
require(
s.authz.as_ref(),
&principal,
Capability::AppManageDomains(app.id),
)
.await?;
s.apps
.set_cors(app.id, input.allowed_origins.clone())
.await?;
refresh_domain_cache(&s).await?;
Ok(Json(CorsConfig {
allowed_origins: input.allowed_origins,
}))
}
// ----------------------------------------------------------------------------
// Helpers
// ----------------------------------------------------------------------------
@@ -602,6 +651,10 @@ pub async fn refresh_domain_cache(state: &AppsState) -> Result<(), AppsApiError>
})
.collect();
state.domain_table.replace(compiled);
// F-030: push the per-app CORS allow-lists into the same in-memory cache so
// the orchestrator can answer preflights + tag responses without a DB hit.
let cors: std::collections::HashMap<_, _> = state.apps.cors_all().await?.into_iter().collect();
state.domain_table.replace_cors(cors);
Ok(())
}

View File

@@ -1681,6 +1681,7 @@ fn summarize(resp: &ExecResponse) -> ExecResponseSummary {
status_code: resp.status_code,
headers: resp.headers.clone(),
body: resp.body.clone(),
body_base64: resp.body_base64.clone(),
}
}

View File

@@ -150,6 +150,12 @@ pub enum ComparisonOp {
/// `$in` — `= ANY($M::text[])` where the value list is bound as
/// a TEXT[].
In,
/// `$contains` — JSONB containment (`@>`) for array-membership:
/// matches a doc whose stored *array* field contains the given
/// scalar element (e.g. `{ tags: { "$contains": "rust" } }` finds
/// docs tagged `rust`). The `$in` operator is the inverse (field is
/// one of a list); this one is (list field contains a value).
Contains,
}
impl ComparisonOp {
@@ -169,6 +175,7 @@ impl ComparisonOp {
"$lt" => Ok(Self::Lt),
"$lte" => Ok(Self::Lte),
"$in" => Ok(Self::In),
"$contains" => Ok(Self::Contains),
other => Err(FilterParseError::UnsupportedOperator(format!(
"docs::find: operator '{other}' is not supported in v1.1.2; planned for v1.2 advanced query"
))),
@@ -321,6 +328,7 @@ const fn op_name(op: ComparisonOp) -> &'static str {
ComparisonOp::Lt => "$lt",
ComparisonOp::Lte => "$lte",
ComparisonOp::In => "$in",
ComparisonOp::Contains => "$contains",
}
}
@@ -465,6 +473,17 @@ mod tests {
// $in needs an array.
let f = parse(json!({ "tier": { "$in": ["gold", "platinum"] } })).unwrap();
assert_eq!(f.conditions[0].op, ComparisonOp::In);
// $contains takes a scalar (the element to find in an array field).
let f = parse(json!({ "tags": { "$contains": "rust" } })).unwrap();
assert_eq!(f.conditions[0].op, ComparisonOp::Contains);
}
#[test]
fn contains_with_object_value_rejected() {
// The element to find must be a scalar, not a nested object/array.
let err = parse(json!({ "tags": { "$contains": { "k": 1 } } })).unwrap_err();
assert!(err.to_string().contains("'$contains'"));
assert!(err.to_string().contains("scalar"));
}
#[test]

View File

@@ -311,6 +311,20 @@ fn emit_condition<'a>(
qb: &mut QueryBuilder<'a, Postgres>,
cond: &'a crate::docs_filter::FieldCondition,
) {
// `$contains` uses JSONB containment (`@>`), which needs the field's
// *JSONB* value, not the text rendering the other operators compare
// against — so it builds its own path expression and short-circuits.
// Emits `jsonb_extract_path(data, $seg…) @> $val::jsonb`, matching a doc
// whose stored array field contains the given scalar element (F-015).
if cond.op == ComparisonOp::Contains {
push_jsonb_path_value(qb, cond.path.segments());
qb.push(" @> ");
// `Value::to_string()` is the compact JSON encoding (`"rust"`, `5`,
// `true`), i.e. a valid JSONB literal — bound, never interpolated.
qb.push_bind(cond.value.to_string());
qb.push("::jsonb");
return;
}
push_jsonb_path(qb, cond.path.segments());
match cond.op {
ComparisonOp::Eq => {
@@ -358,6 +372,7 @@ fn emit_condition<'a>(
qb.push_bind(texts);
qb.push(")");
}
ComparisonOp::Contains => unreachable!("handled before the match"),
}
}
@@ -373,6 +388,18 @@ fn push_jsonb_path<'a>(qb: &mut QueryBuilder<'a, Postgres>, segments: &'a [Strin
qb.push(")");
}
/// Like [`push_jsonb_path`] but returns the *JSONB* value rather than its
/// text rendering (`jsonb_extract_path`, no `_text`). Used by `$contains`,
/// whose `@>` operator needs a JSONB left operand. Segments bound as params.
fn push_jsonb_path_value<'a>(qb: &mut QueryBuilder<'a, Postgres>, segments: &'a [String]) {
qb.push("jsonb_extract_path(data");
for seg in segments {
qb.push(", ");
qb.push_bind(seg.as_str());
}
qb.push(")");
}
/// JSON scalar → TEXT for binding. `Value::Null` is preserved as
/// `None` so the binding lands as SQL NULL (handled specially above for
/// `Eq` / `Ne`). Arrays + objects serialize to compact JSON; the user
@@ -534,6 +561,7 @@ mod sql_shape_tests {
json!({ "$sort": { "created_at": -1 }, "$limit": 5 }),
json!({ "tier": "gold", "$sort": { "created_at": 1 } }),
json!({ "deleted_at": { "$ne": null } }),
json!({ "tags": { "$contains": "rust" } }),
];
for case in cases {
let sql = sql_for(case.clone());
@@ -620,4 +648,16 @@ mod sql_shape_tests {
let sql = sql_for(json!({ "user.email": "a@b" }));
assert!(sql.contains("jsonb_extract_path_text(data"), "sql: {sql}");
}
#[test]
fn contains_emits_jsonb_containment() {
// `$contains` must use the JSONB (not text) extractor + `@>`, with the
// element bound as a `::jsonb` parameter — never interpolated (F-015).
let sql = sql_for(json!({ "tags": { "$contains": "rust" } }));
assert!(sql.contains("jsonb_extract_path(data"), "sql: {sql}");
assert!(!sql.contains("jsonb_extract_path_text(data"), "sql: {sql}");
assert!(sql.contains(" @> "), "sql: {sql}");
assert!(sql.contains("::jsonb"), "sql: {sql}");
assert!(!sql.contains("rust"), "value leaked into SQL string: {sql}");
}
}

View File

@@ -452,6 +452,22 @@ mod tests {
};
arr.iter().any(|v| actual == json_text(v).as_deref())
}
Contains => {
// `actual` is the text rendering of the stored field; for an
// array field that's compact JSON. Parse it back and test
// element membership, mirroring Postgres `@>` (a scalar field
// equal to the value also "contains" it).
let Some(text) = actual else {
return false;
};
match serde_json::from_str::<serde_json::Value>(text) {
Ok(serde_json::Value::Array(items)) => {
items.iter().any(|v| json_text(v).as_deref() == want_ref)
}
Ok(other) => json_text(&other).as_deref() == want_ref,
Err(_) => false,
}
}
}
}
@@ -829,6 +845,35 @@ mod tests {
assert_eq!(hits.len(), 2);
}
#[tokio::test]
async fn find_with_contains_matches_array_membership() {
// F-015: `$contains` finds docs whose ARRAY field holds the element —
// the "posts with tag X" query that `$in` cannot express.
let s = svc();
let cx = anon_cx(AppId::new());
s.create(&cx, "posts", json!({ "tags": ["intro", "rust"] }))
.await
.unwrap();
s.create(&cx, "posts", json!({ "tags": ["rust", "async"] }))
.await
.unwrap();
s.create(&cx, "posts", json!({ "tags": ["cooking"] }))
.await
.unwrap();
let hits = s
.find(&cx, "posts", json!({ "tags": { "$contains": "rust" } }))
.await
.unwrap();
assert_eq!(hits.len(), 2, "expected the two rust-tagged posts");
let none = s
.find(&cx, "posts", json!({ "tags": { "$contains": "python" } }))
.await
.unwrap();
assert!(none.is_empty());
}
#[tokio::test]
async fn find_one_explicit_limit_is_honoured() {
// The service injects limit=1 ONLY when caller didn't set

View File

@@ -56,6 +56,14 @@ pub fn app_users_router(state: AppUsersState) -> Router {
"/apps/{id_or_slug}/users/{user_id}/revoke-sessions",
post(revoke_sessions),
)
.route(
"/apps/{id_or_slug}/users/{user_id}/roles",
post(add_role_handler),
)
.route(
"/apps/{id_or_slug}/users/{user_id}/roles/{role}",
delete(remove_role_handler),
)
.route(
"/apps/{id_or_slug}/invitations",
get(list_invitations_handler).post(create_invitation_handler),
@@ -100,6 +108,11 @@ pub struct PatchUserRequest {
pub display_name: Option<Option<String>>,
}
#[derive(Debug, Deserialize)]
pub struct AddRoleRequest {
pub role: String,
}
#[derive(Debug, Serialize)]
pub struct ResetPasswordResponse {
/// The one-shot raw token. Returned exactly once — the admin
@@ -304,6 +317,50 @@ async fn revoke_sessions(
Ok(Json(RevokeSessionsResponse { revoked }))
}
/// Grant an app-user a role (F-012). Role assignment was previously reachable
/// only from a Rhai script via `users::add_role` — so bootstrapping the first
/// admin needed a bespoke setup-token endpoint. Idempotent (the repo upsert is
/// `ON CONFLICT DO NOTHING`). Authz (`AppUsersAdmin`) is enforced inside the
/// service. Returns the refreshed user so the caller sees the new role set.
async fn add_role_handler(
State(s): State<AppUsersState>,
Extension(principal): Extension<Principal>,
Path((id_or_slug, user_id)): Path<(String, Uuid)>,
Json(input): Json<AddRoleRequest>,
) -> Result<Json<AppUser>, AppUsersApiError> {
let app = resolve_app(&*s.apps, &id_or_slug).await?;
let cx = synth_cx(app.id, &principal);
let uid = AppUserId::from(user_id);
s.users.add_role(&cx, uid, &input.role).await?;
let user = s
.users
.get(&cx, uid)
.await?
.ok_or(AppUsersApiError::UserNotFound)?;
Ok(Json(user))
}
/// Revoke an app-user role (F-012). Idempotent — a `DELETE` of a role the user
/// doesn't hold still succeeds (200) and returns the current user.
async fn remove_role_handler(
State(s): State<AppUsersState>,
Extension(principal): Extension<Principal>,
Path((id_or_slug, user_id, role)): Path<(String, Uuid, String)>,
) -> Result<Json<AppUser>, AppUsersApiError> {
let app = resolve_app(&*s.apps, &id_or_slug).await?;
let cx = synth_cx(app.id, &principal);
let uid = AppUserId::from(user_id);
// The bool (whether a row was actually removed) is intentionally ignored —
// DELETE is idempotent.
let _ = s.users.remove_role(&cx, uid, &role).await?;
let user = s
.users
.get(&cx, uid)
.await?
.ok_or(AppUsersApiError::UserNotFound)?;
Ok(Json(user))
}
async fn list_invitations_handler(
State(s): State<AppUsersState>,
Extension(principal): Extension<Principal>,

View File

@@ -13,11 +13,15 @@
use std::sync::Arc;
use async_trait::async_trait;
use picloud_shared::{SdkCallCx, WorkflowError, WorkflowRunId, WorkflowService};
use picloud_shared::{
SdkCallCx, WorkflowError, WorkflowRunId, WorkflowRunView, WorkflowService, WorkflowStepView,
};
use sqlx::PgPool;
use crate::authz::{self, AuthzRepo, Capability};
use crate::workflow_repo::{get_workflow_by_name, start_run, NewWorkflowRun};
use crate::workflow_repo::{
get_run, get_workflow_by_name, list_run_steps, start_run, NewWorkflowRun,
};
pub struct WorkflowServiceImpl {
pool: PgPool,
@@ -84,4 +88,38 @@ impl WorkflowService for WorkflowServiceImpl {
.await
.map_err(|e| WorkflowError::Backend(e.to_string()))
}
async fn run_status(
&self,
cx: &SdkCallCx,
run_id: WorkflowRunId,
) -> Result<Option<WorkflowRunView>, WorkflowError> {
// Same gate as `start` — an authenticated caller needs AppInvoke; an
// anonymous public script is its own gate. The read is app-scoped:
// `get_run` filters on `cx.app_id`, so a run in another app resolves to
// `None` (the isolation boundary), never leaks.
self.check(cx).await?;
let Some(run) = get_run(&self.pool, cx.app_id, run_id)
.await
.map_err(|e| WorkflowError::Backend(e.to_string()))?
else {
return Ok(None);
};
let steps = list_run_steps(&self.pool, cx.app_id, run_id)
.await
.map_err(|e| WorkflowError::Backend(e.to_string()))?;
Ok(Some(WorkflowRunView {
run_id,
status: run.status,
output: run.output,
error: run.error,
steps: steps
.into_iter()
.map(|s| WorkflowStepView {
name: s.step_name,
status: s.status,
})
.collect(),
}))
}
}

View File

@@ -428,6 +428,7 @@ impl ExecutorClient for FakeExecutor {
status_code: 200,
headers: std::collections::BTreeMap::new(),
body: json!({ "ran": req.script_name }),
body_base64: None,
logs: vec![],
stats: ExecStats::default(),
})
@@ -559,6 +560,7 @@ impl ExecutorClient for ScriptedExecutor {
status_code: 200,
headers: std::collections::BTreeMap::new(),
body,
body_base64: None,
logs: vec![],
stats: ExecStats::default(),
})