Compare commits

..
Author SHA1 Message Date
beardthelion c1b34027d0 ci: trigger the PR checks on a new head 2026-11-04 22:21:38 -06:00
beardthelion 14dae89cab docs(node): reconcile GraphQL fence docs with SDL discovery
Restore the historical OSS audit blocker text and add a dated status
update for #219. README and the maintainer roadmap now describe the
SDL-sourced fence and signer-plus-binding markers.
2026-08-30 16:10:26 -05:00
beardthelion 37e022dc74 fix(node): rebase #219 and require signer gate in the GraphQL fence
Rebase onto current main and skip the obsolete 0.5.1 lockfile commit.
The mutation fence now requires both require_signer(ctx)? and did_matches(
in every registered resolver, and each_mutation_requires_and_binds_signer
covers all four task mutations for unsigned and DID-mismatch denial.
2026-08-30 15:28:45 -05:00
beardthelion 31d7f27f0a 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<Q,M,S>` 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.
2026-08-30 15:06:36 -05:00
beardthelion d979d0e833 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.
2026-08-30 15:06:36 -05:00
beardthelion b948f21057 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.
2026-08-30 15:06:36 -05:00
beardthelion a2a03b7bf4 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.
2026-08-30 15:06:06 -05:00
7 changed files with 669 additions and 481 deletions

No files matched your search

+1 -1
View File
@@ -71,7 +71,7 @@ Known limitations:
- Agent lifecycle revocation is not enforced by HTTP Signature authorization; do not rely on removing or revoking an agent record to block a compromised signer.
- Read visibility is not a blanket data-classification boundary: task, IPFS-pin, and Arweave-anchor listings are not repository-gated; withheld path names can be visible to a root reader; and later visibility changes cannot retract content already announced or externally anchored.
- Peer writes are signed by upgraded nodes, but strict signed-peer enforcement is opt-in during rolling upgrades.
- Current GraphQL mutations require an authenticated signer, but there is no mutation-specific guardrail that prevents a future mutation from omitting that check.
- GraphQL mutations require a signed identity (a verified signer bound to the acting DID), are limited to the agent-task queue, and are covered by a CI completeness fence that requires signer acquisition and actor binding on every registered mutation; repo writes are not exposed over GraphQL and remain REST-only.
- Pull-request review comments do not yet have threaded line-level anchors, and merges do not enforce approval requirements.
See:
-195
View File
@@ -449,198 +449,3 @@ mod lock_pool_shed_tests {
);
}
}
/// #426: end-to-end at the handler seam. A corrupt object store behind a
/// resolving issue ref used to look identical to an absent issue: get returned
/// a fake 404, close returned a fake 404 after releasing the write guard, and
/// list silently under-reported. All three must now surface a git error.
#[cfg(test)]
mod corrupt_store_tests {
use super::*;
use axum::response::IntoResponse;
use sqlx::PgPool;
use tempfile::TempDir;
fn seed_repo(owner_did: &str, name: &str) -> crate::db::RepoRecord {
let now = Utc::now();
crate::db::RepoRecord {
id: Uuid::new_v4().to_string(),
name: name.to_string(),
owner_did: owner_did.to_string(),
description: None,
is_public: true,
default_branch: "main".to_string(),
created_at: now,
updated_at: now,
disk_path: format!("/tmp/{name}"),
forked_from: None,
machine_id: None,
}
}
/// A state whose repo store roots in `repos_dir`, so the test can lay down
/// a real repo and then corrupt it without touching /tmp.
async fn state_with_repos_dir(pool: &PgPool, repos_dir: &std::path::Path) -> AppState {
let mut state = crate::test_support::test_state(pool.clone()).await;
state.repo_store =
crate::git::repo_store::RepoStore::for_testing(repos_dir.to_path_buf(), pool.clone());
state
}
/// Bare repo at the store's expected path, holding one issue ref.
fn bare_repo_with_issue(
repos_dir: &std::path::Path,
owner: &str,
name: &str,
) -> (std::path::PathBuf, String) {
let repo_path = repos_dir
.join(owner.replace([':', '/'], "_"))
.join(format!("{name}.git"));
std::fs::create_dir_all(&repo_path).unwrap();
let out = std::process::Command::new("git")
.args(["init", "--bare"])
.arg(&repo_path)
.output()
.unwrap();
assert!(out.status.success());
let issue_id = "aaaabbbb-0000-0000-0000-000000000000".to_string();
git_issues::create_issue(
&repo_path,
&issue_id,
&serde_json::json!({
"id": issue_id,
"title": "t",
"status": "open",
})
.to_string(),
)
.unwrap();
(repo_path, issue_id)
}
/// Delete the blob object behind the issue ref and put garbage in its
/// place: `cat-file` fails while `rev-parse` still resolves the ref.
fn corrupt_issue_object(repo_path: &std::path::Path, issue_id: &str) {
let out = std::process::Command::new("git")
.args(["rev-parse", &format!("refs/gitlawb/issues/{issue_id}")])
.current_dir(repo_path)
.output()
.unwrap();
let oid = String::from_utf8(out.stdout).unwrap().trim().to_string();
let obj = repo_path.join("objects").join(&oid[..2]).join(&oid[2..]);
std::fs::remove_file(&obj).unwrap();
std::fs::write(&obj, b"garbage").unwrap();
}
/// The regression: closing over a corrupt store used to return a fake 404
/// (or panic deeper in). It must reach the caller as a git 500, and the
/// handler must return rather than unwind.
#[sqlx::test]
async fn close_issue_on_corrupt_store_returns_git_error_not_panic(pool: PgPool) {
let owner = "did:key:zISSUECORRUPTCCCCCCCCCCCCCCCCCCCCCCCCC";
let dir = TempDir::new().unwrap();
let state = state_with_repos_dir(&pool, dir.path()).await;
state
.db
.create_repo(&seed_repo(owner, "corrupt-close"))
.await
.expect("seed repo");
let (repo_path, issue_id) = bare_repo_with_issue(dir.path(), owner, "corrupt-close");
corrupt_issue_object(&repo_path, &issue_id);
let err = close_issue(
State(state),
Extension(AuthenticatedDid(owner.to_string())),
Path((owner.to_string(), "corrupt-close".to_string(), issue_id)),
)
.await
.expect_err("a corrupt store must error, not panic or 404");
assert_eq!(
err.into_response().status(),
StatusCode::INTERNAL_SERVER_ERROR,
"a read failure is a git 500, not a fake not-found"
);
}
/// get on a corrupt store: 500, not the 404 the fold used to produce.
#[sqlx::test]
async fn get_issue_on_corrupt_store_returns_500_not_404(pool: PgPool) {
let owner = "did:key:zISSUECORRUPTGETDDDDDDDDDDDDDDDDDDDDDDD";
let dir = TempDir::new().unwrap();
let state = state_with_repos_dir(&pool, dir.path()).await;
state
.db
.create_repo(&seed_repo(owner, "corrupt-get"))
.await
.expect("seed repo");
let (repo_path, issue_id) = bare_repo_with_issue(dir.path(), owner, "corrupt-get");
corrupt_issue_object(&repo_path, &issue_id);
let err = get_issue(
State(state),
Path((owner.to_string(), "corrupt-get".to_string(), issue_id)),
None,
)
.await
.expect_err("a corrupt store must error, not 404");
assert_eq!(
err.into_response().status(),
StatusCode::INTERNAL_SERVER_ERROR,
);
}
/// list on a corrupt store must fail outright; a 200 that silently drops
/// the unreadable issue is the other half of the fold.
#[sqlx::test]
async fn list_issues_on_corrupt_store_returns_git_error(pool: PgPool) {
let owner = "did:key:zISSUECORRUPTLISTEEEEEEEEEEEEEEEEEEEEEE";
let dir = TempDir::new().unwrap();
let state = state_with_repos_dir(&pool, dir.path()).await;
state
.db
.create_repo(&seed_repo(owner, "corrupt-list"))
.await
.expect("seed repo");
let (repo_path, issue_id) = bare_repo_with_issue(dir.path(), owner, "corrupt-list");
corrupt_issue_object(&repo_path, &issue_id);
let err = list_issues(
State(state),
Path((owner.to_string(), "corrupt-list".to_string())),
None,
)
.await
.expect_err("a corrupt store must error, not under-report");
assert_eq!(
err.into_response().status(),
StatusCode::INTERNAL_SERVER_ERROR,
);
}
/// Must-not direction: a genuinely absent issue still gets the 404.
#[sqlx::test]
async fn get_issue_on_absent_ref_still_returns_404(pool: PgPool) {
let owner = "did:key:zISSUEABSENTFFFFFFFFFFFFFFFFFFFFFFFFFFF";
let dir = TempDir::new().unwrap();
let state = state_with_repos_dir(&pool, dir.path()).await;
state
.db
.create_repo(&seed_repo(owner, "absent-get"))
.await
.expect("seed repo");
bare_repo_with_issue(dir.path(), owner, "absent-get");
let err = get_issue(
State(state),
Path((
owner.to_string(),
"absent-get".to_string(),
"ffffffff-0000-0000-0000-000000000000".to_string(),
)),
None,
)
.await
.expect_err("a missing issue is still not-found");
assert_eq!(err.into_response().status(), StatusCode::NOT_FOUND);
}
}
+567
View File
@@ -279,6 +279,573 @@ 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 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
/// 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 the SDL AST, not from this scrape.
#[test]
fn every_graphql_mutation_has_its_gate() {
// (Rust snake name, schema field in camelCase, expected gate markers). The
// four are Bucket C signer-self: `/graphql` uses optional_signature, so each
// resolver must call `require_signer(ctx)?` and bind the acting DID with
// `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",
&["require_signer(ctx)?", "did_matches("],
),
(
"claim_task",
"claimTask",
&["require_signer(ctx)?", "did_matches("],
),
(
"complete_task",
"completeTask",
&["require_signer(ctx)?", "did_matches("],
),
(
"fail_task",
"failTask",
&["require_signer(ctx)?", "did_matches("],
),
];
// 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, &registered_camel);
}
// Marker present (source scrape; the vacuity limit, backstopped by the
// adversarial tests): each resolver body carries every required gate marker.
let masked = mask_comments(include_str!("../graphql/mutation.rs"));
for (snake, _, markers) in registered {
let body = resolver_body(&masked, snake);
for marker in *markers {
assert!(
body.contains(marker),
"GraphQL mutation `{snake}` is missing its gate marker `{marker}`: gate removed"
);
}
}
}
/// 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};
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 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 \
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"
);
}
/// 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<Q, M, S>(
schema: &async_graphql::Schema<Q, M, S>,
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<Q, M, S>(
schema: &async_graphql::Schema<Q, M, S>,
) -> Vec<String>
where
Q: async_graphql::ObjectType + 'static,
M: async_graphql::ObjectType + 'static,
S: async_graphql::SubscriptionType + 'static,
{
use async_graphql::Value;
let resp = tokio::runtime::Runtime::new()
.expect("tokio runtime")
.block_on(schema.execute(
"{ __schema { mutationType { fields(includeDeprecated: true) { 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 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<String> {
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<String> {
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::<Vec<_>>(),
),
_ => 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 <snake>` 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<char> = 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 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. 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_sdl();
fields.sort();
assert_eq!(
fields,
vec!["claimTask", "completeTask", "createTask", "failTask"],
"the SDL 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<u32> {
let _ = (\"see https://x/y\", did_matches(a, b)); // URL string then gate, one line
Ok(0)
}
async fn last(&self) -> Result<u32> {
// 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.
+27 -240
View File
@@ -61,119 +61,6 @@ pub fn create_issue(repo_path: &Path, issue_id: &str, json: &str) -> Result<()>
Ok(())
}
/// Issue ids are UUIDs; refs under `refs/gitlawb/issues/` only ever carry
/// `[0-9a-zA-Z_-]` names. Anything else is not a value we pass to git.
fn issue_ref_name_is_safe(ref_name: &str) -> bool {
let Some(id) = ref_name.strip_prefix("refs/gitlawb/issues/") else {
return false;
};
!id.is_empty()
&& id
.chars()
.all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_')
}
/// Resolve a ref name without putting it on a command line: enumerate refs
/// (constant argv) and match in memory. A nonzero exit propagates, so "cannot
/// determine" is an error rather than a quiet `false`.
fn ref_name_resolves(repo_path: &Path, ref_name: &str) -> Result<bool> {
// allow-unbounded-git: failure-path existence recheck, module convention.
let out = Command::new("git")
.args(["for-each-ref", "--format=%(refname)"])
.current_dir(repo_path)
.output()
.context("failed to run git for-each-ref")?;
if !out.status.success() {
let stderr = String::from_utf8_lossy(&out.stderr);
anyhow::bail!("git for-each-ref failed: {}", stderr.trim());
}
let stdout = String::from_utf8_lossy(&out.stdout);
Ok(stdout.lines().any(|line| line.trim() == ref_name))
}
/// Read the blob behind an issue ref.
///
/// `Ok(None)` means the ref itself is absent. A ref that resolves but whose
/// object cannot be read (corrupt object, unreadable store, a mid-read gc) is
/// an error, not an absence (#426).
///
/// The ref name reaches git on stdin, never argv: `cat-file --batch` answers
/// `<oid> <type> <size>\n<body>\n` for a readable object and
/// `<spec> missing\n` for an absent ref or an unreadable object alike, so the
/// `missing` case is re-resolved by name enumeration to tell them apart.
fn read_issue_blob(repo_path: &Path, ref_name: &str) -> Result<Option<String>> {
if !issue_ref_name_is_safe(ref_name) {
anyhow::bail!("refusing issue ref outside the issues namespace: {ref_name}");
}
// allow-unbounded-git: this module's spawns predate the bounded runner and
// its callers run inside issue handlers that already hold the repo guard.
let mut child = Command::new("git")
.args(["cat-file", "--batch"])
.current_dir(repo_path)
.stdin(std::process::Stdio::piped())
.stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::piped())
.spawn()
.context("failed to spawn git cat-file --batch")?;
use std::io::Write;
// Reap the child even when the write fails so a stdin error cannot drop an
// unwaited Child and leak a zombie (#53).
let write_result = match child.stdin.take() {
Some(mut stdin) => stdin
.write_all(ref_name.as_bytes())
.and_then(|()| stdin.write_all(b"\n")),
None => Err(std::io::Error::new(
std::io::ErrorKind::BrokenPipe,
"git cat-file --batch stdin unavailable",
)),
};
let output = child
.wait_with_output()
.context("git cat-file --batch failed")?;
write_result.context("failed to write ref name to git cat-file stdin")?;
if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
anyhow::bail!("git cat-file --batch failed: {}", stderr.trim());
}
// "<spec> missing\n" covers an absent ref and an unreadable object; only a
// ref that still resolves is the error case.
let missing = format!("{ref_name} missing\n");
if output.stdout.as_slice() == missing.as_bytes() {
return match ref_name_resolves(repo_path, ref_name)? {
true => anyhow::bail!("issue ref resolves but its object is unreadable: {ref_name}"),
false => Ok(None),
};
}
// Header: "<oid> <type> <size>\n" then exactly <size> bytes plus a newline.
let header_end = output
.stdout
.iter()
.position(|b| *b == b'\n')
.context("unexpected git cat-file --batch output")?;
let header = String::from_utf8_lossy(&output.stdout[..header_end]);
let mut parts = header.split(' ');
parts.next();
let obj_type = parts.next().unwrap_or("");
if obj_type != "blob" {
anyhow::bail!("issue ref {ref_name} resolves to a non-blob object");
}
let size: usize = parts
.next()
.and_then(|s| s.parse().ok())
.context("unexpected git cat-file --batch header")?;
let body = &output.stdout[header_end + 1..];
anyhow::ensure!(
body.len() >= size,
"truncated git cat-file --batch body for {ref_name}"
);
Ok(Some(String::from_utf8_lossy(&body[..size]).to_string()))
}
/// List all issue refs and return their JSON content.
pub fn list_issues(repo_path: &Path) -> Result<Vec<String>> {
// List all refs under refs/gitlawb/issues/
@@ -187,11 +74,9 @@ pub fn list_issues(repo_path: &Path) -> Result<Vec<String>> {
.output()
.context("failed to run git for-each-ref")?;
// for-each-ref exits 0 with empty output when no refs match, so a nonzero
// exit is a real enumeration failure, not "no issues yet".
if !list_output.status.success() {
let stderr = String::from_utf8_lossy(&list_output.stderr);
anyhow::bail!("git for-each-ref failed: {}", stderr.trim());
// No issues yet
return Ok(vec![]);
}
let refs_str = String::from_utf8_lossy(&list_output.stdout);
@@ -203,12 +88,16 @@ pub fn list_issues(repo_path: &Path) -> Result<Vec<String>> {
continue;
}
// Read the blob content. A ref that vanished between for-each-ref and
// here is skipped; a ref that resolves but fails to read propagates so
// a degraded object store errors the listing instead of under-reporting.
match read_issue_blob(repo_path, ref_name)? {
Some(content) => issues.push(content),
None => continue,
// Read the blob content
let cat_output = Command::new("git")
.args(["cat-file", "blob", ref_name])
.current_dir(repo_path)
.output()
.context("failed to run git cat-file")?;
if cat_output.status.success() {
let content = String::from_utf8_lossy(&cat_output.stdout).to_string();
issues.push(content);
}
}
@@ -238,12 +127,8 @@ pub fn resolve_issue_id(repo_path: &Path, id_or_prefix: &str) -> Result<Option<S
.output()
.context("failed to run git for-each-ref")?;
// for-each-ref exits 0 with empty output when nothing matches, so a
// nonzero exit is a real enumeration failure (e.g. not a repo), not
// "issue not found".
if !list.status.success() {
let stderr = String::from_utf8_lossy(&list.stderr);
anyhow::bail!("git for-each-ref failed: {}", stderr.trim());
return Ok(None);
}
let output = String::from_utf8_lossy(&list.stdout);
@@ -276,12 +161,8 @@ pub fn close_issue(repo_path: &Path, issue_id: &str) -> Result<Option<String>> {
None => return Ok(None),
};
// A ref that resolved and is now absent was deleted between the two calls;
// any other read failure propagates as an error rather than panic (#426).
let raw = match get_issue(repo_path, &full_id)? {
Some(raw) => raw,
None => return Ok(None),
};
let raw = get_issue(repo_path, &full_id)?
.expect("ref existed in resolve but not in get — should be impossible");
let mut issue: serde_json::Value =
serde_json::from_str(&raw).context("invalid issue JSON in git ref")?;
@@ -300,7 +181,18 @@ pub fn get_issue(repo_path: &Path, issue_id: &str) -> Result<Option<String>> {
};
let ref_name = format!("refs/gitlawb/issues/{full_id}");
read_issue_blob(repo_path, &ref_name)
let cat_output = Command::new("git")
.args(["cat-file", "blob", &ref_name])
.current_dir(repo_path)
.output()
.context("failed to run git cat-file")?;
if !cat_output.status.success() {
return Ok(None);
}
let content = String::from_utf8_lossy(&cat_output.stdout).to_string();
Ok(Some(content))
}
#[cfg(test)]
@@ -402,109 +294,4 @@ mod tests {
let v: serde_json::Value = serde_json::from_str(&updated).unwrap();
assert_eq!(v["status"], "closed");
}
/// Overwrite the loose object behind an issue ref with garbage, so
/// `cat-file` fails while `rev-parse` still resolves the ref (#426).
fn corrupt_issue_object(dir: &TempDir, full_id: &str) {
let oid_out = Command::new("git")
.args(["rev-parse", &format!("refs/gitlawb/issues/{full_id}")])
.current_dir(dir.path())
.output()
.unwrap();
let oid = String::from_utf8(oid_out.stdout)
.unwrap()
.trim()
.to_string();
let obj = dir
.path()
.join(".git/objects")
.join(&oid[..2])
.join(&oid[2..]);
std::fs::remove_file(&obj).unwrap();
std::fs::write(&obj, b"garbage").unwrap();
}
#[test]
fn test_get_and_close_missing_issue_return_none() {
let dir = TempDir::new().unwrap();
init_repo(&dir);
assert_eq!(get_issue(dir.path(), "nosuch00").unwrap(), None);
assert_eq!(close_issue(dir.path(), "nosuch00").unwrap(), None);
}
// #426: a directory that is not a repo makes `cat-file --batch` exit
// nonzero; that failure is an error, not a quiet None.
#[test]
fn test_issue_read_on_non_repo_errors_instead_of_none() {
let dir = TempDir::new().unwrap();
let not_a_repo = dir.path().join("not-a-repo");
std::fs::create_dir_all(&not_a_repo).unwrap();
assert!(read_issue_blob(&not_a_repo, "refs/gitlawb/issues/deadbeef").is_err());
}
// #426: the ref name is constrained to the issues namespace and a safe
// charset before it is fed to git, even on stdin (a newline would smuggle
// a second `--batch` spec).
#[test]
fn test_issue_read_rejects_ref_outside_issues_namespace() {
let dir = TempDir::new().unwrap();
init_repo(&dir);
assert!(read_issue_blob(dir.path(), "refs/heads/main").is_err());
assert!(read_issue_blob(dir.path(), "refs/gitlawb/issues/../x").is_err());
}
// #426: a failed for-each-ref enumeration is an error, not an empty list.
#[test]
fn test_list_issues_on_non_repo_errors_instead_of_empty() {
let dir = TempDir::new().unwrap();
let not_a_repo = dir.path().join("not-a-repo");
std::fs::create_dir_all(&not_a_repo).unwrap();
assert!(list_issues(&not_a_repo).is_err());
}
// #426: the same enumeration failure inside resolve_issue_id must reach
// get_issue/close_issue as an error, not a "not found" None.
#[test]
fn test_get_and_close_on_non_repo_error_instead_of_none() {
let dir = TempDir::new().unwrap();
let not_a_repo = dir.path().join("not-a-repo");
std::fs::create_dir_all(&not_a_repo).unwrap();
assert!(get_issue(&not_a_repo, "nosuch00").is_err());
assert!(close_issue(&not_a_repo, "nosuch00").is_err());
}
// #426: a ref that resolves but whose object is corrupt is a read error,
// not an absent issue. `close_issue` used to `expect()` on that None and
// panic inside a request handler holding the write guard.
#[test]
fn test_corrupt_issue_object_errors_instead_of_panicking() {
let dir = TempDir::new().unwrap();
init_repo(&dir);
let full_id = "bad00000-0000-0000-0000-000000000000";
create_issue(dir.path(), full_id, r#"{"status":"open"}"#).unwrap();
corrupt_issue_object(&dir, full_id);
assert!(get_issue(dir.path(), full_id).is_err());
assert!(close_issue(dir.path(), full_id).is_err());
assert!(list_issues(dir.path()).is_err());
}
#[test]
fn test_list_issues_returns_all() {
let dir = TempDir::new().unwrap();
init_repo(&dir);
create_issue(
dir.path(),
"aaa00000-0000-0000-0000-000000000000",
r#"{"status":"open"}"#,
)
.unwrap();
create_issue(
dir.path(),
"bbb00000-0000-0000-0000-000000000000",
r#"{"status":"open"}"#,
)
.unwrap();
assert_eq!(list_issues(dir.path()).unwrap().len(), 2);
}
}
+65 -44
View File
@@ -186,57 +186,78 @@ mod tests {
.join("; ")
}
/// N2: GraphQL mutations require a verified signer and bind the acting DID to
/// it. Unsigned is rejected; a signer other than the claimed actor is
/// N2: every GraphQL mutation requires a verified signer and binds the acting
/// DID to it. Unsigned is rejected; a signer other than the claimed actor is
/// rejected; the matching signer passes the auth gate.
#[sqlx::test]
async fn mutation_requires_and_binds_signer(pool: PgPool) {
async fn each_mutation_requires_and_binds_signer(pool: PgPool) {
let state = crate::test_support::test_state(pool).await;
let schema = state.graphql_schema.as_ref();
let assignee = "did:key:zASSIGNEEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
let q = format!(
r#"mutation {{ claimTask(id: "no-such-task", assigneeDid: "{assignee}") {{ id }} }}"#
);
let actor = "did:key:zGQLACTORAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
let other = "did:key:zGQLOTHERBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB";
let cases: Vec<(&str, String)> = vec![
(
"createTask",
format!(
r#"mutation {{ createTask(delegatorDid: "{actor}", input: {{ kind: "build", capability: "repo:write" }}) {{ id }} }}"#
),
),
(
"claimTask",
format!(
r#"mutation {{ claimTask(id: "no-such-task", assigneeDid: "{actor}") {{ id }} }}"#
),
),
(
"completeTask",
format!(
r#"mutation {{ completeTask(id: "no-such-task", byDid: "{actor}", input: {{}}) {{ id }} }}"#
),
),
(
"failTask",
format!(
r#"mutation {{ failTask(id: "no-such-task", byDid: "{actor}", input: {{}}) {{ id }} }}"#
),
),
];
// 1. Unsigned → rejected before any DB work.
let resp = schema.execute(Request::new(&q)).await;
assert!(
errors(&resp).contains("authentication required"),
"unsigned mutation must be rejected: {}",
errors(&resp)
);
for (name, q) in &cases {
let resp = schema.execute(Request::new(q.as_str())).await;
assert!(
errors(&resp).contains("authentication required"),
"{name}: unsigned mutation must be rejected: {}",
errors(&resp)
);
// 2. Signed as someone other than the claimed assignee → rejected.
let resp = schema
.execute(Request::new(&q).data(AuthenticatedDid(
"did:key:zOTHERBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB".into(),
)))
.await;
assert!(
errors(&resp).contains("authenticated signer"),
"DID mismatch must be rejected: {}",
errors(&resp)
);
let resp = schema
.execute(Request::new(q.as_str()).data(AuthenticatedDid(other.into())))
.await;
assert!(
errors(&resp).contains("authenticated signer"),
"{name}: DID mismatch must be rejected: {}",
errors(&resp)
);
// 3. Signed as the claimed assignee → passes the auth gate. The missing
// task is a business error from claim_task, not a sqlx fault, so the
// actionable message must survive (not the opaque DB string) (#250).
let resp = schema
.execute(Request::new(&q).data(AuthenticatedDid(assignee.into())))
.await;
let errs = errors(&resp);
assert!(
!errs.contains("authentication required") && !errs.contains("authenticated signer"),
"matching signer must pass the auth gate: {errs}"
);
assert!(
errs.contains("task not claimable"),
"claim race / missing task must keep its business message: {errs}"
);
assert!(
!errs.contains(crate::graphql::GRAPHQL_DB_ERROR_MESSAGE),
"business error must not be rewritten as opaque DB error: {errs}"
);
let resp = schema
.execute(Request::new(q.as_str()).data(AuthenticatedDid(actor.into())))
.await;
let errs = errors(&resp);
assert!(
!errs.contains("authentication required") && !errs.contains("authenticated signer"),
"{name}: matching signer must pass the auth gate: {errs}"
);
if *name == "claimTask" {
assert!(
errs.contains("task not claimable"),
"claim race / missing task must keep its business message: {errs}"
);
assert!(
!errs.contains(crate::graphql::GRAPHQL_DB_ERROR_MESSAGE),
"business error must not be rewritten as opaque DB error: {errs}"
);
}
}
}
/// #250: mutation DB faults must be opaque; create_task hits agent_tasks.
+1 -1
View File
@@ -41,7 +41,7 @@ Owner focus: protocol/auth.
- Close default-open repo write authorization and wire trusted UCAN delegation into repository permissions.
- Close the remaining visibility gaps: restrict or document task, pin, and anchor metadata routes, and publish an irreversible-publication policy.
- 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 SDL, with an introspection cross-check for hidden fields, so none can slip in unlisted) and requires `require_signer` plus actor binding in each registered resolver; 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.
+8
View File
@@ -151,3 +151,11 @@ Risks:
4. Harden peer registration and outbound fetch behavior against SSRF and peer-list poisoning.
5. Add Docker/installer/release smoke tests to CI.
6. Label PoS/economics docs consistently with the current live contract and rewards status.
## Status updates (post-snapshot)
These entries record security-posture changes after the 2026-05-28 inspection above. They do not rewrite that snapshot's command output or test counts.
### 2026-08-30: GraphQL mutation auth and completeness fence (#87, #219)
At `main` from `bfc44f92` onward, GraphQL mutations require a verified signer bound to the acting DID. The surface stays limited to the agent-task queue; repo writes are not exposed over GraphQL. PR #219 adds `every_graphql_mutation_has_its_gate`, a CI completeness fence that discovers mutations from the built schema SDL (with an introspection cross-check for hidden fields) and requires signer acquisition plus actor binding in each registered resolver.