Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions crates/gitlawb-node/src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,11 @@ mod authz_guard {
(events, "list_repo_events", "authorize_repo_read("),
// Bucket C — signer-self: the acting DID is matched/bound to auth.0
(tasks, "create_task", "did_matches("),
// #496: create_task is signer-self AND owner-when-repo-scoped — a
// repo_id naming a hosted repo admits tasks only from its owner.
// Both halves are pinned: the did_matches row guards the signer
// binding, this one the repo-ownership gate.
(tasks, "create_task", "require_repo_owner("),
(tasks, "claim_task", "did_matches("),
(tasks, "complete_task", "did_matches("),
(tasks, "fail_task", "did_matches("),
Expand Down
34 changes: 34 additions & 0 deletions crates/gitlawb-node/src/api/tasks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,40 @@ pub async fn create_task(
if !crate::api::did_matches(&auth.0, &body.delegator_did) {
return Err(forbidden("delegator_did must be the authenticated signer"));
}
// #496: a caller-supplied repo_id must name a repo the caller owns. Without
// this, any signed caller can plant a task — payload and UCAN included —
// under a foreign repo id, where repo-gated task reads surface it exactly
// to that repo's readers and the named assignee can claim it. Resolve the
// id against hosted repos: a hosted, non-quarantined repo admits tasks
// only from its owner. An id naming no hosted repo (unknown, or
// quarantined — which is hidden as if it did not exist, so 403ing it
// would confirm a real id) is kept as an opaque label with no existence
// oracle either way; such ids resolve to no hosted repo, so repo-scoped
// task read gates treat them as unscoped (delegator/assignee-only under
// the #268/#464 visibility contract). Repo-less tasks are unaffected.
if let Some(repo_id) = body.repo_id.as_deref() {
let record = state.db.get_repo_by_id(repo_id).await.map_err(|e| {
(
StatusCode::INTERNAL_SERVER_ERROR,
Json(json!({ "error": e.to_string() })),
)
})?;
if let Some(record) = record {
let quarantined = state
.db
.is_repo_quarantined(&record.id)
.await
.map_err(|e| {
(
StatusCode::INTERNAL_SERVER_ERROR,
Json(json!({ "error": e.to_string() })),
)
})?;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
if !quarantined && crate::api::require_repo_owner(&record, &auth.0).is_err() {
return Err(forbidden("only the repo owner can file tasks against it"));
}
}
}
let now = Utc::now().to_rfc3339();
let task = AgentTask {
id: Uuid::new_v4().to_string(),
Expand Down
80 changes: 80 additions & 0 deletions crates/gitlawb-node/src/graphql/mutation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,27 @@ impl MutationRoot {
}
let delegator_did = caller.to_string();
let db = ctx.data_unchecked::<Arc<Db>>();
// #496: same repo-ownership gate as the REST handler — a repo_id
// naming a hosted, non-quarantined repo admits tasks only from its
// owner, so a stranger cannot file under a foreign repo id. Unknown
// or quarantined ids stay oracle-free opaque labels (unscoped under
// the #268/#464 read contract); repo-less tasks unaffected.
if let Some(repo_id) = input.repo_id.as_deref() {
let record = db
.get_repo_by_id(repo_id)
.await
.map_err(crate::graphql::graphql_db_err)?;
if let Some(record) = record {
let quarantined = db
.is_repo_quarantined(&record.id)
.await
.map_err(crate::graphql::graphql_db_err)?;
if !quarantined {
crate::api::require_repo_owner(&record, caller)
.map_err(crate::graphql::graphql_app_err)?;
}
}
}
let now = Utc::now().to_rfc3339();
let task = AgentTask {
id: Uuid::new_v4().to_string(),
Expand Down Expand Up @@ -332,4 +353,63 @@ mod tests {
errors(&resp)
);
}

/// #496 (GraphQL): createTask applies the same repo-ownership gate as the
/// REST handler — a stranger naming a hosted repo is rejected, while the
/// owner files.
#[sqlx::test]
async fn create_task_rejects_foreign_repo_id(pool: PgPool) {
let state = crate::test_support::test_state(pool).await;
let owner = "did:key:zGQLTASKOWNERAAAAAAAAAAAAAAAAAAAAAAAAAA";
let stranger = "did:key:zGQLTASKSTRANGERBBBBBBBBBBBBBBBBBBBBBB";
let now = chrono::Utc::now();
let repo = crate::db::RepoRecord {
id: uuid::Uuid::new_v4().to_string(),
name: "gql-task-gate-repo".to_string(),
owner_did: owner.to_string(),
description: None,
is_public: true,
default_branch: "main".to_string(),
created_at: now,
updated_at: now,
disk_path: "/tmp/gql-task-gate-repo".to_string(),
forked_from: None,
machine_id: None,
};
state.db.create_repo(&repo).await.expect("seed repo");
let schema = state.graphql_schema.as_ref();

let q = |actor: &str| {
format!(
r#"mutation {{ createTask(delegatorDid: "{actor}", input: {{ kind: "build", capability: "repo:write", repoId: "{}" }}) {{ id }} }}"#,
repo.id
)
};

// Stranger signs as themselves (so the signer binding passes) but
// names the victim's repo → rejected by the ownership gate, leaking
// nothing about the repo.
let resp = schema
.execute(Request::new(q(stranger)).data(AuthenticatedDid(stranger.into())))
.await;
let errs = errors(&resp);
assert!(
errs.contains("repo owner"),
"a non-owner must not file under a foreign repo id: {errs}"
);
assert!(
!errs.contains(&repo.id) && !errs.contains(owner),
"denial must leak nothing about the repo: {errs}"
);

// The owner files under their own repo id.
let resp = schema
.execute(Request::new(q(owner)).data(AuthenticatedDid(owner.into())))
.await;
assert!(
errors(&resp).is_empty(),
"the owner should file against their repo: {}",
errors(&resp)
);
}
}
106 changes: 106 additions & 0 deletions crates/gitlawb-node/src/test_support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,112 @@ mod tests {
);
}

/// #496: create_task must not bind a caller-supplied repo_id verbatim. A
/// signed caller naming a hosted repo it does not own is rejected (403,
/// leaking nothing about the repo); the owner files (201); repo-less and
/// unknown-id tasks stay allowed (unknown ids are oracle-free opaque
/// labels, unscoped under the #268/#464 read contract); and a quarantined
/// repo id is treated as unknown (no 403 oracle on quarantined existence).
#[sqlx::test]
async fn create_task_rejects_foreign_repo_id(pool: PgPool) {
let owner = "did:key:zTASKREPOOWNERAAAAAAAAAAAAAAAAAAAAAAAAAA";
let stranger = "did:key:zTASKREPOSTRANGERBBBBBBBBBBBBBBBBBBBBBB";
let state = test_state(pool).await;
let repo = seed_repo(owner, "task-gate-repo");
state.db.create_repo(&repo).await.expect("seed repo");

let router = || {
Router::new()
.route(
"/api/v1/tasks",
axum::routing::post(crate::api::tasks::create_task),
)
.with_state(state.clone())
};
let post_as = |signer: &str, body: String| {
router().oneshot(signed_request_as(
signer,
Method::POST,
"/api/v1/tasks",
Body::from(body),
))
};
let body_with = |signer: &str, repo_id: Option<&str>| {
let repo_field = repo_id
.map(|id| format!(r#","repo_id":"{id}""#))
.unwrap_or_default();
format!(
r#"{{"kind":"build","capability":"repo:write","delegator_did":"{signer}"{repo_field}}}"#
)
};

// Stranger filing under the victim's repo id → exact 403.
let resp = post_as(stranger, body_with(stranger, Some(&repo.id)))
.await
.unwrap();
assert_eq!(
resp.status(),
StatusCode::FORBIDDEN,
"a non-owner must not file tasks against a foreign repo id"
);
let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX)
.await
.unwrap();
let text = String::from_utf8_lossy(&bytes);
assert!(
text.contains(r#""error":"forbidden""#),
"denial must keep the forbidden envelope: {text}"
);
assert!(
!text.contains(&repo.id) && !text.contains(owner),
"denial must leak nothing about the repo: {text}"
);

// Owner filing under their own repo id → 201.
let resp = post_as(owner, body_with(owner, Some(&repo.id)))
.await
.unwrap();
assert_eq!(
resp.status(),
StatusCode::CREATED,
"the owner must be able to file tasks against their repo"
);

// Repo-less task → 201 (unaffected).
let resp = post_as(stranger, body_with(stranger, None)).await.unwrap();
assert_eq!(
resp.status(),
StatusCode::CREATED,
"repo-less task creation must keep working"
);

// Unknown repo id → 201 as an opaque label.
let resp = post_as(stranger, body_with(stranger, Some("no-such-repo-id")))
.await
.unwrap();
assert_eq!(
resp.status(),
StatusCode::CREATED,
"an id naming no hosted repo stays an opaque label"
);

// Quarantined repo id → treated as unknown (201), never a 403 that
// would confirm the id names a real repo.
state
.db
.set_repo_quarantine(&repo.id, true)
.await
.expect("quarantine");
let resp = post_as(stranger, body_with(stranger, Some(&repo.id)))
.await
.unwrap();
assert_eq!(
resp.status(),
StatusCode::CREATED,
"a quarantined repo id must not 403 (existence oracle)"
);
}

/// N3: get_tree gates on the REQUESTED subtree, not the repo root. A caller
/// denied a withheld subtree is rejected there (404) but passes the gate on a
/// non-withheld path (so the rejection is path-scoped, not repo-wide).
Expand Down
Loading