From 5d2ba5462e9262db8205b60aee7a57ea2ae88f15 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Thu, 20 Aug 2026 07:57:57 +0000 Subject: [PATCH] feat(security): support scoped integrity validation Motivation: Production release-candidate validation must be able to exercise the new storage and event-authorization checker on selected identifier families without immediately sweeping thousands of repositories. The existing manual command also covered storage only, which made its name and operator workflow misleading. Approach: Apply one validated startup identifier scope to both background passes, with an empty scope retaining the secure all-family default and unmatched names counted as failures. Extend durable manual requests so check-only mode compares refs without mutation and --repair applies the same safe authorization reconciliation after storage repair. Expose the scope consistently through CLI/env, the NixOS module, examples, operator docs, architecture notes, and the v3 security warning. Correctness assumptions: Accepted State, PR, and PR Update events remain authoritative, active precisely-scoped purgatory entries remain valid in-flight exceptions, and unexplained PR refs remain preserved for manual inspection. A scoped pass proves only the named identifiers; full v3 assurance still requires removing the scope and completing the default sweep. Excluded scope: This does not tag v3, alter migration behavior, update the production deployment, or delete unexplained refs. It also does not make the manual request synchronous; the live worker continues to consume durable requests. Validation: - cargo clippy --all-targets --locked -- -D warnings - cargo test --lib --locked (895 passed) - focused scoped-selection and non-mutating reconciliation tests - resource-safe NixOS module evaluation of startupIntegrityIdentifiers --- .env.example | 7 + CHANGELOG.md | 19 +- docs/explanation/architecture.md | 4 +- docs/explanation/git-family-object-storage.md | 26 +- docs/how-to/upgrade-git-family-storage.md | 32 ++ docs/reference/configuration.md | 24 ++ nix/module.nix | 15 + src/config.rs | 58 +++ src/git/authorization_integrity.rs | 369 +++++++++++++++--- src/git/integrity.rs | 195 ++++++--- src/main.rs | 6 +- src/server.rs | 1 + 12 files changed, 638 insertions(+), 118 deletions(-) diff --git a/.env.example b/.env.example index b0ea643..f89ad14 100644 --- a/.env.example +++ b/.env.example @@ -87,6 +87,13 @@ # Default: ./data/relay # NGIT_RELAY_DATA_PATH=./data/relay +# Restrict the automatic startup storage- and authorization-integrity passes +# to these comma-separated repository identifiers. Leave empty/unset for the +# required full sweep. Use a scope only for staged validation, then remove it. +# CLI: --startup-integrity-identifiers +# Default: (empty; check every installed identifier family) +# NGIT_STARTUP_INTEGRITY_IDENTIFIERS=repo-one,repo-two + # Database backend for Nostr events # CLI: --database-backend # Options: lmdb, memory diff --git a/CHANGELOG.md b/CHANGELOG.md index 08fd452..fdbd896 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,8 @@ Expect a bit of downtime as a git data migraiton is performed on startup. The la that construct `Config` with a struct literal must provide it. - Added `base_path` to the public `Config` struct. Rust consumers that construct `Config` with a struct literal must provide it. +- Added `startup_integrity_identifiers` to the public `Config` struct. Rust + consumers that construct `Config` with a struct literal must provide it. ### Security @@ -79,16 +81,25 @@ Expect a bit of downtime as a git data migraiton is performed on startup. The la the requested repository namespace, allowing unauthenticated reads of Git repositories accessible to the service. With GRASP-06 enabled, crafted PR submissions could also write Git objects and PR refs into another hosted - repository without its maintainer authorization. Operators must upgrade - to v3.0.0. On every v3 startup, a non-blocking authorization-integrity pass - compares every served branch, tag, `HEAD`, and `refs/nostr/*` ref with the + repository without its maintainer authorization. Any deployment that enabled + GRASP-06 on a tagged build through v2.1.2 must treat hosted-repository + integrity as potentially compromised until it completes the v3 checks. + Operators must upgrade to v3.0.0. By default, every v3 startup runs a + non-blocking authorization-integrity pass that compares every served branch, + tag, `HEAD`, and `refs/nostr/*` ref with the accepted State, PR, and PR Update events (including precisely scoped in-flight events). It repairs unambiguous differences and emits `manual_inspection=true` errors without deleting unexplained PR refs that may be evidence. GRASP-06 operators must check those logs and the terminal `Git authorization-integrity startup pass completed` summary; any non-zero `manual_inspection` or `failed` count means the named repository still - requires review. + requires review. `NGIT_STARTUP_INTEGRITY_IDENTIFIERS` can temporarily scope + both startup integrity passes for release-candidate validation, but scoped + logs establish integrity only for the named repositories. Operators must + remove the scope and complete the default all-family sweep before treating + the v3 upgrade as complete. The live `integrity-check --identifier` command + now compares storage and event authorization together; it is check-only by + default and applies safe fixes with `--repair`. ### Added diff --git a/docs/explanation/architecture.md b/docs/explanation/architecture.md index 91ab6f1..7569fef 100644 --- a/docs/explanation/architecture.md +++ b/docs/explanation/architecture.md @@ -94,7 +94,9 @@ runtime: database initialization. The former checks family objects and thin-view wiring; the latter reconciles each served ref against accepted State, PR, and PR Update events, auto-repairing only unambiguous differences and - logging preserved evidence for manual inspection + logging preserved evidence for manual inspection. Both cover all families + by default, with a shared temporary identifier scope for staged validation; + durable manual requests run the same pair in check-only or repair mode - Serve HTTP + WebSocket until a caller-supplied shutdown future resolves, then stop background mutation, persist a final state snapshot (purgatory and rejected-events cache), and clean up placeholder refs diff --git a/docs/explanation/git-family-object-storage.md b/docs/explanation/git-family-object-storage.md index 23ecccc..8786dd8 100644 --- a/docs/explanation/git-family-object-storage.md +++ b/docs/explanation/git-family-object-storage.md @@ -421,9 +421,13 @@ authorization bypass. Instead it emits a bounded, structured `ERROR` with authorized State still in purgatory also defers State mutation and is reported for inspection rather than racing event promotion. -Both startup passes run in the background after database initialization. They -do not extend the offline migration window or make relay availability depend -on a remote Git server. Their stable terminal log messages are `Git +Both startup passes run in the background after database initialization. By +default they cover every installed identifier family. A temporary +`NGIT_STARTUP_INTEGRITY_IDENTIFIERS` scope can limit both passes to named +families during staged release-candidate validation; an empty scope is required +for the final full sweep. The passes do not extend the offline migration window +or make relay availability depend on a remote Git server. Their stable terminal +log messages are `Git storage-integrity startup pass completed` and `Git authorization-integrity startup pass completed`. Because the authorization pass is online, it refreshes the accepted events immediately before mutation and refuses to overwrite any @@ -436,16 +440,24 @@ can heal pre-existing missing objects. Unindexed legacy packs are preserved under `.grasp/migration/unindexed-packs/` when their backup is retired; there is no separate legacy repair subsystem. -Operators can queue an identifier-scoped storage check in the live process: +Operators can queue an identifier-scoped storage and event-authorization check +in the live process: ```console ngit-grasp integrity-check --identifier example ngit-grasp integrity-check --identifier example --repair ``` -The command writes a durable request beneath `.grasp/integrity-requests/`. -The server consumes it while holding its normal in-process family locks, so a -manual repair cannot race an object-producing request in another view. +Without `--repair`, the command reports structural faults, ref differences, +and manual-inspection findings without changing objects, refs, or `HEAD`. With +`--repair`, it also heals storage from accepted clone sources and applies the +same unambiguous State/PR/PR-Update ref fixes as the startup pass. Unexplained +PR refs remain preserved in both modes. The command writes a durable request +beneath `.grasp/integrity-requests/`. The server consumes it while holding its +normal in-process family locks, so a manual repair cannot race an +object-producing request in another view. Completion is reported separately as +`Manual Git storage-integrity request completed` and `Manual Git +authorization-integrity request completed` with the identifier and repair mode. ## Security and privacy trade-offs diff --git a/docs/how-to/upgrade-git-family-storage.md b/docs/how-to/upgrade-git-family-storage.md index df6d07f..bb35304 100644 --- a/docs/how-to/upgrade-git-family-storage.md +++ b/docs/how-to/upgrade-git-family-storage.md @@ -63,6 +63,26 @@ Git storage-integrity startup pass completed Git authorization-integrity startup pass completed ``` +For a release-candidate deployment, operators may temporarily limit both +startup passes to selected identifiers while validating runtime and log output: + +```bash +NGIT_STARTUP_INTEGRITY_IDENTIFIERS=repo-one,repo-two +``` + +The equivalent NixOS option is +`startupIntegrityIdentifiers = [ "repo-one" "repo-two" ];`. A scoped summary +establishes integrity only for those names; it is not the v3 security sweep. +Remove the scope and observe a successful all-family pair of terminal summaries +before declaring the upgrade complete. The relay stays online during both +scoped and full passes. + +Any service that enabled GRASP-06 on a tagged release through v2.1.2 must treat +its hosted repositories as potentially compromised until the full pass has +completed and every reported exception has been resolved. Passing a selected +scope is useful validation evidence, but it does not clear repositories that +were not named. + These passes are non-blocking: the relay is online while they inspect and heal the migrated views. In the storage summary, an `unresolved` or `failed` count above zero has a corresponding `ERROR` naming the identifier. In the @@ -84,6 +104,18 @@ The authorization pass treats the accepted event database as authoritative: Legacy unscoped placeholders are preserved but reported because they cannot prove which owner and identifier originally received the push. +To repeat both checks for one family without restarting the relay, queue a +check-only request first and inspect the two manual completion summaries: + +```console +ngit-grasp integrity-check --identifier repo-one +ngit-grasp integrity-check --identifier repo-one --repair +``` + +Only the second command applies safe fixes. Re-run the check-only command after +repair; `repair_needed`, `manual_inspection`, and `failed` must all be zero for +that identifier to be considered clean. + A retained backup protects an unhealthy family's legacy data, and a legacy shallow view continues serving at its pre-upgrade level rather than being made less usable. diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index e0bf3c4..1dbd6d4 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -379,6 +379,30 @@ NGIT_DATABASE_BACKEND=memory --- +#### `NGIT_STARTUP_INTEGRITY_IDENTIFIERS` + +**Description:** Comma-separated repository identifiers to check during the +automatic startup storage- and authorization-integrity passes + +**Type:** String list + +**Default:** Empty, meaning every installed identifier family + +**Required:** No + +```bash +# Staged validation of two families +NGIT_STARTUP_INTEGRITY_IDENTIFIERS=repo-one,repo-two +``` + +Use a non-empty value only to validate a release candidate on selected +families while the relay remains online. Remove the setting for the full +security sweep: scoped startup logs establish integrity only for the named +identifiers. Invalid or duplicate identifiers stop startup. The corresponding +NixOS option is `startupIntegrityIdentifiers`. + +--- + ### Proactive Sync Configuration (GRASP-02) These options configure the proactive sync feature that synchronizes events from other relays. diff --git a/nix/module.nix b/nix/module.nix index 5ff25d0..8a8b1c6 100644 --- a/nix/module.nix +++ b/nix/module.nix @@ -190,6 +190,18 @@ let ''; }; + startupIntegrityIdentifiers = mkOption { + type = types.listOf types.str; + default = [ ]; + example = [ "repo-one" "repo-two" ]; + description = '' + Repository identifiers checked by the automatic startup storage- + and authorization-integrity passes. An empty list checks every + installed identifier family and is required for a complete sweep. + A non-empty list is intended only for staged validation. + ''; + }; + metricsEnabled = mkOption { type = types.bool; default = true; @@ -634,6 +646,9 @@ let NGIT_MAX_CONNECTIONS = toString cfg.maxConnections; } // optionalAttrs (cfg.trustedProxyCidrs != [ ]) { NGIT_TRUSTED_PROXY_CIDRS = concatStringsSep "," cfg.trustedProxyCidrs; + } // optionalAttrs (cfg.startupIntegrityIdentifiers != [ ]) { + NGIT_STARTUP_INTEGRITY_IDENTIFIERS = + concatStringsSep "," cfg.startupIntegrityIdentifiers; } // optionalAttrs (cfg.relayName != null) { NGIT_RELAY_NAME = cfg.relayName; } // optionalAttrs (cfg.archiveReadOnly != null) { diff --git a/src/config.rs b/src/config.rs index ae5e056..3d49d31 100644 --- a/src/config.rs +++ b/src/config.rs @@ -367,6 +367,17 @@ pub struct Config { #[arg(long, env = "NGIT_RELAY_DATA_PATH", default_value = "./data/relay")] pub relay_data_path: String, + /// Restrict automatic startup integrity passes to these repository identifiers. + /// + /// Empty by default, which checks every installed identifier family. This is + /// intended only for staged validation before a full production sweep. + #[arg( + long, + env = "NGIT_STARTUP_INTEGRITY_IDENTIFIERS", + value_delimiter = ',' + )] + pub startup_integrity_identifiers: Vec, + /// Server bind address (IP:PORT) #[arg(long, env = "NGIT_BIND_ADDRESS", default_value = "127.0.0.1:7334")] pub bind_address: String, @@ -978,6 +989,22 @@ impl Config { )); } + let mut startup_integrity_identifiers = std::collections::HashSet::new(); + for identifier in &self.startup_integrity_identifiers { + if !crate::git::validate_repository_identifier(identifier) { + return Err(anyhow!( + "NGIT_STARTUP_INTEGRITY_IDENTIFIERS contains invalid repository identifier {:?}", + identifier + )); + } + if !startup_integrity_identifiers.insert(identifier) { + return Err(anyhow!( + "NGIT_STARTUP_INTEGRITY_IDENTIFIERS contains duplicate repository identifier {:?}", + identifier + )); + } + } + // Validate archive configuration let archive_whitelist = WhitelistEntry::parse_whitelist(&self.archive_whitelist); let archive_grasp_services = self.parse_archive_grasp_services(); @@ -1309,6 +1336,7 @@ impl Config { relay_description: "test description".to_string(), git_data_path: "./test_data/git".to_string(), relay_data_path: "./test_data/relay".to_string(), + startup_integrity_identifiers: Vec::new(), bind_address: "127.0.0.1:7334".to_string(), trusted_proxy_cidrs: Vec::new(), database_backend: DatabaseBackend::Memory, @@ -1415,10 +1443,40 @@ mod tests { assert_eq!(config.base_path, "/"); assert_eq!(config.bind_address, "127.0.0.1:7334"); assert!(config.trusted_proxy_cidrs.is_empty()); + assert!(config.startup_integrity_identifiers.is_empty()); // for_testing() uses Memory, but the actual default is Lmdb assert_eq!(config.database_backend, DatabaseBackend::Memory); } + #[test] + fn startup_integrity_scope_parses_and_validates() { + let config = Config::try_parse_from([ + "ngit-grasp", + "--domain", + "example.com", + "--startup-integrity-identifiers", + "repo-one,repo-two", + ]) + .expect("startup integrity scope should parse"); + + assert_eq!( + config.startup_integrity_identifiers, + ["repo-one", "repo-two"] + ); + + let invalid = Config { + startup_integrity_identifiers: vec!["../escape".to_owned()], + ..Config::for_testing() + }; + assert!(invalid.validate().is_err()); + + let duplicate = Config { + startup_integrity_identifiers: vec!["repo".to_owned(), "repo".to_owned()], + ..Config::for_testing() + }; + assert!(duplicate.validate().is_err()); + } + #[test] fn base_path_parses_and_builds_public_routes() { let config = Config::try_parse_from([ diff --git a/src/git/authorization_integrity.rs b/src/git/authorization_integrity.rs index 8ae7839..6e67fb9 100644 --- a/src/git/authorization_integrity.rs +++ b/src/git/authorization_integrity.rs @@ -14,7 +14,9 @@ use nostr_sdk::prelude::{Event, EventId, FromBech32, PublicKey, ToBech32}; use tracing::{error, info, warn}; use super::authorization::{compute_membership, extract_commit_tag, RepositoryData}; -use super::integrity::{discover_families, discover_views, list_refs, FamilyRepairSource}; +use super::integrity::{ + discover_families, discover_views, list_refs, select_families, FamilyRepairSource, +}; use super::storage::{FamilyKey, LocalGitStorage, ObjectFormat}; use crate::nostr::events::RepositoryState; use crate::purgatory::sync::RealSyncContext; @@ -28,6 +30,7 @@ struct PassStats { views_checked: usize, healthy: usize, repaired: usize, + repair_needed: usize, manual_inspection: usize, failed: usize, refs_created: usize, @@ -50,6 +53,18 @@ impl RepairCounts { } } +#[derive(Clone, Copy)] +struct PassMode { + repair: bool, + trigger: &'static str, +} + +struct ReconcileAccumulator<'a> { + repair: bool, + repairs: &'a mut RepairCounts, + issues: &'a mut Vec, +} + #[derive(Debug)] struct IntegrityIssue { category: &'static str, @@ -116,24 +131,37 @@ struct ViewExpectation { /// The relay is already serving traffic while this runs. Missing objects are /// fetched only through the hardened integrity-fetch path, and each family is /// leased only for the short local ref reconciliation phase. -pub async fn run_startup_pass(storage: &LocalGitStorage, source: &RealSyncContext) { +pub async fn run_startup_pass( + storage: &LocalGitStorage, + source: &RealSyncContext, + identifiers: &[String], +) { let mut stats = PassStats::default(); - let families = match discover_families(storage) { - Ok(families) => families, - Err(error) => { - error!(%error, "Git authorization-integrity startup discovery failed"); - stats.failed = 1; - log_completion(&stats); - return; - } - }; + let (families, missing) = + match discover_families(storage).map(|families| select_families(families, identifiers)) { + Ok(selection) => selection, + Err(error) => { + error!(%error, "Git authorization-integrity startup discovery failed"); + stats.failed = 1; + log_startup_completion(&stats, !identifiers.is_empty(), identifiers.len()); + return; + } + }; + stats.failed = missing.len(); + for identifier in missing { + error!( + %identifier, + manual_inspection = true, + "Configured startup authorization-integrity identifier matched no installed family" + ); + } let accepted_prs = match source.accepted_pr_events_for_integrity().await { Ok(events) => events, Err(error) => { error!(%error, "Git authorization-integrity authoritative event query failed"); stats.families = families.len(); - stats.failed = 1; - log_completion(&stats); + stats.failed += 1; + log_startup_completion(&stats, !identifiers.is_empty(), identifiers.len()); return; } }; @@ -143,18 +171,126 @@ pub async fn run_startup_pass(storage: &LocalGitStorage, source: &RealSyncContex families = families.len(), accepted_pr_events = accepted_prs.len(), pending_pr_entries = pending_prs.len(), + scoped = !identifiers.is_empty(), + requested_identifiers = identifiers.len(), "Git authorization-integrity startup pass started" ); + run_families( + storage, + source, + families, + &accepted_prs, + &pending_prs, + PassMode { + repair: true, + trigger: "startup", + }, + &mut stats, + ) + .await; + + log_startup_completion(&stats, !identifiers.is_empty(), identifiers.len()); +} + +/// Run the same authorization reconciliation for one operator-selected +/// identifier. `repair = false` is a non-mutating comparison; `repair = true` +/// applies the same safe automatic fixes as the startup pass. +pub(crate) async fn run_identifier_pass( + storage: &LocalGitStorage, + source: &RealSyncContext, + identifier: &str, + repair: bool, +) { + let scope = [identifier.to_owned()]; + let mut stats = PassStats::default(); + let (families, missing) = + match discover_families(storage).map(|families| select_families(families, &scope)) { + Ok(selection) => selection, + Err(error) => { + error!(%identifier, %error, "Manual Git authorization-integrity discovery failed"); + stats.failed = 1; + log_manual_completion(&stats, identifier, repair); + return; + } + }; + if !missing.is_empty() { + error!( + %identifier, + manual_inspection = true, + "Manual Git authorization-integrity request matched no installed family" + ); + stats.failed = missing.len(); + log_manual_completion(&stats, identifier, repair); + return; + } + let accepted_prs = match source.accepted_pr_events_for_integrity().await { + Ok(events) => events + .into_iter() + .filter(|event| { + event_relevant_to_identifier( + event, + identifier, + source.service_address_for_integrity(), + ) + }) + .collect::>(), + Err(error) => { + error!( + %identifier, + %error, + "Manual Git authorization-integrity authoritative event query failed" + ); + stats.families = families.len(); + stats.failed = 1; + log_manual_completion(&stats, identifier, repair); + return; + } + }; + let pending_prs = source.pending_pr_entries_for_integrity(); + info!( + %identifier, + repair, + families = families.len(), + accepted_pr_events = accepted_prs.len(), + pending_pr_entries = pending_prs.len(), + "Manual Git authorization-integrity request started" + ); + run_families( + storage, + source, + families, + &accepted_prs, + &pending_prs, + PassMode { + repair, + trigger: "manual", + }, + &mut stats, + ) + .await; + log_manual_completion(&stats, identifier, repair); +} + +async fn run_families( + storage: &LocalGitStorage, + source: &RealSyncContext, + families: Vec, + accepted_prs: &[Event], + pending_prs: &[(String, PrPurgatoryEntry)], + mode: PassMode, + stats: &mut PassStats, +) { for key in families { stats.families += 1; if let Err(error) = check_family( storage, source, &key, - &accepted_prs, - &pending_prs, - &mut stats, + accepted_prs, + pending_prs, + mode, + stats, ) .await { @@ -162,6 +298,8 @@ pub async fn run_startup_pass(storage: &LocalGitStorage, source: &RealSyncContex error!( identifier = %key.identifier, object_format = %key.object_format, + trigger = mode.trigger, + repair = mode.repair, %error, manual_inspection = true, "Git authorization-integrity family check failed" @@ -169,34 +307,56 @@ pub async fn run_startup_pass(storage: &LocalGitStorage, source: &RealSyncContex } tokio::task::yield_now().await; } - - log_completion(&stats); } -fn log_completion(stats: &PassStats) { +fn log_startup_completion(stats: &PassStats, scoped: bool, requested_identifiers: usize) { info!( families = stats.families, views_checked = stats.views_checked, healthy = stats.healthy, repaired = stats.repaired, + repair_needed = stats.repair_needed, manual_inspection = stats.manual_inspection, failed = stats.failed, refs_created = stats.refs_created, refs_updated = stats.refs_updated, refs_deleted = stats.refs_deleted, heads_set = stats.heads_set, + scoped, + requested_identifiers, "Git authorization-integrity startup pass completed" ); } +fn log_manual_completion(stats: &PassStats, identifier: &str, repair: bool) { + info!( + %identifier, + repair, + families = stats.families, + views_checked = stats.views_checked, + healthy = stats.healthy, + repaired = stats.repaired, + repair_needed = stats.repair_needed, + manual_inspection = stats.manual_inspection, + failed = stats.failed, + refs_created = stats.refs_created, + refs_updated = stats.refs_updated, + refs_deleted = stats.refs_deleted, + heads_set = stats.heads_set, + "Manual Git authorization-integrity request completed" + ); +} + async fn check_family( storage: &LocalGitStorage, source: &RealSyncContext, key: &FamilyKey, accepted_prs: &[Event], pending_prs: &[(String, PrPurgatoryEntry)], + mode: PassMode, stats: &mut PassStats, ) -> Result<()> { + let PassMode { repair, trigger } = mode; let repo_data = source .accepted_repository_data_for_integrity(&key.identifier) .await @@ -236,7 +396,7 @@ async fn check_family( } } } - if !missing.is_empty() { + if repair && !missing.is_empty() { if let Some((target_view, _, _, _, _)) = checks.first() { fetch_missing_expected_oids(source, &key.identifier, target_view, &missing).await; } @@ -253,7 +413,13 @@ async fn check_family( let pending_states = source.pending_state_events_for_integrity(&key.identifier); let mut candidate_pr_ids: BTreeSet = accepted_prs .iter() - .filter(|event| event_mentions_identifier(event, &key.identifier)) + .filter(|event| { + event_relevant_to_identifier( + event, + &key.identifier, + source.service_address_for_integrity(), + ) + }) .map(|event| event.id) .collect(); for (view, _, _, _, _) in &checks { @@ -288,14 +454,15 @@ async fn check_family( expectation, &baseline_refs, baseline_head.as_deref(), + repair, ) { Ok((repairs, issues)) => { stats.refs_created += repairs.created; stats.refs_updated += repairs.updated; stats.refs_deleted += repairs.deleted; stats.heads_set += repairs.heads_set; - if issues.is_empty() { - if repairs.any() { + if repairs.any() { + if repair { stats.repaired += 1; info!( identifier = %key.identifier, @@ -303,6 +470,8 @@ async fn check_family( view = %view.display(), view_type = identity.kind(), pubkey = %identity.pubkey_hex(), + trigger, + repair, refs_created = repairs.created, refs_updated = repairs.updated, refs_deleted = repairs.deleted, @@ -310,11 +479,28 @@ async fn check_family( "Git authorization-integrity repaired repository view" ); } else { - stats.healthy += 1; + stats.repair_needed += 1; + warn!( + identifier = %key.identifier, + object_format = %key.object_format, + view = %view.display(), + view_type = identity.kind(), + pubkey = %identity.pubkey_hex(), + trigger, + repair, + refs_to_create = repairs.created, + refs_to_update = repairs.updated, + refs_to_delete = repairs.deleted, + head_to_set = repairs.heads_set, + "Git authorization-integrity repository view differs from authoritative events" + ); } - } else { + } else if issues.is_empty() { + stats.healthy += 1; + } + if !issues.is_empty() { stats.manual_inspection += 1; - log_issues(key, &view, &identity, &issues); + log_issues(key, &view, &identity, &issues, trigger, repair); } } Err(error) => { @@ -325,6 +511,8 @@ async fn check_family( view = %view.display(), view_type = identity.kind(), pubkey = %identity.pubkey_hex(), + trigger, + repair, %error, manual_inspection = true, "Git authorization-integrity repository check failed" @@ -560,6 +748,19 @@ fn event_mentions_identifier(event: &Event, identifier: &str) -> bool { !tagged_owners(event, identifier).is_empty() } +fn event_relevant_to_identifier( + event: &Event, + identifier: &str, + service_address: Option<&str>, +) -> bool { + event_mentions_identifier(event, identifier) + || service_address.is_some_and(|domain| { + crate::grasp06::policy::prs_identifiers_named_by_event_clone_tags(event, domain) + .iter() + .any(|candidate| candidate == identifier) + }) +} + fn placeholder_applies_to_view( entry: &PrPurgatoryEntry, identity: &ViewIdentity, @@ -577,6 +778,7 @@ fn reconcile_view( mut expected: ViewExpectation, baseline_refs: &BTreeMap, baseline_head: Option<&str>, + repair: bool, ) -> Result<(RepairCounts, Vec)> { let current: BTreeMap<_, _> = list_refs(view)?.into_iter().collect(); let mut repairs = RepairCounts::default(); @@ -604,15 +806,19 @@ fn reconcile_view( )); continue; } - match delete_ref_cas(view, reference, actual) { - Ok(()) => repairs.deleted += 1, - Err(error) => expected.issues.push(IntegrityIssue::new( - "stale_state_ref_repair_failed", - reference, - Some(actual.clone()), - None, - error.to_string(), - )), + if repair { + match delete_ref_cas(view, reference, actual) { + Ok(()) => repairs.deleted += 1, + Err(error) => expected.issues.push(IntegrityIssue::new( + "stale_state_ref_repair_failed", + reference, + Some(actual.clone()), + None, + error.to_string(), + )), + } + } else { + repairs.deleted += 1; } } } @@ -622,14 +828,18 @@ fn reconcile_view( ¤t, baseline_refs, &expected.state_refs, - &mut repairs, - &mut expected.issues, + &mut ReconcileAccumulator { + repair, + repairs: &mut repairs, + issues: &mut expected.issues, + }, ); reconcile_head( view, expected.state_head.as_deref(), &expected.state_refs, baseline_head, + repair, &mut repairs, &mut expected.issues, ); @@ -653,8 +863,11 @@ fn reconcile_view( ¤t, baseline_refs, &expected.pr_refs, - &mut repairs, - &mut expected.issues, + &mut ReconcileAccumulator { + repair, + repairs: &mut repairs, + issues: &mut expected.issues, + }, ); for (reference, actual) in current @@ -711,8 +924,7 @@ fn reconcile_expected_refs( current: &BTreeMap, baseline: &BTreeMap, expected: &BTreeMap, - repairs: &mut RepairCounts, - issues: &mut Vec, + accumulator: &mut ReconcileAccumulator<'_>, ) { for (reference, target) in expected { let actual = current.get(reference); @@ -720,7 +932,7 @@ fn reconcile_expected_refs( continue; } if baseline.get(reference) != actual { - issues.push(IntegrityIssue::new( + accumulator.issues.push(IntegrityIssue::new( "concurrent_ref_change", reference, actual.cloned(), @@ -730,7 +942,7 @@ fn reconcile_expected_refs( continue; } if !valid_oid(object_format, target) { - issues.push(IntegrityIssue::new( + accumulator.issues.push(IntegrityIssue::new( "invalid_authoritative_oid", reference, actual.cloned(), @@ -740,7 +952,7 @@ fn reconcile_expected_refs( continue; } if !super::oid_exists(view, target) { - issues.push(IntegrityIssue::new( + accumulator.issues.push(IntegrityIssue::new( "missing_authoritative_object", reference, actual.cloned(), @@ -749,6 +961,14 @@ fn reconcile_expected_refs( )); continue; } + if !accumulator.repair { + if actual.is_some() { + accumulator.repairs.updated += 1; + } else { + accumulator.repairs.created += 1; + } + continue; + } match update_ref_cas( view, reference, @@ -756,9 +976,9 @@ fn reconcile_expected_refs( actual.map(String::as_str), object_format, ) { - Ok(()) if actual.is_some() => repairs.updated += 1, - Ok(()) => repairs.created += 1, - Err(error) => issues.push(IntegrityIssue::new( + Ok(()) if actual.is_some() => accumulator.repairs.updated += 1, + Ok(()) => accumulator.repairs.created += 1, + Err(error) => accumulator.issues.push(IntegrityIssue::new( "authorized_ref_repair_failed", reference, actual.cloned(), @@ -774,6 +994,7 @@ fn reconcile_head( expected_head: Option<&str>, state_refs: &BTreeMap, baseline_head: Option<&str>, + repair: bool, repairs: &mut RepairCounts, issues: &mut Vec, ) { @@ -816,6 +1037,10 @@ fn reconcile_head( )); return; } + if !repair { + repairs.heads_set += 1; + return; + } match super::set_repository_head(view, expected_head) { Ok(()) => repairs.heads_set += 1, Err(error) => issues.push(IntegrityIssue::new( @@ -935,7 +1160,14 @@ fn symbolic_head(repo: &Path) -> Result> { } } -fn log_issues(key: &FamilyKey, view: &Path, identity: &ViewIdentity, issues: &[IntegrityIssue]) { +fn log_issues( + key: &FamilyKey, + view: &Path, + identity: &ViewIdentity, + issues: &[IntegrityIssue], + trigger: &'static str, + repair: bool, +) { for issue in issues.iter().take(MAX_ISSUES_PER_VIEW) { error!( identifier = %key.identifier, @@ -943,6 +1175,8 @@ fn log_issues(key: &FamilyKey, view: &Path, identity: &ViewIdentity, issues: &[I view = %view.display(), view_type = identity.kind(), pubkey = %identity.pubkey_hex(), + trigger, + repair, category = issue.category, reference = %issue.reference, actual_oid = issue.actual_oid.as_deref().unwrap_or(""), @@ -957,6 +1191,8 @@ fn log_issues(key: &FamilyKey, view: &Path, identity: &ViewIdentity, issues: &[I identifier = %key.identifier, object_format = %key.object_format, view = %view.display(), + trigger, + repair, suppressed = issues.len() - MAX_ISSUES_PER_VIEW, manual_inspection = true, "Additional Git authorization-integrity issues suppressed for this view" @@ -1055,6 +1291,7 @@ mod tests { expectation, &baseline_refs, baseline_head.as_deref(), + true, ) .unwrap() } @@ -1108,6 +1345,41 @@ mod tests { assert!(issues.is_empty(), "{issues:#?}"); } + #[test] + fn check_only_reports_safe_repairs_without_mutating_refs() { + let (_temp, _storage, _key, view, commits) = fixture(); + let expected_pr = format!("refs/nostr/{}", "d".repeat(64)); + git(&view, &["update-ref", "refs/heads/main", &commits[0]]); + git(&view, &["update-ref", "refs/heads/stale", &commits[0]]); + let baseline_refs = list_refs(&view).unwrap().into_iter().collect(); + let baseline_head = symbolic_head(&view).unwrap(); + let expectation = ViewExpectation { + state_refs: BTreeMap::from([("refs/heads/main".to_owned(), commits[1].clone())]), + state_head: Some("refs/heads/main".to_owned()), + has_authoritative_state: true, + pr_refs: BTreeMap::from([(expected_pr.clone(), commits[2].clone())]), + ..ViewExpectation::default() + }; + + let (repairs, issues) = reconcile_view( + &view, + ObjectFormat::Sha1, + expectation, + &baseline_refs, + baseline_head.as_deref(), + false, + ) + .unwrap(); + + assert_eq!(repairs.updated, 1); + assert_eq!(repairs.created, 1); + assert_eq!(repairs.deleted, 1); + assert!(issues.is_empty(), "{issues:#?}"); + assert_eq!(git(&view, &["rev-parse", "refs/heads/main"]), commits[0]); + assert!(ref_exists(&view, "refs/heads/stale")); + assert!(!ref_exists(&view, &expected_pr)); + } + #[test] fn missing_authoritative_object_requires_manual_inspection() { let (_temp, _storage, _key, view, commits) = fixture(); @@ -1150,6 +1422,7 @@ mod tests { expectation, &baseline_refs, baseline_head.as_deref(), + true, ) .unwrap(); diff --git a/src/git/integrity.rs b/src/git/integrity.rs index de4fad3..bc82a2f 100644 --- a/src/git/integrity.rs +++ b/src/git/integrity.rs @@ -27,8 +27,8 @@ use super::validate_repository_identifier; const REQUEST_VERSION: u32 = 1; const REQUEST_POLL_INTERVAL: Duration = Duration::from_secs(5); -/// Arguments for queueing an identifier-family integrity check in the live -/// relay process. +/// Arguments for queueing storage- and authorization-integrity checks in the +/// live relay process. #[derive(Debug, Args)] pub struct IntegrityCheckArgs { /// Repository identifier (`d` tag value). All object formats and views for @@ -36,7 +36,8 @@ pub struct IntegrityCheckArgs { #[arg(long)] pub identifier: String, - /// Attempt repair using accepted repository clone URLs. + /// Apply safe storage and ref repairs. Without this flag the request only + /// reports differences from structural and authoritative event state. #[arg(long, default_value_t = false)] pub repair: bool, @@ -157,15 +158,22 @@ struct PassStats { /// Start the non-blocking integrity worker. /// -/// It checks and heals every installed family once after startup, then -/// consumes identifier-scoped requests written by [`enqueue_manual_check`]. +/// It checks and heals the configured startup scope once, then consumes +/// identifier-scoped requests written by [`enqueue_manual_check`]. An empty +/// startup scope means every installed family. pub fn spawn_integrity_worker( storage: LocalGitStorage, source: Arc, + startup_identifiers: Vec, ) -> JoinHandle<()> { tokio::spawn(async move { - run_startup_pass(&storage, source.as_ref()).await; - super::authorization_integrity::run_startup_pass(&storage, source.as_ref()).await; + run_startup_pass(&storage, source.as_ref(), &startup_identifiers).await; + super::authorization_integrity::run_startup_pass( + &storage, + source.as_ref(), + &startup_identifiers, + ) + .await; let first = tokio::time::Instant::now() + REQUEST_POLL_INTERVAL; let mut interval = tokio::time::interval_at(first, REQUEST_POLL_INTERVAL); loop { @@ -175,19 +183,35 @@ pub fn spawn_integrity_worker( }) } -async fn run_startup_pass(storage: &LocalGitStorage, source: &S) { - let families = match discover_families(storage) { - Ok(families) => families, - Err(error) => { - error!(%error, "Git storage-integrity startup discovery failed"); - return; - } +async fn run_startup_pass( + storage: &LocalGitStorage, + source: &S, + identifiers: &[String], +) { + let (families, missing) = + match discover_families(storage).map(|families| select_families(families, identifiers)) { + Ok(selection) => selection, + Err(error) => { + error!(%error, "Git storage-integrity startup discovery failed"); + return; + } + }; + let mut stats = PassStats { + failed: missing.len(), + ..PassStats::default() }; + for identifier in missing { + error!( + %identifier, + "Configured startup integrity identifier matched no installed family" + ); + } info!( families = families.len(), + scoped = !identifiers.is_empty(), + requested_identifiers = identifiers.len(), "Git storage-integrity startup pass started" ); - let mut stats = PassStats::default(); for key in families { run_one_family(storage, &key, source, true, "startup", &mut stats).await; tokio::task::yield_now().await; @@ -202,9 +226,9 @@ async fn run_startup_pass(storage: &LocalGitStor ); } -async fn process_manual_requests( +async fn process_manual_requests( storage: &LocalGitStorage, - source: &S, + source: &crate::purgatory::sync::RealSyncContext, ) { let requests = match load_requests(storage) { Ok(requests) => requests, @@ -214,47 +238,62 @@ async fn process_manual_requests( } }; for (path, request) in requests { - let families = match discover_families(storage) { - Ok(families) => families - .into_iter() - .filter(|key| key.identifier == request.identifier) - .collect::>(), - Err(error) => { - error!( - identifier = %request.identifier, - %error, - "Manual Git integrity family discovery failed" - ); - remove_consumed_request(&path); - continue; - } - }; - if families.is_empty() { - error!( - identifier = %request.identifier, - "Manual Git integrity request matched no identifier family" - ); - remove_consumed_request(&path); - continue; + if run_manual_storage_request(storage, source, &request).await { + super::authorization_integrity::run_identifier_pass( + storage, + source, + &request.identifier, + request.repair, + ) + .await; } - let mut stats = PassStats::default(); - for key in families { - run_one_family(storage, &key, source, request.repair, "manual", &mut stats).await; - } - info!( - identifier = %request.identifier, - repair = request.repair, - checked = stats.checked, - healthy = stats.healthy, - repaired = stats.repaired, - unresolved = stats.unresolved, - failed = stats.failed, - "Manual Git identifier-family integrity request completed" - ); remove_consumed_request(&path); } } +async fn run_manual_storage_request( + storage: &LocalGitStorage, + source: &S, + request: &IntegrityRequest, +) -> bool { + let families = match discover_families(storage) { + Ok(families) => families + .into_iter() + .filter(|key| key.identifier == request.identifier) + .collect::>(), + Err(error) => { + error!( + identifier = %request.identifier, + %error, + "Manual Git integrity family discovery failed" + ); + return false; + } + }; + if families.is_empty() { + error!( + identifier = %request.identifier, + "Manual Git integrity request matched no identifier family" + ); + return false; + } + let mut stats = PassStats::default(); + for key in families { + run_one_family(storage, &key, source, request.repair, "manual", &mut stats).await; + } + info!( + identifier = %request.identifier, + repair = request.repair, + checked = stats.checked, + healthy = stats.healthy, + repaired = stats.repaired, + unresolved = stats.unresolved, + failed = stats.failed, + "Manual Git storage-integrity request completed" + ); + true +} + async fn run_one_family( storage: &LocalGitStorage, key: &FamilyKey, @@ -530,6 +569,34 @@ pub fn discover_families(storage: &LocalGitStorage) -> Result> { Ok(families) } +/// Restrict discovered families to an operator-supplied identifier scope. +/// +/// An empty scope selects every family. The second result lists requested +/// identifiers that did not match either object format, so a typo cannot +/// silently produce a reassuring empty pass. +pub(crate) fn select_families( + families: Vec, + identifiers: &[String], +) -> (Vec, Vec) { + if identifiers.is_empty() { + return (families, Vec::new()); + } + let requested: BTreeSet<_> = identifiers.iter().cloned().collect(); + let mut found = BTreeSet::new(); + let selected = families + .into_iter() + .filter(|key| { + let selected = requested.contains(&key.identifier); + if selected { + found.insert(key.identifier.clone()); + } + selected + }) + .collect(); + let missing = requested.difference(&found).cloned().collect(); + (selected, missing) +} + /// Inspect one family and every owner or `/prs/` view backed by its identifier. pub fn inspect_family(storage: &LocalGitStorage, key: &FamilyKey) -> Result { let family = storage.family_repo_path(key); @@ -1078,6 +1145,22 @@ mod tests { assert!(report.pack_errors[0].contains("pack has no index")); } + #[test] + fn startup_scope_selects_named_families_and_reports_missing_names() { + let families = vec![ + FamilyKey::sha1("alpha").unwrap(), + FamilyKey::new(ObjectFormat::Sha256, "alpha").unwrap(), + FamilyKey::sha1("beta").unwrap(), + ]; + let scope = vec!["alpha".to_owned(), "missing".to_owned()]; + + let (selected, missing) = select_families(families, &scope); + + assert_eq!(selected.len(), 2); + assert!(selected.iter().all(|key| key.identifier == "alpha")); + assert_eq!(missing, vec!["missing"]); + } + #[test] fn manual_requests_are_durable_and_identifier_scoped() { let temp = tempfile::tempdir().unwrap(); @@ -1110,7 +1193,7 @@ mod tests { } #[tokio::test] - async fn manual_requests_are_consumed_by_the_family_worker() { + async fn manual_storage_requests_are_identifier_scoped() { let (temp, storage, _key, _view, _commit) = fixture(); let args = IntegrityCheckArgs { identifier: "shared".to_owned(), @@ -1123,7 +1206,9 @@ mod tests { family_objects: temp.path().join("unused"), }; - process_manual_requests(&storage, &source).await; + let requests = load_requests(&storage).unwrap(); + assert!(run_manual_storage_request(&storage, &source, &requests[0].1).await); + remove_consumed_request(&request); assert!(!request.exists()); } diff --git a/src/main.rs b/src/main.rs index 653caf9..d409139 100644 --- a/src/main.rs +++ b/src/main.rs @@ -37,7 +37,7 @@ enum Cli { /// This is an operator/admin maintenance command and is idempotent. HoldingEject(nostr::lifecycle::HoldingEjectArgs), - /// Queue an identifier-family integrity check in the running relay. + /// Queue storage- and event-authorization integrity checks in the running relay. IntegrityCheck(ngit_grasp::git::integrity::IntegrityCheckArgs), } @@ -73,9 +73,9 @@ async fn main() -> Result<()> { println!( "Queued {} for identifier '{}' at {}", if integrity_args.repair { - "integrity check and repair" + "storage and authorization-integrity check and repair" } else { - "integrity check" + "storage and authorization-integrity check" }, integrity_args.identifier, path.display() diff --git a/src/server.rs b/src/server.rs index 49ae032..d9ae464 100644 --- a/src/server.rs +++ b/src/server.rs @@ -455,6 +455,7 @@ impl RelayServer { background_tasks.push(git::integrity::spawn_integrity_worker( git_storage, sync_ctx.clone(), + config.startup_integrity_identifiers.clone(), )); info!("Git storage and authorization-integrity worker started");