mirror of
https://github.com/Twigpine/node.git
synced 2026-11-12 19:13:02 -10:00
beardthelion
2202b0097f
* 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.