ENT-14297: Improve error message when cf-agent cannot write to an immutable file

cf-agent/verify_files.c

@@ -835,7 +835,29 @@ static PromiseResult WriteContentFromString(EvalContext *ctx, const char *path,
         FILE *f = safe_fopen(override_path, "w");
         if (f == NULL)
         {
-            RecordFailure(ctx, pp, attr, "Cannot open file '%s' for writing", path);
+            const int saved_errno = errno;
+
+            /* Always report the actual system error. When not overriding the
+             * immutable bit we write in place, so also flag it if the file is
+             * immutable - confirmed via the flag, not guessed from errno (which
+             * is EPERM but not exclusive to immutability), so other causes are
+             * not mislabeled. When overriding we write to a temporary copy, so
+             * the immutable bit is not the cause. */
+            bool is_immutable = false;
+            if (!override_immutable
+                && (FSAttrsGetImmutableFlag(changes_path, &is_immutable) == FS_ATTRS_SUCCESS)
+                && is_immutable)
+            {
+                RecordFailure(ctx, pp, attr,
+                              "Cannot open file '%s' for writing: %s (the immutable bit is set)",
+                              path, GetErrorStrFromCode(saved_errno));
+            }
+            else
+            {
+                RecordFailure(ctx, pp, attr,
+                              "Cannot open file '%s' for writing: %s",
+                              path, GetErrorStrFromCode(saved_errno));
+            }
             OverrideImmutableCommit(changes_path, override_path, override_immutable, true);
             return PromiseResultUpdate(result, PROMISE_RESULT_FAIL);
         }

libpromises/override_fsattrs.c

@@ -211,11 +211,33 @@ bool OverrideImmutableRename(
 
     if (rename(old_filename, new_filename) == -1)
     {
-        Log(LOG_LEVEL_ERR,
-            "Failed to replace original file '%s' with copy '%s': %s",
-            new_filename,
-            old_filename,
-            GetErrorStr());
+        const int saved_errno = errno;
+
+        /* This is the common landing point for edit_line, edit_xml, copy_from
+         * and templating (they write a temporary file and rename it into
+         * place). When not overriding the immutable bit, an immutable
+         * destination is the likely reason the rename failed, so flag it -
+         * confirmed via the flag, not guessed from errno, so other causes are
+         * not mislabeled. */
+        bool is_immutable = false;
+        if (!override
+            && (FSAttrsGetImmutableFlag(new_filename, &is_immutable) == FS_ATTRS_SUCCESS)
+            && is_immutable)
+        {
+            Log(LOG_LEVEL_ERR,
+                "Failed to replace original file '%s' with copy '%s': %s (the immutable bit is set)",
+                new_filename,
+                old_filename,
+                GetErrorStrFromCode(saved_errno));
+        }
+        else
+        {
+            Log(LOG_LEVEL_ERR,
+                "Failed to replace original file '%s' with copy '%s': %s",
+                new_filename,
+                old_filename,
+                GetErrorStrFromCode(saved_errno));
+        }
         unlink(old_filename);
         return false;
     }

tests/acceptance/10_files/unsafe/14_immutable_content.cf

@@ -0,0 +1,73 @@
+##############################################################################
+#
+# Test that agent reports the immutable bit in the error message when it
+# cannot write to an immutable file (content attribute, no override).
+#
+# The whole scenario (create the file, set it immutable, then fail to write it)
+# runs in a self-contained sub policy: its setup happens in its own agent
+# process, so it does not depend on this test's evaluation order, and its
+# non-zero exit does not fail this test. We only assert on the captured error
+# output. Teardown clears the immutable bit natively via body fsattrs.
+#
+##############################################################################
+body common control
+{
+  inputs => { "../../default.sub.cf" };
+  bundlesequence => { default("$(this.promise_filename)") };
+  version => "1.0";
+}
+
+bundle agent global
+{
+  vars:
+    "testfile" string => "/tmp/14_immutable.txt";
+}
+
+body fsattrs clear_immutable
+{
+  immutable => "false";
+}
+
+bundle agent test
+{
+  meta:
+    "description" -> { "ENT-14297" }
+      string => "Test that the error message names the immutable bit when agent cannot write to an immutable file";
+
+    "test_skip_unsupported" string => "hpux|aix|solaris|windows";
+}
+
+bundle agent check
+{
+  vars:
+    "run"
+      data => execresult_as_data(
+        "$(sys.cf_agent) -KI -f $(with)", "noshell", "both"
+      ),
+      with => regex_replace("$(this.promise_filename)", "\.cf$", ".sub.cf", "");
+
+  classes:
+    "ok"
+      expression => regcmp(
+        ".*Cannot open file.*the immutable bit is set.*", "$(run[output])"
+      );
+
+  reports:
+    ok::
+      "$(this.promise_filename) Pass";
+
+    !ok::
+      "$(this.promise_filename) FAIL";
+
+    DEBUG::
+      "$(this.bundle): agent exited '$(run[exit_code])', output was:";
+      "$(run[output])";
+}
+
+bundle agent destroy
+{
+  files:
+    "$(global.testfile)"
+      delete => tidy,
+      fsattrs => clear_immutable;
+}

tests/acceptance/10_files/unsafe/14_immutable_content.sub.cf

@@ -0,0 +1,30 @@
+body common control
+{
+  bundlesequence => { "setup", "try_modify" };
+}
+
+body fsattrs set_immutable
+{
+  immutable => "true";
+}
+
+bundle agent setup
+{
+  files:
+    # Create the target and set the immutable bit natively. This runs in its
+    # own agent process (invoked from the test's check bundle), so setup is
+    # ordered before the write below and does not depend on the parent test.
+    "/tmp/14_immutable.txt"
+      content => "original content",
+      fsattrs => set_immutable;
+}
+
+bundle agent try_modify
+{
+  files:
+    # No fsattrs => ... immutable, so the immutable bit is not overridden.
+    # The content differs from what setup wrote, so the agent tries to open the
+    # immutable file for writing and fails.
+    "/tmp/14_immutable.txt"
+      content => "the agent tries to modify an immutable file";
+}

tests/acceptance/10_files/unsafe/15_immutable_edit_line.cf

@@ -0,0 +1,77 @@
+##############################################################################
+#
+# Test that agent reports the immutable bit in the error message when it
+# cannot write to an immutable file via edit_line (no override).
+#
+# edit_line writes a temporary file and renames it over the target, so the
+# failure surfaces from OverrideImmutableRename(). The whole scenario (create
+# the file, set it immutable, then fail to edit it) runs in a self-contained
+# sub policy: its setup happens in its own agent process, so it does not depend
+# on this test's evaluation order, and its non-zero exit does not fail this
+# test. We only assert on the captured error output. Teardown clears the
+# immutable bit natively via body fsattrs.
+#
+##############################################################################
+body common control
+{
+  inputs => { "../../default.sub.cf" };
+  bundlesequence => { default("$(this.promise_filename)") };
+  version => "1.0";
+}
+
+bundle agent global
+{
+  vars:
+    "testfile" string => "/tmp/15_immutable.txt";
+}
+
+body fsattrs clear_immutable
+{
+  immutable => "false";
+}
+
+bundle agent test
+{
+  meta:
+    "description" -> { "ENT-14297" }
+      string => "Test that edit_line on an immutable file names the immutable bit in the error";
+
+    "test_skip_unsupported" string => "hpux|aix|solaris|windows";
+}
+
+bundle agent check
+{
+  vars:
+    "run"
+      data => execresult_as_data(
+        "$(sys.cf_agent) -KI -f $(with)", "noshell", "both"
+      ),
+      with => regex_replace("$(this.promise_filename)", "\.cf$", ".sub.cf", "");
+
+  classes:
+    "ok"
+      expression => regcmp(
+        ".*Failed to replace.*the immutable bit is set.*", "$(run[output])"
+      );
+
+  reports:
+    ok::
+      "$(this.promise_filename) Pass";
+
+    !ok::
+      "$(this.promise_filename) FAIL";
+
+    DEBUG::
+      "$(this.bundle): agent exited '$(run[exit_code])', output was:";
+      "$(run[output])";
+}
+
+bundle agent destroy
+{
+  files:
+    "$(global.testfile)"
+      delete => tidy,
+      fsattrs => clear_immutable;
+
+    "$(global.testfile).cf-before-edit" delete => tidy;
+}

tests/acceptance/10_files/unsafe/15_immutable_edit_line.sub.cf

@@ -0,0 +1,35 @@
+body common control
+{
+  bundlesequence => { "setup", "try_modify" };
+}
+
+body fsattrs set_immutable
+{
+  immutable => "true";
+}
+
+bundle edit_line insert_a_line
+{
+  insert_lines:
+    "a line the agent tries to insert into an immutable file";
+}
+
+bundle agent setup
+{
+  files:
+    # Create the target and set the immutable bit natively. This runs in its
+    # own agent process (invoked from the test's check bundle), so setup is
+    # ordered before the edit below and does not depend on the parent test.
+    "/tmp/15_immutable.txt"
+      content => "original line",
+      fsattrs => set_immutable;
+}
+
+bundle agent try_modify
+{
+  files:
+    # No fsattrs => ... immutable, so the immutable bit is not overridden.
+    # Inserting a new line changes the file, so the agent writes a temporary
+    # file and renames it over the immutable target, which fails.
+    "/tmp/15_immutable.txt" edit_line => insert_a_line;
+}

tests/acceptance/10_files/unsafe/16_immutable_copy_from.cf

@@ -0,0 +1,78 @@
+##############################################################################
+#
+# Test that agent reports the immutable bit in the error message when it
+# cannot write to an immutable file via copy_from (no override).
+#
+# copy_from writes a temporary file and renames it over the target, so the
+# failure surfaces from OverrideImmutableRename(). The whole scenario (create
+# source + immutable destination, then fail to copy) runs in a self-contained
+# sub policy: its setup happens in its own agent process, so it does not depend
+# on this test's evaluation order, and its non-zero exit does not fail this
+# test. We only assert on the captured error output. Teardown clears the
+# immutable bit natively via body fsattrs.
+#
+##############################################################################
+body common control
+{
+  inputs => { "../../default.sub.cf" };
+  bundlesequence => { default("$(this.promise_filename)") };
+  version => "1.0";
+}
+
+bundle agent global
+{
+  vars:
+    "source" string => "/tmp/16_immutable.src";
+    "testfile" string => "/tmp/16_immutable.txt";
+}
+
+body fsattrs clear_immutable
+{
+  immutable => "false";
+}
+
+bundle agent test
+{
+  meta:
+    "description" -> { "ENT-14297" }
+      string => "Test that copy_from onto an immutable file names the immutable bit in the error";
+
+    "test_skip_unsupported" string => "hpux|aix|solaris|windows";
+}
+
+bundle agent check
+{
+  vars:
+    "run"
+      data => execresult_as_data(
+        "$(sys.cf_agent) -KI -f $(with)", "noshell", "both"
+      ),
+      with => regex_replace("$(this.promise_filename)", "\.cf$", ".sub.cf", "");
+
+  classes:
+    "ok"
+      expression => regcmp(
+        ".*Failed to replace.*the immutable bit is set.*", "$(run[output])"
+      );
+
+  reports:
+    ok::
+      "$(this.promise_filename) Pass";
+
+    !ok::
+      "$(this.promise_filename) FAIL";
+
+    DEBUG::
+      "$(this.bundle): agent exited '$(run[exit_code])', output was:";
+      "$(run[output])";
+}
+
+bundle agent destroy
+{
+  files:
+    "$(global.testfile)"
+      delete => tidy,
+      fsattrs => clear_immutable;
+
+    "$(global.source)" delete => tidy;
+}