mirror of
https://github.com/Such-Software/smirk-backend-core.git
synced 2026-10-17 07:21:32 +02:00
fix(config): runtime-safe PUT no longer drags pending restart-required changes live
A runtime-safe config PUT rebuilt the effective config from config_base + the FULL persisted overlay and stored it, which live-applied any restart-required section persisted by an earlier PUT (features/relay/ premium) while the boot-built clients still ran the old value — and cleared restart_pending in the process. The exact desync RuntimeClass exists to prevent. Now the hot-swap applies ONLY the (all-runtime-safe) patch onto the currently-running config, and restart_pending is computed honestly via a shared helper (GET and PUT agree). Also reclassify registration. require_payment as restart-required: it gates the boot-built payment client (None unless require_payment||premium at boot), so hot-enabling it would 500 every registration until restart. Regression tests for both.
This commit is contained in:
+44
-13
@@ -760,6 +760,26 @@ pub struct AdminConfigResponse {
|
||||
restart_pending: bool,
|
||||
}
|
||||
|
||||
/// `restart_pending`: the persisted overlay, applied to the boot base, differs from
|
||||
/// the currently-RUNNING effective config in at least one field — i.e. a
|
||||
/// restart-required change was saved but has not been applied by a restart yet.
|
||||
/// Shared by GET and PUT so both report the identical signal. A persisted overlay
|
||||
/// that no longer validates is treated as not-pending (the running config is the
|
||||
/// authority; GET/PUT surface the validation error separately).
|
||||
fn overlay_restart_pending(
|
||||
config_base: &crate::config::Config,
|
||||
persisted: &crate::config_overlay::SettingsOverlay,
|
||||
running: &crate::config::Config,
|
||||
) -> bool {
|
||||
match config_base.apply_overlay(persisted) {
|
||||
Ok(p) => {
|
||||
serde_json::to_value(p.editable_overlay()).ok()
|
||||
!= serde_json::to_value(running.editable_overlay()).ok()
|
||||
}
|
||||
Err(_) => false,
|
||||
}
|
||||
}
|
||||
|
||||
#[instrument(skip(state, headers))]
|
||||
pub async fn admin_config_get(
|
||||
State(state): State<Arc<AppState>>,
|
||||
@@ -770,17 +790,11 @@ pub async fn admin_config_get(
|
||||
|
||||
let overlay = state.db.load_settings_overlay(&secret).await?;
|
||||
let versions = state.db.settings_versions().await?;
|
||||
let effective = state.cfg().editable_overlay();
|
||||
let running = state.cfg();
|
||||
let effective = running.editable_overlay();
|
||||
|
||||
// restart_pending: the persisted overlay's effective view differs from the RUNNING
|
||||
// effective view (a restart-required field was saved but not yet applied).
|
||||
let restart_pending = match state.config_base.apply_overlay(&overlay) {
|
||||
Ok(persisted) => {
|
||||
serde_json::to_value(persisted.editable_overlay()).ok()
|
||||
!= serde_json::to_value(&effective).ok()
|
||||
}
|
||||
Err(_) => false,
|
||||
};
|
||||
// A restart-required field was saved but not yet applied (see the shared helper).
|
||||
let restart_pending = overlay_restart_pending(&state.config_base, &overlay, &running);
|
||||
|
||||
let runtime_class = crate::config_overlay::EDITABLE_FIELDS
|
||||
.iter()
|
||||
@@ -841,7 +855,9 @@ pub async fn admin_config_put(
|
||||
let merged: crate::config_overlay::SettingsOverlay =
|
||||
serde_json::from_value(merged_json.clone())
|
||||
.map_err(|e| AppError::ValidationError(format!("bad settings: {e}")))?;
|
||||
let candidate = state.config_base.apply_overlay(&merged)?; // ValidationError -> 400
|
||||
// Validate the FULL merged overlay with the same rules boot uses; a bad edit is a
|
||||
// 400 and nothing persists. (The hot-swap below re-validates the narrower change.)
|
||||
state.config_base.apply_overlay(&merged)?; // ValidationError -> 400
|
||||
|
||||
// Classify the CHANGED fields: which sections changed + does any need a restart?
|
||||
let mut any_restart = false;
|
||||
@@ -921,10 +937,25 @@ pub async fn admin_config_put(
|
||||
restart_pending: true,
|
||||
}))
|
||||
} else {
|
||||
state.config.store(std::sync::Arc::new(candidate));
|
||||
// Hot-swap ONLY the runtime-safe patch onto the currently-RUNNING config.
|
||||
// Rebuilding from the full persisted overlay (the old behavior) would drag a
|
||||
// restart-required section persisted by an EARLIER PUT into the live config
|
||||
// while the boot-built clients still run the old value — the exact desync
|
||||
// RuntimeClass exists to prevent. Every field in this patch is runtime-safe
|
||||
// (any_restart == false), so applying it to the running config is sound and
|
||||
// re-validates the change.
|
||||
let running = state.cfg();
|
||||
let hot = running
|
||||
.apply_overlay(&req.patch)
|
||||
.map_err(|e| AppError::Internal(format!("hot-swap re-validate failed: {e}")))?;
|
||||
drop(running);
|
||||
// A restart-required change from an EARLIER PUT may still be pending; report it
|
||||
// honestly rather than clearing the signal just because THIS edit was live.
|
||||
let restart_pending = overlay_restart_pending(&state.config_base, &merged, &hot);
|
||||
state.config.store(std::sync::Arc::new(hot));
|
||||
Ok(Json(PutConfigResponse {
|
||||
applied: "runtime".into(),
|
||||
restart_pending: false,
|
||||
restart_pending,
|
||||
}))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1623,6 +1623,43 @@ mod tests {
|
||||
assert_eq!(applied.registration.gate_mode, GateMode::Any);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn runtime_safe_put_does_not_drag_pending_restart_required_change_live() {
|
||||
// Reproduces the hot-swap desync: an earlier PUT persists a restart-required
|
||||
// change (chain toggle) that is NOT yet applied to the running config; a later
|
||||
// runtime-safe PUT must hot-swap ONLY its own change onto the RUNNING config,
|
||||
// never rebuild from the full persisted overlay (which would drag the pending
|
||||
// restart-required change live while boot-built clients still run the old one).
|
||||
use crate::config_overlay::SettingsOverlay;
|
||||
let base = valid();
|
||||
assert!(!base.features.chains.btc, "precondition: btc off at boot");
|
||||
let running = base.clone(); // boot state; the restart-required change is NOT here
|
||||
|
||||
// The FIX: hot-swap applies only the runtime-safe patch to the running config.
|
||||
let safe_patch: SettingsOverlay =
|
||||
serde_json::from_str(r#"{"landing":{"title":"hello"}}"#).unwrap();
|
||||
let hot = running.apply_overlay(&safe_patch).unwrap();
|
||||
assert!(
|
||||
!hot.features.chains.btc,
|
||||
"restart-required chain toggle must NOT leak into the runtime-safe hot-swap"
|
||||
);
|
||||
assert_eq!(
|
||||
hot.landing.title.as_deref(),
|
||||
Some("hello"),
|
||||
"runtime-safe change applies"
|
||||
);
|
||||
|
||||
// Regression guard: the OLD path (base + full persisted overlay) WOULD drag it.
|
||||
let buggy_merged: SettingsOverlay =
|
||||
serde_json::from_str(r#"{"features":{"chain_btc":true},"landing":{"title":"hello"}}"#)
|
||||
.unwrap();
|
||||
let buggy = base.apply_overlay(&buggy_merged).unwrap();
|
||||
assert!(
|
||||
buggy.features.chains.btc,
|
||||
"sanity: the old rebuild-from-full-overlay path drags the pending change"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn overlay_enabling_pow_without_hmac_key_is_rejected() {
|
||||
// Fail-closed: turning the PoW gate on via the console with no HMAC key
|
||||
|
||||
+25
-8
@@ -73,11 +73,13 @@ pub struct ConsoleOverlay {
|
||||
pub restart_apply_mode: Option<String>,
|
||||
}
|
||||
|
||||
/// Registration gating POLICY knobs (read per-request → runtime-safe). The
|
||||
/// processor WIRING (provider, provider_url, store_id) and the SECRET api_key stay
|
||||
/// in env by design: they pair with the secret and are set-once infra, so they are
|
||||
/// deliberately absent here. Toggling `require_payment` on with incomplete wiring
|
||||
/// is rejected by `Config::validate` at PUT time (fail-closed).
|
||||
/// Registration gating POLICY knobs. Most are read per-request → runtime-safe; the
|
||||
/// exception is `require_payment`, which gates the boot-built payment client and is
|
||||
/// therefore restart-required (see `runtime_class`). The processor WIRING (provider,
|
||||
/// provider_url, store_id) and the SECRET api_key stay in env by design: they pair
|
||||
/// with the secret and are set-once infra, so they are deliberately absent here.
|
||||
/// Toggling `require_payment` on with incomplete wiring is rejected by
|
||||
/// `Config::validate` at PUT time (fail-closed).
|
||||
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
|
||||
#[serde(default)]
|
||||
pub struct RegistrationOverlay {
|
||||
@@ -174,9 +176,14 @@ pub fn runtime_class(section: &str, field: &str) -> RuntimeClass {
|
||||
("retention", _) => RuntimeSafe,
|
||||
("restore", _) => RuntimeSafe,
|
||||
("console", _) => RuntimeSafe,
|
||||
// Registration gates + PoW are read via `state.cfg()` on the registration
|
||||
// path (auth.rs / capabilities.rs), so an overlay change lands on the next
|
||||
// request — hot-swappable.
|
||||
// require_payment gates the BOOT-built payment client (main.rs: the provider is
|
||||
// None unless require_payment || premium.enabled at boot), so hot-enabling it
|
||||
// would 500 every registration until restart — restart-required, like premium.
|
||||
("registration", "require_payment") => RestartRequired,
|
||||
// The other registration gates + PoW are read via `state.cfg()` on the
|
||||
// registration path (auth.rs / capabilities.rs), so an overlay change lands on
|
||||
// the next request — hot-swappable. (payment_amount/currency/etc. are read per
|
||||
// invoice; they only matter once the boot-built client exists.)
|
||||
("registration", _) => RuntimeSafe,
|
||||
("pow", _) => RuntimeSafe,
|
||||
// Feature flags / chain enablement / relay / premium all gate boot-built
|
||||
@@ -581,6 +588,16 @@ mod tests {
|
||||
runtime_class("console", "restart_apply_mode"),
|
||||
RuntimeClass::RuntimeSafe
|
||||
);
|
||||
// require_payment gates the boot-built payment client -> restart-required,
|
||||
// while its sibling registration gates hot-swap.
|
||||
assert_eq!(
|
||||
runtime_class("registration", "require_payment"),
|
||||
RuntimeClass::RestartRequired
|
||||
);
|
||||
assert_eq!(
|
||||
runtime_class("registration", "require_invite"),
|
||||
RuntimeClass::RuntimeSafe
|
||||
);
|
||||
assert_eq!(
|
||||
runtime_class("mystery", "field"),
|
||||
RuntimeClass::RestartRequired
|
||||
|
||||
Reference in New Issue
Block a user