Merge pull request #147 from Govcraft/fix/remaining-open-issues

Fix PATCH authorization, data correctness, attachments, and invitations

Author: rrrodzilla

.Codex/plans/data-correctness-140-142.md

@@ -0,0 +1,13 @@
+# Data correctness issues 140 through 142
+
+Preserve enum data during widening and reorder migrations with Identity; represent removed variants explicitly in a new non-exhaustive ValueTransform variant and null only those values before new constraints are installed. Widening is safe, narrowing requires confirmation. Exercise planner and PostgreSQL/SurrealDB generated migrations plus storage behavior where available.
+
+Define null equality as absent or explicit null; inequality is its complement, including non-null comparisons. Compile PostgreSQL null predicates without binds and use IS DISTINCT FROM for non-null inequality; align SurrealDB and MSSQL missing fields. Test filtering and counts together using reusable backend test cases.
+
+Make entity query ordering total by appending id ascending unless explicitly sorted by id. Preserve descending id requests, accept direction as the HTTP alias for order, and validate reserved id filter paths. Fix MSSQL id extraction and SurrealDB record-id literals. Test stable pages, duplicate sort values, descending order and id ranges.
+
+Keep existing typed IDs/errors and pure query/codegen helpers. No dependencies required. Additive transform IR suggests core minor version; parent coordinates all version changes. Run focused nextest and deny-warning Clippy, then parent integration gates for all supported backends.
+
+# Attachment clear concurrency support (issue 120 review)
+
+Add default-unsupported `EntityStore::update_field_if_matches(schema, id, FieldName, expected DynamicValue, value DynamicValue) -> Result<bool, BackendError>` and dynamic bridge. Backend implementations atomically compare the current field and update just it, returning false for a missing/stale attachment. PostgreSQL UPDATE RETURNING participates in the existing revision transaction; SurrealDB uses a conditional UPDATE; SQL Server uses JSON_MODIFY with a bound exact JSON comparison. Shared tests verify stale snapshots do not detach replacements, concurrent clears have one winner, unrelated updates survive and null/missing results are safe. Parent owns actor message, HTTP route and audit handling.

.github/workflows/cel-kani.yml

@@ -0,0 +1,44 @@
+name: CEL scalar proofs
+
+on:
+  pull_request:
+    paths:
+      - 'crates/schema-forge-cel/**'
+      - 'crates/schema-forge-core/**'
+      - 'Cargo.lock'
+      - 'Cargo.toml'
+      - '.github/workflows/cel-kani.yml'
+  push:
+    branches: [main]
+    paths:
+      - 'crates/schema-forge-cel/**'
+      - 'crates/schema-forge-core/**'
+      - 'Cargo.lock'
+      - 'Cargo.toml'
+      - '.github/workflows/cel-kani.yml'
+  workflow_dispatch:
+
+permissions:
+  contents: read
+
+jobs:
+  scalar-proofs:
+    runs-on: ubuntu-24.04
+    timeout-minutes: 45
+    steps:
+      - uses: actions/checkout@v4
+      - uses: dtolnay/rust-toolchain@stable
+      - name: Install build prerequisites
+        run: sudo apt-get update && sudo apt-get install -y protobuf-compiler
+      - name: Install pinned integer solver
+        run: |
+          curl -fLsS -o "$RUNNER_TEMP/z3.zip" https://github.com/Z3Prover/z3/releases/download/z3-5.1.0/z3-5.1.0-x64-glibc-2.39.zip
+          echo "f47be8d27d3230e823bf1eeede2fe0abaca55bb78d0b59974370e6689a92284a  $RUNNER_TEMP/z3.zip" | sha256sum -c -
+          unzip -q "$RUNNER_TEMP/z3.zip" -d "$RUNNER_TEMP/z3"
+          echo "$RUNNER_TEMP/z3/z3-5.1.0-x64-glibc-2.39/bin" >> "$GITHUB_PATH"
+      - name: Install pinned verifier
+        run: |
+          cargo install --locked kani-verifier --version 0.67.0
+          cargo kani setup
+      - name: Prove production scalar contracts and temporal arithmetic
+        run: cargo kani -p schema-forge-cel -Z function-contracts --jobs 4 --output-format terse

Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "schema-forge-acton"
-version = "0.39.0"
+version = "0.40.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.9.0", path = "../schema-forge-cel" }
+schema-forge-cel = { version = "0.10.0", path = "../schema-forge-cel" }
 rust_xlsxwriter = { version = "0.95.0", features = ["chrono"] }
 zip = "8.6.0"
 

crates/schema-forge-acton/Cargo.toml

@@ -299,6 +299,45 @@ pub fn filter_entity_fields(
     fields_to_remove
 }
 
+/// Filter a PATCH delta using the complete post-patch resource for Cedar's
+/// required attributes and resource-dependent policies. Authorization failures
+/// abort the write; explicit policy denials retain ordinary field filtering.
+pub(crate) fn filter_patch_fields(
+    store: &Arc<PolicyStore>,
+    delta: &mut Entity,
+    resource: &Entity,
+    schema: &SchemaDefinition,
+    claims: Option<&Claims>,
+) -> Result<(), ForgeError> {
+    let mut denied = Vec::new();
+    for name in delta.fields.keys() {
+        if schema
+            .field(name)
+            .is_none_or(|field| field.field_access().is_none())
+        {
+            continue;
+        }
+        let decision =
+            authorize_field(store, claims, schema, resource, name, FieldDirection::Write).map_err(
+                |_| ForgeError::Forbidden {
+                    message: "Could not authorize a patched field.".into(),
+                },
+            )?;
+        if !decision.errors.is_empty() {
+            return Err(ForgeError::Forbidden {
+                message: "Could not authorize a patched field.".into(),
+            });
+        }
+        if !decision.is_allow() {
+            denied.push(name.clone());
+        }
+    }
+    for name in denied {
+        delta.fields.remove(&name);
+    }
+    Ok(())
+}
+
 /// Inject tenant scoping filter into a query.
 ///
 /// Adds `_tenant = <tenant_id>` filter based on the deepest tenant in the
@@ -1094,6 +1133,71 @@ mod tests {
         Arc::new(PolicyStore::new(snapshot))
     }
 
+    #[test]
+    fn patch_authorization_errors_abort_without_filtering_the_delta() {
+        let schema = schema_forge_dsl::parse(
+            r#"
+            schema Settings {
+                key: text required
+                draft: text @field_access(read: ["admin"], write: ["admin"])
+            }
+        "#,
+        )
+        .unwrap()
+        .remove(0);
+        let store = store_for(&schema, None);
+        let claims = make_claims(&["admin"]);
+        let mut delta = Entity::with_id(
+            EntityId::new("settings"),
+            schema.name.clone(),
+            BTreeMap::from([("draft".into(), DynamicValue::Text("changed".into()))]),
+        );
+        // A malformed full resource must never turn authorization failure
+        // into a successful empty update, even for a permitted role.
+        let incomplete = delta.clone();
+        let error = filter_patch_fields(&store, &mut delta, &incomplete, &schema, Some(&claims))
+            .unwrap_err();
+        assert!(matches!(error, ForgeError::Forbidden { .. }));
+        assert_eq!(delta.fields, incomplete.fields);
+
+        let mut complete = incomplete.clone();
+        complete
+            .fields
+            .insert("key".into(), DynamicValue::Text("deployment".into()));
+        filter_patch_fields(&store, &mut delta, &complete, &schema, Some(&claims)).unwrap();
+        assert_eq!(delta.fields, incomplete.fields);
+
+        let store = store_for(
+            &schema,
+            Some(
+                r#"
+            forbid (
+                principal,
+                action == Action::"WriteFieldSettings_draft",
+                resource is Settings
+            ) when { 9223372036854775807 + 1 == 0 };
+        "#,
+            ),
+        );
+        let decision = authorize_field(
+            &store,
+            Some(&claims),
+            &schema,
+            &complete,
+            "draft",
+            FieldDirection::Write,
+        )
+        .unwrap();
+        assert!(
+            !decision.errors.is_empty(),
+            "overflow must be a Cedar evaluation error"
+        );
+        let error =
+            filter_patch_fields(&store, &mut delta, &complete, &schema, Some(&claims)).unwrap_err();
+        assert!(matches!(error, ForgeError::Forbidden { .. }));
+        assert_eq!(delta.fields, incomplete.fields);
+    }
+
     #[test]
     fn export_unauthenticated_is_rejected() {
         let schema = schema_read_for_analyst();

crates/schema-forge-acton/src/access.rs

@@ -501,6 +501,28 @@ fn configure_backend_operations(actor: &mut ManagedActor<Idle, ForgeActor>) {
         })
     });
 
+    actor.act_on::<crate::messages::UpdateFieldIfMatches>(|actor, ctx| {
+        let backend = actor.model.backend.clone();
+        let request = ctx.message().clone();
+        Reply::pending(async move {
+            let result = match backend {
+                Some(backend) => {
+                    backend
+                        .update_field_if_matches(
+                            &request.schema,
+                            &request.id,
+                            &request.field,
+                            &request.expected,
+                            &request.value,
+                        )
+                        .await
+                }
+                None => Err(no_backend_error()),
+            };
+            request.reply.send(result).await;
+        })
+    });
+
     actor.act_on::<UpdateEntity>(|actor, ctx| {
         let backend = actor.model.backend.clone();
         let entity = ctx.message().entity.clone();