From b5fc7dd5bc04d01fb19a55e15eb5de3e769a538e Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Wed, 3 Jun 2026 09:22:50 -0500 Subject: [PATCH 1/2] feat: path-scoped repository visibility (Phase 1) Implements per-path read visibility for repositories, addressing #18. A repo's private status becomes a property of a path subtree rather than a whole-repo boolean, enforced on the git read path via the node's existing DID identity and RFC 9421 HTTP signatures. Phase 1 scope (whole-repo enforcement, data model for the rest): - visibility_rules data model (path_glob, mode, reader_dids) + DB access - visibility_check pure decision function (owner-always-allow, most- specific rule wins, reader-DID allow-list, is_public fallback) - enforcement on git_info_refs and git_upload_pack: unauthorized reads return 404 byte-identical to a missing repo (no existence leak) - owner-only management API: PUT/DELETE/GET /api/v1/repos/{o}/{r}/visibility - gl visibility set/remove/list CLI mode A (hide) is whole-repo ("/") only by construction: hiding a subtree's existence would rewrite ancestor tree hashes and diverge history. Subtrees use mode B (content withheld, SHAs intact); B-subtree clone filtering is deferred to a later phase and the CLI/API say so. 260 tests (69 node + 191 gl); fmt + clippy clean. --- crates/gitlawb-node/src/api/mod.rs | 1 + crates/gitlawb-node/src/api/repos.rs | 25 ++ crates/gitlawb-node/src/api/visibility.rs | 159 +++++++++++ crates/gitlawb-node/src/db/mod.rs | 131 +++++++++ crates/gitlawb-node/src/main.rs | 1 + crates/gitlawb-node/src/server.rs | 15 +- crates/gitlawb-node/src/visibility.rs | 245 +++++++++++++++++ crates/gl/src/http.rs | 18 ++ crates/gl/src/main.rs | 5 + crates/gl/src/visibility.rs | 310 ++++++++++++++++++++++ 10 files changed, 906 insertions(+), 4 deletions(-) create mode 100644 crates/gitlawb-node/src/api/visibility.rs create mode 100644 crates/gitlawb-node/src/visibility.rs create mode 100644 crates/gl/src/visibility.rs diff --git a/crates/gitlawb-node/src/api/mod.rs b/crates/gitlawb-node/src/api/mod.rs index b009daa..2595c48 100644 --- a/crates/gitlawb-node/src/api/mod.rs +++ b/crates/gitlawb-node/src/api/mod.rs @@ -17,4 +17,5 @@ pub mod repos; pub mod resolve; pub mod stars; pub mod tasks; +pub mod visibility; pub mod webhooks; diff --git a/crates/gitlawb-node/src/api/repos.rs b/crates/gitlawb-node/src/api/repos.rs index 96d322e..0993d4b 100644 --- a/crates/gitlawb-node/src/api/repos.rs +++ b/crates/gitlawb-node/src/api/repos.rs @@ -14,6 +14,7 @@ use crate::cert; use crate::error::{AppError, Result}; use crate::git::{smart_http, store}; use crate::state::AppState; +use crate::visibility::{visibility_check, Decision}; use crate::webhooks; // ── Request / Response types ─────────────────────────────────────────────── @@ -308,6 +309,7 @@ pub async fn git_info_refs( State(state): State, Path((owner, repo)): Path<(String, String)>, Query(query): Query, + auth: Option>, ) -> Result { let name = repo.trim_end_matches(".git"); tracing::info!(owner = %owner, repo = %name, "info/refs request"); @@ -322,6 +324,20 @@ pub async fn git_info_refs( .ok_or_else(|| AppError::BadRequest("missing ?service= parameter".into()))?; tracing::debug!(service = %service, repo = %name, "info/refs service"); + // Enforce read (clone/fetch) visibility. The push advertisement + // (service=git-receive-pack) is authorized separately on the + // git-receive-pack POST, so leave it untouched here. + if service == "git-upload-pack" { + let rules = state.db.list_visibility_rules(&record.id).await?; + let caller = auth.as_ref().map(|e| e.0 .0.as_str()); + if visibility_check(&rules, record.is_public, &record.owner_did, caller, "/") + == Decision::Deny + { + tracing::debug!(repo = %name, caller = ?caller, "info/refs read denied by visibility"); + return Err(AppError::RepoNotFound(format!("{owner}/{name}"))); + } + } + // For receive-pack (push), download the latest from Tigris so the client // sees the same refs that acquire_write() will operate on. let disk_path = if service == "git-receive-pack" { @@ -352,6 +368,7 @@ pub async fn git_info_refs( pub async fn git_upload_pack( State(state): State, Path((owner, repo)): Path<(String, String)>, + auth: Option>, body: Bytes, ) -> Result { let name = repo.trim_end_matches(".git"); @@ -361,6 +378,14 @@ pub async fn git_upload_pack( .await? .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{name}")))?; + let rules = state.db.list_visibility_rules(&record.id).await?; + let caller = auth.as_ref().map(|e| e.0 .0.as_str()); + if visibility_check(&rules, record.is_public, &record.owner_did, caller, "/") == Decision::Deny + { + tracing::debug!(repo = %name, caller = ?caller, "upload-pack read denied by visibility"); + return Err(AppError::RepoNotFound(format!("{owner}/{name}"))); + } + let disk_path = state .repo_store .acquire(&record.owner_did, &record.name) diff --git a/crates/gitlawb-node/src/api/visibility.rs b/crates/gitlawb-node/src/api/visibility.rs new file mode 100644 index 0000000..28c1c76 --- /dev/null +++ b/crates/gitlawb-node/src/api/visibility.rs @@ -0,0 +1,159 @@ +//! Path-scoped visibility management API. Owner-only, mirrors `api/protect.rs`. + +use axum::extract::{Extension, Path, State}; +use axum::http::StatusCode; +use axum::Json; +use serde::Deserialize; + +use crate::auth::AuthenticatedDid; +use crate::db::VisibilityMode; +use crate::error::{AppError, Result}; +use crate::state::AppState; + +#[derive(Deserialize)] +pub struct SetVisibilityRequest { + pub path_glob: String, + /// "a" or "b"; defaults to "b" if absent or unrecognized. + #[serde(default)] + pub mode: Option, + #[serde(default)] + pub reader_dids: Vec, +} + +#[derive(Deserialize)] +pub struct RemoveVisibilityRequest { + pub path_glob: String, +} + +fn require_owner(record: &crate::db::RepoRecord, caller: &str) -> Result<()> { + let owner_short = record + .owner_did + .split(':') + .next_back() + .unwrap_or(&record.owner_did); + if caller != record.owner_did && caller != owner_short { + return Err(AppError::BadRequest( + "only the repo owner can manage visibility".into(), + )); + } + Ok(()) +} + +/// PUT /api/v1/repos/{owner}/{repo}/visibility +pub async fn set_visibility( + State(state): State, + Extension(auth): Extension, + Path((owner, repo)): Path<(String, String)>, + Json(req): Json, +) -> Result<(StatusCode, Json)> { + let record = state + .db + .get_repo(&owner, &repo) + .await? + .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; + require_owner(&record, &auth.0)?; + + let mode = match req.mode.as_deref() { + Some("a") => VisibilityMode::A, + _ => VisibilityMode::B, + }; + + // Mode A hides existence and is only coherent for the whole repo; a subtree + // cannot hide its existence without rewriting git history (see spec). + if mode == VisibilityMode::A && req.path_glob != "/" { + return Err(AppError::BadRequest( + "mode 'a' (hide) is only allowed for the whole repo (path_glob '/'); use mode 'b' for subtrees".into(), + )); + } + + // An empty reader_dids list is valid and intentional: the owner is always + // allowed by visibility_check, so a "/" rule with no readers is exactly the + // whole-repo "private to owner only" case. + state + .db + .set_visibility_rule(&record.id, &req.path_glob, mode, &req.reader_dids, &auth.0) + .await?; + + let subtree_warning = req.path_glob != "/"; + tracing::info!( + repo = %repo, caller = %auth.0, path_glob = %req.path_glob, mode = %mode.as_str(), + subtree_pending = subtree_warning, + "visibility rule set" + ); + + Ok(( + StatusCode::CREATED, + Json(serde_json::json!({ + "status": "set", + "repo": format!("{owner}/{repo}"), + "path_glob": req.path_glob, + "mode": mode.as_str(), + "reader_dids": req.reader_dids, + "subtree_clone_enforcement_pending": subtree_warning, + })), + )) +} + +/// DELETE /api/v1/repos/{owner}/{repo}/visibility +pub async fn remove_visibility( + State(state): State, + Extension(auth): Extension, + Path((owner, repo)): Path<(String, String)>, + Json(req): Json, +) -> Result> { + let record = state + .db + .get_repo(&owner, &repo) + .await? + .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; + require_owner(&record, &auth.0)?; + + state + .db + .remove_visibility_rule(&record.id, &req.path_glob) + .await?; + + tracing::info!( + repo = %repo, caller = %auth.0, path_glob = %req.path_glob, + "visibility rule removed" + ); + + Ok(Json(serde_json::json!({ + "status": "removed", + "repo": format!("{owner}/{repo}"), + "path_glob": req.path_glob, + }))) +} + +/// GET /api/v1/repos/{owner}/{repo}/visibility +pub async fn list_visibility( + State(state): State, + Extension(auth): Extension, + Path((owner, repo)): Path<(String, String)>, +) -> Result> { + let record = state + .db + .get_repo(&owner, &repo) + .await? + .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; + require_owner(&record, &auth.0)?; + + let rules = state.db.list_visibility_rules(&record.id).await?; + let rules_json: Vec<_> = rules + .into_iter() + .map(|r| { + serde_json::json!({ + "path_glob": r.path_glob, + "mode": r.mode.as_str(), + "reader_dids": r.reader_dids, + "created_by": r.created_by, + }) + }) + .collect(); + + Ok(Json(serde_json::json!({ + "repo": format!("{owner}/{repo}"), + "count": rules_json.len(), + "rules": rules_json, + }))) +} diff --git a/crates/gitlawb-node/src/db/mod.rs b/crates/gitlawb-node/src/db/mod.rs index ff8c62b..d3e892f 100644 --- a/crates/gitlawb-node/src/db/mod.rs +++ b/crates/gitlawb-node/src/db/mod.rs @@ -22,6 +22,51 @@ pub struct RepoRecord { pub machine_id: Option, } +/// Per-rule replication mode for a visibility rule. +/// `A` hides existence entirely (only valid at whole-repo scope `/`). +/// `B` keeps object SHAs and the path visible but withholds content +/// (the only mode allowed for subtrees; enforced on clones in Phase 3). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum VisibilityMode { + A, + B, +} + +impl VisibilityMode { + pub fn as_str(&self) -> &'static str { + match self { + VisibilityMode::A => "a", + VisibilityMode::B => "b", + } + } + + pub fn from_db(s: &str) -> Self { + match s { + "a" => VisibilityMode::A, + "b" => VisibilityMode::B, + other => { + tracing::warn!("unknown visibility mode in DB: {other:?}, defaulting to B"); + VisibilityMode::B + } + } + } +} + +/// A path-scoped visibility rule. `path_glob` is "/" for whole-repo, or a +/// subtree pattern such as "/secret-pkg/**". The repo owner is always an +/// implicit reader regardless of `reader_dids`. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct VisibilityRule { + pub id: String, + pub repo_id: String, + pub path_glob: String, + pub mode: VisibilityMode, + pub reader_dids: Vec, + pub created_by: String, + pub created_at: DateTime, +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct PullRequest { pub id: String, @@ -564,6 +609,18 @@ const MIGRATIONS: &[Migration] = &[ UNIQUE(repo_id, branch) )"#, "CREATE INDEX IF NOT EXISTS idx_protected_branches_repo ON protected_branches(repo_id)", + // ── Path-scoped visibility ──────────────────────────────────────── + r#"CREATE TABLE IF NOT EXISTS visibility_rules ( + id TEXT NOT NULL PRIMARY KEY, + repo_id TEXT NOT NULL, + path_glob TEXT NOT NULL, + mode TEXT NOT NULL, + reader_dids TEXT NOT NULL, + created_by TEXT NOT NULL, + created_at TEXT NOT NULL, + UNIQUE(repo_id, path_glob) + )"#, + "CREATE INDEX IF NOT EXISTS idx_visibility_rules_repo ON visibility_rules(repo_id)", // ── Repo stars ────────────────────────────────────────────────── r#"CREATE TABLE IF NOT EXISTS repo_stars ( id TEXT NOT NULL PRIMARY KEY, @@ -2072,6 +2129,80 @@ impl Db { } } +// ── Path-scoped Visibility ──────────────────────────────────────────────────── + +impl Db { + pub async fn set_visibility_rule( + &self, + repo_id: &str, + path_glob: &str, + mode: VisibilityMode, + reader_dids: &[String], + created_by: &str, + ) -> Result<()> { + let id = Uuid::new_v4().to_string(); + let now = Utc::now().to_rfc3339(); + let readers = serde_json::to_string(reader_dids).unwrap_or_else(|_| "[]".to_string()); + sqlx::query( + "INSERT INTO visibility_rules + (id, repo_id, path_glob, mode, reader_dids, created_by, created_at) + VALUES ($1, $2, $3, $4, $5, $6, $7) + ON CONFLICT (repo_id, path_glob) DO UPDATE + SET mode = EXCLUDED.mode, + reader_dids = EXCLUDED.reader_dids, + created_by = EXCLUDED.created_by, + created_at = EXCLUDED.created_at", + ) + .bind(&id) + .bind(repo_id) + .bind(path_glob) + .bind(mode.as_str()) + .bind(&readers) + .bind(created_by) + .bind(&now) + .execute(&self.pool) + .await?; + Ok(()) + } + + pub async fn remove_visibility_rule(&self, repo_id: &str, path_glob: &str) -> Result<()> { + sqlx::query("DELETE FROM visibility_rules WHERE repo_id = $1 AND path_glob = $2") + .bind(repo_id) + .bind(path_glob) + .execute(&self.pool) + .await?; + Ok(()) + } + + pub async fn list_visibility_rules(&self, repo_id: &str) -> Result> { + let rows = sqlx::query( + "SELECT id, repo_id, path_glob, mode, reader_dids, created_by, created_at + FROM visibility_rules WHERE repo_id = $1 ORDER BY path_glob", + ) + .bind(repo_id) + .fetch_all(&self.pool) + .await?; + Ok(rows + .into_iter() + .map(|r| { + let readers: String = r.get("reader_dids"); + let created_at: String = r.get("created_at"); + VisibilityRule { + id: r.get("id"), + repo_id: r.get("repo_id"), + path_glob: r.get("path_glob"), + mode: VisibilityMode::from_db(&r.get::("mode")), + reader_dids: serde_json::from_str(&readers).unwrap_or_default(), + created_by: r.get("created_by"), + created_at: created_at + .parse::>() + .unwrap_or_else(|_| Utc::now()), + } + }) + .collect()) + } +} + // ── Repo Stars ──────────────────────────────────────────────────────────────── impl Db { diff --git a/crates/gitlawb-node/src/main.rs b/crates/gitlawb-node/src/main.rs index e12ae03..1946637 100644 --- a/crates/gitlawb-node/src/main.rs +++ b/crates/gitlawb-node/src/main.rs @@ -17,6 +17,7 @@ mod rate_limit; mod server; mod state; mod sync; +mod visibility; mod webhooks; use anyhow::{Context, Result}; diff --git a/crates/gitlawb-node/src/server.rs b/crates/gitlawb-node/src/server.rs index 16cb444..4a8ec37 100644 --- a/crates/gitlawb-node/src/server.rs +++ b/crates/gitlawb-node/src/server.rs @@ -14,7 +14,7 @@ use tracing::Level; use crate::api::{ agents, arweave, bounties, certs, changelog, events, ipfs, issues, labels, peers, profiles, - protect, pulls, register, replicas, repos, resolve, stars, tasks, webhooks, + protect, pulls, register, replicas, repos, resolve, stars, tasks, visibility, webhooks, }; use crate::auth; use crate::rate_limit; @@ -141,6 +141,12 @@ pub fn build_router(state: AppState) -> Router { .route( "/api/v1/repos/{owner}/{repo}/labels/{label}", axum::routing::delete(labels::remove_label), + ) + .route( + "/api/v1/repos/{owner}/{repo}/visibility", + axum::routing::put(visibility::set_visibility) + .delete(visibility::remove_visibility) + .get(visibility::list_visibility), ), state.clone(), ); @@ -336,18 +342,19 @@ pub fn build_router(state: AppState) -> Router { .route( "/api/v1/repos/{owner}/{repo}/replicas", get(replicas::list_replicas), - ) - .route("/{owner}/{repo}/info/refs", get(repos::git_info_refs)); + ); // git-upload-pack (clone/fetch) — same raised body limit as receive-pack so // large pack responses from the server don't get truncated on the client side. let git_read_routes = Router::new() + .route("/{owner}/{repo}/info/refs", get(repos::git_info_refs)) .route( "/{owner}/{repo}/git-upload-pack", post(repos::git_upload_pack), ) .layer(DefaultBodyLimit::disable()) - .layer(RequestBodyLimitLayer::new(pack_limit)); + .layer(RequestBodyLimitLayer::new(pack_limit)) + .layer(middleware::from_fn(auth::optional_signature)); // ── Meta ────────────────────────────────────────────────────────────── let meta_routes = Router::new() diff --git a/crates/gitlawb-node/src/visibility.rs b/crates/gitlawb-node/src/visibility.rs new file mode 100644 index 0000000..b246dbf --- /dev/null +++ b/crates/gitlawb-node/src/visibility.rs @@ -0,0 +1,245 @@ +//! Pure read-authorization logic for path-scoped visibility. +//! +//! `visibility_check` decides whether a caller may read a given path in a repo, +//! based on the repo's visibility rules with a fallback to the legacy +//! `is_public` flag. It performs no I/O so it is exhaustively unit tested. + +use crate::db::VisibilityRule; + +#[derive(Debug, PartialEq, Eq)] +pub enum Decision { + Allow, + Deny, +} + +/// True if `caller` is the repo owner (matches full did:key or its short form), +/// mirroring the owner-match idiom in `api/protect.rs`. +fn is_owner(owner_did: &str, caller: &str) -> bool { + let owner_short = owner_did.split(':').next_back().unwrap_or(owner_did); + caller == owner_did || caller == owner_short +} + +/// The match prefix for a glob: "/" stays "/", "/secret/**" becomes "/secret". +fn glob_prefix(glob: &str) -> &str { + let p = glob.trim_end_matches("**").trim_end_matches('/'); + if p.is_empty() { + "/" + } else { + p + } +} + +/// Does `glob` match `path`? "/" matches everything; "/secret" matches +/// "/secret" and any "/secret/..." descendant. +fn glob_matches(glob: &str, path: &str) -> bool { + let prefix = glob_prefix(glob); + if prefix == "/" { + return true; + } + path == prefix || path.starts_with(&format!("{prefix}/")) +} + +/// Specificity = length of the match prefix; longer is more specific. +fn specificity(glob: &str) -> usize { + glob_prefix(glob).len() +} + +/// Decide whether `caller` (None = anonymous) may read `path` in a repo. +/// `path` is "/" for a whole-repo clone/fetch. +/// +/// Reader DIDs in a rule are matched exactly, so they must be stored in full +/// `did:key:...` form. The owner is the only identity matched in both full and +/// short form. +pub fn visibility_check( + rules: &[VisibilityRule], + is_public: bool, + owner_did: &str, + caller: Option<&str>, + path: &str, +) -> Decision { + // The owner can always read everything. + if let Some(c) = caller { + if is_owner(owner_did, c) { + return Decision::Allow; + } + } + + // Most-specific matching rule wins. On equal specificity the last rule in + // DB order is chosen; `list_visibility_rules` orders by `path_glob`, so this + // is deterministic. + let best = rules + .iter() + .filter(|r| glob_matches(&r.path_glob, path)) + .max_by_key(|r| specificity(&r.path_glob)); + + match best { + Some(rule) => { + // Phase 1 treats every matching rule as an allow-list keyed by + // `reader_dids`. `rule.mode` (A vs B) is stored from day one but not + // acted on here; it governs replication behavior in Phases 2-3. + let allowed = caller + .map(|c| rule.reader_dids.iter().any(|d| d == c)) + .unwrap_or(false); + if allowed { + Decision::Allow + } else { + Decision::Deny + } + } + None => { + if is_public { + Decision::Allow + } else { + Decision::Deny + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::db::VisibilityMode; + use chrono::Utc; + + fn rule(path_glob: &str, mode: VisibilityMode, readers: &[&str]) -> VisibilityRule { + VisibilityRule { + id: "x".into(), + repo_id: "r1".into(), + path_glob: path_glob.into(), + mode, + reader_dids: readers.iter().map(|s| s.to_string()).collect(), + created_by: "did:key:z6MkOwner".into(), + created_at: Utc::now(), + } + } + + const OWNER: &str = "did:key:z6MkOwner"; + + #[test] + fn no_rules_public_allows_anonymous() { + assert_eq!( + visibility_check(&[], true, OWNER, None, "/"), + Decision::Allow + ); + } + + #[test] + fn no_rules_private_denies_anonymous() { + assert_eq!( + visibility_check(&[], false, OWNER, None, "/"), + Decision::Deny + ); + } + + #[test] + fn root_rule_denies_anonymous() { + let rules = [rule("/", VisibilityMode::A, &[])]; + assert_eq!( + visibility_check(&rules, true, OWNER, None, "/"), + Decision::Deny + ); + } + + #[test] + fn root_rule_allows_owner() { + let rules = [rule("/", VisibilityMode::A, &[])]; + assert_eq!( + visibility_check(&rules, true, OWNER, Some(OWNER), "/"), + Decision::Allow + ); + } + + #[test] + fn root_rule_allows_owner_short_form() { + let rules = [rule("/", VisibilityMode::A, &[])]; + assert_eq!( + visibility_check(&rules, true, OWNER, Some("z6MkOwner"), "/"), + Decision::Allow + ); + } + + #[test] + fn root_rule_allows_listed_reader() { + let rules = [rule("/", VisibilityMode::A, &["did:key:z6MkFriend"])]; + assert_eq!( + visibility_check(&rules, true, OWNER, Some("did:key:z6MkFriend"), "/"), + Decision::Allow + ); + } + + #[test] + fn root_rule_denies_unlisted_reader() { + let rules = [rule("/", VisibilityMode::A, &["did:key:z6MkFriend"])]; + assert_eq!( + visibility_check(&rules, true, OWNER, Some("did:key:z6MkStranger"), "/"), + Decision::Deny + ); + } + + #[test] + fn subtree_rule_matches_descendant_path() { + let rules = [rule( + "/secret/**", + VisibilityMode::B, + &["did:key:z6MkFriend"], + )]; + assert_eq!( + visibility_check( + &rules, + true, + OWNER, + Some("did:key:z6MkStranger"), + "/secret/a.rs" + ), + Decision::Deny + ); + assert_eq!( + visibility_check( + &rules, + true, + OWNER, + Some("did:key:z6MkFriend"), + "/secret/a.rs" + ), + Decision::Allow + ); + } + + #[test] + fn subtree_rule_does_not_affect_root_clone() { + // A subtree rule must not gate a whole-repo (path "/") read: the public + // fallback applies because the subtree glob does not match "/". + let rules = [rule("/secret/**", VisibilityMode::B, &[])]; + assert_eq!( + visibility_check(&rules, true, OWNER, None, "/"), + Decision::Allow + ); + } + + #[test] + fn most_specific_rule_wins() { + // Public repo, but /secret is locked. A stranger reading /secret is denied + // by the more specific rule even though "/" would allow. + let rules = [ + rule("/", VisibilityMode::A, &["did:key:z6MkStranger"]), + rule("/secret/**", VisibilityMode::B, &["did:key:z6MkFriend"]), + ]; + // stranger is a root reader but not a /secret reader + assert_eq!( + visibility_check( + &rules, + true, + OWNER, + Some("did:key:z6MkStranger"), + "/secret/a.rs" + ), + Decision::Deny + ); + // stranger can still read root + assert_eq!( + visibility_check(&rules, true, OWNER, Some("did:key:z6MkStranger"), "/"), + Decision::Allow + ); + } +} diff --git a/crates/gl/src/http.rs b/crates/gl/src/http.rs index 71d0e7f..32e90a1 100644 --- a/crates/gl/src/http.rs +++ b/crates/gl/src/http.rs @@ -34,6 +34,24 @@ impl NodeClient { .with_context(|| format!("GET {url}")) } + /// GET with RFC 9421 HTTP Signature auth, for owner-only read endpoints. + /// Signs over the empty body (same shape the node verifies for signed reads). + pub async fn get_signed(&self, path: &str) -> Result { + let url = format!("{}{}", self.node_url, path); + let kp = self + .keypair + .as_ref() + .context("get_signed requires an identity keypair")?; + let signed = sign_request(kp, "GET", path, b""); + let req = self + .inner + .get(&url) + .header("Content-Digest", signed.content_digest) + .header("Signature-Input", signed.signature_input) + .header("Signature", signed.signature); + req.send().await.with_context(|| format!("GET {url}")) + } + /// POST with JSON body + RFC 9421 HTTP Signature auth. pub async fn post(&self, path: &str, body: &[u8]) -> Result { let url = format!("{}{}", self.node_url, path); diff --git a/crates/gl/src/main.rs b/crates/gl/src/main.rs index 9946ffd..0af7398 100644 --- a/crates/gl/src/main.rs +++ b/crates/gl/src/main.rs @@ -30,6 +30,7 @@ mod status; mod sync; mod task; mod ucan_cmd; +mod visibility; mod webhook; mod whoami; @@ -119,6 +120,9 @@ enum Commands { /// Manage branch protection rules Protect(protect::ProtectArgs), + /// Manage path-scoped read visibility rules + Visibility(visibility::VisibilityArgs), + /// Show unified activity changelog for a repository Changelog(changelog::ChangelogArgs), @@ -167,6 +171,7 @@ async fn main() -> Result<()> { Commands::Agent(args) => agent::run(args).await, Commands::Profile(args) => profile::run(args).await, Commands::Protect(args) => protect::run(args).await, + Commands::Visibility(args) => visibility::run(args).await, Commands::Changelog(args) => changelog::run(args).await, Commands::Bounty(args) => bounty::run(args).await, Commands::Ucan(args) => ucan_cmd::run(args).await, diff --git a/crates/gl/src/visibility.rs b/crates/gl/src/visibility.rs new file mode 100644 index 0000000..eef9ecb --- /dev/null +++ b/crates/gl/src/visibility.rs @@ -0,0 +1,310 @@ +//! `gl visibility`: manage path-scoped read visibility rules. + +use anyhow::{Context, Result}; +use clap::{Args, Subcommand}; +use serde_json::Value; +use std::path::{Path, PathBuf}; + +use crate::http::NodeClient; +use crate::identity::load_keypair_from_dir; + +#[derive(Args)] +pub struct VisibilityArgs { + #[command(subcommand)] + pub cmd: VisibilityCmd, +} + +#[derive(Subcommand)] +pub enum VisibilityCmd { + /// Set a visibility rule. Use "/" for the whole repo. + Set { + /// Path glob, e.g. "/" or "/secret-pkg/**" + path_glob: String, + #[arg(long)] + repo: String, + /// Comma-separated reader DIDs allowed to read this path + #[arg(long, value_delimiter = ',')] + readers: Vec, + /// Replication mode: "a" (hide, whole-repo only) or "b" (lock content) + #[arg(long, default_value = "b")] + mode: String, + #[arg(long, default_value = "https://node.gitlawb.com", env = "GITLAWB_NODE")] + node: String, + #[arg(long)] + dir: Option, + }, + /// Remove a visibility rule. + Remove { + path_glob: String, + #[arg(long)] + repo: String, + #[arg(long, default_value = "https://node.gitlawb.com", env = "GITLAWB_NODE")] + node: String, + #[arg(long)] + dir: Option, + }, + /// List visibility rules for a repo (owner only). + List { + #[arg(long)] + repo: String, + #[arg(long, default_value = "https://node.gitlawb.com", env = "GITLAWB_NODE")] + node: String, + #[arg(long)] + dir: Option, + }, +} + +pub async fn run(args: VisibilityArgs) -> Result<()> { + match args.cmd { + VisibilityCmd::Set { + path_glob, + repo, + readers, + mode, + node, + dir, + } => cmd_set(path_glob, repo, readers, mode, node, dir).await, + VisibilityCmd::Remove { + path_glob, + repo, + node, + dir, + } => cmd_remove(path_glob, repo, node, dir).await, + VisibilityCmd::List { repo, node, dir } => cmd_list(repo, node, dir).await, + } +} + +async fn resolve_owner_repo( + repo: &str, + node: &str, + dir: Option<&Path>, +) -> Result<(String, String)> { + if let Some((owner, name)) = repo.split_once('/') { + return Ok((owner.to_string(), name.to_string())); + } + let short = if let Ok(kp) = load_keypair_from_dir(dir) { + let did = kp.did().to_string(); + did.split(':').next_back().unwrap_or(&did).to_string() + } else { + let client = NodeClient::new(node, None); + let info: Value = client + .get("/") + .await? + .json() + .await + .context("failed to fetch node info")?; + let did = info["did"].as_str().context("node missing DID")?; + did.split(':').next_back().unwrap_or(did).to_string() + }; + Ok((short, repo.to_string())) +} + +async fn cmd_set( + path_glob: String, + repo: String, + readers: Vec, + mode: String, + node: String, + dir: Option, +) -> Result<()> { + let kp = load_keypair_from_dir(dir.as_deref()) + .context("identity not found, run `gl identity new` first")?; + let (owner, name) = resolve_owner_repo(&repo, &node, dir.as_deref()).await?; + let client = NodeClient::new(&node, Some(kp)); + + let body = serde_json::to_vec(&serde_json::json!({ + "path_glob": path_glob, + "mode": mode, + "reader_dids": readers, + }))?; + + let resp = client + .put(&format!("/api/v1/repos/{owner}/{name}/visibility"), &body) + .await + .context("failed to connect to node")?; + + let status = resp.status(); + let body: Value = resp.json().await.unwrap_or_default(); + if !status.is_success() { + let msg = body["message"].as_str().unwrap_or("unknown error"); + anyhow::bail!("visibility set failed ({status}): {msg}"); + } + + println!("✓ Visibility rule set on {owner}/{name}: {path_glob} (mode {mode})"); + if path_glob != "/" { + println!( + " Note: subtree content is NOT withheld from clones yet (Phase 3). Only whole-repo (\"/\") rules are enforced today. This rule is stored and will take effect when subtree enforcement ships." + ); + } + Ok(()) +} + +async fn cmd_remove( + path_glob: String, + repo: String, + node: String, + dir: Option, +) -> Result<()> { + let kp = load_keypair_from_dir(dir.as_deref()) + .context("identity not found, run `gl identity new` first")?; + let (owner, name) = resolve_owner_repo(&repo, &node, dir.as_deref()).await?; + let client = NodeClient::new(&node, Some(kp)); + + let body = serde_json::to_vec(&serde_json::json!({ "path_glob": path_glob }))?; + let resp = client + .delete(&format!("/api/v1/repos/{owner}/{name}/visibility"), &body) + .await + .context("failed to connect to node")?; + + let status = resp.status(); + let body: Value = resp.json().await.unwrap_or_default(); + if !status.is_success() { + let msg = body["message"].as_str().unwrap_or("unknown error"); + anyhow::bail!("visibility remove failed ({status}): {msg}"); + } + + println!("✓ Visibility rule removed from {owner}/{name}: {path_glob}"); + Ok(()) +} + +async fn cmd_list(repo: String, node: String, dir: Option) -> Result<()> { + let kp = load_keypair_from_dir(dir.as_deref()) + .context("identity not found, run `gl identity new` first")?; + let (owner, name) = resolve_owner_repo(&repo, &node, dir.as_deref()).await?; + let client = NodeClient::new(&node, Some(kp)); + + // owner-only endpoint: must send a signed request + let resp = client + .get_signed(&format!("/api/v1/repos/{owner}/{name}/visibility")) + .await + .context("failed to connect to node")?; + + let status = resp.status(); + let body: Value = resp.json().await.unwrap_or_default(); + if !status.is_success() { + let msg = body["message"].as_str().unwrap_or("unknown error"); + anyhow::bail!("visibility list failed ({status}): {msg}"); + } + + let rules = body["rules"].as_array().cloned().unwrap_or_default(); + if rules.is_empty() { + println!("No visibility rules on {owner}/{name} (repo follows its is_public flag)."); + } else { + println!("Visibility rules for {owner}/{name}:"); + for r in rules { + let glob = r["path_glob"].as_str().unwrap_or("?"); + let mode = r["mode"].as_str().unwrap_or("?"); + let readers = r["reader_dids"].as_array().cloned().unwrap_or_default(); + let readers: Vec<&str> = readers.iter().filter_map(|d| d.as_str()).collect(); + let readers_str = if readers.is_empty() { + "none".to_string() + } else { + readers.join(", ") + }; + println!(" {glob} (mode {mode}) readers: {readers_str}"); + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn test_cmd_set_success() { + let mut server = mockito::Server::new_async().await; + let dir = tempfile::TempDir::new().unwrap(); + let kp = gitlawb_core::identity::Keypair::generate(); + std::fs::write( + dir.path().join("identity.pem"), + kp.to_pem().unwrap().as_bytes(), + ) + .unwrap(); + + let _m = server + .mock( + "PUT", + mockito::Matcher::Regex(r"^/api/v1/repos/[^/]+/myrepo/visibility".to_string()), + ) + .with_status(201) + .with_header("content-type", "application/json") + .with_body(r#"{"status":"set"}"#) + .create_async() + .await; + + cmd_set( + "/".to_string(), + "myrepo".to_string(), + vec!["did:key:abc".to_string()], + "b".to_string(), + server.url(), + Some(dir.path().to_path_buf()), + ) + .await + .unwrap(); + } + + #[tokio::test] + async fn test_cmd_list_success() { + let mut server = mockito::Server::new_async().await; + let dir = tempfile::TempDir::new().unwrap(); + let kp = gitlawb_core::identity::Keypair::generate(); + std::fs::write( + dir.path().join("identity.pem"), + kp.to_pem().unwrap().as_bytes(), + ) + .unwrap(); + + let _m = server + .mock( + "GET", + mockito::Matcher::Regex(r"^/api/v1/repos/[^/]+/myrepo/visibility".to_string()), + ) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"rules":[{"path_glob":"/","mode":"b","reader_dids":["did:key:abc"]}]}"#) + .create_async() + .await; + + cmd_list( + "myrepo".to_string(), + server.url(), + Some(dir.path().to_path_buf()), + ) + .await + .unwrap(); + } + + #[tokio::test] + async fn test_cmd_remove_success() { + let mut server = mockito::Server::new_async().await; + let dir = tempfile::TempDir::new().unwrap(); + let kp = gitlawb_core::identity::Keypair::generate(); + std::fs::write( + dir.path().join("identity.pem"), + kp.to_pem().unwrap().as_bytes(), + ) + .unwrap(); + + let _m = server + .mock( + "DELETE", + mockito::Matcher::Regex(r"^/api/v1/repos/[^/]+/myrepo/visibility".to_string()), + ) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"status":"removed"}"#) + .create_async() + .await; + + cmd_remove( + "/".to_string(), + "myrepo".to_string(), + server.url(), + Some(dir.path().to_path_buf()), + ) + .await + .unwrap(); + } +} From 5a590643da9672e47a1dc8160f5f9dc5c3ece0fd Mon Sep 17 00:00:00 2001 From: beardthelion <56458543+beardthelion@users.noreply.github.com> Date: Thu, 4 Jun 2026 14:39:18 -0500 Subject: [PATCH 2/2] fix: address review on path-scoped visibility - migrations: move visibility_rules DDL out of the v1 migration into a new v3 (CodeRabbit, critical). Appending to v1 meant nodes that had already recorded v1 would skip the statements forever, so the table would be missing after upgrade. v3 matches the convention #23 set with v2 (agent_profiles) and the documented 'never append to v1' rule. - api: validate path_glob on set (must start with '/', no trailing '/', only a trailing '/**' wildcard, '/' for whole-repo not '/**'). Empty or slash-less globs would otherwise silently misconfigure access. Added unit tests for the accepted/rejected forms. 281 -> 283 tests pass (88 node + 195 gl); fmt + clippy clean. --- crates/gitlawb-node/src/api/visibility.rs | 48 +++++++++++++++++++++++ crates/gitlawb-node/src/db/mod.rs | 29 ++++++++------ 2 files changed, 65 insertions(+), 12 deletions(-) diff --git a/crates/gitlawb-node/src/api/visibility.rs b/crates/gitlawb-node/src/api/visibility.rs index 28c1c76..531c724 100644 --- a/crates/gitlawb-node/src/api/visibility.rs +++ b/crates/gitlawb-node/src/api/visibility.rs @@ -39,6 +39,32 @@ fn require_owner(record: &crate::db::RepoRecord, caller: &str) -> Result<()> { Ok(()) } +/// Reject malformed globs before they reach the store, where they would +/// silently misconfigure access (an empty glob behaves like "/", and a glob +/// without a leading "/" never matches a real repo path). The accepted forms +/// match what `visibility_check` understands: "/", "/prefix", or "/prefix/**". +fn validate_path_glob(path_glob: &str) -> Result<()> { + if !path_glob.starts_with('/') { + return Err(AppError::BadRequest("path_glob must start with '/'".into())); + } + if path_glob == "/**" { + return Err(AppError::BadRequest( + "use '/' for whole-repo scope, not '/**'".into(), + )); + } + if path_glob != "/" && path_glob.ends_with('/') { + return Err(AppError::BadRequest( + "path_glob must not end with '/'".into(), + )); + } + if path_glob.contains('*') && !path_glob.ends_with("/**") { + return Err(AppError::BadRequest( + "the only supported wildcard is a trailing '/**'".into(), + )); + } + Ok(()) +} + /// PUT /api/v1/repos/{owner}/{repo}/visibility pub async fn set_visibility( State(state): State, @@ -52,6 +78,7 @@ pub async fn set_visibility( .await? .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?; require_owner(&record, &auth.0)?; + validate_path_glob(&req.path_glob)?; let mode = match req.mode.as_deref() { Some("a") => VisibilityMode::A, @@ -157,3 +184,24 @@ pub async fn list_visibility( "rules": rules_json, }))) } + +#[cfg(test)] +mod tests { + use super::validate_path_glob; + + #[test] + fn accepts_supported_globs() { + for g in ["/", "/secret", "/secret/**", "/a/b/c", "/a/b/**"] { + assert!(validate_path_glob(g).is_ok(), "{g} should be valid"); + } + } + + #[test] + fn rejects_malformed_globs() { + // empty, no leading slash, whole-repo via "/**", trailing slash, and + // non-trailing wildcards are all rejected. + for g in ["", "secret/**", "/**", "/secret/", "/a*b", "/*/x"] { + assert!(validate_path_glob(g).is_err(), "{g} should be rejected"); + } + } +} diff --git a/crates/gitlawb-node/src/db/mod.rs b/crates/gitlawb-node/src/db/mod.rs index d3e892f..6af5ee8 100644 --- a/crates/gitlawb-node/src/db/mod.rs +++ b/crates/gitlawb-node/src/db/mod.rs @@ -609,18 +609,6 @@ const MIGRATIONS: &[Migration] = &[ UNIQUE(repo_id, branch) )"#, "CREATE INDEX IF NOT EXISTS idx_protected_branches_repo ON protected_branches(repo_id)", - // ── Path-scoped visibility ──────────────────────────────────────── - r#"CREATE TABLE IF NOT EXISTS visibility_rules ( - id TEXT NOT NULL PRIMARY KEY, - repo_id TEXT NOT NULL, - path_glob TEXT NOT NULL, - mode TEXT NOT NULL, - reader_dids TEXT NOT NULL, - created_by TEXT NOT NULL, - created_at TEXT NOT NULL, - UNIQUE(repo_id, path_glob) - )"#, - "CREATE INDEX IF NOT EXISTS idx_visibility_rules_repo ON visibility_rules(repo_id)", // ── Repo stars ────────────────────────────────────────────────── r#"CREATE TABLE IF NOT EXISTS repo_stars ( id TEXT NOT NULL PRIMARY KEY, @@ -715,6 +703,23 @@ const MIGRATIONS: &[Migration] = &[ )"#, ], }, + Migration { + version: 3, + name: "visibility_rules", + stmts: &[ + r#"CREATE TABLE IF NOT EXISTS visibility_rules ( + id TEXT NOT NULL PRIMARY KEY, + repo_id TEXT NOT NULL, + path_glob TEXT NOT NULL, + mode TEXT NOT NULL, + reader_dids TEXT NOT NULL, + created_by TEXT NOT NULL, + created_at TEXT NOT NULL, + UNIQUE(repo_id, path_glob) + )"#, + "CREATE INDEX IF NOT EXISTS idx_visibility_rules_repo ON visibility_rules(repo_id)", + ], + }, ]; // ── Repos ─────────────────────────────────────────────────────────────────────