From 6f4ae27ae46341134a0d49f1950070df6788b4be Mon Sep 17 00:00:00 2001
From: Nick Anderson <nick@cmdln.org>
Date: Wed, 29 Jul 2026 01:19:26 -0500
Subject: [PATCH] Fixed reports promise being kept when the report could not be
 written

A report which never made it to its file was still audited as kept and still
defined the _kept class. The promise fails now instead.

The test for this has been soft-failing since 2015. It makes the file immutable
with chattr, which needs root, so it also moves to an unsafe directory where
testall runs it as root.

Ticket: CFE-4699
Changelog: Title
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 libpromises/verify_reports.c                  | 29 ++++++++++++++-----
 .../failed_reports_are_not_kept.cf            | 12 ++++----
 2 files changed, 27 insertions(+), 14 deletions(-)
 rename tests/acceptance/14_reports/{ => unsafe}/failed_reports_are_not_kept.cf (80%)

diff --git a/libpromises/verify_reports.c b/libpromises/verify_reports.c
index e9fe09cf88..59b96c49d5 100644
--- a/libpromises/verify_reports.c
+++ b/libpromises/verify_reports.c
@@ -44,7 +44,7 @@
 #include <actuator.h>
 
 static bool PrintFile(const char *filename, ssize_t max_lines);
-static void ReportToFile(const char *logfile, const char *message);
+static bool ReportToFile(const char *logfile, const char *message);
 static void ReportToLog(const char *message);
 
 PromiseResult VerifyReportPromise(EvalContext *ctx, const Promise *pp)
@@ -109,16 +109,20 @@ PromiseResult VerifyReportPromise(EvalContext *ctx, const Promise *pp)
         return PROMISE_RESULT_WARN;
     }
 
+    PromiseResult result = PROMISE_RESULT_NOOP;
+
     if (a.report.to_file)
     {
-        ReportToFile(a.report.to_file, pp->promiser);
+        if (!ReportToFile(a.report.to_file, pp->promiser))
+        {
+            result = PromiseResultUpdate(result, PROMISE_RESULT_FAIL);
+        }
     }
     else
     {
         ReportToLog(pp->promiser);
     }
 
-    PromiseResult result = PROMISE_RESULT_NOOP;
     if (a.report.haveprintfile)
     {
         if (!PrintFile(a.report.filename, a.report.numlines))
@@ -145,18 +149,29 @@ static void ReportToLog(const char *message)
     free(report_message);
 }
 
-static void ReportToFile(const char *logfile, const char *message)
+static bool ReportToFile(const char *logfile, const char *message)
 {
     FILE *fp = safe_fopen_create_perms(logfile, "a", CF_PERMS_DEFAULT);
     if (!fp)
     {
         Log(LOG_LEVEL_ERR, "Could not open log file '%s', message '%s'. (fopen: %s)", logfile, message, GetErrorStr());
+        return false;
     }
-    else
+
+    bool reported = (fprintf(fp, "%s\n", message) >= 0);
+    if (!reported)
+    {
+        Log(LOG_LEVEL_ERR, "Could not write to log file '%s', message '%s'. (fprintf: %s)", logfile, message, GetErrorStr());
+    }
+
+    /* The write may still be buffered, so a failed close loses the message too */
+    if (fclose(fp) != 0)
     {
-        fprintf(fp, "%s\n", message);
-        fclose(fp);
+        Log(LOG_LEVEL_ERR, "Could not close log file '%s', message '%s'. (fclose: %s)", logfile, message, GetErrorStr());
+        reported = false;
     }
+
+    return reported;
 }
 
 static bool PrintFile(const char *filename, ssize_t max_lines)
diff --git a/tests/acceptance/14_reports/failed_reports_are_not_kept.cf b/tests/acceptance/14_reports/unsafe/failed_reports_are_not_kept.cf
similarity index 80%
rename from tests/acceptance/14_reports/failed_reports_are_not_kept.cf
rename to tests/acceptance/14_reports/unsafe/failed_reports_are_not_kept.cf
index e02190f919..53ea2cd4f5 100644
--- a/tests/acceptance/14_reports/failed_reports_are_not_kept.cf
+++ b/tests/acceptance/14_reports/unsafe/failed_reports_are_not_kept.cf
@@ -1,7 +1,7 @@
 # Check that reports are printed just once (Redmine#3446 https://cfengine.com/dev/issues/3446)
 body common control
 {
-  inputs => { "../default.sub.cf" };
+  inputs => { "../../default.sub.cf" };
   bundlesequence => { default("$(this.promise_filename)") };
   version => "1.0";
 }
@@ -22,13 +22,11 @@ bundle agent init
 bundle agent test
 {
   meta:
-    "description"
-      string => "Test that we do not conisder failure to report to a file a kept outcome";
+    "description" -> { "CFE-4699", "redmine7833" }
+      string => "Test that we do not consider failure to report to a file a kept outcome";
 
-    # this test should be skipped on platforms that do not have chattr!
-    "test_soft_fail"
-      string => "any",
-      meta => { "redmine7833" };
+    # The test makes the file immutable with chattr to provoke the failure
+    "test_skip_unsupported" string => "hpux|aix|solaris|windows";
 
   reports:
     "Hello World"