Add layered settings, and fix a migration mechanism that silently lied
Two things, because the second could not be built on the first. The schema stamp was part of the same execute_batch as the CREATE TABLE IF NOT EXISTS statements, and it wrote unconditionally. Opening an older file therefore added no columns but flipped the version forward anyway; opening a *newer* file stamped it back down and then wrote rows the newer build could not read. Both produced a database whose recorded version was a lie, and every future schema change would have inherited it. Now the version is read before anything is applied, migrations are ordered and forward-only inside one transaction, a newer file is refused with SchemaTooNew rather than downgraded, and a supported version raised without a step to reach it fails at the first open instead of claiming success. Tested by stamping a file at version 99 and asserting both the refusal and that the stamp is left untouched. lumbridge-settings resolves compiled default -> settings.toml -> environment. The environment sits above the file deliberately: decision 0016 calls LUMBRIDGE_CLAUDE_OAUTH=0 "one switch off", and a switch a config file can silently re-enable is not a switch. A pinned value renders disabled and names the variable, rather than accepting an edit that would do nothing. Every field carries a WriteAuthority. Routing all writes through Configure is the obvious design and would hand a layout-only agent the program every future pane launches — the guarantee decision 0006 exists to make. Anything naming a program, path or destination is Human-only, asserted by a test that reads the path rather than trusting the author. Four paths are permanently not settings, with the reason recorded beside each and a test asserting their absence: the usage endpoint URL, the credentials path, the client identity, and the shell program. A configuration file that can redirect where an access token is sent is a credential exfiltration path with a friendly name. Environment access is a trait rather than std::env, because the workspace forbids unsafe, set_var is unsafe in Rust 2024, and the layering rule has to be testable without mutating the process running the test. Verified live with LUMBRIDGE_CLAUDE_OAUTH=0: the account-endpoint row reads off, greyed, "pinned by LUMBRIDGE_CLAUDE_OAUTH". The Advanced page names every file, endpoint and child process Lumbridge touches and states that nothing is sent anywhere else — as a fact, not as a toggle nobody can flip. File loading, comment-preserving writes and editable controls are not in this pass; 0022 records why that order is the honest one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Metal Agent
co-authored by
Claude Opus 5
Claude Opus 5
parent
72887cb4ab
commit
e5d7a3efd5
@@ -9,15 +9,22 @@ use std::path::Path;
|
||||
use lumbridge_core::{RemoteHost, RemoteTransport};
|
||||
use rusqlite::{Connection, OptionalExtension, params};
|
||||
|
||||
/// The schema version this build writes and understands.
|
||||
pub const SUPPORTED_SCHEMA_VERSION: u32 = 2;
|
||||
|
||||
/// The baseline schema.
|
||||
///
|
||||
/// Creation only. It no longer stamps the version, because it used to do so
|
||||
/// unconditionally inside the same batch as the `CREATE TABLE IF NOT EXISTS`
|
||||
/// statements — so opening an older file added no columns but flipped the stamp
|
||||
/// forward anyway, and opening a *newer* file silently stamped it back down.
|
||||
/// Both produced a database whose recorded version was a lie.
|
||||
const SCHEMA: &str = r"
|
||||
CREATE TABLE IF NOT EXISTS app_meta (
|
||||
key TEXT PRIMARY KEY NOT NULL,
|
||||
value TEXT NOT NULL
|
||||
) STRICT;
|
||||
|
||||
INSERT INTO app_meta (key, value) VALUES ('schema_version', '2')
|
||||
ON CONFLICT(key) DO UPDATE SET value = excluded.value;
|
||||
|
||||
CREATE TABLE IF NOT EXISTS remote_hosts (
|
||||
id TEXT PRIMARY KEY NOT NULL,
|
||||
label TEXT NOT NULL,
|
||||
@@ -87,6 +94,23 @@ CREATE TABLE IF NOT EXISTS workspace_snapshots (
|
||||
pub enum StorageError {
|
||||
Database(rusqlite::Error),
|
||||
UnknownTransport(String),
|
||||
/// The file was written by a newer build.
|
||||
///
|
||||
/// Refused rather than opened: this binary does not know what the extra
|
||||
/// columns mean, and the old code path would have quietly stamped the
|
||||
/// version back down and then written rows that the newer build could not
|
||||
/// read. A migration is forward-only, so the only safe answer is to stop.
|
||||
SchemaTooNew {
|
||||
found: u32,
|
||||
supported: u32,
|
||||
},
|
||||
/// The supported version was raised without a step to reach it.
|
||||
///
|
||||
/// A programming error, surfaced at the first open rather than by stamping
|
||||
/// a version the file has not reached.
|
||||
MissingMigration {
|
||||
from: u32,
|
||||
},
|
||||
}
|
||||
|
||||
impl std::fmt::Display for StorageError {
|
||||
@@ -94,6 +118,14 @@ impl std::fmt::Display for StorageError {
|
||||
match self {
|
||||
Self::Database(error) => write!(formatter, "SQLite error: {error}"),
|
||||
Self::UnknownTransport(value) => write!(formatter, "unknown remote transport: {value}"),
|
||||
Self::MissingMigration { from } => write!(
|
||||
formatter,
|
||||
"no migration is defined from schema {from}; this build cannot upgrade the file"
|
||||
),
|
||||
Self::SchemaTooNew { found, supported } => write!(
|
||||
formatter,
|
||||
"this workspace was written by a newer Lumbridge (schema {found}; this build understands {supported})"
|
||||
),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -140,10 +172,79 @@ impl Store {
|
||||
if mode == "wal" {
|
||||
connection.execute_batch("PRAGMA synchronous = NORMAL;")?;
|
||||
}
|
||||
connection.execute_batch(SCHEMA)?;
|
||||
Self::migrate(&connection)?;
|
||||
Ok(Self { connection })
|
||||
}
|
||||
|
||||
/// Brings a database up to [`SUPPORTED_SCHEMA_VERSION`], or refuses.
|
||||
///
|
||||
/// The version is read *before* anything is applied, which is the whole
|
||||
/// point: a migration has to know where it is starting from.
|
||||
fn migrate(connection: &Connection) -> Result<()> {
|
||||
connection.execute_batch(
|
||||
"CREATE TABLE IF NOT EXISTS app_meta (
|
||||
key TEXT PRIMARY KEY NOT NULL,
|
||||
value TEXT NOT NULL
|
||||
) STRICT;",
|
||||
)?;
|
||||
let found: Option<u32> = connection
|
||||
.query_row(
|
||||
"SELECT value FROM app_meta WHERE key = 'schema_version'",
|
||||
[],
|
||||
|row| row.get::<_, String>(0),
|
||||
)
|
||||
.optional()?
|
||||
.and_then(|raw| raw.parse().ok());
|
||||
|
||||
if let Some(found) = found
|
||||
&& found > SUPPORTED_SCHEMA_VERSION
|
||||
{
|
||||
return Err(StorageError::SchemaTooNew {
|
||||
found,
|
||||
supported: SUPPORTED_SCHEMA_VERSION,
|
||||
});
|
||||
}
|
||||
|
||||
// One transaction: an interrupted migration leaves the file at its old
|
||||
// version rather than half-way between two, which is what makes
|
||||
// interrupt-and-restart safe.
|
||||
connection.execute_batch("BEGIN IMMEDIATE;")?;
|
||||
let applied = (|| -> Result<()> {
|
||||
if found.is_none() {
|
||||
connection.execute_batch(SCHEMA)?;
|
||||
}
|
||||
// Ordered, forward-only steps. Each takes the database from the
|
||||
// version named to the next one; none may be edited once shipped.
|
||||
for step in found.unwrap_or(SUPPORTED_SCHEMA_VERSION)..SUPPORTED_SCHEMA_VERSION {
|
||||
Self::migration_step(connection, step)?;
|
||||
}
|
||||
connection.execute(
|
||||
"INSERT INTO app_meta (key, value) VALUES ('schema_version', ?1)
|
||||
ON CONFLICT(key) DO UPDATE SET value = excluded.value",
|
||||
[SUPPORTED_SCHEMA_VERSION.to_string()],
|
||||
)?;
|
||||
Ok(())
|
||||
})();
|
||||
if applied.is_err() {
|
||||
connection.execute_batch("ROLLBACK;")?;
|
||||
} else {
|
||||
connection.execute_batch("COMMIT;")?;
|
||||
}
|
||||
applied
|
||||
}
|
||||
|
||||
/// One forward step, from the named version to the next.
|
||||
///
|
||||
/// There are none yet: version 2 is the baseline. An unhandled step is an
|
||||
/// error rather than a silent success, so raising
|
||||
/// [`SUPPORTED_SCHEMA_VERSION`] without writing the step that earns it
|
||||
/// fails at the first open instead of stamping a version the file has not
|
||||
/// actually reached. A step must never be edited once it has shipped.
|
||||
fn migration_step(_connection: &Connection, from: u32) -> Result<()> {
|
||||
// 1 => connection.execute_batch("ALTER TABLE …")?,
|
||||
Err(StorageError::MissingMigration { from })
|
||||
}
|
||||
|
||||
/// Return the currently installed schema version.
|
||||
///
|
||||
/// # Errors
|
||||
@@ -285,7 +386,8 @@ impl Store {
|
||||
mod tests {
|
||||
use lumbridge_core::{RemoteHost, RemoteTransport};
|
||||
|
||||
use super::Store;
|
||||
use super::{SUPPORTED_SCHEMA_VERSION, StorageError, Store};
|
||||
use rusqlite::Connection;
|
||||
|
||||
#[test]
|
||||
fn initializes_schema_and_round_trips_remote_host() {
|
||||
@@ -344,4 +446,93 @@ mod tests {
|
||||
Some(r#"{"selected":7,"panels":[{"id":7,"kind":"terminal"}]}"#)
|
||||
);
|
||||
}
|
||||
|
||||
/// The defect this mechanism exists for.
|
||||
///
|
||||
/// The stamp used to be part of the creation batch, so opening a file
|
||||
/// written by a newer build wrote the older version number over it. The
|
||||
/// file then claimed a schema it did not have, and this binary would go on
|
||||
/// to write rows the newer one could not read.
|
||||
#[test]
|
||||
fn a_newer_database_is_refused_rather_than_stamped_backwards() {
|
||||
let directory = tempfile::tempdir().expect("a temporary directory");
|
||||
let path = directory.path().join("workspace.db");
|
||||
{
|
||||
let store = Store::open(&path).expect("a fresh store");
|
||||
store
|
||||
.connection
|
||||
.execute(
|
||||
"INSERT INTO app_meta (key, value) VALUES ('schema_version', '99')
|
||||
ON CONFLICT(key) DO UPDATE SET value = excluded.value",
|
||||
[],
|
||||
)
|
||||
.expect("stamp a future version");
|
||||
}
|
||||
let Err(error) = Store::open(&path) else {
|
||||
panic!("a future schema must be refused");
|
||||
};
|
||||
assert!(
|
||||
matches!(
|
||||
error,
|
||||
StorageError::SchemaTooNew {
|
||||
found: 99,
|
||||
supported: SUPPORTED_SCHEMA_VERSION
|
||||
}
|
||||
),
|
||||
"expected SchemaTooNew, got {error}"
|
||||
);
|
||||
|
||||
// And the file is untouched: the version it claims is still its own.
|
||||
let connection = Connection::open(&path).expect("reopen");
|
||||
let found: String = connection
|
||||
.query_row(
|
||||
"SELECT value FROM app_meta WHERE key = 'schema_version'",
|
||||
[],
|
||||
|row| row.get(0),
|
||||
)
|
||||
.expect("a stamp");
|
||||
assert_eq!(found, "99", "a refused open must not rewrite the stamp");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn opening_twice_is_idempotent_and_keeps_the_data() {
|
||||
let directory = tempfile::tempdir().expect("a temporary directory");
|
||||
let path = directory.path().join("workspace.db");
|
||||
let host = RemoteHost {
|
||||
id: "host-1".into(),
|
||||
label: "Build box".into(),
|
||||
hostname: "build.example".into(),
|
||||
username: None,
|
||||
port: None,
|
||||
transport: RemoteTransport::OpenSsh,
|
||||
};
|
||||
{
|
||||
let store = Store::open(&path).expect("a fresh store");
|
||||
assert_eq!(
|
||||
store.schema_version().expect("version"),
|
||||
SUPPORTED_SCHEMA_VERSION
|
||||
);
|
||||
store.save_remote_host(&host).expect("save");
|
||||
}
|
||||
let store = Store::open(&path).expect("reopen");
|
||||
assert_eq!(
|
||||
store.schema_version().expect("version"),
|
||||
SUPPORTED_SCHEMA_VERSION
|
||||
);
|
||||
assert_eq!(
|
||||
store.remote_host("host-1").expect("host should load"),
|
||||
Some(host),
|
||||
"the data survives a reopen"
|
||||
);
|
||||
}
|
||||
|
||||
/// A database with no stamp at all is a fresh one, not a corrupt one.
|
||||
#[test]
|
||||
fn a_database_with_no_stamp_is_created_from_the_baseline() {
|
||||
let store = Store::open_in_memory().expect("an in-memory store");
|
||||
assert_eq!(
|
||||
store.schema_version().expect("version"),
|
||||
SUPPORTED_SCHEMA_VERSION
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user