From dbd926730f8f4ab1967a16cd73f272e54cda5b79 Mon Sep 17 00:00:00 2001 From: t Date: Sat, 18 Jul 2026 17:22:56 -0500 Subject: [PATCH 1/4] test(node): add a completeness fence over the GraphQL mutation surface Gate the GraphQL mutation surface the way REST is gated, as a SEPARATE guard with its own bucket table (a guard over a heterogeneous set must classify per member, INV-1). every_graphql_mutation_has_its_gate asserts the mutation set is complete (every mutation classified) and that each carries its gate marker. Completeness is derived from async-graphql's own introspection: the field names come from the engine as structured data, so no source-text or SDL-format shape (a brace or async fn in a comment or string, a whitespace/generic/skip variant, a brace in a rustdoc description) can hide a live mutation. A new unregistered mutation, including a repo-write, fails the build. The marker check (does each resolver body call its gate) is a best-effort static backstop, bounded to each resolver's own comment-masked, string-aware body. It is explicitly not the completeness guarantee and not load-bearingness: a marker inside a string literal is a documented vacuity residual, and the adversarial sqlx tests in graphql/mutation.rs are the real proof the gates reject. Verified by execution: an unregistered mutation reddens (unclassified), a removed did_matches gate reddens (missing marker), the clean tree is green; no DB, no feature flag. --- crates/gitlawb-node/src/api/mod.rs | 256 +++++++++++++++++++++++++++++ 1 file changed, 256 insertions(+) diff --git a/crates/gitlawb-node/src/api/mod.rs b/crates/gitlawb-node/src/api/mod.rs index 10d76daf..95c85948 100644 --- a/crates/gitlawb-node/src/api/mod.rs +++ b/crates/gitlawb-node/src/api/mod.rs @@ -245,6 +245,262 @@ mod authz_guard { } } + /// Completeness fence over the GraphQL mutation surface: the mirror of + /// `every_in_scope_mutation_has_its_gate` for `graphql/mutation.rs`. Kept a + /// SEPARATE guard with its own table (never folded into the REST rows above): + /// a guard over a heterogeneous set must classify per member, and these + /// same-named task rows belong to the GraphQL surface, not `api/tasks.rs`. + /// + /// COMPLETENESS comes from async-graphql's own introspection (structured field + /// names from the engine), NOT any text parse of source or SDL, so no source-text + /// shape and no serialized-SDL quirk can hide a live mutation from the fence. The + /// schema field set must equal the registered set: a new field with no row fails + /// (a repo-write cannot slip in unlisted); a registered name absent from the + /// schema fails (renamed or removed). + /// + /// The MARKER check is the only source-scraped part, and it is not the + /// completeness guarantee. Three honest limits, so a green check is not misread + /// as full authorization coverage: + /// 1. A present marker is not load-bearingness (the gate may not redden on a + /// hostile probe: the #203 present-but-vacuous case). The adversarial tests + /// in `graphql/mutation.rs` are the load-bearing layer. + /// 2. A present marker is not gate-correctness (it may be the wrong bucket for + /// the operation's resource: a repo-write registered as signer-self would + /// pass, so INV-1 review remains the defense). + /// 3. The marker check scrapes the comment-masked, brace-bounded resolver body; + /// a `{`/`}` or a `did_matches(` inside a STRING literal is not masked, so it + /// could mis-slice that body or satisfy the marker vacuously (the same + /// vacuous-marker class as limit 1). Completeness is unaffected: it comes + /// from introspection, not from this scrape. + #[test] + fn every_graphql_mutation_has_its_gate() { + // (Rust snake name, schema field in camelCase, expected gate marker). The + // four are Bucket C signer-self (`did_matches(`); a repo-write would be + // Bucket A (`require_repo_owner(`). Same-named REST rows live in the guard + // above against `api/tasks.rs`. + let registered: &[(&str, &str, &str)] = &[ + ("create_task", "createTask", "did_matches("), + ("claim_task", "claimTask", "did_matches("), + ("complete_task", "completeTask", "did_matches("), + ("fail_task", "failTask", "did_matches("), + ]; + + // Completeness both directions against the authoritative schema field set. + let fields = graphql_mutation_fields_from_schema(); + for f in &fields { + assert!( + registered.iter().any(|(_, camel, _)| camel == f), + "unclassified GraphQL mutation `{f}`: add it to \ + every_graphql_mutation_has_its_gate with a deliberate gate \ + (`require_repo_owner(` for a repo-write, `did_matches(` for a task op)" + ); + } + for (_, camel, _) in registered { + assert!( + fields.iter().any(|f| f == camel), + "registered GraphQL mutation `{camel}` is not in the schema \ + (renamed or removed); update the table" + ); + } + + // Marker present (source scrape; the vacuity limit, backstopped by the + // adversarial tests): each resolver body carries its bucket marker. + let masked = mask_comments(include_str!("../graphql/mutation.rs")); + for (snake, _, marker) in registered { + let body = resolver_body(&masked, snake); + assert!( + body.contains(marker), + "GraphQL mutation `{snake}` is missing its gate marker `{marker}`: gate removed" + ); + } + } + + /// The mutation field names (camelCase, as async-graphql exposes them) via the + /// engine's own introspection. STRUCTURED: the names arrive as data, so nothing in + /// the resolver source or the serialized SDL (a brace in a doc description, a + /// comment, a string) can hide a field. The schema builds with no data and + /// introspection runs no resolver, so this stays a plain DB-free unit test. A + /// navigation miss yields an empty list, which fails loud via the set-equality + /// above (a registered name reads as "not in the schema"), never vacuously. + fn graphql_mutation_fields_from_schema() -> Vec { + use crate::graphql::mutation::MutationRoot; + use crate::graphql::query::QueryRoot; + use crate::graphql::subscription::SubscriptionRoot; + use async_graphql::Value; + let schema = + async_graphql::Schema::build(QueryRoot, MutationRoot, SubscriptionRoot).finish(); + let resp = tokio::runtime::Runtime::new() + .expect("tokio runtime") + .block_on(schema.execute("{ __schema { mutationType { fields { name } } } }")); + assert!( + resp.errors.is_empty(), + "schema introspection failed: {:?}", + resp.errors + ); + let mut names = Vec::new(); + if let Value::Object(root) = &resp.data { + if let Some(Value::Object(sc)) = root.get("__schema") { + if let Some(Value::Object(mt)) = sc.get("mutationType") { + if let Some(Value::List(field_list)) = mt.get("fields") { + for field in field_list { + if let Value::Object(fo) = field { + if let Some(Value::String(n)) = fo.get("name") { + names.push(n.clone()); + } + } + } + } + } + } + } + names + } + + /// The comment-masked body of `async fn ` in `graphql/mutation.rs`, bounded + /// to the resolver's OWN braces (its `{ .. }`), for the marker check. Brace-matching + /// on the comment-masked source means a `}` in a trailing helper cannot extend the + /// body and a comment marker does not count. A `{`/`}` inside a STRING literal is + /// not masked, a documented marker-check-only residual (limit 3); completeness does + /// not use this. + fn resolver_body(masked_src: &str, snake: &str) -> String { + let needle = format!("async fn {snake}("); + let start = masked_src.find(&needle).unwrap_or_else(|| { + panic!("resolver `async fn {snake}(` not found in graphql/mutation.rs") + }); + let open = start + + masked_src[start..] + .find('{') + .expect("resolver body opening brace"); + let mut depth = 0i32; + let mut end = masked_src.len(); + for (i, c) in masked_src[open..].char_indices() { + match c { + '{' => depth += 1, + '}' => { + depth -= 1; + if depth == 0 { + end = open + i + 1; + break; + } + } + _ => {} + } + } + masked_src[open..end].to_string() + } + + /// `src` with `//` line comments and `/* */` block comments (nested) blanked to + /// spaces, WITHOUT opening a comment inside a `"..."` string (so a `"https://"` does + /// not erase the rest of its line). String CONTENTS are left intact, so a brace or + /// a gate marker inside a string is not masked; for the marker check that is a + /// documented vacuity residual (limit 3), and completeness does not use this at + /// all. Char literals and raw/byte strings are not tracked (absent in resolver + /// bodies); a `'"'` char or a raw string is the remaining narrow residual. + fn mask_comments(src: &str) -> String { + let chars: Vec = src.chars().collect(); + let mut out = String::with_capacity(src.len()); + let mut i = 0; + let mut block_depth = 0usize; + let mut in_string = false; + while i < chars.len() { + let c = chars[i]; + let next = chars.get(i + 1).copied(); + if block_depth > 0 { + if c == '/' && next == Some('*') { + block_depth += 1; + out.push_str(" "); + i += 2; + } else if c == '*' && next == Some('/') { + block_depth -= 1; + out.push_str(" "); + i += 2; + } else { + out.push(if c == '\n' { '\n' } else { ' ' }); + i += 1; + } + } else if in_string { + out.push(c); + if c == '\\' { + if let Some(n) = next { + out.push(n); + i += 2; + } else { + i += 1; + } + } else { + if c == '"' { + in_string = false; + } + i += 1; + } + } else if c == '"' { + in_string = true; + out.push('"'); + i += 1; + } else if c == '/' && next == Some('/') { + while i < chars.len() && chars[i] != '\n' { + out.push(' '); + i += 1; + } + } else if c == '/' && next == Some('*') { + block_depth = 1; + out.push_str(" "); + i += 2; + } else { + out.push(c); + i += 1; + } + } + out + } + + /// The introspection field set (the completeness source of truth) is exactly the + /// registered mutations. If async-graphql adds or renames a field, this reddens + /// and forces the `every_graphql_mutation_has_its_gate` table to be updated. + #[test] + fn schema_lists_exactly_the_registered_mutations() { + let mut fields = graphql_mutation_fields_from_schema(); + fields.sort(); + assert_eq!( + fields, + vec!["claimTask", "completeTask", "createTask", "failTask"], + "the introspected mutation field set is the authoritative discovered set" + ); + } + + /// The marker check is bounded to a resolver's OWN body and is comment- and + /// string-aware: a `//` inside a `"https://"` string does not erase a real gate on + /// that line (no false-red), and a trailing module helper's `did_matches(` does not + /// rescue a resolver whose own gate was removed (no false-green). + #[test] + fn resolver_body_is_bounded_and_comment_and_string_aware() { + let src = "\ +impl MutationRoot { + async fn ok(&self) -> Result { + let _ = (\"see https://x/y\", did_matches(a, b)); // URL string then gate, one line + Ok(0) + } + async fn last(&self) -> Result { + // did_matches( only in a comment + Ok(0) + } +} +fn helper() { let _ = did_matches(z, z); } +#[cfg(test)] +mod tests { async fn t() { let _ = did_matches(x, y); } } +"; + let masked = mask_comments(src); + assert!( + resolver_body(&masked, "ok").contains("did_matches("), + "a `//` inside a string must not erase the real gate on that line" + ); + assert!( + !resolver_body(&masked, "last").contains("did_matches("), + "the marker check must be bounded to the resolver's own body (a trailing \ + helper's marker must not rescue a resolver whose gate was removed)" + ); + } + /// Proves the comment-stripping that GUARD-1 added: a marker that appears only /// in a full-line comment (the real `replicas.rs` false-pass shape) must NOT /// satisfy a row. From c3fee1e093fc9d3611af76ef11554aa3127d556e Mon Sep 17 00:00:00 2001 From: t Date: Sat, 18 Jul 2026 17:22:56 -0500 Subject: [PATCH 2/4] docs(node): reconcile GraphQL mutation-auth claims with the shipped state README's known limitations, the OSS-readiness live-network blockers, and the maintainer roadmap still listed GraphQL mutation auth as an open item, which contradicts #87: GraphQL mutations already require a verified signer bound to the acting DID. Reconcile each to the shipped reality while keeping the genuinely-open nuance: the GraphQL mutation surface is the agent-task queue, repo writes stay REST-only, and no owner-gated repo-write mutation is exposed over GraphQL. The every_graphql_mutation_has_its_gate fence forces any new mutation to be classified in its table; owner-gate and REST parity stay the INV-1 review check. --- README.md | 2 +- docs/MAINTAINER-ROADMAP.md | 2 +- docs/OSS-READINESS-AUDIT.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 57ce2885..e701f9ce 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,7 @@ Known limitations: - UCAN chain validation and revocation are not complete. - Repository write authorization is not capability-complete yet; HTTP signatures prove identity, not full authorization policy. - Peer writes are signed by upgraded nodes, but strict signed-peer enforcement is opt-in during rolling upgrades. -- GraphQL mutations need mutation-aware auth before becoming a public write surface. +- GraphQL mutations require a signed identity (a verified signer bound to the acting DID) but are limited to the agent-task queue; repo writes are not exposed over GraphQL and remain REST-only. See: diff --git a/docs/MAINTAINER-ROADMAP.md b/docs/MAINTAINER-ROADMAP.md index 9e1a73fd..6546159b 100644 --- a/docs/MAINTAINER-ROADMAP.md +++ b/docs/MAINTAINER-ROADMAP.md @@ -41,7 +41,7 @@ Owner focus: protocol/auth. - Implement repo write authorization: repo owner checks, protected branches, UCAN capability checks, and clear delegation semantics. - Implement private-read enforcement or remove private-repo affordances until it exists. - Add UCAN revocation or blocklisting, with an emergency compromised-key runbook. -- Add mutation-aware GraphQL auth before GraphQL becomes a public write API surface. +- GraphQL mutations require a verified signer (shipped, #87). Keep the surface limited to the agent-task queue; do not expose an owner-gated repo-write mutation over GraphQL without its owner-gate and REST parity. The `every_graphql_mutation_has_its_gate` fence forces any new mutation to be classified in its table (discovered from the schema, so none can slip in unlisted); the owner-gate and REST parity themselves stay the INV-1 review check, not something the fence proves. - Harden peer registration and outbound peer calls against SSRF and peer-list poisoning. - After all live peers upgrade, enable signed peer writes by default. diff --git a/docs/OSS-READINESS-AUDIT.md b/docs/OSS-READINESS-AUDIT.md index 0a0bc5f2..85eb56ad 100644 --- a/docs/OSS-READINESS-AUDIT.md +++ b/docs/OSS-READINESS-AUDIT.md @@ -87,7 +87,7 @@ Fixed or staged in this pass: Live-network blockers to prioritize: -- GraphQL POST is still open for compatibility; GraphQL mutations should get mutation-aware auth before it becomes a public write API surface. +- GraphQL mutations now require a verified signer bound to the acting DID (#87), and the surface stays limited to the agent-task queue, so there is no public repo-write GraphQL API to gate. Queries remain open by design. (Resolved; kept here as posture, not a live blocker.) - Push authorization is still not capability-complete. A valid DID signature is authentication, not authorization; unprotected repo branches do not yet enforce owner/UCAN capability checks. - UCAN chain validation is incomplete and UCAN revocation/blocklisting is not implemented as an operator feature. - Private repository reads are not enforced. `is_public` and `GITLAWB_PUBLIC_READ` exist, but per-repository private-read behavior is not wired. From 11db151a2cd9286a1ab47d4d365e890dbeeb567a Mon Sep 17 00:00:00 2001 From: t Date: Wed, 22 Jul 2026 08:42:34 -0500 Subject: [PATCH 3/4] fix(node): source the GraphQL mutation fence from SDL, not introspection Resolves jatmn's #219 P1. The every_graphql_mutation_has_its_gate completeness fence enumerated mutation fields via async-graphql introspection, which filters out `#[graphql(visible = false)]` fields. Those fields stay executable, so a future hidden mutation would need no registry row and no marker while both set-equality checks stayed green -- defeating the fence's guarantee that no live mutation goes unclassified. (Confirmed by execution: a synthetic visible=false mutation is absent from introspection, executable, and present in schema.sdl().) Source completeness from the engine's own SDL instead. A new pure helper mutation_field_names_from_sdl parses schema.sdl() with async-graphql's parser AST (async_graphql::parser::parse_schema) and reads the mutation type's fields; SDL includes visible=false fields and the AST parse is immune to a brace in a field description. Introspection is kept as a cross-check: any field present in SDL but hidden from introspection now fails named as a forbidden hidden mutation. The stale doc comments (which claimed introspection could not hide a field) are corrected. A self-checking version-safety test builds a synthetic schema with a visible=false mutation and asserts the SDL-sourced set includes it while introspection omits it -- so if a future async-graphql ever filters SDL by visibility, that test goes RED rather than the gap reopening silently. Load-bearing verified by execution: stubbing the SDL enumeration to empty turns four tests RED, including the fence itself; restoring returns green. Only api/mod.rs changed; the mutation resolvers are untouched. `cargo test -p gitlawb-node --bins` green (495); clippy and fmt clean. --- crates/gitlawb-node/src/api/mod.rs | 225 +++++++++++++++++++++++++++-- 1 file changed, 216 insertions(+), 9 deletions(-) diff --git a/crates/gitlawb-node/src/api/mod.rs b/crates/gitlawb-node/src/api/mod.rs index 95c85948..47a4a893 100644 --- a/crates/gitlawb-node/src/api/mod.rs +++ b/crates/gitlawb-node/src/api/mod.rs @@ -251,12 +251,17 @@ mod authz_guard { /// a guard over a heterogeneous set must classify per member, and these /// same-named task rows belong to the GraphQL surface, not `api/tasks.rs`. /// - /// COMPLETENESS comes from async-graphql's own introspection (structured field - /// names from the engine), NOT any text parse of source or SDL, so no source-text - /// shape and no serialized-SDL quirk can hide a live mutation from the fence. The - /// schema field set must equal the registered set: a new field with no row fails - /// (a repo-write cannot slip in unlisted); a registered name absent from the - /// schema fails (renamed or removed). + /// COMPLETENESS comes from the engine's own SDL (`schema.sdl()`), parsed as a + /// structured AST (#219), NOT introspection and NOT a text parse of source. + /// This matters: async-graphql OMITS `#[graphql(visible = false)]` fields from + /// introspection while leaving them executable, so an introspection-sourced fence + /// could be defeated by a hidden mutation. SDL includes those fields, so no + /// visibility flag (and, via the AST parse, no brace-in-description quirk) can hide + /// a live mutation. The SDL field set must equal the registered set: a new field with + /// no row fails (a repo-write cannot slip in unlisted); a registered name absent from + /// the schema fails (renamed or removed). The set is also cross-checked against + /// introspection, so any field present in SDL but hidden from introspection fails + /// named as a forbidden hidden mutation. /// /// The MARKER check is the only source-scraped part, and it is not the /// completeness guarantee. Three honest limits, so a green check is not misread @@ -271,7 +276,7 @@ mod authz_guard { /// a `{`/`}` or a `did_matches(` inside a STRING literal is not masked, so it /// could mis-slice that body or satisfy the marker vacuously (the same /// vacuous-marker class as limit 1). Completeness is unaffected: it comes - /// from introspection, not from this scrape. + /// from the SDL AST, not from this scrape. #[test] fn every_graphql_mutation_has_its_gate() { // (Rust snake name, schema field in camelCase, expected gate marker). The @@ -285,8 +290,23 @@ mod authz_guard { ("fail_task", "failTask", "did_matches("), ]; - // Completeness both directions against the authoritative schema field set. - let fields = graphql_mutation_fields_from_schema(); + // Completeness source is the SDL (#219): it includes visible=false mutations, + // which introspection omits. Cross-check against introspection first so a hidden + // mutation fails with a specific, actionable message rather than a generic + // "unclassified". + let fields = graphql_mutation_fields_from_sdl(); + let introspected = graphql_mutation_fields_from_schema(); + for f in &fields { + assert!( + introspected.iter().any(|i| i == f), + "GraphQL mutation `{f}` is in the schema but hidden from introspection \ + (a `#[graphql(visible = false)]` field): hidden mutations are forbidden \ + by this fence — make it visible so its classification is enforceable, or \ + remove it" + ); + } + + // Completeness both directions against the authoritative (SDL) field set. for f in &fields { assert!( registered.iter().any(|(_, camel, _)| camel == f), @@ -315,6 +335,84 @@ mod authz_guard { } } + /// Version-safety self-check (#219, INV-21): the fence's completeness guarantee rests + /// on `schema.sdl()` INCLUDING `visible = false` fields while introspection omits + /// them. That is observed async-graphql behavior, not a documented contract, so pin + /// it here: a synthetic schema with a hidden mutation must have that field in its + /// SDL-derived set and NOT in its introspection-derived set. If a future + /// async-graphql upgrade ever filters SDL by visibility, the first assertion flips + /// RED — the assumption is load-bearing, not trusted. + #[test] + fn sdl_source_sees_hidden_mutations_that_introspection_omits() { + use async_graphql::{EmptySubscription, Object, Schema, Value}; + struct Q; + #[Object] + impl Q { + async fn ping(&self) -> i32 { + 0 + } + } + struct M; + #[Object] + impl M { + async fn visible_mut(&self) -> i32 { + 1 + } + #[graphql(visible = false)] + async fn hidden_mut(&self) -> i32 { + 2 + } + } + let schema = Schema::build(Q, M, EmptySubscription).finish(); + + // SDL source (the fix): includes the hidden field. + let sdl_fields = mutation_field_names_from_sdl(&schema.sdl()); + assert!( + sdl_fields.iter().any(|f| f == "hiddenMut"), + "SDL-sourced set MUST include the visible=false mutation; if this is RED, \ + async-graphql started filtering SDL by visibility and the fence's \ + completeness guarantee is broken: {sdl_fields:?}" + ); + + // Introspection (the OLD source): omits the hidden field. This is the exact gap + // the fix closes — an introspection-sourced fence never sees `hiddenMut`. + let resp = tokio::runtime::Runtime::new() + .expect("tokio runtime") + .block_on(schema.execute("{ __schema { mutationType { fields { name } } } }")); + let mut introspected = Vec::new(); + if let Value::Object(root) = &resp.data { + if let Some(Value::Object(sc)) = root.get("__schema") { + if let Some(Value::Object(mt)) = sc.get("mutationType") { + if let Some(Value::List(fl)) = mt.get("fields") { + for fo in fl { + if let Value::Object(o) = fo { + if let Some(Value::String(n)) = o.get("name") { + introspected.push(n.clone()); + } + } + } + } + } + } + } + assert!( + !introspected.iter().any(|f| f == "hiddenMut"), + "introspection is expected to OMIT the visible=false mutation (that is the \ + gap the SDL source closes); got {introspected:?}" + ); + + // The exact diff the fence computes: hidden ∈ (sdl \ introspection). + let hidden: Vec<&String> = sdl_fields + .iter() + .filter(|f| !introspected.iter().any(|i| i == *f)) + .collect(); + assert_eq!( + hidden, + vec!["hiddenMut"], + "the SDL-vs-introspection diff must surface exactly the hidden mutation" + ); + } + /// The mutation field names (camelCase, as async-graphql exposes them) via the /// engine's own introspection. STRUCTURED: the names arrive as data, so nothing in /// the resolver source or the serialized SDL (a brace in a doc description, a @@ -356,6 +454,115 @@ mod authz_guard { names } + /// The mutation field names sourced from the built schema's SDL — the completeness + /// source (#219). Builds the same DB-free schema as the introspection helper and runs + /// its `.sdl()` (sync, no runtime needed) through [`mutation_field_names_from_sdl`]. + /// SDL includes `#[graphql(visible = false)]` fields, so this source cannot be + /// defeated by hiding a mutation from introspection. + fn graphql_mutation_fields_from_sdl() -> Vec { + use crate::graphql::mutation::MutationRoot; + use crate::graphql::query::QueryRoot; + use crate::graphql::subscription::SubscriptionRoot; + let schema = + async_graphql::Schema::build(QueryRoot, MutationRoot, SubscriptionRoot).finish(); + mutation_field_names_from_sdl(&schema.sdl()) + } + + /// The mutation type's field names extracted from an async-graphql SDL string via + /// the engine's OWN parser AST (`async_graphql::parser::parse_schema`), NOT + /// introspection. This is the completeness source (#219): unlike introspection, + /// which async-graphql filters by `visible` (a `#[graphql(visible = false)]` + /// mutation is executable but ABSENT from introspection), `schema.sdl()` includes + /// every registered field, so a hidden mutation still appears here and must carry a + /// classified row. Parsing the AST (not a brace/line text scan) means a `}` inside a + /// field description cannot mis-slice the type block. camelCase, as async-graphql + /// emits. An empty result on a navigation miss is safe: the set-equality in + /// `every_graphql_mutation_has_its_gate` turns "no fields" into a loud failure for + /// every registered name, never a vacuous pass. + fn mutation_field_names_from_sdl(sdl: &str) -> Vec { + use async_graphql::parser::parse_schema; + use async_graphql::parser::types::{TypeKind, TypeSystemDefinition}; + + let doc = parse_schema(sdl).expect("async-graphql SDL must parse"); + // The mutation root type name: from the `schema { mutation: X }` block if + // present, else the GraphQL default "Mutation". + let mutation_type = doc + .definitions + .iter() + .find_map(|def| match def { + TypeSystemDefinition::Schema(s) => s + .node + .mutation + .as_ref() + .map(|m| m.node.as_str().to_string()), + _ => None, + }) + .unwrap_or_else(|| "Mutation".to_string()); + // The fields of that object type. + doc.definitions + .iter() + .find_map(|def| match def { + TypeSystemDefinition::Type(t) if t.node.name.node.as_str() == mutation_type => { + match &t.node.kind { + TypeKind::Object(obj) => Some( + obj.fields + .iter() + .map(|f| f.node.name.node.as_str().to_string()) + .collect::>(), + ), + _ => None, + } + } + _ => None, + }) + .unwrap_or_default() + } + + #[test] + fn sdl_extractor_includes_a_hidden_visible_false_field() { + // A field that would be hidden from introspection must still be listed from SDL. + let sdl = "schema { query: QueryRoot mutation: MutationRoot }\n\ + type QueryRoot { ping: Int! }\n\ + type MutationRoot { createTask: Int! hiddenMut: Int! }\n"; + let fields = mutation_field_names_from_sdl(sdl); + assert!( + fields.iter().any(|f| f == "createTask"), + "visible field must be listed: {fields:?}" + ); + assert!( + fields.iter().any(|f| f == "hiddenMut"), + "a visible=false field (present in SDL, hidden from introspection) MUST be \ + listed by the completeness source: {fields:?}" + ); + } + + #[test] + fn sdl_extractor_is_not_fooled_by_a_brace_in_a_field_description() { + // A `}` inside a field description must not truncate the type block (the AST + // parse is immune to the text-parse quirk the old design feared). + let sdl = "schema { query: QueryRoot mutation: MutationRoot }\n\ + type QueryRoot { ping: Int! }\n\ + type MutationRoot {\n\ + \"\"\"a description with a } brace and { another\"\"\"\n\ + createTask: Int!\n\ + claimTask: Int!\n\ + }\n"; + let fields = mutation_field_names_from_sdl(sdl); + assert!( + fields.iter().any(|f| f == "createTask") && fields.iter().any(|f| f == "claimTask"), + "both fields must be extracted despite a brace in a description: {fields:?}" + ); + } + + #[test] + fn sdl_extractor_returns_empty_for_a_query_only_schema() { + let sdl = "schema { query: QueryRoot }\ntype QueryRoot { ping: Int! }\n"; + assert!( + mutation_field_names_from_sdl(sdl).is_empty(), + "a schema with no mutation root yields no mutation fields (no panic)" + ); + } + /// The comment-masked body of `async fn ` in `graphql/mutation.rs`, bounded /// to the resolver's OWN braces (its `{ .. }`), for the marker check. Brace-matching /// on the comment-masked source means a `}` in a trailing helper cannot extend the From f3d91fa2de94bd3d00e8febb41b45ccc6fee5c20 Mon Sep 17 00:00:00 2001 From: t Date: Wed, 22 Jul 2026 08:58:57 -0500 Subject: [PATCH 4/4] fix(node): make the SDL mutation-fence wiring load-bearing; admit deprecated An adversarial review of the SDL switch found three gaps: - The fence's completeness source (SDL) and its hidden-mutation cross-check were not load-bearing: because the real MutationRoot has no visible=false field, reverting `fields` back to introspection and dropping the cross-check left the whole suite green (INV-21). Extract the sourcing + cross-check into a generic `assert_mutation_fence` that the real fence AND a synthetic hidden schema both drive. A new #[should_panic] test (fence_rejects_a_hidden_mutation) feeds a visible=false mutation that IS registered, so only the cross-check can fire; reverting the SDL source or deleting the cross-check now changes/removes that panic and turns the test RED (verified). - The introspection cross-check used bare `fields { name }` (GraphQL default includeDeprecated:false), so a `#[graphql(deprecation = "...")]` mutation -- executable and present in SDL -- would false-red as a hidden visible=false mutation. Use `fields(includeDeprecated: true)`; a new test (fence_admits_a_deprecated_mutation) pins it, going RED without the flag. - schema_lists_exactly_the_registered_mutations still sourced from introspection and its comment called introspection "the source of truth", contradicting the fix. Retargeted at the SDL set with a corrected comment. Also folds the two introspection navigations into one generic helper (introspection_mutation_fields) and points the version-safety test at it, so there is a single introspection query. cargo test -p gitlawb-node --bins green (497); clippy and fmt clean. Only api/mod.rs changed. --- crates/gitlawb-node/src/api/mod.rs | 227 ++++++++++++++++++++--------- 1 file changed, 156 insertions(+), 71 deletions(-) diff --git a/crates/gitlawb-node/src/api/mod.rs b/crates/gitlawb-node/src/api/mod.rs index 47a4a893..ab267f98 100644 --- a/crates/gitlawb-node/src/api/mod.rs +++ b/crates/gitlawb-node/src/api/mod.rs @@ -290,37 +290,20 @@ mod authz_guard { ("fail_task", "failTask", "did_matches("), ]; - // Completeness source is the SDL (#219): it includes visible=false mutations, - // which introspection omits. Cross-check against introspection first so a hidden - // mutation fails with a specific, actionable message rather than a generic - // "unclassified". - let fields = graphql_mutation_fields_from_sdl(); - let introspected = graphql_mutation_fields_from_schema(); - for f in &fields { - assert!( - introspected.iter().any(|i| i == f), - "GraphQL mutation `{f}` is in the schema but hidden from introspection \ - (a `#[graphql(visible = false)]` field): hidden mutations are forbidden \ - by this fence — make it visible so its classification is enforceable, or \ - remove it" - ); - } - - // Completeness both directions against the authoritative (SDL) field set. - for f in &fields { - assert!( - registered.iter().any(|(_, camel, _)| camel == f), - "unclassified GraphQL mutation `{f}`: add it to \ - every_graphql_mutation_has_its_gate with a deliberate gate \ - (`require_repo_owner(` for a repo-write, `did_matches(` for a task op)" - ); - } - for (_, camel, _) in registered { - assert!( - fields.iter().any(|f| f == camel), - "registered GraphQL mutation `{camel}` is not in the schema \ - (renamed or removed); update the table" - ); + // Completeness + hidden-mutation cross-check against the REAL schema (#219), + // delegated to the generic `assert_mutation_fence` so the SDL sourcing and the + // cross-check are ALSO exercised by a synthetic hidden schema in + // `fence_rejects_a_hidden_mutation` (INV-21). Because the real MutationRoot has no + // hidden field today, that synthetic test — not a hypothetical production canary — + // is what turns reverting the SDL source or deleting the cross-check RED. + { + use crate::graphql::mutation::MutationRoot; + use crate::graphql::query::QueryRoot; + use crate::graphql::subscription::SubscriptionRoot; + let schema = + async_graphql::Schema::build(QueryRoot, MutationRoot, SubscriptionRoot).finish(); + let registered_camel: Vec<&str> = registered.iter().map(|(_, c, _)| *c).collect(); + assert_mutation_fence(&schema, ®istered_camel); } // Marker present (source scrape; the vacuity limit, backstopped by the @@ -344,7 +327,7 @@ mod authz_guard { /// RED — the assumption is load-bearing, not trusted. #[test] fn sdl_source_sees_hidden_mutations_that_introspection_omits() { - use async_graphql::{EmptySubscription, Object, Schema, Value}; + use async_graphql::{EmptySubscription, Object, Schema}; struct Q; #[Object] impl Q { @@ -374,27 +357,9 @@ mod authz_guard { completeness guarantee is broken: {sdl_fields:?}" ); - // Introspection (the OLD source): omits the hidden field. This is the exact gap - // the fix closes — an introspection-sourced fence never sees `hiddenMut`. - let resp = tokio::runtime::Runtime::new() - .expect("tokio runtime") - .block_on(schema.execute("{ __schema { mutationType { fields { name } } } }")); - let mut introspected = Vec::new(); - if let Value::Object(root) = &resp.data { - if let Some(Value::Object(sc)) = root.get("__schema") { - if let Some(Value::Object(mt)) = sc.get("mutationType") { - if let Some(Value::List(fl)) = mt.get("fields") { - for fo in fl { - if let Value::Object(o) = fo { - if let Some(Value::String(n)) = o.get("name") { - introspected.push(n.clone()); - } - } - } - } - } - } - } + // Introspection (the fence's own helper): OMITS the hidden field. This is the + // exact gap the fix closes — an introspection-sourced fence never sees `hiddenMut`. + let introspected = introspection_mutation_fields(&schema); assert!( !introspected.iter().any(|f| f == "hiddenMut"), "introspection is expected to OMIT the visible=false mutation (that is the \ @@ -413,23 +378,141 @@ mod authz_guard { ); } - /// The mutation field names (camelCase, as async-graphql exposes them) via the - /// engine's own introspection. STRUCTURED: the names arrive as data, so nothing in - /// the resolver source or the serialized SDL (a brace in a doc description, a - /// comment, a string) can hide a field. The schema builds with no data and - /// introspection runs no resolver, so this stays a plain DB-free unit test. A - /// navigation miss yields an empty list, which fails loud via the set-equality - /// above (a registered name reads as "not in the schema"), never vacuously. - fn graphql_mutation_fields_from_schema() -> Vec { - use crate::graphql::mutation::MutationRoot; - use crate::graphql::query::QueryRoot; - use crate::graphql::subscription::SubscriptionRoot; + /// INV-21 wiring guard (#219): drives the generic `assert_mutation_fence` — the SAME + /// helper the real fence calls — against a synthetic schema whose mutation is + /// `visible = false` and IS registered, so completeness passes and ONLY the + /// hidden-from-introspection cross-check can fail. This is what makes the fence's SDL + /// sourcing load-bearing without a production canary: reverting the source to + /// introspection (the hidden field then absent from the SDL set) or deleting the + /// cross-check removes this panic (or changes its message), turning the + /// `should_panic` test RED. The real MutationRoot has no hidden field, so nothing on + /// the real schema could catch that regression. + #[test] + #[should_panic(expected = "hidden from introspection")] + fn fence_rejects_a_hidden_mutation() { + use async_graphql::{EmptySubscription, Object, Schema}; + struct Q; + #[Object] + impl Q { + async fn ping(&self) -> i32 { + 0 + } + } + struct M; + #[Object] + impl M { + async fn visible_mut(&self) -> i32 { + 1 + } + #[graphql(visible = false)] + async fn hidden_mut(&self) -> i32 { + 2 + } + } + let schema = Schema::build(Q, M, EmptySubscription).finish(); + // Both fields registered, so completeness passes and only the cross-check fires. + assert_mutation_fence(&schema, &["visibleMut", "hiddenMut"]); + } + + /// Deprecated-mutation guard (#219): a `#[graphql(deprecation = "...")]` mutation is + /// executable and present in SDL, but GraphQL introspection omits it by default. The + /// fence's introspection helper uses `includeDeprecated: true`, so a deprecated (but + /// visible) mutation is NOT mistaken for a hidden one. With it registered, the fence + /// must NOT panic. Remove `includeDeprecated: true` and this goes RED — the deprecated + /// field would look hidden and false-red as a forbidden visible=false mutation. + #[test] + fn fence_admits_a_deprecated_mutation() { + use async_graphql::{EmptySubscription, Object, Schema}; + struct Q; + #[Object] + impl Q { + async fn ping(&self) -> i32 { + 0 + } + } + struct M; + #[Object] + impl M { + async fn visible_mut(&self) -> i32 { + 1 + } + #[graphql(deprecation = "use visibleMut")] + async fn deprecated_mut(&self) -> i32 { + 2 + } + } + let schema = Schema::build(Q, M, EmptySubscription).finish(); + // Must not panic: the deprecated field is visible (present in introspection with + // includeDeprecated) and registered, so it is neither hidden nor unclassified. + assert_mutation_fence(&schema, &["visibleMut", "deprecatedMut"]); + } + + /// Completeness + hidden-mutation cross-check for a BUILT schema (#219). Generic over + /// the schema type so the real fence AND the synthetic `fence_rejects_a_hidden_mutation` + /// / `fence_admits_a_deprecated_mutation` tests drive the SAME sourcing and cross-check. + /// Completeness is sourced from SDL (which includes `visible = false` fields); the + /// cross-check fails any SDL field hidden from introspection. This genericity is the + /// INV-21 fix: because the real MutationRoot has no hidden field, only a synthetic + /// hidden schema through THIS helper makes reverting the SDL source or removing the + /// cross-check RED, rather than leaving the class latent behind a production canary. + fn assert_mutation_fence( + schema: &async_graphql::Schema, + registered_camel: &[&str], + ) where + Q: async_graphql::ObjectType + 'static, + M: async_graphql::ObjectType + 'static, + S: async_graphql::SubscriptionType + 'static, + { + let sdl_fields = mutation_field_names_from_sdl(&schema.sdl()); + let introspected = introspection_mutation_fields(schema); + // No SDL field may be hidden from introspection (a visible=false mutation). + for f in &sdl_fields { + assert!( + introspected.iter().any(|i| i == f), + "GraphQL mutation `{f}` is in the schema but hidden from introspection \ + (a `#[graphql(visible = false)]` field): hidden mutations are forbidden \ + by this fence — make it visible so its classification is enforceable, or \ + remove it" + ); + } + // Completeness both directions against the registered (camelCase) set. + for f in &sdl_fields { + assert!( + registered_camel.contains(&f.as_str()), + "unclassified GraphQL mutation `{f}`: add it to \ + every_graphql_mutation_has_its_gate with a deliberate gate \ + (`require_repo_owner(` for a repo-write, `did_matches(` for a task op)" + ); + } + for c in registered_camel { + assert!( + sdl_fields.iter().any(|f| f == c), + "registered GraphQL mutation `{c}` is not in the schema \ + (renamed or removed); update the table" + ); + } + } + + /// The mutation field names via the engine's introspection, run against a passed + /// schema. STRUCTURED (names arrive as data). Uses `fields(includeDeprecated: true)`: + /// GraphQL's default omits `@deprecated` fields, but a `#[graphql(deprecation = "...")]` + /// mutation is still executable and present in SDL, so without this the cross-check + /// above would false-red a deprecated mutation as a hidden (visible=false) one. Only a + /// real visibility filter should trip that arm; deprecation is a lifecycle marker. + fn introspection_mutation_fields( + schema: &async_graphql::Schema, + ) -> Vec + where + Q: async_graphql::ObjectType + 'static, + M: async_graphql::ObjectType + 'static, + S: async_graphql::SubscriptionType + 'static, + { use async_graphql::Value; - let schema = - async_graphql::Schema::build(QueryRoot, MutationRoot, SubscriptionRoot).finish(); let resp = tokio::runtime::Runtime::new() .expect("tokio runtime") - .block_on(schema.execute("{ __schema { mutationType { fields { name } } } }")); + .block_on(schema.execute( + "{ __schema { mutationType { fields(includeDeprecated: true) { name } } } }", + )); assert!( resp.errors.is_empty(), "schema introspection failed: {:?}", @@ -661,17 +744,19 @@ mod authz_guard { out } - /// The introspection field set (the completeness source of truth) is exactly the + /// The SDL field set (the completeness source of truth, #219) is exactly the /// registered mutations. If async-graphql adds or renames a field, this reddens - /// and forces the `every_graphql_mutation_has_its_gate` table to be updated. + /// and forces the `every_graphql_mutation_has_its_gate` table to be updated. Uses + /// SDL, not introspection: introspection omits `visible = false` fields, so an + /// introspection-based pin here would go green while a hidden mutation existed. #[test] fn schema_lists_exactly_the_registered_mutations() { - let mut fields = graphql_mutation_fields_from_schema(); + let mut fields = graphql_mutation_fields_from_sdl(); fields.sort(); assert_eq!( fields, vec!["claimTask", "completeTask", "createTask", "failTask"], - "the introspected mutation field set is the authoritative discovered set" + "the SDL mutation field set is the authoritative discovered set" ); }