Skip to content

Commit 6b5e5bc

Browse files
authored
fix(node): bound list_ref_certificates with LIMIT and add upsert to prevent unbounded growth (#147) (#149)
* fix(node): bound list_ref_certificates with LIMIT and add upsert to prevent unbounded growth (#147) * test: add INV-7 v10 upgrade-path dedup test * fix INV-7 test: correct id-DESC tiebreaker ids and fix fmt * fix: insert_ref_certificate returns persisted id via RETURNING; issue_ref_certificate uses it * fix: guard cert upsert on issued_at so older certs cannot regress newer ones * fix: .max(0) in events limit parsing, re-add lost cert tests after rebase * address reviewer findings: return full persisted row, add monotonic clock comment, note test drift risk * fix: add ?prefix= server-side cert lookup for CLI short-ID resolution --------- Co-authored-by: Gravirei <gravirei@users.noreply.github.com>
1 parent dfcaa22 commit 6b5e5bc

6 files changed

Lines changed: 911 additions & 16 deletions

File tree

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

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,44 @@
11
//! API handlers for ref certificates.
22
3-
use axum::extract::{Extension, Path, State};
3+
use std::collections::HashMap;
4+
5+
use axum::extract::{Extension, Path, Query, State};
46
use axum::Json;
57

68
use crate::auth::AuthenticatedDid;
79
use crate::error::{AppError, Result};
810
use crate::state::AppState;
911

10-
/// GET /api/v1/repos/{owner}/{repo}/certs
12+
/// GET /api/v1/repos/{owner}/{repo}/certs?limit=50
1113
pub async fn list_certs(
1214
State(state): State<AppState>,
1315
Path((owner, name)): Path<(String, String)>,
16+
Query(params): Query<HashMap<String, String>>,
1417
auth: Option<Extension<AuthenticatedDid>>,
1518
) -> Result<Json<serde_json::Value>> {
19+
let limit = params
20+
.get("limit")
21+
.and_then(|v| v.parse::<i64>().ok())
22+
.map(|v| v.max(1))
23+
.unwrap_or(50)
24+
.min(200);
25+
1626
let caller = auth.as_ref().map(|e| e.0 .0.as_str());
1727
let (record, _rules) =
1828
crate::api::authorize_repo_read(&state, &owner, &name, caller, "/").await?;
1929

20-
let certs = state.db.list_ref_certificates(&record.id).await?;
30+
// When a prefix is given (short-ID resolution from the CLI) use a
31+
// generous limit and delegate to the prefix-matched query so the
32+
// caller can resolve IDs regardless of how many certs exist.
33+
let prefix = params.get("prefix").filter(|p| !p.is_empty());
34+
let certs = if let Some(prefix) = prefix {
35+
state
36+
.db
37+
.list_ref_certificates_by_prefix(&record.id, prefix, 200)
38+
.await?
39+
} else {
40+
state.db.list_ref_certificates(&record.id, limit).await?
41+
};
2142
let certs_json: Vec<serde_json::Value> = certs
2243
.iter()
2344
.map(|c| {
@@ -35,7 +56,10 @@ pub async fn list_certs(
3556
})
3657
.collect();
3758

38-
Ok(Json(serde_json::json!({ "certificates": certs_json })))
59+
let count = certs_json.len();
60+
Ok(Json(
61+
serde_json::json!({ "certificates": certs_json, "count": count }),
62+
))
3963
}
4064

4165
/// GET /api/v1/repos/{owner}/{repo}/certs/{id}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ pub async fn list_ref_updates(
129129
let limit = params
130130
.get("limit")
131131
.and_then(|v| v.parse::<i64>().ok())
132+
.map(|v| v.max(0))
132133
.unwrap_or(50)
133134
.clamp(0, MAX_VISIBLE_REF_UPDATES);
134135

@@ -177,6 +178,7 @@ pub async fn list_repo_events(
177178
let limit = params
178179
.get("limit")
179180
.and_then(|v| v.parse::<i64>().ok())
181+
.map(|v| v.max(0))
180182
.unwrap_or(50)
181183
.clamp(0, MAX_VISIBLE_REF_UPDATES);
182184

@@ -209,7 +211,7 @@ pub async fn list_repo_events(
209211
// into an empty 200, matching the gossip half below.
210212
let cert_events: Vec<serde_json::Value> = state
211213
.db
212-
.list_ref_certificates(&record.id)
214+
.list_ref_certificates(&record.id, limit)
213215
.await?
214216
.iter()
215217
.map(|c| {

‎crates/gitlawb-node/src/cert.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ pub async fn issue_ref_certificate(
5252
issued_at,
5353
};
5454

55-
state.db.insert_ref_certificate(&cert).await?;
56-
Ok(cert)
55+
// Persist and return the row as it exists in the database (on a
56+
// conflict the existing row survives when it is newer).
57+
state.db.insert_ref_certificate(&cert).await
5758
}

0 commit comments

Comments
 (0)