Skip to content

Commit 6e29147

Browse files
committed
fix(node): fail closed on claim for unresolvable repo
task_claimable returned true for an unassigned task whose repo_id is slash-form (mirror row) or names a repo this node does not host, while task_visible fails closed on the same rows. A signed stranger could claim such a task even though reads 404. Return false for a Some(repo_id) the node cannot resolve to a locally-hosted repo, matching task_visible's fail-closed check; keep the None arm open (repo-less tasks claimable by design). Adds a regression test asserting a stranger's claim 404s and leaves the assignee unchanged for both row classes.
1 parent fd75b92 commit 6e29147

1 file changed

Lines changed: 56 additions & 2 deletions

File tree

‎crates/gitlawb-node/src/api/tasks.rs‎

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -460,11 +460,16 @@ pub(crate) fn task_claimable(
460460
// Unscoped open task (no repo_id) is claimable by any authenticated agent
461461
return true;
462462
};
463+
// Fail closed when the task's repo cannot be resolved to a locally-hosted
464+
// repo: slash-form ids are mirror rows and a repo the node does not host
465+
// carries no visibility rules, so neither can establish claim eligibility.
466+
// This mirrors the fail-closed check `task_visible` already runs on the
467+
// same row class; unscoped repo-less tasks stay claimable by design.
463468
if repo_id.contains('/') {
464-
return true;
469+
return false;
465470
}
466471
let Some(record) = repos_by_id.get(repo_id) else {
467-
return true;
472+
return false;
468473
};
469474
let rules = rules_by_repo
470475
.get(&record.id)
@@ -2094,6 +2099,55 @@ mod visible_tasks_tests {
20942099
assert_not_found_envelope(&body_json(claim_resp).await);
20952100
}
20962101

2102+
/// Fail-closed claim gate: an unassigned task whose `repo_id` is slash-form
2103+
/// (mirror row) or names a repo this node does not host is claimable by
2104+
/// nobody, matching `task_visible`'s fail-closed check on the same row
2105+
/// class. Goes RED if `task_claimable` re-opens the unknown-repo arms: the
2106+
/// stranger's claim would succeed and steal the task.
2107+
#[sqlx::test]
2108+
async fn claim_task_on_unresolvable_repo_task_returns_404_and_keeps_assignee(pool: PgPool) {
2109+
let state = test_state(pool).await;
2110+
state
2111+
.db
2112+
.create_task(&task("slash-task", Some("owner/mirror-repo"), DELEGATOR))
2113+
.await
2114+
.unwrap();
2115+
state
2116+
.db
2117+
.create_task(&task("ghost-task", Some("ghost-repo"), DELEGATOR))
2118+
.await
2119+
.unwrap();
2120+
2121+
for task_id in ["slash-task", "ghost-task"] {
2122+
let claim_resp = full_task_router(state.clone())
2123+
.oneshot(signed_request_as(
2124+
STRANGER,
2125+
Method::POST,
2126+
&format!("/api/v1/tasks/{task_id}/claim"),
2127+
Body::from(format!(r#"{{"assignee_did":"{STRANGER}"}}"#)),
2128+
))
2129+
.await
2130+
.unwrap();
2131+
assert_eq!(
2132+
claim_resp.status(),
2133+
StatusCode::NOT_FOUND,
2134+
"claiming an unresolvable-repo task must 404, not succeed or leak via 409"
2135+
);
2136+
assert_not_found_envelope(&body_json(claim_resp).await);
2137+
assert_eq!(
2138+
state
2139+
.db
2140+
.get_task(task_id)
2141+
.await
2142+
.unwrap()
2143+
.unwrap()
2144+
.assignee_did,
2145+
None,
2146+
"a denied claim must leave the task's assignee unchanged"
2147+
);
2148+
}
2149+
}
2150+
20972151
#[sqlx::test]
20982152
async fn open_repoless_task_create_claim_complete_lifecycle(pool: PgPool) {
20992153
let state = test_state(pool).await;

0 commit comments

Comments
 (0)