fix(security): deliver config fixes to a running app the marker calls uninstalled
Found while VERIFYINGa05956c4on archi-dev-box rather than assuming it. GET /lnd-connect-info is correctly 401 with no cookies over the LAN address. But POST /bitcoin-rpc/ on :8334 still answered an unauthenticated caller with a real block height, and still carried `Access-Control-Allow-Origin: *`. The node looked patched. Half of it was not. The rendered /var/lib/archipelago/bitcoin-ui/nginx.conf was dated 2026-06-30 — the pre-fix version — even though the running binary carries the new template. a05956c4's commit message claimed the template "is re-rendered on every reconcile pass, so this ships atomically with the binary". That is false in one specific state, and this node was in it: 1. bitcoin-ui sits in the durable user-uninstalled marker. 2. reconcile returns on that marker BEFORE run_pre_start_hooks, which is what renders the config. 3. The container keeps running regardless, because it is owned by systemd via a Quadlet unit (archy-bitcoin-ui.service, active, restarted 17:25 after the daemon restart) — not by this reconciler. So a container systemd keeps alive, that the orchestrator has stopped reconciling, never receives a config fix shipped inside the binary. An OTA carryinga05956c4would have silently failed to close this on every node in that state, while the LND half closed correctly — the most misleading possible outcome. archy-electrs-ui is in the same state on this node, so it is not a one-app accident. A container that is actually running is a live attack surface whatever a marker says about it. Its security-relevant config is now reconciled even behind the marker, and it is restarted so nginx actually loads it. Deliberately narrow: - Nothing is created, pulled, built, started or resurrected. The "must stay removed" contract only ever gets weaker if a container is ALREADY running, which by definition means it was never removed. - A hook error is swallowed, not propagated: an app the user uninstalled must not be able to fail the reconcile pass for everything after it. - The pre-existing marker test still passes unchanged, which is what proves the removal contract survived. Verified: 11/11 reconcile tests and 9/9 bitcoin_ui tests pass, including a new regression test that pins the whole chain — stale conf in, gate present out, container restarted, nothing created. No node has been touched. The live exposure on archi-dev-box stands until this is deployed and the operator restarts archy-bitcoin-ui. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
f4a323226e
commit
f6b5245b0d
@@ -1954,6 +1954,60 @@ impl ProdContainerOrchestrator {
|
|||||||
let user_uninstalled =
|
let user_uninstalled =
|
||||||
crate::crash_recovery::load_user_uninstalled(&self.data_dir).await;
|
crate::crash_recovery::load_user_uninstalled(&self.data_dir).await;
|
||||||
if user_uninstalled.contains(&app_id) || user_uninstalled.contains(&name) {
|
if user_uninstalled.contains(&app_id) || user_uninstalled.contains(&name) {
|
||||||
|
// The marker says "removed", but the container can still be
|
||||||
|
// RUNNING: a Quadlet unit is owned by systemd, which starts it
|
||||||
|
// on boot entirely independently of this reconciler. On
|
||||||
|
// archi-dev-box (2026-08-02) `bitcoin-ui` sat in this exact
|
||||||
|
// state — marker set, `archy-bitcoin-ui.service` active, port
|
||||||
|
// 8334 published — and so it never received the /bitcoin-rpc/
|
||||||
|
// auth_request gate that a05956c4 shipped INSIDE the binary.
|
||||||
|
// The node looked patched while an unauthenticated caller could
|
||||||
|
// still drive Bitcoin Core RPC through a credential-injecting
|
||||||
|
// proxy.
|
||||||
|
//
|
||||||
|
// A container that is actually running is a live attack surface
|
||||||
|
// whatever a marker says about it, so its security-relevant
|
||||||
|
// config gets reconciled even here. This deliberately does NOT
|
||||||
|
// create, start or resurrect anything — the "must stay removed"
|
||||||
|
// contract is untouched for every path that could.
|
||||||
|
//
|
||||||
|
// Hook failure is swallowed rather than propagated: an app the
|
||||||
|
// user has uninstalled must not be able to fail the reconcile
|
||||||
|
// pass for everything after it.
|
||||||
|
if matches!(
|
||||||
|
self.runtime.get_container_status(&name).await,
|
||||||
|
Ok(status) if matches!(status.state, ContainerState::Running)
|
||||||
|
) {
|
||||||
|
match self.run_pre_start_hooks(&app_id).await {
|
||||||
|
Ok(Some(HookOutcome::Rewritten)) => {
|
||||||
|
tracing::warn!(
|
||||||
|
app_id = %app_id,
|
||||||
|
container = %name,
|
||||||
|
"rewrote config for a user-uninstalled app whose container is \
|
||||||
|
still RUNNING (systemd/Quadlet keeps it alive independently of \
|
||||||
|
reconcile) — restarting so it picks the new config up"
|
||||||
|
);
|
||||||
|
restart_container_scoped_if_pasta(
|
||||||
|
self.runtime.as_ref(),
|
||||||
|
&resolved_manifest,
|
||||||
|
&name,
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.with_context(|| format!("reconcile restart {name}"))?;
|
||||||
|
let _ = self.run_post_start_hooks(&app_id).await;
|
||||||
|
return Ok(ReconcileAction::Started);
|
||||||
|
}
|
||||||
|
Ok(_) => {}
|
||||||
|
Err(e) => {
|
||||||
|
tracing::warn!(
|
||||||
|
app_id = %app_id,
|
||||||
|
error = %e,
|
||||||
|
"config hook failed for a user-uninstalled but running app — \
|
||||||
|
leaving it as-is"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
tracing::debug!(
|
tracing::debug!(
|
||||||
app_id = %app_id,
|
app_id = %app_id,
|
||||||
container = %name,
|
container = %name,
|
||||||
@@ -5976,6 +6030,77 @@ app:
|
|||||||
assert!(!calls.iter().any(|c| c.starts_with("create_container:")));
|
assert!(!calls.iter().any(|c| c.starts_with("create_container:")));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn reconcile_rewrites_security_config_for_a_user_uninstalled_but_running_app() {
|
||||||
|
// Regression, archi-dev-box 2026-08-02: `bitcoin-ui` carried a durable
|
||||||
|
// user-uninstalled marker WHILE systemd/Quadlet kept archy-bitcoin-ui
|
||||||
|
// running and publishing :8334. Reconcile returned on the marker before
|
||||||
|
// reaching the pre-start hook, so the /bitcoin-rpc/ auth_request gate
|
||||||
|
// that a05956c4 shipped inside the binary never reached the rendered
|
||||||
|
// nginx.conf. The node looked patched while an unauthenticated caller
|
||||||
|
// on the LAN or mesh could still drive Bitcoin Core RPC through a
|
||||||
|
// credential-injecting proxy.
|
||||||
|
let rt = Arc::new(MockRuntime::default());
|
||||||
|
let mut orch = orch_with(rt.clone()).await;
|
||||||
|
|
||||||
|
// Own RenderPaths, not the process-wide static one, so a stale
|
||||||
|
// starting state can't race the other bitcoin-ui tests.
|
||||||
|
let dir = tempfile::TempDir::new().expect("test tmpdir");
|
||||||
|
std::fs::write(dir.path().join("bitcoin-rpc-password"), "test-pass\n")
|
||||||
|
.expect("seed password");
|
||||||
|
let rendered_path = dir.path().join("nginx.conf");
|
||||||
|
std::fs::write(&rendered_path, "# pre-fix config: no auth_request gate\n")
|
||||||
|
.expect("seed stale conf");
|
||||||
|
orch.set_bitcoin_ui_paths(bitcoin_ui::RenderPaths {
|
||||||
|
secret_path: dir.path().join("bitcoin-rpc-password"),
|
||||||
|
rendered_path: rendered_path.clone(),
|
||||||
|
});
|
||||||
|
|
||||||
|
orch.insert_manifest_for_test(
|
||||||
|
build_manifest(
|
||||||
|
"bitcoin-ui",
|
||||||
|
"/opt/archy/docker/bitcoin-ui",
|
||||||
|
"archy-bitcoin-ui:local",
|
||||||
|
),
|
||||||
|
PathBuf::from("/opt/archy/apps/bitcoin-ui"),
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
rt.set_state("archy-bitcoin-ui", ContainerState::Running);
|
||||||
|
crate::crash_recovery::mark_user_uninstalled(&orch.data_dir, "bitcoin-ui").await;
|
||||||
|
|
||||||
|
let report = orch.reconcile_existing().await;
|
||||||
|
|
||||||
|
let contents = std::fs::read_to_string(&rendered_path)
|
||||||
|
.expect("nginx.conf must still exist after reconcile");
|
||||||
|
assert!(
|
||||||
|
contents.contains("auth_request /_session_check"),
|
||||||
|
"the session gate must reach a running container even when the app \
|
||||||
|
carries an uninstall marker:\n{contents}"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
!contents.contains("{{BITCOIN_RPC_AUTH}}"),
|
||||||
|
"placeholder was not substituted:\n{contents}"
|
||||||
|
);
|
||||||
|
|
||||||
|
// Rewriting the file is not enough — nginx only loads it on restart.
|
||||||
|
let calls = rt.calls();
|
||||||
|
assert!(
|
||||||
|
calls.iter().any(|c| c == "start_container:archy-bitcoin-ui"),
|
||||||
|
"container must be restarted so nginx picks the new config up: {calls:?}"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
report.actions,
|
||||||
|
vec![("bitcoin-ui".to_string(), ReconcileAction::Started)]
|
||||||
|
);
|
||||||
|
assert!(report.failures.is_empty());
|
||||||
|
|
||||||
|
// The "must stay removed" contract is untouched: nothing is created,
|
||||||
|
// pulled or built for an app the user uninstalled.
|
||||||
|
assert!(!calls.iter().any(|c| c.starts_with("create_container:")));
|
||||||
|
assert!(!calls.iter().any(|c| c.starts_with("pull_image:")));
|
||||||
|
assert!(!calls.iter().any(|c| c.starts_with("build_image:")));
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn reconcile_existing_skips_archival_baseline_apps_on_pruned_hosts() {
|
async fn reconcile_existing_skips_archival_baseline_apps_on_pruned_hosts() {
|
||||||
let rt = Arc::new(MockRuntime::default());
|
let rt = Arc::new(MockRuntime::default());
|
||||||
|
|||||||
Reference in New Issue
Block a user