fix(node): bound list_ref_certificates with LIMIT and add upsert to prevent unbounded growth (#147)

Author: Gravirei

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

@@ -1,23 +1,33 @@
 //! API handlers for ref certificates.
 
-use axum::extract::{Path, State};
+use std::collections::HashMap;
+
+use axum::extract::{Path, Query, State};
 use axum::Json;
 
 use crate::error::{AppError, Result};
 use crate::state::AppState;
 
-/// GET /api/v1/repos/{owner}/{repo}/certs
+/// GET /api/v1/repos/{owner}/{repo}/certs?limit=50
 pub async fn list_certs(
     State(state): State<AppState>,
     Path((owner, name)): Path<(String, String)>,
+    Query(params): Query<HashMap<String, String>>,
 ) -> Result<Json<serde_json::Value>> {
+    let limit = params
+        .get("limit")
+        .and_then(|v| v.parse::<i64>().ok())
+        .map(|v| v.max(1))
+        .unwrap_or(50)
+        .min(200);
+
     let record = state
         .db
         .get_repo(&owner, &name)
         .await?
         .ok_or_else(|| AppError::RepoNotFound(format!("{owner}/{name}")))?;
 
-    let certs = state.db.list_ref_certificates(&record.id).await?;
+    let certs = state.db.list_ref_certificates(&record.id, limit).await?;
     let certs_json: Vec<serde_json::Value> = certs
         .iter()
         .map(|c| {
@@ -35,7 +45,10 @@ pub async fn list_certs(
         })
         .collect();
 
-    Ok(Json(serde_json::json!({ "certificates": certs_json })))
+    let count = certs_json.len();
+    Ok(Json(
+        serde_json::json!({ "certificates": certs_json, "count": count }),
+    ))
 }
 
 /// GET /api/v1/repos/{owner}/{repo}/certs/{id}

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

@@ -75,10 +75,11 @@ pub async fn list_repo_events(
     };
 
     // Fetch local ref certificates for this repo (if the repo exists on this node)
+    // Bound to the same limit as the overall response to avoid unbounded reads.
     let cert_events: Vec<serde_json::Value> = if let Some(ref record) = repo_record {
         state
             .db
-            .list_ref_certificates(&record.id)
+            .list_ref_certificates(&record.id, limit)
             .await
             .unwrap_or_default()
             .iter()

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

@@ -823,6 +823,25 @@ const MIGRATIONS: &[Migration] = &[
             "ALTER TABLE repos ADD COLUMN IF NOT EXISTS quarantined BOOLEAN NOT NULL DEFAULT FALSE",
         ],
     },
+    Migration {
+        version: 10,
+        name: "ref_cert_unique_per_ref",
+        stmts: &[
+            // Dedup before the unique index: keep only the most recent row per
+            // (repo_id, ref_name) so the CREATE UNIQUE INDEX below does not fail
+            // on existing databases that accumulated duplicates.
+            r#"DELETE FROM ref_certificates
+               WHERE id IN (
+                   SELECT id FROM (
+                       SELECT id, ROW_NUMBER() OVER (
+                           PARTITION BY repo_id, ref_name ORDER BY issued_at DESC
+                       ) AS rn
+                       FROM ref_certificates
+                   ) dups WHERE dups.rn > 1
+               )"#,
+            "CREATE UNIQUE INDEX IF NOT EXISTS idx_ref_certs_repo_ref ON ref_certificates(repo_id, ref_name)",
+        ],
+    },
 ];
 
 // ── Repos ─────────────────────────────────────────────────────────────────────
@@ -1909,7 +1928,15 @@ impl Db {
         sqlx::query(
             "INSERT INTO ref_certificates
              (id, repo_id, ref_name, old_sha, new_sha, pusher_did, node_did, signature, issued_at)
-             VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)",
+             VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)
+             ON CONFLICT (repo_id, ref_name) DO UPDATE SET
+                id        = EXCLUDED.id,
+                old_sha   = EXCLUDED.old_sha,
+                new_sha   = EXCLUDED.new_sha,
+                pusher_did = EXCLUDED.pusher_did,
+                node_did  = EXCLUDED.node_did,
+                signature = EXCLUDED.signature,
+                issued_at = EXCLUDED.issued_at",
         )
         .bind(&cert.id)
         .bind(&cert.repo_id)
@@ -1925,12 +1952,17 @@ impl Db {
         Ok(())
     }
 
-    pub async fn list_ref_certificates(&self, repo_id: &str) -> Result<Vec<RefCertificate>> {
+    pub async fn list_ref_certificates(
+        &self,
+        repo_id: &str,
+        limit: i64,
+    ) -> Result<Vec<RefCertificate>> {
         let rows = sqlx::query(
             "SELECT id, repo_id, ref_name, old_sha, new_sha, pusher_did, node_did, signature, issued_at
-             FROM ref_certificates WHERE repo_id = $1 ORDER BY issued_at DESC",
+             FROM ref_certificates WHERE repo_id = $1 ORDER BY issued_at DESC LIMIT $2",
         )
         .bind(repo_id)
+        .bind(limit)
         .fetch_all(&self.pool)
         .await?;
         Ok(rows.into_iter().map(row_to_cert).collect())
@@ -4249,3 +4281,150 @@ mod icaptcha_quarantine_tests {
         assert!(with_stars.iter().all(|(r, _)| r.name != "spam"));
     }
 }
+
+#[cfg(test)]
+mod ref_certificate_tests {
+    use super::{Db, RefCertificate, RepoRecord};
+    use chrono::Utc;
+    use sqlx::PgPool;
+
+    async fn db(pool: PgPool) -> Db {
+        let db = Db::for_testing(pool);
+        db.run_migrations().await.unwrap();
+        db
+    }
+
+    fn make_cert(
+        id: &str,
+        repo_id: &str,
+        ref_name: &str,
+        old_sha: &str,
+        new_sha: &str,
+        issued_at: &str,
+    ) -> RefCertificate {
+        RefCertificate {
+            id: id.to_string(),
+            repo_id: repo_id.to_string(),
+            ref_name: ref_name.to_string(),
+            old_sha: old_sha.to_string(),
+            new_sha: new_sha.to_string(),
+            pusher_did: "did:key:zPUSHER".to_string(),
+            node_did: "did:key:zNODE".to_string(),
+            signature: "sig".to_string(),
+            issued_at: issued_at.to_string(),
+        }
+    }
+
+    #[sqlx::test]
+    async fn list_ref_certificates_respects_limit(pool: PgPool) {
+        let db = db(pool).await;
+        let repo_id = uuid::Uuid::new_v4().to_string();
+
+        // Create a repo to satisfy FK
+        db.create_repo(&RepoRecord {
+            id: repo_id.clone(),
+            name: "limit-test".into(),
+            owner_did: "did:key:zOWNER".into(),
+            description: None,
+            is_public: true,
+            default_branch: "main".into(),
+            created_at: Utc::now(),
+            updated_at: Utc::now(),
+            disk_path: "/tmp/limit-test".into(),
+            forked_from: None,
+            machine_id: None,
+        })
+        .await
+        .unwrap();
+
+        // Insert 5 certs with descending issued_at
+        for i in 0..5 {
+            db.insert_ref_certificate(&make_cert(
+                &format!("cert-{i}"),
+                &repo_id,
+                &format!("refs/heads/feature-{i}"),
+                "0000",
+                "1111",
+                &format!("2026-07-03T20:0{i}:00Z"),
+            ))
+            .await
+            .unwrap();
+        }
+
+        // limit=2 returns only 2
+        let certs = db.list_ref_certificates(&repo_id, 2).await.unwrap();
+        assert_eq!(certs.len(), 2, "LIMIT 2 must return exactly 2 certs");
+        assert_eq!(certs[0].id, "cert-4", "most recent first");
+        assert_eq!(certs[1].id, "cert-3", "second most recent");
+
+        // limit=10 returns all 5 (no padding)
+        let all = db.list_ref_certificates(&repo_id, 10).await.unwrap();
+        assert_eq!(all.len(), 5, "LIMIT >= row count returns all rows");
+    }
+
+    #[sqlx::test]
+    async fn insert_ref_certificate_upserts_on_repo_ref(pool: PgPool) {
+        let db = db(pool).await;
+        let repo_id = uuid::Uuid::new_v4().to_string();
+
+        db.create_repo(&RepoRecord {
+            id: repo_id.clone(),
+            name: "upsert-test".into(),
+            owner_did: "did:key:zOWNER".into(),
+            description: None,
+            is_public: true,
+            default_branch: "main".into(),
+            created_at: Utc::now(),
+            updated_at: Utc::now(),
+            disk_path: "/tmp/upsert-test".into(),
+            forked_from: None,
+            machine_id: None,
+        })
+        .await
+        .unwrap();
+
+        // First insert
+        db.insert_ref_certificate(&make_cert(
+            "cert-original",
+            &repo_id,
+            "refs/heads/main",
+            "0000",
+            "1111",
+            "2026-07-03T20:00:00Z",
+        ))
+        .await
+        .unwrap();
+
+        // Upsert same ref with new values
+        db.insert_ref_certificate(&make_cert(
+            "cert-upserted",
+            &repo_id,
+            "refs/heads/main",
+            "aaaa",
+            "bbbb",
+            "2026-07-03T21:00:00Z",
+        ))
+        .await
+        .unwrap();
+
+        // Only one row exists for this ref
+        let certs = db.list_ref_certificates(&repo_id, 10).await.unwrap();
+        assert_eq!(certs.len(), 1, "upsert must not create a duplicate row");
+        assert_eq!(
+            certs[0].id, "cert-upserted",
+            "upsert must replace the original cert"
+        );
+        assert_eq!(certs[0].old_sha, "aaaa", "old_sha updated");
+        assert_eq!(certs[0].new_sha, "bbbb", "new_sha updated");
+    }
+
+    #[sqlx::test]
+    async fn list_ref_certificates_empty_repo_returns_empty(pool: PgPool) {
+        let db = db(pool).await;
+        let certs = db
+            .list_ref_certificates("nonexistent-repo-id", 10)
+            .await
+            .unwrap();
+        assert!(certs.is_empty());
+    }
+}