Fixed usemodule() reporting success when the module exited non-zero

usemodule() ignored the module's exit status and defined its class either way.
It returns false now when the module ran and failed, and only fails as a
function when the module could not be run or read at all. The distinction
matters: a failed function call leaves the class undefined rather than false,
so "not => usemodule(...)" would not have become true either.

The test could not pass or fail whatever the code did. It looked for the modules
one directory too high, its Pass guard asked for a class and its own negation,
and the module it runs exits 1 on purpose which failed the whole test bundle.
With those corrected it fails on the old code and passes on the new.

Ticket: CFE-942
Changelog: Title
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit a5b28ff)

Author: nickanderson

libpromises/evalfunction.c

@@ -100,7 +100,7 @@ static char *StripPatterns(char *file_buffer, const char *pattern, const char *f
 static int BuildLineArray(EvalContext *ctx, const Bundle *bundle, const char *array_lval, const char *file_buffer,
                           const char *split, int maxent, DataType type, bool int_index);
 static JsonElement* BuildData(EvalContext *ctx, const char *file_buffer,  const char *split, int maxent, bool make_array);
-static bool ExecModule(EvalContext *ctx, char *command);
+static bool ExecModule(EvalContext *ctx, char *command, int *retcode);
 
 static bool CheckIDChar(const char ch);
 static bool CheckID(const char *id);
@@ -2574,12 +2574,15 @@ static FnCallResult FnCallUseModule(EvalContext *ctx,
 
     Log(LOG_LEVEL_VERBOSE, "Executing and using module [%s]", modulecmd);
 
-    if (!ExecModule(ctx, modulecmd))
+    /* A module which exits non-zero has not told us anything we should act on,
+     * so the function is false. Not being able to run it at all is a failure. */
+    int retcode = 0;
+    if (!ExecModule(ctx, modulecmd, &retcode))
     {
         return FnFailure();
     }
 
-    return FnReturnContext(true);
+    return FnReturnContext(retcode == 0);
 }
 
 /*********************************************************************/
@@ -8843,7 +8846,7 @@ static FnCallResult FnCallFindfilesUp(ARG_UNUSED EvalContext *ctx, ARG_UNUSED co
 
 /*********************************************************************/
 
-static bool ExecModule(EvalContext *ctx, char *command)
+static bool ExecModule(EvalContext *ctx, char *command, int *retcode)
 {
     FILE *pp = cf_popen(command, "rt", true);
     if (!pp)
@@ -8874,7 +8877,7 @@ static bool ExecModule(EvalContext *ctx, char *command)
         ModuleProtocol(ctx, command, line, print, context, sizeof(context), tags, &persistence);
     }
     bool atend = feof(pp);
-    cf_pclose(pp);
+    *retcode = cf_pclose(pp);
     free(line);
     StringSetDestroy(tags);
 
@@ -8884,6 +8887,11 @@ static bool ExecModule(EvalContext *ctx, char *command)
         return false;
     }
 
+    if (*retcode != 0)
+    {
+        Log(LOG_LEVEL_ERR, "Module '%s' returned non-zero exit code %d", command, *retcode);
+    }
+
     return true;
 }
 

tests/acceptance/02_classes/02_functions/usemodule-returns-nonzero.cf

@@ -42,10 +42,8 @@ bundle agent test
     "description" -> { "CFE-942" }
       string => "Test that when a module executed by usemodule() returns nonzero, it's not interpreted as successful";
 
-      "test_soft_fail"
-        string => "any",
-        meta => { "CFE-942" },
-        comment => "usemodule seems to return true no matter if the module exists returning 0 or nonzero";
+      # The modules this test writes are /bin/sh scripts
+      "test_skip_unsupported" string => "windows";
 
   classes:
 
@@ -54,26 +52,35 @@ bundle agent test
       "usemodule_expect_no_class_defined_because_return_nonzero"
         expression => usemodule( "foo-usemodule", "" ),
         scope => "namespace",
-        if => isexecutable( "$(sys.workdir)/foo-usemodule" );
+        if => isexecutable( "$(sys.workdir)/modules/foo-usemodule" );
 
       # Since the module exists non zero, we should get this class
       "usemodule_expect_class_defined_because_return_nonzero"
         not => usemodule( "foo-usemodule", "" ),
         scope => "namespace",
-        if => isexecutable( "$(sys.workdir)/foo-usemodule" );
+        if => isexecutable( "$(sys.workdir)/modules/foo-usemodule" );
 
   commands:
       "$(sys.workdir)/modules/foo-commands_module"
         module => "true",
+        classes => expected_nonzero_exit,
         if => isexecutable( $(this.promiser) );
 }
+
+body classes expected_nonzero_exit
+# @brief The module here exits non-zero on purpose, so don't fail the promise
+# over it and take the whole test bundle down with it
+{
+      kept_returncodes => { "1" };
+}
+
 bundle agent check
 {
   methods:
     usemodule_expect_no_class_defined_because_return_nonzero.!usemodule_expect_class_defined_because_return_nonzero::
       "FAIL" usebundle => dcs_fail( $(this.promise_filename) );
 
-    !usemodule_expect_class_defined_because_return_nonzero.usemodule_expect_class_defined_because_return_nonzero::
+    !usemodule_expect_no_class_defined_because_return_nonzero.usemodule_expect_class_defined_because_return_nonzero::
       "Pass" usebundle => dcs_pass( $(this.promise_filename) );
 
   reports: