From 7a78815e29b01c83f3d0ec195ba717a2eba8cd37 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Mon, 1 Dec 2025 11:56:49 +0000 Subject: reject push when refs/nostr/ doesnt match known event and delete incorrect ref on event receive --- src/git/authorization.rs | 142 +++++++++++++++-- src/git/handlers.rs | 211 ++++++++++++++----------- src/git/mod.rs | 210 ++++++++++++++++++------- src/nostr/builder.rs | 389 ++++++++++++++++++++++++++++++++++++++++------- src/nostr/events.rs | 6 + 5 files changed, 742 insertions(+), 216 deletions(-) (limited to 'src') diff --git a/src/git/authorization.rs b/src/git/authorization.rs index bb3bd01..3b0e759 100644 --- a/src/git/authorization.rs +++ b/src/git/authorization.rs @@ -35,7 +35,8 @@ use std::sync::Arc; use tracing::debug; use crate::nostr::events::{ - RepositoryAnnouncement, RepositoryState, KIND_REPOSITORY_ANNOUNCEMENT, KIND_REPOSITORY_STATE, + RepositoryAnnouncement, RepositoryState, KIND_PR, KIND_PR_UPDATE, KIND_REPOSITORY_ANNOUNCEMENT, + KIND_REPOSITORY_STATE, }; /// Repository data fetched from the database @@ -172,9 +173,9 @@ fn get_maintainers_recursive( checked.insert(pubkey.to_string()); // Mark as checked // Find the announcement event for this pubkey+identifier - let announcement = announcements.iter().find(|a| { - a.event.pubkey.to_hex() == pubkey && a.identifier == identifier - }); + let announcement = announcements + .iter() + .find(|a| a.event.pubkey.to_hex() == pubkey && a.identifier == identifier); let Some(announcement) = announcement else { return; // No announcement found for this pubkey @@ -195,19 +196,19 @@ pub fn collect_all_authorized_maintainers( ) -> HashSet { let by_owner = collect_authorized_maintainers(announcements); let mut all_authorized = HashSet::new(); - + for maintainers in by_owner.values() { for maintainer in maintainers { all_authorized.insert(maintainer.clone()); } } - + debug!( "Collected {} total authorized maintainers from {} owners", all_authorized.len(), by_owner.len() ); - + all_authorized } @@ -601,10 +602,7 @@ pub fn validate_push_refs( pushed_refs: &[(String, String, String)], // (old_oid, new_oid, ref_name) ) -> Result<()> { for (old_oid, new_oid, ref_name) in pushed_refs { - debug!( - "Validating push: {} {} -> {}", - ref_name, old_oid, new_oid - ); + debug!("Validating push: {} {} -> {}", ref_name, old_oid, new_oid); // Handle branch updates if let Some(branch_name) = ref_name.strip_prefix("refs/heads/") { @@ -657,7 +655,10 @@ pub fn validate_push_refs( )); } // Valid EventId format - allow push (skip state event check) - debug!("refs/nostr/{} push authorized (valid EventId)", event_id_str); + debug!( + "refs/nostr/{} push authorized (valid EventId)", + event_id_str + ); continue; // Skip the rest of ref validation for this ref } else { return Err(anyhow!("Invalid refs/nostr/ format: {}", ref_name)); @@ -805,6 +806,119 @@ pub fn npub_to_pubkey(npub: &str) -> Result { Ok(pk.to_hex()) } +/// Fetch an event by ID from the database and extract the `c` tag commit hash +/// +/// This is used for validating pushes to refs/nostr/. Per GRASP-01, +/// if a PR or PR Update event with this ID exists in the database, the pushed +/// commit must match the commit in the event's `c` tag. +/// +/// # Returns +/// - `Ok(Some(commit))` if the event exists and has a valid `c` tag +/// - `Ok(None)` if the event doesn't exist (push should be allowed) +/// - `Err(_)` on database errors +pub async fn get_event_commit_tag( + database: &Arc, + event_id: &EventId, +) -> Result> { + // Query for PR (1618) and PR Update (1619) events with this ID + let filter = Filter::new() + .ids([*event_id]) + .kinds([Kind::from(KIND_PR), Kind::from(KIND_PR_UPDATE)]); + + let events: Vec = database + .query(filter) + .await + .map_err(|e| anyhow!("Database query failed: {}", e))? + .into_iter() + .collect(); + + if events.is_empty() { + debug!("No PR/PR Update event found with ID {}", event_id); + return Ok(None); + } + + // Get the first (should be only) event + let event = &events[0]; + + // Extract the `c` tag (commit hash) + // Per NIP-34, PR events have a `c` tag with the head commit + let commit = event + .tags + .iter() + .find(|tag| tag.as_slice().first().map(|s| s.as_str()) == Some("c")) + .and_then(|tag| tag.as_slice().get(1).map(|s| s.to_string())); + + debug!( + "Found PR event {} with commit tag: {:?}", + event_id, + commit.as_ref() + ); + + Ok(commit) +} + +/// Validate refs/nostr/ pushes against existing PR/PR Update events +/// +/// For each ref being pushed to refs/nostr/: +/// 1. Validate the event ID format (error if invalid) +/// 2. Check if a corresponding event exists in the database +/// 3. If event exists, verify the pushed commit matches the `c` tag +/// +/// # Arguments +/// * `database` - The nostr database to query +/// * `pushed_refs` - List of (old_oid, new_oid, ref_name) tuples +/// +/// # Returns +/// * `Ok(())` if all refs/nostr/ pushes are valid +/// * `Err(_)` if any ref has invalid event ID format or fails commit validation +pub async fn validate_nostr_ref_pushes( + database: &Arc, + pushed_refs: &[(String, String, String)], +) -> Result<()> { + for (_, new_oid, ref_name) in pushed_refs { + // Only check refs/nostr/ refs + if let Some(event_id_str) = ref_name.strip_prefix("refs/nostr/") { + // Parse the event ID - error on invalid format + let event_id = EventId::parse(event_id_str).map_err(|_| { + anyhow!( + "Invalid event ID format '{}' in ref: {}", + event_id_str, + ref_name + ) + })?; + + // Check if event exists and get commit tag + match get_event_commit_tag(database, &event_id).await? { + Some(expected_commit) => { + // Event exists - verify commit matches + if new_oid != &expected_commit { + return Err(anyhow!( + "Push to {} rejected: event {} specifies commit {}, but push contains {}", + ref_name, + event_id_str, + expected_commit, + new_oid + )); + } + debug!( + "Push to {} validated: commit {} matches event's c tag", + ref_name, new_oid + ); + } + None => { + // No event exists yet - allow push + debug!( + "Push to {} allowed: no PR/PR Update event with ID {} found yet", + ref_name, event_id_str + ); + } + } + } + } + + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -920,7 +1034,7 @@ mod tests { let eve = create_test_keys(); // Not authorized let identifier = "test-repo"; - // Alice lists Bob as maintainer + // Alice lists Bob as maintainer let alice_announcement = create_announcement_event(&alice, identifier, &[&bob]); let events = vec![alice_announcement]; @@ -1084,4 +1198,4 @@ mod tests { let back_to_hex = npub_to_pubkey(&npub).unwrap(); assert_eq!(hex, back_to_hex); } -} \ No newline at end of file +} diff --git a/src/git/handlers.rs b/src/git/handlers.rs index 23d4b5b..00f2449 100644 --- a/src/git/handlers.rs +++ b/src/git/handlers.rs @@ -2,17 +2,18 @@ //! //! This module implements the HTTP handlers for Git Smart HTTP protocol. -use std::path::PathBuf; -use std::sync::Arc; -use hyper::{body::Bytes, Response, StatusCode}; use http_body_util::Full; +use hyper::{body::Bytes, Response, StatusCode}; use nostr_relay_builder::prelude::MemoryDatabase; use nostr_sdk::EventId; +use std::path::PathBuf; +use std::sync::Arc; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tracing::{debug, error, info, warn}; use super::authorization::{ - get_authorization_for_owner, parse_pushed_refs, validate_push_refs, AuthorizationResult, + get_authorization_for_owner, parse_pushed_refs, validate_nostr_ref_pushes, validate_push_refs, + AuthorizationResult, }; use super::protocol::{GitService, PktLine}; use super::subprocess::GitSubprocess; @@ -27,7 +28,10 @@ pub async fn handle_info_refs( repo_path: PathBuf, service: GitService, ) -> Result>, GitError> { - debug!("Handling info/refs for {:?} with service {:?}", repo_path, service); + debug!( + "Handling info/refs for {:?} with service {:?}", + repo_path, service + ); // Check if repository exists if !repo_path.exists() { @@ -36,55 +40,54 @@ pub async fn handle_info_refs( } // Spawn git with --advertise-refs - let mut git = GitSubprocess::spawn(service, &repo_path, true) - .map_err(|e| { - error!("Failed to spawn git process: {}", e); - GitError::ProcessSpawnFailed(e) - })?; + let mut git = GitSubprocess::spawn(service, &repo_path, true).map_err(|e| { + error!("Failed to spawn git process: {}", e); + GitError::ProcessSpawnFailed(e) + })?; // Read the output from git let mut output = Vec::new(); let mut stderr_output = Vec::new(); - + if let Some(stdout) = git.take_stdout() { let mut stdout = stdout; - stdout.read_to_end(&mut output).await - .map_err(|e| { - error!("Failed to read git output: {}", e); - GitError::IoError(e) - })?; + stdout.read_to_end(&mut output).await.map_err(|e| { + error!("Failed to read git output: {}", e); + GitError::IoError(e) + })?; } - + if let Some(stderr) = git.take_stderr() { let mut stderr = stderr; - stderr.read_to_end(&mut stderr_output).await - .map_err(|e| { - error!("Failed to read git stderr: {}", e); - GitError::IoError(e) - })?; + stderr.read_to_end(&mut stderr_output).await.map_err(|e| { + error!("Failed to read git stderr: {}", e); + GitError::IoError(e) + })?; } // Wait for process to complete - let status = git.wait().await - .map_err(|e| { - error!("Failed to wait for git process: {}", e); - GitError::IoError(e) - })?; + let status = git.wait().await.map_err(|e| { + error!("Failed to wait for git process: {}", e); + GitError::IoError(e) + })?; if !status.success() { let stderr_str = String::from_utf8_lossy(&stderr_output); - error!("Git process failed with status: {:?}, stderr: {}", status, stderr_str); + error!( + "Git process failed with status: {:?}, stderr: {}", + status, stderr_str + ); return Err(GitError::GitFailed(status.code())); } // Build response with pkt-line header let mut response_body = Vec::new(); - + // First line: service advertisement let service_line = format!("# service={}\n", service.as_str()); response_body.extend_from_slice(&PktLine::data(service_line.as_bytes()).encode()); response_body.extend_from_slice(&PktLine::flush().encode()); - + // Then the git output response_body.extend_from_slice(&output); @@ -113,7 +116,9 @@ pub async fn handle_upload_pack( // Write request to git's stdin if let Some(mut stdin) = git.take_stdin() { - stdin.write_all(&request_body).await + stdin + .write_all(&request_body) + .await .map_err(GitError::IoError)?; // Close stdin to signal end of input drop(stdin); @@ -122,22 +127,25 @@ pub async fn handle_upload_pack( // Read response from git's stdout let mut output = Vec::new(); let mut stderr_output = Vec::new(); - + if let Some(stdout) = git.take_stdout() { let mut stdout = stdout; - stdout.read_to_end(&mut output).await + stdout + .read_to_end(&mut output) + .await .map_err(GitError::IoError)?; } - + if let Some(stderr) = git.take_stderr() { let mut stderr = stderr; - stderr.read_to_end(&mut stderr_output).await + stderr + .read_to_end(&mut stderr_output) + .await .map_err(GitError::IoError)?; } // Wait for process - let status = git.wait().await - .map_err(GitError::IoError)?; + let status = git.wait().await.map_err(GitError::IoError)?; if !status.success() { let stderr_str = String::from_utf8_lossy(&stderr_output); @@ -194,10 +202,7 @@ pub async fn handle_receive_pack( match authorize_push(db, identifier, owner_pubkey, &request_body).await { Ok(auth_result) => { if !auth_result.authorized { - warn!( - "Push rejected for {}: {}", - identifier, auth_result.reason - ); + warn!("Push rejected for {}: {}", identifier, auth_result.reason); return Err(GitError::Unauthorized); } info!( @@ -209,10 +214,7 @@ pub async fn handle_receive_pack( authorized_state = auth_result.state; } Err(e) => { - warn!( - "Authorization check failed for {}: {}", - identifier, e - ); + warn!("Authorization check failed for {}: {}", identifier, e); return Err(GitError::Unauthorized); } } @@ -226,7 +228,9 @@ pub async fn handle_receive_pack( // Write request to git's stdin if let Some(mut stdin) = git.take_stdin() { - stdin.write_all(&request_body).await + stdin + .write_all(&request_body) + .await .map_err(GitError::IoError)?; drop(stdin); } @@ -234,22 +238,25 @@ pub async fn handle_receive_pack( // Read response from git's stdout let mut output = Vec::new(); let mut stderr_output = Vec::new(); - + if let Some(stdout) = git.take_stdout() { let mut stdout = stdout; - stdout.read_to_end(&mut output).await + stdout + .read_to_end(&mut output) + .await .map_err(GitError::IoError)?; } - + if let Some(stderr) = git.take_stderr() { let mut stderr = stderr; - stderr.read_to_end(&mut stderr_output).await + stderr + .read_to_end(&mut stderr_output) + .await .map_err(GitError::IoError)?; } // Wait for process - let status = git.wait().await - .map_err(GitError::IoError)?; + let status = git.wait().await.map_err(GitError::IoError)?; if !status.success() { let stderr_str = String::from_utf8_lossy(&stderr_output); @@ -266,10 +273,7 @@ pub async fn handle_receive_pack( if let Some(commit) = state.get_branch_commit(branch_name) { match try_set_head_if_available(&repo_path, head_ref, commit) { Ok(true) => { - info!( - "Set HEAD to {} after push to {:?}", - head_ref, repo_path - ); + info!("Set HEAD to {} after push to {:?}", head_ref, repo_path); } Ok(false) => { debug!( @@ -278,10 +282,7 @@ pub async fn handle_receive_pack( ); } Err(e) => { - warn!( - "Failed to set HEAD after push: {}", - e - ); + warn!("Failed to set HEAD after push: {}", e); } } } @@ -291,7 +292,10 @@ pub async fn handle_receive_pack( Ok(Response::builder() .status(StatusCode::OK) - .header("content-type", GitService::ReceivePack.result_content_type()) + .header( + "content-type", + GitService::ReceivePack.result_content_type(), + ) .header("cache-control", "no-cache") .body(Full::new(Bytes::from(output))) .unwrap()) @@ -305,6 +309,7 @@ pub async fn handle_receive_pack( /// 3. Collects authorized publishers from that announcement (owner + maintainers) /// 4. Gets the latest authorized state from those publishers /// 5. Validates that pushed refs match the state +/// 6. Validates refs/nostr/ has valid event id and if event exists, `c` tag matches ref async fn authorize_push( database: &Arc, identifier: &str, @@ -323,59 +328,79 @@ async fn authorize_push( debug!(" {} {} -> {}", ref_name, old_oid, new_oid); } - // Check if ALL pushed refs are to refs/nostr/ with valid EventId format + // Separate refs/nostr/ refs from other refs // Per GRASP-01: "MUST accept pushes via this service to `refs/nostr/`" - // These pushes only require EventId format validation, not state validation - let all_refs_nostr_valid = !pushed_refs.is_empty() - && pushed_refs.iter().all(|(_, _, ref_name)| { - if let Some(event_id_str) = ref_name.strip_prefix("refs/nostr/") { - // Validate it parses as a valid EventId - EventId::parse(event_id_str).is_ok() - } else { - false - } - }); - - if all_refs_nostr_valid { - debug!("All refs are refs/nostr/ with valid EventId format - authorized without state check"); - // Return success for refs/nostr/ pushes without requiring state + let (nostr_refs, other_refs): (Vec<_>, Vec<_>) = pushed_refs + .iter() + .partition(|(_, _, ref_name)| ref_name.starts_with("refs/nostr/")); + + // Validate refs/nostr/ refs if any exist + if !nostr_refs.is_empty() { + debug!( + "Found {} refs/nostr/ refs - validating against events", + nostr_refs.len() + ); + + // Validate refs/nostr/ pushes: checks event ID format and commit matching + let nostr_refs_owned: Vec<(String, String, String)> = nostr_refs + .into_iter() + .map(|(a, b, c)| (a.clone(), b.clone(), c.clone())) + .collect(); + if let Err(e) = validate_nostr_ref_pushes(database, &nostr_refs_owned).await { + warn!("refs/nostr/ validation failed: {}", e); + return Ok(AuthorizationResult::denied(format!( + "refs/nostr/ validation failed: {}", + e + ))); + } + debug!("refs/nostr/ push validated successfully"); + } + + // If only refs/nostr/ refs, we're done - return success + if other_refs.is_empty() { + debug!("Only refs/nostr/ refs in push - authorization complete"); return Ok(AuthorizationResult { authorized: true, - reason: "Push to refs/nostr/ with valid EventId format".to_string(), + reason: "Push to refs/nostr/ validated against events".to_string(), state: None, maintainers: vec![], }); } - // For non-refs/nostr/ pushes, require state validation as normal - debug!("Non-refs/nostr/ push detected - checking state authorization"); + // For non-refs/nostr/ refs, require state validation + debug!( + "Found {} non-refs/nostr/ refs - checking state authorization", + other_refs.len() + ); let auth_result = get_authorization_for_owner(database, identifier, owner_pubkey).await?; if !auth_result.authorized { return Ok(auth_result); } - // Parse refs from the push request - let pushed_refs = parse_pushed_refs(request_body); - debug!("Parsed {} refs from push request", pushed_refs.len()); - for (old_oid, new_oid, ref_name) in &pushed_refs { - debug!(" {} {} -> {}", ref_name, old_oid, new_oid); - } + // Convert other_refs for validation + let other_refs_owned: Vec<(String, String, String)> = other_refs + .into_iter() + .map(|(a, b, c)| (a.clone(), b.clone(), c.clone())) + .collect(); - // Validate refs against state + // Validate non-refs/nostr/ refs against state if let Some(ref state) = auth_result.state { - debug!("Validating against state with {} branches", state.branches.len()); - + debug!( + "Validating against state with {} branches", + state.branches.len() + ); + // If we have a state event but couldn't parse any refs, reject the push. // This protects against parsing failures allowing unauthorized pushes. - if pushed_refs.is_empty() && !state.branches.is_empty() { + if other_refs_owned.is_empty() && !state.branches.is_empty() { warn!("No refs parsed from push request but state event has branches - rejecting"); return Ok(AuthorizationResult::denied( - "Failed to parse refs from push request - cannot validate against state" + "Failed to parse refs from push request - cannot validate against state", )); } - - if let Err(e) = validate_push_refs(state, &pushed_refs) { + + if let Err(e) = validate_push_refs(state, &other_refs_owned) { warn!("Ref validation failed: {}", e); return Ok(AuthorizationResult::denied(format!( "Ref validation failed: {}", @@ -423,4 +448,4 @@ impl GitError { _ => StatusCode::INTERNAL_SERVER_ERROR, } } -} \ No newline at end of file +} diff --git a/src/git/mod.rs b/src/git/mod.rs index 076e211..494f8b9 100644 --- a/src/git/mod.rs +++ b/src/git/mod.rs @@ -40,7 +40,7 @@ use tracing::{debug, info}; pub fn resolve_repo_path(git_data_path: &str, npub: &str, identifier: &str) -> PathBuf { // Remove .git suffix if present let identifier = identifier.strip_suffix(".git").unwrap_or(identifier); - + PathBuf::from(git_data_path) .join(npub) .join(format!("{}.git", identifier)) @@ -89,7 +89,10 @@ pub fn commit_exists(repo_path: &Path, commit_hash: &str) -> bool { pub fn set_repository_head(repo_path: &Path, head_ref: &str) -> Result<(), String> { // Validate the ref format if !head_ref.starts_with("refs/heads/") { - return Err(format!("Invalid HEAD ref: {} (must start with refs/heads/)", head_ref)); + return Err(format!( + "Invalid HEAD ref: {} (must start with refs/heads/)", + head_ref + )); } debug!("Setting HEAD to {} in {}", head_ref, repo_path.display()); @@ -130,7 +133,10 @@ pub fn try_set_head_if_available( ) -> Result { // Check if repository exists if !repo_path.exists() { - debug!("Repository not found at {}, cannot set HEAD", repo_path.display()); + debug!( + "Repository not found at {}, cannot set HEAD", + repo_path.display() + ); return Ok(false); } @@ -149,6 +155,115 @@ pub fn try_set_head_if_available( Ok(true) } +/// Get the commit hash that a ref points to +/// +/// # Arguments +/// * `repo_path` - Path to the bare git repository +/// * `ref_name` - The ref name (e.g., "refs/nostr/") +/// +/// # Returns +/// Some(commit_hash) if the ref exists, None otherwise +pub fn get_ref_commit(repo_path: &Path, ref_name: &str) -> Option { + let output = Command::new("git") + .args(["rev-parse", ref_name]) + .current_dir(repo_path) + .output() + .ok()?; + + if output.status.success() { + Some(String::from_utf8_lossy(&output.stdout).trim().to_string()) + } else { + None + } +} + +/// Delete a git ref from the repository +/// +/// # Arguments +/// * `repo_path` - Path to the bare git repository +/// * `ref_name` - The ref name to delete (e.g., "refs/nostr/") +/// +/// # Returns +/// Ok(()) if successful, Err with error message otherwise +pub fn delete_ref(repo_path: &Path, ref_name: &str) -> Result<(), String> { + debug!("Deleting ref {} from {}", ref_name, repo_path.display()); + + let output = Command::new("git") + .args(["update-ref", "-d", ref_name]) + .current_dir(repo_path) + .output() + .map_err(|e| format!("Failed to execute git update-ref: {}", e))?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(format!("git update-ref -d failed: {}", stderr)); + } + + info!("Deleted ref {} from {}", ref_name, repo_path.display()); + Ok(()) +} + +/// Validate refs/nostr/ ref against expected commit +/// +/// If the ref exists but points to a different commit than expected, +/// the ref is deleted. This is called when a PR event is received to +/// ensure refs/nostr refs are consistent with their corresponding events. +/// +/// # Arguments +/// * `repo_path` - Path to the bare git repository +/// * `event_id` - The event ID (hex string) +/// * `expected_commit` - The commit hash from the event's `c` tag +/// +/// # Returns +/// Ok(true) if ref was deleted (mismatch), Ok(false) if no action taken, Err on failure +pub fn validate_nostr_ref( + repo_path: &Path, + event_id: &str, + expected_commit: &str, +) -> Result { + let ref_name = format!("refs/nostr/{}", event_id); + + // Check if repository exists + if !repo_path.exists() { + debug!( + "Repository not found at {}, skipping ref validation", + repo_path.display() + ); + return Ok(false); + } + + // Check if the ref exists + let current_commit = match get_ref_commit(repo_path, &ref_name) { + Some(commit) => commit, + None => { + debug!("Ref {} does not exist in {}", ref_name, repo_path.display()); + return Ok(false); + } + }; + + // Compare commits + if current_commit == expected_commit { + debug!( + "Ref {} points to correct commit {} in {}", + ref_name, + expected_commit, + repo_path.display() + ); + return Ok(false); + } + + // Commit mismatch - delete the ref + info!( + "Deleting mismatched ref {} in {}: expected {}, found {}", + ref_name, + repo_path.display(), + expected_commit, + current_commit + ); + delete_ref(repo_path, &ref_name)?; + Ok(true) +} + /// Get the current HEAD ref from a repository /// /// # Arguments @@ -178,25 +293,25 @@ pub fn get_repository_head(repo_path: &Path) -> Option { pub fn parse_git_url(path: &str) -> Option<(&str, &str, &str)> { // Remove leading slash let path = path.strip_prefix('/').unwrap_or(path); - + // Split into components let parts: Vec<&str> = path.splitn(3, '/').collect(); - + if parts.len() < 3 { return None; } - + let npub = parts[0]; let repo_part = parts[1]; let subpath = parts[2]; - + // Extract identifier (remove .git suffix if present for the middle part) let identifier = if repo_part.ends_with(".git") { &repo_part[..repo_part.len() - 4] } else { repo_part }; - + Some((npub, identifier, subpath)) } @@ -210,13 +325,13 @@ mod tests { fn create_test_repo() -> (TempDir, PathBuf) { let temp_dir = TempDir::new().unwrap(); let repo_path = temp_dir.path().join("test.git"); - + // Initialize bare repository Command::new("git") .args(["init", "--bare", repo_path.to_str().unwrap()]) .output() .unwrap(); - + (temp_dir, repo_path) } @@ -225,19 +340,23 @@ mod tests { let temp_dir = TempDir::new().unwrap(); let work_dir = temp_dir.path().join("work"); let bare_repo = temp_dir.path().join("test.git"); - + // Initialize bare repository Command::new("git") - .args(["init", "--bare", bare_repo.to_str().unwrap()]) + .args(["init", "--bare", "--initial-branch=main", bare_repo.to_str().unwrap()]) .output() .unwrap(); - + // Clone to working directory Command::new("git") - .args(["clone", bare_repo.to_str().unwrap(), work_dir.to_str().unwrap()]) + .args([ + "clone", + bare_repo.to_str().unwrap(), + work_dir.to_str().unwrap(), + ]) .output() .unwrap(); - + // Configure git for commits Command::new("git") .args(["config", "user.email", "test@test.com"]) @@ -249,7 +368,7 @@ mod tests { .current_dir(&work_dir) .output() .unwrap(); - + // Create a file and commit fs::write(work_dir.join("README.md"), "# Test").unwrap(); Command::new("git") @@ -262,7 +381,7 @@ mod tests { .current_dir(&work_dir) .output() .unwrap(); - + // Get commit hash let output = Command::new("git") .args(["rev-parse", "HEAD"]) @@ -270,41 +389,27 @@ mod tests { .output() .unwrap(); let commit_hash = String::from_utf8_lossy(&output.stdout).trim().to_string(); - + // Push to bare repo Command::new("git") - .args(["push", "origin", "master"]) + .args(["push", "origin", "main"]) .current_dir(&work_dir) .output() .unwrap(); - + (temp_dir, bare_repo, commit_hash) } #[test] fn test_resolve_repo_path() { - let path = resolve_repo_path( - "/data/git", - "npub1abc123", - "my-repo" - ); - assert_eq!( - path, - PathBuf::from("/data/git/npub1abc123/my-repo.git") - ); + let path = resolve_repo_path("/data/git", "npub1abc123", "my-repo"); + assert_eq!(path, PathBuf::from("/data/git/npub1abc123/my-repo.git")); } #[test] fn test_resolve_repo_path_with_git_suffix() { - let path = resolve_repo_path( - "/data/git", - "npub1abc123", - "my-repo.git" - ); - assert_eq!( - path, - PathBuf::from("/data/git/npub1abc123/my-repo.git") - ); + let path = resolve_repo_path("/data/git", "npub1abc123", "my-repo.git"); + assert_eq!(path, PathBuf::from("/data/git/npub1abc123/my-repo.git")); } #[test] @@ -332,7 +437,10 @@ mod tests { #[test] fn test_commit_exists_nonexistent() { let (_temp_dir, repo_path) = create_test_repo(); - assert!(!commit_exists(&repo_path, "deadbeef1234567890abcdef1234567890abcdef")); + assert!(!commit_exists( + &repo_path, + "deadbeef1234567890abcdef1234567890abcdef" + )); } #[test] @@ -344,11 +452,11 @@ mod tests { #[test] fn test_set_repository_head() { let (_temp_dir, repo_path, _commit_hash) = create_test_repo_with_commit(); - + // Default HEAD might be refs/heads/master let result = set_repository_head(&repo_path, "refs/heads/main"); assert!(result.is_ok()); - + let head = get_repository_head(&repo_path); assert_eq!(head, Some("refs/heads/main".to_string())); } @@ -356,7 +464,7 @@ mod tests { #[test] fn test_set_repository_head_invalid_ref() { let (_temp_dir, repo_path) = create_test_repo(); - + // Invalid ref format should fail let result = set_repository_head(&repo_path, "main"); assert!(result.is_err()); @@ -366,13 +474,13 @@ mod tests { #[test] fn test_try_set_head_if_available_commit_missing() { let (_temp_dir, repo_path) = create_test_repo(); - + let result = try_set_head_if_available( &repo_path, "refs/heads/main", "deadbeef1234567890abcdef1234567890abcdef", ); - + // Should return Ok(false) - commit not found assert!(result.is_ok()); assert!(!result.unwrap()); @@ -381,19 +489,15 @@ mod tests { #[test] fn test_try_set_head_if_available_success() { let (_temp_dir, repo_path, commit_hash) = create_test_repo_with_commit(); - - let result = try_set_head_if_available( - &repo_path, - "refs/heads/main", - &commit_hash, - ); - + + let result = try_set_head_if_available(&repo_path, "refs/heads/main", &commit_hash); + // Should return Ok(true) - HEAD was set assert!(result.is_ok()); assert!(result.unwrap()); - + // Verify HEAD was set let head = get_repository_head(&repo_path); assert_eq!(head, Some("refs/heads/main".to_string())); } -} \ No newline at end of file +} diff --git a/src/nostr/builder.rs b/src/nostr/builder.rs index 7aa2b97..8e9926a 100644 --- a/src/nostr/builder.rs +++ b/src/nostr/builder.rs @@ -14,8 +14,8 @@ use nostr_relay_builder::prelude::*; use crate::config::{Config, DatabaseBackend}; use crate::git; use crate::nostr::events::{ - validate_announcement, validate_state, RepositoryAnnouncement, RepositoryState, - KIND_REPOSITORY_ANNOUNCEMENT, KIND_REPOSITORY_STATE, + validate_announcement, validate_state, RepositoryAnnouncement, RepositoryState, KIND_PR, + KIND_PR_UPDATE, KIND_REPOSITORY_ANNOUNCEMENT, KIND_REPOSITORY_STATE, }; /// NIP-34 Write Policy with Full GRASP-01 Event Validation @@ -36,7 +36,11 @@ pub struct Nip34WritePolicy { } impl Nip34WritePolicy { - pub fn new(domain: impl Into, database: Arc, git_data_path: impl Into) -> Self { + pub fn new( + domain: impl Into, + database: Arc, + git_data_path: impl Into, + ) -> Self { Self { domain: domain.into(), database, @@ -48,7 +52,7 @@ impl Nip34WritePolicy { /// Path format: //.git fn ensure_bare_repository(&self, announcement: &RepositoryAnnouncement) -> Result<(), String> { let repo_path = self.git_data_path.join(&announcement.repo_path()); - + // Check if repository already exists if repo_path.exists() { tracing::debug!("Repository already exists at {}", repo_path.display()); @@ -56,13 +60,12 @@ impl Nip34WritePolicy { } // Create parent directory (npub directory) - let parent = repo_path.parent().ok_or_else(|| { - format!("Invalid repository path: {}", repo_path.display()) - })?; - - std::fs::create_dir_all(parent).map_err(|e| { - format!("Failed to create directory {}: {}", parent.display(), e) - })?; + let parent = repo_path + .parent() + .ok_or_else(|| format!("Invalid repository path: {}", repo_path.display()))?; + + std::fs::create_dir_all(parent) + .map_err(|e| format!("Failed to create directory {}: {}", parent.display(), e))?; // Initialize bare repository using git command let output = std::process::Command::new("git") @@ -165,7 +168,11 @@ impl Nip34WritePolicy { tracing::debug!( "Found authorized announcement for {}: owner={}, maintainer={}", identifier, - if is_owner { event.pubkey.to_hex() } else { "n/a".to_string() }, + if is_owner { + event.pubkey.to_hex() + } else { + "n/a".to_string() + }, is_maintainer ); authorized.push(announcement); @@ -198,10 +205,7 @@ impl Nip34WritePolicy { let head_ref = match &state.head { Some(h) => h, None => { - tracing::debug!( - "State event for {} has no HEAD reference", - state.identifier - ); + tracing::debug!("State event for {} has no HEAD reference", state.identifier); return Ok(0); } }; @@ -232,11 +236,9 @@ impl Nip34WritePolicy { }; // Find all announcements where state author is authorized - let announcements = Self::find_authorized_announcements( - database, - &state.identifier, - &state.event.pubkey, - ).await?; + let announcements = + Self::find_authorized_announcements(database, &state.identifier, &state.event.pubkey) + .await?; if announcements.is_empty() { tracing::debug!( @@ -271,7 +273,7 @@ impl Nip34WritePolicy { } // Build repository path: //.git - let repo_path = self.git_data_path.join(&announcement.repo_path()); + let repo_path = self.git_data_path.join(announcement.repo_path().clone()); match git::try_set_head_if_available(&repo_path, head_ref, head_commit) { Ok(true) => { @@ -291,11 +293,7 @@ impl Nip34WritePolicy { ); } Err(e) => { - tracing::warn!( - "Failed to set HEAD in {}: {}", - repo_path.display(), - e - ); + tracing::warn!("Failed to set HEAD in {}: {}", repo_path.display(), e); } } } @@ -338,6 +336,191 @@ impl Nip34WritePolicy { (addressable_refs, event_refs) } + /// Validate refs/nostr/ ref against a PR or PR Update event's `c` tag + /// + /// When a PR event (kind 1618) or PR Update event (kind 1619) is received, + /// this checks if a corresponding refs/nostr/ ref exists in the + /// repository and validates that it points to the correct commit (from the + /// `c` tag). If the ref exists but points to a different commit, the ref is + /// deleted. + /// + /// PR and PR Update events can have multiple `a` tags to update multiple + /// repositories simultaneously. + /// + /// This is part of GRASP-01 compliance: ensuring refs/nostr refs are consistent + /// with their corresponding events. + /// + /// # Arguments + /// * `database` - Database for looking up repository announcements + /// * `event` - The PR event (kind 1618) or PR Update event (kind 1619) + /// + /// # Returns + /// Ok(Some(n)) if n refs were deleted, Ok(None) if no action taken, Err on failure + async fn validate_pr_nostr_ref( + &self, + database: &Arc, + event: &Event, + ) -> Result, String> { + let event_id = event.id.to_hex(); + + // Extract the `c` tag (commit hash) from the PR event + let expected_commit = event.tags.iter().find_map(|tag| { + let tag_vec = tag.clone().to_vec(); + if tag_vec.len() >= 2 && tag_vec[0] == "c" { + Some(tag_vec[1].clone()) + } else { + None + } + }); + + let expected_commit = match expected_commit { + Some(c) => c, + None => { + tracing::debug!( + "PR event {} has no 'c' tag, skipping ref validation", + event_id + ); + return Ok(None); + } + }; + + // Extract ALL `a` tags (repository references) from the PR event + // PR events can reference multiple repositories + // Format: 30617:: + let repo_refs: Vec = event + .tags + .iter() + .filter_map(|tag| { + let tag_vec = tag.clone().to_vec(); + if tag_vec.len() >= 2 && tag_vec[0] == "a" && tag_vec[1].starts_with("30617:") { + Some(tag_vec[1].clone()) + } else { + None + } + }) + .collect(); + + if repo_refs.is_empty() { + tracing::debug!( + "PR event {} has no repo 'a' tags, skipping ref validation", + event_id + ); + return Ok(None); + } + + let mut deleted_count = 0; + + // Process each repository reference + for repo_ref in repo_refs { + // Parse the repo reference: 30617:: + let parts: Vec<&str> = repo_ref.split(':').collect(); + if parts.len() < 3 { + tracing::debug!( + "PR event {} has invalid 'a' tag format: {}", + event_id, + repo_ref + ); + continue; + } + + let repo_pubkey = match PublicKey::from_hex(parts[1]) { + Ok(pk) => pk, + Err(_) => { + tracing::debug!( + "PR event {} has invalid pubkey in 'a' tag: {}", + event_id, + parts[1] + ); + continue; + } + }; + let identifier = parts[2]; + + // Look up repository announcement to get the npub for path + let filter = Filter::new() + .kind(Kind::from(KIND_REPOSITORY_ANNOUNCEMENT)) + .author(repo_pubkey) + .custom_tag( + SingleLetterTag::lowercase(Alphabet::D), + identifier.to_string(), + ); + + let announcements: Vec = match database.query(filter).await { + Ok(events) => events.into_iter().collect(), + Err(e) => { + tracing::warn!( + "Failed to query for repository announcement for PR {}: {}", + event_id, + e + ); + continue; + } + }; + + if announcements.is_empty() { + tracing::debug!( + "No repository announcement found for PR event {} (repo {}:{})", + event_id, + repo_pubkey.to_hex(), + identifier + ); + continue; + } + + // Process each matching announcement (there could be multiple) + for announcement_event in announcements { + let announcement = match RepositoryAnnouncement::from_event(announcement_event) { + Ok(a) => a, + Err(e) => { + tracing::warn!( + "Failed to parse announcement for PR {} validation: {}", + event_id, + e + ); + continue; + } + }; + + // Build repository path + let repo_path = self.git_data_path.join(&announcement.repo_path()); + + // Validate the ref + match git::validate_nostr_ref(&repo_path, &event_id, &expected_commit) { + Ok(true) => { + tracing::info!( + "Deleted mismatched refs/nostr/{} in {} (expected commit {})", + event_id, + repo_path.display(), + expected_commit + ); + deleted_count += 1; + } + Ok(false) => { + tracing::debug!( + "refs/nostr/{} in {} is valid or doesn't exist", + event_id, + repo_path.display() + ); + } + Err(e) => { + tracing::warn!( + "Failed to validate refs/nostr/{} in {}: {}", + event_id, + repo_path.display(), + e + ); + } + } + } + } + + if deleted_count > 0 { + Ok(Some(deleted_count)) + } else { + Ok(None) + } + } + /// Check if any addressable events (repositories) exist in database /// Returns the first matching addressable reference found, or None if none match async fn find_accepted_repository( @@ -377,16 +560,17 @@ impl Nip34WritePolicy { use std::collections::HashMap; let mut by_kind: HashMap> = HashMap::new(); for (addr, kind, pubkey, identifier) in parsed_refs { - by_kind.entry(kind).or_default().push((addr, pubkey, identifier)); + by_kind + .entry(kind) + .or_default() + .push((addr, pubkey, identifier)); } // Query each kind group for (kind, refs) in by_kind { let authors: Vec = refs.iter().map(|(_, pk, _)| *pk).collect(); - - let filter = Filter::new() - .kind(Kind::from(kind)) - .authors(authors); + + let filter = Filter::new().kind(Kind::from(kind)).authors(authors); match database.query(filter).await { Ok(events) => { @@ -445,7 +629,7 @@ impl Nip34WritePolicy { event: &Event, ) -> Result { let kind_u16 = event.kind.as_u16(); - + // Check if this is any kind of replaceable event let is_regular_replaceable = kind_u16 >= 10000 && kind_u16 < 20000; let is_parameterized_replaceable = kind_u16 >= 30000 && kind_u16 < 40000; @@ -454,7 +638,9 @@ impl Nip34WritePolicy { // Build the appropriate address format based on event type let address = if is_parameterized_replaceable { // For parameterized replaceable: kind:pubkey:d-identifier format (2 colons) - let identifier = event.tags.iter() + let identifier = event + .tags + .iter() .find_map(|tag| { let tag_vec = tag.clone().to_vec(); if tag_vec.len() >= 2 && tag_vec[0] == "d" { @@ -464,12 +650,17 @@ impl Nip34WritePolicy { } }) .unwrap_or_default(); // Empty string if no 'd' tag - format!("{}:{}:{}", event.kind.as_u16(), event.pubkey.to_hex(), identifier) + format!( + "{}:{}:{}", + event.kind.as_u16(), + event.pubkey.to_hex(), + identifier + ) } else { // For regular replaceable: kind:pubkey format (1 colon) format!("{}:{}", event.kind.as_u16(), event.pubkey.to_hex()) }; - + // Check addressable reference tags: a, A, q (with address format) let addressable_tags = [ SingleLetterTag::lowercase(Alphabet::A), // 'a' - addressable event reference @@ -479,7 +670,7 @@ impl Nip34WritePolicy { for tag_type in &addressable_tags { let filter = Filter::new().custom_tag(tag_type.clone(), address.clone()); - + match database.query(filter).await { Ok(events) => { if !events.is_empty() { @@ -492,7 +683,7 @@ impl Nip34WritePolicy { } else { // For regular events, check event ID reference tags: e, E, q (with hex ID) let event_id_hex = event.id.to_hex(); - + let event_id_tags = [ SingleLetterTag::lowercase(Alphabet::E), // 'e' - standard event reference SingleLetterTag::uppercase(Alphabet::E), // 'E' - NIP-22 root event reference @@ -501,7 +692,7 @@ impl Nip34WritePolicy { for tag_type in &event_id_tags { let filter = Filter::new().custom_tag(tag_type.clone(), event_id_hex.clone()); - + match database.query(filter).await { Ok(events) => { if !events.is_empty() { @@ -545,7 +736,7 @@ impl WritePolicy for Nip34WritePolicy { // Note: We still accept the event even if repo creation fails // The git operation failure shouldn't prevent event acceptance } - + tracing::debug!( "Accepted repository announcement: {}", event_id_str @@ -563,11 +754,7 @@ impl WritePolicy for Nip34WritePolicy { } } Err(e) => { - tracing::warn!( - "Rejected repository announcement {}: {}", - event_id_str, - e - ); + tracing::warn!("Rejected repository announcement {}: {}", event_id_str, e); PolicyResult::Reject(e.to_string()) } }, @@ -577,7 +764,10 @@ impl WritePolicy for Nip34WritePolicy { match RepositoryState::from_event(event.clone()) { Ok(state) => { // Try to set HEAD for all authorized repos if this is the latest state - match self.try_set_head_for_authorized_repos(&database, &state).await { + match self + .try_set_head_for_authorized_repos(&database, &state) + .await + { Ok(count) if count > 0 => { tracing::info!( "Set HEAD from state event {} for {} repo(s) with identifier {}", @@ -600,11 +790,8 @@ impl WritePolicy for Nip34WritePolicy { ); } } - - tracing::debug!( - "Accepted repository state: {}", - event_id_str - ); + + tracing::debug!("Accepted repository state: {}", event_id_str); PolicyResult::Accept } Err(e) => { @@ -620,14 +807,104 @@ impl WritePolicy for Nip34WritePolicy { } } Err(e) => { + tracing::warn!("Rejected repository state {}: {}", event_id_str, e); + PolicyResult::Reject(e.to_string()) + } + }, + // KIND_PR (1618) and KIND_PR_UPDATE (1619): Validate refs/nostr/ refs before acceptance + KIND_PR | KIND_PR_UPDATE => { + // Validate refs/nostr refs for this PR event + // This deletes any refs/nostr/ that points to wrong commit + if let Err(e) = self.validate_pr_nostr_ref(&database, event).await { tracing::warn!( - "Rejected repository state {}: {}", + "Failed to validate refs/nostr for PR event {}: {}", event_id_str, e ); - PolicyResult::Reject(e.to_string()) + // Don't reject - just log the error and proceed with normal validation } - }, + + // Continue with standard reference checking (same as default case) + let (addressable_refs, event_refs) = Self::extract_reference_tags(event); + + // Check 1: Does this event reference an accepted repository? + match Self::find_accepted_repository(&database, &addressable_refs).await { + Ok(Some(addr_ref)) => { + tracing::debug!( + "Accepted PR event {}: references accepted repository {}", + event_id_str, + addr_ref + ); + return PolicyResult::Accept; + } + Ok(None) => { + // No matching repositories, continue to next check + } + Err(e) => { + tracing::warn!( + "Database query failed for PR {}, rejecting (fail-secure): {}", + event_id_str, + e + ); + return PolicyResult::Reject(format!("Database query failed: {}", e)); + } + } + + // Check 2: Does this event reference an accepted event? + match Self::find_accepted_event(&database, &event_refs).await { + Ok(Some(event_ref)) => { + tracing::debug!( + "Accepted PR event {}: references accepted event {}", + event_id_str, + event_ref + ); + return PolicyResult::Accept; + } + Ok(None) => { + // No matching events, continue to next check + } + Err(e) => { + tracing::warn!( + "Database query failed for PR {}, rejecting (fail-secure): {}", + event_id_str, + e + ); + return PolicyResult::Reject(format!("Database query failed: {}", e)); + } + } + + // Check 3: Is this event referenced by an accepted event? + match Self::is_referenced_by_accepted(&database, event).await { + Ok(true) => { + tracing::debug!( + "Accepted PR event {}: referenced by accepted event", + event_id_str + ); + return PolicyResult::Accept; + } + Ok(false) => { + // No forward references found, continue to rejection + } + Err(e) => { + tracing::warn!( + "Database query failed for PR {}, rejecting (fail-secure): {}", + event_id_str, + e + ); + return PolicyResult::Reject(format!("Database query failed: {}", e)); + } + } + + // No valid references found - reject as orphan event + tracing::info!( + "Rejected orphan PR event {}: no references to accepted repos or events", + event_id_str + ); + PolicyResult::Reject( + "PR event must reference an accepted repository or accepted event" + .to_string(), + ) + } // GRASP-01: Check if event references accepted repositories or events _ => { // Extract all reference tags from event @@ -709,7 +986,7 @@ impl WritePolicy for Nip34WritePolicy { event_refs.len() ); PolicyResult::Reject( - "Event must reference an accepted repository or accepted event".to_string() + "Event must reference an accepted repository or accepted event".to_string(), ) } } @@ -786,4 +1063,4 @@ pub fn create_relay(config: &Config) -> Result { relay: LocalRelay::new(builder), database, }) -} \ No newline at end of file +} diff --git a/src/nostr/events.rs b/src/nostr/events.rs index 97688b1..6a62ccd 100644 --- a/src/nostr/events.rs +++ b/src/nostr/events.rs @@ -15,6 +15,12 @@ pub const KIND_REPOSITORY_ANNOUNCEMENT: u16 = 30617; /// NIP-34 Repository State Announcement (kind 30618) pub const KIND_REPOSITORY_STATE: u16 = 30618; +/// NIP-34 Pull Request (kind 1618) - has `c` tag for commit +pub const KIND_PR: u16 = 1618; + +/// NIP-34 Pull Request Update (kind 1619) - has `c` tag for commit +pub const KIND_PR_UPDATE: u16 = 1619; + /// Repository announcement details extracted from NIP-34 event #[derive(Debug, Clone)] pub struct RepositoryAnnouncement { -- cgit v1.2.3