fix(appgate+container): stop stripping app cookies; create named volumes correctly
Two daemon bugs, one debugging arc (2026-08-05, operator-reported):
1. The app gate removed the ENTIRE Cookie header before proxying. That
broke the data plane of every first-party companion UI behind the gate
(lnd-ui/bitcoin-ui/electrs-ui/fips-ui render their shell, then every
/proxy/* and /lnd-connect-info call 401s — observed as "LND UI
unreachable"), and silently logged users out of every gated app with
its own cookie login (vaultwarden, nextcloud, gitea) on each request.
The gate now strips only its own cookie pairs (session, csrf_token);
a new per-port manifest opt-in `session_passthrough: true` forwards
the node session to first-party UIs whose nginx proxies the daemon's
authenticated endpoints. Undeclared ports never get passthrough.
2. podman_client::create_container sent named volumes to the libpod API
as bind mounts with the bare volume name as source, so creating any
manifest app with a `type: volume` mount failed. On .38 the reconciler
removed indeedhub-postgres/-minio for env drift and then could never
create their replacements, leaving the stack half-missing forever.
Named volumes now ride the spec's `volumes` field ({Name, Dest,
Options}). Also: the reconcile-failure log now prints the full anyhow
chain — `%e` showed only "create_container X" and hid the real error.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
4ace62fad9
commit
ada59acdd5
@@ -35,6 +35,11 @@ pub struct GatedPort {
|
||||
/// Tor-upstream bind — must key on this flag: acting on an undeclared
|
||||
/// port is the v1.7.121 incident class, whatever the action.
|
||||
pub declared: bool,
|
||||
/// Manifest opt-in (`session_passthrough: true` on the port): forward the
|
||||
/// node session cookie to the app on authorised requests. First-party
|
||||
/// companion UIs proxy that cookie to the daemon's authenticated
|
||||
/// endpoints; for every other app the gate strips its own credential.
|
||||
pub session_passthrough: bool,
|
||||
}
|
||||
|
||||
/// A port deliberately left unauthenticated, and the manifest's stated reason.
|
||||
@@ -226,6 +231,7 @@ fn classify_manifest(manifest: &AppManifest, map: &mut PortMap) {
|
||||
app_name: app_name.clone(),
|
||||
icon: icon.clone(),
|
||||
declared: true,
|
||||
session_passthrough: port.session_passthrough,
|
||||
},
|
||||
);
|
||||
}
|
||||
@@ -273,6 +279,9 @@ fn classify_manifest(manifest: &AppManifest, map: &mut PortMap) {
|
||||
app_name: app_name.clone(),
|
||||
icon: icon.clone(),
|
||||
declared: false,
|
||||
// An undeclared port never gets the node session —
|
||||
// passthrough is an explicit manifest opt-in only.
|
||||
session_passthrough: false,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
@@ -136,7 +136,7 @@ impl AppGate {
|
||||
}
|
||||
|
||||
match self.authorize(req.headers(), &app.app_id).await {
|
||||
Authorization::Allow => proxy_to_app(req, app.port).await,
|
||||
Authorization::Allow => proxy_to_app(req, app).await,
|
||||
// 401 rather than a redirect: a redirect to a login page is
|
||||
// indistinguishable from the app itself redirecting, and machine
|
||||
// clients would follow it and parse HTML as if it were their API
|
||||
@@ -375,7 +375,8 @@ fn percent_decode(input: &str) -> String {
|
||||
}
|
||||
|
||||
/// Forward an authorised request to the app on loopback.
|
||||
async fn proxy_to_app(req: Request<Body>, port: u16) -> Response<Body> {
|
||||
async fn proxy_to_app(req: Request<Body>, app: &GatedPort) -> Response<Body> {
|
||||
let port = app.port;
|
||||
let path_and_query = req
|
||||
.uri()
|
||||
.path_and_query()
|
||||
@@ -389,10 +390,16 @@ async fn proxy_to_app(req: Request<Body>, port: u16) -> Response<Body> {
|
||||
|
||||
let (mut parts, body) = req.into_parts();
|
||||
parts.uri = uri;
|
||||
// Strip the gate's own credential before it reaches the app: the app has
|
||||
// no use for the node session and should never be in a position to log,
|
||||
// echo, or forward it.
|
||||
parts.headers.remove(header::COOKIE);
|
||||
// Strip the gate's own credential before it reaches the app — the app
|
||||
// should never be in a position to log, echo, or forward the node
|
||||
// session. But ONLY the gate's cookies: apps run their own cookie logins
|
||||
// (vaultwarden, nextcloud, gitea…), and removing the whole header logged
|
||||
// every one of them out on each request. Companion UIs that proxy the
|
||||
// daemon's authenticated endpoints opt in to keeping the session via
|
||||
// `session_passthrough: true` on their gated port.
|
||||
if !app.session_passthrough {
|
||||
strip_gate_cookies(&mut parts.headers);
|
||||
}
|
||||
parts.headers.remove(header::AUTHORIZATION);
|
||||
|
||||
let client = hyper::Client::new();
|
||||
@@ -402,6 +409,44 @@ async fn proxy_to_app(req: Request<Body>, port: u16) -> Response<Body> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Cookie names owned by the gate/daemon, never the app's to see.
|
||||
const GATE_COOKIE_NAMES: &[&str] = &["session", "csrf_token"];
|
||||
|
||||
/// Remove the gate's own cookie pairs from the Cookie header, preserving the
|
||||
/// app's cookies (its login/session/prefs) untouched. Drops the header
|
||||
/// entirely when nothing remains.
|
||||
fn strip_gate_cookies(headers: &mut hyper::HeaderMap) {
|
||||
let Some(cookie) = headers.get(header::COOKIE) else {
|
||||
return;
|
||||
};
|
||||
let Ok(raw) = cookie.to_str() else {
|
||||
// Not valid UTF-8 — can't safely filter pairs, so fail closed.
|
||||
headers.remove(header::COOKIE);
|
||||
return;
|
||||
};
|
||||
let kept: Vec<&str> = raw
|
||||
.split(';')
|
||||
.map(str::trim)
|
||||
.filter(|pair| {
|
||||
let name = pair.split('=').next().unwrap_or("").trim();
|
||||
!GATE_COOKIE_NAMES.contains(&name)
|
||||
})
|
||||
.filter(|pair| !pair.is_empty())
|
||||
.collect();
|
||||
if kept.is_empty() {
|
||||
headers.remove(header::COOKIE);
|
||||
return;
|
||||
}
|
||||
match header::HeaderValue::from_str(&kept.join("; ")) {
|
||||
Ok(v) => {
|
||||
headers.insert(header::COOKIE, v);
|
||||
}
|
||||
Err(_) => {
|
||||
headers.remove(header::COOKIE);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn set_session_cookie(resp: &mut Response<Body>, token: &str) {
|
||||
// No Domain attribute, so the cookie is host-only. Cookies ignore port,
|
||||
// which is what makes one sign-in cover the dashboard and every app port
|
||||
@@ -793,6 +838,7 @@ mod tests {
|
||||
app_name: "Strfry Relay".to_string(),
|
||||
icon: None,
|
||||
declared: true,
|
||||
session_passthrough: false,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -946,6 +992,44 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// The gate must remove ONLY its own cookie pairs: an app's login cookie
|
||||
/// riding the same header has to survive, or every gated app with its
|
||||
/// own auth (vaultwarden, nextcloud, gitea) is logged out on each
|
||||
/// request — the 2026-08-05 companion-UI/"app logged me out" regression.
|
||||
#[test]
|
||||
fn strip_gate_cookies_keeps_app_cookies() {
|
||||
let mut headers = HeaderMap::new();
|
||||
headers.insert(
|
||||
header::COOKIE,
|
||||
"session=abc; vw_session=keepme; csrf_token=def; theme=dark"
|
||||
.parse()
|
||||
.unwrap(),
|
||||
);
|
||||
strip_gate_cookies(&mut headers);
|
||||
assert_eq!(
|
||||
headers.get(header::COOKIE).unwrap().to_str().unwrap(),
|
||||
"vw_session=keepme; theme=dark"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strip_gate_cookies_drops_header_when_only_gate_cookies() {
|
||||
let mut headers = HeaderMap::new();
|
||||
headers.insert(
|
||||
header::COOKIE,
|
||||
"session=abc; csrf_token=def".parse().unwrap(),
|
||||
);
|
||||
strip_gate_cookies(&mut headers);
|
||||
assert!(headers.get(header::COOKIE).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strip_gate_cookies_no_header_is_a_noop() {
|
||||
let mut headers = HeaderMap::new();
|
||||
strip_gate_cookies(&mut headers);
|
||||
assert!(headers.get(header::COOKIE).is_none());
|
||||
}
|
||||
|
||||
/// The load-bearing 2FA property: a session still awaiting its TOTP code
|
||||
/// fails `validate()`, so the gate rejects it without knowing anything
|
||||
/// about second factors.
|
||||
|
||||
@@ -1754,7 +1754,10 @@ impl ProdContainerOrchestrator {
|
||||
}
|
||||
Ok(action) => report.record(&app_id, action),
|
||||
Err(e) => {
|
||||
tracing::error!(app_id = %app_id, error = %e, "reconcile failed");
|
||||
// `{:#}` prints the whole anyhow chain — `%e` alone showed
|
||||
// only the outer context ("create_container X") and hid
|
||||
// the actual libpod error for days.
|
||||
tracing::error!(app_id = %app_id, error = %format!("{e:#}"), "reconcile failed");
|
||||
report.failures.push((app_id, e.to_string()));
|
||||
}
|
||||
}
|
||||
@@ -4440,6 +4443,7 @@ mod tests {
|
||||
bind: String::new(),
|
||||
auth: None,
|
||||
auth_rationale: None,
|
||||
session_passthrough: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -599,6 +599,19 @@ pub struct PortMapping {
|
||||
/// means the author expected an exemption they did not get.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub auth_rationale: Option<String>,
|
||||
/// Forward the node session cookie to the app on authorised requests.
|
||||
///
|
||||
/// The gate normally strips its own credential before proxying — an app
|
||||
/// must never be in a position to log or replay the node session. The
|
||||
/// first-party companion UIs (lnd-ui, bitcoin-ui, electrs-ui, fips-ui)
|
||||
/// are the exception their design requires: their nginx forwards the
|
||||
/// browser's session cookie to the daemon's authenticated endpoints
|
||||
/// (`/proxy/lnd/*`, `/rpc/v1`, `/lnd-connect-info`), so stripping it
|
||||
/// breaks every data call behind the gate with a 401 while the page
|
||||
/// shell still renders (observed as "LND UI unreachable", 2026-08-05).
|
||||
/// Only meaningful on a `auth: gated` port.
|
||||
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
|
||||
pub session_passthrough: bool,
|
||||
}
|
||||
|
||||
impl PortMapping {
|
||||
@@ -626,6 +639,7 @@ impl From<(u16, u16)> for PortMapping {
|
||||
bind: String::new(),
|
||||
auth: None,
|
||||
auth_rationale: None,
|
||||
session_passthrough: false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -366,6 +366,7 @@ impl PodmanClient {
|
||||
}
|
||||
|
||||
let mut mounts = Vec::new();
|
||||
let mut named_volumes = Vec::new();
|
||||
for volume in &manifest.app.volumes {
|
||||
if volume.volume_type == "tmpfs" {
|
||||
let options: Vec<String> = volume
|
||||
@@ -382,6 +383,19 @@ impl PodmanClient {
|
||||
"type": "tmpfs",
|
||||
"options": options,
|
||||
}));
|
||||
} else if volume.volume_type == "volume" {
|
||||
// Named podman volume. The libpod create spec carries these in
|
||||
// the separate `volumes` field ({Name, Dest, Options}), NOT in
|
||||
// `mounts`: sending one as a bind mount makes the API treat
|
||||
// the bare volume name as a host path and the create fails —
|
||||
// which left indeedhub-postgres/-minio permanently absent on
|
||||
// legacy-path nodes (the reconciler removed the old container
|
||||
// for drift, then could never create its replacement).
|
||||
named_volumes.push(serde_json::json!({
|
||||
"Name": volume.source,
|
||||
"Dest": volume.target,
|
||||
"Options": volume.options,
|
||||
}));
|
||||
} else {
|
||||
mounts.push(serde_json::json!({
|
||||
"destination": volume.target,
|
||||
@@ -464,6 +478,7 @@ impl PodmanClient {
|
||||
"image": image_ref,
|
||||
"portmappings": port_mappings,
|
||||
"mounts": mounts,
|
||||
"volumes": named_volumes,
|
||||
"env": env_map,
|
||||
"secret_env": secret_env_map,
|
||||
"labels": labels_map,
|
||||
|
||||
Reference in New Issue
Block a user