Hotfix 4.3.1

book/src/cli/chsr.md

@@ -290,7 +290,7 @@ Read the resulting configuration before you answer yes: what <code>chsr</code> s
 
 ## `chsr upgrade`
 
-A new version of RootAsRole may change how the policy is written. After an upgrade, `chsr upgrade` rewrites the policy files in the format of the installed version. Packages run it for you during their updates.
+A new version of RootAsRole may change how the policy is written. After an upgrade, `chsr upgrade` might do necessary migrations.
 
 ```bash
 chsr upgrade --check
@@ -304,8 +304,6 @@ Going back to an older version needs the same care, the other way. Before instal
 chsr upgrade --to 4.1.0
 ```
 
-`chsr` writes every policy file for that version, and changes what that version would read with another meaning. With `--check`, it writes nothing. For a version older than 4.1.0, the installed `dosr` refuses the files written this way until the older version is installed.
-
 ## File Format Conversion
 
 `chsr convert` converts the policy between JSON and CBOR. JSON is easy to read and to review. CBOR is a binary format, much faster to load: when your policy is ready, you can convert it. In CBOR, `chsr` writes each key of the policy with one character, which makes the file smaller. `dosr` and `chsr` read both the short and the long keys, in both formats, but `dosr` 4.2.0 does not: before going back to it with a CBOR policy, run `chsr upgrade --to 4.2.0`.

book/src/cli/dosr.md

@@ -197,7 +197,7 @@ Today, the CLI of `dosr` follows the one of sudo, to keep old scripts working. W
 
 `Permission denied` --> Two equivalent tasks match: the policy must be fixed. The log has the details.
 
-`Configuration error` --> The policy was refused: for example a file written by hand without its `version`, or written by a version older than 4.1.0 that `chsr upgrade` did not migrate. `dosr` never shows on the terminal what is wrong in the policy, but it says where to read it: ``dosr: see the reason with `journalctl -t dosr -p info`, or ask your system administrator``. The same line follows any error when a file of the policy was refused, like a file of a policy directory that other users can write. The command comes from the build of `dosr` (`RAR_LOG_COMMAND`, see [Build-time default configuration](../install/build.md)).
+`Configuration error` --> The policy was refused: for example a file written by hand without its `version`. `dosr` never shows on the terminal what is wrong in the policy, but it says where to read it: ``dosr: see the reason with `journalctl -t dosr -p info`, or ask your system administrator``. The same line follows any error when a file of the policy was refused, like a file of a policy directory that other users can write. The command comes from the build of `dosr` (`RAR_LOG_COMMAND`, see [Build-time default configuration](../install/build.md)).
 
 `dosr: the task needs capabilities that this session cannot obtain anymore: …` --> The task needs capabilities that are out of the bounding set of your session, for example in a container.
 

book/src/configuration.md

@@ -569,7 +569,8 @@ For `sys-admin`, the most critical role, the team requires authentication on eve
 
 ### Migrating configurations from version 4.0
 
-Upgrading from version 4.0 requires running `chsr upgrade` as root. Package maintenance scripts run it automatically during updates.
+Upgrading from breaking changes version requires running `chsr upgrade`. 
+Package maintenance scripts run it automatically during updates, but if you ran the package upgrade with `dosr`, you might not have `CAP_LINUX_IMMUTABLE`, blocking the file modification. In that case, run `dosr chsr upgrade` manually to migrate the configuration. We'll try to avoid breaking changes in the future, but security-default changes are sometimes unavoidable.
 
 This utility addresses differences in pattern syntax and list inheritance:
 

core/src/database/migrations.rs

@@ -542,14 +542,38 @@ fn describe_changes(before: &Value, after: &Value, pointer: &str, changes: &mut
     }
 }
 
-/// Migrate a raw policy document to the current version, before it is deserialized.
+/// Migrate a raw policy document written by an older version, before it is deserialized: to
+/// [`FORMAT_VERSION`], the version that chsr writes.
 ///
-/// A document without a parsable version, or already at the current version, is left unchanged.
+/// A document without a parsable version, or one that the current version reads as it is (see
+/// [`reads_as_is`]), is left unchanged, its version included: no migration would change it.
 /// Returns what the migration changed, if the document was migrated.
 /// # Errors
 /// Returns an error if a migration fails or if there is no migration path to the current version.
 pub fn migrate_document(document: &mut Value) -> Result<Option<DocumentMigration>, MigrationError> {
-    migrate_document_to(document, &PACKAGE_VERSION)
+    if document_version(document).is_some_and(|version| reads_as_is(&version)) {
+        return Ok(None);
+    }
+    let migration = migrate_document_to(document, &PACKAGE_VERSION)?;
+    // the migrations after FORMAT_VERSION change nothing
+    if migration.is_some()
+        && let Value::Object(map) = document
+    {
+        map.insert(
+            "version".to_string(),
+            Value::String(FORMAT_VERSION.to_string()),
+        );
+    }
+    Ok(migration)
+}
+
+/// The version of a raw policy document, under its long or its short key.
+fn document_version(document: &Value) -> Option<Version> {
+    document
+        .get("version")
+        .or_else(|| document.get("v"))
+        .and_then(Value::as_str)
+        .and_then(|version| Version::parse(version).ok())
 }
 
 /// Migrate a raw policy document to the version `to`, newer or older than its own.
@@ -562,12 +586,7 @@ pub fn migrate_document_to(
     document: &mut Value,
     to: &Version,
 ) -> Result<Option<DocumentMigration>, MigrationError> {
-    let Some(version) = document
-        .get("version")
-        .or_else(|| document.get("v"))
-        .and_then(Value::as_str)
-        .and_then(|version| Version::parse(version).ok())
-    else {
+    let Some(version) = document_version(document) else {
         return Ok(None);
     };
     if version == *to {
@@ -614,6 +633,15 @@ pub fn writable_version(text: &str) -> Result<Version, String> {
 /// migration that rewrites the documents on the way up raises it to its `to`.
 pub const UNCHANGED_SINCE: Version = Version::new(4, 1, 0);
 
+/// The version that chsr writes in a policy file that it saves: the last version whose migration
+/// changes the documents, on the way up or down.
+///
+/// The versions from it to the current one read and write the same documents: chsr does not
+/// rewrite a file for the current version alone, and a package of one of these versions reads
+/// what a later one wrote, a downgrade among them included. A migration that changes the
+/// documents, in either direction, raises it to its `to`.
+pub const FORMAT_VERSION: Version = Version::new(4, 3, 0);
+
 /// Why the current version does not read a policy document as it is.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum UnreadableVersion {
@@ -662,15 +690,25 @@ pub fn read_as_is(version: Option<&str>) -> Result<(), UnreadableVersion> {
         .map_err(|_| UnreadableVersion::Invalid(text.escape_debug().to_string()))?;
     // the build metadata does not change the format
     version.build = semver::BuildMetadata::EMPTY;
-    if version < UNCHANGED_SINCE {
+    if reads_as_is(&version) {
+        Ok(())
+    } else if version < UNCHANGED_SINCE {
         Err(UnreadableVersion::Older(version))
-    } else if version > PACKAGE_VERSION {
-        Err(UnreadableVersion::Newer(version))
     } else {
-        Ok(())
+        Err(UnreadableVersion::Newer(version))
     }
 }
 
+/// Whether the current version reads a document of `version` as it is: from [`UNCHANGED_SINCE`]
+/// to the current version. dosr reads it, and chsr does not rewrite it, as it is.
+#[must_use]
+pub fn reads_as_is(version: &Version) -> bool {
+    let mut version = version.clone();
+    // the build metadata does not change the format
+    version.build = semver::BuildMetadata::EMPTY;
+    UNCHANGED_SINCE <= version && version <= PACKAGE_VERSION
+}
+
 #[cfg(test)]
 mod tests {
     use serde_json::json;
@@ -714,7 +752,8 @@ mod tests {
                 r#"/roles/0/tasks/0/options/env/delete/0: "XDG_.*_DIR" -> "XDG_*_DIR""#,
             ]
         );
-        assert_eq!(document["version"], PACKAGE_VERSION.to_string());
+        // written for the format, which the current version reads
+        assert_eq!(document["version"], FORMAT_VERSION.to_string());
         assert_eq!(document["options"]["env"]["check"], json!(["LC_*", "TZ"]));
         assert_eq!(document["options"]["env"]["keep"], json!(["HOME"]));
         // values of the set list are not patterns
@@ -1257,8 +1296,13 @@ mod tests {
             "options": {"env": {"default": "delete", "check": ["LC_*"]}, "path": {"default": "keep-safe"}},
             "roles": [{"name": "r", "tasks": [{"name": "t", "commands": ["/usr/bin/ls ^.*$"]}]}]
         });
+        // the current version reads it as it is: chsr does not rewrite it
         let mut document = original.clone();
-        let migration = migrate_document(&mut document).unwrap().unwrap();
+        assert_eq!(migrate_document(&mut document).unwrap(), None);
+        assert_eq!(document, original);
+        let migration = migrate_document_to(&mut document, &PACKAGE_VERSION)
+            .unwrap()
+            .unwrap();
         assert_eq!(migration.from, Version::new(4, 1, 0));
         assert_eq!(document["version"], PACKAGE_VERSION.to_string());
         let mut unchanged = original.clone();
@@ -1287,8 +1331,13 @@ mod tests {
             "options": {"password": {"tries": 2}, "tty": {"owner": "target"}},
             "roles": [{"name": "r", "tasks": [{"name": "t", "cred": {"setgid": {"from": "caller"}}, "commands": ["/usr/bin/ls ^a|b$"]}]}]
         });
+        // the current version reads it as it is: chsr does not rewrite it
         let mut document = original.clone();
-        let migration = migrate_document(&mut document).unwrap().unwrap();
+        assert_eq!(migrate_document(&mut document).unwrap(), None);
+        assert_eq!(document, original);
+        let migration = migrate_document_to(&mut document, &PACKAGE_VERSION)
+            .unwrap()
+            .unwrap();
         assert_eq!(migration.from, Version::new(4, 2, 0));
         assert_eq!(document["version"], PACKAGE_VERSION.to_string());
         let mut unchanged = original.clone();
@@ -1309,21 +1358,30 @@ mod tests {
         assert_eq!(document, original);
     }
 
-    // 4.3.1 only changes how dosr reads an older policy, not the format
+    // the versions from FORMAT_VERSION to the current one read and write the same documents: a
+    // document of one of them is not migrated, and goes from one to another unchanged
     #[test]
-    fn test_4_3_0_documents_up_and_down() {
+    fn test_format_version() {
+        assert!(UNCHANGED_SINCE <= FORMAT_VERSION && FORMAT_VERSION <= PACKAGE_VERSION);
         let original = json!({
-            "version": "4.3.0",
-            "options": {"env": {"default": "delete", "keep": ["HOME"]}, "password": {"tries": 2}, "tty": {"owner": "target"}},
-            "roles": [{"name": "r", "tasks": [{"name": "t", "cred": {"setgid": {"from": "caller"}}, "commands": ["/usr/bin/ls"]}]}]
+            "version": FORMAT_VERSION.to_string(),
+            "o": {"e": {"d": "delete", "k": ["HOME"]}},
+            "options": {"password": {"tries": 2}, "tty": {"owner": "target"}, "path": {"default": "keep-safe"}},
+            "roles": [{"name": "r", "tasks": [{"name": "t", "cred": {"setgid": {"from": "caller"}}, "commands": ["/usr/bin/ls ^a|b$", "/usr/bin/cat"]}]}]
         });
+        for version in [FORMAT_VERSION, PACKAGE_VERSION] {
+            let mut document = original.clone();
+            document["version"] = json!(version.to_string());
+            let read = document.clone();
+            assert_eq!(migrate_document(&mut document).unwrap(), None, "{version}");
+            assert_eq!(document, read, "{version}");
+        }
         let mut document = original.clone();
-        let migration = migrate_document(&mut document).unwrap().unwrap();
-        assert_eq!(migration.from, Version::new(4, 3, 0));
-        assert!(migration.changes.is_empty(), "{:?}", migration.changes);
-        assert_eq!(document["version"], PACKAGE_VERSION.to_string());
-        migrate_document_to(&mut document, &Version::new(4, 3, 0)).unwrap();
+        migrate_document_to(&mut document, &PACKAGE_VERSION).unwrap();
+        migrate_document_to(&mut document, &FORMAT_VERSION).unwrap();
         assert_eq!(document, original);
+        // the data that chsr creates or saves is written for it
+        assert_eq!(Versioning::new(()).version, FORMAT_VERSION);
     }
 
     #[test]

core/src/database/versionning.rs

@@ -3,9 +3,9 @@ use semver::Version;
 use serde::{Deserialize, Serialize, Serializer, ser::SerializeMap};
 use std::{cell::RefCell, fmt::Debug, rc::Rc};
 
-use crate::{
-    PACKAGE_VERSION,
-    database::migration::{Migration, MigrationError},
+use crate::database::{
+    migration::{Migration, MigrationError},
+    migrations::{FORMAT_VERSION, reads_as_is},
 };
 
 #[derive(Deserialize, Debug, Clone)]
@@ -62,7 +62,7 @@ impl<T: VersionedData> Serialize for Versioning<T> {
 impl<T> Versioning<T> {
     pub const fn new(data: T) -> Self {
         Self {
-            version: PACKAGE_VERSION,
+            version: FORMAT_VERSION,
             data,
         }
     }
@@ -71,7 +71,7 @@ impl<T> Versioning<T> {
 impl<T: Default> Default for Versioning<T> {
     fn default() -> Self {
         Self {
-            version: PACKAGE_VERSION,
+            version: FORMAT_VERSION,
             data: T::default(),
         }
     }
@@ -81,8 +81,12 @@ impl<T> Versioning<T> {
     /// # Errors
     /// Returns an error if the migration process fails.
     pub fn upgrade_version(&mut self, migrations: &[Migration<T>]) -> Result<bool, MigrationError> {
+        // read as it is: it keeps its version until it is saved, written for FORMAT_VERSION
+        if reads_as_is(&self.version) {
+            return Ok(false);
+        }
         let res = Migration::migrate(&self.version, &mut self.data, migrations)?;
-        self.version = PACKAGE_VERSION;
+        self.version = FORMAT_VERSION;
         Ok(res)
     }
 }

core/src/file.rs

@@ -21,7 +21,10 @@ use crate::{
     PACKAGE_VERSION, SettingsContent,
     database::{
         migration::{Migration, MigrationError},
-        migrations::{DocumentMigration, migrate_document, migrate_document_to, writable_version},
+        migrations::{
+            DocumentMigration, FORMAT_VERSION, migrate_document, migrate_document_to,
+            writable_version,
+        },
         ser::field,
         structs::SPolicy,
         versionning::{VersionedData, Versioning},
@@ -630,9 +633,15 @@ impl FileSettings {
         //
         // only the files that changed: the others are not rewritten (in the typed form of the
         // actors, without their immutable flag for a while...)
+        //
+        // a file that is written gets the version of the format, which its content follows; the
+        // others keep theirs
         let mut has_errors = if !self.root.changed() {
             false
-        } else if let Err(e) = self.root.save(RAR_CFG_TYPE, immutable) {
+        } else if let Err(e) = {
+            self.root.data.version = FORMAT_VERSION;
+            self.root.save(RAR_CFG_TYPE, immutable)
+        } {
             error!("Failed to save root settings: {e}");
             true
         } else {
@@ -641,6 +650,7 @@ impl FileSettings {
         };
 
         for config in self.map.iter_mut().filter(|config| config.changed()) {
+            config.data.version = FORMAT_VERSION;
             if let Err(e) = config.save(self.root.data.data.storage.method, immutable) {
                 error!(
                     "Failed to save settings for {}: {}",
@@ -713,8 +723,12 @@ impl FileSettings {
     ) -> FileDowngrade {
         let mut write = || -> Result<DocumentMigration, Box<dyn Error>> {
             let mut document = file.document(method)?;
+            // its version is known, the one of the loaded data: only already `to`
             let Some(migration) = migrate_document_to(&mut document, to)? else {
-                return Err(format!("its version is not known, or is already {to}").into());
+                return Ok(DocumentMigration {
+                    from: to.clone(),
+                    changes: Vec::new(),
+                });
             };
             if !check {
                 file.save_document(&document, method, immutable)?;
@@ -728,6 +742,12 @@ impl FileSettings {
         }
     }
 
+    /// Whether a loaded file was migrated, and is to be saved by `save_migrated`.
+    #[must_use]
+    pub fn has_migration(&self) -> bool {
+        self.root.migration.is_some() || self.map.iter().any(|config| config.migration.is_some())
+    }
+
     /// The version of each loaded file, root settings first.
     #[must_use]
     pub fn get_versions(&self) -> Vec<FileVersion<'_>> {
@@ -861,7 +881,8 @@ mod tests {
         )
         .unwrap();
         let policy = FileSettings::read_policy(&path, StorageMethod::JSON).unwrap();
-        assert_eq!(policy.data.version, PACKAGE_VERSION);
+        // migrated for the format, which the current version reads
+        assert_eq!(policy.data.version, FORMAT_VERSION);
         let options = policy.data.data.as_ref().borrow().options.clone().unwrap();
         let check = options
             .as_ref()
@@ -1454,7 +1475,7 @@ mod tests {
         file.read_to_end(&mut content).unwrap();
         let deserialized: Versioning<Rc<RefCell<SPolicy>>> =
             cbor4ii::serde::from_reader(&content[..]).unwrap();
-        assert_eq!(deserialized.version, PACKAGE_VERSION);
+        assert_eq!(deserialized.version, FORMAT_VERSION);
     }
 
     #[test]

src/chsr/cli/editor.rs

@@ -10,7 +10,9 @@ use log::{debug, warn};
 use rootasrole_core::{
     PACKAGE_VERSION,
     database::{
-        migrations::{DocumentMigration, UnreadableVersion, migrate_document, read_as_is},
+        migrations::{
+            DocumentMigration, FORMAT_VERSION, UnreadableVersion, migrate_document, read_as_is,
+        },
         structs::SPolicy,
         warn::Warn,
     },
@@ -326,7 +328,7 @@ where
                 if let Some(migration) = migration {
                     writeln!(
                         output,
-                        "Written for RootAsRole {}, migrated to {PACKAGE_VERSION}:",
+                        "Written for RootAsRole {}, migrated to {FORMAT_VERSION}:",
                         migration.from
                     )?;
                     // the values are JSON, and the keys escaped
@@ -888,9 +890,11 @@ echo '{ "storage": { "method": "json" }, "unknown_config_field": "foo" }' > "$fi
             &serde_json::json!({"version": "4.0.3", "options": {"env": {"check": ["LC_.*"]}}}),
             b"y\n",
         );
+        // written for the format, which this version reads
+        let format = rootasrole_core::database::migrations::FORMAT_VERSION.to_string();
         assert!(
             output.contains(&format!(
-                "Written for RootAsRole 4.0.3, migrated to {version}:"
+                "Written for RootAsRole 4.0.3, migrated to {format}:"
             )),
             "{output}"
         );
@@ -899,7 +903,7 @@ echo '{ "storage": { "method": "json" }, "unknown_config_field": "foo" }' > "$fi
             "{output}"
         );
         let saved: Value = serde_json::from_str(&saved).unwrap();
-        assert_eq!(saved["version"], version.as_str());
+        assert_eq!(saved["version"], format.as_str());
         assert_eq!(
             saved["options"]["env"]["check"],
             serde_json::json!(["LC_*"])