mirror of
https://relay.ngit.dev/npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr/ngit-grasp.git
synced 2026-10-05 15:08:24 +00:00
feat(whitelist): restore holding scopes on whitelist updates
This commit is contained in:
@@ -367,6 +367,10 @@ reconciled (for announcements that list this relay service but no longer match
|
||||
When `NGIT_BLACKLIST_AUTO_RESTORE=true`, startup also runs a blacklist restore
|
||||
pass between blacklist parity delete and whitelist parity delete.
|
||||
|
||||
Startup also runs a whitelist restore pass after whitelist parity delete. This
|
||||
restores holding scopes tagged `holding-source=whitelist` when they now match
|
||||
`repository_whitelist` (while still respecting blacklist precedence).
|
||||
|
||||
**Future Enhancement (Dynamic Updates):**
|
||||
- Watch for configuration file changes
|
||||
- Trigger deletion/restore immediately on blacklist addition/removal
|
||||
|
||||
@@ -1006,17 +1006,23 @@ Blacklist does **not** affect NIP-11 metadata:
|
||||
|
||||
**Behavior:**
|
||||
|
||||
- When `false` (default): startup runs delete-only parity reconciliation for blacklist/whitelist; unblacklisted repositories in holding are restored only by existing recovery triggers (for example re-announcement).
|
||||
- When `false` (default): startup does not run blacklist-source auto-restore; unblacklisted repositories in holding are restored only by existing recovery triggers (for example re-announcement).
|
||||
- When `true`: startup also scans holding metadata with `holding-source=blacklist`, deduplicates owner+identifier scopes, and attempts restore for scopes that:
|
||||
- are still within `NGIT_HOLDING_RETENTION_SECS`, and
|
||||
- no longer match `NGIT_REPOSITORY_BLACKLIST`.
|
||||
- Scopes that are still blacklisted are skipped (not restored).
|
||||
|
||||
**Related startup behavior (always on when repository whitelist is enabled):**
|
||||
|
||||
- Startup scans holding metadata with `holding-source=whitelist` and attempts restore for scopes that are within retention, now match `NGIT_REPOSITORY_WHITELIST`, and are not blacklisted.
|
||||
- Scopes still not matching `NGIT_REPOSITORY_WHITELIST` are skipped.
|
||||
|
||||
**Startup ordering:**
|
||||
|
||||
1. Blacklist parity delete
|
||||
2. Blacklist auto-restore (if enabled)
|
||||
3. Whitelist parity delete
|
||||
4. Whitelist restore (for scopes now matching `NGIT_REPOSITORY_WHITELIST`)
|
||||
|
||||
This ordering keeps startup reconciliation deterministic when multiple policies interact.
|
||||
|
||||
|
||||
+20
-1
@@ -235,7 +235,9 @@ async fn run_relay(config: Config) -> Result<()> {
|
||||
// 2) blacklist auto-restore (optional, restore previously blacklisted
|
||||
// repos now unblacklisted and still within holding retention)
|
||||
// 3) whitelist parity delete (final acceptance reconciliation for
|
||||
// service-listed repos, preventing restore/re-delete churn on later starts)
|
||||
// service-listed repos)
|
||||
// 4) whitelist restore (restore previously whitelist-deleted repos now
|
||||
// matching repository_whitelist and still within holding retention)
|
||||
let blacklist_stats = relay_with_db
|
||||
.write_policy
|
||||
.run_startup_blacklist_parity_pass()
|
||||
@@ -284,6 +286,23 @@ async fn run_relay(config: Config) -> Result<()> {
|
||||
);
|
||||
}
|
||||
|
||||
let whitelist_restore_stats = relay_with_db
|
||||
.write_policy
|
||||
.run_startup_whitelist_restore_pass()
|
||||
.await;
|
||||
if whitelist_restore_stats.scanned_scopes > 0
|
||||
|| whitelist_restore_stats.attempted_restores > 0
|
||||
{
|
||||
info!(
|
||||
scanned_scopes = whitelist_restore_stats.scanned_scopes,
|
||||
attempted = whitelist_restore_stats.attempted_restores,
|
||||
succeeded = whitelist_restore_stats.successful_restores,
|
||||
failed = whitelist_restore_stats.failed_restores,
|
||||
skipped = whitelist_restore_stats.skipped_scopes,
|
||||
"Startup whitelist restore pass completed"
|
||||
);
|
||||
}
|
||||
|
||||
// Wire the GRASP-06 `/prs/` filesystem cleanup context into
|
||||
// purgatory so the standard expiry sweep can delete dangling
|
||||
// refs/nostr/<event-id> refs (and zero-ref bare repos) when a
|
||||
|
||||
@@ -60,6 +60,20 @@ lazy_static! {
|
||||
.expect("register blacklist startup restore metric");
|
||||
metric
|
||||
};
|
||||
static ref WHITELIST_STARTUP_RESTORE_TOTAL: CounterVec = {
|
||||
let metric = CounterVec::new(
|
||||
Opts::new(
|
||||
"ngit_whitelist_startup_restore_total",
|
||||
"Whitelist startup restore operations by result and reason",
|
||||
),
|
||||
&["result", "reason"],
|
||||
)
|
||||
.expect("build whitelist startup restore metric");
|
||||
REGISTRY
|
||||
.register(Box::new(metric.clone()))
|
||||
.expect("register whitelist startup restore metric");
|
||||
metric
|
||||
};
|
||||
static ref HOLDING_CLEANUP_RUNS_TOTAL: Counter = {
|
||||
let metric = Counter::with_opts(Opts::new(
|
||||
"ngit_holding_cleanup_runs_total",
|
||||
@@ -182,6 +196,30 @@ pub fn record_blacklist_startup_restore_skipped(reason: &str) {
|
||||
.inc();
|
||||
}
|
||||
|
||||
pub fn record_whitelist_startup_restore_attempt() {
|
||||
WHITELIST_STARTUP_RESTORE_TOTAL
|
||||
.with_label_values(&["attempted", "none"])
|
||||
.inc();
|
||||
}
|
||||
|
||||
pub fn record_whitelist_startup_restore_success() {
|
||||
WHITELIST_STARTUP_RESTORE_TOTAL
|
||||
.with_label_values(&["succeeded", "none"])
|
||||
.inc();
|
||||
}
|
||||
|
||||
pub fn record_whitelist_startup_restore_failure() {
|
||||
WHITELIST_STARTUP_RESTORE_TOTAL
|
||||
.with_label_values(&["failed", "none"])
|
||||
.inc();
|
||||
}
|
||||
|
||||
pub fn record_whitelist_startup_restore_skipped(reason: &str) {
|
||||
WHITELIST_STARTUP_RESTORE_TOTAL
|
||||
.with_label_values(&["skipped", reason])
|
||||
.inc();
|
||||
}
|
||||
|
||||
pub fn record_holding_cleanup_run(
|
||||
metadata_deleted: usize,
|
||||
archived_events_deleted: usize,
|
||||
|
||||
@@ -76,6 +76,15 @@ pub struct BlacklistRestoreStats {
|
||||
pub skipped_scopes: usize,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)]
|
||||
pub struct WhitelistRestoreStats {
|
||||
pub scanned_scopes: usize,
|
||||
pub attempted_restores: usize,
|
||||
pub successful_restores: usize,
|
||||
pub failed_restores: usize,
|
||||
pub skipped_scopes: usize,
|
||||
}
|
||||
|
||||
impl std::fmt::Debug for Nip34WritePolicy {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.debug_struct("Nip34WritePolicy")
|
||||
@@ -434,6 +443,118 @@ impl Nip34WritePolicy {
|
||||
stats
|
||||
}
|
||||
|
||||
/// Startup-only whitelist restore pass.
|
||||
///
|
||||
/// Scans holding metadata for scopes deleted by whitelist parity and restores
|
||||
/// owner+identifier scopes that now match `repository_whitelist`, are not
|
||||
/// blacklisted, and are still inside holding retention.
|
||||
pub async fn run_startup_whitelist_restore_pass(&self) -> WhitelistRestoreStats {
|
||||
let whitelist = self.ctx.config.repository_config();
|
||||
if !whitelist.enabled() {
|
||||
return WhitelistRestoreStats::default();
|
||||
}
|
||||
|
||||
let now = Timestamp::now();
|
||||
let retention = self.ctx.config.holding_retention();
|
||||
let scopes = self
|
||||
.ctx
|
||||
.holding
|
||||
.eligible_recovery_scopes_by_source(DeletionSource::Whitelist, now, retention)
|
||||
.await;
|
||||
|
||||
let blacklist = self.ctx.config.blacklist_config();
|
||||
let mut stats = WhitelistRestoreStats {
|
||||
scanned_scopes: scopes.len(),
|
||||
..WhitelistRestoreStats::default()
|
||||
};
|
||||
|
||||
for scope in scopes {
|
||||
let owner_pubkey = match PublicKey::from_hex(&scope.owner_pubkey_hex) {
|
||||
Ok(pubkey) => pubkey,
|
||||
Err(e) => {
|
||||
stats.skipped_scopes += 1;
|
||||
crate::metrics::record_whitelist_startup_restore_skipped("invalid_owner");
|
||||
tracing::warn!(
|
||||
owner = %scope.owner_pubkey_hex,
|
||||
identifier = %scope.identifier,
|
||||
reason = "invalid_owner",
|
||||
error = %e,
|
||||
"Whitelist startup restore: skipping scope"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
|
||||
let owner_npub = match owner_pubkey.to_bech32() {
|
||||
Ok(npub) => npub,
|
||||
Err(e) => {
|
||||
stats.skipped_scopes += 1;
|
||||
crate::metrics::record_whitelist_startup_restore_skipped("owner_bech32_failed");
|
||||
tracing::warn!(
|
||||
owner = %scope.owner_pubkey_hex,
|
||||
identifier = %scope.identifier,
|
||||
reason = "owner_bech32_failed",
|
||||
error = %e,
|
||||
"Whitelist startup restore: skipping scope"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
|
||||
if let Some(reason) = blacklist.check(&owner_npub, &scope.identifier) {
|
||||
stats.skipped_scopes += 1;
|
||||
crate::metrics::record_whitelist_startup_restore_skipped("still_blacklisted");
|
||||
tracing::info!(
|
||||
owner = %scope.owner_pubkey_hex,
|
||||
identifier = %scope.identifier,
|
||||
reason = %reason,
|
||||
"Whitelist startup restore: skipping scope still blacklisted"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
if !whitelist.matches(&owner_npub, &scope.identifier) {
|
||||
stats.skipped_scopes += 1;
|
||||
crate::metrics::record_whitelist_startup_restore_skipped("not_whitelisted");
|
||||
tracing::info!(
|
||||
owner = %scope.owner_pubkey_hex,
|
||||
identifier = %scope.identifier,
|
||||
reason = "not_whitelisted",
|
||||
"Whitelist startup restore: skipping scope"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
stats.attempted_restores += 1;
|
||||
crate::metrics::record_whitelist_startup_restore_attempt();
|
||||
|
||||
if self
|
||||
.maybe_recover_deleted_repository_for_scope(&owner_pubkey, &scope.identifier)
|
||||
.await
|
||||
{
|
||||
stats.successful_restores += 1;
|
||||
crate::metrics::record_whitelist_startup_restore_success();
|
||||
tracing::info!(
|
||||
owner = %scope.owner_pubkey_hex,
|
||||
identifier = %scope.identifier,
|
||||
reason = "now_whitelisted_within_retention",
|
||||
"Whitelist startup restore: restored repository scope"
|
||||
);
|
||||
} else {
|
||||
stats.failed_restores += 1;
|
||||
crate::metrics::record_whitelist_startup_restore_failure();
|
||||
tracing::error!(
|
||||
owner = %scope.owner_pubkey_hex,
|
||||
identifier = %scope.identifier,
|
||||
reason = "recovery_pipeline_failed",
|
||||
"Whitelist startup restore: recovery pipeline failed"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
stats
|
||||
}
|
||||
|
||||
/// Set the local relay for purgatory notifications.
|
||||
///
|
||||
/// This must be called after the relay is created since the relay depends
|
||||
|
||||
@@ -366,6 +366,214 @@ async fn startup_whitelist_scan_leaves_matching_repositories_untouched() {
|
||||
.is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn startup_whitelist_restore_recovers_now_whitelisted_scope_and_is_idempotent() {
|
||||
let relay_dir = tempfile::tempdir().expect("relay tempdir");
|
||||
let git_dir = tempfile::tempdir().expect("git tempdir");
|
||||
let db: SharedDatabase = Arc::new(nostr_memory::MemoryDatabase::unbounded());
|
||||
let holding = HoldingStore::open_lmdb(relay_dir.path(), git_dir.path())
|
||||
.await
|
||||
.expect("open holding lmdb");
|
||||
|
||||
let owner = Keys::generate();
|
||||
let owner_npub = owner.public_key().to_bech32().expect("owner npub");
|
||||
let announcement =
|
||||
make_announcement_with_domain(&owner, "whitelist-restore-repo", "test.example.com");
|
||||
let issue = make_issue(&owner, &announcement);
|
||||
|
||||
db.save_event(&announcement)
|
||||
.await
|
||||
.expect("save announcement");
|
||||
db.save_event(&issue).await.expect("save issue");
|
||||
|
||||
let repo_path = git_dir
|
||||
.path()
|
||||
.join(owner_npub.clone())
|
||||
.join("whitelist-restore-repo.git");
|
||||
std::fs::create_dir_all(repo_path.join("refs")).expect("create bare repo dir");
|
||||
|
||||
let deletion_policy = make_policy(
|
||||
Config {
|
||||
repository_whitelist: "allowed-repo".to_string(),
|
||||
..base_config()
|
||||
},
|
||||
db.clone(),
|
||||
holding.clone(),
|
||||
git_dir.path(),
|
||||
);
|
||||
let deletion_stats = deletion_policy.run_startup_whitelist_parity_pass().await;
|
||||
assert_eq!(deletion_stats.successful_deletions, 1);
|
||||
|
||||
if repo_path.exists() {
|
||||
std::fs::remove_dir_all(&repo_path).expect("remove stale repo directory");
|
||||
}
|
||||
|
||||
let announcement_meta = holding.metadata_for_event(&announcement.id).await;
|
||||
let archive_rel = announcement_meta
|
||||
.iter()
|
||||
.flat_map(|m| m.tags.iter())
|
||||
.find_map(|tag| {
|
||||
let v = tag.as_slice();
|
||||
(v.len() >= 2 && v[0] == HOLDING_ARCHIVE_PATH_TAG).then(|| v[1].clone())
|
||||
})
|
||||
.expect("whitelist deletion should record archive path");
|
||||
let archive_abs = holding
|
||||
.archive_absolute_path(&archive_rel)
|
||||
.expect("resolve archive path");
|
||||
assert!(archive_abs.exists(), "archive must exist before restore");
|
||||
|
||||
let restore_policy = make_policy(
|
||||
Config {
|
||||
repository_whitelist: format!("allowed-repo,{}", owner_npub),
|
||||
..base_config()
|
||||
},
|
||||
db.clone(),
|
||||
holding.clone(),
|
||||
git_dir.path(),
|
||||
);
|
||||
|
||||
let before = render_metrics_snapshot();
|
||||
let restore_stats = restore_policy.run_startup_whitelist_restore_pass().await;
|
||||
assert_eq!(restore_stats.scanned_scopes, 1);
|
||||
assert_eq!(restore_stats.attempted_restores, 1);
|
||||
assert_eq!(restore_stats.successful_restores, 1);
|
||||
assert_eq!(restore_stats.failed_restores, 0);
|
||||
assert_eq!(restore_stats.skipped_scopes, 0);
|
||||
|
||||
assert!(
|
||||
db.event_by_id(&announcement.id)
|
||||
.await
|
||||
.expect("query announcement")
|
||||
.is_some(),
|
||||
"announcement should be restored from holding"
|
||||
);
|
||||
assert!(
|
||||
db.event_by_id(&issue.id)
|
||||
.await
|
||||
.expect("query issue")
|
||||
.is_some(),
|
||||
"dependent events should be restored from holding"
|
||||
);
|
||||
assert!(
|
||||
holding
|
||||
.metadata_for_event(&announcement.id)
|
||||
.await
|
||||
.is_empty(),
|
||||
"holding metadata should be cleaned after restore"
|
||||
);
|
||||
assert!(!archive_abs.exists(), "consumed archive should be deleted");
|
||||
|
||||
let after = render_metrics_snapshot();
|
||||
let attempted_before = metric_value(
|
||||
&before,
|
||||
"ngit_whitelist_startup_restore_total",
|
||||
&[("result", "attempted"), ("reason", "none")],
|
||||
);
|
||||
let attempted_after = metric_value(
|
||||
&after,
|
||||
"ngit_whitelist_startup_restore_total",
|
||||
&[("result", "attempted"), ("reason", "none")],
|
||||
);
|
||||
assert_eq!(attempted_after, attempted_before + 1.0);
|
||||
|
||||
let success_before = metric_value(
|
||||
&before,
|
||||
"ngit_whitelist_startup_restore_total",
|
||||
&[("result", "succeeded"), ("reason", "none")],
|
||||
);
|
||||
let success_after = metric_value(
|
||||
&after,
|
||||
"ngit_whitelist_startup_restore_total",
|
||||
&[("result", "succeeded"), ("reason", "none")],
|
||||
);
|
||||
assert_eq!(success_after, success_before + 1.0);
|
||||
|
||||
let second_stats = restore_policy.run_startup_whitelist_restore_pass().await;
|
||||
assert_eq!(second_stats.scanned_scopes, 0);
|
||||
assert_eq!(second_stats.attempted_restores, 0);
|
||||
assert_eq!(second_stats.successful_restores, 0);
|
||||
assert_eq!(second_stats.failed_restores, 0);
|
||||
assert_eq!(second_stats.skipped_scopes, 0);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn startup_whitelist_restore_skips_scopes_still_not_whitelisted() {
|
||||
let relay_dir = tempfile::tempdir().expect("relay tempdir");
|
||||
let git_dir = tempfile::tempdir().expect("git tempdir");
|
||||
let db: SharedDatabase = Arc::new(nostr_memory::MemoryDatabase::unbounded());
|
||||
let holding = HoldingStore::open_lmdb(relay_dir.path(), git_dir.path())
|
||||
.await
|
||||
.expect("open holding lmdb");
|
||||
|
||||
let owner = Keys::generate();
|
||||
let owner_npub = owner.public_key().to_bech32().expect("owner npub");
|
||||
let announcement =
|
||||
make_announcement_with_domain(&owner, "whitelist-still-blocked-repo", "test.example.com");
|
||||
db.save_event(&announcement)
|
||||
.await
|
||||
.expect("save announcement");
|
||||
|
||||
std::fs::create_dir_all(
|
||||
git_dir
|
||||
.path()
|
||||
.join(owner_npub)
|
||||
.join("whitelist-still-blocked-repo.git")
|
||||
.join("refs"),
|
||||
)
|
||||
.expect("create bare repo dir");
|
||||
|
||||
let deletion_policy = make_policy(
|
||||
Config {
|
||||
repository_whitelist: "allowed-repo".to_string(),
|
||||
..base_config()
|
||||
},
|
||||
db.clone(),
|
||||
holding.clone(),
|
||||
git_dir.path(),
|
||||
);
|
||||
let deletion_stats = deletion_policy.run_startup_whitelist_parity_pass().await;
|
||||
assert_eq!(deletion_stats.successful_deletions, 1);
|
||||
|
||||
let restore_policy = make_policy(
|
||||
Config {
|
||||
repository_whitelist: "allowed-repo".to_string(),
|
||||
..base_config()
|
||||
},
|
||||
db.clone(),
|
||||
holding,
|
||||
git_dir.path(),
|
||||
);
|
||||
|
||||
let before = render_metrics_snapshot();
|
||||
let restore_stats = restore_policy.run_startup_whitelist_restore_pass().await;
|
||||
assert_eq!(restore_stats.scanned_scopes, 1);
|
||||
assert_eq!(restore_stats.attempted_restores, 0);
|
||||
assert_eq!(restore_stats.successful_restores, 0);
|
||||
assert_eq!(restore_stats.failed_restores, 0);
|
||||
assert_eq!(restore_stats.skipped_scopes, 1);
|
||||
|
||||
assert!(
|
||||
db.event_by_id(&announcement.id)
|
||||
.await
|
||||
.expect("query announcement")
|
||||
.is_none(),
|
||||
"still-non-whitelisted scope must not be restored"
|
||||
);
|
||||
|
||||
let after = render_metrics_snapshot();
|
||||
let skipped_before = metric_value(
|
||||
&before,
|
||||
"ngit_whitelist_startup_restore_total",
|
||||
&[("result", "skipped"), ("reason", "not_whitelisted")],
|
||||
);
|
||||
let skipped_after = metric_value(
|
||||
&after,
|
||||
"ngit_whitelist_startup_restore_total",
|
||||
&[("result", "skipped"), ("reason", "not_whitelisted")],
|
||||
);
|
||||
assert_eq!(skipped_after, skipped_before + 1.0);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn startup_blacklist_restore_recovers_unblacklisted_scope_and_is_idempotent() {
|
||||
let relay_dir = tempfile::tempdir().expect("relay tempdir");
|
||||
@@ -699,6 +907,7 @@ async fn metrics_increment_on_blacklist_cleanup_recovery_and_manual_ejection_pat
|
||||
let rendered = render_metrics_snapshot();
|
||||
assert!(rendered.contains("ngit_blacklist_deletions_total"));
|
||||
assert!(rendered.contains("ngit_blacklist_startup_restore_total"));
|
||||
assert!(rendered.contains("ngit_whitelist_startup_restore_total"));
|
||||
assert!(rendered.contains("ngit_holding_cleanup_runs_total"));
|
||||
assert!(rendered.contains("ngit_recovery_total"));
|
||||
assert!(rendered.contains("ngit_manual_ejections_total"));
|
||||
|
||||
Reference in New Issue
Block a user