From 98cfb9087236dc17a4ce530375d5b3eca03ad6a2 Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 12:36:59 -0600 Subject: [PATCH 01/38] fix: separate PostgreSQL planning connections from bookkeeping writes --- crates/schema-forge-cli/src/commands/mod.rs | 38 +++++++++--- crates/schema-forge-postgres/src/backend.rs | 63 ++++++++++++++------ crates/schema-forge-surrealdb/src/backend.rs | 4 +- 3 files changed, 77 insertions(+), 28 deletions(-) diff --git a/crates/schema-forge-cli/src/commands/mod.rs b/crates/schema-forge-cli/src/commands/mod.rs index d435718..f1ebd7b 100644 --- a/crates/schema-forge-cli/src/commands/mod.rs +++ b/crates/schema-forge-cli/src/commands/mod.rs @@ -38,6 +38,22 @@ use crate::progress; pub async fn connect_backend( db_params: &DbParams, output: &OutputContext, +) -> Result, CliError> { + connect_backend_with_mode(db_params, output, false).await +} + +/// Connect without PostgreSQL bookkeeping DDL for inspection and planning. +pub async fn connect_backend_read_only( + db_params: &DbParams, + output: &OutputContext, +) -> Result, CliError> { + connect_backend_with_mode(db_params, output, true).await +} + +async fn connect_backend_with_mode( + db_params: &DbParams, + output: &OutputContext, + read_only: bool, ) -> Result, CliError> { let spinner = if output.show_progress() { Some(progress::create_spinner("Connecting to backend...")) @@ -45,7 +61,7 @@ pub async fn connect_backend( None }; - let result = connect_backend_inner(db_params).await; + let result = connect_backend_inner(db_params, read_only).await; match result { Ok(backend) => { @@ -63,7 +79,12 @@ pub async fn connect_backend( } } -async fn connect_backend_inner(db_params: &DbParams) -> Result, CliError> { +async fn connect_backend_inner( + db_params: &DbParams, + read_only: bool, +) -> Result, CliError> { + // Other backends do not bootstrap PostgreSQL bookkeeping tables. + let _ = read_only; match db_params { #[cfg(feature = "surrealdb")] DbParams::Surrealdb(p) => { @@ -81,7 +102,7 @@ async fn connect_backend_inner(db_params: &DbParams) -> Result { eprintln!( "Warning: Could not connect to {}; falling back to in-memory backend: {remote_err}", - p.url + db_params.redacted_url() ); let b = schema_forge_surrealdb::SurrealBackend::connect_memory( &p.namespace, @@ -95,9 +116,12 @@ async fn connect_backend_inner(db_params: &DbParams) -> Result { - let b = schema_forge_postgres::PgBackend::connect(&p.url) - .await - .map_err(CliError::Backend)?; + let b = if read_only { + schema_forge_postgres::PgBackend::connect_read_only(&p.url).await + } else { + schema_forge_postgres::PgBackend::connect(&p.url).await + } + .map_err(CliError::Backend)?; Ok(Arc::new(b)) } #[cfg(feature = "mssql")] @@ -111,7 +135,7 @@ async fn connect_backend_inner(db_params: &DbParams) -> Result Err(CliError::Config { message: format!( "backend '{}' is not enabled in this build (check Cargo features)", - other.url() + other.redacted_url() ), }), } diff --git a/crates/schema-forge-postgres/src/backend.rs b/crates/schema-forge-postgres/src/backend.rs index 1293fe3..04269fe 100644 --- a/crates/schema-forge-postgres/src/backend.rs +++ b/crates/schema-forge-postgres/src/backend.rs @@ -121,21 +121,28 @@ impl PgBackend { /// Supports `postgres://` and `postgresql://` URL schemes. /// Creates a connection pool with sensible defaults. pub async fn connect(url: &str) -> Result { - let pool = PgPoolOptions::new() - .max_connections(10) - .connect(url) - .await - .map_err(|e| BackendError::ConnectionError { - message: format!("failed to connect to PostgreSQL at {url}: {e}"), - })?; + Self::connect_with_max_connections(url, 10).await + } - let backend = Self::from_parts(pool); + /// Connect for inspection or migration planning without creating bookkeeping tables. + /// + /// An empty database is treated as an empty schema registry. The caller should + /// use a database role with read-only privileges when writes must be forbidden. + pub async fn connect_read_only(url: &str) -> Result { + Self::connect_without_bootstrap(url, 10).await + } + + /// Connect with a custom maximum connection count and initialize bookkeeping. + pub async fn connect_with_max_connections( + url: &str, + max_connections: u32, + ) -> Result { + let backend = Self::connect_without_bootstrap(url, max_connections).await?; backend.ensure_metadata_table().await?; Ok(backend) } - /// Connect with a custom maximum connection count. - pub async fn connect_with_max_connections( + async fn connect_without_bootstrap( url: &str, max_connections: u32, ) -> Result { @@ -143,13 +150,16 @@ impl PgBackend { .max_connections(max_connections) .connect(url) .await - .map_err(|e| BackendError::ConnectionError { - message: format!("failed to connect to PostgreSQL at {url}: {e}"), + .map_err(|_| BackendError::ConnectionError { + // Driver errors may themselves repeat credentials from malformed URLs. + message: "failed to connect to PostgreSQL; check the database address, credentials, and server availability".into(), })?; + Ok(Self::from_parts(pool)) + } - let backend = Self::from_parts(pool); - backend.ensure_metadata_table().await?; - Ok(backend) + /// Use an existing pool without executing bookkeeping DDL. + pub fn from_pool_read_only(pool: PgPool) -> Self { + Self::from_parts(pool) } /// Create a backend from an existing connection pool. @@ -412,6 +422,15 @@ impl PgBackend { .map_err(|e| BackendError::ConnectionError { message: e.to_string(), })?; + let exists: bool = sqlx::query_scalar("SELECT to_regclass('_schema_metadata') IS NOT NULL") + .fetch_one(&mut *connection) + .await + .map_err(|error| BackendError::QueryError { + message: format!("failed to inspect schema registry: {error}"), + })?; + if !exists { + return Ok(Vec::new()); + } Self::fetch_metadata_on(&mut connection).await } @@ -742,15 +761,21 @@ impl SchemaBackend for PgBackend { name: &SchemaName, ) -> Result, BackendError> { let name_str = name.as_str(); - let row: Option = sqlx::query(&format!( + let row: Option = match sqlx::query(&format!( "SELECT \"definition\" FROM \"{SCHEMA_META_TABLE}\" WHERE \"name\" = $1;" )) .bind(name_str) .fetch_optional(&self.pool) .await - .map_err(|e| BackendError::QueryError { - message: format!("failed to load schema metadata: {e}"), - })?; + { + Ok(row) => row, + Err(sqlx::Error::Database(error)) if error.code().as_deref() == Some("42P01") => None, + Err(error) => { + return Err(BackendError::QueryError { + message: format!("failed to load schema metadata: {error}"), + }) + } + }; match row { None => Ok(None), diff --git a/crates/schema-forge-surrealdb/src/backend.rs b/crates/schema-forge-surrealdb/src/backend.rs index 6dd5d42..6197bff 100644 --- a/crates/schema-forge-surrealdb/src/backend.rs +++ b/crates/schema-forge-surrealdb/src/backend.rs @@ -136,9 +136,9 @@ impl SurrealBackend { username: Option<&str>, password: Option<&str>, ) -> Result { - let db = surrealdb::engine::any::connect(url).await.map_err(|e| { + let db = surrealdb::engine::any::connect(url).await.map_err(|_| { BackendError::ConnectionError { - message: format!("failed to connect to {url}: {e}"), + message: "failed to connect to SurrealDB; check the database address, credentials, and server availability".into(), } })?; From b43248e26abcfce455485617333d42c8e8608434 Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 12:40:07 -0600 Subject: [PATCH 02/38] fix: validate array filters and match PostgreSQL text literally --- crates/schema-forge-core/src/query.rs | 42 ++++- crates/schema-forge-postgres/src/query.rs | 33 +++- .../tests/planning_and_filters.rs | 178 ++++++++++++++++++ docs/migrations/safe-schema-changes.md | 8 + docs/query-api-reference.md | 12 ++ 5 files changed, 266 insertions(+), 7 deletions(-) create mode 100644 crates/schema-forge-postgres/tests/planning_and_filters.rs diff --git a/crates/schema-forge-core/src/query.rs b/crates/schema-forge-core/src/query.rs index 17beaa8..0f9a8fa 100644 --- a/crates/schema-forge-core/src/query.rs +++ b/crates/schema-forge-core/src/query.rs @@ -746,7 +746,10 @@ fn check_type_compat( FieldType::Bytes(_) => matches!(value, DynamicValue::Bytes(_)), FieldType::Enum(_) => matches!(value, DynamicValue::Enum(_) | DynamicValue::Text(_)), FieldType::Map { .. } => matches!(value, DynamicValue::Map(_)), - _ => true, // Json, Relation, Array, Composite — accept anything + // Array bindings are not portable across backend query compilers yet. + // Reject them before execution instead of returning a database type error. + FieldType::Array(_) => false, + _ => true, // Json, Relation, Composite retain their existing semantics. }; if !compatible { errors.push(QueryError::TypeMismatch { @@ -1201,6 +1204,43 @@ mod tests { .unwrap() } + #[test] + fn array_filters_reject_unsupported_values_before_backend_execution() { + let schema = SchemaDefinition::new( + SchemaId::new(), + SchemaName::new("Tagged").unwrap(), + vec![FieldDefinition::new( + FieldName::new("tags").unwrap(), + FieldType::Array(Box::new(FieldType::Text(TextConstraints::unconstrained()))), + )], + vec![], + ) + .unwrap(); + let path = FieldPath::single("tags"); + for value in [ + DynamicValue::Text("vip".into()), + DynamicValue::Array(vec![DynamicValue::Text("vip".into())]), + ] { + for filter in [ + Filter::eq(path.clone(), value.clone()), + Filter::ne(path.clone(), value.clone()), + Filter::gt(path.clone(), value.clone()), + Filter::gte(path.clone(), value.clone()), + Filter::lt(path.clone(), value.clone()), + Filter::lte(path.clone(), value.clone()), + Filter::In { + path: path.clone(), + values: vec![value], + }, + ] { + assert!(validate_filter(&filter, &schema).is_err(), "{filter:?}"); + } + } + assert!(validate_filter(&Filter::contains(path.clone(), "vip"), &schema).is_err()); + assert!(validate_filter(&Filter::eq(path.clone(), DynamicValue::Null), &schema).is_ok()); + assert!(validate_filter(&Filter::ne(path, DynamicValue::Null), &schema).is_ok()); + } + #[test] fn reserved_id_filters_and_total_order() { let schema = test_schema(); diff --git a/crates/schema-forge-postgres/src/query.rs b/crates/schema-forge-postgres/src/query.rs index a3266ee..61d1489 100644 --- a/crates/schema-forge-postgres/src/query.rs +++ b/crates/schema-forge-postgres/src/query.rs @@ -163,12 +163,26 @@ pub fn filter_to_sql(filter: &Filter, params: &mut Vec) -> String format!("{} <= ${}", field_path_to_sql(path), params.len()) } Filter::Contains { path, value } => { - params.push(DynamicValue::Text(format!("%{value}%"))); - format!("{} ILIKE ${}", field_path_to_sql(path), params.len()) + params.push(DynamicValue::Text(format!( + "%{}%", + escape_like_literal(value) + ))); + format!( + "{} LIKE ${} ESCAPE E'\\\\'", + field_path_to_sql(path), + params.len() + ) } Filter::StartsWith { path, value } => { - params.push(DynamicValue::Text(format!("{value}%"))); - format!("{} ILIKE ${}", field_path_to_sql(path), params.len()) + params.push(DynamicValue::Text(format!( + "{}%", + escape_like_literal(value) + ))); + format!( + "{} LIKE ${} ESCAPE E'\\\\'", + field_path_to_sql(path), + params.len() + ) } Filter::In { path, values } => { if values.is_empty() { @@ -211,6 +225,13 @@ pub fn filter_to_sql(filter: &Filter, params: &mut Vec) -> String } } +fn escape_like_literal(value: &str) -> String { + value + .replace('\\', "\\\\") + .replace('%', "\\%") + .replace('_', "\\_") +} + /// Convert a `FieldPath` to its PostgreSQL quoted representation. /// /// For nested paths (e.g., `company.industry`), uses PostgreSQL JSONB @@ -338,7 +359,7 @@ mod tests { let compiled = query_to_sql(&q, "Contact"); assert_eq!( compiled.sql, - "SELECT * FROM \"Contact\" WHERE \"email\" ILIKE $1;" + "SELECT * FROM \"Contact\" WHERE \"email\" LIKE $1 ESCAPE E'\\\\';" ); assert_eq!( compiled.params, @@ -353,7 +374,7 @@ mod tests { let compiled = query_to_sql(&q, "Contact"); assert_eq!( compiled.sql, - "SELECT * FROM \"Contact\" WHERE \"name\" ILIKE $1;" + "SELECT * FROM \"Contact\" WHERE \"name\" LIKE $1 ESCAPE E'\\\\';" ); assert_eq!(compiled.params, vec![DynamicValue::Text("J%".into())]); } diff --git a/crates/schema-forge-postgres/tests/planning_and_filters.rs b/crates/schema-forge-postgres/tests/planning_and_filters.rs new file mode 100644 index 0000000..d148890 --- /dev/null +++ b/crates/schema-forge-postgres/tests/planning_and_filters.rs @@ -0,0 +1,178 @@ +//! Regression coverage for side-effect-free planning and literal text matching. +use schema_forge_backend::traits::SchemaBackend; +use schema_forge_core::query::{FieldPath, Filter, Query}; +use schema_forge_core::types::{DynamicValue, EntityId, SchemaId, SchemaName}; +use schema_forge_postgres::{query::query_to_sql, PgBackend}; +use sqlx::postgres::PgPoolOptions; + +#[tokio::test] +async fn connection_errors_do_not_expose_credentials_or_driver_input() { + for url in [ + "postgres://private-user:private-password@localhost:invalid/db", + "postgres://private-user:private-password@localhost/db?port=private-password", + ] { + for result in [ + PgBackend::connect(url).await, + PgBackend::connect_read_only(url).await, + PgBackend::connect_with_max_connections(url, 1).await, + ] { + let error = result + .err() + .expect("malformed connection must fail") + .to_string(); + assert!(!error.contains("private-password"), "{error}"); + assert!(!error.contains("private-user"), "{error}"); + assert!(!error.contains(url), "{error}"); + } + } +} + +#[tokio::test] +#[ignore = "requires SCHEMAFORGE_TEST_POSTGRES_URL with CREATE SCHEMA and CREATEROLE privileges"] +async fn planning_connections_do_not_create_tables_and_read_only_roles_can_inspect() { + let url = std::env::var("SCHEMAFORGE_TEST_POSTGRES_URL").unwrap(); + let admin = PgPoolOptions::new() + .max_connections(1) + .connect(&url) + .await + .unwrap(); + let namespace = EntityId::new("planning").to_string(); + let role = EntityId::new("reader").to_string(); + sqlx::query(&format!("CREATE SCHEMA \"{namespace}\"")) + .execute(&admin) + .await + .unwrap(); + sqlx::query(&format!("CREATE ROLE \"{role}\"")) + .execute(&admin) + .await + .unwrap(); + sqlx::query(&format!( + "GRANT USAGE ON SCHEMA \"{namespace}\" TO \"{role}\"" + )) + .execute(&admin) + .await + .unwrap(); + let scope = namespace.clone(); + let reader = role.clone(); + let pool = PgPoolOptions::new() + .max_connections(1) + .after_connect(move |connection, _| { + let scope = scope.clone(); + let reader = reader.clone(); + Box::pin(async move { + sqlx::query(&format!("SET ROLE \"{reader}\"")) + .execute(&mut *connection) + .await?; + sqlx::query("SELECT set_config('search_path', $1, false)") + .bind(scope) + .execute(&mut *connection) + .await?; + Ok(()) + }) + }) + .connect(&url) + .await + .unwrap(); + let backend = PgBackend::from_pool_read_only(pool.clone()); + let name = SchemaName::new("Example").unwrap(); + let connector_url = format!( + "{url}{}options=-csearch_path%3D{namespace}", + if url.contains('?') { "&" } else { "?" } + ); + let planning = PgBackend::connect_read_only(&connector_url).await.unwrap(); + assert!(planning.list_schema_metadata().await.unwrap().is_empty()); + planning.pool().close().await; + let fixture_admin = admin.clone(); + let fixture_namespace = namespace.clone(); + let fixture_role = role.clone(); + let result = tokio::spawn(async move { + assert!(backend.list_schema_metadata().await.unwrap().is_empty()); + assert!(backend.load_schema_metadata(&name).await.unwrap().is_none()); + // The write constructor demonstrates this role really cannot bootstrap. + assert!(PgBackend::from_pool(backend.pool().clone()).await.is_err()); + let count: i64 = sqlx::query_scalar("SELECT count(*) FROM pg_tables WHERE schemaname = $1") + .bind(&fixture_namespace).fetch_one(&fixture_admin).await.unwrap(); + assert_eq!(count, 0, "planning must not create bookkeeping tables"); + sqlx::query(&format!("CREATE TABLE \"{fixture_namespace}\"._schema_metadata (name TEXT PRIMARY KEY, definition JSONB NOT NULL)")) + .execute(&fixture_admin).await.unwrap(); + sqlx::query(&format!("GRANT SELECT ON \"{fixture_namespace}\"._schema_metadata TO \"{fixture_role}\"")) + .execute(&fixture_admin).await.unwrap(); + let existing = PgBackend::from_pool_read_only(backend.pool().clone()); + assert!(existing.list_schema_metadata().await.unwrap().is_empty()); + assert!(existing.load_schema_metadata(&name).await.unwrap().is_none()); + }) + .await; + let count: i64 = sqlx::query_scalar("SELECT count(*) FROM pg_tables WHERE schemaname = $1") + .bind(&namespace) + .fetch_one(&admin) + .await + .unwrap(); + pool.close().await; + sqlx::query(&format!("DROP SCHEMA \"{namespace}\" CASCADE")) + .execute(&admin) + .await + .unwrap(); + sqlx::query(&format!("DROP ROLE \"{role}\"")) + .execute(&admin) + .await + .unwrap(); + result.unwrap(); + assert_eq!(count, 1, "only the fixture metadata table should exist"); + admin.close().await; +} + +#[tokio::test] +#[ignore = "requires SCHEMAFORGE_TEST_POSTGRES_URL"] +async fn postgres_text_filters_are_literal_and_case_sensitive() { + let url = std::env::var("SCHEMAFORGE_TEST_POSTGRES_URL").unwrap(); + let pool = PgPoolOptions::new() + .max_connections(1) + .connect(&url) + .await + .unwrap(); + sqlx::query("CREATE TEMP TABLE filter_literals (id TEXT, value TEXT)") + .execute(&pool) + .await + .unwrap(); + for (id, value) in [ + ("one", "A%_\\tail"), + ("two", "aXXtail"), + ("three", "Azzztail"), + ] { + sqlx::query("INSERT INTO filter_literals VALUES ($1, $2)") + .bind(id) + .bind(value) + .execute(&pool) + .await + .unwrap(); + } + for (filter, expected) in [ + ( + Filter::contains(FieldPath::single("value"), "%_\\"), + vec!["one"], + ), + ( + Filter::starts_with(FieldPath::single("value"), "A%_\\"), + vec!["one"], + ), + (Filter::contains(FieldPath::single("value"), "axx"), vec![]), + ] { + let compiled = query_to_sql( + &Query::new(SchemaId::new()).with_filter(filter), + "filter_literals", + ); + let DynamicValue::Text(pattern) = &compiled.params[0] else { + panic!("text pattern expected") + }; + let rows: Vec<(String, String)> = sqlx::query_as(&compiled.sql) + .bind(pattern) + .fetch_all(&pool) + .await + .unwrap(); + assert_eq!( + rows.into_iter().map(|(id, _)| id).collect::>(), + expected + ); + } + pool.close().await; +} diff --git a/docs/migrations/safe-schema-changes.md b/docs/migrations/safe-schema-changes.md index c06b3de..249a20d 100644 --- a/docs/migrations/safe-schema-changes.md +++ b/docs/migrations/safe-schema-changes.md @@ -164,3 +164,11 @@ Other transitions have different requirements: For every case, commit physical changes, validated ownership, and the canonical annotation update together. Other database backends require equivalent native DDL and metadata changes; the PostgreSQL SQL above is not portable. + +## Read-only PostgreSQL planning + +`apply --dry-run`, `migrate` without `--execute`, and `inspect` connect without +creating PostgreSQL bookkeeping tables. A fresh database is treated as an empty +schema registry. An inspection role needs schema `USAGE` and `SELECT` on existing +bookkeeping tables, but does not need `CREATE`. Normal write commands initialize +bookkeeping as before. diff --git a/docs/query-api-reference.md b/docs/query-api-reference.md index 7a646e8..6d4c1ed 100644 --- a/docs/query-api-reference.md +++ b/docs/query-api-reference.md @@ -582,3 +582,15 @@ The JSON sort key accepts both `order` and `direction`, with `asc` or `desc`. The implicit `id` field supports filters, including `gt` for id-range paging. This makes pages deterministic for an unchanged dataset. Concurrent inserts, deletes, or sort-field edits can still move records between offset pages. + +### Array filter limitations + +Array fields currently support only null equality and inequality checks (`eq` / +`ne` with a JSON null). Other comparisons, including whole-array equality, `in`, +`contains`, and `startswith`, return HTTP 400 `invalid_query` before reaching the +database. Element membership and overlap operators are not yet supported. Model +searchable tags as related records when membership queries are needed. + +Text `contains` and `startswith` match literal, case-sensitive text on all +backends. Percent signs, underscores, and backslashes in their values are literal +characters, not SQL wildcard patterns. From d36a24fef35bd0180de0046e5f9104cdd796ae10 Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 12:39:46 -0600 Subject: [PATCH 03/38] fix(webhooks): enforce payload and destination boundaries --- README.md | 2 + .../schema-forge-acton/src/routes/schemas.rs | 16 + crates/schema-forge-acton/src/webhook.rs | 370 ++++++++++++++++-- docs/webhooks.md | 46 +++ 4 files changed, 394 insertions(+), 40 deletions(-) create mode 100644 docs/webhooks.md diff --git a/README.md b/README.md index 2c77e34..6ce2341 100644 --- a/README.md +++ b/README.md @@ -1056,3 +1056,5 @@ See the project repository for license information. Platform administrators can browse recorded audit events and verify bounded chain ranges through the [audit API](docs/audit-api-reference.md). Access is deployment-wide, uses the active framework audit store, and reports collection limits separately from local chain consistency. Available in v0.42.0. See [safe schema changes](docs/migrations/safe-schema-changes.md) for declared field renames, destructive migration opt-ins, PostgreSQL relation integrity, and explicit tenancy migrations. + +See the [webhook delivery contract](docs/webhooks.md) for delivery guarantees, payload format, and destination policy. diff --git a/crates/schema-forge-acton/src/routes/schemas.rs b/crates/schema-forge-acton/src/routes/schemas.rs index 935a548..789d176 100644 --- a/crates/schema-forge-acton/src/routes/schemas.rs +++ b/crates/schema-forge-acton/src/routes/schemas.rs @@ -675,6 +675,14 @@ pub async fn create_schema( // 4a. Run the inverse-relation pairing pass across the full registry so // any `-> X[]` field paired with an FK from an existing schema is marked // as derived before the migration plan is generated. + crate::webhook::validate_schema_webhooks( + &definition, + &state.config().custom.schema_forge.webhooks, + ) + .await + .map_err(|error| ForgeError::ValidationFailed { + details: vec![error.to_string()], + })?; let paired_registry = pair_with_registry(&forge, &mut definition).await?; // 4b. Pre-validate the proposed Cedar bundle BEFORE running any DB @@ -892,6 +900,14 @@ pub async fn update_schema( // 4a. Run the inverse-relation pairing pass before diffing, so newly // added `-> X[]` fields are classified as derived (and therefore // produce no AddRelation step for a physical column). + crate::webhook::validate_schema_webhooks( + &new_definition, + &state.config().custom.schema_forge.webhooks, + ) + .await + .map_err(|error| ForgeError::ValidationFailed { + details: vec![error.to_string()], + })?; let paired_registry = pair_with_registry(&forge, &mut new_definition).await?; // 4b. Dry-run the Cedar bundle for the proposed registry state so an diff --git a/crates/schema-forge-acton/src/webhook.rs b/crates/schema-forge-acton/src/webhook.rs index eb6673d..a3cfe33 100644 --- a/crates/schema-forge-acton/src/webhook.rs +++ b/crates/schema-forge-acton/src/webhook.rs @@ -1,4 +1,4 @@ -use std::net::IpAddr; +use std::net::{IpAddr, SocketAddr}; use std::sync::Arc; use std::time::Duration; @@ -37,6 +37,8 @@ pub const VALID_EVENTS: &[&str] = &["created", "updated", "deleted"]; /// The JSON payload delivered to webhook subscribers. #[derive(Debug, Clone, Serialize)] pub struct WebhookEvent { + /// Wire payload format version (plain JSON fields). + pub payload_version: u8, /// Unique delivery ID (UUID v4). pub event_id: String, /// Event type: `entity.created`, `entity.updated`, or `entity.deleted`. @@ -55,34 +57,37 @@ pub struct WebhookEvent { impl WebhookEvent { /// Build an event from a create operation. - pub fn from_create(schema: &str, entity: &Entity, actor: Option<&str>) -> Self { + pub fn from_create(schema: &SchemaDefinition, entity: &Entity, actor: Option<&str>) -> Self { Self { + payload_version: 2, event_id: uuid::Uuid::new_v4().to_string(), event_type: "entity.created".to_string(), - schema: schema.to_string(), + schema: schema.name.to_string(), entity_id: entity.id.as_str().to_string(), timestamp: now_iso8601(), actor: actor.map(String::from), - payload: Some(entity_fields_to_json(entity)), + payload: Some(entity_fields_to_json(entity, schema)), } } /// Build an event from an update operation. - pub fn from_update(schema: &str, entity: &Entity, actor: Option<&str>) -> Self { + pub fn from_update(schema: &SchemaDefinition, entity: &Entity, actor: Option<&str>) -> Self { Self { + payload_version: 2, event_id: uuid::Uuid::new_v4().to_string(), event_type: "entity.updated".to_string(), - schema: schema.to_string(), + schema: schema.name.to_string(), entity_id: entity.id.as_str().to_string(), timestamp: now_iso8601(), actor: actor.map(String::from), - payload: Some(entity_fields_to_json(entity)), + payload: Some(entity_fields_to_json(entity, schema)), } } /// Build an event from a delete operation (no payload — entity is gone). pub fn from_delete(schema: &str, entity_id: &str, actor: Option<&str>) -> Self { Self { + payload_version: 2, event_id: uuid::Uuid::new_v4().to_string(), event_type: "entity.deleted".to_string(), schema: schema.to_string(), @@ -95,8 +100,15 @@ impl WebhookEvent { } /// Convert entity fields to a JSON value. -fn entity_fields_to_json(entity: &Entity) -> serde_json::Value { - serde_json::to_value(&entity.fields).unwrap_or(serde_json::Value::Null) +fn entity_fields_to_json(entity: &Entity, schema: &SchemaDefinition) -> serde_json::Value { + let mut fields = crate::conversions::entity_to_response(entity, schema).fields; + // Subscribers have no caller claims. Use a fixed conservative field policy. + fields.retain(|name, _| { + schema + .field(name) + .is_none_or(|field| field.field_access().is_none()) + }); + serde_json::Value::Object(fields) } /// Get current UTC time as RFC 3339 string. @@ -190,7 +202,6 @@ pub struct ResolvedSubscription { /// exponential backoff. Never blocks the calling HTTP handler. #[derive(Clone)] pub struct WebhookDispatcher { - client: reqwest::Client, config: WebhookConfig, semaphore: Arc, } @@ -201,11 +212,7 @@ impl WebhookDispatcher { let semaphore = Arc::new(tokio::sync::Semaphore::new( config.max_concurrent_deliveries, )); - Self { - client: reqwest::Client::new(), - config, - semaphore, - } + Self { config, semaphore } } /// Fire-and-forget: spawn a background delivery task for each subscription. @@ -213,7 +220,6 @@ impl WebhookDispatcher { /// Returns immediately — webhook delivery never blocks the API response. pub fn dispatch(&self, event: WebhookEvent, subscriptions: Vec) { for sub in subscriptions { - let client = self.client.clone(); let event = event.clone(); let config = self.config.clone(); let semaphore = self.semaphore.clone(); @@ -225,7 +231,7 @@ impl WebhookDispatcher { return; } }; - deliver_with_retry(&client, &event, &sub, &config).await; + deliver_with_retry(&event, &sub, &config).await; }); } } @@ -281,7 +287,6 @@ impl WebhookDispatcher { /// Deliver a webhook event with exponential backoff retry. async fn deliver_with_retry( - client: &reqwest::Client, event: &WebhookEvent, subscription: &ResolvedSubscription, config: &WebhookConfig, @@ -307,10 +312,24 @@ async fn deliver_with_retry( for attempt in 0..=max_retries { if attempt > 0 { - let backoff = Duration::from_millis(500 * 2u64.pow(attempt - 1)); + let backoff = Duration::from_millis( + 500_u64 + .saturating_mul(2_u64.saturating_pow(attempt - 1)) + .min(60_000), + ); tokio::time::sleep(backoff).await; } + let client = match checked_client(&subscription.url, config, timeout).await { + Ok(client) => client, + Err(error) => { + warn!(%error, event_id = %event.event_id, "webhook destination refused"); + if matches!(error, WebhookUrlError::Resolution) { + continue; + } + return; + } + }; let mut request = client .post(&subscription.url) .header("Content-Type", "application/json") @@ -471,40 +490,137 @@ async fn query_webhook_subscriptions( /// /// Rejects private/loopback IPs and enforces allowed URL schemes. pub fn validate_webhook_url(url: &str, allowed_schemes: &[String]) -> Result<(), WebhookUrlError> { - // Basic URL parsing without the `url` crate - let (scheme, rest) = url.split_once("://").ok_or(WebhookUrlError::InvalidUrl)?; - - if !allowed_schemes.iter().any(|s| s == scheme) { - return Err(WebhookUrlError::DisallowedScheme(scheme.to_string())); + let parsed = reqwest::Url::parse(url).map_err(|_| WebhookUrlError::InvalidUrl)?; + if !matches!(parsed.scheme(), "http" | "https") + || !allowed_schemes.iter().any(|s| s == parsed.scheme()) + { + return Err(WebhookUrlError::DisallowedScheme(parsed.scheme().into())); } - - // Extract host (before any port or path) - let host_part = rest.split('/').next().unwrap_or(rest); - let host = host_part.split(':').next().unwrap_or(host_part); - - if host.is_empty() { + if !parsed.username().is_empty() || parsed.password().is_some() || parsed.fragment().is_some() { return Err(WebhookUrlError::InvalidUrl); } - - if host == "localhost" { + let host = parsed.host_str().ok_or(WebhookUrlError::InvalidUrl)?; + let host = host.trim_start_matches('[').trim_end_matches(']'); + if host.trim_end_matches('.').eq_ignore_ascii_case("localhost") + || host.parse::().is_ok_and(|ip| is_private_ip(&ip)) + { return Err(WebhookUrlError::PrivateIp); } + Ok(()) +} - if let Ok(ip) = host.parse::() { - if is_private_ip(&ip) { - return Err(WebhookUrlError::PrivateIp); +/// Conservative globally routable address policy, including mapped IPv4. +fn is_private_ip(ip: &IpAddr) -> bool { + match ip { + IpAddr::V4(v4) => { + let [a, b, c, _] = v4.octets(); + v4.is_private() + || v4.is_loopback() + || v4.is_link_local() + || a == 0 + || a >= 224 + || (a == 100 && (64..=127).contains(&b)) + || (a == 192 && b == 0 && (c == 0 || c == 2)) + || (a == 192 && b == 88 && c == 99) + || (a == 198 && (b == 18 || b == 19 || (b == 51 && c == 100))) + || (a == 203 && b == 0 && c == 113) + } + IpAddr::V6(v6) => { + if let Some(v4) = v6.to_ipv4_mapped() { + return is_private_ip(&IpAddr::V4(v4)); + } + let s = v6.segments(); + // Permit ordinary global unicast only, excluding special allocations, + // documentation ranges and transition mechanisms. + s[0] & 0xe000 != 0x2000 + || (s[0] == 0x2001 && s[1] < 0x200) + || (s[0] == 0x2001 && s[1] == 0xdb8) + || s[0] == 0x2002 + || (s[0] == 0x3fff && s[1] < 0x1000) } } +} +fn validate_addresses(addresses: &[SocketAddr]) -> Result<(), WebhookUrlError> { + if addresses.is_empty() { + return Err(WebhookUrlError::Resolution); + } + if addresses.iter().any(|address| is_private_ip(&address.ip())) { + return Err(WebhookUrlError::PrivateIp); + } Ok(()) } -/// Check whether an IP address is private/loopback. -fn is_private_ip(ip: &IpAddr) -> bool { - match ip { - IpAddr::V4(v4) => v4.is_loopback() || v4.is_private() || v4.is_link_local(), - IpAddr::V6(v6) => v6.is_loopback(), +async fn resolved_destination( + url: &str, + config: &WebhookConfig, + timeout: Duration, +) -> Result<(reqwest::Url, Vec), WebhookUrlError> { + validate_webhook_url(url, &config.allowed_url_schemes)?; + let parsed = reqwest::Url::parse(url).map_err(|_| WebhookUrlError::InvalidUrl)?; + let host = parsed + .host_str() + .ok_or(WebhookUrlError::InvalidUrl)? + .trim_start_matches('[') + .trim_end_matches(']'); + let port = parsed + .port_or_known_default() + .ok_or(WebhookUrlError::InvalidUrl)?; + let addresses = tokio::time::timeout(timeout, tokio::net::lookup_host((host, port))) + .await + .map_err(|_| WebhookUrlError::Resolution)? + .map_err(|_| WebhookUrlError::Resolution)? + .collect::>(); + validate_addresses(&addresses)?; + Ok((parsed, addresses)) +} + +async fn checked_client( + url: &str, + config: &WebhookConfig, + timeout: Duration, +) -> Result { + let (parsed, addresses) = resolved_destination(url, config, timeout).await?; + pinned_client(&parsed, &addresses) +} + +fn pinned_client( + parsed: &reqwest::Url, + addresses: &[SocketAddr], +) -> Result { + let host = parsed.host_str().ok_or(WebhookUrlError::InvalidUrl)?; + reqwest::Client::builder() + .redirect(reqwest::redirect::Policy::none()) + .no_proxy() + .resolve_to_addrs(host, addresses) + .build() + .map_err(|_| WebhookUrlError::Transport) +} + +/// Validate the effective fields before persisting a webhook subscription. +pub async fn validate_subscription_fields( + schema: &SchemaDefinition, + fields: &std::collections::BTreeMap, + config: &WebhookConfig, +) -> Result<(), WebhookUrlError> { + if schema.name.as_str() == "WebhookSubscription" { + let Some(DynamicValue::Text(url)) = fields.get("url") else { + return Err(WebhookUrlError::InvalidUrl); + }; + resolved_destination(url, config, Duration::from_secs(10)).await?; } + Ok(()) +} + +/// Validate an inline webhook destination before applying a schema. +pub async fn validate_schema_webhooks( + schema: &SchemaDefinition, + config: &WebhookConfig, +) -> Result<(), WebhookUrlError> { + if let Some(Annotation::Webhook { url: Some(url), .. }) = schema.webhook_annotation() { + resolved_destination(url, config, Duration::from_secs(10)).await?; + } + Ok(()) } /// Errors from webhook URL validation. @@ -513,11 +629,15 @@ pub enum WebhookUrlError { InvalidUrl, DisallowedScheme(String), PrivateIp, + Resolution, + Transport, } impl std::fmt::Display for WebhookUrlError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { + Self::Resolution => write!(f, "webhook destination DNS resolution failed"), + Self::Transport => write!(f, "webhook HTTP client initialization failed"), Self::InvalidUrl => write!(f, "invalid URL"), Self::DisallowedScheme(s) => write!(f, "disallowed URL scheme: {s}"), Self::PrivateIp => write!(f, "private or loopback IP addresses are not allowed"), @@ -535,6 +655,176 @@ impl std::error::Error for WebhookUrlError {} mod tests { use super::*; + #[test] + fn event_projection_omits_nonpublic_fields_and_uses_plain_json() { + let schema = schema_forge_dsl::parse( + r#"schema Note { + title: text + secret: text @hidden + salary: int @field_access(read: ["hr"], write: ["hr"]) + }"#, + ) + .unwrap() + .remove(0); + let entity = Entity::new( + schema.name.clone(), + std::collections::BTreeMap::from([ + ("title".into(), DynamicValue::Text("hello".into())), + ("secret".into(), DynamicValue::Text("private".into())), + ("salary".into(), DynamicValue::Integer(100)), + ]), + ); + for event in [ + WebhookEvent::from_create(&schema, &entity, None), + WebhookEvent::from_update(&schema, &entity, None), + ] { + assert_eq!(event.payload_version, 2); + assert_eq!(event.payload, Some(serde_json::json!({"title": "hello"}))); + } + } + + #[test] + fn destination_policy_covers_special_address_ranges() { + for host in [ + "127.1", + "2130706433", + "0.0.0.0", + "100.64.1.2", + "192.0.0.8", + "198.18.0.1", + "224.0.0.1", + "255.255.255.255", + "[::1]", + "[::]", + "[fc00::1]", + "[fe80::1]", + "[::ffff:127.0.0.1]", + "[2002:7f00:1::]", + "[2001:db8::1]", + "[64:ff9b::7f00:1]", + "localhost.", + ] { + assert!( + validate_webhook_url(&format!("https://{host}/hook"), &["https".into()]).is_err(), + "{host}" + ); + } + for host in ["8.8.8.8", "[2606:4700:4700::1111]", "example.com"] { + assert!( + validate_webhook_url(&format!("https://{host}/hook"), &["https".into()]).is_ok(), + "{host}" + ); + } + for url in [ + "https://user:pass@example.com/", + "https://example.com/#fragment", + "file:///tmp/test", + ] { + assert!(validate_webhook_url(url, &["https".into(), "file".into()]).is_err()); + } + } + + #[test] + fn mixed_public_private_dns_results_are_refused() { + let public = "8.8.8.8:443".parse().unwrap(); + let private = "10.0.0.1:443".parse().unwrap(); + assert!(validate_addresses(&[public]).is_ok()); + assert!(validate_addresses(&[public, private]).is_err()); + assert!(validate_addresses(&[]).is_err()); + } + + #[tokio::test] + async fn pinned_transport_uses_checked_address_and_does_not_follow_redirects() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + let initial = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let destination = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let checked_address = initial.local_addr().unwrap(); + let location = format!("http://{}/private", destination.local_addr().unwrap()); + let server = tokio::spawn(async move { + let (mut connection, _) = initial.accept().await.unwrap(); + let mut request = vec![0; 4096]; + let count = connection.read(&mut request).await.unwrap(); + assert!(String::from_utf8_lossy(&request[..count]).contains("POST /hook")); + connection.write_all(format!("HTTP/1.1 302 Found\r\nLocation: {location}\r\nContent-Length: 0\r\nConnection: close\r\n\r\n").as_bytes()).await.unwrap(); + }); + // Inject a fixture address at the transport boundary, after the policy + // boundary tested separately. This hostname cannot resolve via DNS. + let url = reqwest::Url::parse(&format!( + "http://webhook.invalid:{}/hook", + checked_address.port() + )) + .unwrap(); + let client = pinned_client(&url, &[checked_address]).unwrap(); + let response = client + .post(url) + .timeout(Duration::from_secs(2)) + .send() + .await + .unwrap(); + assert_eq!(response.status(), reqwest::StatusCode::FOUND); + server.await.unwrap(); + assert!( + tokio::time::timeout(Duration::from_millis(50), destination.accept()) + .await + .is_err() + ); + } + + #[tokio::test] + async fn write_validation_rejects_inline_and_subscription_destinations() { + let inline = schema_forge_dsl::parse( + r#"@webhook(url: "https://[::1]/hook") schema Note { title: text }"#, + ) + .unwrap() + .remove(0); + let config = WebhookConfig::default(); + assert!(validate_schema_webhooks(&inline, &config).await.is_err()); + let subscription = schema_forge_dsl::parse("schema WebhookSubscription { url: text }") + .unwrap() + .remove(0); + for url in ["http://example.com/hook", "https://127.0.0.1/hook"] { + let fields = + std::collections::BTreeMap::from([("url".into(), DynamicValue::Text(url.into()))]); + assert!( + validate_subscription_fields(&subscription, &fields, &config) + .await + .is_err() + ); + } + assert!( + validate_subscription_fields(&subscription, &Default::default(), &config) + .await + .is_err() + ); + assert!( + validate_subscription_fields(&inline, &Default::default(), &config) + .await + .is_ok() + ); + } + + #[tokio::test] + async fn delivery_rejects_unsafe_stored_subscription_without_connecting() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let subscription = ResolvedSubscription { + url: format!("http://{}/hook", listener.local_addr().unwrap()), + secret: None, + retry_count: Some(0), + timeout_seconds: Some(1), + }; + let config = WebhookConfig { + allowed_url_schemes: vec!["http".into()], + ..Default::default() + }; + let event = WebhookEvent::from_delete("Note", "note_test", None); + deliver_with_retry(&event, &subscription, &config).await; + assert!( + tokio::time::timeout(Duration::from_millis(50), listener.accept()) + .await + .is_err() + ); + } + #[test] fn compute_signature_with_secret() { let sub = ResolvedSubscription { diff --git a/docs/webhooks.md b/docs/webhooks.md new file mode 100644 index 0000000..47ea78d --- /dev/null +++ b/docs/webhooks.md @@ -0,0 +1,46 @@ +# Webhook delivery contract + +Enable webhooks with `[schema_forge.webhooks] enabled = true`, mark a schema with +`@webhook`, and configure an inline URL or a `WebhookSubscription`. + +Delivery is **best effort**. Entity persistence completes before subscription +lookup and background delivery start. A process failure between those steps can +lose an event. Pending deliveries and retries exist only in memory, so a restart +loses them. Exhausted retries are logged and discarded. There is no durable +outbox, delivery history, dead-letter queue, replay endpoint, or gap-detection +sequence. A successful entity API response does not mean its webhook arrived. + +The default is one attempt plus three retries with exponential backoff. HTTP +4xx responses stop retries; other failures retry up to the configured limit. +The `X-SchemaForge-Delivery` ID stays the same across retries of one event. +Subscribers should deduplicate that ID: a timeout can occur after the subscriber +has already processed a request. The ID does not identify missing events. + +Integrations that need current state should periodically reconcile through the +entity list/query APIs, using their normal authorization and tenant scope. +Polling can recover current visible records but cannot reconstruct intermediate +updates or deleted records. Applications requiring a complete change history +must maintain a separate durable change log; webhooks do not provide one. + +## Payload format + +Events include `payload_version: 2`. Create and update payloads are JSON objects +using the REST API's value encoding, including JSON nulls and RFC 3339 datetimes. +This replaces the earlier tagged `DynamicValue` encoding. Delete events have a +null payload. Consumers of the old format must update their decoders. + +`@hidden` fields are always omitted. Fields with `@field_access` annotations are +also always omitted because subscriptions have no authenticated field-read +identity. This conservative field policy applies regardless of the triggering +caller's role. Other fields can still be limited by the triggering request's +read projection. Events contain the entity ID, schema, actor, and timestamp. + +## Destination policy + +Only public HTTP(S) destinations are supported. `allowed_url_schemes` defaults +to `["https"]`; permitting `http` is an explicit operator choice. Subscription +writes and inline schema application validate destinations, including DNS +resolution. Delivery repeats the check for existing records and DNS changes, +rejects an answer containing any non-public address, and connects only to the +checked addresses. Redirects and environment HTTP proxies are disabled. URL +credentials and fragments are rejected. Private destinations are unsupported. From 54a32d1f28c8d04ae27e3f9f4442763fdb81fcc8 Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 12:40:22 -0600 Subject: [PATCH 04/38] test(webhooks): use canonical integer field syntax --- crates/schema-forge-acton/src/webhook.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/schema-forge-acton/src/webhook.rs b/crates/schema-forge-acton/src/webhook.rs index a3cfe33..36b245c 100644 --- a/crates/schema-forge-acton/src/webhook.rs +++ b/crates/schema-forge-acton/src/webhook.rs @@ -661,7 +661,7 @@ mod tests { r#"schema Note { title: text secret: text @hidden - salary: int @field_access(read: ["hr"], write: ["hr"]) + salary: integer @field_access(read: ["hr"], write: ["hr"]) }"#, ) .unwrap() From a5cebdfeda525fcc2c3133717eabc2283d6747ef Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 12:42:15 -0600 Subject: [PATCH 05/38] fix: enforce tenant and API validation boundaries --- crates/schema-forge-acton/src/error.rs | 80 +++++- .../src/graphql/resolvers.rs | 7 +- .../schema-forge-acton/src/routes/entities.rs | 227 ++++++++++++++++-- .../schema-forge-acton/src/routes/schemas.rs | 6 +- .../tests/cross_entity_reads.rs | 145 +++++++++++ crates/schema-forge-backend/src/tenant.rs | 30 +++ docs/tenant-isolation.md | 4 +- 7 files changed, 472 insertions(+), 27 deletions(-) diff --git a/crates/schema-forge-acton/src/error.rs b/crates/schema-forge-acton/src/error.rs index e7bcf87..a2c8122 100644 --- a/crates/schema-forge-acton/src/error.rs +++ b/crates/schema-forge-acton/src/error.rs @@ -193,6 +193,19 @@ impl ForgeError { } } +impl ForgeError { + /// Return a client-safe message, retaining backend diagnostics only in server logs. + pub(crate) fn client_message(&self) -> String { + match self { + Self::BackendUnavailable { message } | Self::Internal { message } => { + tracing::error!(error = %message, "API backend operation failed"); + "The server could not complete the operation".into() + } + _ => self.to_string(), + } + } +} + impl IntoResponse for ForgeError { fn into_response(self) -> Response { let status = self.status_code(); @@ -203,22 +216,22 @@ impl IntoResponse for ForgeError { "message": message, }), Self::ForeignKeyViolation { schema, constraint } => { - serde_json::json!({ "schema": schema, "constraint": constraint }) + serde_json::json!({ "error": self.error_kind(), "message": self.client_message(), "schema": schema, "constraint": constraint }) } Self::UniqueViolation { schema, field } => serde_json::json!({ "error": "unique_violation", "schema": schema, "field": field, - "message": self.to_string(), + "message": self.client_message(), }), Self::ExportTooLarge { max_rows, .. } => serde_json::json!({ "error": "export_too_large", "max_rows": max_rows, - "message": self.to_string(), + "message": self.client_message(), }), _ => serde_json::json!({ "error": self.error_kind(), - "message": self.to_string(), + "message": self.client_message(), }), }; (status, axum::Json(body)).into_response() @@ -624,6 +637,42 @@ mod tests { assert_eq!(json["message"], "msg"); } + #[tokio::test] + async fn foreign_key_response_has_standard_envelope() { + let response = ForgeError::ForeignKeyViolation { + schema: "Line".into(), + constraint: "Line_order_fkey".into(), + } + .into_response(); + assert_eq!(response.status(), StatusCode::CONFLICT); + let bytes = response.into_body().collect().await.unwrap().to_bytes(); + let json: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + assert_eq!(json["error"], "foreign_key_violation"); + assert_eq!(json["schema"], "Line"); + assert_eq!(json["constraint"], "Line_order_fkey"); + assert!(json["message"].as_str().unwrap().contains("Line")); + } + + #[tokio::test] + async fn backend_responses_do_not_expose_diagnostics() { + for error in [ + ForgeError::from(BackendError::QueryError { + message: "private SQL table secret".into(), + }), + ForgeError::Internal { + message: "private configuration".into(), + }, + ] { + let response = error.into_response(); + let bytes = response.into_body().collect().await.unwrap().to_bytes(); + let json: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + assert_eq!( + json["message"], + "The server could not complete the operation" + ); + } + } + #[tokio::test] async fn into_response_unique_violation_has_field_in_body() { let err = ForgeError::UniqueViolation { @@ -689,3 +738,26 @@ mod tests { } } } + +/// JSON request extractor using the SchemaForge validation error envelope. +pub struct JsonBody(pub T); + +impl axum::extract::FromRequest for JsonBody +where + S: Send + Sync, + T: serde::de::DeserializeOwned, +{ + type Rejection = ForgeError; + + async fn from_request( + request: axum::extract::Request, + state: &S, + ) -> Result { + as axum::extract::FromRequest>::from_request(request, state) + .await + .map(|axum::Json(value)| Self(value)) + .map_err(|error| ForgeError::ValidationFailed { + details: vec![error.body_text()], + }) + } +} diff --git a/crates/schema-forge-acton/src/graphql/resolvers.rs b/crates/schema-forge-acton/src/graphql/resolvers.rs index db8f4e3..712841e 100644 --- a/crates/schema-forge-acton/src/graphql/resolvers.rs +++ b/crates/schema-forge-acton/src/graphql/resolvers.rs @@ -38,7 +38,7 @@ pub fn forge_error_to_gql(err: ForgeError) -> async_graphql::Error { | ForgeError::InvalidEntityId { .. } => "BAD_REQUEST", _ => "INTERNAL_ERROR", }; - async_graphql::Error::new(err.to_string()).extend_with(|_, e| e.set("code", code)) + async_graphql::Error::new(err.client_message()).extend_with(|_, e| e.set("code", code)) } /// Resolve a single entity by ID. @@ -260,7 +260,7 @@ pub async fn resolve_create_entity<'a>( axum::extract::Path(schema_name.to_owned()), crate::access::OptionalClaims(gql_ctx.claims.clone()), axum::http::HeaderMap::new(), - axum::Json(mutation_request(ctx)?), + crate::error::JsonBody(mutation_request(ctx)?), ) .await .map_err(forge_error_to_gql)?; @@ -282,7 +282,7 @@ pub async fn resolve_update_entity<'a>( axum::extract::Path((schema_name.to_owned(), id)), crate::access::OptionalClaims(gql_ctx.claims.clone()), axum::http::HeaderMap::new(), - axum::Json(mutation_request(ctx)?), + crate::error::JsonBody(mutation_request(ctx)?), ) .await .map_err(forge_error_to_gql)? @@ -619,5 +619,6 @@ mod tests { }; let gql_err = forge_error_to_gql(err); assert_eq!(extension_code(&gql_err).as_deref(), Some("INTERNAL_ERROR")); + assert_eq!(gql_err.message, "The server could not complete the operation"); } } diff --git a/crates/schema-forge-acton/src/routes/entities.rs b/crates/schema-forge-acton/src/routes/entities.rs index 95f5ea1..abbf355 100644 --- a/crates/schema-forge-acton/src/routes/entities.rs +++ b/crates/schema-forge-acton/src/routes/entities.rs @@ -30,7 +30,7 @@ use crate::access::{ use crate::actor::ForgeActor; use crate::authz::{authorize, namespace::ActionVerb}; use crate::config::SchemaForgeConfig; -use crate::error::ForgeError; +use crate::error::{ForgeError, JsonBody}; use crate::hooks::{ run_before_hook, DispatchHook, HookDispatchActor, HookDispatcher, HookInvocation, HooksConfig, }; @@ -776,8 +776,14 @@ pub fn json_to_entity_fields_with_mode( let dynamic_value = if let Some(def) = field_def { convert_json_with_type_hint(value, &def.field_type) } else { - // Unknown field -- convert based on JSON type - convert_json_untyped(value) + if key != "_tenant" || !schema.is_tenanted() { + errors.push(format!("unknown field '{key}'")); + continue; + } + convert_json_with_type_hint( + value, + &FieldType::Text(schema_forge_core::types::TextConstraints::unconstrained()), + ) }; match dynamic_value { @@ -1754,6 +1760,9 @@ async fn resolve_relation_displays( let mut targets: HashMap> = HashMap::new(); for field in &parent_schema.fields { + if field.is_hidden() { + continue; + } if let FieldType::Relation { target, .. } = &field.field_type { targets .entry(target.as_str().to_string()) @@ -1962,6 +1971,86 @@ fn collect_relation_ids(value: &DynamicValue, out: &mut HashSet) { /// /// Fast path: when no `@require` on the schema references `related.*`, no I/O is /// performed and the pure binding set is used directly. +/// Validate the final relation values before any write reaches storage. +async fn validate_relation_targets( + forge: &acton_service::prelude::ActorHandle, + policy_store: &Arc, + schema: &SchemaDefinition, + fields: &BTreeMap, + claims: Option<&Claims>, + tenant_config: &Option, +) -> Result<(), ForgeError> { + if !schema.is_tenanted() + || !tenant_config + .as_ref() + .is_some_and(|config| config.is_enabled()) + || claims.is_some_and(|claims| claims.has_role("platform_admin")) + { + return Ok(()); + } + for field in &schema.fields { + let FieldType::Relation { target, .. } = &field.field_type else { + continue; + }; + if field.is_derived() { + continue; + } + let Some(value) = fields.get(field.name.as_str()) else { + continue; + }; + let mut ids = HashSet::new(); + collect_relation_ids(value, &mut ids); + if ids.is_empty() { + continue; + } + let definitions = fetch_schemas_batch(forge, vec![target.as_str().to_string()]).await?; + let failure = || ForgeError::ValidationFailed { + details: vec![format!( + "field '{}': relation target is unavailable", + field.name.as_str() + )], + }; + let target_schema = definitions.get(target.as_str()).ok_or_else(failure)?; + if !target_schema.is_tenanted() { + continue; + } + let mut query = schema_forge_core::query::Query::new(target_schema.id.clone()) + .with_filter(Filter::In { + path: FieldPath::single("id"), + values: ids.iter().cloned().map(DynamicValue::Text).collect(), + }) + .without_total_count(); + inject_tenant_scope(&mut query, claims, tenant_config, target_schema); + let (tx, rx) = oneshot::channel(); + forge + .send(QueryEntities { + query, + reply: ReplyChannel::new(tx), + }) + .await; + let result = ask_forge(rx).await?.map_err(ForgeError::from)?; + let visible: HashSet<_> = result + .entities + .iter() + .filter(|entity| { + require_entity_action( + policy_store, + target_schema, + claims, + entity, + ActionVerb::Read, + ) + .is_ok() + }) + .map(|entity| entity.id.as_str().to_string()) + .collect(); + if !ids.is_subset(&visible) { + return Err(failure()); + } + } + Ok(()) +} + async fn check_requires_with_related( forge: &acton_service::prelude::ActorHandle, schema: &SchemaDefinition, @@ -2415,6 +2504,9 @@ fn apply_relation_displays( display_map: &HashMap>, ) { for field in &parent_schema.fields { + if field.is_hidden() { + continue; + } let FieldType::Relation { cardinality, .. } = &field.field_type else { continue; }; @@ -2509,7 +2601,7 @@ pub async fn create_entity( Path(schema): Path, OptionalClaims(claims): OptionalClaims, headers: HeaderMap, - Json(body): Json, + JsonBody(body): JsonBody, ) -> Result { let schema_name = validate_schema_name(&schema)?; let forge = state @@ -2684,6 +2776,26 @@ pub async fn create_entity( .await?; } + validate_relation_targets( + &forge, + &policy_store, + &schema_def, + &fields, + claims.as_ref(), + &tenant_config, + ) + .await?; + + crate::webhook::validate_subscription_fields( + &schema_def, + &fields, + &state.config().custom.schema_forge.webhooks, + ) + .await + .map_err(|error| ForgeError::ValidationFailed { + details: vec![error.to_string()], + })?; + // Create the entity, filtering write-restricted fields let entity = Entity::with_id(supplied.id, schema_name, fields); validate_required_fields(&schema_def, &entity.fields)?; @@ -2762,7 +2874,7 @@ pub async fn create_entity( // Webhook: fire notifications let webhook_event = crate::webhook::WebhookEvent::from_create( - &schema, + &schema_def, &created, claims.as_ref().map(|c| c.sub.as_str()), ); @@ -2943,7 +3055,7 @@ pub async fn query_entities( Path(schema): Path, OptionalClaims(claims): OptionalClaims, headers: HeaderMap, - Json(body): Json, + JsonBody(body): JsonBody, ) -> Result { let schema_name = validate_schema_name(&schema)?; let forge = state @@ -3304,7 +3416,7 @@ pub async fn update_entity( Path((schema, id)): Path<(String, String)>, OptionalClaims(claims): OptionalClaims, headers: HeaderMap, - Json(body): Json, + JsonBody(body): JsonBody, ) -> Result { let schema_name = validate_schema_name(&schema)?; let forge = state @@ -3555,6 +3667,26 @@ pub async fn update_entity( .await?; } + validate_relation_targets( + &forge, + &policy_store, + &schema_def, + &fields, + claims.as_ref(), + &tenant_config, + ) + .await?; + + crate::webhook::validate_subscription_fields( + &schema_def, + &fields, + &state.config().custom.schema_forge.webhooks, + ) + .await + .map_err(|error| ForgeError::ValidationFailed { + details: vec![error.to_string()], + })?; + // Build entity with specific ID, filtering write-restricted fields let entity = Entity::with_id(entity_id, schema_name, fields); validate_required_fields(&schema_def, &entity.fields)?; @@ -3608,7 +3740,7 @@ pub async fn update_entity( // Webhook: fire notifications let webhook_event = crate::webhook::WebhookEvent::from_update( - &schema, + &schema_def, &updated, claims.as_ref().map(|c| c.sub.as_str()), ); @@ -3636,7 +3768,7 @@ pub async fn patch_entity( Path((schema, id)): Path<(String, String)>, OptionalClaims(claims): OptionalClaims, headers: HeaderMap, - Json(body): Json, + JsonBody(body): JsonBody, ) -> Result { let schema_name = validate_schema_name(&schema)?; let forge = state @@ -3797,6 +3929,16 @@ pub async fn patch_entity( apply_computed(&schema_def, &mut merged, claims.as_ref(), rules_now) .map_err(rule_error_to_forge)?; + validate_relation_targets( + &forge, + &policy_store, + &schema_def, + &merged, + claims.as_ref(), + &tenant_config, + ) + .await?; + validate_required_fields(&schema_def, &merged)?; // Tenant config for cross-entity-read tenant scoping (#95). @@ -3863,6 +4005,16 @@ pub async fn patch_entity( validate_required_fields(&schema_def, &merged)?; check_field_constraints(&schema_def, &merged)?; + crate::webhook::validate_subscription_fields( + &schema_def, + &merged, + &state.config().custom.schema_forge.webhooks, + ) + .await + .map_err(|error| ForgeError::ValidationFailed { + details: vec![error.to_string()], + })?; + // Compute the delta: only keys whose final value differs from the // loaded baseline go to the backend. This keeps PATCH's SQL UPDATE // actually partial, which makes the whole class of "null column @@ -3936,7 +4088,7 @@ pub async fn patch_entity( // Webhook: fire notifications let webhook_event = crate::webhook::WebhookEvent::from_update( - &schema, + &schema_def, &updated, claims.as_ref().map(|c| c.sub.as_str()), ); @@ -4442,17 +4594,16 @@ mod tests { } #[test] - fn json_to_entity_fields_unknown_field_accepted() { + fn json_to_entity_fields_unknown_field_rejected() { let schema = make_test_schema(); let mut json_fields = serde_json::Map::new(); json_fields.insert("name".into(), serde_json::json!("Alice")); json_fields.insert("extra".into(), serde_json::json!("extra value")); - let result = json_to_entity_fields(&schema, &json_fields).unwrap(); - assert_eq!( - result.get("extra"), - Some(&DynamicValue::Text("extra value".into())) - ); + for mode in [ConversionMode::Replace, ConversionMode::Merge] { + let errors = json_to_entity_fields_with_mode(&schema, &json_fields, mode).unwrap_err(); + assert_eq!(errors, vec!["unknown field 'extra'"]); + } } #[test] @@ -4561,6 +4712,50 @@ mod tests { ); } + #[test] + fn apply_relation_displays_omits_hidden_relation() { + use schema_forge_core::types::{ + Cardinality, FieldDefinition, FieldName, FieldType, SchemaId, SchemaName, + }; + + // Schema with one Relation(One) field "agency". + let mut schema = SchemaDefinition::new( + SchemaId::new(), + SchemaName::new("Opportunity").unwrap(), + vec![FieldDefinition::new( + FieldName::new("agency").unwrap(), + FieldType::Relation { + target: SchemaName::new("Agency").unwrap(), + cardinality: Cardinality::One, + }, + )], + Vec::new(), + ) + .unwrap(); + + schema.fields[0].annotations.push(FieldAnnotation::Hidden); + let mut fields = BTreeMap::new(); + fields.insert( + "agency".to_string(), + DynamicValue::Text("entity_01abcd".into()), + ); + let entity = Entity::new(schema.name.clone(), fields); + let mut response = entity_to_response(&entity, &schema); + + let mut id_to_display = HashMap::new(); + id_to_display.insert( + "entity_01abcd".to_string(), + "Department of Homeland Security".to_string(), + ); + let mut display_map = HashMap::new(); + display_map.insert("agency".to_string(), id_to_display); + + apply_relation_displays(&mut response, &schema, &entity, &display_map); + + assert!(!response.fields.contains_key("agency")); + assert!(!response.fields.contains_key("agency__display")); + } + #[test] fn apply_relation_displays_many_preserves_order_and_nulls() { use schema_forge_core::types::{ diff --git a/crates/schema-forge-acton/src/routes/schemas.rs b/crates/schema-forge-acton/src/routes/schemas.rs index 789d176..cedf4e6 100644 --- a/crates/schema-forge-acton/src/routes/schemas.rs +++ b/crates/schema-forge-acton/src/routes/schemas.rs @@ -22,7 +22,7 @@ use crate::access::{ }; use crate::actor::ForgeActor; use crate::config::SchemaForgeConfig; -use crate::error::ForgeError; +use crate::error::{ForgeError, JsonBody}; use crate::messages::{ApplyPreparedSchemaChange, GetSchema, ListSchemas, ReplyChannel}; // --------------------------------------------------------------------------- @@ -601,7 +601,7 @@ fn schema_to_response(schema: &SchemaDefinition) -> SchemaResponse { pub async fn create_schema( State(state): State>, OptionalClaims(claims): OptionalClaims, - Json(body): Json, + JsonBody(body): JsonBody, ) -> Result { let claims = require_auth(&claims)?; if let Err(e) = require_admin(claims) { @@ -814,7 +814,7 @@ pub async fn update_schema( State(state): State>, Path(name): Path, OptionalClaims(claims): OptionalClaims, - Json(body): Json, + JsonBody(body): JsonBody, ) -> Result { let claims = require_auth(&claims)?; if let Err(e) = require_admin(claims) { diff --git a/crates/schema-forge-acton/tests/cross_entity_reads.rs b/crates/schema-forge-acton/tests/cross_entity_reads.rs index a99ad77..e6ea4b4 100644 --- a/crates/schema-forge-acton/tests/cross_entity_reads.rs +++ b/crates/schema-forge-acton/tests/cross_entity_reads.rs @@ -635,3 +635,148 @@ async fn multi_hop_related_read_is_rejected_with_clear_error() { "body should mention multi-hop: {body}" ); } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn relation_writes_validate_tenant_targets_and_request_fields() { + let backend = Arc::new( + SurrealBackend::connect_memory("test", "relation_writes") + .await + .unwrap(), + ); + let mut registry = HashMap::new(); + let access = Annotation::Access { + read: vec!["member".into()], + write: vec!["member".into()], + delete: vec!["member".into()], + cross_tenant_read: vec![], + }; + for (name, fields, tenant) in [ + ("Organization", vec![text_field("name")], TenantKind::Root), + ( + "Approval", + vec![text_field("name")], + TenantKind::Child { + parent: SchemaName::new("Organization").unwrap(), + }, + ), + ( + "Document", + vec![ + text_field("title"), + relation_field("approval", "Approval", Cardinality::One), + relation_field("reviewers", "Approval", Cardinality::Many), + ], + TenantKind::Child { + parent: SchemaName::new("Organization").unwrap(), + }, + ), + ] { + let schema = SchemaDefinition::new( + SchemaId::new(), + SchemaName::new(name).unwrap(), + fields, + vec![access.clone(), Annotation::Tenant(tenant)], + ) + .unwrap(); + apply_and_register(&backend, &mut registry, schema).await; + } + let tenant_config = + TenantConfig::from_schemas(®istry.values().cloned().collect::>()).unwrap(); + let state = build_state(backend, registry, Some(tenant_config)).await; + let app_a = app_with_claims(state.clone(), claims_in_tenant(&["member"], "org-a")); + let app_b = app_with_claims(state.clone(), claims_in_tenant(&["member"], "org-b")); + let (status, own) = json_request( + &app_a, + Method::POST, + "/schemas/Approval/entities", + Some(serde_json::json!({"fields":{"name":"own"}})), + ) + .await; + assert_eq!(status, StatusCode::CREATED, "{own}"); + let (status, other) = json_request( + &app_b, + Method::POST, + "/schemas/Approval/entities", + Some(serde_json::json!({"fields":{"name":"other"}})), + ) + .await; + assert_eq!(status, StatusCode::CREATED, "{other}"); + let (status, document) = json_request( + &app_a, + Method::POST, + "/schemas/Document/entities", + Some(serde_json::json!({"fields":{"title":"own","approval":own["id"]}})), + ) + .await; + assert_eq!(status, StatusCode::CREATED, "{document}"); + let entity_path = format!( + "/schemas/Document/entities/{}", + document["id"].as_str().unwrap() + ); + let missing = schema_forge_core::types::EntityId::new("approval"); + for method in [Method::POST, Method::PUT, Method::PATCH] { + let path = if method == Method::POST { + "/schemas/Document/entities" + } else { + &entity_path + }; + for field in ["approval", "reviewers"] { + let mut failures = Vec::new(); + for id in [other["id"].as_str().unwrap(), missing.as_str()] { + let value = if field == "reviewers" { + serde_json::json!([own["id"], id]) + } else { + serde_json::json!(id) + }; + let (status, body) = json_request( + &app_a, + method.clone(), + path, + Some(serde_json::json!({"fields":{"title":"candidate",field:value}})), + ) + .await; + assert_eq!( + status, + StatusCode::UNPROCESSABLE_ENTITY, + "{method} {field}: {body}" + ); + failures.push(body); + } + assert_eq!( + failures[0], failures[1], + "missing and inaccessible targets must be indistinguishable" + ); + } + let (status, body) = json_request( + &app_a, + method.clone(), + path, + Some(serde_json::json!({"fields":{"title":"candidate","bogus_key":"x"}})), + ) + .await; + assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY, "{method}: {body}"); + assert_eq!(body["error"], "validation_failed"); + assert!(body["message"].as_str().unwrap().contains("bogus_key")); + let (status, body) = json_request( + &app_a, + method.clone(), + path, + Some(serde_json::json!({"title":"missing wrapper"})), + ) + .await; + assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY); + assert_eq!(body["error"], "validation_failed"); + } + let (status, unchanged) = json_request(&app_a, Method::GET, &entity_path, None).await; + assert_eq!(status, StatusCode::OK); + assert_eq!(unchanged["fields"]["approval"], own["id"]); + let admin = app_with_claims(state, claims(&["platform_admin"])); + let (status, body) = json_request( + &admin, + Method::PATCH, + &entity_path, + Some(serde_json::json!({"fields":{"approval":other["id"]}})), + ) + .await; + assert_eq!(status, StatusCode::OK, "administrator override: {body}"); +} diff --git a/crates/schema-forge-backend/src/tenant.rs b/crates/schema-forge-backend/src/tenant.rs index 4d87068..4fc09cd 100644 --- a/crates/schema-forge-backend/src/tenant.rs +++ b/crates/schema-forge-backend/src/tenant.rs @@ -31,6 +31,8 @@ pub struct TenantLevel { #[derive(Debug, Clone, PartialEq, Eq)] #[non_exhaustive] pub enum TenantConfigError { + /// An application schema omitted its tenant annotation. + MissingAnnotation { schema: String }, /// Multiple schemas have `@tenant(root)`. MultipleRoots { first: String, second: String }, /// A `@tenant(child: "X")` references a non-existent schema. @@ -42,6 +44,10 @@ pub enum TenantConfigError { impl fmt::Display for TenantConfigError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { + Self::MissingAnnotation { schema } => write!( + f, + "schema '{schema}' must declare @tenant when a tenant root exists" + ), Self::MultipleRoots { first, second } => { write!( f, @@ -119,6 +125,16 @@ impl TenantConfig { }); } + if root.is_some() { + for schema in schemas { + if !schema.is_tenanted() && !schema.is_system() { + return Err(TenantConfigError::MissingAnnotation { + schema: schema.name.as_str().to_string(), + }); + } + } + } + // Collect all tenant schema names for validation let tenant_schemas: HashSet = levels .iter() @@ -294,6 +310,20 @@ mod tests { assert!(config.hierarchy.is_empty()); } + #[test] + fn tenant_root_rejects_unannotated_application_schema() { + let root = make_schema("Org", vec![Annotation::Tenant(TenantKind::Root)]); + let note = make_schema("Note", vec![]); + assert_eq!( + TenantConfig::from_schemas(&[root.clone(), note]).unwrap_err(), + TenantConfigError::MissingAnnotation { + schema: "Note".into() + } + ); + let system = make_schema("Account", vec![Annotation::System]); + assert!(TenantConfig::from_schemas(&[root, system]).is_ok()); + } + #[test] fn from_schemas_empty_returns_disabled() { let config = TenantConfig::from_schemas(&[]).unwrap(); diff --git a/docs/tenant-isolation.md b/docs/tenant-isolation.md index 9a5b9d4..fc0eb70 100644 --- a/docs/tenant-isolation.md +++ b/docs/tenant-isolation.md @@ -1,6 +1,6 @@ # Tenant isolation -Only schemas with `@tenant(root)` or `@tenant(parent: "...")` receive automatic tenant filters and tenant stamps. Shared schemas remain accessible according to their Cedar policies, including when referenced from tenant-owned rows. +When a tenant root exists, every application schema must declare `@tenant(root)` or `@tenant(parent: "...")`. Validation rejects unannotated application schemas before apply or startup; system schemas remain exempt. Projects without a tenant root continue to support unannotated schemas. Existing projects must annotate previously unscoped application schemas and assign valid ownership to existing rows before serving them. A root row's identity defines its tenant. New root rows store `_tenant = id`; authorization and list scoping derive that value from `id` for existing roots too. Legacy roots with NULL or inconsistent `_tenant` metadata therefore remain accessible to their own members without exposing other roots or requiring a data rewrite. @@ -9,3 +9,5 @@ A tenant-owned child must carry valid `_tenant` metadata. Generated Cedar polici Tenant members cannot move rows by supplying `_tenant` in PUT or PATCH: the server removes that input and preserves stored ownership. Platform administrators can reassign child rows. Root identities remain immutable for every caller. Invitation tenant targets require a configured tenant schema and a type/id pair in the caller's effective tenant chain. Active-tenant narrowing applies before delegation; platform administrators may delegate across tenants. + +Relation writes are checked after rules and hooks against tenant scope and read authorization. Missing and inaccessible targets return the same validation error. This applies to single and collection relations on create, PUT and PATCH. Platform administrators retain cross-tenant access. From a0f0193fa79419b3b602cb78874a00424264d896 Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 12:43:22 -0600 Subject: [PATCH 06/38] fix(cli): preflight migrations and respect operator configuration --- .Codex/plans/cli-migration-behavior.md | 12 ++ Cargo.lock | 1 + crates/schema-forge-cli/Cargo.toml | 1 + crates/schema-forge-cli/src/cli.rs | 37 ++++- crates/schema-forge-cli/src/commands/apply.rs | 43 ++++-- .../schema-forge-cli/src/commands/inspect.rs | 2 +- .../schema-forge-cli/src/commands/migrate.rs | 35 ++++- .../src/commands/schema_update.rs | 95 +++++++++++- crates/schema-forge-cli/src/commands/serve.rs | 19 ++- crates/schema-forge-cli/src/config.rs | 87 ++++++++++- crates/schema-forge-cli/src/http.rs | 144 +++++++++++++++++- crates/schema-forge-core/src/migration.rs | 61 +++++++- docs/entity-cli-reference.md | 8 + docs/migrations/safe-schema-changes.md | 21 +++ docs/rate-limiting.md | 79 ++++++++++ docs/rule-ordering-reference.md | 49 +++++- skills/schemaforge/SKILL.md | 4 +- skills/schemaforge/config-reference.md | 12 ++ skills/schemaforge/dsl-quickref.md | 2 +- skills/schemaforge/dsl-reference.md | 4 +- skills/schemaforge/patterns.md | 4 +- 21 files changed, 661 insertions(+), 59 deletions(-) create mode 100644 .Codex/plans/cli-migration-behavior.md create mode 100644 docs/rate-limiting.md diff --git a/.Codex/plans/cli-migration-behavior.md b/.Codex/plans/cli-migration-behavior.md new file mode 100644 index 0000000..279d474 --- /dev/null +++ b/.Codex/plans/cli-migration-behavior.md @@ -0,0 +1,12 @@ +# CLI migration and operator behavior + +Apply Rust planner/author standards using existing domain and error types. + +1. Plan every desired schema before apply/migrate execution and reject any destructive noninteractive batch before migration, metadata, or revision preparation writes. Preserve interactive per-schema consent and dry-run plans. Test mixed safe/destructive batches with zero writes. +2. Preserve configured listener host/port with optional flags. Keep SchemaForge loopback as its unconfigured host, respecting the framework config search and ACTON environment layers. Test omitted flags, explicit default overrides, and config-file bind/port. +3. Keep the existing public RequiresConfirmation enum variant for source compatibility, but label it review in Display/serialized output, accept legacy serialized spelling, and document it as informational consistently. Introduce plan-aware step classification for fresh unique constraints and test both existing/new schema uniqueness. +4. Add bounded 429 retries to the entity HTTP client, with --max-retries and Retry-After seconds/date support. Rebuild identical requests only for explicit 429 responses, no transport or 5xx retries. Test exhaustion, eventual success, non-429 refusal, and delay parsing. +5. Correct the rule-ordering reference and document governor defaults, reverse-proxy trust and probe configuration. +6. Coordinate read-only CLI connections and webhook validation with owning agents. + +Validation: cargo nextest run for core and CLI with postgres feature, cargo clippy warnings denied, formatting. Root performs workspace integration and release. Semver recommendation: minor because migration machine-readable review labels change and CLI functionality is added; retain deserialization compatibility. diff --git a/Cargo.lock b/Cargo.lock index 0276705..c830309 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7697,6 +7697,7 @@ dependencies = [ "dialoguer", "glob", "heck 0.5.0", + "httpdate", "indicatif", "miette", "mime_guess", diff --git a/crates/schema-forge-cli/Cargo.toml b/crates/schema-forge-cli/Cargo.toml index 53a7569..5641ada 100644 --- a/crates/schema-forge-cli/Cargo.toml +++ b/crates/schema-forge-cli/Cargo.toml @@ -39,6 +39,7 @@ mime_guess = { version = "2.0.5" } sha2 = "0.11.0" tokio-util = { version = "0.7.18", features = ["io"] } schema-forge-mssql = { version = "0.5.0", path = "../schema-forge-mssql", optional = true } +httpdate = "1.0.3" [features] default = ["surrealdb"] diff --git a/crates/schema-forge-cli/src/cli.rs b/crates/schema-forge-cli/src/cli.rs index b38101f..cc19c9d 100644 --- a/crates/schema-forge-cli/src/cli.rs +++ b/crates/schema-forge-cli/src/cli.rs @@ -710,13 +710,13 @@ pub struct ServeArgs { #[arg(long)] pub allow_destructive_migrations: bool, - /// Host address to bind - #[arg(short = 'H', long = "host", default_value = "127.0.0.1")] - pub host: IpAddr, + /// Host address to bind (default: configuration, then 127.0.0.1) + #[arg(short = 'H', long = "host")] + pub host: Option, - /// Port to listen on - #[arg(short = 'p', long = "port", default_value = "3000")] - pub port: u16, + /// Port to listen on (default: configuration, then 3000) + #[arg(short = 'p', long = "port")] + pub port: Option, /// Schema files to load on startup #[arg(long = "schemas", default_value = "schemas/")] @@ -941,6 +941,10 @@ pub struct EntityConnectionArgs { /// Per-request timeout in seconds. #[arg(long = "timeout")] pub timeout: Option, + + /// Maximum retries after HTTP 429 (0 disables retries). + #[arg(long, default_value_t = 3, value_parser = clap::value_parser!(u32).range(0..=10))] + pub max_retries: u32, } /// Field input shared by `create`, `replace`, and `patch`. @@ -1302,6 +1306,23 @@ mod tests { use super::*; use clap::CommandFactory; + #[test] + fn absent_listener_flags_do_not_override_configuration() { + let cli = Cli::try_parse_from(["schemaforge", "serve"]).unwrap(); + let Commands::Serve(args) = cli.command else { + panic!("expected serve") + }; + assert_eq!(args.host, None); + assert_eq!(args.port, None); + let cli = + Cli::try_parse_from(["schemaforge", "serve", "-H", "127.0.0.1", "-p", "3000"]).unwrap(); + let Commands::Serve(args) = cli.command else { + panic!("expected serve") + }; + assert_eq!(args.host, Some(std::net::Ipv4Addr::LOCALHOST.into())); + assert_eq!(args.port, Some(3000)); + } + #[test] fn verify_cli_structure() { // This validates the derive macros produce a valid clap command. @@ -1514,8 +1535,8 @@ mod tests { ]) .unwrap(); if let Commands::Serve(args) = cli.command { - assert_eq!(args.host, std::net::Ipv4Addr::UNSPECIFIED); - assert_eq!(args.port, 8080); + assert_eq!(args.host, Some(std::net::Ipv4Addr::UNSPECIFIED.into())); + assert_eq!(args.port, Some(8080)); assert!(args.watch); } else { panic!("expected Serve command"); diff --git a/crates/schema-forge-cli/src/commands/apply.rs b/crates/schema-forge-cli/src/commands/apply.rs index ee15fa6..94011b8 100644 --- a/crates/schema-forge-cli/src/commands/apply.rs +++ b/crates/schema-forge-cli/src/commands/apply.rs @@ -21,9 +21,24 @@ pub async fn run( output.status(&format!(" {} schemas parsed.", schemas.len())); let svc_config = load_svc_config(global)?; + for schema in &schemas { + schema_forge_acton::webhook::validate_schema_webhooks( + schema, + &svc_config.custom.schema_forge.webhooks, + ) + .await + .map_err(|error| CliError::Config { + message: format!("invalid webhook on {}: {error}", schema.name), + })?; + } + let db_params = resolve_db_params(&svc_config)?; - let backend = super::connect_backend(&db_params, output).await?; + let backend = if args.dry_run { + super::connect_backend_read_only(&db_params, output).await? + } else { + super::connect_backend(&db_params, output).await? + }; apply_to_backend(&args, &schemas, backend.as_ref(), output).await } @@ -43,10 +58,22 @@ pub(super) async fn apply_to_backend( let mut applied_schemas = 0usize; let mut metadata_only_updates = 0usize; + let mut updates = Vec::with_capacity(schemas.len()); for schema in schemas { let existing = backend.load_schema_metadata(&schema.name).await?; let update = SchemaUpdate::plan(existing.as_ref(), schema)?; + updates.push(update); + } + super::schema_update::preflight_destructive_batch( + &updates, + !args.dry_run, + args.force, + Term::stderr().is_term(), + )?; + + for update in &updates { + let schema = &update.schema; let plan = &update.migration; if update.is_empty() { output.status(&format!(" {} .... no changes", schema.name.as_str())); @@ -70,7 +97,7 @@ pub(super) async fn apply_to_backend( schema.name.as_str() )); for (i, step) in plan.steps.iter().enumerate() { - let safety = step.safety(); + let safety = plan.step_safety(step); output.status(&format!(" {}. {} [{}]", i + 1, step, safety)); } @@ -98,12 +125,10 @@ pub(super) async fn apply_to_backend( " {:<16} METADATA UPDATE (0 migration steps)", schema.name.as_str() )); - } else if plan.steps.len() == 1 - && matches!( - &plan.steps[0], - schema_forge_core::migration::MigrationStep::CreateSchema { .. } - ) - { + } else if matches!( + &plan.steps[0], + schema_forge_core::migration::MigrationStep::CreateSchema { .. } + ) { output.status(&format!( " {:<16} CREATE ({} fields){}", schema.name.as_str(), @@ -190,7 +215,7 @@ pub(super) async fn apply_to_backend( fn format_safety_tag(safety: MigrationSafety) -> String { match safety { MigrationSafety::Safe => " [safe]".to_string(), - MigrationSafety::RequiresConfirmation => " [requires_confirmation]".to_string(), + MigrationSafety::RequiresConfirmation => " [review]".to_string(), MigrationSafety::Destructive => " [destructive]".to_string(), _ => String::new(), } diff --git a/crates/schema-forge-cli/src/commands/inspect.rs b/crates/schema-forge-cli/src/commands/inspect.rs index 6929af6..078b679 100644 --- a/crates/schema-forge-cli/src/commands/inspect.rs +++ b/crates/schema-forge-cli/src/commands/inspect.rs @@ -14,7 +14,7 @@ pub async fn run( let svc_config = load_svc_config(global)?; let db_params = resolve_db_params(&svc_config)?; - let backend = super::connect_backend(&db_params, output).await?; + let backend = super::connect_backend_read_only(&db_params, output).await?; let all_schemas = backend.list_schema_metadata().await?; diff --git a/crates/schema-forge-cli/src/commands/migrate.rs b/crates/schema-forge-cli/src/commands/migrate.rs index db1e5d9..849a652 100644 --- a/crates/schema-forge-cli/src/commands/migrate.rs +++ b/crates/schema-forge-cli/src/commands/migrate.rs @@ -19,9 +19,24 @@ pub async fn run( let schemas = parse_all_schemas_with_global(&args.paths, global, output)?; let svc_config = load_svc_config(global)?; + for schema in &schemas { + schema_forge_acton::webhook::validate_schema_webhooks( + schema, + &svc_config.custom.schema_forge.webhooks, + ) + .await + .map_err(|error| CliError::Config { + message: format!("invalid webhook on {}: {error}", schema.name), + })?; + } + let db_params = resolve_db_params(&svc_config)?; - let backend = super::connect_backend(&db_params, output).await?; + let backend = if args.execute { + super::connect_backend(&db_params, output).await? + } else { + super::connect_backend_read_only(&db_params, output).await? + }; migrate_on_backend(&args, &schemas, backend.as_ref(), output).await } @@ -70,6 +85,13 @@ pub(super) async fn migrate_on_backend( plans.push(update); } + super::schema_update::preflight_destructive_batch( + &plans, + args.execute, + args.force, + Term::stderr().is_term(), + )?; + // Render plan match output.mode { OutputMode::Human => { @@ -96,7 +118,7 @@ pub(super) async fn migrate_on_backend( plan.overall_safety() ); for (i, step) in plan.steps.iter().enumerate() { - println!(" {}. {} [{}]", i + 1, step, step.safety()); + println!(" {}. {} [{}]", i + 1, step, plan.step_safety(step)); } println!(); } @@ -118,7 +140,7 @@ pub(super) async fn migrate_on_backend( .map(|s| { serde_json::json!({ "description": s.to_string(), - "safety": s.safety().to_string(), + "safety": plan.step_safety(s).to_string(), }) }) .collect(); @@ -148,7 +170,12 @@ pub(super) async fn migrate_on_backend( println!("{}\tmetadata update\tsafe", schema.name.as_str()); } for step in &plan.steps { - println!("{}\t{}\t{}", schema.name.as_str(), step, step.safety()); + println!( + "{}\t{}\t{}", + schema.name.as_str(), + step, + plan.step_safety(step) + ); } } } diff --git a/crates/schema-forge-cli/src/commands/schema_update.rs b/crates/schema-forge-cli/src/commands/schema_update.rs index 34253c1..1faf675 100644 --- a/crates/schema-forge-cli/src/commands/schema_update.rs +++ b/crates/schema-forge-cli/src/commands/schema_update.rs @@ -43,6 +43,25 @@ pub(super) async fn preflight_schema_batch( validate_tenant_hierarchy(&merge_schema_definitions(existing, desired)) } +/// Refuse a known destructive batch before any schema or revision writes. +pub(super) fn preflight_destructive_batch( + updates: &[SchemaUpdate], + execute: bool, + force: bool, + interactive: bool, +) -> Result<(), CliError> { + if execute + && !force + && !interactive + && updates + .iter() + .any(|update| update.migration.has_destructive_steps()) + { + return Err(CliError::RequiresForce); + } + Ok(()) +} + pub(super) struct SchemaUpdate { pub schema: SchemaDefinition, pub migration: MigrationPlan, @@ -180,7 +199,7 @@ mod tests { } fn load_schema_metadata<'a>( &'a self, - _: &'a SchemaName, + name: &'a SchemaName, ) -> Pin< Box< dyn Future, BackendError>> @@ -189,7 +208,15 @@ mod tests { + 'a, >, > { - Box::pin(async move { Ok(self.stored.lock().unwrap().schema.clone()) }) + Box::pin(async move { + Ok(self + .stored + .lock() + .unwrap() + .schema + .clone() + .filter(|schema| &schema.name == name)) + }) } fn list_schema_metadata( &self, @@ -264,6 +291,70 @@ mod tests { } } + #[tokio::test] + async fn destructive_batch_is_refused_before_safe_schema_or_revision_writes() { + let desired = [ + schema("schema Aaa { label: text }"), + schema("schema Note { title: text }"), + ]; + for command in [Command::Apply, Command::Migrate] { + let mut backend = Backend::seeded(schema("schema Note { title: text extra: text }")); + backend.revisions_supported = true; + let result = match command { + Command::Apply => { + super::super::apply::apply_to_backend( + &ApplyArgs { + paths: vec![], + dry_run: false, + force: false, + with_policies: false, + prepare_record_revisions: true, + }, + &desired, + &backend, + &output(), + ) + .await + } + Command::Migrate => { + super::super::migrate::migrate_on_backend( + &MigrateArgs { + paths: vec![], + execute: true, + force: false, + schema: None, + }, + &desired, + &backend, + &output(), + ) + .await + } + }; + assert!(matches!(result, Err(CliError::RequiresForce)), "{result:?}"); + let stored = backend.stored.lock().unwrap(); + assert_eq!(stored.migrations, 0); + assert_eq!(stored.writes, 0); + assert_eq!(stored.preparations, 0); + } + } + + #[test] + fn destructive_preflight_preserves_dry_run_force_and_interactive_modes() { + let original = schema("schema Note { title: text extra: text }"); + let updates = [ + SchemaUpdate::plan(Some(&original), &schema("schema Note { title: text }")).unwrap(), + ]; + for (execute, force, interactive) in [ + (false, false, false), + (true, true, false), + (true, false, true), + ] { + assert!(preflight_destructive_batch(&updates, execute, force, interactive).is_ok()); + } + assert!(preflight_destructive_batch(&updates, true, false, false).is_err()); + } + #[tokio::test] async fn tenancy_changes_never_write_even_with_force() { for force in [false, true] { diff --git a/crates/schema-forge-cli/src/commands/serve.rs b/crates/schema-forge-cli/src/commands/serve.rs index 50811d9..f890c1a 100644 --- a/crates/schema-forge-cli/src/commands/serve.rs +++ b/crates/schema-forge-cli/src/commands/serve.rs @@ -74,6 +74,17 @@ pub async fn run( Err(e) => return Err(e), }; + for schema in &schemas { + schema_forge_acton::webhook::validate_schema_webhooks( + schema, + &svc_config.custom.schema_forge.webhooks, + ) + .await + .map_err(|error| CliError::Config { + message: format!("invalid webhook on {}: {error}", schema.name), + })?; + } + // 4. Connect to database (try remote, fail explicitly for production) let connected = connect_with_retries(&db_params, output).await?; let backend_arc = connected.backend.clone(); @@ -321,8 +332,12 @@ pub async fn run( // fields here. Database/SurrealDB sections are not touched here — they // were resolved up-front by `load_svc_config` so acton-service's pool // and the schema-forge backend pool see the same URL by construction. - svc_config.service.bind = args.host; - svc_config.service.port = args.port; + if let Some(host) = args.host { + svc_config.service.bind = host; + } + if let Some(port) = args.port { + svc_config.service.port = port; + } svc_config.service.name = "schemaforge".to_string(); // Token auth public paths: both endpoints must be reachable without a diff --git a/crates/schema-forge-cli/src/config.rs b/crates/schema-forge-cli/src/config.rs index 019a780..43b9d69 100644 --- a/crates/schema-forge-cli/src/config.rs +++ b/crates/schema-forge-cli/src/config.rs @@ -110,7 +110,9 @@ impl std::fmt::Display for DbParams { write!( f, "surrealdb {}/{}@{} (user={user}, pass={masked_pass})", - p.namespace, p.database, self.redacted_url() + p.namespace, + p.database, + self.redacted_url() ) } DbParams::Postgres(_) => write!(f, "postgres {}", self.redacted_url()), @@ -140,10 +142,56 @@ pub fn load_svc_config(global: &GlobalOpts) -> Result, } })?, }; + // acton-service defaults to all interfaces; SchemaForge defaults to loopback. + // Preserve an explicitly configured unspecified address, including 0.0.0.0. + if !service_bind_is_configured(global.config.as_deref())? { + svc.service.bind = std::net::Ipv4Addr::LOCALHOST.into(); + } apply_cli_overrides(&mut svc, global)?; Ok(svc) } +/// Inspect the same file layers as acton-service without changing their precedence. +fn service_bind_is_configured(explicit: Option<&Path>) -> Result { + if std::env::var_os("ACTON_SERVICE_BIND").is_some() { + return Ok(true); + } + let mut paths = if let Some(path) = explicit { + vec![path.to_path_buf()] + } else { + vec![ + PathBuf::from("config.toml"), + Config::::recommended_path("schemaforge"), + ] + }; + if explicit.is_none() { + #[cfg(unix)] + paths.push(PathBuf::from("/etc/acton-service/schemaforge/config.toml")); + #[cfg(windows)] + if let Some(root) = std::env::var_os("PROGRAMDATA") { + paths.push(PathBuf::from(root).join("acton-service/schemaforge/config.toml")); + } + } + for path in paths { + let source = match std::fs::read_to_string(&path) { + Ok(source) => source, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue, + Err(source) => return Err(CliError::Io { path, source }), + }; + let config: toml::Value = toml::from_str(&source).map_err(|error| CliError::Config { + message: format!("failed to load {}: {error}", path.display()), + })?; + if config + .get("service") + .and_then(|service| service.get("bind")) + .is_some() + { + return Ok(true); + } + } + Ok(false) +} + fn load_svc_config_from_path(path: &Path) -> Result, CliError> { let path_str = path.to_str().ok_or_else(|| CliError::Config { message: format!("config path is not valid UTF-8: {}", path.display()), @@ -434,6 +482,8 @@ pub struct ResolvedClient { pub insecure: bool, /// Per-request timeout. pub timeout: Duration, + /// Maximum retries after an explicit HTTP 429 response. + pub max_retries: u32, } impl std::fmt::Debug for ResolvedClient { @@ -445,6 +495,7 @@ impl std::fmt::Debug for ResolvedClient { .field("ca_cert", &self.ca_cert) .field("insecure", &self.insecure) .field("timeout", &self.timeout) + .field("max_retries", &self.max_retries) .finish() } } @@ -487,6 +538,7 @@ pub fn resolve_client_config( ca_cert, insecure: conn.insecure, timeout: Duration::from_secs(timeout_secs), + max_retries: conn.max_retries, }) } @@ -571,6 +623,24 @@ mod tests { } } + #[test] + fn listener_file_configuration_and_loopback_fallback() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + let global = GlobalOpts { + config: Some(path.clone()), + ..empty_global() + }; + std::fs::write(&path, "[service]\nport = 3899\n").unwrap(); + let config = load_svc_config(&global).unwrap(); + assert_eq!(config.service.port, 3899); + assert_eq!(config.service.bind, std::net::Ipv4Addr::LOCALHOST); + std::fs::write(&path, "[service]\nport = 8080\nbind = \"0.0.0.0\"\n").unwrap(); + let config = load_svc_config(&global).unwrap(); + assert_eq!(config.service.port, 8080); + assert_eq!(config.service.bind, std::net::Ipv4Addr::UNSPECIFIED); + } + #[test] fn is_postgres_url_recognizes_both_schemes() { assert!(is_postgres_url("postgres://user:pass@host/db")); @@ -863,11 +933,20 @@ mod connection_redaction_tests { #[test] fn connection_labels_do_not_disclose_uri_or_dsn_credentials() { let params = DbParams::Postgres(PostgresParams { - url: "postgresql://operator:SECRET@localhost:5432/example?password=SECRET#SECRET".into(), + url: "postgresql://operator:SECRET@localhost:5432/example?password=SECRET#SECRET" + .into(), }); assert_eq!(params.redacted_url(), "postgresql://localhost:5432/example"); assert!(!params.to_string().contains("SECRET")); - assert_eq!(redact_connection_url("Server=localhost;User ID=operator;Password=SECRET;Database=example"), "(configured database)"); - assert_eq!(redact_connection_url("not a URL with SECRET"), "(configured database)"); + assert_eq!( + redact_connection_url( + "Server=localhost;User ID=operator;Password=SECRET;Database=example" + ), + "(configured database)" + ); + assert_eq!( + redact_connection_url("not a URL with SECRET"), + "(configured database)" + ); } } diff --git a/crates/schema-forge-cli/src/http.rs b/crates/schema-forge-cli/src/http.rs index 75f3f37..a40e854 100644 --- a/crates/schema-forge-cli/src/http.rs +++ b/crates/schema-forge-cli/src/http.rs @@ -182,11 +182,31 @@ pub fn classify_http_error(status: u16, body: &str) -> CliError { } } +/// Honor Retry-After seconds or HTTP dates; malformed/missing headers use +/// exponential backoff from 1 to 30 seconds. The retry count remains bounded. +fn retry_delay( + header: Option<&str>, + retry: u32, + now: std::time::SystemTime, +) -> std::time::Duration { + use std::time::Duration; + if let Some(header) = header { + if let Ok(seconds) = header.trim().parse::() { + return Duration::from_secs(seconds); + } + if let Ok(date) = httpdate::parse_http_date(header) { + return date.duration_since(now).unwrap_or_default(); + } + } + Duration::from_secs(1u64.checked_shl(retry).unwrap_or(30).min(30)) +} + /// HTTP client bound to one running instance and (optionally) one token. pub struct ForgeClient { http: Client, base: String, token: Option, + max_retries: u32, } impl ForgeClient { @@ -229,6 +249,7 @@ impl ForgeClient { http, base: forge_base(&rc.server, &rc.api_version), token: rc.token.clone(), + max_retries: rc.max_retries, }) } @@ -247,6 +268,39 @@ impl ForgeClient { Ok(url) } + /// Retry only explicit rate-limit refusals. Transport errors and server + /// failures have ambiguous write outcomes and must never be replayed here. + async fn send_with_retry( + &self, + request: reqwest::RequestBuilder, + ) -> Result { + let mut retries = 0; + loop { + let attempt = request.try_clone().ok_or_else(|| CliError::Config { + message: "cannot replay a streaming request".into(), + })?; + let response = attempt.send().await.map_err(|error| CliError::Connection { + message: error.to_string(), + })?; + if response.status() != reqwest::StatusCode::TOO_MANY_REQUESTS + || retries >= self.max_retries + { + return Ok(response); + } + let delay = retry_delay( + response + .headers() + .get(reqwest::header::RETRY_AFTER) + .and_then(|value| value.to_str().ok()), + retries, + std::time::SystemTime::now(), + ); + drop(response); + tokio::time::sleep(delay).await; + retries += 1; + } + } + /// Send a request and decode the response. /// /// Returns `Ok(None)` for a 2xx with no body (e.g. 204 on delete), @@ -272,9 +326,7 @@ impl ForgeClient { req = req.json(b); } - let resp = req.send().await.map_err(|e| CliError::Connection { - message: e.to_string(), - })?; + let resp = self.send_with_retry(req).await?; let status = resp.status(); let text = resp.text().await.map_err(|e| CliError::Connection { @@ -570,9 +622,7 @@ impl ForgeClient { req = req.bearer_auth(tok); } - let resp = req.send().await.map_err(|e| CliError::Connection { - message: e.to_string(), - })?; + let resp = self.send_with_retry(req).await?; let status = resp.status(); let content_type = resp @@ -693,6 +743,88 @@ fn content_disposition_filename(header: Option<&str>) -> Option { mod tests { use super::*; + #[test] + fn rate_limit_delays_honor_seconds_dates_and_bounded_fallback() { + use std::time::{Duration, UNIX_EPOCH}; + let now = UNIX_EPOCH + Duration::from_secs(1_700_000_000); + assert_eq!(retry_delay(Some("7"), 0, now), Duration::from_secs(7)); + assert_eq!( + retry_delay( + Some(&httpdate::fmt_http_date(now + Duration::from_secs(9))), + 0, + now + ), + Duration::from_secs(9) + ); + assert_eq!( + retry_delay( + Some(&httpdate::fmt_http_date(now - Duration::from_secs(9))), + 0, + now + ), + Duration::ZERO + ); + assert_eq!(retry_delay(None, 0, now), Duration::from_secs(1)); + assert_eq!(retry_delay(Some("invalid"), 2, now), Duration::from_secs(4)); + assert_eq!(retry_delay(None, u32::MAX, now), Duration::from_secs(30)); + } + + #[tokio::test] + async fn rate_limit_retries_only_429_and_preserves_request_body() { + schema_forge_acton::crypto::install_default_crypto_provider(); + use std::sync::{ + atomic::{AtomicUsize, Ordering}, + Arc, + }; + for (status, failures, max_retries, expected_attempts, success) in [ + (429, 2, 3, 3, true), + (429, 5, 2, 3, false), + (429, 1, 0, 1, false), + (503, 1, 3, 1, false), + ] { + let attempts = Arc::new(AtomicUsize::new(0)); + let counter = attempts.clone(); + let app = axum::Router::new().route( + "/api/v1/forge/schemas/Note/entities", + axum::routing::post(move |axum::Json(body): axum::Json| { + let counter = counter.clone(); + async move { + assert_eq!(body, serde_json::json!({"fields": {"title": "hello"}})); + let attempt = counter.fetch_add(1, Ordering::SeqCst); + let response_status = if attempt < failures { status } else { 200 }; + ( + axum::http::StatusCode::from_u16(response_status).unwrap(), + [("retry-after", "0")], + axum::Json(serde_json::json!({"id": "note_example", "fields": {}})), + ) + } + }), + ); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let server = tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + let client = ForgeClient { + http: Client::new(), + base: forge_base(&format!("http://{address}"), "v1"), + token: None, + max_retries, + }; + let result = client + .send( + Method::POST, + client.url(&["schemas", "Note", "entities"]).unwrap(), + &[], + Some(&serde_json::json!({"fields": {"title": "hello"}})), + ) + .await; + assert_eq!(result.is_ok(), success, "{result:?}"); + assert_eq!(attempts.load(Ordering::SeqCst), expected_attempts); + server.abort(); + } + } + #[test] fn forge_base_joins_versioned_path() { assert_eq!( diff --git a/crates/schema-forge-core/src/migration.rs b/crates/schema-forge-core/src/migration.rs index dda69d8..041f644 100644 --- a/crates/schema-forge-core/src/migration.rs +++ b/crates/schema-forge-core/src/migration.rs @@ -81,7 +81,10 @@ impl<'de> Deserialize<'de> for MigrationId { pub enum MigrationSafety { /// The step is safe and can be applied automatically. Safe, - /// The step may require confirmation (e.g. adding a required field without default). + /// Informational review warning (e.g. a new constraint on existing data). + /// Does not require approval; only destructive changes are gated. + /// The Rust variant name is retained for source compatibility. + #[serde(rename = "Review", alias = "RequiresConfirmation")] RequiresConfirmation, /// The step is destructive and may cause data loss (e.g. dropping a field or schema). Destructive, @@ -91,7 +94,7 @@ impl fmt::Display for MigrationSafety { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::Safe => write!(f, "safe"), - Self::RequiresConfirmation => write!(f, "requires_confirmation"), + Self::RequiresConfirmation => write!(f, "review"), Self::Destructive => write!(f, "destructive"), } } @@ -399,7 +402,7 @@ impl MigrationPlan { pub fn overall_safety(&self) -> MigrationSafety { let mut worst = MigrationSafety::Safe; for step in &self.steps { - let s = step.safety(); + let s = self.step_safety(step); worst = match (worst, s) { (MigrationSafety::Destructive, _) | (_, MigrationSafety::Destructive) => { MigrationSafety::Destructive @@ -414,6 +417,18 @@ impl MigrationPlan { worst } + /// Classify a step in its plan context. Constraints on a newly created + /// table cannot conflict with existing data and are safe. + pub fn step_safety(&self, step: &MigrationStep) -> MigrationSafety { + if matches!(self.steps.first(), Some(MigrationStep::CreateSchema { .. })) + && matches!(step, MigrationStep::AddUnique { .. }) + { + MigrationSafety::Safe + } else { + step.safety() + } + } + /// Returns true if the plan has no steps. pub fn is_empty(&self) -> bool { self.steps.is_empty() @@ -447,7 +462,7 @@ impl fmt::Display for MigrationPlan { self.overall_safety() )?; for (i, step) in self.steps.iter().enumerate() { - writeln!(f, " {}. {} [{}]", i + 1, step, step.safety())?; + writeln!(f, " {}. {} [{}]", i + 1, step, self.step_safety(step))?; } Ok(()) } @@ -1054,6 +1069,39 @@ mod tests { // -- MigrationId tests -- + #[test] + fn unique_constraints_are_safe_only_when_creating_a_new_table() { + let step = MigrationStep::AddUnique { + field: FieldName::new("code").unwrap(), + per_tenant: false, + }; + let mut plan = MigrationPlan::new( + SchemaId::new(), + SchemaName::new("Note").unwrap(), + vec![step.clone()], + ); + assert_eq!(plan.overall_safety(), MigrationSafety::RequiresConfirmation); + assert_eq!(plan.overall_safety().to_string(), "review"); + plan.steps.insert( + 0, + MigrationStep::CreateSchema { + name: plan.schema_name.clone(), + fields: vec![], + tenanted: false, + }, + ); + assert_eq!(plan.overall_safety(), MigrationSafety::Safe); + assert_eq!(plan.step_safety(&step), MigrationSafety::Safe); + assert_eq!( + serde_json::from_str::("\"RequiresConfirmation\"").unwrap(), + MigrationSafety::RequiresConfirmation + ); + assert_eq!( + serde_json::to_string(&MigrationSafety::RequiresConfirmation).unwrap(), + "\"Review\"" + ); + } + #[test] fn migration_id_has_correct_prefix() { let id = MigrationId::new(); @@ -1096,10 +1144,7 @@ mod tests { #[test] fn safety_display() { assert_eq!(MigrationSafety::Safe.to_string(), "safe"); - assert_eq!( - MigrationSafety::RequiresConfirmation.to_string(), - "requires_confirmation" - ); + assert_eq!(MigrationSafety::RequiresConfirmation.to_string(), "review"); assert_eq!(MigrationSafety::Destructive.to_string(), "destructive"); } diff --git a/docs/entity-cli-reference.md b/docs/entity-cli-reference.md index 9967cae..fb1a26f 100644 --- a/docs/entity-cli-reference.md +++ b/docs/entity-cli-reference.md @@ -550,3 +550,11 @@ if ! printf '%s' "$FORGE_TOKEN" \ exit "$code" fi ``` + +## Rate-limit retries + +Entity JSON API and export requests retry HTTP 429 up to `--max-retries` times +(default 3, maximum 10; 0 disables). `Retry-After` seconds and HTTP dates are +honored; otherwise delays double from one second up to 30 seconds. Other HTTP +statuses and transport failures are not retried. See +[rate limiting](rate-limiting.md) for proxy and server configuration. diff --git a/docs/migrations/safe-schema-changes.md b/docs/migrations/safe-schema-changes.md index 249a20d..5f43e32 100644 --- a/docs/migrations/safe-schema-changes.md +++ b/docs/migrations/safe-schema-changes.md @@ -172,3 +172,24 @@ creating PostgreSQL bookkeeping tables. A fresh database is treated as an empty schema registry. An inspection role needs schema `USAGE` and `SELECT` on existing bookkeeping tables, but does not need `CREATE`. Normal write commands initialize bookkeeping as before. + +## Review classifications and batch preflight + +Migration safety has three displayed values: `safe`, `review`, and +`destructive`. `review` is informational across `apply`, `migrate`, `serve`, and +runtime schema PUT. It warns that existing rows can prevent a constraint or +conversion from succeeding; it does not request confirmation. Clean conflicting +data before applying. `--force` permits destructive changes but cannot override +database constraints. The Rust enum retains `RequiresConfirmation` for source +compatibility; serialization emits `Review` and accepts the legacy spelling. + +Unique constraints created with a new table are `safe`, and the CLI labels the +plan `CREATE` even when it includes additional constraint steps. Adding a unique +constraint to an existing table is `review`. + +Before a noninteractive `apply` or `migrate --execute` writes any schema, +metadata, or record-revision state, it plans the complete selected batch. Any +destructive plan refuses the whole batch unless `--force` is present. Dry runs +remain available without force. Interactive users can still approve or skip +individual destructive schemas. Backend failures during execution can still +leave earlier schemas committed; preflight is not a transaction for the batch. diff --git a/docs/rate-limiting.md b/docs/rate-limiting.md new file mode 100644 index 0000000..bcf15b4 --- /dev/null +++ b/docs/rate-limiting.md @@ -0,0 +1,79 @@ +# HTTP rate limiting and proxy deployment + +`schemaforge serve` enables acton-service's governor limiter by default. Its +configuration lives at the top-level `[rate_limit]`, separate from +`[schema_forge.export.rate_limit]`. Defaults are `auto_apply = true`, +`per_user_rpm = 200`, `per_client_rpm = 1000`, `window_secs = 60`, and +`trust_forwarded_headers = false`. Governor replenishes per-minute quotas +continuously and uses a global burst of 10% of the applicable quota (20 for +anonymous/user requests). `window_secs` does not change governor's per-minute +quota calculation. + +Authenticated requests use the user/client identity when claims are available. +Anonymous requests, including login, health, readiness and metadata requests, +fall back to the direct TCP peer address and share its user-limit bucket. +Behind a reverse proxy, that address is usually the proxy itself. + +For a server reachable only through a trusted proxy, set +`trust_forwarded_headers = true` and configure the proxy to overwrite incoming +`X-Forwarded-For` and `X-Real-IP`. Governor reads the first forwarded address. +Prevent direct access to the application port, since otherwise callers could +choose their own bucket by spoofing headers. If those conditions cannot be +met, keep this option false and size the shared quota for aggregate traffic, +or enforce rate limits at the proxy and set `auto_apply = false`. + +```toml +[rate_limit] +per_user_rpm = 200 +per_client_rpm = 1000 +trust_forwarded_headers = true + +[rate_limit.routes."POST /api/v1/forge/auth/login"] +requests_per_minute = 30 +burst_size = 10 +per_user = true + +[rate_limit.routes."GET /health"] +requests_per_minute = 6000 +burst_size = 1000 +per_user = false + +[rate_limit.routes."GET /ready"] +requests_per_minute = 6000 +burst_size = 1000 +per_user = false +``` + +Route overrides use full incoming paths, optionally prefixed by the HTTP +method, and replace the global quota for matching requests. `per_user = true` +uses an identity or anonymous IP bucket; `false` uses one bucket shared by all +callers to that route. The example gives probes separate generously sized +buckets so login traffic cannot consume their allowance. Adjust paths to match +your deployed probe endpoints. Governor has no per-route exemption switch; +to guarantee unthrottled probes, disable automatic governor application and +configure the proxy limiter to exclude probe routes. A quota of zero is not an +exemption. + +## Entity CLI retries + +Entity JSON API requests and export requests retry explicit HTTP 429 responses +up to `--max-retries` times (default 3, maximum 10). `--max-retries 0` disables +retries. The client honors `Retry-After` in seconds or HTTP-date form; without a +valid header it waits 1, 2, 4 seconds and so on, capped at 30 seconds per delay. +The retry count bounds attempts; a server-supplied delay can be longer than the +fallback cap. A past date permits an immediate retry. The final 429 becomes the +normal CLI HTTP error. Transport failures and other status codes are not +retried because a write may already have committed. Direct object-storage file +transfers do not use these API retries. + +```sh +schemaforge entity create Note --set title=example --max-retries 5 +``` + +## Listener precedence + +Explicit `serve -H` and `-p` flags override `ACTON_SERVICE_BIND` and +`ACTON_SERVICE_PORT`, which override `[service] bind` and `port` in the selected +configuration. With no configured listener, SchemaForge binds to +`127.0.0.1:3000`. An explicit `0.0.0.0` remains supported. The startup banner +shows the effective address. diff --git a/docs/rule-ordering-reference.md b/docs/rule-ordering-reference.md index 36b2e51..0fd2e77 100644 --- a/docs/rule-ordering-reference.md +++ b/docs/rule-ordering-reference.md @@ -13,19 +13,44 @@ bundles, and rollout. ## 1. Canonical write-path ordering (issue #105) -For every entity create, update (PUT), and patch (PATCH), the engine executes a -single, fixed, engine-controlled sequence. The order does not depend on schema -authoring or on field declaration order across phases: +The route handlers prepare and authorize the proposed row before evaluating +rules. Field declaration order does not change the order of phases: ```text -@default → @compute → @require → before_* hooks → PERSIST → { after_* hooks, webhook dispatch } -└───────────── rule phases (in-transaction) ─────────┘ └ network ┘ └────────── detached fan-out ──────────┘ +CREATE +schema Cedar write check -> reject hidden keys -> JSON conversion +-> tenant/owner stamping -> field write filtering -> tenant/owner re-stamping +-> audit columns -> @default -> @compute -> required-field validation +-> Cedar Create on computed entity -> related-row prefetch -> @require +-> before_validate -> before_change -> required-field re-check -> PERSIST +-> after hooks and webhook dispatch + +PUT / PATCH +Cedar and record checks on stored row -> reject hidden keys -> JSON conversion +-> remove client owner/_tenant edits +-> PUT: null omitted optional fields and restore server-owned fields + PATCH: merge supplied fields with stored row +-> field write filtering (restore denied fields from stored row) +-> audit columns -> @compute -> required-field validation +-> related-row prefetch -> @require +-> before_validate -> before_change -> required-field re-check -> PERSIST +-> after hooks and webhook dispatch ``` The three rule phases live in `crates/schema-forge-acton/src/rules.rs` (`apply_defaults`, `apply_computed`, `check_requires`); the route handlers in `crates/schema-forge-acton/src/routes/entities.rs` call them in this order -before dispatching any `before_*` hook. +before dispatching any `before_*` hook. Rules and hooks see only permitted +client input, together with stored and server-derived values. PUT replaces +writable fields: omitted optional fields become `null`, so computations and +requirements observe those nulls. PATCH retains omitted fields. Neither update +method applies insert-only defaults. Create authorization evaluates the proposed +entity after computation and before `@require`. + +Optional create-reconciliation headers can resolve a previously completed +create before this new-write sequence; see [create reconciliation](create-reconciliation.md). +Related-row prefetch performs I/O before requirement evaluation. This sequence +does not promise a database transaction spanning rules and external hooks. ### 1.1 Why rules run before hooks @@ -39,7 +64,9 @@ never costs a hook call. | Invariant | Guarantee | |---|---| -| In-transaction, pre-persistence | All three rule phases run before the backend write, inside the same request that persists. | +| Pre-persistence | Rules run before the backend write in the same request; related reads and hooks are not one database transaction. | +| Authorized rule inputs | Field write filtering precedes every rule phase; PUT nulls omitted writable optional fields and PATCH preserves omitted values. | +| Computed create authorization | Cedar `Create` runs after `@compute` and required validation, before `@require`. | | Rules ahead of `before_*` hooks | `@default`/`@compute`/`@require` all complete before the first `before_*` hook is dispatched. | | Deterministic, no reentrancy | Phases run in the fixed order above; each visits fields in schema declaration order; a phase never re-invokes an earlier phase. | | Rejection suppresses all downstream work | A `@require` failure returns **422** and fires **no** `before_*` hook, persists **nothing**, and therefore fires **no** `after_*` hook and **no** webhook. | @@ -60,7 +87,13 @@ never costs a hook call. `Ok(true)`; a definite `false` is a 422 rejection, and an error or non-boolean result is a 500 (a broken predicate can never let a write through). -This ordering is proven by integration tests: +The filtering and replacement edges are pinned by +`write_rules_observe_only_authorized_input_and_keep_server_values`, +`required_fields_reject_null_and_put_does_not_apply_create_defaults`, and +`put_omission_matches_persisted_values_and_preserves_denied_fields` in +`crates/schema-forge-acton/tests/conditional_entities.rs`. + +The phase ordering is also covered by integration tests: `rule_phase_order_default_then_compute_then_require_is_observable` (in `crates/schema-forge-acton/tests/integration.rs`) and `require_rejection_fires_no_before_or_after_hook_and_persists_nothing` / diff --git a/skills/schemaforge/SKILL.md b/skills/schemaforge/SKILL.md index ecdd802..0976113 100644 --- a/skills/schemaforge/SKILL.md +++ b/skills/schemaforge/SKILL.md @@ -653,7 +653,7 @@ The two backends are **mutually exclusive** at build time (enforced by acton-ser - Allowed on `text`, `integer`, `float`, `datetime`, `enum`. Other types are a parse error (`UniqueOnUnsupportedType`) — `richtext`, `json`, `boolean`, arrays, `composite`, `relation`, `file`. - For a `@tenant(parent: "...")` schema the underlying constraint is composite on `(_tenant, field)`; two tenants can hold the same value. - For a `@tenant(root)` schema it is a plain table-wide constraint on `(field)`. A root's rows *are* the tenants, so there is no outer tenant to scope them by. (Earlier releases scoped them to `(_tenant, field)` like a child, which silently accepted every duplicate — `_tenant` is empty on a platform-level create and SQL treats NULLs as distinct. See #134 and the CHANGELOG migration note if you have a database applied before the fix.) -- Adding `unique` to a column with existing duplicates fails at apply time. The migration step `AddUnique` is classified `RequiresConfirmation`; clean data first or pass `--force`. +- Adding `unique` to a column with existing duplicates fails at apply time. The migration step `AddUnique` is classified `review`; clean duplicate data first. This informational class does not require `--force`, and `--force` cannot bypass database constraints. - A write that collides returns **HTTP 409** with body `{ "error": "unique_violation", "schema": "...", "field": "...", "message": "..." }`. Generated edit forms route this onto the offending field via `react-hook-form`'s `setError`. ## Quick Reference — Annotations @@ -858,7 +858,7 @@ From a `.schema` file, SchemaForge produces: | Entity body without `fields` wrapper | `{"fields": {"name": "value"}}` | | `active: boolean unique` (or `json` / array / relation / file / composite / richtext) | Move `unique` to a `text`/`integer`/`float`/`datetime`/`enum` field — those are the only types it's allowed on | | Expecting `unique` on a `@tenant(parent: ...)` schema to be table-wide | It's **per-tenant**: different tenants may hold the same value. For cross-tenant uniqueness, drop the `@tenant` annotation or move the field onto the tenant-root entity, where `unique` *is* table-wide | -| Adding `unique` in a migration against a column with existing duplicates | Clean the duplicates first; `AddUnique` is classified `RequiresConfirmation` and the apply will fail otherwise | +| Adding `unique` in a migration against a column with existing duplicates | Clean the duplicates first; `AddUnique` is classified `review` and the apply will fail otherwise | ## Additional Resources diff --git a/skills/schemaforge/config-reference.md b/skills/schemaforge/config-reference.md index 28a290c..917bbc3 100644 --- a/skills/schemaforge/config-reference.md +++ b/skills/schemaforge/config-reference.md @@ -250,3 +250,15 @@ schema-forge serve --db-url postgres://user:pass@host:5432/dbname ``` The two backends are **mutually exclusive** at build time (enforced by acton-service). The binary ships with one or the other. + +## HTTP governor rate limiting + +The top-level `[rate_limit]` applies to all HTTP traffic by default, including +anonymous login and probe routes. Defaults: `per_user_rpm = 200`, +`per_client_rpm = 1000`, `auto_apply = true`, `trust_forwarded_headers = false`. +Anonymous clients share a bucket by TCP peer address, which is the proxy +address behind a reverse proxy. `[rate_limit.routes."POST /api/v1/forge/auth/login"]` +supports `requests_per_minute`, `burst_size`, and `per_user` overrides. +See [rate limiting and proxy deployment](../../docs/rate-limiting.md) for trusted +forwarded-header configuration, separate probe quotas and limiter disabling. +This is independent of `[schema_forge.export.rate_limit]`. diff --git a/skills/schemaforge/dsl-quickref.md b/skills/schemaforge/dsl-quickref.md index c134615..b372817 100644 --- a/skills/schemaforge/dsl-quickref.md +++ b/skills/schemaforge/dsl-quickref.md @@ -40,7 +40,7 @@ At-a-glance tables. For full grammar see [dsl-reference.md](dsl-reference.md). F - Allowed on `text`, `integer`, `float`, `datetime`, `enum`. Other types are a parse error (`UniqueOnUnsupportedType`) — `richtext`, `json`, `boolean`, arrays, `composite`, `relation`, `file`. - For a `@tenant(parent: "...")` schema the underlying constraint is composite on `(_tenant, field)`; two tenants can hold the same value. - For a `@tenant(root)` schema it is a plain table-wide constraint on `(field)`. A root's rows *are* the tenants, so there is no outer tenant to scope them by. (Earlier releases scoped them to `(_tenant, field)` like a child, which silently accepted every duplicate — `_tenant` is empty on a platform-level create and SQL treats NULLs as distinct. See #134 and the CHANGELOG migration note if you have a database applied before the fix.) -- Adding `unique` to a column with existing duplicates fails at apply time. The migration step `AddUnique` is classified `RequiresConfirmation`; clean data first or pass `--force`. +- Adding `unique` to a column with existing duplicates fails at apply time. The migration step `AddUnique` is classified `review`; clean duplicate data first. This informational class does not require `--force`, and `--force` cannot bypass database constraints. - A write that collides returns **HTTP 409** with body `{ "error": "unique_violation", "schema": "...", "field": "...", "message": "..." }`. Generated edit forms route this onto the offending field via `react-hook-form`'s `setError`. ## Schema-Level Annotations (before `schema` keyword) diff --git a/skills/schemaforge/dsl-reference.md b/skills/schemaforge/dsl-reference.md index ba8f203..e47a2bd 100644 --- a/skills/schemaforge/dsl-reference.md +++ b/skills/schemaforge/dsl-reference.md @@ -361,7 +361,7 @@ Database mapping: - **SurrealDB (tenanted)**: `DEFINE INDEX uq_{table}_{field} ON {table} FIELDS _tenant, {field} UNIQUE;` - **SurrealDB (non-tenanted)**: `DEFINE INDEX uq_{table}_{field} ON {table} FIELDS {field} UNIQUE;` -Migration safety: `AddUnique` is classified as `RequiresConfirmation` +Migration safety: `AddUnique` is classified as `review` because adding a unique constraint to a column with pre-existing duplicate values will fail at apply time. Plan for data cleanup before running the migration. @@ -808,7 +808,7 @@ Semantics and limits: | `@require` / `@compute` / `@default` expression must type-check against the schema's field types | Apply-time error (`RuleTypeError`, mapped to `line:column`) | | `@require(...)` requires both a CEL expression and a message argument | Parse error | | `unique` modifier allowed only on `text`/`integer`/`float`/`datetime`/`enum` fields | Parse error (`UniqueOnUnsupportedType`) | -| Adding `unique` to a column with duplicate existing rows | Migration apply error (`RequiresConfirmation` safety class — operator must clean data first) | +| Adding `unique` to a column with duplicate existing rows | Migration apply error (`review` safety class — operator must clean data first) | | Write that collides with a `unique` field | API 409 (`unique_violation`) with `{ schema, field }` body | ## Round-Trip Fidelity diff --git a/skills/schemaforge/patterns.md b/skills/schemaforge/patterns.md index 7c8a6d4..0b84598 100644 --- a/skills/schemaforge/patterns.md +++ b/skills/schemaforge/patterns.md @@ -198,12 +198,12 @@ Rule: any schema with `@tenant(...)` (root *or* child) gets a composite `(_tenan ### Migration safety -`AddUnique` is classified `RequiresConfirmation` — `schema-forge apply` will refuse it without `--force` because the DDL fails against existing duplicate rows. Workflow: +`AddUnique` on an existing table is classified `review`, an informational warning that does not require approval. Existing duplicate rows must be cleaned first; `--force` cannot bypass the database constraint. Unique constraints on a newly created table are `safe`. Workflow: 1. Add `unique` to the schema. 2. `schema-forge migrate` — shows the `AddUnique` step. 3. Query/clean any existing duplicates: `SELECT slug, COUNT(*) FROM Organization GROUP BY slug HAVING COUNT(*) > 1`. -4. `schema-forge apply --force` (or `migrate --execute --force`). +4. Apply the migration after cleaning duplicate rows. `RemoveUnique` is `Safe` — drops the index/constraint with no data risk. From 46c67f264a4cf3d45f40df8ec1c506a4dfc206bc Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 12:44:42 -0600 Subject: [PATCH 07/38] fix(cli): list destructive batch steps before refusing writes --- .../src/commands/schema_update.rs | 58 ++++++++++++++++--- crates/schema-forge-cli/src/error.rs | 5 ++ crates/schema-forge-cli/src/http.rs | 4 +- 3 files changed, 55 insertions(+), 12 deletions(-) diff --git a/crates/schema-forge-cli/src/commands/schema_update.rs b/crates/schema-forge-cli/src/commands/schema_update.rs index 1faf675..1b99911 100644 --- a/crates/schema-forge-cli/src/commands/schema_update.rs +++ b/crates/schema-forge-cli/src/commands/schema_update.rs @@ -50,14 +50,25 @@ pub(super) fn preflight_destructive_batch( force: bool, interactive: bool, ) -> Result<(), CliError> { - if execute - && !force - && !interactive - && updates + if execute && !force && !interactive { + let destructive_steps: Vec<_> = updates .iter() - .any(|update| update.migration.has_destructive_steps()) - { - return Err(CliError::RequiresForce); + .flat_map(|update| { + update + .migration + .steps + .iter() + .filter(|step| { + step.safety() == schema_forge_core::migration::MigrationSafety::Destructive + }) + .map(|step| format!(" {}: {step}", update.schema.name)) + }) + .collect(); + if !destructive_steps.is_empty() { + return Err(CliError::RequiresForceBatch { + details: destructive_steps.join("\n"), + }); + } } Ok(()) } @@ -331,7 +342,10 @@ mod tests { .await } }; - assert!(matches!(result, Err(CliError::RequiresForce)), "{result:?}"); + assert!( + matches!(result, Err(CliError::RequiresForceBatch { .. })), + "{result:?}" + ); let stored = backend.stored.lock().unwrap(); assert_eq!(stored.migrations, 0); assert_eq!(stored.writes, 0); @@ -339,6 +353,32 @@ mod tests { } } + #[test] + fn destructive_preflight_reports_every_schema_and_step() { + let original = schema("schema Note { title: text extra: text other: text }"); + let task = schema("schema Task { title: text obsolete: text }"); + let updates = [ + SchemaUpdate::plan(Some(&original), &schema("schema Note { title: text }")).unwrap(), + SchemaUpdate::plan(Some(&task), &schema("schema Task { title: text }")).unwrap(), + ]; + let error = preflight_destructive_batch(&updates, true, false, false).unwrap_err(); + assert_eq!( + error.exit_code() as i32, + CliError::RequiresForce.exit_code() as i32 + ); + let message = error.to_string(); + for part in [ + "Note", + "extra", + "other", + "Task", + "obsolete", + "no schemas were applied", + ] { + assert!(message.contains(part), "missing {part}: {message}"); + } + } + #[test] fn destructive_preflight_preserves_dry_run_force_and_interactive_modes() { let original = schema("schema Note { title: text extra: text }"); @@ -445,7 +485,7 @@ mod tests { let backend = Backend::seeded(original.clone()); let result = command.run(&backend, schema(new), true).await; assert!( - matches!(result, Err(CliError::RequiresForce)), + matches!(result, Err(CliError::RequiresForceBatch { .. })), "{command:?}: {result:?}" ); let stored = backend.stored.lock().unwrap(); diff --git a/crates/schema-forge-cli/src/error.rs b/crates/schema-forge-cli/src/error.rs index c079a28..3cd4f57 100644 --- a/crates/schema-forge-cli/src/error.rs +++ b/crates/schema-forge-cli/src/error.rs @@ -84,6 +84,10 @@ pub enum CliError { #[error("destructive changes require --force in non-interactive mode; for field renames declare @renamed_from(\"old_name\") to preserve data")] RequiresForce, + /// Noninteractive migration preflight reports every destructive step. + #[error("destructive changes require --force in non-interactive mode; no schemas were applied:\n{details}\nFor field renames declare @renamed_from(\"old_name\") to preserve data")] + RequiresForceBatch { details: String }, + /// HTTP server errors. #[error("server error: {message}")] Server { message: String }, @@ -171,6 +175,7 @@ impl CliError { | Self::SchemaNotFound { .. } | Self::DirectoryExists { .. } | Self::RequiresForce + | Self::RequiresForceBatch { .. } | Self::Other(_) => ExitCode::GeneralError, } } diff --git a/crates/schema-forge-cli/src/http.rs b/crates/schema-forge-cli/src/http.rs index a40e854..44d481b 100644 --- a/crates/schema-forge-cli/src/http.rs +++ b/crates/schema-forge-cli/src/http.rs @@ -558,9 +558,7 @@ impl ForgeClient { if let Some(tok) = &self.token { req = req.bearer_auth(tok); } - let mut resp = req.send().await.map_err(|e| CliError::Connection { - message: e.to_string(), - })?; + let mut resp = self.send_with_retry(req).await?; let status = resp.status(); let final_url = resp.url().to_string(); From d38af47a50eb86fe08382f84e959d0dffc32a4bb Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 12:42:42 -0600 Subject: [PATCH 08/38] docs: clarify required tenant annotations --- skills/schemaforge/dsl-quickref.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/skills/schemaforge/dsl-quickref.md b/skills/schemaforge/dsl-quickref.md index b372817..8f7374f 100644 --- a/skills/schemaforge/dsl-quickref.md +++ b/skills/schemaforge/dsl-quickref.md @@ -57,6 +57,8 @@ At-a-glance tables. For full grammar see [dsl-reference.md](dsl-reference.md). F | Hook | `@hook(event) """intent"""` | declare a lifecycle hook (see hooks-reference.md) | | Export | `@export(formats: [csv\|ndjson\|xlsx\|zip], bundle_files: bool, max_rows: N)` | enable bulk export (fail-closed; distinct Cedar `Export{Entity}` action, not `Read`). See export.md | +When a tenant root exists, every application schema must declare a tenant annotation. Only `@system` schemas are exempt. Unannotated application schemas are rejected during apply and startup. Relation writes must reference visible targets within the caller's tenant scope; platform administrators may cross tenant boundaries. See [tenant isolation](../../docs/tenant-isolation.md). + ## Field-Level Annotations (after modifiers on a field line) | Annotation | Syntax | Purpose | From 24857cf4621548d398fa5e5093c5df9287ab48a6 Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 12:45:22 -0600 Subject: [PATCH 09/38] fix: validate patch relations after hooks and tenant lookup --- .../schema-forge-acton/src/routes/entities.rs | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/crates/schema-forge-acton/src/routes/entities.rs b/crates/schema-forge-acton/src/routes/entities.rs index abbf355..f3cee0f 100644 --- a/crates/schema-forge-acton/src/routes/entities.rs +++ b/crates/schema-forge-acton/src/routes/entities.rs @@ -1954,23 +1954,6 @@ fn collect_relation_ids(value: &DynamicValue, out: &mut HashSet) { // Cross-entity reads in @require: prefetch-and-bind (#95) // --------------------------------------------------------------------------- -/// Run `@require` validation with cross-entity-read (`related..`) -/// support (#95). -/// -/// The CEL engine stays pure: this resolver performs ALL I/O *before* -/// evaluation, dereferences each referenced `Relation{One}` field to its -/// committed, tenant-scoped related row, projects the row to a `CelValue::Map`, -/// and injects a `related` binding next to `principal`/`now` — exactly the -/// "prefetch-and-bind" pattern the request clock `now` already uses. It then -/// calls the pure [`check_requires_with_bindings`]. -/// -/// Fail-closed: if a referenced relation's FK is absent/null, the related row -/// does not exist, or tenant scope hides it, that `related.F` entry is simply -/// NOT bound; a `@require` referencing it then hits an absent reference and the -/// existing fail-closed contract turns it into a rejection/eval-error. -/// -/// Fast path: when no `@require` on the schema references `related.*`, no I/O is -/// performed and the pure binding set is used directly. /// Validate the final relation values before any write reaches storage. async fn validate_relation_targets( forge: &acton_service::prelude::ActorHandle, @@ -2051,6 +2034,23 @@ async fn validate_relation_targets( Ok(()) } +/// Run `@require` validation with cross-entity-read (`related..`) +/// support (#95). +/// +/// The CEL engine stays pure: this resolver performs ALL I/O *before* +/// evaluation, dereferences each referenced `Relation{One}` field to its +/// committed, tenant-scoped related row, projects the row to a `CelValue::Map`, +/// and injects a `related` binding next to `principal`/`now` — exactly the +/// "prefetch-and-bind" pattern the request clock `now` already uses. It then +/// calls the pure [`check_requires_with_bindings`]. +/// +/// Fail-closed: if a referenced relation's FK is absent/null, the related row +/// does not exist, or tenant scope hides it, that `related.F` entry is simply +/// NOT bound; a `@require` referencing it then hits an absent reference and the +/// existing fail-closed contract turns it into a rejection/eval-error. +/// +/// Fast path: when no `@require` on the schema references `related.*`, no I/O is +/// performed and the pure binding set is used directly. async fn check_requires_with_related( forge: &acton_service::prelude::ActorHandle, schema: &SchemaDefinition, @@ -3929,16 +3929,6 @@ pub async fn patch_entity( apply_computed(&schema_def, &mut merged, claims.as_ref(), rules_now) .map_err(rule_error_to_forge)?; - validate_relation_targets( - &forge, - &policy_store, - &schema_def, - &merged, - claims.as_ref(), - &tenant_config, - ) - .await?; - validate_required_fields(&schema_def, &merged)?; // Tenant config for cross-entity-read tenant scoping (#95). @@ -4005,6 +3995,16 @@ pub async fn patch_entity( validate_required_fields(&schema_def, &merged)?; check_field_constraints(&schema_def, &merged)?; + validate_relation_targets( + &forge, + &policy_store, + &schema_def, + &merged, + claims.as_ref(), + &tenant_config, + ) + .await?; + crate::webhook::validate_subscription_fields( &schema_def, &merged, From 1f3cb232eee87e03510e1ef879eb4476cdbd325d Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 12:46:47 -0600 Subject: [PATCH 10/38] chore(release): prepare schemaforge v0.46.0 --- CHANGELOG.md | 56 ++++++++++++++++++++++++ Cargo.lock | 18 ++++---- SECURITY.md | 4 +- crates/schema-forge-acton/Cargo.toml | 4 +- crates/schema-forge-backend/Cargo.toml | 2 +- crates/schema-forge-cel/Cargo.toml | 4 +- crates/schema-forge-cli/Cargo.toml | 4 +- crates/schema-forge-core/Cargo.toml | 2 +- crates/schema-forge-dsl/Cargo.toml | 4 +- crates/schema-forge-mssql/Cargo.toml | 6 +-- crates/schema-forge-postgres/Cargo.toml | 2 +- crates/schema-forge-surrealdb/Cargo.toml | 2 +- 12 files changed, 82 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a18fc5b..45f7ef7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,62 @@ is pre-1.0; breaking changes bump the **minor** version per ## [Unreleased] +## [0.46.0] - 2026-09-25 + +### Runtime and API behavior + +- Validate tenant declarations consistently across startup, CLI schema application, + and runtime schema changes. Applications with a tenant root must annotate every + application schema; built-in system schemas remain shared. +- Validate relation targets against tenant scope and read authorization before + create, PUT, or PATCH persists. Platform administrators retain their documented + cross-tenant capabilities. +- Apply hidden-field projection to relation display labels and webhook payloads. + Webhooks use a fixed conservative field policy and plain JSON payload version 2. +- Enforce configured webhook URL schemes and public destinations during + configuration and delivery. Delivery checks and pins DNS results, with redirects + and environment proxies disabled. +- Reject undeclared entity fields before persistence. Foreign-key errors include + machine-readable `error` and `message` fields; entity and schema JSON rejections + use the API error envelope. Internal storage diagnostics stay out of REST and + GraphQL error messages, and database connection errors omit credentials. + +### Database and operator fixes + +- PostgreSQL planning and inspection connections perform no bookkeeping DDL. + Fresh databases plan as empty registries, and read-only roles can inspect existing + metadata without schema creation privileges. +- PostgreSQL `contains` and `startswith` now match literal, case-sensitive text. + Unsupported array filter comparisons return validation errors before execution. +- `apply` and `migrate --execute` preflight all selected migration plans before + applying a noninteractive batch. Refusals identify every destructive schema and + step requiring `--force`. +- Migration warnings display `review` when they are informational. New-table unique + constraints are safe, and new schemas retain their `CREATE` label. +- Explicit listener flags override environment and file settings; omitted flags + preserve configuration. An unconfigured server defaults to `127.0.0.1:3000`. +- Entity CLI requests retry HTTP 429 with `Retry-After` support and bounded fallback + backoff, controlled by `--max-retries`. Transport failures are not retried. +- Document governor quotas, proxy configuration, probe routes, the full write-rule + order, and webhook delivery guarantees. + +### Upgrade notes + +Webhook consumers must support `payload_version: 2` and plain JSON field values. +Hidden fields and fields with field-access annotations are excluded. Webhooks +remain best effort with no durable history or replay; applications must reconcile +current state separately when delivery gaps matter. See [webhooks](docs/webhooks.md). + +Before upgrading a tenanted deployment, annotate every application schema with its +intended tenant relationship and migrate existing ownership explicitly. Unannotated +application schemas are no longer implicitly shared when a tenant root exists. +See [tenant isolation](docs/tenant-isolation.md). + +Migration safety serialization emits `Review`; legacy `RequiresConfirmation` input +is still accepted. Rust embedders must update webhook event constructor calls to +pass schema definitions, and handler callers must use the new JSON extractor. +Workspace crate versions are coordinated for the updated public core/backend types. + ## [0.45.0] - 2026-09-24 ### Security and correctness diff --git a/Cargo.lock b/Cargo.lock index c830309..ec8d009 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7598,7 +7598,7 @@ dependencies = [ [[package]] name = "schema-forge-acton" -version = "0.44.0" +version = "0.45.0" dependencies = [ "acton-service", "arc-swap", @@ -7655,7 +7655,7 @@ dependencies = [ [[package]] name = "schema-forge-backend" -version = "0.18.0" +version = "0.19.0" dependencies = [ "acton-service", "argon2", @@ -7670,7 +7670,7 @@ dependencies = [ [[package]] name = "schema-forge-cel" -version = "0.11.0" +version = "0.12.0" dependencies = [ "base64", "chrono", @@ -7686,7 +7686,7 @@ dependencies = [ [[package]] name = "schema-forge-cli" -version = "0.45.0" +version = "0.46.0" dependencies = [ "acton-service", "assert_cmd", @@ -7729,7 +7729,7 @@ dependencies = [ [[package]] name = "schema-forge-core" -version = "0.18.0" +version = "0.19.0" dependencies = [ "base64", "chrono", @@ -7743,7 +7743,7 @@ dependencies = [ [[package]] name = "schema-forge-dsl" -version = "0.14.0" +version = "0.15.0" dependencies = [ "logos 0.15.1", "proptest", @@ -7754,7 +7754,7 @@ dependencies = [ [[package]] name = "schema-forge-mssql" -version = "0.5.0" +version = "0.6.0" dependencies = [ "acton-service", "bb8", @@ -7770,7 +7770,7 @@ dependencies = [ [[package]] name = "schema-forge-postgres" -version = "0.13.0" +version = "0.14.0" dependencies = [ "arc-swap", "argon2", @@ -7808,7 +7808,7 @@ dependencies = [ [[package]] name = "schema-forge-surrealdb" -version = "0.13.0" +version = "0.14.0" dependencies = [ "chrono", "schema-forge-backend", diff --git a/SECURITY.md b/SECURITY.md index f2f6cd0..0daab57 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -31,8 +31,8 @@ Security fixes go into the latest release. Older versions do not receive backpor | Version | Supported | |---|---| -| 0.45.x (latest release) | Yes | -| earlier than 0.45 | No, please upgrade | +| 0.46.x (latest release) | Yes | +| earlier than 0.46 | No, please upgrade | ## Scope diff --git a/crates/schema-forge-acton/Cargo.toml b/crates/schema-forge-acton/Cargo.toml index 0cfdd65..e3f790f 100644 --- a/crates/schema-forge-acton/Cargo.toml +++ b/crates/schema-forge-acton/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "schema-forge-acton" -version = "0.44.0" +version = "0.45.0" edition = "2021" [dependencies] @@ -44,7 +44,7 @@ aws-lc-rs = { version = "1", features = ["fips"], optional = true } rustls = { version = "0.23", default-features = false, features = ["std", "aws_lc_rs", "logging"] } schema-forge-signing = { version = "0.1.0", path = "../schema-forge-signing" } lettre = { version = "0.11.22", default-features = false, features = ["tokio1-rustls", "aws-lc-rs", "webpki-roots", "smtp-transport", "builder", "pool", "hostname"] } -schema-forge-cel = { version = "0.11.0", path = "../schema-forge-cel" } +schema-forge-cel = { version = "0.12.0", path = "../schema-forge-cel" } rust_xlsxwriter = { version = "0.95.0", features = ["chrono"] } zip = "8.6.0" diff --git a/crates/schema-forge-backend/Cargo.toml b/crates/schema-forge-backend/Cargo.toml index f59c493..77ac0a3 100644 --- a/crates/schema-forge-backend/Cargo.toml +++ b/crates/schema-forge-backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "schema-forge-backend" -version = "0.18.0" +version = "0.19.0" edition = "2021" [dependencies] diff --git a/crates/schema-forge-cel/Cargo.toml b/crates/schema-forge-cel/Cargo.toml index d1c4e14..0834d15 100644 --- a/crates/schema-forge-cel/Cargo.toml +++ b/crates/schema-forge-cel/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "schema-forge-cel" -version = "0.11.0" +version = "0.12.0" edition = "2021" description = "Minimal, owned CEL (Common Expression Language) evaluator over SchemaForge DynamicValue." @@ -17,7 +17,7 @@ chrono-tz = "0.10.4" # catastrophic backtracking (no ReDoS), matching the evaluator's # guaranteed-terminating / DoS-hardened posture. regex = "1.12.3" -schema-forge-core = { version = "0.18.0", path = "../schema-forge-core" } +schema-forge-core = { version = "0.19.0", path = "../schema-forge-core" } serde_json = "1.0.150" tracing = "0.1" diff --git a/crates/schema-forge-cli/Cargo.toml b/crates/schema-forge-cli/Cargo.toml index 5641ada..f3522ec 100644 --- a/crates/schema-forge-cli/Cargo.toml +++ b/crates/schema-forge-cli/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "schema-forge-cli" -version = "0.45.0" +version = "0.46.0" edition = "2021" [[bin]] @@ -38,7 +38,7 @@ rust-embed = { version = "8.11.0", features = ["interpolate-folder-path"], optio mime_guess = { version = "2.0.5" } sha2 = "0.11.0" tokio-util = { version = "0.7.18", features = ["io"] } -schema-forge-mssql = { version = "0.5.0", path = "../schema-forge-mssql", optional = true } +schema-forge-mssql = { version = "0.6.0", path = "../schema-forge-mssql", optional = true } httpdate = "1.0.3" [features] diff --git a/crates/schema-forge-core/Cargo.toml b/crates/schema-forge-core/Cargo.toml index c549cc8..43837fc 100644 --- a/crates/schema-forge-core/Cargo.toml +++ b/crates/schema-forge-core/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "schema-forge-core" -version = "0.18.0" +version = "0.19.0" edition = "2021" [dependencies] diff --git a/crates/schema-forge-dsl/Cargo.toml b/crates/schema-forge-dsl/Cargo.toml index 77f024b..4e65a94 100644 --- a/crates/schema-forge-dsl/Cargo.toml +++ b/crates/schema-forge-dsl/Cargo.toml @@ -1,11 +1,11 @@ [package] name = "schema-forge-dsl" -version = "0.14.0" +version = "0.15.0" edition = "2021" [dependencies] schema-forge-core = { path = "../schema-forge-core" } -schema-forge-cel = { version = "0.11.0", path = "../schema-forge-cel" } +schema-forge-cel = { version = "0.12.0", path = "../schema-forge-cel" } logos = "0.15" tracing = "0.1" diff --git a/crates/schema-forge-mssql/Cargo.toml b/crates/schema-forge-mssql/Cargo.toml index 4ee2252..416803c 100644 --- a/crates/schema-forge-mssql/Cargo.toml +++ b/crates/schema-forge-mssql/Cargo.toml @@ -1,14 +1,14 @@ [package] name = "schema-forge-mssql" -version = "0.5.0" +version = "0.6.0" edition = "2021" [dependencies] acton-service = { version = "0.43.1", features = ["mssql", "crypto-aws-lc-rs"] } bb8 = "0.9.1" bb8-tiberius = "0.16.0" -schema-forge-backend = { version = "0.18.0", path = "../schema-forge-backend" } -schema-forge-core = { version = "0.18.0", path = "../schema-forge-core" } +schema-forge-backend = { version = "0.19.0", path = "../schema-forge-backend" } +schema-forge-core = { version = "0.19.0", path = "../schema-forge-core" } serde_json = "1.0.151" tiberius = "0.12.3" diff --git a/crates/schema-forge-postgres/Cargo.toml b/crates/schema-forge-postgres/Cargo.toml index 0ff2422..9242d23 100644 --- a/crates/schema-forge-postgres/Cargo.toml +++ b/crates/schema-forge-postgres/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "schema-forge-postgres" -version = "0.13.0" +version = "0.14.0" edition = "2021" [dependencies] diff --git a/crates/schema-forge-surrealdb/Cargo.toml b/crates/schema-forge-surrealdb/Cargo.toml index f0775b5..2f9ba8a 100644 --- a/crates/schema-forge-surrealdb/Cargo.toml +++ b/crates/schema-forge-surrealdb/Cargo.toml @@ -1,7 +1,7 @@ [package] rust-version = "1.97.1" name = "schema-forge-surrealdb" -version = "0.13.0" +version = "0.14.0" edition = "2021" [dependencies] From aa285efbb558a382577e67074ba8e65eb1debd43 Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 12:46:26 -0600 Subject: [PATCH 11/38] test: align tenant fixtures and verify subscription write validation --- crates/schema-forge-acton/tests/auth_demo.rs | 2 +- .../tests/cross_entity_reads.rs | 54 ++++++++++++++ .../tests/graphql_tenants.rs | 23 ++++-- .../tests/graphql_writes.rs | 3 +- schemas/demo.schema | 13 +++- scripts/seed-demo-data.sh | 70 ++++++++++--------- skills/schemaforge/patterns.md | 2 +- 7 files changed, 127 insertions(+), 40 deletions(-) diff --git a/crates/schema-forge-acton/tests/auth_demo.rs b/crates/schema-forge-acton/tests/auth_demo.rs index a57d287..ce84a3f 100644 --- a/crates/schema-forge-acton/tests/auth_demo.rs +++ b/crates/schema-forge-acton/tests/auth_demo.rs @@ -609,7 +609,7 @@ async fn demo_multi_tenancy_isolation() { .unwrap(); register_schema(&org_schema, &backend, &mut registry).await; - // Declare Project as a tenant child; unannotated schemas are shared. + // Declare Project as a tenant child; all application schemas require tenancy. // @access with empty lists = all authenticated users permitted (testing tenancy, not schema-level) let project_schema = SchemaDefinition::new( SchemaId::new(), diff --git a/crates/schema-forge-acton/tests/cross_entity_reads.rs b/crates/schema-forge-acton/tests/cross_entity_reads.rs index e6ea4b4..dcef92b 100644 --- a/crates/schema-forge-acton/tests/cross_entity_reads.rs +++ b/crates/schema-forge-acton/tests/cross_entity_reads.rs @@ -780,3 +780,57 @@ async fn relation_writes_validate_tenant_targets_and_request_fields() { .await; assert_eq!(status, StatusCode::OK, "administrator override: {body}"); } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn subscription_writes_reject_unsafe_urls_before_persistence() { + use schema_forge_backend::{Entity, EntityStore}; + let backend = Arc::new( + SurrealBackend::connect_memory("test", "subscription_urls") + .await + .unwrap(), + ); + let mut registry = HashMap::new(); + let schema = + schema_forge_dsl::parse("@system schema WebhookSubscription { url: text required }") + .unwrap() + .remove(0); + apply_and_register(&backend, &mut registry, schema.clone()).await; + let existing = Entity::new( + schema.name, + std::collections::BTreeMap::from([( + "url".into(), + schema_forge_core::types::DynamicValue::Text("https://8.8.8.8/hook".into()), + )]), + ); + EntityStore::create(backend.as_ref(), &existing) + .await + .unwrap(); + let state = build_state(backend, registry, None).await; + let app = app_with_claims(state, claims(&["platform_admin"])); + let entity_path = format!("/schemas/WebhookSubscription/entities/{}", existing.id); + for method in [Method::POST, Method::PUT, Method::PATCH] { + let path = if method == Method::POST { + "/schemas/WebhookSubscription/entities" + } else { + &entity_path + }; + for url in ["https://127.0.0.1/hook", "http://8.8.8.8/hook"] { + let (status, body) = json_request( + &app, + method.clone(), + path, + Some(serde_json::json!({"fields":{"url":url}})), + ) + .await; + assert_eq!( + status, + StatusCode::UNPROCESSABLE_ENTITY, + "{method} {url}: {body}" + ); + assert_eq!(body["error"], "validation_failed"); + } + } + let (status, body) = json_request(&app, Method::GET, &entity_path, None).await; + assert_eq!(status, StatusCode::OK); + assert_eq!(body["fields"]["url"], "https://8.8.8.8/hook"); +} diff --git a/crates/schema-forge-acton/tests/graphql_tenants.rs b/crates/schema-forge-acton/tests/graphql_tenants.rs index 43e985c..0dab455 100644 --- a/crates/schema-forge-acton/tests/graphql_tenants.rs +++ b/crates/schema-forge-acton/tests/graphql_tenants.rs @@ -31,6 +31,10 @@ async fn app() -> (Router, String, String) { } async fn app_with_relations(relations: bool) -> (Router, String, String) { + app_with_options(relations, true).await +} + +async fn app_with_options(relations: bool, tenancy: bool) -> (Router, String, String) { let backend = SurrealBackend::connect_with_auth("mem://", "graphql", "tenants", None, None) .await .unwrap(); @@ -40,6 +44,7 @@ async fn app_with_relations(relations: bool) -> (Router, String, String) { @access(read: ["member"], write: ["member"], delete: ["member"]) schema Org { name: text required } @access(read: ["member"], write: ["member"]) + @tenant(parent: "Org") schema Catalog { name: text required secret: text hidden_value: text } "#, ) @@ -49,12 +54,20 @@ async fn app_with_relations(relations: bool) -> (Router, String, String) { schema_forge_dsl::parse( r#" @access(read: ["member"], write: ["member"]) + @tenant(parent: "Org") schema Link { one: -> Catalog many: -> Catalog[] } "#, ) .unwrap(), ); } + if !tenancy { + for schema in &mut schemas { + schema.annotations.retain(|annotation| { + !matches!(annotation, schema_forge_core::types::Annotation::Tenant(_)) + }); + } + } for schema in &schemas { backend .apply_migration(&schema.name, &DiffEngine::create_new(schema).steps) @@ -148,7 +161,7 @@ async fn query(app: &Router, query: &str, tenant: &str) -> Value { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn graphql_scopes_legacy_roots_and_keeps_shared_catalog_usable() { +async fn graphql_scopes_legacy_roots_and_application_catalog() { let (app, own, foreign) = app().await; let get = query( &app, @@ -186,9 +199,11 @@ async fn graphql_scopes_legacy_roots_and_keeps_shared_catalog_usable() { .await; assert!(created.get("errors").is_none(), "{created}"); let catalog = query(&app, "{ catalogs { items { name } } }", &foreign).await; + assert_eq!(catalog["data"]["catalogs"]["items"], json!([]), "{catalog}"); + let own_catalog = query(&app, "{ catalogs { items { name } } }", &own).await; assert_eq!( - catalog["data"]["catalogs"]["items"][0]["name"], "shared", - "{catalog}" + own_catalog["data"]["catalogs"]["items"][0]["name"], + "shared" ); // These routes intentionally have no tenant middleware. A permissive // operator policy must never replace concrete Cedar tenant authorization. @@ -246,7 +261,7 @@ async fn administer_catalog(app: &Router, method: Method, body: Value) { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn graphql_uses_live_field_security_and_refuses_removed_schemas() { - let (app, tenant, _) = app().await; + let (app, tenant, _) = app_with_options(false, false).await; let created = query(&app, "mutation { createCatalog(input: {name: \"visible\", secret: \"restricted\", hidden_value: \"private\"}) { id } }", &tenant).await; diff --git a/crates/schema-forge-acton/tests/graphql_writes.rs b/crates/schema-forge-acton/tests/graphql_writes.rs index 5da4791..62ca47d 100644 --- a/crates/schema-forge-acton/tests/graphql_writes.rs +++ b/crates/schema-forge-acton/tests/graphql_writes.rs @@ -27,6 +27,7 @@ async fn app() -> Router { let schemas = schema_forge_dsl::parse( r#" @access(read: ["staff", "manager"], write: ["staff", "manager"]) + @tenant(parent: "Org") schema Line { label: text required number: text @field_access(read: ["staff", "manager"], write: ["manager"]) @@ -99,7 +100,7 @@ async fn app() -> Router { } async fn query(app: &Router, query: &str, role: &str) -> Value { - query_with_tenant(app, query, role, None).await + query_with_tenant(app, query, role, Some("org_graphql")).await } async fn query_with_tenant(app: &Router, query: &str, role: &str, tenant: Option<&str>) -> Value { diff --git a/schemas/demo.schema b/schemas/demo.schema index 4f45bd9..1679e70 100644 --- a/schemas/demo.schema +++ b/schemas/demo.schema @@ -95,6 +95,7 @@ schema Department { @display("full_name") @dashboard(widgets: ["count"]) @access(read: ["member", "hr", "manager", "admin"], write: ["hr", "admin"], delete: ["admin"]) +@tenant(parent: "Organization") schema Employee { full_name: text(max: 255) required indexed email: text(max: 512) required indexed @widget("email") @@ -135,6 +136,7 @@ schema Employee { @display("full_name") @dashboard(widgets: ["count"]) @access(read: ["sales", "marketing", "manager", "admin"], write: ["sales", "admin"], delete: ["admin"]) +@tenant(parent: "Organization") schema Contact { full_name: text(max: 255) required indexed email: text(max: 512) indexed @widget("email") @@ -167,6 +169,7 @@ schema Contact { @display("name") @dashboard(widgets: ["count"]) @access(read: ["sales", "marketing", "finance", "manager", "admin"], write: ["sales", "admin"], delete: ["admin"]) +@tenant(parent: "Organization") schema Company { name: text(max: 255) required indexed domain: text(max: 255) indexed @widget("url") @@ -205,6 +208,7 @@ schema Company { // Parameterized @webhook — POSTs to an external endpoint on the listed lifecycle // events (only created/updated/deleted are valid), signed with the shared secret. @webhook(events: ["created", "updated", "deleted"], url: "https://example.com/hooks/deals", secret: "whsec_demo_replace_me") +@tenant(parent: "Organization") schema Deal { name: text(max: 255) required indexed value: float(precision: 2) required @field_access(read: ["sales", "finance", "admin"], write: ["sales"]) @format("currency") @@ -233,6 +237,7 @@ schema Deal { // Bare @webhook — registers the schema for outbound notifications using the // instance-wide default endpoint/secret (no inline parameters). @webhook +@tenant(parent: "Organization") schema Activity { subject: text(max: 500) required activity_type: enum("call", "email", "meeting", "note", "task", "demo", "follow_up") required @widget("status_badge") @@ -259,6 +264,7 @@ schema Activity { @display("name") @dashboard(widgets: ["count"], layout: "kanban", group_by: "status") @access(read: ["member", "manager", "admin"], write: ["manager", "admin"], delete: ["admin"]) +@tenant(parent: "Organization") schema Project { name: text(max: 255) required indexed code: text(max: 20) required indexed @widget("code") @@ -287,6 +293,7 @@ schema Project { @display("title") @dashboard(widgets: ["count"], layout: "kanban", group_by: "status") @access(read: ["member", "manager", "admin"], write: ["member", "manager", "admin"], delete: ["manager", "admin"]) +@tenant(parent: "Organization") schema Task { title: text(max: 500) required indexed description: richtext @widget("markdown") @@ -311,6 +318,7 @@ schema Task { @display("name") @access(read: ["member", "manager", "admin"], write: ["manager", "admin"], delete: ["admin"]) +@tenant(parent: "Organization") schema Milestone { // @list(...) hints control list-view rendering: `primary` is the headline // cell, `column` forces inclusion, `hidden` suppresses an otherwise-shown field. @@ -334,6 +342,7 @@ schema Milestone { @display("title") @dashboard(widgets: ["count"]) @access(read: ["member", "manager", "admin"], write: ["member", "manager", "admin"], delete: ["manager", "admin"]) +@tenant(parent: "Organization") schema Document { title: text(max: 500) required indexed content: richtext required @widget("markdown") @@ -356,6 +365,7 @@ schema Document { /* Comments are threaded discussions on any entity. */ @display("preview") +@tenant(parent: "Organization") schema Comment { preview: text(max: 255) required body: richtext required @@ -419,8 +429,9 @@ schema Theme { // cross_tenant_read lets these roles read Tag rows owned by OTHER tenants — // appropriate for shared reference data. read/write/delete stay tenant-scoped. @access(read: ["member", "manager", "admin"], write: ["manager", "admin"], delete: ["admin"], cross_tenant_read: ["member", "manager", "admin"]) +@tenant(parent: "Organization") schema Tag { - // Tag is not tenant-scoped, so `unique` enforces a single global namespace. + // Tag names are unique within each organization. name: text(max: 100) required indexed unique color: text(max: 7) @widget("color") category: enum("general", "priority", "status", "department", "skill", "industry") default("general") @widget("status_badge") @enum_colors(general: "neutral", priority: "rose", status: "teal", department: "violet", skill: "green", industry: "gray") diff --git a/scripts/seed-demo-data.sh b/scripts/seed-demo-data.sh index a57786c..f19e528 100755 --- a/scripts/seed-demo-data.sh +++ b/scripts/seed-demo-data.sh @@ -76,6 +76,12 @@ create_entity() { local schema="$1" local body="$2" + # The platform administrator must explicitly stamp ownership on child rows. + case "$schema" in + Organization|Theme|Workflow) ;; + *) body=$(jq --arg tenant "$ORG1_ID" '.fields._tenant = (.fields._tenant // .fields.parent_org // $tenant)' <<< "$body") ;; + esac + local response local http_code local tmp @@ -131,7 +137,38 @@ echo "" # ========================================================================= -# Layer 1: Tags (no dependencies) +# Layer 1: Organizations (no dependencies) +# ========================================================================= +echo -e "${CYAN}--- Organizations ---${NC}" + +ORG1_ID=$(create_entity "Organization" "$(jq -n '{fields: { + name: "Acme Corporation", + slug: "acme-corp", + billing_email: "billing@acme-corp.io", + plan: "business", + max_seats: 50, + logo_url: "https://example.com/acme-logo.png", + settings: {theme: "dark", timezone: "America/New_York"}, + founded: "2018-03-15T00:00:00Z", + active: true, + owner_id: "system" +}}')") + +ORG2_ID=$(create_entity "Organization" "$(jq -n '{fields: { + name: "Globex Industries", + slug: "globex", + billing_email: "accounts@globex.io", + plan: "enterprise", + max_seats: 200, + logo_url: "https://example.com/globex-logo.png", + settings: {theme: "light", timezone: "America/Los_Angeles", sso_enabled: true}, + founded: "2015-07-01T00:00:00Z", + active: true, + owner_id: "system" +}}')") + +# ========================================================================= +# Layer 2: Tags (owned by Acme) # ========================================================================= echo -e "${CYAN}--- Tags ---${NC}" @@ -175,37 +212,6 @@ TAG5_ID=$(create_entity "Tag" "$(jq -n '{fields: { active: true }}')") -# ========================================================================= -# Layer 2: Organizations (no dependencies) -# ========================================================================= -echo -e "${CYAN}--- Organizations ---${NC}" - -ORG1_ID=$(create_entity "Organization" "$(jq -n '{fields: { - name: "Acme Corporation", - slug: "acme-corp", - billing_email: "billing@acme-corp.io", - plan: "business", - max_seats: 50, - logo_url: "https://example.com/acme-logo.png", - settings: {theme: "dark", timezone: "America/New_York"}, - founded: "2018-03-15T00:00:00Z", - active: true, - owner_id: "system" -}}')") - -ORG2_ID=$(create_entity "Organization" "$(jq -n '{fields: { - name: "Globex Industries", - slug: "globex", - billing_email: "accounts@globex.io", - plan: "enterprise", - max_seats: 200, - logo_url: "https://example.com/globex-logo.png", - settings: {theme: "light", timezone: "America/Los_Angeles", sso_enabled: true}, - founded: "2015-07-01T00:00:00Z", - active: true, - owner_id: "system" -}}')") - # ========================================================================= # Layer 3: Departments (→ Organization) # ========================================================================= diff --git a/skills/schemaforge/patterns.md b/skills/schemaforge/patterns.md index 0b84598..2fe7cc6 100644 --- a/skills/schemaforge/patterns.md +++ b/skills/schemaforge/patterns.md @@ -14,7 +14,7 @@ Use `@tenant` to scope data to organizational boundaries. ### Rules 1. Exactly one schema should be `@tenant(root)` — it anchors the hierarchy -2. Child schemas reference the parent with `@tenant(parent: "ParentName")` +2. Every other application schema must declare `@tenant(parent: "ParentName")`; only `@system` schemas are exempt 3. All data in child schemas is automatically scoped to the root tenant 4. The root schema should have `owner_id: text required @owner` for ownership From 919b7598e8de9f28ab78bbf4078ac7f29db2e696 Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 12:48:38 -0600 Subject: [PATCH 12/38] fix(cli): match framework listener configuration providers --- crates/schema-forge-cli/src/config.rs | 174 ++++++++++++++++++++------ 1 file changed, 138 insertions(+), 36 deletions(-) diff --git a/crates/schema-forge-cli/src/config.rs b/crates/schema-forge-cli/src/config.rs index 43b9d69..d61271e 100644 --- a/crates/schema-forge-cli/src/config.rs +++ b/crates/schema-forge-cli/src/config.rs @@ -19,6 +19,10 @@ use std::path::{Path, PathBuf}; use std::time::Duration; use acton_service::config::Config; +use figment::{ + providers::{Env, Format, Toml}, + Figment, +}; use schema_forge_acton::config::ClientConfig; use schema_forge_acton::SchemaForgeConfig; use schema_forge_signing::{SigningConfig, SigningMode, VerifyPolicy}; @@ -144,52 +148,65 @@ pub fn load_svc_config(global: &GlobalOpts) -> Result, }; // acton-service defaults to all interfaces; SchemaForge defaults to loopback. // Preserve an explicitly configured unspecified address, including 0.0.0.0. - if !service_bind_is_configured(global.config.as_deref())? { + if !service_bind_is_configured(global.config.as_deref()) { svc.service.bind = std::net::Ipv4Addr::LOCALHOST.into(); } apply_cli_overrides(&mut svc, global)?; Ok(svc) } -/// Inspect the same file layers as acton-service without changing their precedence. -fn service_bind_is_configured(explicit: Option<&Path>) -> Result { - if std::env::var_os("ACTON_SERVICE_BIND").is_some() { - return Ok(true); +/// Match acton-service's user config discovery, without recommended_path's +/// advisory relative fallback (which the framework does not actually load). +fn user_service_config_path() -> Option { + #[cfg(unix)] + let root = std::env::var_os("XDG_CONFIG_HOME") + .map(PathBuf::from) + .filter(|path| path.is_absolute()) + .or_else(|| std::env::var_os("HOME").map(|home| PathBuf::from(home).join(".config"))); + #[cfg(windows)] + let root = std::env::var_os("APPDATA").map(PathBuf::from).or_else(|| { + std::env::var_os("USERPROFILE") + .map(|home| PathBuf::from(home).join("AppData").join("Roaming")) + }); + #[cfg(not(any(unix, windows)))] + let root: Option = None; + root.map(|root| { + root.join("acton-service") + .join("schemaforge") + .join("config.toml") + }) +} + +fn service_config_paths(explicit: Option<&Path>) -> Vec { + if let Some(path) = explicit { + return vec![path.to_path_buf()]; } - let mut paths = if let Some(path) = explicit { - vec![path.to_path_buf()] - } else { - vec![ - PathBuf::from("config.toml"), - Config::::recommended_path("schemaforge"), - ] - }; - if explicit.is_none() { - #[cfg(unix)] - paths.push(PathBuf::from("/etc/acton-service/schemaforge/config.toml")); - #[cfg(windows)] - if let Some(root) = std::env::var_os("PROGRAMDATA") { - paths.push(PathBuf::from(root).join("acton-service/schemaforge/config.toml")); - } + let mut paths = vec![PathBuf::from("config.toml")]; + if let Some(path) = user_service_config_path().filter(|path| path.is_file()) { + paths.push(path); } - for path in paths { - let source = match std::fs::read_to_string(&path) { - Ok(source) => source, - Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue, - Err(source) => return Err(CliError::Io { path, source }), - }; - let config: toml::Value = toml::from_str(&source).map_err(|error| CliError::Config { - message: format!("failed to load {}: {error}", path.display()), - })?; - if config - .get("service") - .and_then(|service| service.get("bind")) - .is_some() - { - return Ok(true); + #[cfg(unix)] + paths.push(PathBuf::from("/etc/acton-service/schemaforge/config.toml")); + #[cfg(windows)] + if let Some(root) = std::env::var_os("PROGRAMDATA") { + paths.push(PathBuf::from(root).join("acton-service/schemaforge/config.toml")); + } + paths +} + +/// Inspect the framework's actual providers, without its default values. +/// Env handles case-insensitive names and structured ACTON_SERVICE dictionaries. +/// The framework has already validated and loaded the complete configuration. +fn service_bind_is_configured(explicit: Option<&Path>) -> bool { + let mut configured = Figment::new(); + for path in service_config_paths(explicit).iter().rev() { + if path.exists() { + configured = configured.merge(Toml::file(path)); } } - Ok(false) + configured + .merge(Env::prefixed("ACTON_").split("_")) + .contains("service.bind") } fn load_svc_config_from_path(path: &Path) -> Result, CliError> { @@ -623,6 +640,91 @@ mod tests { } } + #[test] + fn listener_environment_provider_respects_case_and_dictionary_overrides() { + const CHILD: &str = "SCHEMAFORGE_LISTENER_TEST_CHILD"; + if std::env::var_os(CHILD).is_some() { + let global = GlobalOpts { + config: Some(PathBuf::from("config.toml")), + ..empty_global() + }; + let config = load_svc_config(&global).unwrap(); + assert_eq!(config.service.bind, std::net::Ipv4Addr::UNSPECIFIED); + assert_eq!(config.service.port, 3899); + return; + } + let directory = tempfile::tempdir().unwrap(); + std::fs::write( + directory.path().join("config.toml"), + "[service]\nport = 3899\n", + ) + .unwrap(); + for (key, value) in [ + ("ACTON_SERVICE_BIND", "0.0.0.0"), + ("ACTON_SERVICE_bind", "0.0.0.0"), + ("ACTON_SERVICE", "{bind=\"0.0.0.0\"}"), + ] { + let mut child = std::process::Command::new(std::env::current_exe().unwrap()); + child.args(["--exact", "config::tests::listener_environment_provider_respects_case_and_dictionary_overrides", "--nocapture"]) + .current_dir(directory.path()).env(CHILD, "1"); + for (name, _) in std::env::vars_os() { + if name + .to_string_lossy() + .to_ascii_uppercase() + .starts_with("ACTON_") + { + child.env_remove(name); + } + } + let output = child.env(key, value).output().unwrap(); + assert!( + output.status.success(), + "{key}: {} {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + } + } + + #[test] + fn missing_home_does_not_discover_the_recommended_relative_fallback() { + const CHILD: &str = "SCHEMAFORGE_CONFIG_PATH_TEST_CHILD"; + if std::env::var_os(CHILD).is_some() { + assert!(user_service_config_path().is_none()); + assert!(!service_config_paths(None) + .contains(&PathBuf::from("acton-service/schemaforge/config.toml"))); + return; + } + let directory = tempfile::tempdir().unwrap(); + let fallback = directory.path().join("acton-service/schemaforge"); + std::fs::create_dir_all(&fallback).unwrap(); + std::fs::write( + fallback.join("config.toml"), + "[service]\nbind = \"0.0.0.0\"\n", + ) + .unwrap(); + let output = std::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "config::tests::missing_home_does_not_discover_the_recommended_relative_fallback", + "--nocapture", + ]) + .current_dir(directory.path()) + .env(CHILD, "1") + .env_remove("HOME") + .env_remove("XDG_CONFIG_HOME") + .env_remove("APPDATA") + .env_remove("USERPROFILE") + .output() + .unwrap(); + assert!( + output.status.success(), + "{} {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + } + #[test] fn listener_file_configuration_and_loopback_fallback() { let dir = tempfile::tempdir().unwrap(); From a9ae27332b64350fe43e1e068f6d1332981b78c9 Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 12:49:03 -0600 Subject: [PATCH 13/38] fix: align configuration providers and write-order documentation --- Cargo.lock | 1 + crates/schema-forge-cli/Cargo.toml | 1 + docs/rule-ordering-reference.md | 8 ++++++-- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ec8d009..675d97f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7695,6 +7695,7 @@ dependencies = [ "clap_complete", "console", "dialoguer", + "figment", "glob", "heck 0.5.0", "httpdate", diff --git a/crates/schema-forge-cli/Cargo.toml b/crates/schema-forge-cli/Cargo.toml index f3522ec..eb974cf 100644 --- a/crates/schema-forge-cli/Cargo.toml +++ b/crates/schema-forge-cli/Cargo.toml @@ -40,6 +40,7 @@ sha2 = "0.11.0" tokio-util = { version = "0.7.18", features = ["io"] } schema-forge-mssql = { version = "0.6.0", path = "../schema-forge-mssql", optional = true } httpdate = "1.0.3" +figment = { version = "0.10.19", features = ["env", "toml"] } [features] default = ["surrealdb"] diff --git a/docs/rule-ordering-reference.md b/docs/rule-ordering-reference.md index 0fd2e77..40ed9c6 100644 --- a/docs/rule-ordering-reference.md +++ b/docs/rule-ordering-reference.md @@ -22,7 +22,9 @@ schema Cedar write check -> reject hidden keys -> JSON conversion -> tenant/owner stamping -> field write filtering -> tenant/owner re-stamping -> audit columns -> @default -> @compute -> required-field validation -> Cedar Create on computed entity -> related-row prefetch -> @require --> before_validate -> before_change -> required-field re-check -> PERSIST +-> before_validate -> before_change +-> relation visibility and webhook destination checks -> required-field re-check +-> PERSIST -> after hooks and webhook dispatch PUT / PATCH @@ -33,7 +35,9 @@ Cedar and record checks on stored row -> reject hidden keys -> JSON conversion -> field write filtering (restore denied fields from stored row) -> audit columns -> @compute -> required-field validation -> related-row prefetch -> @require --> before_validate -> before_change -> required-field re-check -> PERSIST +-> before_validate -> before_change +-> relation visibility and webhook destination checks -> required-field re-check +-> PERSIST -> after hooks and webhook dispatch ``` From 9bd9aba91ee40e5911ebe608d8cb4cd442d3ff8a Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 13:20:28 -0600 Subject: [PATCH 14/38] test: align connection diagnostics and order extractor definitions --- crates/schema-forge-acton/src/error.rs | 46 ++++++++++---------- crates/schema-forge-surrealdb/src/backend.rs | 18 +++++--- 2 files changed, 34 insertions(+), 30 deletions(-) diff --git a/crates/schema-forge-acton/src/error.rs b/crates/schema-forge-acton/src/error.rs index a2c8122..89e0b1c 100644 --- a/crates/schema-forge-acton/src/error.rs +++ b/crates/schema-forge-acton/src/error.rs @@ -311,6 +311,29 @@ impl From for ForgeError { } } +/// JSON request extractor using the SchemaForge validation error envelope. +pub struct JsonBody(pub T); + +impl axum::extract::FromRequest for JsonBody +where + S: Send + Sync, + T: serde::de::DeserializeOwned, +{ + type Rejection = ForgeError; + + async fn from_request( + request: axum::extract::Request, + state: &S, + ) -> Result { + as axum::extract::FromRequest>::from_request(request, state) + .await + .map(|axum::Json(value)| Self(value)) + .map_err(|error| ForgeError::ValidationFailed { + details: vec![error.body_text()], + }) + } +} + #[cfg(test)] mod tests { use super::*; @@ -738,26 +761,3 @@ mod tests { } } } - -/// JSON request extractor using the SchemaForge validation error envelope. -pub struct JsonBody(pub T); - -impl axum::extract::FromRequest for JsonBody -where - S: Send + Sync, - T: serde::de::DeserializeOwned, -{ - type Rejection = ForgeError; - - async fn from_request( - request: axum::extract::Request, - state: &S, - ) -> Result { - as axum::extract::FromRequest>::from_request(request, state) - .await - .map(|axum::Json(value)| Self(value)) - .map_err(|error| ForgeError::ValidationFailed { - details: vec![error.body_text()], - }) - } -} diff --git a/crates/schema-forge-surrealdb/src/backend.rs b/crates/schema-forge-surrealdb/src/backend.rs index 6197bff..34a1280 100644 --- a/crates/schema-forge-surrealdb/src/backend.rs +++ b/crates/schema-forge-surrealdb/src/backend.rs @@ -962,14 +962,18 @@ mod tests { } #[tokio::test] - async fn connect_invalid_url() { - let result = SurrealBackend::connect("badscheme://x", "a", "b").await; - assert!(result.is_err(), "connect with invalid scheme should fail"); + async fn connect_invalid_url_does_not_expose_credentials() { + let url = + "badscheme://private-user:private-password@example.invalid/db?token=private-token"; + let result = SurrealBackend::connect(url, "a", "b").await; if let Err(BackendError::ConnectionError { message }) = result { - assert!( - message.contains("badscheme"), - "error should mention the bad scheme" - ); + assert!(message.contains("failed to connect to SurrealDB")); + for secret in [url, "private-user", "private-password", "private-token"] { + assert!( + !message.contains(secret), + "connection error exposed credentials" + ); + } } else { panic!("expected ConnectionError"); } From 7f6bde5bf79d1967212e7c20114200d283a64cfc Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 13:21:11 -0600 Subject: [PATCH 15/38] test: preserve authorization assertions with redacted errors --- .../tests/file_authorization.rs | 56 +++++++++++++++++-- .../tests/graphql_writes.rs | 8 ++- 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/crates/schema-forge-acton/tests/file_authorization.rs b/crates/schema-forge-acton/tests/file_authorization.rs index 745cf83..5f381e3 100644 --- a/crates/schema-forge-acton/tests/file_authorization.rs +++ b/crates/schema-forge-acton/tests/file_authorization.rs @@ -28,11 +28,15 @@ use schema_forge_core::types::{ use schema_forge_surrealdb::SurrealBackend; use std::{ collections::{BTreeMap, HashMap}, - sync::Arc, + sync::{ + atomic::{AtomicBool, Ordering}, + Arc, + }, time::Duration, }; use tokio::sync::oneshot; use tower::ServiceExt; +use tracing::instrument::WithSubscriber; fn claims(tenant: Option<&str>, roles: &[&str]) -> Claims { let mut custom = HashMap::new(); @@ -221,6 +225,36 @@ async fn fixture_with_options( ) } +/// Observe the server-side storage diagnostic without exposing it to the client. +#[derive(Clone, Default)] +struct StorageLookupObserver(Arc); + +impl tracing::field::Visit for StorageLookupObserver { + fn record_debug(&mut self, field: &tracing::field::Field, value: &dyn std::fmt::Debug) { + if field.name() == "error" + && format!("{value:?}").contains("storage backend 'documents' not configured") + { + self.0.store(true, Ordering::Relaxed); + } + } +} + +impl tracing::Subscriber for StorageLookupObserver { + fn enabled(&self, _: &tracing::Metadata<'_>) -> bool { + true + } + fn new_span(&self, _: &tracing::span::Attributes<'_>) -> tracing::span::Id { + tracing::span::Id::from_u64(1) + } + fn record(&self, _: &tracing::span::Id, _: &tracing::span::Record<'_>) {} + fn record_follows_from(&self, _: &tracing::span::Id, _: &tracing::span::Id) {} + fn event(&self, event: &tracing::Event<'_>) { + event.record(&mut self.clone()); + } + fn enter(&self, _: &tracing::span::Id) {} + fn exit(&self, _: &tracing::span::Id) {} +} + async fn status(app: &Router, path: &str, operation: &str) -> StatusCode { status_with_tenant(app, path, operation, None).await } @@ -253,18 +287,32 @@ async fn status_with_tenant( if let Some(tenant) = tenant { request = request.header("x-active-tenant", tenant); } + let observer = StorageLookupObserver::default(); let response = app .clone() .oneshot(request.body(Body::from(body)).unwrap()) + .with_subscriber(observer.clone()) .await .unwrap(); let status = response.status(); if status == StatusCode::INTERNAL_SERVER_ERROR { let bytes = response.into_body().collect().await.unwrap().to_bytes(); - let body = String::from_utf8(bytes.to_vec()).unwrap(); + let body: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + assert_eq!( + body, + serde_json::json!({ + "error": "internal_error", + "message": "The server could not complete the operation", + }) + ); + assert!( + observer.0.load(Ordering::Relaxed), + "authorized request must reach storage lookup" + ); + } else if status == StatusCode::FORBIDDEN { assert!( - body.contains("storage backend 'documents' not configured"), - "unexpected internal error: {body}" + !observer.0.load(Ordering::Relaxed), + "denied request must stop before storage lookup" ); } status diff --git a/crates/schema-forge-acton/tests/graphql_writes.rs b/crates/schema-forge-acton/tests/graphql_writes.rs index 62ca47d..c1fc1ad 100644 --- a/crates/schema-forge-acton/tests/graphql_writes.rs +++ b/crates/schema-forge-acton/tests/graphql_writes.rs @@ -100,7 +100,13 @@ async fn app() -> Router { } async fn query(app: &Router, query: &str, role: &str) -> Value { - query_with_tenant(app, query, role, Some("org_graphql")).await + query_with_tenant( + app, + query, + role, + (role != "platform_admin").then_some("org_graphql"), + ) + .await } async fn query_with_tenant(app: &Router, query: &str, role: &str, tenant: Option<&str>) -> Value { From ebf5d882f18be8c23e73067060c0230fd30aa9ea Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 13:31:02 -0600 Subject: [PATCH 16/38] ci: validate generated site types and lint before browser tests --- crates/schema-forge-cli/tests/site_e2e/run.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/schema-forge-cli/tests/site_e2e/run.sh b/crates/schema-forge-cli/tests/site_e2e/run.sh index 615fbf1..8d5809f 100755 --- a/crates/schema-forge-cli/tests/site_e2e/run.sh +++ b/crates/schema-forge-cli/tests/site_e2e/run.sh @@ -97,6 +97,11 @@ if [[ ! -d node_modules ]]; then pnpm install --frozen-lockfile 2>&1 | tail -5 || pnpm install 2>&1 | tail -5 fi +# Vite's development server transpiles without type checking. Validate the +# generated application before browser tests so broken form types cannot pass. +pnpm build +pnpm lint + VITE_LOG="$TMP_ROOT/vite.log" VITE_FORGE_UPSTREAM="http://127.0.0.1:$BACKEND_PORT" \ pnpm exec vite --host 127.0.0.1 --port "$VITE_PORT" >"$VITE_LOG" 2>&1 & From 0aa7984177f0c5537dacca2974e9f0225f38155e Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 13:37:39 -0600 Subject: [PATCH 17/38] build: enable JSON serialization in site templates --- Cargo.lock | 1 + crates/schema-forge-cli/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 675d97f..e0642af 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5163,6 +5163,7 @@ checksum = "805bfd7352166bae857ee569628b52bcd85a1cecf7810861ebceb1686b72b75d" dependencies = [ "memo-map", "serde", + "serde_json", ] [[package]] diff --git a/crates/schema-forge-cli/Cargo.toml b/crates/schema-forge-cli/Cargo.toml index eb974cf..2dda8f7 100644 --- a/crates/schema-forge-cli/Cargo.toml +++ b/crates/schema-forge-cli/Cargo.toml @@ -29,7 +29,7 @@ glob = "0.3" axum = { version = "0.8" } acton-service = { version = "0.43.1", default-features = false, features = ["http", "observability", "otel-metrics", "journald", "governor", "resilience", "audit", "openapi", "auth", "crypto-aws-lc-rs", "windows-auth"] } heck = "0.5.0" -minijinja = "2.19.0" +minijinja = { version = "2.19.0", features = ["json"] } tracing = "0.1.44" schema-forge-signing = { version = "0.1.0", path = "../schema-forge-signing" } sigstore-trust-root = { version = "0.7.0", default-features = false, features = ["tuf"] } From 91f384f92d9266c330f4b1fb0285410e28dd5a5b Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 13:37:42 -0600 Subject: [PATCH 18/38] fix: scope generated site requests and clarify error feedback --- .../schema-forge-cli/src/commands/site/mod.rs | 5 + .../src/commands/site/vendor.rs | 8 +- .../site/src/app/pages/detail.tsx.jinja | 1 + .../site/src/app/pages/edit.tsx.jinja | 4 +- .../site/src/app/pages/list.tsx.jinja | 1 + .../site/src/generated/api-client.ts.jinja | 75 ++++++--- .../templates/site/src/lib/auth.ts.jinja | 19 ++- .../site/src/lib/error-toast.ts.jinja | 23 +++ .../templates/site/src/main.tsx.jinja | 10 +- .../playwright/tests/client-errors.spec.ts | 158 ++++++++++++++++++ .../schema-forge-cli/tests/site_generate.rs | 23 +++ docs/generated-site-errors.md | 11 ++ 12 files changed, 298 insertions(+), 40 deletions(-) create mode 100644 crates/schema-forge-cli/templates/site/src/lib/error-toast.ts.jinja create mode 100644 crates/schema-forge-cli/tests/site_e2e/playwright/tests/client-errors.spec.ts create mode 100644 docs/generated-site-errors.md diff --git a/crates/schema-forge-cli/src/commands/site/mod.rs b/crates/schema-forge-cli/src/commands/site/mod.rs index 0c905bc..022be6c 100644 --- a/crates/schema-forge-cli/src/commands/site/mod.rs +++ b/crates/schema-forge-cli/src/commands/site/mod.rs @@ -291,6 +291,11 @@ fn build_plan(ctx: &SiteContext, renderer: &SiteRenderer) -> Result`. `AttachmentDownload` renders the read-only // view used on detail pages, honoring the field's `access` mode. import { useState } from "react" -import { tokenStore } from "@/lib/auth" +import { authenticatedHeaders } from "@/lib/auth" const API_BASE = (import.meta.env.VITE_API_BASE as string | undefined) ?? "" const FORGE_API_PREFIX = "/api/v1/forge" @@ -1772,8 +1773,7 @@ function entityUrlOrNull(schema: string, entityId: string | undefined): string | } function authHeaders(): Record { - const token = tokenStore.get() - return token ? { Authorization: `Bearer ${token}` } : {} + return Object.fromEntries(authenticatedHeaders()) } async function safeBody(res: Response): Promise { diff --git a/crates/schema-forge-cli/templates/site/src/app/pages/detail.tsx.jinja b/crates/schema-forge-cli/templates/site/src/app/pages/detail.tsx.jinja index 07e9b50..49809f5 100644 --- a/crates/schema-forge-cli/templates/site/src/app/pages/detail.tsx.jinja +++ b/crates/schema-forge-cli/templates/site/src/app/pages/detail.tsx.jinja @@ -21,6 +21,7 @@ export function {{ entity.pascal }}Detail() { const { id } = useParams<{ id: string }>() useDocumentTitle("{{ entity.title }} record") const query = useQuery({ + meta: { suppressGlobalError: true }, queryKey: ["{{ entity.snake }}", "detail", id], queryFn: () => get{{ entity.pascal }}(id!), enabled: Boolean(id), diff --git a/crates/schema-forge-cli/templates/site/src/app/pages/edit.tsx.jinja b/crates/schema-forge-cli/templates/site/src/app/pages/edit.tsx.jinja index c4fb899..8017c56 100644 --- a/crates/schema-forge-cli/templates/site/src/app/pages/edit.tsx.jinja +++ b/crates/schema-forge-cli/templates/site/src/app/pages/edit.tsx.jinja @@ -17,6 +17,7 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query" import { toast } from "sonner" import { ApiError, + formatApiError, create{{ entity.pascal }}, get{{ entity.pascal }}, update{{ entity.pascal }}, @@ -59,6 +60,7 @@ export function {{ entity.pascal }}Edit() { }) const mutation = useMutation({ + meta: { suppressGlobalError: true }, mutationFn: async (values: {{ entity.pascal }}FormValues) => { const payload = normalize{{ entity.pascal }}Payload(values, form) if (isEdit && id) return update{{ entity.pascal }}(id, payload as Partial<{{ entity.pascal }}>) @@ -80,7 +82,7 @@ export function {{ entity.pascal }}Edit() { return } } - toast.error(err instanceof Error ? err.message : "Save failed") + toast.error(formatApiError(err)) }, }) diff --git a/crates/schema-forge-cli/templates/site/src/app/pages/list.tsx.jinja b/crates/schema-forge-cli/templates/site/src/app/pages/list.tsx.jinja index e9e3a12..6fbffff 100644 --- a/crates/schema-forge-cli/templates/site/src/app/pages/list.tsx.jinja +++ b/crates/schema-forge-cli/templates/site/src/app/pages/list.tsx.jinja @@ -63,6 +63,7 @@ export function {{ entity.pascal }}List() { : undefined const query = useQuery({ + meta: { suppressGlobalError: true }, queryKey: [ "{{ entity.snake }}", "list", diff --git a/crates/schema-forge-cli/templates/site/src/generated/api-client.ts.jinja b/crates/schema-forge-cli/templates/site/src/generated/api-client.ts.jinja index ba06a01..8e8c6ad 100644 --- a/crates/schema-forge-cli/templates/site/src/generated/api-client.ts.jinja +++ b/crates/schema-forge-cli/templates/site/src/generated/api-client.ts.jinja @@ -1,7 +1,7 @@ // Auto-generated typed REST client. // One set of CRUD functions per schema. Regenerated on every // `schema-forge site generate` run. -import { refreshToken, tokenStore } from "@/lib/auth" +import { authenticatedHeaders, refreshToken, tokenStore } from "@/lib/auth" {%- for entity in entities %} import type { {{ entity.pascal }} } from "./entity-types" {%- endfor %} @@ -49,15 +49,15 @@ export type EntityPermissions = { /** Structured error thrown by the request helper on any non-OK status. * Carries the parsed JSON body when the server returned one so callers * can branch on the stable `error` discriminator (e.g. `unique_violation`, - * `validation_failed`, `forbidden`). Falls back to the raw text for - * endpoints that don't return JSON. */ + * `validation_failed`, `forbidden`). Raw text remains available for diagnostics; + * the displayed message uses readable envelope text or a status fallback. */ export class ApiError extends Error { readonly status: number readonly body: unknown readonly raw: string constructor(status: number, body: unknown, raw: string) { - super(`API ${status}: ${raw || (body ? JSON.stringify(body) : "")}`) + super(responseErrorMessage(status, body)) this.status = status this.body = body this.raw = raw @@ -84,6 +84,43 @@ export class ApiError extends Error { } } +function readableMessage(value: unknown): string | undefined { + if (typeof value !== "string") return undefined + const message = value.trim() + if (!message || ["{", "[", "<"].includes(message.charAt(0)) || /^[A-Z][A-Z0-9_]*$/.test(message) || /^[a-z][a-z0-9_]*$/.test(message)) return undefined + return message +} + +function responseErrorMessage(status: number, body: unknown): string { + if (body && typeof body === "object") { + const envelope = body as { message?: unknown; error?: unknown } + const message = readableMessage(envelope.message) ?? readableMessage(envelope.error) + if (message) return message + } + switch (status) { + case 401: return "Your session has expired. Please sign in again." + case 403: return "You don't have permission to do that." + case 404: return "The requested record could not be found." + case 409: return "This change conflicts with an existing record." + case 400: + case 422: return "Check the information and try again." + case 429: return "Too many requests. Try again shortly." + default: return "The server could not complete the request. Try again shortly." + } +} + +/** A readable message for either API envelope, network errors, and local errors. */ +export function formatApiError(error: unknown): string { + if (error instanceof ApiError) return error.message + if (error instanceof TypeError && /fetch|network/i.test(error.message)) { + return "Unable to connect. Check your connection and try again." + } + const message = readableMessage(error instanceof Error ? error.message : error) + return message && !/^API \d+:/.test(message) + ? message + : "Something went wrong. Please try again." +} + export type ListResponse = { /** Each item carries an optional `__permissions` companion field with * the Cedar decision the server made for that row. */ @@ -113,19 +150,12 @@ export async function request(path: string, init?: RequestInit): Promise { const next = encodeURIComponent(window.location.pathname + window.location.search) window.location.replace(`/login?next=${next}`) } - throw new Error("API 401: unauthorized") + throw new ApiError(401, undefined, "") } if (!res.ok) { // Preserve the structured JSON body when present so the form layer can // map error codes like `unique_violation` to inline field errors. - const raw = await res.text() - let body: unknown - try { - body = raw ? JSON.parse(raw) : undefined - } catch { - body = undefined - } - throw new ApiError(res.status, body, raw) + throw await responseError(res) } // 204 No Content: don't try to parse JSON. if (res.status === 204) { @@ -134,15 +164,16 @@ export async function request(path: string, init?: RequestInit): Promise { return res.json() as Promise } +async function responseError(response: Response): Promise { + const raw = await response.text() + let body: unknown + try { body = raw ? JSON.parse(raw) : undefined } catch { body = undefined } + return new ApiError(response.status, body, raw) +} + async function sendRequest(path: string, init?: RequestInit): Promise { - const token = tokenStore.get() - const headers: Record = { - "Content-Type": "application/json", - ...((init?.headers as Record | undefined) ?? {}), - } - if (token) { - headers["Authorization"] = `Bearer ${token}` - } + const headers = authenticatedHeaders(init?.headers) + if (!headers.has("Content-Type")) headers.set("Content-Type", "application/json") return fetch(`${API_BASE}${path}`, { ...init, headers }) } @@ -296,7 +327,7 @@ export async function getMeta(): Promise { headers: { "Content-Type": "application/json" }, }) if (!res.ok) { - throw new Error(`API ${res.status}: ${await res.text()}`) + throw await responseError(res) } return res.json() as Promise } diff --git a/crates/schema-forge-cli/templates/site/src/lib/auth.ts.jinja b/crates/schema-forge-cli/templates/site/src/lib/auth.ts.jinja index 74a83da..b832b99 100644 --- a/crates/schema-forge-cli/templates/site/src/lib/auth.ts.jinja +++ b/crates/schema-forge-cli/templates/site/src/lib/auth.ts.jinja @@ -172,6 +172,18 @@ export const activeTenantStore = { }, } +/** Build same-origin API headers from the current session and tenant choice. */ +export function authenticatedHeaders(initial?: HeadersInit): Headers { + const headers = new Headers(initial) + const token = tokenStore.get() + if (token) headers.set("Authorization", `Bearer ${token}`) + const active = activeTenantStore.get() + if (active && !headers.has(ACTIVE_TENANT_HEADER)) { + headers.set(ACTIVE_TENANT_HEADER, active) + } + return headers +} + function storeAndSchedule(body: LoginResponse): LoginResponse { tokenStore.set(body.token, body.expires_at, body.roles ?? []) scheduleRefresh(body.expires_at) @@ -340,12 +352,7 @@ export type MeResponse = { export async function fetchMe(): Promise { const token = tokenStore.get() if (!token) throw new Error("no stored token") - const headers: Record = { - Authorization: `Bearer ${token}`, - } - const active = activeTenantStore.get() - if (active) headers[ACTIVE_TENANT_HEADER] = active - const res = await fetch(`${API_BASE}${ME_PATH}`, { headers }) + const res = await fetch(`${API_BASE}${ME_PATH}`, { headers: authenticatedHeaders() }) if (!res.ok) throw new Error(`auth/me failed: ${res.status}`) return (await res.json()) as MeResponse } diff --git a/crates/schema-forge-cli/templates/site/src/lib/error-toast.ts.jinja b/crates/schema-forge-cli/templates/site/src/lib/error-toast.ts.jinja new file mode 100644 index 0000000..d47891d --- /dev/null +++ b/crates/schema-forge-cli/templates/site/src/lib/error-toast.ts.jinja @@ -0,0 +1,23 @@ +// Scaffolded once. Customize or disable global error notifications here. +import { toast } from "sonner" +import { formatApiError } from "@/generated/api-client" + +type ErrorSource = { meta?: Record } +type MutationSource = ErrorSource & { options?: { onError?: unknown } } + +/** Set meta.suppressGlobalError on a query that renders its own recovery UI. */ +export function onQueryError(error: unknown, query: ErrorSource): void { + if (query.meta?.suppressGlobalError !== true) toast.error(formatApiError(error)) +} + +/** Mutations with inline or local error handling should opt out through meta. */ +export function onMutationError( + error: unknown, + _variables: unknown, + _context: unknown, + mutation: MutationSource, +): void { + if (mutation.meta?.suppressGlobalError !== true && !mutation.options?.onError) { + toast.error(formatApiError(error)) + } +} diff --git a/crates/schema-forge-cli/templates/site/src/main.tsx.jinja b/crates/schema-forge-cli/templates/site/src/main.tsx.jinja index 3c385b1..0ac6be2 100644 --- a/crates/schema-forge-cli/templates/site/src/main.tsx.jinja +++ b/crates/schema-forge-cli/templates/site/src/main.tsx.jinja @@ -7,7 +7,7 @@ import { QueryClient, QueryClientProvider, } from "@tanstack/react-query" -import { toast } from "sonner" +import { onQueryError, onMutationError } from "./lib/error-toast" import App from "./App" import { ensureRefreshScheduled } from "./lib/auth" import "./index.css" @@ -22,14 +22,10 @@ ensureRefreshScheduled() // instead of a toast; they reach this cache only as a final fallback. const queryClient = new QueryClient({ queryCache: new QueryCache({ - onError: (error) => { - toast.error(`Load failed: ${error instanceof Error ? error.message : String(error)}`) - }, + onError: onQueryError, }), mutationCache: new MutationCache({ - onError: (error) => { - toast.error(`Action failed: ${error instanceof Error ? error.message : String(error)}`) - }, + onError: onMutationError, }), }) diff --git a/crates/schema-forge-cli/tests/site_e2e/playwright/tests/client-errors.spec.ts b/crates/schema-forge-cli/tests/site_e2e/playwright/tests/client-errors.spec.ts new file mode 100644 index 0000000..75fc840 --- /dev/null +++ b/crates/schema-forge-cli/tests/site_e2e/playwright/tests/client-errors.spec.ts @@ -0,0 +1,158 @@ +import { expect, test, type Page, type Route } from "@playwright/test" + +async function session(page: Page, entityResponse?: (route: Route) => Promise) { + await page.addInitScript(() => { + sessionStorage.setItem("schemaforge.token", "test-token") + sessionStorage.setItem("schemaforge.token_expires_at", "2099-01-01T00:00:00Z") + sessionStorage.setItem("schemaforge.roles", JSON.stringify(["member"])) + if (!sessionStorage.getItem("schemaforge.active_tenant")) { + sessionStorage.setItem("schemaforge.active_tenant", "Org:org-a") + } + }) + await page.route("**/api/v1/forge/auth/me", async route => { + const active = route.request().headers()["x-active-tenant"] + await route.fulfill({ json: { + user_id: "member", email: "member@example.com", display_name: "Member", roles: ["member"], + tenant_chain: [{ schema: "Org", entity_id: "org-a" }, { schema: "Org", entity_id: "org-b" }], + active_tenant: active ? { tenant_type: "Org", tenant_id: active.split(":")[1] } : null, + } }) + }) + await page.route("**/api/v1/forge/schemas", route => route.fulfill({ json: { + schemas: [{ name: "Company", annotations: [], permissions: { create: true } }], count: 1, + } })) + await page.route("**/api/v1/forge/schemas/Company/entities**", async route => { + if (entityResponse) return entityResponse(route) + const tenant = route.request().headers()["x-active-tenant"] + if (!tenant) return route.fulfill({ status: 400, json: { code: "ACTIVE_TENANT_REQUIRED" } }) + return route.fulfill({ json: { + entities: [{ id: "company_test", schema: "Company", fields: { name: `Company for ${tenant}`, status: "active" } }], + count: 1, permissions: { create: true }, + } }) + }) +} + +test("multi-membership tenant picker scopes the entity list after switching", async ({ page }) => { + await session(page) + await page.goto("/app/company") + await expect(page.getByText("Company for Org:org-a", { exact: true })).toBeVisible() + await page.getByRole("combobox", { name: "Switch active tenant" }).selectOption("Org:org-b") + await expect(page.getByText("Company for Org:org-b", { exact: true })).toBeVisible() + await expect(page.getByText("Company for Org:org-a", { exact: true })).toHaveCount(0) +}) + +test("request headers preserve explicit tenant overrides and refresh retries", async ({ page }) => { + await page.goto("/login") + let retries = 0 + await page.route("**/api/v1/forge/auth/refresh", route => route.fulfill({ json: { + token: "new-token", expires_at: "2099-01-01T00:00:00Z", roles: ["member"], + } })) + await page.route("**/api/v1/forge/header-test**", async route => { + if (route.request().url().endsWith("/retry") && retries++ === 0) { + return route.fulfill({ status: 401, json: { error: "unauthorized" } }) + } + return route.fulfill({ json: route.request().headers() }) + }) + const headers = await page.evaluate(async () => { + const authPath = "/src/lib/auth.ts" + const apiPath = "/src/generated/api-client.ts" + const auth = await import(authPath) + const api = await import(apiPath) + auth.tokenStore.set("old-token", "2099-01-01T00:00:00Z", ["member"]) + auth.activeTenantStore.set("Org", "org-a") + const result = [] + for (const method of ["GET", "POST", "PATCH", "DELETE"]) { + result.push(await api.request("/api/v1/forge/header-test", { method })) + } + result.push(await api.request("/api/v1/forge/header-test", { + headers: new Headers({ "x-active-tenant": "Org:explicit", "X-Extra": "preserved" }), + })) + result.push(await api.request("/api/v1/forge/header-test", { + headers: [["x-active-tenant", "Org:tuple"]], + })) + auth.activeTenantStore.set("Org", "org-b") + result.push(await api.request("/api/v1/forge/header-test/retry")) + auth.activeTenantStore.clear() + result.push(await api.request("/api/v1/forge/header-test")) + return result + }) + for (const value of headers.slice(0, 4)) expect(value["x-active-tenant"]).toBe("Org:org-a") + expect(headers[4]["x-active-tenant"]).toBe("Org:explicit") + expect(headers[4]["x-extra"]).toBe("preserved") + expect(headers[5]["x-active-tenant"]).toBe("Org:tuple") + expect(headers[6]["x-active-tenant"]).toBe("Org:org-b") + expect(headers[6].authorization).toBe("Bearer new-token") + expect(headers[7]["x-active-tenant"]).toBeUndefined() + expect(retries).toBe(2) +}) + +test("API errors use readable envelope messages and status fallbacks", async ({ page }) => { + await page.goto("/login") + const messages = await page.evaluate(async () => { + const path = "/src/generated/api-client.ts" + const { ApiError, formatApiError } = await import(path) + return [ + formatApiError(new ApiError(422, { error: "validation_failed", message: "A title is required" }, "raw")), + formatApiError(new ApiError(401, { error: "This token is expired", code: "INVALID_TOKEN" }, "raw")), + formatApiError(new ApiError(403, { error: "forbidden" }, "raw")), + formatApiError(new ApiError(429, undefined, "proxy error")), + formatApiError(new TypeError("Failed to fetch")), + formatApiError({ arbitrary: "object" }), + ] + }) + expect(messages).toEqual([ + "A title is required", "This token is expired", "You don't have permission to do that.", + "Too many requests. Try again shortly.", "Unable to connect. Check your connection and try again.", + "Something went wrong. Please try again.", + ]) +}) + +test("unique save errors stay inline without a global toast", async ({ page }) => { + await session(page, route => route.fulfill({ status: 409, json: { + error: "unique_violation", field: "name", message: "This name is already in use", + } })) + await page.goto("/app/company/new") + await page.getByLabel(/^name/i).fill("Duplicate") + await page.getByRole("button", { name: /^create$/i }).click() + await expect(page.getByText("Already in use", { exact: true })).toBeVisible() + await expect(page.locator("[data-sonner-toast]")).toHaveCount(0) +}) + +test("save errors show one readable local toast", async ({ page }) => { + await session(page, route => route.fulfill({ status: 422, json: { + error: "validation_failed", message: "Company name must contain a letter", + } })) + await page.goto("/app/company/new") + await page.getByLabel(/^name/i).fill("123") + await page.getByRole("button", { name: /^create$/i }).click() + await expect(page.locator("[data-sonner-toast]")).toHaveCount(1) + await expect(page.locator("[data-sonner-toast]")).toContainText("Company name must contain a letter") +}) + +test("list load errors render recovery inline without a global toast", async ({ page }) => { + await session(page, route => route.fulfill({ status: 403, json: { error: "forbidden" } })) + await page.goto("/app/company") + await expect(page.getByRole("alert")).toContainText("You don't have permission to do that.", { timeout: 15_000 }) + await expect(page.getByRole("button", { name: "Retry", exact: true })).toBeVisible() + await expect(page.locator("[data-sonner-toast]")).toHaveCount(0) +}) + + +test("customizable global handlers honor query and mutation opt-outs", async ({ page }) => { + await page.goto("/login") + await page.evaluate(async () => { + const path = "/src/lib/error-toast.ts" + const handlers = await import(path) + const error = new Error("Handled locally") + handlers.onQueryError(error, { meta: { suppressGlobalError: true } }) + handlers.onMutationError(error, undefined, undefined, { meta: { suppressGlobalError: true } }) + handlers.onMutationError(error, undefined, undefined, { options: { onError: () => {} } }) + }) + await expect(page.locator("[data-sonner-toast]")).toHaveCount(0) + await page.evaluate(async () => { + const path = "/src/lib/error-toast.ts" + const handlers = await import(path) + handlers.onQueryError(new Error("Global load failure"), {}) + }) + await expect(page.locator("[data-sonner-toast]")).toHaveCount(1) + await expect(page.locator("[data-sonner-toast]")).toContainText("Global load failure") +}) diff --git a/crates/schema-forge-cli/tests/site_generate.rs b/crates/schema-forge-cli/tests/site_generate.rs index ab48167..fc01fb0 100644 --- a/crates/schema-forge-cli/tests/site_generate.rs +++ b/crates/schema-forge-cli/tests/site_generate.rs @@ -251,6 +251,29 @@ fn preserve_pages_survive_rerun() { assert_eq!(after, "// user edit\n"); } +#[test] +fn custom_error_handlers_survive_regeneration_and_drift_check() { + let tmp = TempDir::new().unwrap(); + let schema_dir = tmp.path().join("schemas"); + let out_dir = tmp.path().join("site"); + write_schemas(&schema_dir, V0_EMPLOYEE); + run_generate(&schema_dir, &out_dir, "Employee", &[]) + .assert() + .success(); + let handler = out_dir.join("src/lib/error-toast.ts"); + let scaffold = fs::read_to_string(&handler).unwrap(); + assert!(scaffold.contains("suppressGlobalError")); + let customization = "export function onQueryError() {}\nexport function onMutationError() {}\n"; + fs::write(&handler, customization).unwrap(); + run_generate(&schema_dir, &out_dir, "Employee", &[]) + .assert() + .success(); + run_generate(&schema_dir, &out_dir, "Employee", &["--check"]) + .assert() + .success(); + assert_eq!(fs::read_to_string(handler).unwrap(), customization); +} + #[test] fn issue_40_preserve_shell_survives_while_generated_refreshes() { // Acceptance criterion from #40: schema changes flow into the diff --git a/docs/generated-site-errors.md b/docs/generated-site-errors.md new file mode 100644 index 0000000..bc1f19b --- /dev/null +++ b/docs/generated-site-errors.md @@ -0,0 +1,11 @@ +# Generated site request context and error handling + +The generated API client sends the current `X-Active-Tenant` selection on entity and invitation requests, including retries after token refresh. File upload and download API calls use the same session header builder. An explicit tenant header in a request takes precedence, regardless of header casing or whether headers are supplied as an object, tuples, or a `Headers` instance. Public login and invitation acceptance do not require a tenant selection. + +`ApiError` retains `status`, parsed `body`, and `raw` for application logic and diagnostics. Its message and `formatApiError(error)` display the human message from either supported API envelope, with status and network fallbacks when needed. Components should use the formatter rather than displaying the raw response. + +Global query and mutation notifications live in `src/lib/error-toast.ts`. The generator scaffolds this file once and preserves your changes during regeneration and `--check`. Customize its exported handlers to change notification appearance, send errors to monitoring, or disable global notifications entirely. + +Queries and mutations can set `meta: { suppressGlobalError: true }` when they render their own recovery controls. The default global mutation handler also skips mutations with their own `onError` callback. Generated edit forms handle unique conflicts inline and show one local notification for other save failures. Generated list and detail pages render an inline error and Retry button. + +When upgrading an existing generated project, regenerate the owned client and shared components to receive the fixes. Your existing page shells are preserved. Add `meta: { suppressGlobalError: true }` to list and detail queries that already render `ErrorBlock`, and use `formatApiError(error)` in custom local handlers. Existing edit mutations with `onError` automatically avoid duplicate global notifications through the new default global handler. If you previously customized `src/lib/error-toast.ts`, retain or adapt those policies explicitly. From 06e17afb3e897ec1a19e14d0edd8d4a5bb3a28c1 Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 13:37:58 -0600 Subject: [PATCH 19/38] feat(site): configure product branding and asset overrides --- README.md | 2 + crates/schema-forge-acton/src/config.rs | 17 ++ crates/schema-forge-cli/src/cli.rs | 20 ++ .../src/commands/site/branding.rs | 127 ++++++++++++ .../src/commands/site/context.rs | 1 + .../schema-forge-cli/src/commands/site/mod.rs | 85 +++++--- .../src/commands/site/render.rs | 38 ++-- .../src/commands/site/vendor.rs | 60 ++---- .../templates/site/index.html.jinja | 3 +- .../templates/site/src/App.tsx.jinja | 7 +- .../templates/site/src/lib/branding.ts.jinja | 4 + .../site/src/pages/accessibility.tsx.jinja | 8 +- .../templates/site/src/pages/login.tsx.jinja | 27 +-- .../schema-forge-cli/tests/site_branding.rs | 187 ++++++++++++++++++ .../site_e2e/playwright/tests/smoke.spec.ts | 11 ++ crates/schema-forge-cli/tests/site_e2e/run.sh | 4 +- docs/site-branding.md | 69 +++++++ 17 files changed, 549 insertions(+), 121 deletions(-) create mode 100644 crates/schema-forge-cli/src/commands/site/branding.rs create mode 100644 crates/schema-forge-cli/templates/site/src/lib/branding.ts.jinja create mode 100644 crates/schema-forge-cli/tests/site_branding.rs create mode 100644 docs/site-branding.md diff --git a/README.md b/README.md index 6ce2341..e282a3d 100644 --- a/README.md +++ b/README.md @@ -1058,3 +1058,5 @@ Platform administrators can browse recorded audit events and verify bounded chai See [safe schema changes](docs/migrations/safe-schema-changes.md) for declared field renames, destructive migration opt-ins, PostgreSQL relation integrity, and explicit tenancy migrations. See the [webhook delivery contract](docs/webhooks.md) for delivery guarantees, payload format, and destination policy. + +Configure product names, title suffixes, and SVG marks with [generated-site branding](docs/site-branding.md). diff --git a/crates/schema-forge-acton/src/config.rs b/crates/schema-forge-acton/src/config.rs index 48accb8..da1d377 100644 --- a/crates/schema-forge-acton/src/config.rs +++ b/crates/schema-forge-acton/src/config.rs @@ -88,6 +88,21 @@ pub struct SchemaForgeSettings { /// environment variables still override these values. #[serde(default)] pub client: ClientConfig, + + /// Branding for generated sites. + #[serde(default)] + pub site: SiteBrandingConfig, +} + +/// Optional generated-site identity and SVG assets. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct SiteBrandingConfig { + pub name: Option, + /// Defaults to name; an empty string disables the suffix. + pub title_suffix: Option, + pub logo: Option, + pub logo_on_dark: Option, + pub favicon: Option, } /// `[schema_forge.client]` section of config.toml. @@ -165,6 +180,7 @@ impl Default for SchemaForgeSettings { authz: AuthzConfig::default(), signing: SigningConfig::default(), client: ClientConfig::default(), + site: SiteBrandingConfig::default(), } } } @@ -195,6 +211,7 @@ mod tests { authz: AuthzConfig::default(), signing: SigningConfig::default(), client: ClientConfig::default(), + site: SiteBrandingConfig::default(), }, }; let json = serde_json::to_string(&config).unwrap(); diff --git a/crates/schema-forge-cli/src/cli.rs b/crates/schema-forge-cli/src/cli.rs index cc19c9d..6d1a2e5 100644 --- a/crates/schema-forge-cli/src/cli.rs +++ b/crates/schema-forge-cli/src/cli.rs @@ -395,6 +395,26 @@ pub struct SiteGenerateArgs { #[arg(short = 'o', long, default_value = "site")] pub out_dir: PathBuf, + /// Product name shown in the generated site (overrides config). + #[arg(long)] + pub name: Option, + + /// Browser-title suffix; pass an empty string to disable. + #[arg(long)] + pub title_suffix: Option, + + /// SVG logo for light surfaces (overrides config). + #[arg(long)] + pub logo: Option, + + /// SVG logo for dark surfaces (defaults to logo). + #[arg(long)] + pub logo_on_dark: Option, + + /// SVG favicon (defaults to logo). + #[arg(long)] + pub favicon: Option, + /// Pick a single schema by name. Defaults to the first schema in the directory. #[arg(long)] pub schema: Option, diff --git a/crates/schema-forge-cli/src/commands/site/branding.rs b/crates/schema-forge-cli/src/commands/site/branding.rs new file mode 100644 index 0000000..91ab09c --- /dev/null +++ b/crates/schema-forge-cli/src/commands/site/branding.rs @@ -0,0 +1,127 @@ +//! Resolve product identity independently of output directory names. + +use std::path::Path; + +use heck::{ToKebabCase, ToTitleCase}; +use schema_forge_acton::config::SiteBrandingConfig; +use schema_forge_core::types::{Annotation, SchemaDefinition, TenantKind}; +use serde::Serialize; + +use crate::cli::{GlobalOpts, SiteGenerateArgs}; +use crate::error::CliError; + +#[derive(Debug, Clone, Serialize)] +pub struct Branding { + pub name: String, + pub name_json: String, + pub title_suffix_json: String, + pub theme_key_json: String, + pub logo: Option, + pub logo_on_dark: Option, + pub favicon: Option, +} + +impl Branding { + pub fn resolve( + args: &SiteGenerateArgs, + config: &SiteBrandingConfig, + global: &GlobalOpts, + schemas: &[SchemaDefinition], + ) -> Result { + let name = args + .name + .clone() + .or_else(|| config.name.clone()) + .unwrap_or_else(|| { + schemas + .iter() + .find(|schema| { + schema.annotations.iter().any(|annotation| { + matches!(annotation, Annotation::Tenant(TenantKind::Root)) + }) + }) + .map(|schema| schema.name.as_str().to_title_case()) + .or_else(|| { + args.schema_dir.canonicalize().ok().and_then(|path| { + path.parent() + .and_then(Path::file_name) + .map(|name| name.to_string_lossy().to_title_case()) + }) + }) + .unwrap_or_else(|| "Application".into()) + }); + if name.trim().is_empty() || name.chars().any(char::is_control) { + return Err(CliError::Config { + message: "site name must be nonempty and contain no control characters".into(), + }); + } + let title_suffix = args + .title_suffix + .as_ref() + .or(config.title_suffix.as_ref()) + .unwrap_or(&name); + let config_base = global + .config + .as_deref() + .and_then(Path::parent) + .unwrap_or(Path::new(".")); + let logo = read_asset(args.logo.as_deref(), config.logo.as_deref(), config_base)?; + let logo_on_dark = read_asset( + args.logo_on_dark.as_deref(), + config.logo_on_dark.as_deref(), + config_base, + )? + .or_else(|| logo.clone()); + let favicon = read_asset( + args.favicon.as_deref(), + config.favicon.as_deref(), + config_base, + )? + .or_else(|| logo.clone()); + let theme_key = format!("{}.theme", name.to_kebab_case()); + Ok(Self { + name_json: json_string(&name)?, + title_suffix_json: json_string(title_suffix)?, + theme_key_json: json_string(&theme_key)?, + name, + logo, + logo_on_dark, + favicon, + }) + } +} + +fn json_string(value: &str) -> Result { + serde_json::to_string(value).map_err(|error| CliError::Config { + message: format!("failed to encode site branding: {error}"), + }) +} + +fn read_asset( + flag: Option<&Path>, + configured: Option<&Path>, + config_base: &Path, +) -> Result, CliError> { + let Some(path) = flag + .map(Path::to_path_buf) + .or_else(|| configured.map(|path| config_base.join(path))) + else { + return Ok(None); + }; + if !path + .extension() + .is_some_and(|extension| extension.eq_ignore_ascii_case("svg")) + { + return Err(CliError::Config { + message: format!("site branding asset {} must be an SVG file", path.display()), + }); + } + let contents = + std::fs::read_to_string(&path).map_err(|source| CliError::Io { path, source })?; + if !contents.contains(" String { /// `entity` without reaching through the list. #[derive(Debug, Clone, Serialize)] pub struct SiteContext { + pub branding: super::branding::Branding, /// Kebab-cased project name (for `package.json`, ``, etc.). pub project_name: String, /// Every non-system schema, projected into a generator-friendly view. diff --git a/crates/schema-forge-cli/src/commands/site/mod.rs b/crates/schema-forge-cli/src/commands/site/mod.rs index 022be6c..d896eac 100644 --- a/crates/schema-forge-cli/src/commands/site/mod.rs +++ b/crates/schema-forge-cli/src/commands/site/mod.rs @@ -6,6 +6,7 @@ //! route manifest) and per-entity pages. `--schema NAME` narrows generation //! to a single schema for debugging or partial regen. +mod branding; mod context; mod mapping; mod render; @@ -70,12 +71,21 @@ fn generate( output.status(&format!(" target: {}", def.name.as_str())); } - let project_name = args - .out_dir - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("schema-forge-site") - .to_kebab_case(); + let config = crate::config::load_svc_config(global)?; + let branding = + branding::Branding::resolve(&args, &config.custom.schema_forge.site, global, &schemas)?; + let slug: String = branding + .name + .to_kebab_case() + .chars() + .filter(|ch| ch.is_ascii_alphanumeric() || *ch == '-') + .collect(); + let slug = slug.trim_matches('-').to_string(); + let project_name = if slug.is_empty() { + "application".into() + } else { + slug + }; // Build a catalog of every known schema so mapping can resolve // relation targets (display field, kebab slug) even when the target @@ -111,6 +121,7 @@ fn generate( } let ctx = SiteContext { + branding, project_name: project_name.clone(), entities, accessibility_contact: args.accessibility_contact.clone(), @@ -255,7 +266,10 @@ fn build_plan(ctx: &SiteContext, renderer: &SiteRenderer) -> Result<Vec<FilePlan vendor::TSCONFIG_NODE_JSON.to_string(), )); plan.push(owned(".gitignore", vendor::GITIGNORE.to_string())); - plan.push(owned("eslint.config.js", vendor::ESLINT_CONFIG_JS.to_string())); + plan.push(owned( + "eslint.config.js", + vendor::ESLINT_CONFIG_JS.to_string(), + )); // Brand marks. Vite serves `public/` at the URL root, so the templates // can reference `/logo-mark-white.svg` and `/logo-mark.svg` directly @@ -263,17 +277,41 @@ fn build_plan(ctx: &SiteContext, renderer: &SiteRenderer) -> Result<Vec<FilePlan // and login left panel; the ink mark is the favicon. plan.push(owned( "public/logo-mark-white.svg", - vendor::LOGO_MARK_WHITE_SVG.to_string(), + ctx.branding + .logo_on_dark + .clone() + .map(Ok) + .unwrap_or_else(|| renderer.render("public/logo-mark-white.svg", ctx))?, )); plan.push(owned( "public/logo-mark.svg", - vendor::LOGO_MARK_INK_SVG.to_string(), + ctx.branding + .logo + .clone() + .map(Ok) + .unwrap_or_else(|| renderer.render("public/logo-mark.svg", ctx))?, + )); + + plan.push(owned( + "public/favicon.svg", + ctx.branding + .favicon + .clone() + .map(Ok) + .unwrap_or_else(|| renderer.render("public/favicon.svg", ctx))?, )); // ---- src/ scaffolding ---- plan.push(owned("src/main.tsx", renderer.render("src/main.tsx", ctx)?)); plan.push(owned("src/App.tsx", renderer.render("src/App.tsx", ctx)?)); - plan.push(owned("src/index.css", vendor::INDEX_CSS.to_string())); + plan.push(owned( + "src/index.css", + renderer.render("src/index.css", ctx)?, + )); + plan.push(owned( + "src/lib/branding.ts", + renderer.render("src/lib/branding.ts", ctx)?, + )); plan.push(owned( "src/lib/utils.ts", vendor::SHADCN_UTILS_TS.to_string(), @@ -288,7 +326,7 @@ fn build_plan(ctx: &SiteContext, renderer: &SiteRenderer) -> Result<Vec<FilePlan )); plan.push(owned( "src/lib/use-document-title.ts", - vendor::USE_DOCUMENT_TITLE.to_string(), + renderer.render("src/lib/use-document-title.ts", ctx)?, )); plan.push(preserve( @@ -522,12 +560,8 @@ mod tests { FieldDefinition::with_annotations( FieldName::new("stage").unwrap(), FieldType::Enum( - EnumVariants::new(vec![ - "qualifying".into(), - "won".into(), - "lost".into(), - ]) - .unwrap(), + EnumVariants::new(vec!["qualifying".into(), "won".into(), "lost".into()]) + .unwrap(), ), vec![FieldModifier::Required], vec![FieldAnnotation::EnumColors { colors }], @@ -545,10 +579,7 @@ mod tests { let schema = opportunity_schema_with_enum_colors(); let mut catalog = BTreeMap::new(); - catalog.insert( - "Opportunity".to_string(), - SchemaMeta::from_schema(&schema), - ); + catalog.insert("Opportunity".to_string(), SchemaMeta::from_schema(&schema)); let output = crate::output::OutputContext { mode: crate::output::OutputMode::Plain, verbose: 0, @@ -600,19 +631,14 @@ mod tests { // Explicit column hint. FieldDefinition::with_annotations( FieldName::new("stage").unwrap(), - FieldType::Enum( - EnumVariants::new(vec!["new".into(), "won".into()]).unwrap(), - ), + FieldType::Enum(EnumVariants::new(vec!["new".into(), "won".into()]).unwrap()), vec![FieldModifier::Required], vec![FieldAnnotation::List { hint: ListHint::Column, }], ), // Rich text auto-hides by default. - FieldDefinition::new( - FieldName::new("description").unwrap(), - FieldType::RichText, - ), + FieldDefinition::new(FieldName::new("description").unwrap(), FieldType::RichText), // Unannotated integer -> column. FieldDefinition::new( FieldName::new("pwin").unwrap(), @@ -813,8 +839,7 @@ mod tests { "FileUpload meta must carry the byte limit" ); assert!( - rendered.contains("\"application/pdf\"") - && rendered.contains("\"image/*\""), + rendered.contains("\"application/pdf\"") && rendered.contains("\"image/*\""), "FileUpload meta must carry the mime allowlist" ); diff --git a/crates/schema-forge-cli/src/commands/site/render.rs b/crates/schema-forge-cli/src/commands/site/render.rs index f7d8ba9..d83e897 100644 --- a/crates/schema-forge-cli/src/commands/site/render.rs +++ b/crates/schema-forge-cli/src/commands/site/render.rs @@ -7,13 +7,9 @@ //! 2. Fall back to the slice baked into the binary at build time //! (`EMBEDDED_SITE_TEMPLATES`, emitted by `build.rs`). //! -//! This lets framework users iterate on generator output without a CLI -//! rebuild: drop an override tree next to your schemas, tweak `.jinja` -//! files, re-run `schema-forge site generate`, and only the overridden -//! files swap — every other template still comes from the embedded -//! defaults. When the overrides look right, copy them back into -//! `crates/schema-forge-cli/templates/site/` and they become the new -//! baked-in default. +//! Projects can keep persistent customizations in this override tree. Overrides +//! participate in generation and drift checking, including CSS, title helpers, +//! and brand SVGs that otherwise use embedded vendor defaults. //! //! Logical template names (`"src/App.tsx"`, `"package.json"`, …) are the //! post-`.jinja`-strip relative paths, which is also the final output @@ -38,17 +34,24 @@ impl SiteRenderer { /// /// If `override_dir` is `Some`, the loader checks that directory for /// `<logical_name>.jinja` before falling back to the embedded defaults. - /// Read errors on an override file are treated as "not overridden" and - /// silently fall through to the embedded template. + /// Unreadable override files fail generation rather than silently falling back. pub fn new(override_dir: Option<PathBuf>) -> Result<Self, CliError> { let mut env = Environment::new(); env.set_loader(move |name: &str| { if let Some(ref dir) = override_dir { let candidate = dir.join(format!("{name}.jinja")); if candidate.is_file() { - if let Ok(content) = std::fs::read_to_string(&candidate) { - return Ok(Some(content)); - } + return std::fs::read_to_string(&candidate) + .map(Some) + .map_err(|error| { + minijinja::Error::new( + minijinja::ErrorKind::InvalidOperation, + format!( + "cannot read template override {}: {error}", + candidate.display() + ), + ) + }); } } for (logical, content) in EMBEDDED_SITE_TEMPLATES { @@ -56,7 +59,16 @@ impl SiteRenderer { return Ok(Some((*content).to_string())); } } - Ok(None) + let vendor = match name { + "public/logo-mark.svg" | "public/favicon.svg" => { + Some(super::vendor::LOGO_MARK_INK_SVG) + } + "public/logo-mark-white.svg" => Some(super::vendor::LOGO_MARK_WHITE_SVG), + "src/index.css" => Some(super::vendor::INDEX_CSS), + "src/lib/use-document-title.ts" => Some(super::vendor::USE_DOCUMENT_TITLE), + _ => None, + }; + Ok(vendor.map(str::to_string)) }); Ok(Self { env }) } diff --git a/crates/schema-forge-cli/src/commands/site/vendor.rs b/crates/schema-forge-cli/src/commands/site/vendor.rs index e4f9864..3d46f6a 100644 --- a/crates/schema-forge-cli/src/commands/site/vendor.rs +++ b/crates/schema-forge-cli/src/commands/site/vendor.rs @@ -718,61 +718,25 @@ dist .DS_Store "#; -/// Govcraft brand mark, white fill — used on the inked sidebar rail and -/// the dark login left panel. Vendored from the Govcraft DS (paths only, -/// no rasterized stroke). Vite's `public/` dir serves these at the URL -/// root, so the React templates can `<img src="/logo-mark-white.svg" />` -/// without bundler involvement. -pub const LOGO_MARK_WHITE_SVG: &str = r##"<?xml version="1.0" encoding="UTF-8" standalone="no"?> -<svg version="1.1" id="svg1" width="886.4729" height="720.97095" viewBox="0 0 886.47289 720.97095" xmlns="http://www.w3.org/2000/svg" xmlns:svg="http://www.w3.org/2000/svg"> - <defs id="defs1"></defs> - <g id="layer-MC0" transform="translate(9.3134156e-4,-75.917999)"> - <path id="path1" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#ffffff;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,270.4512,284.1136)"></path> - <path id="path2" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#ffffff;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,270.45107,400.17613)"></path> - <path id="path3" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#ffffff;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,474.93093,400.17613)"></path> - <path id="path4" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#ffffff;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,679.4108,400.17627)"></path> - <path id="path5" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#ffffff;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,270.68787,515.8356)"></path> - <path id="path6" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#ffffff;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,270.68787,631.89813)"></path> - <path id="path7" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#ffffff;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,475.24653,631.89827)"></path> - <path id="path8" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#ffffff;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,543.4328,631.89827)"></path> - <path id="path9" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#ffffff;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,679.80533,631.89827)"></path> - <path id="path10" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#ffffff;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,679.80533,515.83573)"></path> - <path id="path11" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#ffffff;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,543.19613,284.1136)"></path> - </g> -</svg> +/// Neutral four-square application mark on dark surfaces. +pub const LOGO_MARK_WHITE_SVG: &str = r##"<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 32 32"><path fill="#fff" d="M2 2h12v12H2zM18 2h12v12H18zM2 18h12v12H2zM18 18h12v12H18z"/></svg> "##; -/// Govcraft brand mark, ink fill — used as the favicon and on light surfaces. -pub const LOGO_MARK_INK_SVG: &str = r##"<?xml version="1.0" encoding="UTF-8" standalone="no"?> -<svg version="1.1" id="svg1" width="886.4729" height="720.97095" viewBox="0 0 886.47289 720.97095" xmlns="http://www.w3.org/2000/svg" xmlns:svg="http://www.w3.org/2000/svg"> - <defs id="defs1"></defs> - <g id="layer-MC0" transform="translate(9.3134156e-4,-75.917999)"> - <path id="path1" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#000000;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,270.4512,284.1136)"></path> - <path id="path2" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#000000;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,270.45107,400.17613)"></path> - <path id="path3" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#000000;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,474.93093,400.17613)"></path> - <path id="path4" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#000000;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,679.4108,400.17627)"></path> - <path id="path5" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#000000;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,270.68787,515.8356)"></path> - <path id="path6" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#000000;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,270.68787,631.89813)"></path> - <path id="path7" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#000000;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,475.24653,631.89827)"></path> - <path id="path8" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#000000;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,543.4328,631.89827)"></path> - <path id="path9" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#000000;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,679.80533,631.89827)"></path> - <path id="path10" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#000000;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,679.80533,515.83573)"></path> - <path id="path11" d="M 0,0 H -46.026 V 78.342 H 0 Z" style="fill:#000000;fill-opacity:1;fill-rule:nonzero;stroke:none" transform="matrix(1.3333333,0,0,-1.3333333,543.19613,284.1136)"></path> - </g> -</svg> +/// Neutral application mark on light surfaces. +pub const LOGO_MARK_INK_SVG: &str = r##"<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 32 32"><path fill="#171717" d="M2 2h12v12H2zM18 2h12v12H18zM2 18h12v12H2zM18 18h12v12H18z"/></svg> "##; pub const INDEX_CSS: &str = r#"@import "tailwindcss"; /* ========================================================= - SchemaForge generated-site baseline — Govcraft Design System + Application design tokens Two type families (IBM Plex Sans + Mono), one signal accent (Signal Orange), paper ground + ink in light, console in dark. - shadcn primitive tokens are rebound onto these Govcraft tokens + shadcn primitive tokens are rebound onto these application tokens so Button / Input / Card render in-brand without changes. ========================================================= */ -/* ---- Govcraft palette + DS tokens (light is default) ---- */ +/* ---- Application palette and tokens (light is default) ---- */ :root { --gc-ink: #0A0A0A; --gc-ink-2: #1F1F1F; @@ -870,7 +834,7 @@ pub const INDEX_CSS: &str = r#"@import "tailwindcss"; --link: #91A8C8; } -/* ---- Bind shadcn v4 tokens onto Govcraft DS so primitives render in-brand ---- */ +/* ---- Bind shadcn v4 tokens onto application tokens so primitives render in-brand ---- */ @theme inline { --color-background: var(--app-bg); --color-foreground: var(--app-fg-1); @@ -2090,8 +2054,8 @@ function StatusChip({ status }: { status: FileAttachment["status"] }) { /// history, and screen-magnification users all rely on a unique, /// descriptive title per page. Restores the title on unmount so cross- /// route navigation doesn't leak the previous page's name. -pub const USE_DOCUMENT_TITLE: &str = r#"// Generated by schema-forge — edit freely. -import { useEffect } from "react" +pub const USE_DOCUMENT_TITLE: &str = r#"import { useEffect } from "react" +import { SITE_NAME, TITLE_SUFFIX } from "@/lib/branding" /** * Set the document title for the lifetime of the current page. @@ -2100,12 +2064,12 @@ import { useEffect } from "react" * useDocumentTitle(`${schema} · ${recordName}`) * * The previous title is restored on unmount. The product suffix - * (· SchemaForge) is appended automatically so callers stay terse. + * is configured by the project. An empty suffix disables it. */ export function useDocumentTitle(title: string): void { useEffect(() => { const previous = document.title - document.title = title ? `${title} · SchemaForge` : "SchemaForge" + document.title = title ? (TITLE_SUFFIX ? `${title} · ${TITLE_SUFFIX}` : title) : SITE_NAME return () => { document.title = previous } diff --git a/crates/schema-forge-cli/templates/site/index.html.jinja b/crates/schema-forge-cli/templates/site/index.html.jinja index 61adc0f..8baa0e4 100644 --- a/crates/schema-forge-cli/templates/site/index.html.jinja +++ b/crates/schema-forge-cli/templates/site/index.html.jinja @@ -11,7 +11,8 @@ <meta charset="UTF-8" /> <meta name="viewport" content="width=device-width, initial-scale=1.0" /> <meta name="theme-color" content="#0A0A0A" /> - <title>{{ project_name }} + {{ branding.name | escape }} + alt="" style={ { width: 26, height: 22, display: "block" }} /> -
{{ project_name }}
+
{SITE_NAME}
diff --git a/crates/schema-forge-cli/templates/site/src/lib/branding.ts.jinja b/crates/schema-forge-cli/templates/site/src/lib/branding.ts.jinja new file mode 100644 index 0000000..70f30b7 --- /dev/null +++ b/crates/schema-forge-cli/templates/site/src/lib/branding.ts.jinja @@ -0,0 +1,4 @@ +// Project identity. Configure through site generate flags or [schema_forge.site]. +export const SITE_NAME: string = {{ branding.name_json }} +export const TITLE_SUFFIX: string = {{ branding.title_suffix_json }} +export const THEME_STORAGE_KEY: string = {{ branding.theme_key_json }} diff --git a/crates/schema-forge-cli/templates/site/src/pages/accessibility.tsx.jinja b/crates/schema-forge-cli/templates/site/src/pages/accessibility.tsx.jinja index d5cf798..6105424 100644 --- a/crates/schema-forge-cli/templates/site/src/pages/accessibility.tsx.jinja +++ b/crates/schema-forge-cli/templates/site/src/pages/accessibility.tsx.jinja @@ -6,6 +6,7 @@ // office; do not delete the page — it is required by 36 CFR 1194 // §§603.2–603.3 to be reachable from every page in both authed and // unauthed states. +import { SITE_NAME } from "@/lib/branding" import { useDocumentTitle } from "@/lib/use-document-title" const ACCESSIBILITY_CONTACT = @@ -21,7 +22,7 @@ export function AccessibilityPage() {

- {{ project_name }} is committed to providing a digital experience + {SITE_NAME} is committed to providing a digital experience that conforms with Section 508 of the Rehabilitation Act of 1973 (29 U.S.C. § 794d) and the Web Content Accessibility Guidelines (WCAG) 2.1 Level AA, consistent with OMB Memorandum M-24-08 and @@ -30,8 +31,7 @@ export function AccessibilityPage() {

Conformance status

- This site is built on the SchemaForge generated-site baseline, - which targets WCAG 2.1 AA. Conformance is monitored in CI via + {SITE_NAME} targets WCAG 2.1 AA. Conformance is monitored in CI via automated axe scans (no critical or serious violations gate deployment) and verified by manual assistive-technology testing on each release. @@ -62,7 +62,7 @@ export function AccessibilityPage() { or ask questions by emailing{" "} {ACCESSIBILITY_CONTACT} diff --git a/crates/schema-forge-cli/templates/site/src/pages/login.tsx.jinja b/crates/schema-forge-cli/templates/site/src/pages/login.tsx.jinja index 1ef781f..6ae3e67 100644 --- a/crates/schema-forge-cli/templates/site/src/pages/login.tsx.jinja +++ b/crates/schema-forge-cli/templates/site/src/pages/login.tsx.jinja @@ -1,19 +1,5 @@ -// Scaffolded once by `schema-forge site generate` — edit freely. -// -// Login page for the generated {{ project_name }} site. Split layout -// per the Govcraft DS: -// -// - Left: always-inked identity panel. The Govcraft brand mark in the -// top-left, the SchemaForge product wordmark + tagline in the middle, -// four-key posture readout (API / Backend / Auth / Build) at the foot, -// and a 5%-opacity wallpaper of the same mark behind everything. -// - Right: paper credentials form (theme-aware) — `§ 01 / Sign in` -// eyebrow, `Sign in to {project}` heading, the seeded-admin help line, -// inputs with mono uppercase labels, an accent "Sign in" button, and a -// mono `POST /auth/login` telemetry footer. -// -// Exchanges credentials for a PASETO token at /api/v1/forge/auth/login -// via the auth helper. +// Sign-in page with a project identity panel. +import { SITE_NAME } from "@/lib/branding" import { useEffect, useState } from "react" import { useNavigate, useSearchParams } from "react-router-dom" import { useForm } from "react-hook-form" @@ -114,12 +100,11 @@ export function LoginPage() { marginBottom: 12, }} > - Govcraft · SchemaForge + {SITE_NAME}

- +

- One schema file. Migrations, CRUD, validation, audit — generated. - The same source of truth, all the way down. + Your workspace, ready when you are.

@@ -159,7 +144,7 @@ export function LoginPage() { color: "var(--app-fg-1)", }} > - Sign in to {{ project_name }} + Sign in to {SITE_NAME}

TempDir { + let dir = TempDir::new().unwrap(); + fs::create_dir(dir.path().join("schemas")).unwrap(); + fs::write( + dir.path().join("schemas/test.schema"), + "schema Invoice { number: text required }", + ) + .unwrap(); + dir +} + +fn generate(root: &Path) -> Command { + let mut command = cargo_bin_cmd!("schemaforge"); + command + .current_dir(root) + .args(["site", "generate", "-s", "schemas", "-o", "site"]); + command +} + +fn read(root: &Path, file: &str) -> String { + fs::read_to_string(root.join("site").join(file)).unwrap() +} + +#[test] +fn configured_branding_survives_generation_and_check() { + let dir = fixture(); + let root = dir.path(); + fs::create_dir(root.join("branding")).unwrap(); + for (name, color) in [("logo", "red"), ("dark", "white"), ("icon", "blue")] { + fs::write(root.join(format!("branding/{name}.svg")), format!(r#""#)).unwrap(); + } + fs::write( + root.join("config.toml"), + r#" +[schema_forge.site] +name = 'Acme "Operations" & ' +title_suffix = "" +logo = "branding/logo.svg" +logo_on_dark = "branding/dark.svg" +favicon = "branding/icon.svg" +"#, + ) + .unwrap(); + generate(root).assert().success(); + let branding = read(root, "src/lib/branding.ts"); + assert!(branding.contains(r#""Acme \"Operations\" & ""#)); + assert!(branding.contains("TITLE_SUFFIX: string = \"\"")); + let html = read(root, "index.html"); + assert!(html.contains("<Review>")); + assert!(html.contains("/favicon.svg")); + for (output, color) in [ + ("logo-mark", "red"), + ("logo-mark-white", "white"), + ("favicon", "blue"), + ] { + assert!(read(root, &format!("public/{output}.svg")).contains(&format!("fill=\"{color}\""))); + } + generate(root).arg("--check").assert().success(); + generate(root) + .args(["--name", "Other Product", "--title-suffix", "Acme"]) + .assert() + .success(); + assert!(read(root, "src/lib/branding.ts").contains("Other Product")); + assert!(read(root, "src/pages/login.tsx").contains("{SITE_NAME}")); + generate(root) + .args([ + "--name", + "Other Product", + "--title-suffix", + "Acme", + "--check", + ]) + .assert() + .success(); + fs::write(root.join("site/public/logo-mark.svg"), "changed").unwrap(); + generate(root).arg("--check").assert().failure(); +} + +#[test] +fn vendor_outputs_accept_template_overrides_and_defaults_are_neutral() { + let dir = fixture(); + let root = dir.path(); + generate(root).assert().success(); + for file in [ + "public/logo-mark.svg", + "public/logo-mark-white.svg", + "src/index.css", + "src/pages/login.tsx", + "src/pages/accessibility.tsx", + "src/lib/use-document-title.ts", + ] { + let contents = read(root, file); + assert!(!contents.contains("Govcraft"), "{file}"); + assert!(!contents.contains("· SchemaForge"), "{file}"); + } + let overrides = root.join("custom"); + for (file, content) in [ + ("public/logo-mark-white.svg", "custom dark mark"), + ("public/favicon.svg", "custom favicon"), + ( + "src/index.css", + "/* custom styles for {{ branding.name }} */", + ), + ("src/lib/use-document-title.ts", "// custom title hook"), + ] { + let path = overrides.join(format!("{file}.jinja")); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, content).unwrap(); + } + generate(root) + .args(["--templates-dir", "custom", "--name", "Acme"]) + .assert() + .success(); + assert!(read(root, "public/logo-mark-white.svg").contains("custom dark mark")); + assert!(read(root, "public/favicon.svg").contains("custom favicon")); + assert!(read(root, "src/index.css").contains("custom styles for Acme")); + assert!(read(root, "src/lib/use-document-title.ts").contains("custom title hook")); + generate(root) + .args(["--templates-dir", "custom", "--name", "Acme", "--check"]) + .assert() + .success(); +} + +#[test] +fn tenant_root_supplies_name_independent_of_output_directory() { + let dir = fixture(); + fs::write( + dir.path().join("schemas/test.schema"), + "@tenant(root) schema AcmeOrganization { name: text }", + ) + .unwrap(); + generate(dir.path()).assert().success(); + assert!(read(dir.path(), "src/lib/branding.ts").contains("Acme Organization")); +} + +#[test] +fn unsupported_assets_and_blank_names_fail_clearly() { + let dir = fixture(); + fs::write(dir.path().join("logo.png"), b"PNG").unwrap(); + generate(dir.path()) + .args(["--logo", "logo.png"]) + .assert() + .failure() + .stderr(predicates::str::contains("must be an SVG file")); + generate(dir.path()) + .args(["--name", " "]) + .assert() + .failure() + .stderr(predicates::str::contains("site name must be nonempty")); +} + +#[test] +fn config_relative_assets_and_flag_overrides_use_their_own_bases() { + let dir = fixture(); + let root = dir.path(); + fs::create_dir(root.join("settings")).unwrap(); + fs::write( + root.join("settings/config.toml"), + "[schema_forge.site]\nname = 'Configured'\nlogo = 'brand.svg'\n", + ) + .unwrap(); + fs::write( + root.join("settings/brand.svg"), + "configured mark", + ) + .unwrap(); + fs::write(root.join("override.svg"), "flag mark").unwrap(); + generate(root) + .args(["--config", "settings/config.toml"]) + .assert() + .success(); + assert!(read(root, "public/logo-mark.svg").contains("configured mark")); + generate(root) + .args(["--config", "settings/config.toml", "--logo", "override.svg"]) + .assert() + .success(); + assert!(read(root, "public/logo-mark.svg").contains("flag mark")); + assert!(read(root, "public/logo-mark-white.svg").contains("flag mark")); + assert!(read(root, "public/favicon.svg").contains("flag mark")); +} diff --git a/crates/schema-forge-cli/tests/site_e2e/playwright/tests/smoke.spec.ts b/crates/schema-forge-cli/tests/site_e2e/playwright/tests/smoke.spec.ts index bed404c..280389b 100644 --- a/crates/schema-forge-cli/tests/site_e2e/playwright/tests/smoke.spec.ts +++ b/crates/schema-forge-cli/tests/site_e2e/playwright/tests/smoke.spec.ts @@ -48,3 +48,14 @@ test("create → detail round-trip on Company", async ({ page }) => { page.getByRole("heading", { name: "Playwright Test Co" }), ).toBeVisible() }) + + +test("project branding renders safely in titles and navigation", async ({ page }) => { + await page.goto("/login") + await expect(page).toHaveTitle("Sign in · Workspace") + await expect(page.getByRole("heading", { name: 'Sign in to Acme "Operations" & ' })).toBeVisible() + await login(page) + await expect(page.locator(".rail-title")).toHaveText('Acme "Operations" & ') + await expect(page).toHaveTitle(/ · Workspace$/) + await expect(page.locator('link[rel="icon"]')).toHaveAttribute("href", "/favicon.svg") +}) diff --git a/crates/schema-forge-cli/tests/site_e2e/run.sh b/crates/schema-forge-cli/tests/site_e2e/run.sh index 8d5809f..6c3cdba 100755 --- a/crates/schema-forge-cli/tests/site_e2e/run.sh +++ b/crates/schema-forge-cli/tests/site_e2e/run.sh @@ -43,7 +43,9 @@ cargo build --package schema-forge-cli --bin schemaforge --quiet # ---------- generate the site ---------- ./target/debug/schemaforge site generate \ --schema-dir "$SCHEMAS_DIR" \ - --out-dir "$SITE_DIR" + --out-dir "$SITE_DIR" \ + --name 'Acme "Operations" & ' \ + --title-suffix 'Workspace' # ---------- backend ---------- BACKEND_LOG="$TMP_ROOT/backend.log" diff --git a/docs/site-branding.md b/docs/site-branding.md new file mode 100644 index 0000000..1d93a19 --- /dev/null +++ b/docs/site-branding.md @@ -0,0 +1,69 @@ +# Generated-site branding + +Set product identity in your project configuration: + +```toml +[schema_forge.site] +name = "Acme Operations" +# Defaults to name. Set to "" to omit the page-title suffix. +title_suffix = "Acme Operations" +logo = "branding/logo.svg" +logo_on_dark = "branding/logo-white.svg" +favicon = "branding/favicon.svg" +``` + +Generate and verify using the same configuration: + +```sh +schemaforge --config config.toml site generate -s schemas -o site +schemaforge --config config.toml site generate -s schemas -o site --check +``` + +`site generate` accepts matching `--name`, `--title-suffix`, `--logo`, +`--logo-on-dark`, and `--favicon` flags. Flags override configuration values. +Configured asset paths are relative to the explicit `--config` file's directory; +with automatically discovered configuration, paths are relative to the working +directory. Asset flag paths are always relative to the working directory. +Only UTF-8 SVG assets are supported. PNG, JPEG, ICO, and other extensions are +rejected with an error. Configured SVG content is copied into managed files in +`public/`, so it is included in regeneration and drift checks. + +The name appears in the navigation rail, login panel, browser title, and +accessibility statement. The document-title suffix defaults to the name; +`--title-suffix ''` disables it. Theme preferences use a key derived from the +name. Renaming a product therefore starts with its default theme preference. +The package name uses a separate safe lowercase slug. + +If no name is configured, the first tenant-root schema supplies it. Otherwise, +the generator uses the schema directory's parent name, falling back to +`Application`. The output directory's basename does not determine the name. +Default marks are neutral geometric placeholders. A configured `logo` supplies +the dark mark and favicon too, unless those are explicitly configured. + +## Template overrides + +`--templates-dir` also supports these previously vendored outputs: + +- `public/logo-mark.svg.jinja` +- `public/logo-mark-white.svg.jinja` +- `public/favicon.svg.jinja` +- `src/index.css.jinja` +- `src/lib/use-document-title.ts.jinja` + +Overrides can access `branding.name`. Assets explicitly selected through config +or flags take precedence over asset templates. An unreadable template override +fails generation. Overrides remain part of the expected output for `--check`. +Use an override to customize CSS or the accessibility conformance statement. + +## Existing generated sites + +Brand constants, CSS, marks, title helpers, the app shell, and accessibility page +are owned outputs and update on regeneration. The login page and per-entity +page shells are preserved user files. Newly scaffolded login pages import the +owned `src/lib/branding.ts`, so future branding changes require no page rewrite. + +For an existing login page, update its product copy to use the exported +`SITE_NAME` and retain `/logo-mark-white.svg` as its mark. Alternatively, back up +customizations and regenerate with `--force-user-files`; this replaces **all** +preserved page shells, not just login. This one-time migration removes the old +hard-coded vendor copy without silently overwriting user customization. From 534e0d48442d9c8a650c4ad880d3bf674bcf4d9e Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 13:41:41 -0600 Subject: [PATCH 20/38] docs: remove organization branding from page template guidance --- .../templates/site/src/app/pages/detail.tsx.jinja | 2 +- .../templates/site/src/app/pages/edit.tsx.jinja | 2 +- .../templates/site/src/app/pages/list.tsx.jinja | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/schema-forge-cli/templates/site/src/app/pages/detail.tsx.jinja b/crates/schema-forge-cli/templates/site/src/app/pages/detail.tsx.jinja index 49809f5..4574a66 100644 --- a/crates/schema-forge-cli/templates/site/src/app/pages/detail.tsx.jinja +++ b/crates/schema-forge-cli/templates/site/src/app/pages/detail.tsx.jinja @@ -7,7 +7,7 @@ // through automatically while your layout and surrounding widgets stay // put in this preserve file. // -// Layout follows the Govcraft "spec sheet": a 56px § margin column +// Layout follows the "spec sheet": a 56px § margin column // alongside mono uppercase labels and the rendered value, with hairline // row dividers. Edit/Back actions live in the page header. import { Link, useParams } from "react-router-dom" diff --git a/crates/schema-forge-cli/templates/site/src/app/pages/edit.tsx.jinja b/crates/schema-forge-cli/templates/site/src/app/pages/edit.tsx.jinja index 8017c56..51821e4 100644 --- a/crates/schema-forge-cli/templates/site/src/app/pages/edit.tsx.jinja +++ b/crates/schema-forge-cli/templates/site/src/app/pages/edit.tsx.jinja @@ -7,7 +7,7 @@ // changes flow through automatically while your layout, section headers, // and mutation intercepts stay put in this preserve file. // -// Layout follows the Govcraft form-grid: 220px mono uppercase labels in a +// Layout follows the form grid: 220px mono uppercase labels in a // left column, fields in a right column, accent primary button. import { useMemo } from "react" import { Link, useNavigate, useParams } from "react-router-dom" diff --git a/crates/schema-forge-cli/templates/site/src/app/pages/list.tsx.jinja b/crates/schema-forge-cli/templates/site/src/app/pages/list.tsx.jinja index 6fbffff..cf5ab94 100644 --- a/crates/schema-forge-cli/templates/site/src/app/pages/list.tsx.jinja +++ b/crates/schema-forge-cli/templates/site/src/app/pages/list.tsx.jinja @@ -7,7 +7,7 @@ // schema changes while keeping your layout, charts, and custom state in // this preserve file. // -// Layout follows the Govcraft DS: eyebrow + page title, mono toolbar with +// Layout follows the site design system: eyebrow + page title, mono toolbar with // field-targeted contains filter, compact 32px-row table with mono // uppercase headers + signal-orange sort arrows, hairline pager. import { useEffect, useMemo, useState } from "react" From ba29c48512f19893eb583f1def349c7d8786c775 Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 13:40:10 -0600 Subject: [PATCH 21/38] test(site): keep mocked session expiry within browser timer bounds --- .../tests/site_e2e/playwright/tests/client-errors.spec.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/schema-forge-cli/tests/site_e2e/playwright/tests/client-errors.spec.ts b/crates/schema-forge-cli/tests/site_e2e/playwright/tests/client-errors.spec.ts index 75fc840..35b5c6f 100644 --- a/crates/schema-forge-cli/tests/site_e2e/playwright/tests/client-errors.spec.ts +++ b/crates/schema-forge-cli/tests/site_e2e/playwright/tests/client-errors.spec.ts @@ -2,8 +2,9 @@ import { expect, test, type Page, type Route } from "@playwright/test" async function session(page: Page, entityResponse?: (route: Route) => Promise) { await page.addInitScript(() => { + // Stay below the browser timer limit so background refresh does not fire during setup. sessionStorage.setItem("schemaforge.token", "test-token") - sessionStorage.setItem("schemaforge.token_expires_at", "2099-01-01T00:00:00Z") + sessionStorage.setItem("schemaforge.token_expires_at", new Date(Date.now() + 60 * 60 * 1000).toISOString()) sessionStorage.setItem("schemaforge.roles", JSON.stringify(["member"])) if (!sessionStorage.getItem("schemaforge.active_tenant")) { sessionStorage.setItem("schemaforge.active_tenant", "Org:org-a") @@ -44,7 +45,7 @@ test("request headers preserve explicit tenant overrides and refresh retries", a await page.goto("/login") let retries = 0 await page.route("**/api/v1/forge/auth/refresh", route => route.fulfill({ json: { - token: "new-token", expires_at: "2099-01-01T00:00:00Z", roles: ["member"], + token: "new-token", expires_at: new Date(Date.now() + 60 * 60 * 1000).toISOString(), roles: ["member"], } })) await page.route("**/api/v1/forge/header-test**", async route => { if (route.request().url().endsWith("/retry") && retries++ === 0) { @@ -57,7 +58,7 @@ test("request headers preserve explicit tenant overrides and refresh retries", a const apiPath = "/src/generated/api-client.ts" const auth = await import(authPath) const api = await import(apiPath) - auth.tokenStore.set("old-token", "2099-01-01T00:00:00Z", ["member"]) + auth.tokenStore.set("old-token", new Date(Date.now() + 60 * 60 * 1000).toISOString(), ["member"]) auth.activeTenantStore.set("Org", "org-a") const result = [] for (const method of ["GET", "POST", "PATCH", "DELETE"]) { From 7982ee41c62791e2bf73d1f2a32b38fd8b26b804 Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 13:42:23 -0600 Subject: [PATCH 22/38] fix(site): authenticate proxied attachment downloads --- .../src/commands/site/vendor.rs | 69 +++++++++-------- .../tests/attachment-download.spec.ts | 74 +++++++++++++++++++ 2 files changed, 113 insertions(+), 30 deletions(-) create mode 100644 crates/schema-forge-cli/tests/site_e2e/playwright/tests/attachment-download.spec.ts diff --git a/crates/schema-forge-cli/src/commands/site/vendor.rs b/crates/schema-forge-cli/src/commands/site/vendor.rs index 3d46f6a..460b849 100644 --- a/crates/schema-forge-cli/src/commands/site/vendor.rs +++ b/crates/schema-forge-cli/src/commands/site/vendor.rs @@ -1934,8 +1934,8 @@ type AttachmentDownloadProps = { * short-TTL URL, then opens it in a new tab. Avoids the auto-302 path so * the bearer token can be attached to the metadata fetch (browsers strip * `Authorization` across cross-origin redirects to S3). - * * `proxied` — links the field endpoint directly. The runtime streams - * bytes through the daemon with auth applied; no extra round trip needed. + * * `proxied` — fetches bytes with session and tenant headers, then starts + * a browser download from a temporary object URL. */ export function AttachmentDownload({ schema, @@ -1954,16 +1954,32 @@ export function AttachmentDownload({ schema, )}/entities/${encodeURIComponent(entityId)}/fields/${encodeURIComponent(fieldName)}` - async function openPresigned(e: React.MouseEvent) { + async function openAttachment(e: React.MouseEvent) { e.preventDefault() if (!available || busy) return setBusy(true) setErr(null) try { - const res = await fetch(`${fieldUrl}?redirect=false`, { headers: authHeaders() }) - if (!res.ok) throw new Error(`download failed: ${res.status}`) - const body = (await res.json()) as { url: string } - window.open(body.url, "_blank", "noopener,noreferrer") + const url = access === "proxied" ? fieldUrl : `${fieldUrl}?redirect=false` + const res = await fetch(url, { headers: authHeaders() }) + if (!res.ok) throw new Error(`Unable to download this file (HTTP ${res.status}).`) + if (access === "proxied") { + const blobUrl = URL.createObjectURL(await res.blob()) + const link = document.createElement("a") + link.href = blobUrl + link.download = downloadFilename(res.headers.get("Content-Disposition"), filename) + try { + document.body.appendChild(link) + link.click() + } finally { + link.remove() + // Give the browser time to start consuming the blob before releasing it. + window.setTimeout(() => URL.revokeObjectURL(blobUrl), 1000) + } + } else { + const body = (await res.json()) as { url: string } + window.open(body.url, "_blank", "noopener,noreferrer") + } } catch (e2) { setErr(e2 instanceof Error ? e2.message : String(e2)) } finally { @@ -1971,38 +1987,18 @@ export function AttachmentDownload({ } } - // Proxied access: link the field endpoint directly. The runtime applies - // auth via the bearer cookie / header chain on the daemon side and streams - // the response. - if (access === "proxied") { - return ( - - {compact ? "Download" : `Download ${filename}`} - (opens in a new tab) - - ) - } - - // Presigned access: round-trip through ?redirect=false so we can attach - // the bearer token on the metadata fetch. + // Both modes need a fetch first so tenant selection and bearer auth apply. return ( {err ? ( @@ -2014,6 +2010,19 @@ export function AttachmentDownload({ ) } +function downloadFilename(disposition: string | null, fallback: string): string { + const extended = disposition?.match(/filename\*=UTF-8''([^;]+)/i)?.[1] + let name: string | undefined + if (extended) { + try { name = decodeURIComponent(extended.trim()) } catch { /* Use the plain filename. */ } + } + if (!name) { + const plain = disposition?.match(/filename\s*=\s*(?:"((?:\\.|[^"])*)"|([^;]+))/i) + name = plain?.[1]?.replace(/\\(.)/g, "$1") ?? plain?.[2] + } + return name?.split(/[/\\]/).pop()?.trim() || fallback +} + function attachmentLabel(a: FileAttachment): string { const tail = a.key.split("/").pop() || a.key return `${tail} (${formatBytes(a.size)})` diff --git a/crates/schema-forge-cli/tests/site_e2e/playwright/tests/attachment-download.spec.ts b/crates/schema-forge-cli/tests/site_e2e/playwright/tests/attachment-download.spec.ts new file mode 100644 index 0000000..9f5eb4e --- /dev/null +++ b/crates/schema-forge-cli/tests/site_e2e/playwright/tests/attachment-download.spec.ts @@ -0,0 +1,74 @@ +import { expect, test, type Page } from "@playwright/test" +import { readFile } from "node:fs/promises" + +async function mountDownload(page: Page) { + await page.goto("/login") + await page.evaluate(async () => { + // The login page has loaded these Vite-prebundled React modules already. + const reactPath = "/node_modules/.vite/deps/react.js" + const domPath = "/node_modules/.vite/deps/react-dom_client.js" + const componentPath = "/src/components/ui/file-upload.tsx" + const authPath = "/src/lib/auth.ts" + const React = await import(reactPath) + const ReactDOM = await import(domPath) + const { AttachmentDownload } = await import(componentPath) + const auth = await import(authPath) + auth.tokenStore.set("download-token", new Date(Date.now() + 3600000).toISOString(), ["member"]) + auth.activeTenantStore.set("Org", "org-selected") + const host = document.createElement("div") + host.id = "attachment-fixture" + document.body.appendChild(host) + const originalCreate = URL.createObjectURL.bind(URL) + const originalRevoke = URL.revokeObjectURL.bind(URL) + URL.createObjectURL = (blob: Blob | MediaSource) => { + const url = originalCreate(blob) + host.dataset.createdBlob = url + return url + } + URL.revokeObjectURL = (url: string) => { + host.dataset.revokedBlob = url + originalRevoke(url) + } + ReactDOM.createRoot(host).render(React.createElement(AttachmentDownload, { + schema: "Company", entityId: "company_fixture", fieldName: "document", access: "proxied", + attachment: { key: "company_fixture/original.txt", size: 16, mime: "text/plain", status: "available" }, + })) + }) +} + +for (const [disposition, expected] of [ + ["attachment; filename*=UTF-8''quarterly%20r%C3%A9sum%C3%A9.txt", "quarterly résumé.txt"], + ['attachment; filename="report.txt"', "report.txt"], + ["", "original.txt"], +]) { + test(`proxied download carries session context and saves ${expected}`, async ({ page }) => { + await page.route("**/api/v1/forge/schemas/Company/entities/company_fixture/fields/document", route => { + expect(route.request().headers().authorization).toBe("Bearer download-token") + expect(route.request().headers()["x-active-tenant"]).toBe("Org:org-selected") + return route.fulfill({ status: 200, contentType: "text/plain", body: "private contents", headers: disposition ? { "content-disposition": disposition } : {} }) + }) + await mountDownload(page) + const pending = page.waitForEvent("download") + await page.locator("#attachment-fixture").getByRole("button", { name: /Download/ }).click() + const download = await pending + expect(download.suggestedFilename()).toBe(expected) + const path = await download.path() + expect(path).not.toBeNull() + expect(await readFile(path!, "utf8")).toBe("private contents") + await expect.poll(() => page.locator("#attachment-fixture").evaluate(element => { + const host = element as HTMLElement + return Boolean(host.dataset.createdBlob) && host.dataset.createdBlob === host.dataset.revokedBlob + })).toBe(true) + await expect(page.locator('a[href^="blob:"]')).toHaveCount(0) + }) +} + +test("proxied download failure shows an error and allows retry", async ({ page }) => { + await page.route("**/api/v1/forge/schemas/Company/entities/company_fixture/fields/document", route => route.fulfill({ status: 403, json: { error: "forbidden" } })) + await mountDownload(page) + const fixture = page.locator("#attachment-fixture") + await fixture.getByRole("button", { name: /Download/ }).click() + await expect(fixture.getByRole("alert")).toContainText("Unable to download this file (HTTP 403).") + await expect(fixture.getByRole("button", { name: /Download/ })).toBeEnabled() + await expect(fixture).not.toHaveAttribute("data-created-blob") +}) From 907a814ebba37395f7127c70799cb3c0cbee6538 Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 13:42:40 -0600 Subject: [PATCH 23/38] test(site): load Vite React default exports in download fixture --- .../site_e2e/playwright/tests/attachment-download.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/schema-forge-cli/tests/site_e2e/playwright/tests/attachment-download.spec.ts b/crates/schema-forge-cli/tests/site_e2e/playwright/tests/attachment-download.spec.ts index 9f5eb4e..0fac565 100644 --- a/crates/schema-forge-cli/tests/site_e2e/playwright/tests/attachment-download.spec.ts +++ b/crates/schema-forge-cli/tests/site_e2e/playwright/tests/attachment-download.spec.ts @@ -9,8 +9,8 @@ async function mountDownload(page: Page) { const domPath = "/node_modules/.vite/deps/react-dom_client.js" const componentPath = "/src/components/ui/file-upload.tsx" const authPath = "/src/lib/auth.ts" - const React = await import(reactPath) - const ReactDOM = await import(domPath) + const React = (await import(reactPath)).default + const ReactDOM = (await import(domPath)).default const { AttachmentDownload } = await import(componentPath) const auth = await import(authPath) auth.tokenStore.set("download-token", new Date(Date.now() + 3600000).toISOString(), ["member"]) From 4b96b9a2ee907cfd6fc69fc46fae999ecd31c5d7 Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Fri, 25 Sep 2026 13:43:49 -0600 Subject: [PATCH 24/38] fix(site): generate typed fields and respect form write authority --- .Codex/plans/site-field-authority.md | 11 + .../ui-design-review-site-field-authority.md | 3 + .../src/commands/site/context.rs | 54 +++- .../src/commands/site/mapping.rs | 163 +++++++++- .../src/app/pages/edit.generated.tsx.jinja | 278 ++++-------------- .../site/src/generated/formatters.ts.jinja | 31 +- .../site/src/generated/zod-schemas.ts.jinja | 120 +++++++- .../tests/site_e2e/demo.schema | 21 ++ .../site_e2e/playwright/tests/fields.spec.ts | 101 +++++++ .../schema-forge-cli/tests/site_generate.rs | 130 ++++++-- docs/site-guide.md | 32 +- 11 files changed, 681 insertions(+), 263 deletions(-) create mode 100644 .Codex/plans/site-field-authority.md create mode 100644 .Codex/ui-design-review-site-field-authority.md create mode 100644 crates/schema-forge-cli/tests/site_e2e/playwright/tests/fields.spec.ts diff --git a/.Codex/plans/site-field-authority.md b/.Codex/plans/site-field-authority.md new file mode 100644 index 0000000..720b3c4 --- /dev/null +++ b/.Codex/plans/site-field-authority.md @@ -0,0 +1,11 @@ +# Generated field types and authority, issues 191 and 192 + +Use the existing view-model and template architecture. Keep display types complete while filtering form authority separately. + +- Add duration (Go-style duration string validation), bytes (base64 text with decoded size constraint), and map (typed Record JSON textarea) mapping. Preserve recursive composite/array type projection. Format duration wire strings into readable units without altering submitted values. +- Project computed/read-role/write-role metadata on FieldView. Omit computed and derived fields from form controls and form validators. Use current auth roles at render/parse/submit time, not module initialization. Hide read-denied controls, render write-denied values inert, and ensure validation does not require denied fields. +- Use recursive metadata to filter initial and submitted state, stripping read-denied, computed/derived and unwritable payload fields including nested composites. Preserve readable, write-denied initial values for read-only display. Normalize JSON and composite values recursively so nested supported fields remain round-trippable. +- Update field-type and permissions documentation. Add generator regression checks plus Playwright behavior tests for serialization, validation and role changes. Fail generation for any remaining unsupported required field. +- Preserve existing generated styling and accessibility labels, avoiding controls which imply unavailable actions (UI design expert, interaction patterns). + +No new dependencies or error types required. Semver: fixes in release already planned by root. Run only targeted cargo check locally; root runs generation, TypeScript build/lint, and browser checks in CI. Sign conventional commits; no push. diff --git a/.Codex/ui-design-review-site-field-authority.md b/.Codex/ui-design-review-site-field-authority.md new file mode 100644 index 0000000..2ada17c --- /dev/null +++ b/.Codex/ui-design-review-site-field-authority.md @@ -0,0 +1,3 @@ +# UI Design Review + +The generated forms imply users can edit values the server will discard. Apply Norman's affordance principle: omit computed inputs, hide unreadable fields, and render readable but unwritable values as inert text. Share gating across validation and payload construction so hidden required controls cannot block saving. Keep existing form styling and labels; provide duration and base64 format hints. Browser regression coverage should assert visible controls and actual submitted payloads for both permitted and denied roles. diff --git a/crates/schema-forge-cli/src/commands/site/context.rs b/crates/schema-forge-cli/src/commands/site/context.rs index 00e3c00..e7e3a6f 100644 --- a/crates/schema-forge-cli/src/commands/site/context.rs +++ b/crates/schema-forge-cli/src/commands/site/context.rs @@ -162,6 +162,10 @@ pub struct EntityView { /// components when the entity actually has a file field, otherwise /// `noUnusedLocals` rejects the generated file. pub has_file_field: bool, + /// Writable file controls require an entity identifier. + pub has_form_file_field: bool, + pub has_form_fields: bool, + pub has_form_controls: bool, } impl EntityView { @@ -180,8 +184,11 @@ impl EntityView { match field_to_view(f, catalog) { Ok(v) => fields.push(v), Err(FieldMapError::Unsupported { field, reason }) => { + if f.is_required() { + return Err(CliError::Config { message: format!("cannot generate a usable create form: required field {name}.{field} is unsupported: {reason}") }); + } output.warn(&format!( - "site v0: skipping field `{name}.{field}` — {reason}" + "site: skipping optional field `{name}.{field}`: {reason}" )); } } @@ -193,6 +200,7 @@ impl EntityView { }); let has_json_field = fields.iter().any(|f| f.kind == "json"); let has_file_field = fields.iter().any(|f| f.kind == "file"); + let has_form_file_field = fields.iter().any(has_form_file); let display_field = def.display_field().map(|s| s.to_string()); // `@display("field")` auto-promotes to `primary` when no explicit @@ -208,6 +216,8 @@ impl EntityView { } } + let has_form_fields = fields.iter().any(|field| !field.computed && !field.derived); + let has_form_controls = fields.iter().any(has_form_control); let pascal = name.to_pascal_case(); Ok(Self { pascal_plural: pluralize(&pascal), @@ -222,14 +232,31 @@ impl EntityView { has_relation_link, has_json_field, has_file_field, + has_form_file_field, + has_form_fields, + has_form_controls, }) } } +fn has_form_file(field: &FieldView) -> bool { + !field.computed + && !field.derived + && (field.kind == "file" || field.sub_fields.iter().any(has_form_file)) +} + +fn has_form_control(field: &FieldView) -> bool { + !field.computed + && !field.derived + && (field.kind != "composite" || field.sub_fields.iter().any(has_form_control)) +} + /// Recursive check: does this field or any nested composite sub-field /// contain a `relation_one`? fn has_relation_one_field(f: &FieldView) -> bool { - f.kind == "relation_one" || f.sub_fields.iter().any(has_relation_one_field) + !f.computed + && !f.derived + && (f.kind == "relation_one" || f.sub_fields.iter().any(has_relation_one_field)) } /// One schema field projected into a TS/Zod-aware view model. @@ -305,6 +332,11 @@ pub struct FieldView { /// the detail view. Reads already flow through the standard relation /// envelope, populated by the backend's inverse-collection pass. pub derived: bool, + /// Server-computed values are read-only and excluded from forms. + pub computed: bool, + /// Declarative role hints; Cedar remains authoritative. + pub read_roles: Vec, + pub write_roles: Vec, /// For `kind == "file"`: metadata the template needs to render the /// upload widget (accept attribute, max-size guard, proxied vs. presigned /// behavior). `None` for non-file fields. @@ -391,6 +423,24 @@ pub fn make_field_view( None => default_list_placement(kind).to_string(), }, derived: field.is_derived(), + computed: field.annotations.iter().any(|annotation| { + matches!( + annotation, + schema_forge_core::types::FieldAnnotation::Compute { .. } + ) + }), + read_roles: match field.field_access() { + Some(schema_forge_core::types::FieldAnnotation::FieldAccess { read, .. }) => { + read.clone() + } + _ => Vec::new(), + }, + write_roles: match field.field_access() { + Some(schema_forge_core::types::FieldAnnotation::FieldAccess { write, .. }) => { + write.clone() + } + _ => Vec::new(), + }, } } diff --git a/crates/schema-forge-cli/src/commands/site/mapping.rs b/crates/schema-forge-cli/src/commands/site/mapping.rs index 8c1adcf..c7b8d94 100644 --- a/crates/schema-forge-cli/src/commands/site/mapping.rs +++ b/crates/schema-forge-cli/src/commands/site/mapping.rs @@ -64,6 +64,47 @@ fn field_to_view_with_prefix( Vec::new(), )) } + FieldType::Duration => { + let zod = optional_form_zod("durationSchema".into(), required); + Ok(make_field_view( + field, + "string".into(), + zod, + "duration", + false, + None, + Vec::new(), + )) + } + FieldType::Bytes(constraints) => { + let mut zod = "base64Schema".to_string(); + if let Some(max) = constraints.max_size { + zod.push_str(&format!( + ".refine(value => decodedBase64Size(value) <= {max}, \"Maximum {max} bytes\")" + )); + } + Ok(make_field_view( + field, + "string".into(), + optional_form_zod(zod, required), + "bytes", + false, + None, + Vec::new(), + )) + } + FieldType::Map { value, .. } => { + let validator = format!("jsonTextSchema(z.record({}))", wire_zod(value)); + Ok(make_field_view( + field, + format!("Record", ts_type_for_field_type(value)), + optional_form_zod(validator, required), + "json", + false, + None, + Vec::new(), + )) + } FieldType::Integer(_) => { let mut zod = "z.coerce.number().int()".to_string(); if !required { @@ -295,7 +336,16 @@ fn field_to_view_with_prefix( for sv in &sub_fields { let opt = if sv.required { "" } else { "?" }; ts_parts.push(format!("{}{}: {}", sv.leaf, opt, sv.ts_type)); - zod_parts.push(format!("{}: {}", sv.leaf, sv.zod)); + if !sv.derived && !sv.computed { + zod_parts.push(format!( + "{}: formFieldSchema({}, {}, {}, {} as FormFieldSpec[])", + sv.leaf, + sv.zod, + serde_json::to_string(&sv.read_roles).unwrap_or_default(), + serde_json::to_string(&sv.write_roles).unwrap_or_default(), + serde_json::to_string(&sv.sub_fields).unwrap_or_default() + )); + } } let ts_type = format!("{{ {} }}", ts_parts.join(", ")); let mut zod = format!("z.object({{ {} }})", zod_parts.join(", ")); @@ -341,8 +391,7 @@ fn field_to_view_with_prefix( mime_allowlist, access: constraints.access.as_str().to_string(), }; - let mut view = - make_field_view(field, ts_type, zod, "file", false, None, Vec::new()); + let mut view = make_field_view(field, ts_type, zod, "file", false, None, Vec::new()); view.file_meta = Some(file_meta); Ok(view) } @@ -359,6 +408,52 @@ fn field_to_view_with_prefix( }) } +/// Optional text controls treat a blank input as absent before validation. +fn optional_form_zod(zod: String, required: bool) -> String { + if required { + zod + } else { + format!("z.preprocess(value => value === \"\" ? undefined : value, {zod}.nullish())") + } +} + +/// JSON wire validators used for homogeneous map values. +fn wire_zod(field_type: &FieldType) -> String { + match field_type { + FieldType::Text(_) + | FieldType::RichText + | FieldType::DateTime + | FieldType::Relation { .. } => "z.string()".into(), + FieldType::Duration => "durationSchema".into(), + FieldType::Bytes(_) => "base64Schema".into(), + FieldType::Integer(_) => "z.number().int()".into(), + FieldType::Float(_) => "z.number()".into(), + FieldType::Boolean => "z.boolean()".into(), + FieldType::Enum(variants) => format!( + "z.enum({} as [string, ...string[]])", + serde_json::to_string(variants.as_slice()).unwrap_or_default() + ), + FieldType::Array(inner) => format!("z.array({})", wire_zod(inner)), + FieldType::Map { value, .. } => format!("z.record({})", wire_zod(value)), + FieldType::Composite(fields) => { + let entries: Vec<_> = fields + .iter() + .filter(|field| !field.is_hidden()) + .map(|field| { + let optional = if field.is_required() { + "" + } else { + ".nullish()" + }; + format!("{}: {}{optional}", field.name, wire_zod(&field.field_type)) + }) + .collect(); + format!("z.object({{ {} }})", entries.join(", ")) + } + _ => "z.unknown()".into(), + } +} + /// Format a byte count as a short human-readable string for template labels. /// /// Uses base-1024 (matching the DSL parser) and always drops unnecessary @@ -414,17 +509,23 @@ fn with_relation_metadata( /// `unknown` since the JSON wire shape is opaque to the generator. fn ts_type_for_field_type(ft: &FieldType) -> String { match ft { - FieldType::Text(_) | FieldType::RichText | FieldType::DateTime => "string".to_string(), + FieldType::Text(_) + | FieldType::RichText + | FieldType::DateTime + | FieldType::Duration + | FieldType::Bytes(_) => "string".to_string(), FieldType::Integer(_) | FieldType::Float(_) => "number".to_string(), FieldType::Boolean => "boolean".to_string(), FieldType::Enum(v) => { - let parts: Vec = - v.as_slice().iter().map(|s| format!("\"{s}\"")).collect(); + let parts: Vec = v.as_slice().iter().map(|s| format!("\"{s}\"")).collect(); format!("({})", parts.join(" | ")) } FieldType::Json => "unknown".to_string(), FieldType::Relation { .. } => "string".to_string(), FieldType::Array(inner) => format!("{}[]", ts_type_for_field_type(inner)), + FieldType::Map { value, .. } => { + format!("Record", ts_type_for_field_type(value)) + } FieldType::Composite(sub_defs) => { let mut parts = Vec::with_capacity(sub_defs.len()); for sub in sub_defs { @@ -508,6 +609,54 @@ mod tests { field_to_view(field, &empty_catalog()) } + #[test] + fn duration_bytes_and_maps_have_complete_form_and_read_types() { + use schema_forge_core::types::BytesConstraints; + let duration = project(&field("timeout", FieldType::Duration, true)).unwrap(); + assert_eq!(duration.kind, "duration"); + assert_eq!(duration.ts_type, "string"); + assert_eq!(duration.zod, "durationSchema"); + let bytes = project(&field( + "checksum", + FieldType::Bytes(BytesConstraints::with_max_size(32)), + true, + )) + .unwrap(); + assert_eq!(bytes.kind, "bytes"); + assert_eq!(bytes.ts_type, "string"); + assert!(bytes.zod.contains("decodedBase64Size(value) <= 32")); + let schemas = schema_forge_dsl::parse("schema Job { labels: map }").unwrap(); + let map = project(&schemas[0].fields[0]).unwrap(); + assert_eq!(map.kind, "json"); + assert_eq!(map.ts_type, "Record"); + assert!(map + .zod + .contains("jsonTextSchema(z.record(z.number().int()))")); + let array = project(&field( + "delays", + FieldType::Array(Box::new(FieldType::Duration)), + true, + )) + .unwrap(); + assert_eq!(array.ts_type, "string[]"); + } + + #[test] + fn computed_and_field_roles_are_projected_into_form_authority() { + let schemas = schema_forge_dsl::parse( + r#"schema Invoice { + tax: float @compute("1.0") + secret: text @field_access(read: ["finance"], write: ["lead"]) + }"#, + ) + .unwrap(); + let computed = project(&schemas[0].fields[0]).unwrap(); + assert!(computed.computed); + let restricted = project(&schemas[0].fields[1]).unwrap(); + assert_eq!(restricted.read_roles, vec!["finance"]); + assert_eq!(restricted.write_roles, vec!["lead"]); + } + #[test] fn text_with_max_required() { let v = project(&field( @@ -804,7 +953,7 @@ mod tests { assert_eq!(v.sub_fields[0].ts_type, "boolean[][]"); } -#[test] + #[test] fn derived_relation_many_marks_view_as_derived() { let mut fd = field( "documents", diff --git a/crates/schema-forge-cli/templates/site/src/app/pages/edit.generated.tsx.jinja b/crates/schema-forge-cli/templates/site/src/app/pages/edit.generated.tsx.jinja index a15dadf..d97d8a4 100644 --- a/crates/schema-forge-cli/templates/site/src/app/pages/edit.generated.tsx.jinja +++ b/crates/schema-forge-cli/templates/site/src/app/pages/edit.generated.tsx.jinja @@ -6,15 +6,23 @@ // page, so users can customize layout, section headers, and intercepts // there without ever touching the schema-shaped data here. import type { UseFormReturn } from "react-hook-form" +{%- if entity.has_form_controls %} import { FormField } from "@/components/ui/form" +{%- endif %} {%- if entity.has_relation_one %} import { RelationSelect } from "@/components/ui/relation-select" {%- endif %} -{%- if entity.has_file_field %} +{%- if entity.has_form_file_field %} import { FileUpload, type FileAttachment } from "@/components/ui/file-upload" {%- endif %} import type { {{ entity.pascal }} } from "@/generated/entity-types" -import type { {{ entity.pascal }}FormValues } from "@/generated/zod-schemas" +import { +{%- if entity.has_form_fields %} + canReadFormField, canWriteFormField, canWriteFormChildren, +{%- endif %} + normalizeFormValues, normalizeFormPayload, + type FormFieldSpec, type {{ entity.pascal }}FormValues, +} from "@/generated/zod-schemas" {#- -- Shared macros ---------------------------------------------------------- @@ -23,6 +31,8 @@ import type { {{ entity.pascal }}FormValues } from "@/generated/zod-schemas" single `.form-row` block that fits inside the parent `.form-grid`. -#} {% macro control(f) -%} +{%- if not f.derived and not f.computed %} + {canReadFormField({{ f.read_roles | tojson }}) ? (

+ {canWriteFormField({{ f.read_roles | tojson }}, {{ f.write_roles | tojson }}) && canWriteFormChildren({{ f.sub_fields | tojson }} as FormFieldSpec[]) ? ( + <> {%- if f.kind == "boolean" %}