diff --git a/apps/gitea/manifest.yml b/apps/gitea/manifest.yml index f5486191..ca2e957e 100644 --- a/apps/gitea/manifest.yml +++ b/apps/gitea/manifest.yml @@ -106,10 +106,3 @@ app: - Issue tracking and pull requests - CI/CD via Gitea Actions - Lightweight SQLite deployment - - nginx_proxy: - listen: 3000 - proxy_pass: http://127.0.0.1:3001 - extra_headers: - - proxy_hide_header X-Frame-Options - - proxy_hide_header Content-Security-Policy diff --git a/apps/portainer/manifest.yml b/apps/portainer/manifest.yml index 32cb3e24..1bec2d7c 100644 --- a/apps/portainer/manifest.yml +++ b/apps/portainer/manifest.yml @@ -14,8 +14,16 @@ app: container: image: source.archipelago-foundation.org/lfg2025/portainer:2.45.0 pull_policy: if-not-present + # Portainer fetches Git sources and images from services on this same node. + # Rootless pasta copies the host LAN address into its namespace, so a LAN + # URL points back at Portainer itself. Give it a private address with the + # supported rootless slirp backend; public app URLs still traverse the gate. + network: slirp4netns data_uid: "1000:1000" + # Snapshot state before an upgrade recreates this app with new networking. + backup_on_network_change: true + dependencies: - storage: 1Gi diff --git a/core/archipelago/src/api/rpc/package/install.rs b/core/archipelago/src/api/rpc/package/install.rs index a81208d3..a2bfd47e 100644 --- a/core/archipelago/src/api/rpc/package/install.rs +++ b/core/archipelago/src/api/rpc/package/install.rs @@ -1699,32 +1699,10 @@ autopilot.active=false\n", patch_indeedhub_nostr_provider().await; } - // Gitea: keep it on its native host port (3001). The UI opens Gitea - // in a new tab on that direct port so absolute asset URLs must be - // rooted at the host port rather than Archipelago's /app/gitea/ path. - if package_id == "gitea" { - let _ = tokio::fs::remove_file("/etc/nginx/conf.d/gitea-iframe.conf").await; - - // Set ROOT_URL to the direct launch route so links/assets stay - // anchored under the same origin Gitea is launched from. - let host_ip = &self.config.host_ip; - let _ = tokio::process::Command::new("podman") - .args(["exec", "gitea", "sh", "-c", - &format!("grep -q ROOT_URL /data/gitea/conf/app.ini && sed -i 's|ROOT_URL.*|ROOT_URL = http://{}:3001/|' /data/gitea/conf/app.ini || true", host_ip)]) - .output() - .await; - // Also ensure X_FRAME_OPTIONS is empty so Gitea doesn't send the header - let _ = tokio::process::Command::new("podman") - .args(["exec", "gitea", "sh", "-c", - "grep -q X_FRAME_OPTIONS /data/gitea/conf/app.ini && sed -i 's|X_FRAME_OPTIONS.*|X_FRAME_OPTIONS =|' /data/gitea/conf/app.ini || sed -i '/^\\[security\\]/a X_FRAME_OPTIONS =' /data/gitea/conf/app.ini"]) - .output() - .await; - - info!( - "Gitea: ROOT_URL set to http://{}:3001/, X_FRAME_OPTIONS cleared", - host_ip - ); - } + // Gitea owns its public URL and security settings in app.ini, including + // values chosen in its first-run setup. Do not rewrite operator values + // or claim success from best-effort grep/sed commands. The app gate + // fronts its declared HTTP port and handles frame headers separately. if package_id == "nextcloud" { let host_ip = &self.config.host_ip; diff --git a/core/archipelago/src/container/migration_backup.rs b/core/archipelago/src/container/migration_backup.rs new file mode 100644 index 00000000..b27b467a --- /dev/null +++ b/core/archipelago/src/container/migration_backup.rs @@ -0,0 +1,251 @@ +//! Consistent, private snapshots for declaratively opted-in network migrations. +use anyhow::{bail, Context, Result}; +use archipelago_container::AppManifest; +use std::os::unix::fs::PermissionsExt; +use std::path::{Path, PathBuf}; + +pub fn enabled(manifest: &AppManifest) -> Result { + match manifest.app.extensions.get("backup_on_network_change") { + None => Ok(false), + Some(value) => value + .as_bool() + .context("backup_on_network_change must be boolean"), + } +} + +fn relative_sources(manifest: &AppManifest, data_dir: &Path) -> Result> { + let mut sources = Vec::new(); + for volume in &manifest.app.volumes { + if volume.options.iter().any(|v| v == "ro") || volume.volume_type == "tmpfs" { + continue; + } + // A runtime socket is a connection, not application state. + if volume.source == "/run/user/1000/podman/podman.sock" { + continue; + } + if volume.volume_type != "bind" { + bail!("network migration backup requires bind-mounted persistent state"); + } + let path = Path::new(&volume.source); + let relative = path + .strip_prefix(data_dir) + .context("network migration state must be inside the node data directory")?; + if relative.as_os_str().is_empty() + || relative + .components() + .any(|c| !matches!(c, std::path::Component::Normal(_))) + { + bail!("invalid network migration state path"); + } + sources.push(relative.to_path_buf()); + } + sources.sort(); + sources.dedup(); + let mut roots: Vec = Vec::new(); + for source in sources { + if !roots.iter().any(|root| source.starts_with(root)) { + roots.push(source); + } + } + if roots.is_empty() { + bail!("network migration backup has no persistent state mounts"); + } + Ok(roots) +} + +/// Caller must gracefully stop the app before this function, and resume the old +/// service if it fails. No source files are changed or deleted by this operation. +pub async fn snapshot( + manifest: &AppManifest, + data_dir: &Path, + previous_unit: Option<&[u8]>, +) -> Result { + let mut command = tokio::process::Command::new("podman"); + command.args(["unshare", "tar"]); + snapshot_with_command(manifest, data_dir, previous_unit, command).await +} + +async fn snapshot_with_command( + manifest: &AppManifest, + data_dir: &Path, + previous_unit: Option<&[u8]>, + mut command: tokio::process::Command, +) -> Result { + let sources = relative_sources(manifest, data_dir)?; + let canonical_root = tokio::fs::canonicalize(data_dir).await?; + for source in &sources { + let path = data_dir.join(source); + if tokio::fs::symlink_metadata(&path) + .await? + .file_type() + .is_symlink() + { + bail!("network migration state mount is a symlink; explicit backup required"); + } + let canonical = tokio::fs::canonicalize(&path).await?; + if !canonical.starts_with(&canonical_root) { + bail!("network migration state path resolves outside node data directory"); + } + } + let root = data_dir.join("migration-backups"); + tokio::fs::create_dir_all(&root).await?; + tokio::fs::set_permissions(&root, std::fs::Permissions::from_mode(0o700)).await?; + let dir = root.join(uuid::Uuid::new_v4().to_string()); + tokio::fs::create_dir(&dir).await?; + tokio::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700)).await?; + if let Some(unit) = previous_unit { + let path = dir.join("previous.container"); + tokio::fs::write(&path, unit).await?; + tokio::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)).await?; + tokio::fs::File::open(&path).await?.sync_all().await?; + } + let partial = dir.join("state.tar.partial"); + let archive = dir.join("state.tar"); + let output = command + .args([ + "--create", + "--numeric-owner", + "--acls", + "--xattrs", + "--file", + ]) + .arg(&partial) + .arg("--directory") + .arg(data_dir) + .arg("--") + .args(&sources) + .output() + .await + .context("start rootless migration snapshot")?; + if !output.status.success() { + // No tar stderr in public logs: it can contain private filenames. + let _ = tokio::fs::remove_file(&partial).await; + bail!("persistent-state snapshot failed; original state was left intact"); + } + tokio::fs::set_permissions(&partial, std::fs::Permissions::from_mode(0o600)).await?; + tokio::fs::File::open(&partial).await?.sync_all().await?; + tokio::fs::rename(&partial, &archive).await?; + let metadata = serde_json::json!({"app": manifest.app.id, "version": manifest.app.version, + "network": manifest.app.container.network, "sources": sources}); + tokio::fs::write( + dir.join("metadata.json"), + serde_json::to_vec_pretty(&metadata)?, + ) + .await?; + tokio::fs::File::open(&dir).await?.sync_all().await?; + Ok(archive) +} + +#[cfg(test)] +mod tests { + use super::*; + fn portainer() -> AppManifest { + AppManifest::parse(include_str!("../../../../apps/portainer/manifest.yml")).unwrap() + } + #[tokio::test] + async fn stopped_state_archive_round_trips_database_compose_and_old_unit() { + let dir = tempfile::tempdir().unwrap(); + let state = dir.path().join("portainer"); + tokio::fs::create_dir_all(state.join("compose")) + .await + .unwrap(); + tokio::fs::write(state.join("portainer.db"), b"fixture database") + .await + .unwrap(); + tokio::fs::write(state.join("compose/stack.yml"), b"services: {}\n") + .await + .unwrap(); + let mut m = portainer(); + m.app.volumes[0].source = state.display().to_string(); + m.app.volumes[1].source = state.join("compose").display().to_string(); + let archive = snapshot_with_command( + &m, + dir.path(), + Some(b"old unit"), + tokio::process::Command::new("tar"), + ) + .await + .unwrap(); + assert_eq!( + std::fs::metadata(&archive).unwrap().permissions().mode() & 0o777, + 0o600 + ); + assert_eq!( + tokio::fs::read(archive.parent().unwrap().join("previous.container")) + .await + .unwrap(), + b"old unit" + ); + let restored = tempfile::tempdir().unwrap(); + assert!(tokio::process::Command::new("tar") + .arg("-xf") + .arg(archive) + .arg("-C") + .arg(restored.path()) + .status() + .await + .unwrap() + .success()); + assert_eq!( + tokio::fs::read(restored.path().join("portainer/portainer.db")) + .await + .unwrap(), + b"fixture database" + ); + assert_eq!( + tokio::fs::read(restored.path().join("portainer/compose/stack.yml")) + .await + .unwrap(), + b"services: {}\n" + ); + assert_eq!( + tokio::fs::read(state.join("portainer.db")).await.unwrap(), + b"fixture database" + ); + } + + #[tokio::test] + async fn failed_snapshot_never_publishes_archive_or_changes_original_state() { + let dir = tempfile::tempdir().unwrap(); + let state = dir.path().join("portainer"); + tokio::fs::create_dir_all(state.join("compose")) + .await + .unwrap(); + tokio::fs::write(state.join("portainer.db"), b"unchanged") + .await + .unwrap(); + let mut m = portainer(); + m.app.volumes[0].source = state.display().to_string(); + m.app.volumes[1].source = state.join("compose").display().to_string(); + assert!( + snapshot_with_command(&m, dir.path(), None, tokio::process::Command::new("false")) + .await + .is_err() + ); + assert_eq!( + tokio::fs::read(state.join("portainer.db")).await.unwrap(), + b"unchanged" + ); + for entry in std::fs::read_dir(dir.path().join("migration-backups")).unwrap() { + assert!(!entry.unwrap().path().join("state.tar").exists()); + } + } + + #[test] + fn backup_covers_all_portainer_state_once_and_excludes_runtime_socket() { + let m = portainer(); + assert!(enabled(&m).unwrap()); + assert_eq!( + relative_sources(&m, Path::new("/var/lib/archipelago")).unwrap(), + vec![PathBuf::from("portainer")] + ); + } + #[test] + fn backup_refuses_unknown_state_locations_instead_of_silently_omitting_them() { + let mut m = portainer(); + m.app.volumes[0].source = "/other/operator/state".into(); + assert!(relative_sources(&m, Path::new("/var/lib/archipelago")).is_err()); + m.app.volumes[0].source = "/var/lib/archipelago/../secret".into(); + assert!(relative_sources(&m, Path::new("/var/lib/archipelago")).is_err()); + } +} diff --git a/core/archipelago/src/container/mod.rs b/core/archipelago/src/container/mod.rs index 4d19e4a7..3455a0e2 100644 --- a/core/archipelago/src/container/mod.rs +++ b/core/archipelago/src/container/mod.rs @@ -12,6 +12,7 @@ pub mod hooks; pub mod image_policy; pub mod image_versions; pub mod lnd; +pub mod migration_backup; pub mod prod_orchestrator; pub mod quadlet; pub mod registry; diff --git a/core/archipelago/src/container/prod_orchestrator.rs b/core/archipelago/src/container/prod_orchestrator.rs index d764d47d..949b1989 100644 --- a/core/archipelago/src/container/prod_orchestrator.rs +++ b/core/archipelago/src/container/prod_orchestrator.rs @@ -91,6 +91,14 @@ fn is_builtin_network_mode(network: &str) -> bool { ) } +// Only an explicitly selected rootless mode establishes drift. An omitted +// network delegates to Podman and must not recreate unrelated installed apps. +fn rootless_network_mode_drifted(expected: Option<&str>, actual: &str) -> bool { + matches!(expected, Some("slirp4netns" | "pasta")) + && !actual.trim().is_empty() + && actual.trim().split(':').next() != expected +} + fn uses_pasta_network(manifest: &AppManifest) -> bool { manifest.app.container.network.as_deref() == Some("pasta") } @@ -2499,6 +2507,7 @@ impl ProdContainerOrchestrator { return Ok(ReconcileAction::NoOp); } tracing::info!(app_id = %app_id, container = %name, "container env drift detected — recreating"); + self.backup_network_change(&name, &resolved_manifest).await?; let _ = self.runtime.stop_container(&name).await; let _ = self.runtime.remove_container(&name).await; self.install_fresh(lm).await?; @@ -2555,6 +2564,7 @@ impl ProdContainerOrchestrator { .await { tracing::info!(app_id = %app_id, container = %name, "stopped container env/port drift detected — recreating"); + self.backup_network_change(&name, &resolved_manifest).await?; let _ = self.runtime.remove_container(&name).await; self.install_fresh(lm).await?; return Ok(ReconcileAction::Installed); @@ -3080,13 +3090,9 @@ impl ProdContainerOrchestrator { /// app is a companion (companion.rs owns those units), or when no /// unit file exists yet (install_via_quadlet handles first-write). /// - /// We DON'T restart the .service when content changes — running - /// containers keep their current config until an operator-initiated - /// restart picks up the new file. That's the right tradeoff: file - /// updates are cheap and non-destructive; service restarts are - /// destructive (the SIGKILL cascade we're trying to eliminate). - /// systemctl --user daemon-reload runs only when content actually - /// changed, so steady-state reconcile ticks pay just one fs read. + /// Ordinary metadata changes wait for an operator restart. Runtime-affecting + /// changes restart the service and retain a durable pending marker until + /// that succeeds, including across daemon restarts and failed reloads. async fn sync_quadlet_unit(&self, lm: &LoadedManifest, name: &str) -> Result<()> { // Companions: same reasoning as migrate_to_quadlet_if_needed — // companion.rs renders these units with a different shape, syncing @@ -3106,7 +3112,7 @@ impl ProdContainerOrchestrator { } let old_body = tokio::fs::read_to_string(&unit_path) .await - .unwrap_or_default(); + .with_context(|| format!("read existing quadlet for {name}"))?; let restart_required = quadlet::contains_stale_health_gate(&old_body); let mut resolved = lm.manifest.clone(); @@ -3122,49 +3128,47 @@ impl ProdContainerOrchestrator { quadlet::network_aliases_changed(&old_body, &new_body); let restart_for_exec_change = quadlet::exec_changed(&old_body, &new_body); let restart_for_health_change = quadlet::health_cmd_changed(&old_body, &new_body); + let needs_restart = restart_required + || restart_for_port_change + || restart_for_network_alias_change + || restart_for_exec_change + || restart_for_health_change; + // Record the obligation BEFORE replacing the unit. A failed reload or + // restart must not become a no-op on the next tick just because the + // generated file already matches the manifest. + let pending = quadlet::RestartObligation::prepare(&unit_path, needs_restart).await?; + if pending.is_pending() { + self.ensure_resolved_source_available(lm).await?; + } + if restart_for_network_alias_change { + self.backup_network_change(name, &resolved).await?; + } let changed = quadlet::write_if_changed(&unit, &unit_dir) .await .with_context(|| format!("drift-sync quadlet unit for {name}"))?; - if changed { + if changed || pending.is_pending() { quadlet::daemon_reload_user() .await .context("systemctl --user daemon-reload after drift-syncing quadlet unit")?; - tracing::info!( - app_id = %lm.manifest.app.id, - container = %name, - "Quadlet unit drift-synced — file rewritten, .service NOT restarted (operator restart picks up new config)" - ); } - if changed - && (restart_required - || restart_for_port_change - || restart_for_network_alias_change - || restart_for_exec_change - || restart_for_health_change) - { - self.ensure_resolved_source_available(lm).await?; + if pending.is_pending() { let service = unit.service_name(); - let reason = if restart_required { - "stale health gate" - } else if restart_for_port_change { - "port binding drift" - } else if restart_for_network_alias_change { - "network alias drift" - } else if restart_for_health_change { - "health command drift" - } else { - "exec drift" - }; tracing::info!( app_id = %lm.manifest.app.id, container = %name, service = %service, - reason = reason, - "Quadlet unit rewrite requires service restart" + "Applying pending Quadlet runtime change" ); quadlet::restart_service(&service) .await .with_context(|| format!("restart drifted quadlet service {service}"))?; + pending.complete().await?; + } else if changed { + tracing::info!( + app_id = %lm.manifest.app.id, + container = %name, + "Quadlet metadata updated; operator restart will apply it" + ); } Ok(()) } @@ -3866,6 +3870,61 @@ impl ProdContainerOrchestrator { Ok(()) } + async fn backup_network_change(&self, name: &str, manifest: &AppManifest) -> Result<()> { + if !crate::container::migration_backup::enabled(manifest)? { + return Ok(()); + } + // Only back up an actual network migration, not ordinary env drift. + let output = tokio::process::Command::new("podman") + .args(["inspect", name, "--format", "{{.HostConfig.NetworkMode}}"]) + .output().await.context("inspect network before migration backup")?; + let present = if output.status.success() { + if !rootless_network_mode_drifted(manifest.app.container.network.as_deref(), &String::from_utf8_lossy(&output.stdout)) { + return Ok(()); + } + true + } else { + // A crash after gracefully stopping a --rm Quadlet container can + // leave only its data and old unit. Prove absence before snapshotting + // stopped state; an inspect/Podman failure is not proof of absence. + let exists = tokio::process::Command::new("podman") + .args(["container", "exists", name]).status().await?; + if exists.code() != Some(1) { + anyhow::bail!("cannot verify existing container before network migration backup"); + } + false + }; + let service = format!("{name}.service"); + let managed = quadlet::unit_exists(name).await; + let previous_unit = if managed { + Some(tokio::fs::read(quadlet::unit_dir().await?.join(format!("{name}.container"))).await?) + } else { + None + }; + if managed { + quadlet::stop_service(&service).await?; + } else if present { + self.runtime.stop_container(name).await?; + } + match crate::container::migration_backup::snapshot(manifest, &self.data_dir, previous_unit.as_deref()).await { + Ok(archive) => { + tracing::info!(container = %name, backup = %archive.display(), "Persistent state saved before network migration"); + Ok(()) + } + Err(error) => { + // The unit has not been rewritten yet. Restore its previous + // service on backup failure and report the migration failure. + let restored = if managed { + quadlet::enable_now(&service).await + } else { + self.runtime.start_container(name).await + }; + restored.context("restore original app after failed migration snapshot")?; + Err(error) + } + } + } + async fn container_env_drifted(&self, name: &str, manifest: &AppManifest) -> bool { if cfg!(test) { return false; @@ -3875,6 +3934,23 @@ impl ProdContainerOrchestrator { return true; } + // Quadlet handles declarative Network= drift above. Legacy rootless + // Podman containers need the same convergence when no unit owns them. + if matches!(manifest.app.container.network.as_deref(), Some("slirp4netns" | "pasta")) { + if let Ok(output) = tokio::process::Command::new("podman") + .args(["inspect", name, "--format", "{{.HostConfig.NetworkMode}}"]) + .output() + .await + { + if output.status.success() && rootless_network_mode_drifted( + manifest.app.container.network.as_deref(), + &String::from_utf8_lossy(&output.stdout), + ) { + return true; + } + } + } + let inspect = tokio::process::Command::new("podman") .args([ "inspect", @@ -4917,6 +4993,17 @@ mod tests { /// recovered when its siblings have live containers (the stack is /// installed), and left alone when the whole stack is gone or the app /// is not a stack member at all. + #[test] + fn explicit_rootless_network_change_converges_without_guessing_defaults() { + assert!(rootless_network_mode_drifted(Some("slirp4netns"), "pasta")); + assert!(rootless_network_mode_drifted(Some("slirp4netns"), "bridge")); + assert!(!rootless_network_mode_drifted(Some("slirp4netns"), "slirp4netns")); + assert!(!rootless_network_mode_drifted(Some("slirp4netns"), "slirp4netns:allow_host_loopback=true")); + assert!(!rootless_network_mode_drifted(None, "pasta")); + assert!(!rootless_network_mode_drifted(Some("slirp4netns"), "")); + assert!(!rootless_network_mode_drifted(Some("archy-net"), "bridge")); + } + #[test] fn absent_stack_member_recovery_requires_a_live_sibling() { let present: HashSet = ["indeedhub-redis", "indeedhub-relay", "indeedhub"] diff --git a/core/archipelago/src/container/quadlet.rs b/core/archipelago/src/container/quadlet.rs index a6150398..280605eb 100644 --- a/core/archipelago/src/container/quadlet.rs +++ b/core/archipelago/src/container/quadlet.rs @@ -938,6 +938,53 @@ pub fn health_cmd_changed(old_body: &str, new_body: &str) -> bool { != directive_values(new_body, "HealthRetries=") } +/// A unit rewrite and a successful systemd restart are separate operations. +/// Keep the restart obligation across errors or a management-daemon restart. +pub struct RestartObligation { + marker: PathBuf, + pending: bool, +} + +impl RestartObligation { + pub async fn prepare(unit_path: &Path, newly_required: bool) -> Result { + let marker = unit_path.with_extension("restart-pending"); + if newly_required { + // Contents contain no manifest environment or credentials. sync_all + // makes the obligation durable before the subsequent unit rename. + let file = tokio::fs::OpenOptions::new() + .write(true) + .create(true) + .truncate(false) + .open(&marker) + .await + .context("record pending Quadlet restart")?; + file.sync_all().await?; + if let Some(parent) = marker.parent() { + tokio::fs::File::open(parent).await?.sync_all().await?; + } + } + let pending = tokio::fs::try_exists(&marker).await?; + Ok(Self { marker, pending }) + } + + pub fn is_pending(&self) -> bool { + self.pending + } + + /// Call only after systemd accepted the replacement service successfully. + pub async fn complete(self) -> Result<()> { + if self.pending { + tokio::fs::remove_file(&self.marker) + .await + .context("clear completed Quadlet restart")?; + if let Some(parent) = self.marker.parent() { + tokio::fs::File::open(parent).await?.sync_all().await?; + } + } + Ok(()) + } +} + pub fn publish_ports_changed(old_body: &str, new_body: &str) -> bool { let old_ports = directive_values(old_body, "PublishPort="); let new_ports = directive_values(new_body, "PublishPort="); @@ -1541,6 +1588,28 @@ app: assert!(!s.contains("Network=host")); } + #[test] + fn portainer_catalog_network_repairs_same_node_routing_without_exposing_backend() { + let manifest = AppManifest::parse(include_str!( + "../../../../apps/portainer/manifest.yml" + )) + .expect("shipped Portainer manifest must parse"); + let new = QuadletUnit::from_manifest(&manifest, "portainer").render(); + assert!(new.contains("Network=slirp4netns\n")); + assert!(!new.contains("NetworkAlias=")); + assert!(new.contains("PublishPort=127.0.0.1:9000:9000/tcp")); + assert!(!new.contains("PublishPort=0.0.0.0")); + // The upgrade changes networking only: retain both state mounts and the + // existing rootless socket, without an app.ini or repository rewrite. + assert!(new.contains("Volume=/var/lib/archipelago/portainer:/data")); + assert!(new.contains("Volume=/var/lib/archipelago/portainer/compose:/data/compose")); + assert!(new.contains("Volume=/run/user/1000/podman/podman.sock:/var/run/docker.sock")); + let old = new.replace("Network=slirp4netns\n", ""); + assert!(network_aliases_changed(&old, &new)); + assert!(!network_aliases_changed(&new, &new)); + assert!(!publish_ports_changed(&old, &new)); + } + #[test] fn from_manifest_slirp4netns_omits_network_alias() { let yaml = r#" @@ -1891,6 +1960,35 @@ app: assert!(!network_aliases_changed(new, new)); } + #[tokio::test] + async fn failed_runtime_change_remains_pending_when_unit_already_matches() { + let dir = tempfile::tempdir().unwrap(); + let unit = dir.path().join("portainer.container"); + tokio::fs::write(&unit, "[Container]\n").await.unwrap(); + let pending = RestartObligation::prepare(&unit, true).await.unwrap(); + assert!(pending.is_pending()); + tokio::fs::write(&unit, "[Container]\nNetwork=slirp4netns\n") + .await + .unwrap(); + // Simulate systemctl failure or daemon interruption after unit rewrite. + drop(pending); + let retry = RestartObligation::prepare(&unit, false).await.unwrap(); + assert!(retry.is_pending(), "matching unit must not discard failed restart"); + retry.complete().await.unwrap(); + assert!(!RestartObligation::prepare(&unit, false).await.unwrap().is_pending()); + } + + #[tokio::test] + async fn pending_runtime_change_errors_are_not_reported_as_success() { + let dir = tempfile::tempdir().unwrap(); + let missing = dir.path().join("missing/app.container"); + assert!(RestartObligation::prepare(&missing, true).await.is_err()); + let unit = dir.path().join("app.container"); + let pending = RestartObligation::prepare(&unit, true).await.unwrap(); + tokio::fs::remove_file(unit.with_extension("restart-pending")).await.unwrap(); + assert!(pending.complete().await.is_err()); + } + #[test] fn network_aliases_changed_detects_network_mode_drift() { let old = "[Container]\nNetwork=slirp4netns\n"; diff --git a/core/container/src/manifest.rs b/core/container/src/manifest.rs index e23669b6..80245a5f 100644 --- a/core/container/src/manifest.rs +++ b/core/container/src/manifest.rs @@ -1746,40 +1746,28 @@ app: } } exempt.sort(); - // 28 as of 2026-08-23: the 26 below plus cuprate's two exemptions — - // 18183 (Monero p2p gossip, same reasoning as bitcoin's 8333) and - // 18090 (host mapping for Monero's canonical 18089 restricted RPC, - // upstream's own safe-for-public - // subset that wallets connect to directly as a "remote node" over - // plain HTTP JSON-RPC — same reasoning as electrumx's 50001). - // cuprate's unrestricted RPC (full node control) stays loopback-only - // (auth: local), not in this set. - // - // 26 as of 2026-08-16: the 25 below plus phoenixd 9740, a - // loopback-only JSON API whose own generated http password - // authenticates every request (added with the phoenixd onboarding, - // which did not update this count — exactly the drift this test - // exists to catch). - // - // 25 as of the v1.7.123 port-policy round: bitcoin p2p (8333 ×2), - // core-lightning 9736/9835, electrumx 50001, fedimint 8173/8174, - // fedimint-gateway 8176/9737, gitea ssh 2222, lightning-stack - // 8091/9738/10010, lnd 9735/10009/18080, netbird 3478/8086/8087, - // pine TLS 10381 + the three voice ports (10200/10300/10400 — the - // disclosed known gap), router SSDP/mDNS 1900/5353. Every one is a - // deliberate, rationale-carrying exemption; the release-gate test - // stage timed out that cycle, so the count here lagged at 17. - assert_eq!( - exempt.len(), - 28, - "unauthenticated port set changed — review before updating this count: {exempt:?}" - ); + // Reviewed 2026-09-30: lightning-stack's three retired endpoints + // disappeared; Cuprate restricted RPC moved from none to gate-open. + // Compare exact endpoints, not just a count that can hide substitutions. + let expected = [ + ("bitcoin-core", 8333), ("bitcoin-knots", 8333), + ("core-lightning", 9736), ("core-lightning", 9835), + ("cuprate", 18183), ("electrumx", 50001), + ("fedimint", 8173), ("fedimint", 8174), + ("fedimint-gateway", 8176), ("fedimint-gateway", 9737), + ("gitea", 2222), ("lnd", 9735), ("lnd", 10009), ("lnd", 18080), + ("netbird", 8087), ("netbird-server", 3478), ("netbird-server", 8086), + ("phoenixd", 9740), ("pine", 10381), ("pine-openwakeword", 10400), + ("pine-piper", 10200), ("pine-whisper", 10300), + ("router", 1900), ("router", 5353), + ].into_iter().map(|(id, port)| (id.to_owned(), port)).collect::>(); + assert_eq!(exempt, expected, "unauthenticated endpoint set changed; review each exemption"); } /// `auth: open` ports are served by the gate WITHOUT its login challenge, /// so they are the second unauthenticated-by-the-gate surface and get the - /// same review guard as `auth: none`. Each one must be an app that - /// enforces a real login of its own. + /// same review guard as `auth: none`. Each must enforce its own login or + /// have an explicitly reviewed public protocol purpose. #[test] fn gate_open_ports_are_all_accounted_for() { let apps = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../apps"); @@ -1801,6 +1789,8 @@ app: } } open.sort(); + // Cuprate 18090 is its deliberately public restricted RPC subset; + // unrestricted node-control RPC remains container-loopback-only. // Gitea 3001 (git clients speak basic-auth, not browser cookies), // BTCPay 23000 (checkout/invoice/webhook endpoints must be reachable // by anonymous payers), and — since the v1.8.7 platform round — the @@ -1812,11 +1802,12 @@ app: open, vec![ ("btcpay-server".to_string(), 23000u16), + ("cuprate".to_string(), 18090u16), ("gitea".to_string(), 3001u16), ("nginx-proxy-manager".to_string(), 8081u16), ("tailscale".to_string(), 8240u16), ], - "gate-open port set changed — every entry must be an app with its own login" + "gate-open port set changed — review login or intentional public protocol purpose" ); } diff --git a/core/container/src/podman_client.rs b/core/container/src/podman_client.rs index b59ed7a4..5f39c379 100644 --- a/core/container/src/podman_client.rs +++ b/core/container/src/podman_client.rs @@ -310,59 +310,7 @@ impl PodmanClient { ); continue; } - // Honour the manifest's protocol (default tcp). netbird's STUN port - // is 3478/udp; forcing tcp here would publish the wrong protocol and - // silently break relay discovery. - let protocol = match port.protocol.to_ascii_lowercase().as_str() { - "udp" => "udp", - "sctp" => "sctp", - _ => "tcp", - }; - // Effective bind. A gated port with no declared bind would - // publish 0.0.0.0 — the app would own every host address, which - // is both the exposure itself and the reason the daemon's app - // gate cannot bind those addresses to authenticate them. Pin it - // to loopback so the gate can take the external addresses. - // - // Doing it HERE, at container creation, is the point: the pin and - // the gate's takeover then both come from the daemon and cannot - // disagree. The earlier attempt put this decision in manifest - // data instead, and a node whose manifests lagged the binary - // published Bitcoin's loopback-only RPC across the LAN - // (test node, 2026-08-03). - // - // A port that already declares a bind is never overridden — that - // is exactly what keeps `bind: 127.0.0.1` ports host-local and - // leaves `auth: none` protocol ports (LND gRPC/REST, electrum) - // published as they are, so remote wallets keep working. - // NOTE: the daemon deliberately does NOT rewrite this. Pinning a - // published port to loopback is how an app hands its external - // addresses to the gate, but it belongs in the manifest, not in - // daemon-side inference: - // - // * `bind` is already honoured by every publish path (here and - // in package::install), so a manifest edit needs no code. - // * inference here would cover only THIS path — proven on - // a test node, where a recreate went through another one and - // the pin never applied. - // * and inferring from an ABSENT field is what republished - // Bitcoin's loopback RPC across the LAN, and came within one - // container-recreate of pinning LND's gRPC/REST and breaking - // every remote wallet. - // - // So the migration ships as `bind: 127.0.0.1` in the signed - // catalog. Verified 2026-08-03 that a disk-only manifest edit is - // overridden by the catalog, which is precisely why the catalog is - // the right and only place to carry it. - let mut mapping = serde_json::json!({ - "container_port": port.container, - "host_port": port.host, - "protocol": protocol, - }); - if !port.bind.is_empty() { - mapping["host_ip"] = serde_json::json!(port.bind); - } - port_mappings.push(mapping); + port_mappings.push(podman_publish_mapping(port)); } let mut mounts = Vec::new(); @@ -751,6 +699,25 @@ pub fn image_uses_insecure_registry(image: &str) -> bool { .is_some_and(|host| INSECURE_REGISTRY_HOSTS.contains(&host)) } +// Keep the explicitly declared bind and transport identical to Quadlet. The +// app gate owns external listeners; container publication must not bypass it. +fn podman_publish_mapping(port: &crate::manifest::PortMapping) -> serde_json::Value { + let protocol = match port.protocol.to_ascii_lowercase().as_str() { + "udp" => "udp", + "sctp" => "sctp", + _ => "tcp", + }; + let mut mapping = serde_json::json!({ + "container_port": port.container, + "host_port": port.host, + "protocol": protocol, + }); + if !port.bind.is_empty() { + mapping["host_ip"] = serde_json::json!(port.bind); + } + mapping +} + fn podman_network_settings( network: Option<&str>, network_policy: &str, @@ -1110,6 +1077,15 @@ mod tests { )); } + #[test] + fn portainer_manifest_keeps_private_network_and_loopback_api_publication() { + let m = AppManifest::parse(include_str!("../../../apps/portainer/manifest.yml")).unwrap(); + assert_eq!(podman_network_settings(m.app.container.network.as_deref(), &m.app.security.network_policy), ("slirp4netns", None)); + assert_eq!(podman_publish_mapping(&m.app.ports[0]), serde_json::json!({ + "container_port": 9000, "host_port": 9000, "protocol": "tcp", "host_ip": "127.0.0.1" + })); + } + #[test] fn podman_network_settings_uses_networks_map_for_custom_networks() { assert_eq!( diff --git a/core/container/src/runtime.rs b/core/container/src/runtime.rs index c1e662da..6d7c8eb9 100644 --- a/core/container/src/runtime.rs +++ b/core/container/src/runtime.rs @@ -618,6 +618,28 @@ impl DockerRuntime { } } +// Docker is a development fallback. Refuse Podman-only network modes instead +// of silently installing a different topology; still honor binds for other apps. +fn docker_network_and_ports(manifest: &AppManifest, offset: u16) -> Result> { + let network = manifest.app.container.network.as_deref() + .filter(|v| !v.is_empty()) + .unwrap_or(&manifest.app.security.network_policy); + if matches!(network, "slirp4netns" | "pasta") { + anyhow::bail!("this app requires rootless Podman networking ({network})"); + } + let mut args = Vec::new(); + if !network.is_empty() && network != "isolated" { + args.extend(["--network".to_owned(), network.to_owned()]); + } + for port in &manifest.app.ports { + let host = port.host.checked_add(offset).context("published port offset overflow")?; + let bind = if port.bind.is_empty() { String::new() } else { format!("{}:", port.bind) }; + let protocol = if port.protocol.is_empty() { "tcp" } else { &port.protocol }; + args.extend(["-p".to_owned(), format!("{bind}{host}:{}/{protocol}", port.container)]); + } + Ok(args) +} + #[async_trait] impl ContainerRuntime for DockerRuntime { async fn pull_image(&self, image: &str, signature: Option<&str>) -> Result<()> { @@ -657,25 +679,7 @@ impl ContainerRuntime for DockerRuntime { cmd.arg("--read-only"); } - match manifest.app.security.network_policy.as_str() { - "host" => { - cmd.arg("--network").arg("host"); - } - "isolated" => { - // Docker uses bridge network by default - } - _ => { - cmd.arg("--network") - .arg(&manifest.app.security.network_policy); - } - } - - // Port mappings with offset - for port in &manifest.app.ports { - let host_port = port.host + port_offset; - cmd.arg("-p") - .arg(format!("{}:{}", host_port, port.container)); - } + cmd.args(docker_network_and_ports(manifest, port_offset)?); // Volumes for volume in &manifest.app.volumes { @@ -1035,6 +1039,17 @@ mod tests { use super::*; use std::collections::HashMap; + #[test] + fn docker_fallback_rejects_rootless_only_topology_and_preserves_bind_protocol() { + let mut m = AppManifest::parse(include_str!("../../../apps/portainer/manifest.yml")).unwrap(); + assert!(docker_network_and_ports(&m, 0).is_err()); + m.app.container.network = Some("bridge".into()); + m.app.ports[0].protocol = "udp".into(); + let args = docker_network_and_ports(&m, 1).unwrap(); + assert_eq!(args, vec!["--network", "bridge", "-p", "127.0.0.1:9001:9000/udp"]); + assert!(docker_network_and_ports(&m, u16::MAX).is_err()); + } + #[test] fn missing_container_classifier_covers_podman5_phrasings() { // podman 5.x `inspect` phrasing for a missing container. diff --git a/docs/app-manifest-spec.md b/docs/app-manifest-spec.md index d740d07a..13cb2899 100644 --- a/docs/app-manifest-spec.md +++ b/docs/app-manifest-spec.md @@ -290,3 +290,15 @@ app: Validate with `scripts/validate-app-manifest.sh` and regenerate the catalog with `scripts/generate-app-catalog.py` (drift-checked in CI by `scripts/check-app-catalog-drift.py`). + +### Persistent-state backup for network migrations + +`app.backup_on_network_change: true` opts an app into a stopped-state snapshot +before an explicitly selected rootless network mode is migrated. The orchestrator +archives writable persistent bind mounts under the node data directory, collapses +nested mounts, excludes the runtime Podman socket, and preserves the previous +Quadlet definition for rollback. Named volumes, outside-data-root state and +symlinked mount roots fail closed rather than silently producing an incomplete +backup. A failed snapshot resumes the original service and leaves migration +pending. Private archives are retained under `migration-backups/`; fresh installs +and unchanged network configurations do not create migration snapshots. diff --git a/docs/gitea-portainer-repair-20260930.md b/docs/gitea-portainer-repair-20260930.md new file mode 100644 index 00000000..65558066 --- /dev/null +++ b/docs/gitea-portainer-repair-20260930.md @@ -0,0 +1,104 @@ +# Same-node Gitea sources in Portainer + +Status: root cause reproduced and network repair verified in disposable Portainer +instances; final migration integration and release acceptance remain in progress. +This change belongs to the next signed catalog, OTA and ISO. It does not modify +published 1.8.21 artifacts. + +## Confirmed cause + +On the affected X250, Gitea 1.27.3 and Portainer 2.45.0 run in rootless Podman +5.4.2, managed by user Quadlet services. Gitea publishes HTTP on loopback and the +Archipelago app gate serves its public port. Gitea's public ROOT_URL already +matches that gate URL. + +Portainer had no explicit network selection and Podman selected pasta. Its +network namespace contained the host's LAN address. A Git request to that same +LAN address therefore reached Portainer's namespace rather than the host gate: +connection refused before authentication. The exact smart-HTTP request from the +host returned 200 with `application/x-git-upload-pack-advertisement`. From +Portainer's actual namespace the LAN request was refused, while its host mapping +returned a Git advertisement and the expected branch tip. Direct container-IP +requests timed out. Container health and host-only HTTP checks missed the defect. + +A disposable Portainer using `slirp4netns` successfully created a Source through +Portainer's own API, using the original LAN clone URL. Returning that fixture to +pasta reproduced the refusal; recreating with slirp repaired it while preserving +its account and saved Source. Restart also passed. The requested branch tip and +Compose file were read from that actual Portainer network namespace. No user +stack was deployed. Deployment addresses and repository details are kept outside +this public record. + +## Source changes + +- Declare Portainer's rootless `slirp4netns` mode in its manifest. No shared static + container IP, host networking, all-interface backend publication or auth bypass. +- Keep Gitea's loopback HTTP backend and gate port; machine Git uses Gitea's + authentication. Remove obsolete port-3000 nginx metadata/template and the old + best-effort installer commands which silently rewrote app.ini and falsely + claimed success. Gitea owns first-run setup and operator configuration. +- Existing Quadlet reconciliation applies Network= drift. Record a durable + pending restart before updating the unit and clear it only after a successful + restart, so failed reloads/restarts and management interruptions retry. +- Detect explicit rootless network-mode drift in the older Podman runtime too. + Unspecified networks do not trigger inferred changes to unrelated apps. +- Portainer opts into `backup_on_network_change`. Before recreation, gracefully + stop the app and archive its writable persistent bind mounts, including nested + Compose state, once each. Runtime sockets are excluded. Save the previous + Quadlet definition, where present. Archives live under the node data directory's + private `migration-backups//` directory; state is never deleted. Backup + failures resume the original service and fail the migration visibly. +- Keep Podman API and Quadlet bind/network behavior covered by actual-manifest + tests. Docker remains a development fallback: it now preserves bind/protocol + declarations and rejects Podman-only networking instead of silently changing it. + +## Operator use and diagnostics + +Use Gitea's advertised HTTP(S) clone URL in Portainer Sources, with the Gitea +username and token in the credential fields. On first-run Gitea setup, the public +base URL must match the origin opened through Archipelago (including its port). +Keep a deliberately configured HTTPS/domain origin when one exists. Do not use a +container IP or put a token into the URL. A private repository requires repository +read permission. A successful Source check fetches Git refs; it does not deploy +a stack or establish that a Compose build uses a desired application revision. + +`scripts/check-portainer-git-source.py` calls Portainer's own read-only Source +connection test. Supply a private mode-600 JSON credential file containing +`api_key` or `jwt`, and optionally `git: {username, password}`. Pass +`--portainer-url`, `--repository-url` and `--credentials-file`. It does not create +Sources or stacks and prints no credentials or raw server errors. It distinguishes +Portainer login/API failures from Git connection refusal, timeout, DNS/TLS +failure, HTML/login interception and repository authentication failure. TLS +verification stays enabled and API redirects are refused. + +## Upgrade and rollback + +The signed catalog embeds manifests and overrides installed disk copies. A disk +edit alone cannot deliver this fix. Publish the matching catalog with the tested +runtime, then verify the generated unit, actual network mode and Source API. +Expect a Portainer interruption while the snapshot and recreation run; duration +depends on its saved state size. +Gitea does not need recreation or an app.ini rewrite for this repair. + +Keep the previous trusted catalog/runtime for rollback. Restore that catalog +before restoring the saved `previous.container`, reloading user systemd and +starting Portainer; otherwise reconciliation will correctly reapply the new +manifest. The archive is a stopped-state emergency backup, not an instruction to +roll back a live database automatically. Restore it only with Portainer stopped +and after preserving any newer state. Do not replace Gitea data/config, keys, +repositories or the production Portainer database with disposable test data. + +## Validation and remaining gates + +- Disposable X250 Portainer Source API: old mode refuses; repaired mode succeeds; + saved account/Source survive recreation; restart succeeds. +- Invalid Git credentials produce a repository-authentication error, distinct + from TCP refusal. Requested branch and Compose file read from Portainer context. +- Final expanded backend suite: 1,575 passed, zero failed, four existing ignored + tests, including stopped-state archive round trips and failure preservation. Container runtime suite: 78 passed. + Five diagnostic regression tests passed. Combined tests with the merged + paid-download PRs remain pending. +- Still required before release: live automatic migration with the new runtime, + snapshot/rollback verification, private-repository and install-order acceptance, + lifecycle/reboot convergence, and signed-catalog delivery to the existing app. + Record LFS/registry/SSH/browser checks and actual hardware/runtime coverage. diff --git a/image-recipe/configs/nginx-gitea-iframe.conf b/image-recipe/configs/nginx-gitea-iframe.conf deleted file mode 100644 index 9d33126c..00000000 --- a/image-recipe/configs/nginx-gitea-iframe.conf +++ /dev/null @@ -1,21 +0,0 @@ -# Gitea iframe proxy — strips X-Frame-Options so Gitea works in Archipelago iframe. -# Gitea container binds to port 3001, this proxy listens on port 3000 (the public port). -# Deployed to /etc/nginx/conf.d/gitea-iframe.conf -server { - listen 3000; - server_name _; - client_max_body_size 1G; - - location / { - proxy_pass http://127.0.0.1:3001; - proxy_set_header Host $http_host; - proxy_set_header X-Real-IP $remote_addr; - proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; - proxy_set_header X-Forwarded-Proto $scheme; - proxy_http_version 1.1; - proxy_set_header Upgrade $http_upgrade; - proxy_set_header Connection "upgrade"; - proxy_hide_header X-Frame-Options; - proxy_hide_header Content-Security-Policy; - } -} diff --git a/scripts/check-portainer-git-source.py b/scripts/check-portainer-git-source.py new file mode 100644 index 00000000..e8ebe6a6 --- /dev/null +++ b/scripts/check-portainer-git-source.py @@ -0,0 +1,95 @@ +#!/usr/bin/env python3 +"""Test Git from Portainer's server context without creating a Source or stack.""" +import argparse +import json +import pathlib +import socket +import stat +import urllib.error +import urllib.parse +import urllib.request + + +class NoRedirect(urllib.request.HTTPRedirectHandler): + def redirect_request(self, req, fp, code, msg, headers, newurl): + return None + + +def classify(error): + text = error.lower() + for category, patterns in ( + ('connection-refused', ('connection refused',)), + ('dns-failure', ('no such host', 'name resolution', 'server misbehaving')), + ('timeout', ('timeout', 'timed out', 'deadline exceeded')), + ('tls-failure', ('x509:', 'certificate', 'tls handshake')), + ('proxy-or-login-interception', ('text/html', '/dev/null sudo -n true || { echo 'Isolated backend tests require noninteractive sudo for systemd namespaces.' >&2; exit 1; } metadata=$(mktemp) trap 'rm -f "$metadata"' EXIT -if ! cargo test --manifest-path "$REPO/core/Cargo.toml" -p archipelago --bin archipelago \ +case "${ARCHY_TEST_PACKAGE:-archipelago}" in + archipelago) test_target=(-p archipelago --bin archipelago) ;; + archipelago-container) test_target=(-p archipelago-container --lib) ;; + *) echo 'Unsupported isolated test package' >&2; exit 2 ;; +esac +if ! cargo test --manifest-path "$REPO/core/Cargo.toml" "${test_target[@]}" \ --locked --no-run --message-format=json --config 'profile.test.package.archipelago.opt-level=0' > "$metadata"; then python3 - "$metadata" <<'PYDIAG' import json,sys diff --git a/tests/regression/portainer-git-diagnostics.py b/tests/regression/portainer-git-diagnostics.py new file mode 100644 index 00000000..562285b4 --- /dev/null +++ b/tests/regression/portainer-git-diagnostics.py @@ -0,0 +1,70 @@ +#!/usr/bin/env python3 +import importlib.util +import io +import json +import pathlib +import unittest +import urllib.error + +ROOT = pathlib.Path(__file__).resolve().parents[2] +spec = importlib.util.spec_from_file_location('diagnostic', ROOT / 'scripts/check-portainer-git-source.py') +m = importlib.util.module_from_spec(spec) +spec.loader.exec_module(m) + + +class Response(io.BytesIO): + pass + + +class FakeAPI: + def __init__(self, result=None, error=None): + self.result, self.error, self.request = result, error, None + def open(self, request, timeout): + self.request = request + if self.error: + raise self.error + return Response(json.dumps(self.result).encode()) + + +class Diagnostics(unittest.TestCase): + def test_server_context_credentials_not_in_url_and_tls_stays_enabled(self): + api = FakeAPI({'success': True}) + result = m.check('http://localhost:9000', 'http://node:3001/user/repo', + {'jwt': 'test-jwt', 'git': {'username': 'test-user', 'password': 'test-secret'}}, api) + self.assertTrue(result['success']) + self.assertEqual(api.request.full_url, 'http://localhost:9000/api/gitops/sources/test') + payload = json.loads(api.request.data) + self.assertFalse(payload['tlsSkipVerify']) + self.assertEqual(payload['authentication']['password'], 'test-secret') + self.assertNotIn('test-secret', json.dumps(result)) + + def test_failure_categories_from_source_api(self): + cases = [('dial tcp: connection refused', 'connection-refused'), + ('lookup node: no such host', 'dns-failure'), + ('context deadline exceeded', 'timeout'), + ('unexpected content-type text/html', 'proxy-or-login-interception'), + ('authentication required', 'repository-authentication'), + ('x509: certificate signed by unknown authority', 'tls-failure'), + ('repository not found', 'repository-not-found-or-private')] + for error, expected in cases: + with self.subTest(error=error): + result = m.check('http://localhost:9000', 'http://node/repo', {'jwt': 'test'}, FakeAPI({'success': False, 'error': error})) + self.assertEqual(result['category'], expected) + self.assertFalse(result['success']) + + def test_portainer_auth_is_distinct_from_repository_auth(self): + api = FakeAPI(error=urllib.error.HTTPError('http://localhost', 401, 'Unauthorized', {}, None)) + self.assertEqual(m.check('http://localhost', 'http://node/repo', {'jwt': 'bad'}, api)['category'], 'portainer-authentication') + + def test_html_or_malformed_api_response_never_proves_git_success(self): + for value in ({'status': 1}, {'success': 'true'}, [], 'login'): + self.assertFalse(m.check('http://localhost', 'http://node/repo', {'jwt': 'test'}, FakeAPI(value))['success']) + + def test_credential_urls_rejected_before_request(self): + for value in ('http://user:secret@node/repo', 'http://node/repo?token=secret', 'file:///data/repo'): + with self.assertRaises(ValueError): + m.check('http://localhost', value, {'jwt': 'test'}, FakeAPI()) + + +if __name__ == '__main__': + unittest.main()