feat(node): enforce per-route authorization across the REST and GraphQL surface (#87)

* test(node): add HTTP-API integration harness with CI Postgres

Adds a #[cfg(test)] test-support module: a migrated AppState over a real
#[sqlx::test] pool (test_state), a DB-free variant (test_state_lazy), the
assembled router (app), and signed_request_as for injecting an authenticated
DID without RFC-9421 signing. A test-callable run_migrations reuses the
production migrate() path, and sqlx gains the macros/migrate features so
#[sqlx::test] is available. CI's test job gets a postgres service behind a
pg_isready health gate so the DB-backed tests can run.

The proving test exercises the owner gate on PUT /visibility end to end:
non-owner is rejected with 400, owner succeeds. signed_request_as sets a JSON
content-type so the Json extractor does not 415 before the handler runs.

* feat(node): enforce per-route authorization on REST mutations

Closes the missing-authorization cluster on the signed REST surface, where
'authenticated' meant 'any participant' because identities are permissionless.
Adds two shared helpers in api/mod.rs: require_repo_owner (403) and did_matches,
a method-safe DID comparison that collapses the full did:key vs bare short form
without matching across methods (did:web/did:gitlawb share the base58 space).

Per-route gates:
- merge_pr is owner-only (subsumes branch protection); close_pr and close_issue
  allow the repo owner or the PR/issue author (the issue author is read from the
  git-JSON blob, with a None author falling back to owner-only).
- create_review, create_comment, create_issue_comment, and create_bounty are
  read-gated (participants need read access, not ownership; this also closes the
  private-repo bounty leak).
- the task handlers bind the acting DID to the authenticated signer.
- dispute_bounty, previously ungated, now requires the creator or claimant; the
  other bounty and replica identity checks switch from raw equality to did_matches.

A source-level guard test asserts every in-scope mutation handler still carries
its expected gate marker, so a removed gate or an unclassified new route fails
CI. DB-backed tests (via the integration harness) cover merge owner-only and the
task signer-binding; did_matches has unit coverage for the cross-method case.

* feat(node): authenticate GraphQL mutations

GraphQL mutations were a fully unauthenticated parallel write path to the task
system (N2): the /graphql route had no auth layer and the resolvers took the
acting DID as plain arguments. Apply optional_signature to the /graphql POST so
a verified DID is attached when a signature is present (queries stay open), and
thread it into request-scoped data. Each mutation now reads the verified signer
via require_signer and binds the acting DID to it (rejecting an unsigned request
or one whose claimed delegator/assignee/by_did differs from the signer), reusing
did_matches for the comparison. Read-only subscriptions (/graphql/ws) are left
open. A resolver-level test covers the unsigned, mismatched, and matching cases.

* fix(node): path-scope the get_tree visibility gate

get_tree gated on the repo root ("/") regardless of the requested subtree, so
a caller denied a withheld subtree could still enumerate its filenames and blob
SHAs (N3). Gate on the requested path instead, mirroring get_blob, and reject
traversal segments. A cross-caller test shows the rejection is path-scoped: a
non-reader is denied the withheld subtree but passes the gate on a non-withheld
path.

* fix(node): authorize task completion and read-gate PR/issue creation

Two authorization gaps surfaced by an adversarial review of the auth stack,
both necessary-but-insufficient siblings of fixes already in it.

complete_task/fail_task bound the acting DID to the signer but never checked
the caller against the task's assignee, and finish_task updated by id alone, so
any authenticated did:key could finish or fail any task in any state. Load the
task and require the caller to be its assignee (REST and GraphQL), and add an
'AND status=claimed' predicate so only a claimed task transitions. The
now-unused by_did request fields are removed.

create_pr/create_issue resolved the repo with get_repo but skipped
authorize_repo_read, so a non-reader could open a PR (firing the owner's
webhooks) or file an issue against a private repo they cannot read. Gate both
on read access like their create_review/create_comment/create_bounty siblings.

Adds DB-backed tests covering non-assignee rejection (including the empty-body
bypass), the claimed-state predicate, and private-repo PR/issue denial.

* test(node): harden the authz drift-guard against false passes

The guard asserted a marker STRING appeared in each handler body, which a bare
identifier in a comment or log line could satisfy even after the real gate was
deleted, and its body slice over-ran on any handler not declared exactly
'pub async fn'.

- Markers are now gate-shaped: a call (require_repo_owner(, did_matches(,
  authorize_repo_read() or a binding/comparison expression
  (caller != &record.owner_did, let owner_did = auth.0), never a field name.
- Full-line comments are stripped before matching, so a gate that survives only
  as a comment no longer counts as enforced.
- The body slice bounds at the next top-level fn item across pub async,
  pub(crate) async, async, pub, and bare fn forms.
- Adds rows for the now-read-gated create_pr/create_issue and for fork_repo,
  create_repo, set_profile, claim_bounty; register is excluded with a note (it
  trusts the body did, tracked as P3 D3-1).
- Adds a self-test proving a comment-only marker does not satisfy a row.

* fix(node): P3 consistency batch from the adversarial review

Five low-severity items from the stack review. The sixth (a did_matches
false-negative on non-canonical multibase encodings of the same key) is left
as-is: it is a self-inflicted owner lockout unreachable with the conforming
client, never an impersonation vector, so decoding keys in the auth path is not
warranted.

- register bound the registered DID to the request body, not the signer, so a
  signed caller could create or refresh a trust row under a victim DID. Bind it
  to auth.0 (403 on mismatch) and add a drift-guard row now that it is gated.
- Bounty submit/approve/cancel returned 400 for an identity-mismatch denial
  where dispute returned 403. Return Forbidden, matching the rest of the surface.
- The visibility/protect owner gates (require_owner and the protect.rs inline
  checks) returned 400 for a non-owner; return 403 like require_repo_owner, and
  assert the exact code in the harness.
- get_tree rejected ./.. segments but not empty interior segments, so a path
  like secret//x could reach git while the gate saw a different string. Reject
  empty interior segments too (the empty path remains the root listing).
- Tighten the get_tree path-scope test to assert an exact 200 on the
  non-withheld branch so a future upstream 4xx/5xx cannot masquerade as
  gate-pass.

Adds a register-binding regression test.

* fix(node): unify protect/visibility owner checks on did_matches

CodeRabbit flagged that protect_branch/unprotect_branch and visibility's
require_owner used a trailing-segment owner compare (split(':').next_back())
distinct from the hardened did_matches, so the two owner-match idioms could
drift. It is not exploitable (the authenticated caller is always the full did:
form while the short form is a bare suffix, so no wrong caller passes), but it
is a real inconsistency, and the trailing-segment form is a false-negative for a
bare-stored owner. Replace both with did_matches (collapses did:key full vs bare
on both sides, never across methods), and extend the drift guard to assert
require_owner itself uses did_matches, not just that it is called.

Author: beardthelion

.github/workflows/pr-checks.yml

@@ -54,6 +54,23 @@ jobs:
         toolchain: [stable, beta]
     # Beta is informational: an upstream beta regression should warn, not block merges.
     continue-on-error: ${{ matrix.toolchain == 'beta' }}
+    # Postgres for the DB-backed integration tests (#[sqlx::test] provisions an
+    # isolated database per test off this connection). The health-check gate keeps
+    # `cargo test` from racing a not-yet-ready server.
+    services:
+      postgres:
+        image: postgres:16
+        env:
+          POSTGRES_USER: postgres
+          POSTGRES_PASSWORD: postgres
+          POSTGRES_DB: gitlawb_test
+        ports:
+          - 5432:5432
+        options: >-
+          --health-cmd pg_isready
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
     steps:
       - name: Check out repository
         uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
@@ -71,6 +88,8 @@ jobs:
           key: ${{ matrix.toolchain }}
 
       - name: cargo test
+        env:
+          DATABASE_URL: postgres://postgres:postgres@localhost:5432/gitlawb_test
         run: cargo test --workspace
 
   build-release:

crates/gitlawb-node/Cargo.toml

@@ -28,7 +28,7 @@ async-graphql-axum = "7"
 tokio-stream = { version = "0.1", features = ["sync"] }
 tower = "0.5"
 tower-http = { version = "0.6", features = ["cors", "trace", "request-id", "limit"] }
-sqlx = { version = "0.8", features = ["postgres", "runtime-tokio-rustls", "chrono", "uuid"] }
+sqlx = { version = "0.8", features = ["postgres", "runtime-tokio-rustls", "chrono", "uuid", "macros", "migrate"] }
 clap = { version = "4", features = ["derive", "env"] }
 bytes = "1"
 libc = "0.2"

crates/gitlawb-node/src/api/bounties.rs

@@ -79,12 +79,9 @@ pub async fn create_bounty(
         return Err(AppError::BadRequest("amount must be positive".into()));
     }
 
-    // Verify repo exists
-    let _ = state
-        .db
-        .get_repo(&owner, &repo)
-        .await?
-        .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?;
+    // Read-gate: anyone who can read the repo may fund a bounty on it (this closes
+    // the private-repo bounty leak); ownership is not required.
+    crate::api::authorize_repo_read(&state, &owner, &repo, Some(auth.0.as_str()), "/").await?;
 
     let now = Utc::now().to_rfc3339();
     let bounty = BountyRecord {
@@ -213,8 +210,12 @@ pub async fn submit_bounty(
             bounty.status
         )));
     }
-    if bounty.claimant_did.as_deref() != Some(&auth.0) {
-        return Err(AppError::BadRequest("only the claimant can submit".into()));
+    let is_claimant = bounty
+        .claimant_did
+        .as_deref()
+        .is_some_and(|c| crate::api::did_matches(&auth.0, c));
+    if !is_claimant {
+        return Err(AppError::Forbidden("only the claimant can submit".into()));
     }
 
     let now = Utc::now().to_rfc3339();
@@ -249,8 +250,8 @@ pub async fn approve_bounty(
             bounty.status
         )));
     }
-    if bounty.creator_did != auth.0 {
-        return Err(AppError::BadRequest(
+    if !crate::api::did_matches(&auth.0, &bounty.creator_did) {
+        return Err(AppError::Forbidden(
             "only the bounty creator can approve".into(),
         ));
     }
@@ -296,8 +297,8 @@ pub async fn cancel_bounty(
             bounty.status
         )));
     }
-    if bounty.creator_did != auth.0 {
-        return Err(AppError::BadRequest(
+    if !crate::api::did_matches(&auth.0, &bounty.creator_did) {
+        return Err(AppError::Forbidden(
             "only the bounty creator can cancel".into(),
         ));
     }
@@ -332,6 +333,19 @@ pub async fn dispute_bounty(
         )));
     }
 
+    // Only the bounty creator or the current claimant may dispute (N19 — this
+    // endpoint had no identity check at all).
+    let is_creator = crate::api::did_matches(&auth.0, &bounty.creator_did);
+    let is_claimant = bounty
+        .claimant_did
+        .as_deref()
+        .is_some_and(|c| crate::api::did_matches(&auth.0, c));
+    if !is_creator && !is_claimant {
+        return Err(AppError::Forbidden(
+            "only the bounty creator or claimant can dispute this bounty".into(),
+        ));
+    }
+
     // Check if deadline exceeded
     if let Some(ref claimed_at) = bounty.claimed_at {
         if let Ok(claimed) = chrono::DateTime::parse_from_rfc3339(claimed_at) {

crates/gitlawb-node/src/api/issues.rs

@@ -39,11 +39,11 @@ pub async fn create_issue(
     Path((owner, repo)): Path<(String, String)>,
     Json(req): Json<CreateIssueRequest>,
 ) -> Result<(StatusCode, Json<IssueRecord>)> {
-    let record = state
-        .db
-        .get_repo(&owner, &repo)
-        .await?
-        .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?;
+    // Authorize the caller as a reader before accepting an issue: a non-reader
+    // must not be able to file an issue against a private repo they cannot read.
+    // Mirrors create_issue_comment / create_review / create_bounty.
+    let (record, _rules) =
+        crate::api::authorize_repo_read(&state, &owner, &repo, Some(auth.0.as_str()), "/").await?;
 
     let issue_id = Uuid::new_v4().to_string();
     let now = Utc::now().to_rfc3339();
@@ -161,11 +161,9 @@ pub async fn create_issue_comment(
         ));
     }
 
-    let record = state
-        .db
-        .get_repo(&owner, &repo)
-        .await?
-        .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?;
+    // Read-gate: a commenter must be able to read the repo, but need not own it.
+    let (record, _rules) =
+        crate::api::authorize_repo_read(&state, &owner, &repo, Some(auth.0.as_str()), "/").await?;
 
     let disk_path = state
         .repo_store
@@ -207,7 +205,7 @@ pub async fn list_issue_comments(
 /// POST /api/v1/repos/{owner}/{repo}/issues/{id}/close
 pub async fn close_issue(
     State(state): State<AppState>,
-    Extension(_auth): Extension<AuthenticatedDid>,
+    Extension(auth): Extension<AuthenticatedDid>,
     Path((owner, repo, issue_id)): Path<(String, String, String)>,
 ) -> Result<Json<serde_json::Value>> {
     let record = state
@@ -223,6 +221,33 @@ pub async fn close_issue(
         .map_err(|e| AppError::Git(e.to_string()))?;
     let disk_path = guard.path().to_path_buf();
 
+    // Owner OR issue author may close. The author lives in the issue's git-JSON
+    // blob (not a DB column); a None author (legacy issues) falls back to
+    // owner-only. Read it under the write guard, before mutating.
+    let author_did: Option<String> = match git_issues::get_issue(&disk_path, &issue_id) {
+        Ok(Some(raw)) => serde_json::from_str::<IssueRecord>(&raw)
+            .ok()
+            .and_then(|i| i.author),
+        Ok(None) => {
+            guard.release(false).await;
+            return Err(AppError::NotFound(format!("issue {issue_id} not found")));
+        }
+        Err(e) => {
+            guard.release(false).await;
+            return Err(AppError::Git(e.to_string()));
+        }
+    };
+    let is_owner = crate::api::require_repo_owner(&record, &auth.0).is_ok();
+    let is_author = author_did
+        .as_deref()
+        .is_some_and(|a| crate::api::did_matches(&auth.0, a));
+    if !is_owner && !is_author {
+        guard.release(false).await;
+        return Err(AppError::Forbidden(
+            "only the repo owner or the issue author can close this issue".into(),
+        ));
+    }
+
     let close_result = git_issues::close_issue(&disk_path, &issue_id);
 
     // Always release the advisory lock — even on error; upload to Tigris only on success.

crates/gitlawb-node/src/api/labels.rs

@@ -17,7 +17,7 @@ pub struct LabelRequest {
 /// POST /api/v1/repos/:owner/:repo/labels
 pub async fn add_label(
     State(state): State<AppState>,
-    Extension(_auth): Extension<AuthenticatedDid>,
+    Extension(auth): Extension<AuthenticatedDid>,
     Path((owner, name)): Path<(String, String)>,
     Json(req): Json<LabelRequest>,
 ) -> Result<(StatusCode, Json<serde_json::Value>)> {
@@ -39,6 +39,7 @@ pub async fn add_label(
         .get_repo(&owner, &name)
         .await?
         .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{name}")))?;
+    crate::api::require_repo_owner(&record, &auth.0)?;
 
     let added = state.db.add_label(&record.id, &label).await?;
     let status = if added {
@@ -55,14 +56,15 @@ pub async fn add_label(
 /// DELETE /api/v1/repos/:owner/:repo/labels/:label
 pub async fn remove_label(
     State(state): State<AppState>,
-    Extension(_auth): Extension<AuthenticatedDid>,
+    Extension(auth): Extension<AuthenticatedDid>,
     Path((owner, name, label)): Path<(String, String, String)>,
 ) -> Result<Json<serde_json::Value>> {
     let record = state
         .db
         .get_repo(&owner, &name)
         .await?
         .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{name}")))?;
+    crate::api::require_repo_owner(&record, &auth.0)?;
 
     state.db.remove_label(&record.id, &label).await?;
     Ok(Json(serde_json::json!({ "label": label, "removed": true })))