From 31c60c7e402d709a57ce46771dce3f53a86206f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Sat, 29 Aug 2026 17:28:53 -0700 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=90=9B=20fix(config):=20propagate=20r?= =?UTF-8?q?econcile=20errors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `build_state` discarded repository reconciliation errors, so the server could expose a configured route without its persisted identity. Propagate the storage error before `build_state` publishes application state. Nodes without write access keep skipping repository reconciliation. --- .../peryx/src/config/repository_migration.rs | 22 ++--- crates/peryx/src/server.rs | 15 +++- .../unit/config/repository_migration/tests.rs | 11 +-- crates/peryx/tests/unit/tests/server_tests.rs | 84 +++++++++++++++++++ 4 files changed, 116 insertions(+), 16 deletions(-) diff --git a/crates/peryx/src/config/repository_migration.rs b/crates/peryx/src/config/repository_migration.rs index d0800c2b5..43cbc905c 100644 --- a/crates/peryx/src/config/repository_migration.rs +++ b/crates/peryx/src/config/repository_migration.rs @@ -1,7 +1,7 @@ use std::time::{SystemTime, UNIX_EPOCH}; use peryx_identity::UserId; -use peryx_storage::meta::{DesiredRepository, MetaStore}; +use peryx_storage::meta::{DesiredRepository, MetaStore, ReconcileRepositoryError}; use super::IndexConfig; @@ -27,17 +27,19 @@ fn desired(config: &IndexConfig) -> DesiredRepository { } } -/// Give every configured index a persisted repository record. +/// Persist repository identities before the server exposes configured routes. /// -/// Each index reconciles by route: a new route mints a record, an existing route reuses its id, so a -/// restart adds nothing and a later rename never re-homes a reference. Unchanged configuration bumps -/// no version. A route the store cannot hold as a repository, an over-long one for example, leaves -/// the batch unwritten and logs rather than failing an otherwise healthy boot. -pub fn reconcile_configured_repositories(meta: &MetaStore, configs: &[IndexConfig]) { +/// Route matching keeps IDs stable and advances versions only when a stored definition changes. +/// +/// # Errors +/// Returns an error when validation or persistence prevents the atomic batch. +pub fn reconcile_configured_repositories( + meta: &MetaStore, + configs: &[IndexConfig], +) -> Result<(), ReconcileRepositoryError> { let desired: Vec = configs.iter().map(desired).collect(); - if let Err(error) = meta.reconcile_repositories(&desired, &system_actor(), unix_now()) { - tracing::warn!(%error, "could not assign stable ids to configured repositories"); - } + meta.reconcile_repositories(&desired, &system_actor(), unix_now()) + .map(drop) } #[cfg(test)] diff --git a/crates/peryx/src/server.rs b/crates/peryx/src/server.rs index 77b9c6f8a..7456b2bc1 100644 --- a/crates/peryx/src/server.rs +++ b/crates/peryx/src/server.rs @@ -223,7 +223,7 @@ fn build_state_with_active_backend_and_plugins( } attach_policy_decision_recorders(&meta, &mut indexes, read_only)?; if !read_only { - crate::config::reconcile_configured_repositories(&meta, &configs); + persist_configured_repositories(&meta, &configs)?; } let ecosystem_settings = build_index_settings_with_plugins(&configs, plugins)?; let webhooks = build_webhooks(&configs, plugins)?; @@ -257,6 +257,19 @@ fn build_state_with_active_backend_and_plugins( Ok(Arc::new(state)) } +fn persist_configured_repositories(meta: &MetaStore, configs: &[IndexConfig]) -> anyhow::Result<()> { + crate::config::reconcile_configured_repositories(meta, configs).with_context(|| { + format!( + "persist configured repositories [{}]", + configs + .iter() + .map(|config| config.route.as_str()) + .collect::>() + .join(", ") + ) + }) +} + fn resolve_signing_key(config: &Config) -> anyhow::Result> { const MIN_BYTES: usize = 32; let Some(source) = &config.auth.signing_key else { diff --git a/crates/peryx/tests/unit/config/repository_migration/tests.rs b/crates/peryx/tests/unit/config/repository_migration/tests.rs index c73a852de..d9a316d9f 100644 --- a/crates/peryx/tests/unit/config/repository_migration/tests.rs +++ b/crates/peryx/tests/unit/config/repository_migration/tests.rs @@ -24,13 +24,13 @@ fn test_reconcile_assigns_stable_ids_idempotently_across_boots() { let dir = tempfile::tempdir().unwrap(); let store = MetaStore::open(dir.path().join("peryx.redb")).unwrap(); - reconcile_configured_repositories(&store, &config.indexes); + reconcile_configured_repositories(&store, &config.indexes).unwrap(); let first_boot = routes_to_id_and_version(&store); assert_eq!(first_boot.len(), config.indexes.len()); assert!(first_boot.values().all(|(id, version)| !id.is_empty() && *version == 1)); - reconcile_configured_repositories(&store, &config.indexes); + reconcile_configured_repositories(&store, &config.indexes).unwrap(); assert_eq!(routes_to_id_and_version(&store), first_boot); } @@ -45,10 +45,10 @@ fn test_reconcile_renames_default_virtual_indexes_without_changing_ids() { .name = "root/pypi".to_owned(); let dir = tempfile::tempdir().unwrap(); let store = MetaStore::open(dir.path().join("peryx.redb")).unwrap(); - reconcile_configured_repositories(&store, &old.indexes); + reconcile_configured_repositories(&store, &old.indexes).unwrap(); let previous = store.repository_by_route("root/pypi").unwrap().unwrap(); - reconcile_configured_repositories(&store, &Config::default().indexes); + reconcile_configured_repositories(&store, &Config::default().indexes).unwrap(); let current = store.repository_by_route("root/pypi").unwrap().unwrap(); assert_eq!(current.id, previous.id); @@ -63,7 +63,8 @@ fn test_reconcile_writes_nothing_when_a_route_cannot_be_a_repository() { let dir = tempfile::tempdir().unwrap(); let store = MetaStore::open(dir.path().join("peryx.redb")).unwrap(); - reconcile_configured_repositories(&store, &config.indexes); + let error = reconcile_configured_repositories(&store, &config.indexes).unwrap_err(); + assert_eq!(error.to_string(), "repository route exceeds 512 bytes"); assert!(routes_to_id_and_version(&store).is_empty()); } diff --git a/crates/peryx/tests/unit/tests/server_tests.rs b/crates/peryx/tests/unit/tests/server_tests.rs index 734476cef..e8c13dc5c 100644 --- a/crates/peryx/tests/unit/tests/server_tests.rs +++ b/crates/peryx/tests/unit/tests/server_tests.rs @@ -401,6 +401,90 @@ fn test_build_state_opens_configured_data_dir() { assert!(dir.path().join("peryx.redb").exists()); } +#[test] +fn test_build_state_reports_configured_repository_persistence_failure() { + let dir = tempfile::tempdir().unwrap(); + let config = config_with_corrupt_repository(&dir); + + let error = build_state(&config) + .err() + .expect("expected repository persistence error"); + + assert_eq!( + error.to_string(), + format!("persist configured repositories [{}]", config.indexes[0].route) + ); +} + +#[test] +fn test_build_state_read_only_skips_repository_reconciliation() { + let dir = tempfile::tempdir().unwrap(); + let mut config = config_with_corrupt_repository(&dir); + config.read_only = true; + + let state = build_state(&config).unwrap(); + + assert_eq!(state.serving.indexes[0].route, config.indexes[0].route); +} + +#[test] +fn test_build_state_preserves_configured_repository_identity() { + let dir = tempfile::tempdir().unwrap(); + let config = Config { + data_dir: dir.path().to_path_buf(), + ..neutral_config() + }; + let first = build_state(&config).unwrap(); + let first_repositories = routes_to_id_and_version(&first.serving.meta); + drop(first); + + let second = build_state(&config).unwrap(); + + assert_eq!( + (routes_to_id_and_version(&second.serving.meta), first_repositories.len()), + (first_repositories, config.indexes.len()) + ); +} + +fn config_with_corrupt_repository(dir: &tempfile::TempDir) -> Config { + let mut config = Config { + data_dir: dir.path().to_path_buf(), + ..neutral_config() + }; + config.indexes.truncate(1); + let state = build_state(&config).unwrap(); + let repository = state + .serving + .meta + .repository_by_route(&config.indexes[0].route) + .unwrap() + .unwrap(); + drop(state); + let database = redb::Database::open(dir.path().join("peryx.redb")).unwrap(); + let transaction = database.begin_write().unwrap(); + { + let mut repositories = transaction + .open_table(redb::TableDefinition::<&str, &[u8]>::new("repository")) + .unwrap(); + repositories.insert(repository.id.as_str(), b"{".as_slice()).unwrap(); + } + transaction.commit().unwrap(); + config +} + +fn routes_to_id_and_version(store: &MetaStore) -> std::collections::BTreeMap { + store + .list_repositories(&peryx_storage::meta::RepositoryQuery { + limit: 100, + ..peryx_storage::meta::RepositoryQuery::default() + }) + .unwrap() + .repositories + .into_iter() + .map(|record| (record.route, (record.id.as_str().to_owned(), record.version))) + .collect() +} + #[test] fn test_build_state_repairs_abandoned_quota_before_admission() { let dir = tempfile::tempdir().unwrap(); From f7912c67ed745cc0597d73cfb17b98c6e9f4c68b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Sat, 29 Aug 2026 18:27:30 -0700 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=A7=AA=20test(pypi):=20exercise=20sha?= =?UTF-8?q?dow=20read=20faults?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An oversized repository route stopped reaching the shadow query after startup began enforcing repository persistence errors. Corrupt a valid stored decision through the metadata migration boundary so both fixtures exercise the intended decode failure through the public HTTP route. --- .../tests/system-suite/tests/cases/shadow.rs | 62 ++++++++++++------- .../tests/unit/shadow/coverage_tests.rs | 40 ++++++++++-- 2 files changed, 76 insertions(+), 26 deletions(-) diff --git a/crates/peryx-ecosystem-pypi/tests/system-suite/tests/cases/shadow.rs b/crates/peryx-ecosystem-pypi/tests/system-suite/tests/cases/shadow.rs index 21cdb789b..288444aff 100644 --- a/crates/peryx-ecosystem-pypi/tests/system-suite/tests/cases/shadow.rs +++ b/crates/peryx-ecosystem-pypi/tests/system-suite/tests/cases/shadow.rs @@ -12,7 +12,9 @@ use peryx_ecosystem_pypi::upload::Uploaded; use peryx_ecosystem_pypi::{CoreMetadata, File, Provenance, Yanked}; use peryx_identity::{GrantScope, Role}; use peryx_policy::{PolicyAction, PolicyDecisionState}; -use peryx_storage::meta::NewPolicyDecision; +use peryx_storage::meta::{ + LegacyMetadataSource, MetadataMigration, MetadataRecord, MetadataRecordSet, NewPolicyDecision, +}; use tower::ServiceExt as _; use crate::config::{Config, IndexConfig, IndexKind, SecretSource}; @@ -163,23 +165,6 @@ async fn seeded_state() -> (tempfile::TempDir, Arc) { (dir, state) } -async fn seeded_state_named(repository: &str) -> (tempfile::TempDir, Arc) { - let dir = tempfile::tempdir().unwrap(); - let mut root = virtual_root(); - root.name = repository.to_owned(); - root.route = repository.to_owned(); - let config = Config { - data_dir: dir.path().to_path_buf(), - indexes: vec![cached_pypi(), hosted(), root], - ..Config::default() - }; - let state = build_state(&config).unwrap(); - provision_admin(&state).await; - seed_hosted(&state); - seed_cached(&state); - (dir, state) -} - async fn app() -> (tempfile::TempDir, axum::Router) { let (dir, state) = seeded_state().await; (dir, router_for(state)) @@ -339,13 +324,21 @@ async fn test_a_denied_filename_carries_its_decision_on_every_member_row() { #[tokio::test] async fn test_a_failed_decision_read_surfaces_as_a_server_error() { - let repository = "r".repeat(513); - let (_dir, state) = seeded_state_named(&repository).await; + let (_dir, state) = seeded_state().await; + record_decision( + &state, + Some(HOSTED_FILE), + PolicyAction::Serve, + PolicyDecisionState::Allow, + "read-fault", + None, + ); + state.serving.meta.migrate_metadata(&UnreadableDecision).unwrap(); let router = router_for(state); let (status, _headers, body) = get( &router, - &format!("/+shadow/candidates?repository={repository}&project=acme-pkg"), + "/+shadow/candidates?repository=root/pypi&project=acme-pkg", Some(("Alice", PASSWORD)), ) .await; @@ -355,6 +348,33 @@ async fn test_a_failed_decision_read_surfaces_as_a_server_error() { assert_eq!(document["error"], "shadow query failed"); } +struct UnreadableDecision; + +impl MetadataMigration for UnreadableDecision { + fn name(&self) -> &'static str { + "unreadable-decision" + } + + fn record_sets(&self) -> &[MetadataRecordSet] { + &[MetadataRecordSet::PolicyDecisionHistory] + } + + fn legacy_sources(&self) -> &[LegacyMetadataSource] { + &[] + } + + fn rewrite( + &self, + _record_set: MetadataRecordSet, + record: &MetadataRecord, + ) -> Result, String> { + Ok(Some(MetadataRecord { + key: record.key.clone(), + value: b"{".to_vec(), + })) + } +} + #[tokio::test] async fn test_a_waiting_filename_reports_its_retry_window() { let (_dir, state) = seeded_state().await; diff --git a/crates/peryx-ecosystem-pypi/tests/unit/shadow/coverage_tests.rs b/crates/peryx-ecosystem-pypi/tests/unit/shadow/coverage_tests.rs index 19ccb2001..0da816fb7 100644 --- a/crates/peryx-ecosystem-pypi/tests/unit/shadow/coverage_tests.rs +++ b/crates/peryx-ecosystem-pypi/tests/unit/shadow/coverage_tests.rs @@ -12,7 +12,9 @@ use peryx_identity::{Action, Glob, Grant, GrantScope, IndexAcl, NamedToken, Role use peryx_index::{Index, IndexKind}; use peryx_policy::{Policy, PolicyAction, PolicyDecisionState}; use peryx_storage::blob::BlobStorage; -use peryx_storage::meta::{MetaStore, NewPolicyDecision}; +use peryx_storage::meta::{ + LegacyMetadataSource, MetaStore, MetadataMigration, MetadataRecord, MetadataRecordSet, NewPolicyDecision, +}; use redb::TableDefinition; use rstest::rstest; use tower::ServiceExt as _; @@ -347,13 +349,14 @@ async fn shadow_contract_reports_malformed_parameters() { #[tokio::test] async fn shadow_candidates_hide_decision_store_errors() { - let repository = "r".repeat(513); - let (_directory, state) = seeded_state(&repository, &repository, IndexAcl::default()); - let authorization = local_reader(&state, &repository).await; + let (_directory, state) = seeded_state("root-pypi", "root/pypi", IndexAcl::default()); + record_decisions(&state); + state.serving.meta.migrate_metadata(&UnreadableDecision).unwrap(); + let authorization = local_reader(&state, "root-pypi").await; let (status, _, body) = request( &state, - &format!("/+shadow/candidates?repository={repository}&project={PROJECT}"), + "/+shadow/candidates?repository=root/pypi&project=acme-pkg", Some(HeaderValue::from_str(&authorization).unwrap()), ) .await; @@ -365,6 +368,33 @@ async fn shadow_candidates_hide_decision_store_errors() { ); } +struct UnreadableDecision; + +impl MetadataMigration for UnreadableDecision { + fn name(&self) -> &'static str { + "unreadable-decision" + } + + fn record_sets(&self) -> &[MetadataRecordSet] { + &[MetadataRecordSet::PolicyDecisionHistory] + } + + fn legacy_sources(&self) -> &[LegacyMetadataSource] { + &[] + } + + fn rewrite( + &self, + _record_set: MetadataRecordSet, + record: &MetadataRecord, + ) -> Result, String> { + Ok(Some(MetadataRecord { + key: record.key.clone(), + value: b"{".to_vec(), + })) + } +} + #[rstest] #[case::missing(None)] #[case::wrong_scheme(Some(HeaderValue::from_static("Bearer token")))]