Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -657,13 +657,22 @@ store opened and migrations ran. `nudo-boot-guard`, run as `ExecStartPre=` from
a stable path deliberately outside the symlink it may revert, counts
unconfirmed boots and puts the previous release back after three.

**Off by default, twice.** `NUDO_ALLOW_SELF_UPGRADE` (whoever installs) and a
dashboard toggle (whoever operates) must both be on, and only a managed-layout
binary install on a published target is eligible; containers keep `docker pull`
and legacy binary installs keep the manual commands, now with the one-time
migration to the layout printed beside them. Rollback of the database is
deliberately manual — an automatic restore would silently discard writes made
after the snapshot — and the page says so, with the snapshot's path.
**Off until switched on, once.** The dashboard toggle is the whole gate, and
only a managed-layout binary install on a published target is eligible;
containers keep `docker pull`, and a binary install laid out flat keeps the
manual commands, with the move to the release layout printed beside them.
Rollback of the database is deliberately manual — an automatic restore would
silently discard writes made after the snapshot — and the page says so, with
the snapshot's path.

0.4.0 shipped a second gate, `NUDO_ALLOW_SELF_UPGRADE`, on the theory that
whoever installs and whoever operates are different people and both should
have to say yes. For a tool one person installs and operates that was a second
lock on the same door: its only visible effect was a dashboard that refused an
upgrade the operator had already opted into, and a set-this-env-var instruction
in three places. Removed in 0.4.1; the proto field is reserved rather than
reused. The packaged unit has always put a fresh install in the release layout,
so what a new install now needs is one tick in settings and nothing else.

**Tested end to end.** `crates/allinone/tests/self_upgrade.rs` builds the real
binary, serves a fixture release over loopback, drives the RPC, and asserts the
Expand Down
12 changes: 6 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,12 @@ downloaded, verified against the sha256 the release workflow commits into
atomically — and reverted by `nudo-boot-guard` if it cannot start. Nothing is ever
fetched and piped to a shell.

Self-upgrading is off unless two switches are both on: start the instance with
`NUDO_ALLOW_SELF_UPGRADE=true` *and* enable it in the dashboard's settings. The
trade-off of the layout is that the service user can overwrite its own binaries —
that is what self-upgrading is — so if you will never use it, installing to
`/usr/local/bin` with a root-owned binary remains the tighter posture, and the
`/upgrade` page keeps printing exact manual commands for it.
Self-upgrading is off until you switch it on in the dashboard's settings — one
tick, no configuration. The trade-off of the layout is that the service user can
overwrite its own binaries — that is what self-upgrading is — so if you will
never use it, installing to `/usr/local/bin` with a root-owned binary remains
the tighter posture, and the `/upgrade` page keeps printing exact manual
commands for it.

### From source

Expand Down
12 changes: 9 additions & 3 deletions controlplane.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1123,9 +1123,15 @@ message SelfUpgradeStatus {
string from_version = 2;
string to_version = 3;
string error = 4;
// The three gates, reported separately so the dashboard can say which one
// is closed rather than a generic "unavailable".
bool allowed_by_config = 5;
// The two gates, reported separately so the dashboard can say which one is
// closed rather than a generic "unavailable".
//
// 5 was `allowed_by_config`, a process flag that had to be on as well.
// Removed in 0.4.1: for a tool one person installs and operates, it was a
// second lock on the same door, and its only visible effect was a dashboard
// that refused an upgrade someone had already opted into.
reserved 5;
reserved "allowed_by_config";
bool enabled_in_settings = 6;
bool eligible = 7;
// RFC 3339; empty when no upgrade has ever run.
Expand Down
4 changes: 1 addition & 3 deletions crates/allinone/tests/self_upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ impl Harness {
let web_port = free_port();
let child = Command::new(self_dir.join("current").join("nudo-all-in-one"))
.env("NUDO_SELF_DIR", &self_dir)
.env("NUDO_ALLOW_SELF_UPGRADE", "true")
.env(
"NUDO_SELF_UPGRADE_DOWNLOAD_BASE",
format!("http://127.0.0.1:{}", artifact_server.port),
Expand Down Expand Up @@ -262,8 +261,7 @@ async fn an_instance_upgrades_itself_and_comes_back_as_the_new_version() {
let status = harness
.wait_for_status("the instance to come up", |status| status.state == "idle")
.await;
assert!(status.allowed_by_config, "{}", harness.debug_logs());
assert!(status.enabled_in_settings);
assert!(status.enabled_in_settings, "{}", harness.debug_logs());
assert!(status.eligible, "the child must detect the managed layout");

let pid_before_upgrade = harness.child.id();
Expand Down
8 changes: 3 additions & 5 deletions crates/server/src/api/self_upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ fn to_proto(view: StatusView) -> SelfUpgradeStatus {
from_version: view.from_version,
to_version: view.to_version,
error: view.error,
allowed_by_config: view.allowed_by_config,
enabled_in_settings: view.enabled_in_settings,
eligible: view.eligible,
updated_at: view.updated_at,
Expand Down Expand Up @@ -99,8 +98,7 @@ mod tests {
.expect("status")
.into_inner();
assert_eq!(status.state, "idle");
assert!(!status.allowed_by_config, "default config allows nothing");
assert!(!status.enabled_in_settings);
assert!(!status.enabled_in_settings, "the toggle defaults off");
assert!(!status.eligible);
}

Expand All @@ -118,7 +116,7 @@ mod tests {
}

#[tokio::test]
async fn start_is_refused_by_the_config_gate_and_audited_anyway() {
async fn start_is_refused_while_the_toggle_is_off_and_audited_anyway() {
let context = test_support::context().await;
let service = service(context.clone());
let error = service
Expand All @@ -129,7 +127,7 @@ mod tests {
.await
.expect_err("must refuse");
assert_eq!(error.code(), tonic::Code::FailedPrecondition);
assert!(error.message().contains("--allow-self-upgrade"));
assert!(error.message().contains("switched off"));

// The attempt itself is on the record: asking an instance to replace
// its binaries is audit-worthy even when refused.
Expand Down
17 changes: 4 additions & 13 deletions crates/server/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,21 +86,13 @@ pub struct Config {
#[arg(long, env = "NUDO_ALLOW_SETUP", default_value_t = true)]
pub allow_setup: bool,

/// Allow this instance to download a verified release and replace its own
/// binaries when asked to from the dashboard.
///
/// Off by default, and this flag alone is not enough: the dashboard's own
/// self-upgrade toggle must also be on. Two switches because the flag is
/// set by whoever installs, while the toggle belongs to whoever operates.
#[arg(long, env = "NUDO_ALLOW_SELF_UPGRADE", default_value_t = false)]
pub allow_self_upgrade: bool,

/// Root of the self-release layout: `releases/`, the `current` symlink,
/// and the upgrade journal.
///
/// Set by the packaged unit (`/var/lib/nudo/self`). Absent means the
/// binary was installed the pre-layout way, and self-upgrade is
/// unavailable until the documented one-time migration.
/// Set by the packaged unit (`/var/lib/nudo/self`), so a standard install
/// has it. Absent means the binaries were installed somewhere flat —
/// straight into `/usr/local/bin`, say — and self-upgrade is unavailable
/// until the layout the `/upgrade` page describes is adopted.
#[arg(long, env = "NUDO_SELF_DIR")]
pub self_dir: Option<PathBuf>,

Expand Down Expand Up @@ -198,7 +190,6 @@ impl Default for Config {
update_manifest_url: crate::updates::DEFAULT_MANIFEST_URL.to_string(),
update_interval_hours: 24,
allow_setup: true,
allow_self_upgrade: false,
self_dir: None,
self_upgrade_download_base: crate::self_upgrade::DEFAULT_DOWNLOAD_BASE.to_string(),
}
Expand Down
16 changes: 7 additions & 9 deletions crates/server/src/self_upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,13 @@
//! version is ever executed.
//! - Nothing is piped to a shell; the tarball is unpacked and exec'd by this
//! code.
//! - Two switches must both be on: `--allow-self-upgrade` from whoever
//! installs, and the dashboard toggle from whoever operates.
//! - It is off until switched on. The dashboard toggle is the whole gate: one
//! decision, made by the person who would live with it, in the place they
//! are already looking. An earlier design also required a flag on the
//! process, on the theory that whoever installs and whoever operates are
//! different people — for a self-hosted tool run by one person that was a
//! second lock on the same door, and it only ever made the feature look
//! broken to someone who had already said yes.

use std::path::{Path, PathBuf};
use std::sync::{Arc, Mutex};
Expand Down Expand Up @@ -137,7 +142,6 @@ pub struct StatusView {
pub error: String,
/// RFC 3339, empty when nothing has happened yet.
pub updated_at: String,
pub allowed_by_config: bool,
pub enabled_in_settings: bool,
pub eligible: bool,
}
Expand Down Expand Up @@ -175,7 +179,6 @@ impl SelfUpgrader {
let eligibility = eligibility(&self.config);
let mut view = StatusView {
state: "idle".to_string(),
allowed_by_config: self.config.allow_self_upgrade,
enabled_in_settings: self.store.self_upgrade_enabled().await.unwrap_or(false),
eligible: matches!(eligibility, Eligibility::Managed { .. }),
..StatusView::default()
Expand Down Expand Up @@ -212,11 +215,6 @@ impl SelfUpgrader {
/// gRPC handler whose response must reach the dashboard before exec()
/// replaces the process serving it.
pub async fn start(&self, target_version: &str) -> anyhow::Result<()> {
if !self.config.allow_self_upgrade {
bail!(
"self-upgrade is not allowed by this instance's configuration (--allow-self-upgrade)"
);
}
if !self.store.self_upgrade_enabled().await? {
bail!("self-upgrade is switched off in the dashboard settings");
}
Expand Down
34 changes: 12 additions & 22 deletions crates/server/src/self_upgrade/tests.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use super::*;
use crate::store::Store;

fn config(allow: bool, self_dir: Option<PathBuf>) -> Arc<crate::Config> {
fn config(self_dir: Option<PathBuf>) -> Arc<crate::Config> {
Arc::new(crate::Config {
allow_self_upgrade: allow,
self_dir,
..crate::Config::default()
})
Expand All @@ -28,33 +27,25 @@ fn layout(dir: &Path, version: &str) {

// ---- gates ----

#[tokio::test]
async fn the_config_flag_gates_everything() {
let upgrader = SelfUpgrader::new(store().await, config(false, None));
let error = upgrader.start("99.0.0").await.expect_err("must refuse");
assert!(
error.to_string().contains("--allow-self-upgrade"),
"{error}"
);
}

#[tokio::test]
async fn the_settings_toggle_gates_everything() {
// Flag on, toggle off (the default): still refused. Two different people
// control these two switches; both must have said yes.
let upgrader = SelfUpgrader::new(store().await, config(true, None));
// Off is the default, and the default refuses. This is the whole gate:
// an instance that has not been told it may replace its own binaries
// will not, however it was installed.
let upgrader = SelfUpgrader::new(store().await, config(None));
let error = upgrader.start("99.0.0").await.expect_err("must refuse");
assert!(error.to_string().contains("switched off"), "{error}");
}

#[tokio::test]
async fn a_legacy_install_is_refused_even_with_both_switches_on() {
async fn a_flat_install_is_refused_even_with_the_toggle_on() {
// The test binary does not run from a managed layout, so even a
// configured self_dir leaves eligibility at BinaryLegacy.
// configured self_dir leaves eligibility at BinaryLegacy — the toggle
// cannot conjure a layout that is not there.
let dir = tempfile::tempdir().expect("tempdir");
let store = store().await;
store.set_self_upgrade_enabled(true).await.expect("toggle");
let upgrader = SelfUpgrader::new(store, config(true, Some(dir.path().to_path_buf())));
let upgrader = SelfUpgrader::new(store, config(Some(dir.path().to_path_buf())));
let error = upgrader.start("99.0.0").await.expect_err("must refuse");
assert!(
error.to_string().contains("cannot upgrade itself"),
Expand All @@ -65,10 +56,9 @@ async fn a_legacy_install_is_refused_even_with_both_switches_on() {
#[tokio::test]
async fn the_status_reports_which_gates_are_open() {
let store = store().await;
let upgrader = SelfUpgrader::new(store.clone(), config(true, None));
let upgrader = SelfUpgrader::new(store.clone(), config(None));
let view = upgrader.status().await;
assert!(view.allowed_by_config);
assert!(!view.enabled_in_settings);
assert!(!view.enabled_in_settings, "off until switched on");
assert!(!view.eligible);
assert_eq!(view.state, "idle");

Expand Down Expand Up @@ -380,7 +370,7 @@ async fn a_version_that_is_not_newer_is_refused_before_anything_else_network_sha
.await
.expect("record");
let dir = tempfile::tempdir().expect("tempdir");
let upgrader = SelfUpgrader::new(store, config(true, Some(dir.path().to_path_buf())));
let upgrader = SelfUpgrader::new(store, config(Some(dir.path().to_path_buf())));
// The test binary is not in a managed layout, so this refusal is the
// eligibility one — which is fine: it proves order (gates before network).
upgrader.start("0.0.1").await.expect_err("must refuse");
Expand Down
11 changes: 6 additions & 5 deletions crates/web/src/render/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,11 +204,12 @@ pub fn settings_page(
span {
"Allow this instance to upgrade itself from the "
a href="/upgrade" { "upgrade page" }
". Only half the permission: the instance must \
also have been started with "
code { "NUDO_ALLOW_SELF_UPGRADE=true" }
", and it only applies to a binary install \
running from the release layout."
". Releases are verified against a digest \
published in the manifest and rolled back \
automatically if the new version cannot \
start. Applies to a binary install running \
from the release layout; a container upgrades \
with " code { "docker pull" } "."
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/web/src/render/tests/banners.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,8 @@ fn the_settings_page_carries_every_switch_and_says_nothing_is_sent() {
assert!(rendered.contains("2 hours ago"));
// The claim that matters most on that page.
assert!(rendered.contains("no usage"));
// The self-upgrade toggle must say it is only half the permission.
assert!(rendered.contains("NUDO_ALLOW_SELF_UPGRADE"));
// The self-upgrade toggle must say what it actually authorises.
assert!(rendered.contains("upgrade itself"));
}

#[test]
Expand Down
21 changes: 6 additions & 15 deletions crates/web/src/render/tests/dashboard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@ fn an_upgrade(install: UpgradeInstall) -> UpgradeView {
}
}

/// A managed install with both opt-ins given and nothing in flight.
/// A managed install with self-upgrade switched on and nothing in flight.
fn a_managed_status() -> SelfUpgradeView {
SelfUpgradeView {
state: "idle".to_string(),
allowed_by_config: true,
enabled_in_settings: true,
eligible: true,
..SelfUpgradeView::default()
Expand Down Expand Up @@ -148,9 +147,9 @@ fn the_upgrade_page_never_pipes_anything_into_a_shell() {
#[test]
fn only_a_fully_opted_in_managed_install_gets_the_button() {
// A form on this page is a big deal — it used to be banned outright. It
// appears in exactly one configuration: managed layout, config flag on,
// settings toggle on, newer release available. Everything else stays a
// page of instructions.
// appears in exactly one configuration: managed layout, self-upgrade
// switched on, newer release available. Everything else stays a page of
// instructions.
let with_button = s(upgrade_page(&an_upgrade(UpgradeInstall::BinaryManaged {
status: a_managed_status(),
})));
Expand Down Expand Up @@ -179,15 +178,7 @@ fn only_a_fully_opted_in_managed_install_gets_the_button() {
);
}

// Managed but a gate closed: no form, and it says which gate.
let mut flag_off = a_managed_status();
flag_off.allowed_by_config = false;
let rendered = s(upgrade_page(&an_upgrade(UpgradeInstall::BinaryManaged {
status: flag_off,
})));
assert!(!rendered.contains("<form"));
assert!(rendered.contains("NUDO_ALLOW_SELF_UPGRADE"));

// Managed but switched off: no form, and it says where to switch it on.
let mut toggle_off = a_managed_status();
toggle_off.enabled_in_settings = false;
let rendered = s(upgrade_page(&an_upgrade(UpgradeInstall::BinaryManaged {
Expand Down Expand Up @@ -402,7 +393,7 @@ fn a_legacy_install_is_shown_the_path_to_the_managed_layout() {
"the migration target is named"
);
assert!(
rendered.contains("NUDO_ALLOW_SELF_UPGRADE"),
rendered.contains("settings"),
"the opt-in is explained alongside"
);
assert!(
Expand Down
Loading
Loading