Skip to content

Commit 896aff0

Browse files
committed
fix(node): close quarantine bypass on encrypted blobs and CID serve
Route encrypted and visibility handlers through authorize_repo_read; exclude quarantined rows from list_all_repos; fold canonical quarantine into get_by_cid (with main walk-budget/admission); gate visibility mutations; and add load-bearing HTTP regressions for mirror-only and dual-row CID paths.
1 parent e5a8468 commit 896aff0

6 files changed

Lines changed: 678 additions & 79 deletions

File tree

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

Lines changed: 10 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -6,29 +6,24 @@ use axum::Json;
66
use crate::auth::AuthenticatedDid;
77
use crate::error::{AppError, Result};
88
use crate::state::AppState;
9-
use crate::visibility::{visibility_check, Decision};
109

1110
/// GET /api/v1/repos/{owner}/{repo}/encrypted-blobs
1211
/// Returns [{oid, cid}] for every encrypted blob in the repo, to any caller who
1312
/// can read the repo. Not recipient-scoped: recipient identities are not stored,
1413
/// so access control here is repo readability and decryption is gated by the
1514
/// envelope crypto (only a real recipient can open an envelope).
15+
///
16+
/// Quarantined repos are opaque 404 via [`crate::api::authorize_repo_read`] —
17+
/// same as issues/changelogs — so a public-but-quarantined mirror cannot leak
18+
/// its encrypted blob index.
1619
pub async fn list_encrypted_blobs(
1720
State(state): State<AppState>,
1821
auth: Option<Extension<AuthenticatedDid>>,
1922
Path((owner, repo)): Path<(String, String)>,
2023
) -> Result<Json<serde_json::Value>> {
21-
let record = state
22-
.db
23-
.get_repo(&owner, &repo)
24-
.await?
25-
.ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?;
2624
let caller = auth.as_ref().map(|e| e.0 .0.as_str());
27-
let rules = state.db.list_visibility_rules(&record.id).await?;
28-
if visibility_check(&rules, record.is_public, &record.owner_did, caller, "/") == Decision::Deny
29-
{
30-
return Err(AppError::RepoNotFound(format!("{owner}/{repo}")));
31-
}
25+
let (record, _rules) =
26+
crate::api::authorize_repo_read(&state, &owner, &repo, caller, "/").await?;
3227
let rows = state.db.list_all_encrypted_blobs(&record.id).await?;
3328
let blobs: Vec<_> = rows
3429
.into_iter()
@@ -45,17 +40,9 @@ pub async fn get_encrypted_blob(
4540
auth: Option<Extension<AuthenticatedDid>>,
4641
Path((owner, repo, oid)): Path<(String, String, String)>,
4742
) -> Result<Vec<u8>> {
48-
let record = state
49-
.db
50-
.get_repo(&owner, &repo)
51-
.await?
52-
.ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?;
5343
let caller = auth.as_ref().map(|e| e.0 .0.as_str());
54-
let rules = state.db.list_visibility_rules(&record.id).await?;
55-
if visibility_check(&rules, record.is_public, &record.owner_did, caller, "/") == Decision::Deny
56-
{
57-
return Err(AppError::RepoNotFound(format!("{owner}/{repo}/{oid}")));
58-
}
44+
let (record, _rules) =
45+
crate::api::authorize_repo_read(&state, &owner, &repo, caller, "/").await?;
5946
let cid = state
6047
.db
6148
.encrypted_blob_cid(&record.id, &oid)
@@ -81,17 +68,9 @@ pub async fn replicate_encrypted_blobs(
8168
auth: Option<Extension<AuthenticatedDid>>,
8269
Path((owner, repo)): Path<(String, String)>,
8370
) -> Result<Json<serde_json::Value>> {
84-
let record = state
85-
.db
86-
.get_repo(&owner, &repo)
87-
.await?
88-
.ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?;
8971
let caller = auth.as_ref().map(|e| e.0 .0.as_str());
90-
let rules = state.db.list_visibility_rules(&record.id).await?;
91-
if visibility_check(&rules, record.is_public, &record.owner_did, caller, "/") == Decision::Deny
92-
{
93-
return Err(AppError::RepoNotFound(format!("{owner}/{repo}")));
94-
}
72+
let (record, _rules) =
73+
crate::api::authorize_repo_read(&state, &owner, &repo, caller, "/").await?;
9574
let rows = state.db.list_all_encrypted_blobs(&record.id).await?;
9675
let blobs: Vec<_> = rows
9776
.into_iter()

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

Lines changed: 62 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,16 +40,21 @@ use crate::visibility::{visibility_check, Decision};
4040
/// caller passes. For each iterated row we gate against that row's OWN rules
4141
/// (`visibility_check` at `"/"`), never re-resolving via `authorize_repo_read`
4242
/// — `get_repo`'s fuzzy match could otherwise authorize a different physical
43-
/// row than the one read (KTD2a). We check object existence via
44-
/// `store::object_type` *before* the expensive reachability walk so random-CID
45-
/// spray cannot trigger full-history git walks on repos that don't carry the
46-
/// object. When the row carries path-scoped rules (KTD4) the served object
47-
/// must be either a non-blob (trees/commits are structural; KTD3) OR a blob
48-
/// in the caller's *reachable* allowed-set (`allowed_blob_set_for_caller`).
49-
/// The reachable allowed-set excludes dangling blobs — a blob written via
50-
/// `git hash-object -w` and never committed has no path to gate, so it is
51-
/// fail-closed 404'd under path-scoped rules (#126). Denial and genuine
52-
/// not-found both fall through to an opaque 404.
43+
/// row than the one read (KTD2a). Quarantine is fail-closed in two layers:
44+
/// `list_all_repos` drops rows with `quarantined = TRUE`, and a logical-repo
45+
/// fold drops any surviving twin that shares owner+name with a quarantined
46+
/// *canonical* (UUID) row — so a public mirror cannot serve while its
47+
/// canonical twin is quarantined, without letting a quarantined mirror
48+
/// suppress a healthy canonical (`get_repo` prefers canonical). We check object
49+
/// existence via `store::object_type` *before* the expensive reachability walk
50+
/// so random-CID spray cannot trigger full-history git walks on repos that
51+
/// don't carry the object. When the row carries path-scoped rules (KTD4) the
52+
/// served object must be either a non-blob (trees/commits are structural;
53+
/// KTD3) OR a blob in the caller's *reachable* allowed-set
54+
/// (`allowed_blob_set_for_caller`). The reachable allowed-set excludes dangling
55+
/// blobs — a blob written via `git hash-object -w` and never committed has no
56+
/// path to gate, so it is fail-closed 404'd under path-scoped rules (#126).
57+
/// Denial and genuine not-found both fall through to an opaque 404.
5358
///
5459
/// Scan completeness (F2): the 404 above is returned ONLY when every candidate
5560
/// repo reached a VERDICT — visibility deny, probe-says-absent, walk-gate deny,
@@ -86,6 +91,17 @@ use crate::visibility::{visibility_check, Decision};
8691
/// case. A stale-public mirror row still serves withheld content (tracked
8792
/// separately, #124).
8893
///
94+
/// Normalize owner DID for quarantine logical-repo keys: matches [`did_matches`]
95+
/// for `did:key:` vs bare-key pairs without scanning every quarantined row per
96+
/// candidate repo.
97+
fn quarantine_owner_key(owner_did: &str) -> String {
98+
match owner_did.strip_prefix("did:key:") {
99+
Some(k) if !k.contains(':') => k.to_string(),
100+
_ if !owner_did.contains(':') => owner_did.to_string(),
101+
_ => owner_did.to_string(),
102+
}
103+
}
104+
89105
/// One `/ipfs` request's walk admission: the global pool permit plus the
90106
/// optional per-source sub-permit, both RAII (#174 U1).
91107
///
@@ -216,6 +232,36 @@ pub async fn get_by_cid(
216232
)));
217233
}
218234
};
235+
// Logical-repo quarantine fold: when a *canonical* UUID row is quarantined
236+
// but a public mirror twin (`owner/name`) survives `list_all_repos`, slug
237+
// routes 404 via `authorize_repo_read` (prefers canonical) while CID serve
238+
// would otherwise still hit the mirror. Drop candidates that share
239+
// owner+name with a quarantined *canonical* only — a quarantined mirror
240+
// must not suppress a healthy canonical (same preference as get_repo).
241+
let quarantined = match tokio::time::timeout(
242+
request_deadline.saturating_duration_since(std::time::Instant::now()),
243+
state.db.list_quarantined_repos(),
244+
)
245+
.await
246+
{
247+
Ok(Ok(rows)) => rows,
248+
Ok(Err(e)) => return Err(AppError::Internal(e)),
249+
Err(_elapsed) => {
250+
tracing::warn!(
251+
budget_secs = state.config.ipfs_request_budget_secs,
252+
"/ipfs list_quarantined_repos exceeded the request budget \
253+
(GITLAWB_IPFS_REQUEST_BUDGET_SECS); shedding a retryable 503 and freeing the walk permit"
254+
);
255+
return Err(AppError::Overloaded(format!(
256+
"ipfs scan incomplete (budget) for CID {cid_str}; retry shortly"
257+
)));
258+
}
259+
};
260+
let quarantined_canonical_keys: HashSet<(String, String)> = quarantined
261+
.iter()
262+
.filter(|q| !q.id.contains('/'))
263+
.map(|q| (quarantine_owner_key(&q.owner_did), q.name.clone()))
264+
.collect();
219265

220266
// Fetch every repo's visibility rules in one query rather than one per row
221267
// (the gate runs each row against its OWN rules — KTD2a). A row absent from
@@ -320,6 +366,12 @@ pub async fn get_by_cid(
320366
let mut repos_visited: usize = 0;
321367

322368
for repo in &repos {
369+
if quarantined_canonical_keys
370+
.contains(&(quarantine_owner_key(&repo.owner_did), repo.name.clone()))
371+
{
372+
continue;
373+
}
374+
323375
// Repo-level read gate against THIS row's own rules (KTD2a). Deny is a
324376
// VERDICT: this repo would never serve the caller, so skipping it cannot
325377
// hide content from them.

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,13 +238,16 @@ mod authz_guard {
238238
// PRE-GATED — already owner-gated, in-scope group; guard the gate itself
239239
(protect, "protect_branch", "did_matches("),
240240
(protect, "unprotect_branch", "did_matches("),
241+
(visibility, "set_visibility", "authorize_repo_read("),
241242
(visibility, "set_visibility", "require_owner("),
243+
(visibility, "remove_visibility", "authorize_repo_read("),
242244
(visibility, "remove_visibility", "require_owner("),
245+
(visibility, "list_visibility", "authorize_repo_read("),
243246
(visibility, "list_visibility", "require_owner("),
244247
];
245248

246-
// The visibility rows prove require_owner is CALLED; this proves the helper
247-
// itself does DID-safe matching, not a raw/trailing-segment compare.
249+
// Rows above prove each visibility handler calls authorize_repo_read and
250+
// require_owner; this proves the helper itself does DID-safe matching.
248251
assert!(
249252
fn_body(visibility, "require_owner").contains("did_matches("),
250253
"visibility::require_owner must use did_matches for DID-safe owner matching"

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

Lines changed: 16 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,10 @@ pub async fn set_visibility(
8585
Path((owner, repo)): Path<(String, String)>,
8686
Json(req): Json<SetVisibilityRequest>,
8787
) -> Result<(StatusCode, Json<serde_json::Value>)> {
88-
let record = state
89-
.db
90-
.get_repo(&owner, &repo)
91-
.await?
92-
.ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?;
88+
// Quarantine first (via authorize_repo_read), then owner — same posture as
89+
// list_visibility so a quarantined repo cannot be mutated while reads 404.
90+
let (record, _rules) =
91+
crate::api::authorize_repo_read(&state, &owner, &repo, Some(&auth.0), "/").await?;
9392
require_owner(&record, &auth.0)?;
9493
validate_path_glob(&req.path_glob)?;
9594

@@ -141,11 +140,8 @@ pub async fn remove_visibility(
141140
Path((owner, repo)): Path<(String, String)>,
142141
Json(req): Json<RemoveVisibilityRequest>,
143142
) -> Result<Json<serde_json::Value>> {
144-
let record = state
145-
.db
146-
.get_repo(&owner, &repo)
147-
.await?
148-
.ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?;
143+
let (record, _rules) =
144+
crate::api::authorize_repo_read(&state, &owner, &repo, Some(&auth.0), "/").await?;
149145
require_owner(&record, &auth.0)?;
150146

151147
state
@@ -171,14 +167,12 @@ pub async fn list_visibility(
171167
Extension(auth): Extension<AuthenticatedDid>,
172168
Path((owner, repo)): Path<(String, String)>,
173169
) -> Result<Json<serde_json::Value>> {
174-
let record = state
175-
.db
176-
.get_repo(&owner, &repo)
177-
.await?
178-
.ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?;
170+
// Quarantine first (via authorize_repo_read), then owner — a quarantined
171+
// mirror must be opaque even to a caller matching owner_did.
172+
let (record, rules) =
173+
crate::api::authorize_repo_read(&state, &owner, &repo, Some(&auth.0), "/").await?;
179174
require_owner(&record, &auth.0)?;
180175

181-
let rules = state.db.list_visibility_rules(&record.id).await?;
182176
let rules_json: Vec<_> = rules
183177
.into_iter()
184178
.map(|r| {
@@ -205,28 +199,18 @@ pub async fn list_visibility(
205199
/// denied one (`reinclude`), so a clean-clone client can sparse-exclude the
206200
/// denied subtrees while re-including the allowed nested paths. Unlike
207201
/// `list_visibility` this is not owner-gated and never exposes reader_dids.
202+
///
203+
/// Quarantined repos are opaque 404 via [`crate::api::authorize_repo_read`] —
204+
/// same posture as encrypted-blob discovery — so admission and private-subtree
205+
/// layout are not disclosed to anon, owner, or peers.
208206
pub async fn withheld_paths(
209207
State(state): State<AppState>,
210208
auth: Option<Extension<AuthenticatedDid>>,
211209
Path((owner, repo)): Path<(String, String)>,
212210
) -> Result<Json<serde_json::Value>> {
213-
let record = state
214-
.db
215-
.get_repo(&owner, &repo)
216-
.await?
217-
.ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{repo}")))?;
218-
219-
let rules = state.db.list_visibility_rules(&record.id).await?;
220211
let caller = auth.as_ref().map(|e| e.0 .0.as_str());
221-
222-
// Whole-repo read gate: a caller who cannot read "/" gets repo-not-found,
223-
// matching the git read endpoints, so this never discloses a private repo's
224-
// existence or its path layout to an unauthorized caller.
225-
if crate::visibility::visibility_check(&rules, record.is_public, &record.owner_did, caller, "/")
226-
== crate::visibility::Decision::Deny
227-
{
228-
return Err(AppError::RepoNotFound(format!("{owner}/{repo}")));
229-
}
212+
let (record, rules) =
213+
crate::api::authorize_repo_read(&state, &owner, &repo, caller, "/").await?;
230214

231215
let withheld =
232216
crate::visibility::withheld_globs(&rules, record.is_public, &record.owner_did, caller);

‎crates/gitlawb-node/src/db/mod.rs‎

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1095,14 +1095,18 @@ impl Db {
10951095

10961096
/// Raw list of every repo row — NOT deduped (a mirror row and its canonical
10971097
/// row both appear) and without stars. For enumeration callers that must see
1098-
/// every physical row (e.g. the IPFS object scan in `api::ipfs`), not for
1099-
/// listing surfaces. Listing surfaces dedupe via `list_all_repos_deduped` or
1100-
/// `list_all_repos_with_stars` + `dedupe_canonical_repos`.
1098+
/// every *servable* physical row (e.g. the IPFS object scan in `api::ipfs`).
1099+
/// Quarantined mirrors are excluded — same invariant as
1100+
/// [`crate::api::authorize_repo_read`]: admitted but withheld from every
1101+
/// reader. Listing surfaces that also need star counts use
1102+
/// `list_all_repos_with_stars` / `list_all_repos_deduped`.
11011103
pub async fn list_all_repos(&self) -> Result<Vec<RepoRecord>> {
11021104
let rows = sqlx::query(
11031105
"SELECT id, name, owner_did, description, is_public, default_branch,
11041106
created_at, updated_at, disk_path, forked_from, machine_id
1105-
FROM repos ORDER BY updated_at DESC",
1107+
FROM repos
1108+
WHERE quarantined = FALSE
1109+
ORDER BY updated_at DESC",
11061110
)
11071111
.fetch_all(&self.pool)
11081112
.await?;
@@ -5329,6 +5333,34 @@ mod icaptcha_quarantine_tests {
53295333
assert!(db.list_quarantined_repo_ids().await.unwrap().is_empty());
53305334
}
53315335

5336+
/// IPFS/CID enumeration must not see quarantined rows (serve-side of the
5337+
/// authorize_repo_read quarantine short-circuit).
5338+
#[sqlx::test]
5339+
async fn list_all_repos_excludes_quarantined(pool: PgPool) {
5340+
let db = db(pool).await;
5341+
db.upsert_mirror_repo("z6owner", "live", "/srv/live", None, false)
5342+
.await
5343+
.unwrap();
5344+
db.upsert_mirror_repo("z6owner", "sick", "/srv/sick", None, true)
5345+
.await
5346+
.unwrap();
5347+
let ids: Vec<_> = db
5348+
.list_all_repos()
5349+
.await
5350+
.unwrap()
5351+
.into_iter()
5352+
.map(|r| r.id)
5353+
.collect();
5354+
assert!(
5355+
ids.iter().any(|id| id.contains("live")),
5356+
"live repo must be listed: {ids:?}"
5357+
);
5358+
assert!(
5359+
ids.iter().all(|id| !id.contains("sick")),
5360+
"quarantined repo must be excluded from list_all_repos: {ids:?}"
5361+
);
5362+
}
5363+
53325364
/// A mirror admitted quarantined stays quarantined across a re-sync — the
53335365
/// admission decision is made once and an operator's later release (or the
53345366
/// initial quarantine) must not be reverted by ON CONFLICT.

0 commit comments

Comments
 (0)