From 232126bfb860498682b77baeb7aba3d5d47a8e1b Mon Sep 17 00:00:00 2001 From: Nick Anderson Date: Mon, 29 Jun 2026 15:16:29 -0500 Subject: [PATCH 1/2] test: add case-insensitive signal name tests (CFE-612) SignalFromString() currently uses case-sensitive comparison, so uppercase signal names like "TERM" or "KILL" return -1. These tests document the expected case-insensitive behavior. Ticket: CFE-612 Changelog: None --- .../01_matching/signals_case_insensitive.cf | 112 ++++++++++++++++++ tests/unit/conversion_test.c | 27 +++++ 2 files changed, 139 insertions(+) create mode 100644 tests/acceptance/05_processes/01_matching/signals_case_insensitive.cf diff --git a/tests/acceptance/05_processes/01_matching/signals_case_insensitive.cf b/tests/acceptance/05_processes/01_matching/signals_case_insensitive.cf new file mode 100644 index 0000000000..b3342225f6 --- /dev/null +++ b/tests/acceptance/05_processes/01_matching/signals_case_insensitive.cf @@ -0,0 +1,112 @@ +####################################################### +# +# CFE-612: processes "signals" accepts signal names case-insensitively. +# Signal a backgrounded victim with the UPPERCASE "TERM"; its SIGTERM +# trap records the delivered signal to a sentinel that check verifies. +# Pre-fix, uppercase fails policy validation. SIGKILL/-1 wouldn't fire +# the trap, so the sentinel also proves it was specifically SIGTERM. +# +####################################################### +body common control +{ + inputs => { "../../default.sub.cf" }; + bundlesequence => { default("$(this.promise_filename)") }; + version => "1.0"; +} + +####################################################### +body action background +{ + background => "true"; +} + +####################################################### +bundle agent init +{ + meta: + # Backgrounding + POSIX trap/wait + ps matching need a real POSIX shell. + "test_skip_needs_work" string => "!linux"; + + vars: + "victim" string => "$(G.testdir)/cfe612_signal_victim.sh"; + "sentinel" string => "$(G.testdir)/cfe612_signal_received"; + "ready" string => "$(G.testdir)/cfe612_signal_ready"; + "poll_script" string => "$(G.testdir)/cfe612_poll.sh"; + + files: + # Remove any artifacts left over from a previous run. + "$(sentinel)" delete => tidy; + "$(ready)" delete => tidy; + "$(poll_script)" delete => tidy; + + # Polling helper: wait up to ~10s for a file to become non-empty. + "$(poll_script)" + create => "true", + content => '#!/bin/sh +file="$1" +for _ in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100; do + [ -s "$file" ] && exit 0 + sleep 0.1 +done +exit 1'; + + # Victim: "sleep & wait" so the SIGTERM trap fires promptly; the trap + # records the signal then kills its sleep child (a lingering descendant + # would hold the harness's output pipe open and stall the run). 30s is + # just a safety bound so a never-signalled victim can't run away. + "$(victim)" + create => "true", + content => "#!/bin/sh +trap 'printf TERM > $(sentinel); kill $sleeppid 2>/dev/null; exit 0' TERM +printf ready > $(ready) +sleep 30 & +sleeppid=$! +wait +"; + + commands: + # Launch the victim, then poll until its ready-file appears (cap ~10s). + "$(G.sh) $(victim)" action => background; + "$(G.sh) $(poll_script) \"$(ready)\""; +} + +####################################################### +bundle agent test +{ + meta: + "description" -> { "CFE-612" } + string => "processes 'signals' attribute accepts signal names case-insensitively"; + + processes: + # UPPERCASE signal name; the unique token only matches our victim. + "cfe612_signal_victim" signals => { "TERM" }; + + commands: + # Poll for the sentinel becoming non-empty (cap ~10s). + # processes run before commands, so this runs after the signal. + "$(G.sh) $(init.poll_script) \"$(init.sentinel)\""; +} + +####################################################### +bundle agent check +{ + vars: + "expected" string => "TERM"; + + "actual" + string => readfile("$(init.sentinel)"), + if => fileexists("$(init.sentinel)"); + + classes: + "ok" expression => strcmp("$(expected)", "$(actual)"); + + reports: + DEBUG:: + "Expected sentinel '$(expected)', found '$(actual)'"; + + ok:: + "$(this.promise_filename) Pass"; + + !ok:: + "$(this.promise_filename) FAIL"; +} diff --git a/tests/unit/conversion_test.c b/tests/unit/conversion_test.c index 6767636cb1..323d72d4b2 100644 --- a/tests/unit/conversion_test.c +++ b/tests/unit/conversion_test.c @@ -2,6 +2,7 @@ #include #include +#include static void test_string_is_boolean(void) { @@ -180,6 +181,31 @@ static void test_double_from_string(void) assert_true(val == old_val); } +static void test_signal_from_string(void) +{ + /* Lowercase (baseline -- should always work) */ + assert_int_equal(SignalFromString("hup"), SIGHUP); + assert_int_equal(SignalFromString("int"), SIGINT); + assert_int_equal(SignalFromString("term"), SIGTERM); + assert_int_equal(SignalFromString("kill"), SIGKILL); + + /* Uppercase (CFE-612: should work but currently fails) */ + assert_int_equal(SignalFromString("HUP"), SIGHUP); + assert_int_equal(SignalFromString("INT"), SIGINT); + assert_int_equal(SignalFromString("TERM"), SIGTERM); + assert_int_equal(SignalFromString("KILL"), SIGKILL); + + /* Mixed case */ + assert_int_equal(SignalFromString("Term"), SIGTERM); + assert_int_equal(SignalFromString("HuP"), SIGHUP); + assert_int_equal(SignalFromString("KiLl"), SIGKILL); + + /* Invalid */ + assert_int_equal(SignalFromString("invalid"), -1); + assert_int_equal(SignalFromString(""), -1); + assert_int_equal(SignalFromString(NULL), -1); +} + static void test_CommandArg0_bound(void) { char dst[128]; @@ -284,6 +310,7 @@ int main() unit_test(test_int_from_string), unit_test(test_double_from_string), unit_test(test_CommandArg0_bound), + unit_test(test_signal_from_string), }; return run_tests(tests); From 80a9755b12c73c4f2bf571dd4721cb7659f9794d Mon Sep 17 00:00:00 2001 From: Nick Anderson Date: Mon, 29 Jun 2026 15:26:02 -0500 Subject: [PATCH 2/2] feat: make signal name matching case-insensitive (CFE-612) Use StringEqual_IgnoreCase in SignalFromString, scope (?i:...) to signal names in the validation regex, and add RlistKeyIn_IgnoreCase for the process-signal lookup path. --- cf-agent/verify_processes.c | 4 ++-- libpromises/conversion.c | 6 +++++- libpromises/mod_process.c | 2 +- libpromises/rlist.c | 18 ++++++++++++++++++ libpromises/rlist.h | 1 + tests/unit/rlist_test.c | 25 +++++++++++++++++++++++++ 6 files changed, 52 insertions(+), 4 deletions(-) diff --git a/cf-agent/verify_processes.c b/cf-agent/verify_processes.c index 04427bd795..6f921ddfb2 100644 --- a/cf-agent/verify_processes.c +++ b/cf-agent/verify_processes.c @@ -69,7 +69,7 @@ static bool ProcessSanityChecks(const Attributes *a, const Promise *pp) if (a->restart_class) { - if ((RlistKeyIn(a->signals, "term")) || (RlistKeyIn(a->signals, "kill"))) + if ((RlistKeyIn_IgnoreCase(a->signals, "term")) || (RlistKeyIn_IgnoreCase(a->signals, "kill"))) { Log(LOG_LEVEL_WARNING, "Promise '%s' kills then restarts - never strictly converges", pp->promiser); @@ -387,7 +387,7 @@ static int FindPidMatches(Item **killlist, const Attributes *a, const char *prom { if (pid == 1) { - if (RlistLen(a->signals) == 1 && RlistKeyIn(a->signals, "hup")) + if (RlistLen(a->signals) == 1 && RlistKeyIn_IgnoreCase(a->signals, "hup")) { Log(LOG_LEVEL_VERBOSE, "Okay to send only HUP to init"); } diff --git a/libpromises/conversion.c b/libpromises/conversion.c index 71c5f87312..03dc17b3ab 100644 --- a/libpromises/conversion.c +++ b/libpromises/conversion.c @@ -229,10 +229,14 @@ int SignalFromString(const char *s) SIGHUP, SIGINT, SIGTRAP, SIGKILL, SIGPIPE, SIGCONT, SIGABRT, SIGSTOP, SIGQUIT, SIGTERM, SIGCHLD, SIGUSR1, SIGUSR2, SIGBUS, SIGSEGV }; + if (s == NULL) + { + return -1; + } for (size_t i = 0; i < 15; i++) { - if (StringEqual(s, signal_names[i])) + if (StringEqual_IgnoreCase(s, signal_names[i])) { return signals[i]; } diff --git a/libpromises/mod_process.c b/libpromises/mod_process.c index 8c8992cd47..d98988a917 100644 --- a/libpromises/mod_process.c +++ b/libpromises/mod_process.c @@ -72,7 +72,7 @@ static const ConstraintSyntax processes_constraints[] = ConstraintSyntaxNewString("process_stop", CF_ABSPATHRANGE, "A command used to stop a running process", SYNTAX_STATUS_NORMAL), ConstraintSyntaxNewString("restart_class", CF_IDRANGE, "A class to be defined globally if the process is not running, so that a command: rule can be referred to restart the process", SYNTAX_STATUS_NORMAL), - ConstraintSyntaxNewStringList("signals", "(hup|int|trap|kill|pipe|cont|abrt|stop|quit|term|child|usr1|usr2|bus|segv|[0-9]+s?)", + ConstraintSyntaxNewStringList("signals", "((?i:hup|int|trap|kill|pipe|cont|abrt|stop|quit|term|child|usr1|usr2|bus|segv)|[0-9]+s?)", "A list of strings representing signals to be sent to a process or sleep periods", SYNTAX_STATUS_NORMAL), ConstraintSyntaxNewNull() }; diff --git a/libpromises/rlist.c b/libpromises/rlist.c index ce9e7f7281..829b1ca254 100644 --- a/libpromises/rlist.c +++ b/libpromises/rlist.c @@ -209,6 +209,24 @@ Rlist *RlistKeyIn(Rlist *list, const char *key) return NULL; } +Rlist *RlistKeyIn_IgnoreCase(Rlist *list, const char *key) +{ + for (Rlist *rp = list; rp != NULL; rp = rp->next) + { + if (rp->val.type == RVAL_TYPE_SCALAR) + { + const char *scalar = RlistScalarValue(rp); + if (scalar != NULL && + StringEqual_IgnoreCase(scalar, key)) + { + return rp; + } + } + } + + return NULL; +} + /*******************************************************************/ bool RlistMatchesRegexRlist(const Rlist *list, const Rlist *search) diff --git a/libpromises/rlist.h b/libpromises/rlist.h index a8acafc8ce..a690284341 100644 --- a/libpromises/rlist.h +++ b/libpromises/rlist.h @@ -82,6 +82,7 @@ Rlist *RlistRlistValue(const Rlist *rlist); Rlist *RlistParseShown(const char *string); Rlist *RlistParseString(const char *string); Rlist *RlistKeyIn(Rlist *list, const char *key); +Rlist *RlistKeyIn_IgnoreCase(Rlist *list, const char *key); int RlistLen(const Rlist *start); bool RlistMatchesRegexRlist(const Rlist *list, const Rlist *search); bool RlistMatchesRegex(const Rlist *list, const char *str); diff --git a/tests/unit/rlist_test.c b/tests/unit/rlist_test.c index 688fa9cca9..9703b1394e 100644 --- a/tests/unit/rlist_test.c +++ b/tests/unit/rlist_test.c @@ -760,6 +760,30 @@ static void test_rval_write_raw() WriterClose(writer); } +static void test_key_in_ignore_case() +{ + Rlist *list = NULL; + + RlistAppendScalar(&list, "TERM"); + RlistAppendScalar(&list, "HUP"); + RlistAppendScalar(&list, "KILL"); + + Rlist *match = RlistKeyIn_IgnoreCase(list, "term"); + assert_true(match != NULL); + assert_string_equal(RlistScalarValue(match), "TERM"); + + match = RlistKeyIn_IgnoreCase(list, "HuP"); + assert_true(match != NULL); + assert_string_equal(RlistScalarValue(match), "HUP"); + + match = RlistKeyIn_IgnoreCase(list, "PIPE"); + assert_true(match == NULL); + + RlistDestroy(list); + + assert_true(RlistKeyIn_IgnoreCase(NULL, "term") == NULL); +} + int main() { PRINT_TEST_BANNER(); @@ -796,6 +820,7 @@ int main() unit_test(test_rval_write), unit_test(test_rval_write_quoted), unit_test(test_rval_write_raw), + unit_test(test_key_in_ignore_case), }; return run_tests(tests);