Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
42 changes: 42 additions & 0 deletions crates/gitlawb-node/src/api/tasks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,20 @@ fn forbidden(msg: &str) -> (StatusCode, Json<Value>) {
)
}

/// 500 in this module's error shape with an opaque body: the real error is
/// logged server-side and never serialized (#250), matching the `db_error`
/// envelope `AppError::Db` renders.
fn db_error(e: anyhow::Error) -> (StatusCode, Json<Value>) {
tracing::error!(error = %format!("{e:#}"), "task repo gate database error");
(
StatusCode::INTERNAL_SERVER_ERROR,
Json(json!({
"error": "db_error",
"message": crate::error::DB_ERROR_MESSAGE,
})),
)
}

// ── Request / response types ──────────────────────────────────────────────────

#[derive(Deserialize)]
Expand Down Expand Up @@ -101,6 +115,34 @@ 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 (403 otherwise). 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; 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). The 403-vs-201 split between a hosted repo id and an unknown
// one is itself a one-bit oracle — a stranger holding a private repo's id
// learns it is hosted here — but denying foreign repos requires drawing
// exactly that line, and repo ids already surface via the task list on
// this base. 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(db_error)?;
if let Some(record) = record {
let quarantined = state
.db
.is_repo_quarantined(&record.id)
.await
.map_err(db_error)?;
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
126 changes: 126 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,109 @@ 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 without
/// persisting anything, while the owner files. The quarantined and
/// unknown-id arms are pinned here too, so this copy of the gate cannot
/// regress silently.
#[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 denial must not persist: no task row may carry the victim's
// repo id afterwards.
let stored = state
.db
.list_tasks(None, None, 50)
.await
.expect("list tasks");
assert!(
stored
.iter()
.all(|t| t.repo_id.as_deref() != Some(repo.id.as_str())),
"a denied write must not leave a task under the foreign repo id"
);

// 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)
);

// Unknown repo ids stay accepted as opaque labels.
let q_unknown = format!(
r#"mutation {{ createTask(delegatorDid: "{stranger}", input: {{ kind: "build", capability: "repo:write", repoId: "no-such-repo-id" }}) {{ id }} }}"#
);
let resp = schema
.execute(Request::new(q_unknown).data(AuthenticatedDid(stranger.into())))
.await;
assert!(
errors(&resp).is_empty(),
"an id naming no hosted repo stays an opaque label: {}",
errors(&resp)
);

// A quarantined repo id is treated as unknown (accepted, never a 403
// that would confirm the id names a real repo). This pins the
// `!quarantined` arm on this copy of the gate.
state
.db
.set_repo_quarantine(&repo.id, true)
.await
.expect("quarantine");
let resp = schema
.execute(Request::new(q(stranger)).data(AuthenticatedDid(stranger.into())))
.await;
assert!(
errors(&resp).is_empty(),
"a quarantined repo id must not 403 (existence oracle): {}",
errors(&resp)
);
}
}
120 changes: 120 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,126 @@ 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}"
);

// The denial must not persist: no task row may carry the victim's
// repo id afterwards — a 403 that still writes is the injection.
let stored = state
.db
.list_tasks(None, None, 50)
.await
.expect("list tasks");
assert!(
stored
.iter()
.all(|t| t.repo_id.as_deref() != Some(repo.id.as_str())),
"a denied write must not leave a task under the foreign repo id"
);

// 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