set_null fk cascade bypasses schema and NotNull validation, corrupting surviving records

[fabracht]

Summary

An on_delete=SetNull foreign key writes null directly into the referencing field during a parent delete, skipping both schema validation and constraint validation. On a normally-typed field this produces a stored record that violates its own schema, which then becomes un-updatable. With a NotNull constraint on the same field, the constraint is silently violated.

Both are reproduced by the passing tests at the bottom of this issue.

Affected versions: mqdb-core 0.7.3, mqdb-agent 0.8.10.

Finding 1 — SetNull poisons typed records (no special config needed)

Steps:

1. posts schema declares author_id: String
2. FK posts.author_id -> users.id, on_delete = SetNull
3. create user + post, then delete the user → SetNull writes null into author_id
4. read returns author_id: null — a String-typed field holding null, violating its schema
5. update to an unrelated field (title) → Error::SchemaViolation

The surviving post is now un-updatable through the normal path: every update merges into the stored object and re-validates the whole record (crud.rs:207-226), and the lingering null fails validate_value (schema.rs:60-70). Deleting one entity bricks the update path of an unrelated, surviving entity. This needs only a typed FK field — no NotNull, no ownership config.

Finding 2 — SetNull ignores NotNull on the non-ownership path

Steps:

1. FK posts.author_id -> users.id SetNull plus NotNull on posts.author_id
2. delete the user → delete succeeds, author_id is null in storage despite NotNull
3. a normal update setting author_id: null → Error::NotNullViolation

The update path enforces NotNull but the SetNull cascade does not.

Root cause

Gap	Location
SetNull skips schema validation	crud.rs:393-428 (no validate_entity)
SetNull skips constraint validation	same branch (no validate_update)
No NotNull guard on normal SetNull collection	constraint.rs:362-376
No definition-time compatibility check	schema_ops.rs add_foreign_key (only validate_fields_exist)

The has_not_null_constraint guard (constraint.rs:439) is only reachable via classify_cross_owned_danglers — i.e. only for cross-owned references under an ownership context. The plain SetNull path (constraint.rs:362-376) pushes the operation unconditionally. Existing coverage (test_owner_aware_cascade_cross_owner_blocked_by_not_null) uses Cascade, not SetNull, so the SetNull+NotNull combination was never tested.

The schema type system can't express "nullable String" — a field is String or Null (schema.rs:12-25), with no nullable flag. So SetNull on any typed field inherently produces schema-invalid data.

Proposed fix

Make the bad state unrepresentable at constraint-definition time, in add_foreign_key:

• reject on_delete=SetNull unless the source field is FieldType::Null or schema-less
• reject defining SetNull and NotNull on the same field

Alternatively add first-class nullable fields and let SetNull go through normal validation (passing for nullable fields, blocking the delete otherwise) instead of bypassing it.

Repro

Both findings are covered by these tests. They assert the current buggy behavior (they pass because the bugs exist); they should be inverted once fixed.

use mqdb_agent::Database;
use mqdb_agent::database::CallerContext;
use mqdb_core::schema::{FieldDefinition, FieldType, Schema};
use mqdb_core::types::{OwnershipConfig, ScopeConfig};
use mqdb_core::{Error, OnDeleteAction};
use serde_json::json;
use tempfile::TempDir;

#[tokio::test]
async fn setnull_poisons_typed_record_against_future_updates() {
    let tmp = TempDir::new().unwrap();
    let db = Database::open(tmp.path()).await.unwrap();

    let posts_schema = Schema::new("posts")
        .add_field(FieldDefinition::new("title", FieldType::String))
        .add_field(FieldDefinition::new("author_id", FieldType::String));
    db.add_schema(posts_schema).await.unwrap();

    db.add_foreign_key(
        "posts".into(),
        "author_id".into(),
        "users".into(),
        "id".into(),
        OnDeleteAction::SetNull,
    )
    .await
    .unwrap();

    let user = db
        .create("users".into(), json!({"name": "Alice"}), None, None, None, &ScopeConfig::default())
        .await
        .unwrap();
    let user_id = user["id"].as_str().unwrap().to_string();

    let post = db
        .create(
            "posts".into(),
            json!({"title": "Post 1", "author_id": user_id.clone()}),
            None,
            None,
            None,
            &ScopeConfig::default(),
        )
        .await
        .unwrap();
    let post_id = post["id"].as_str().unwrap().to_string();

    db.delete("users".into(), user_id, None, None, &ScopeConfig::default(), &OwnershipConfig::default())
        .await
        .unwrap();

    let stored = db.read("posts".into(), post_id.clone(), vec![], None).await.unwrap();
    assert_eq!(
        stored["author_id"],
        json!(null),
        "SetNull wrote null into a String-typed field, violating the schema"
    );

    let update_result = db
        .update(
            "posts".into(),
            post_id,
            json!({"title": "Updated title"}),
            None,
            &CallerContext { sender: None, client_id: None, scope_config: &ScopeConfig::default() },
        )
        .await;

    assert!(
        matches!(update_result, Err(Error::SchemaViolation { .. })),
        "updating an unrelated field is now rejected because the lingering \
         null in author_id fails schema re-validation; got {update_result:?}"
    );
}

#[tokio::test]
async fn setnull_bypasses_not_null_constraint_in_normal_path() {
    let tmp = TempDir::new().unwrap();
    let db = Database::open(tmp.path()).await.unwrap();

    db.add_foreign_key(
        "posts".into(),
        "author_id".into(),
        "users".into(),
        "id".into(),
        OnDeleteAction::SetNull,
    )
    .await
    .unwrap();
    db.add_not_null("posts".into(), "author_id".into()).await.unwrap();

    let user = db
        .create("users".into(), json!({"name": "Alice"}), None, None, None, &ScopeConfig::default())
        .await
        .unwrap();
    let user_id = user["id"].as_str().unwrap().to_string();

    let post = db
        .create(
            "posts".into(),
            json!({"title": "Post 1", "author_id": user_id.clone()}),
            None,
            None,
            None,
            &ScopeConfig::default(),
        )
        .await
        .unwrap();
    let post_id = post["id"].as_str().unwrap().to_string();

    let delete_result = db
        .delete("users".into(), user_id, None, None, &ScopeConfig::default(), &OwnershipConfig::default())
        .await;
    assert!(delete_result.is_ok(), "delete proceeds with no NotNull guard on the normal SetNull path");

    let stored = db.read("posts".into(), post_id, vec![], None).await.unwrap();
    assert_eq!(
        stored["author_id"],
        json!(null),
        "author_id is null despite a NotNull constraint on the field"
    );

    let direct_update = db
        .update(
            "posts".into(),
            stored["id"].as_str().unwrap().to_string(),
            json!({"author_id": null}),
            None,
            &CallerContext { sender: None, client_id: None, scope_config: &ScopeConfig::default() },
        )
        .await;
    assert!(
        matches!(direct_update, Err(Error::NotNullViolation { .. })),
        "a normal update setting author_id=null IS rejected by NotNull, \
         proving SetNull reached a state the update path forbids; got {direct_update:?}"
    );
}