From 72405c5c666e025021c906146929ead738f67cd8 Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Thu, 29 Oct 2026 11:26:25 +0100 Subject: [PATCH 01/12] libpromises: Added empty package_module_wrapper.{c,h} Signed-off-by: Lars Erik Wik --- libpromises/Makefile.am | 1 + libpromises/package_module_wrapper.c | 25 +++++++++++++++++++++++++ libpromises/package_module_wrapper.h | 28 ++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+) create mode 100644 libpromises/package_module_wrapper.c create mode 100644 libpromises/package_module_wrapper.h diff --git a/libpromises/Makefile.am b/libpromises/Makefile.am index dc3fcf9dd77..99368a35df4 100644 --- a/libpromises/Makefile.am +++ b/libpromises/Makefile.am @@ -145,6 +145,7 @@ libpromises_la_SOURCES = \ parser_state.h \ patches.c \ pipes.h pipes.c \ + package_module_wrapper.c package_module_wrapper.h \ processes_select.c processes_select.h \ process_lib.h process_unix_priv.h \ promises.c promises.h \ diff --git a/libpromises/package_module_wrapper.c b/libpromises/package_module_wrapper.c new file mode 100644 index 00000000000..d3302eabd17 --- /dev/null +++ b/libpromises/package_module_wrapper.c @@ -0,0 +1,25 @@ +/* + Copyright 2026 Northern.tech AS + + This file is part of CFEngine 3 - written and maintained by Northern.tech AS. + + This program is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by the + Free Software Foundation; version 3. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA + + To the extent this program is licensed as part of the Enterprise + versions of CFEngine, the applicable Commercial Open Source License + (COSL) may apply to this file if you as a licensee so wish it. See + included file COSL.txt. +*/ + +#include diff --git a/libpromises/package_module_wrapper.h b/libpromises/package_module_wrapper.h new file mode 100644 index 00000000000..a8fc96792dd --- /dev/null +++ b/libpromises/package_module_wrapper.h @@ -0,0 +1,28 @@ +/* + Copyright 2026 Northern.tech AS + + This file is part of CFEngine 3 - written and maintained by Northern.tech AS. + + This program is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by the + Free Software Foundation; version 3. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA + + To the extent this program is licensed as part of the Enterprise + versions of CFEngine, the applicable Commercial Open Source License + (COSL) may apply to this file if you as a licensee so wish it. See + included file COSL.txt. +*/ + +#ifndef CFENGINE_PACKAGE_MODULE_WRAPPER_H +#define CFENGINE_PACKAGE_MODULE_WRAPPER_H + +#endif /* CFENGINE_PACKAGE_MODULE_WRAPPER_H */ From 28b3cc3b08162f975909d8362fc3dac6554bf1cb Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Thu, 29 Oct 2026 11:29:25 +0100 Subject: [PATCH 02/12] Moved package module wrapper code from cf-agent to libpromises Signed-off-by: Lars Erik Wik --- cf-agent/package_module.c | 213 -------------------------- cf-agent/package_module.h | 19 +-- libpromises/package_module_wrapper.c | 221 +++++++++++++++++++++++++++ libpromises/package_module_wrapper.h | 20 +++ 4 files changed, 242 insertions(+), 231 deletions(-) diff --git a/cf-agent/package_module.c b/cf-agent/package_module.c index 840483f71b9..200bf620f29 100644 --- a/cf-agent/package_module.c +++ b/cf-agent/package_module.c @@ -47,123 +47,6 @@ static bool UpdateSinglePackageModuleCache(EvalContext *ctx, const PackageModuleWrapper *module_wrapper, UpdateType type, bool force_update); -static void GetPackageModuleExecInfo(const PackageModuleBody *package_module, char **exec_path, - char **script_path, char **script_path_quoted, char **script_exec_opts); -static int NegotiateSupportedAPIVersion(PackageModuleWrapper *wrapper); - - -void DeletePackageModuleWrapper(PackageModuleWrapper *wrapper) -{ - if (wrapper != NULL) - { - free(wrapper->path); - free(wrapper->script_path); - free(wrapper->script_path_quoted); - free(wrapper->script_exec_opts); - free(wrapper->name); - free(wrapper); - } -} - -PackageModuleWrapper *NewPackageModuleWrapper(PackageModuleBody *package_module) -{ - assert(package_module && package_module->name); - - PackageModuleWrapper *wrapper = xmalloc(sizeof(PackageModuleWrapper)); - - GetPackageModuleExecInfo(package_module, &(wrapper->path), - &(wrapper->script_path), &(wrapper->script_path_quoted), - &(wrapper->script_exec_opts)); - wrapper->name = SafeStringDuplicate(package_module->name); - wrapper->package_module = package_module; - - if (wrapper->path == NULL) - { - Log(LOG_LEVEL_ERR, "No executable for the package module '%s'", package_module->name); - DeletePackageModuleWrapper(wrapper); - return NULL; - } - - /* Check if the given files exist and have the required permissions. */ - if (wrapper->script_path != NULL) - { - /* we were given a path to a script to execute with the given - * interpreter */ - const char *interpreter_path = wrapper->path; - const char *script_path = wrapper->script_path; - - if (access(interpreter_path, X_OK) != 0) - { - Log(LOG_LEVEL_ERR, - "Cannot find package module interpreter at location '%s'" - " or access to the file is restricted: %s", - wrapper->path, GetErrorStr()); - DeletePackageModuleWrapper(wrapper); - return NULL; - } - if (access(script_path, R_OK) != 0) - { - Log(LOG_LEVEL_ERR, - "Cannot find package module script at location '%s'" - " or access to the file is restricted: %s", - wrapper->script_path, GetErrorStr()); - DeletePackageModuleWrapper(wrapper); - return NULL; - } - } - else if (access(wrapper->path, X_OK) != 0) - { - /* no script path specified --> the given path has to be executable */ - Log(LOG_LEVEL_ERR, - "Cannot find package module at location '%s' or access to file is restricted: %s", - wrapper->path, GetErrorStr()); - DeletePackageModuleWrapper(wrapper); - return NULL; - } - - /* Negotiate API version */ - wrapper->supported_api_version = NegotiateSupportedAPIVersion(wrapper); - if (wrapper->supported_api_version != 1) - { - Log(LOG_LEVEL_ERR, - "Unsupported package module wrapper API version: %d", - wrapper->supported_api_version); - DeletePackageModuleWrapper(wrapper); - return NULL; - } - - Log(LOG_LEVEL_DEBUG, - "Successfully created package module wrapper for '%s' package module.", - package_module->name); - - return wrapper; -} - -static int PackageWrapperCommunicate(const PackageModuleWrapper *wrapper, const char *args, - const char *request, Rlist **response) -{ - char *all_args = NULL; - if (wrapper->script_path != NULL) - { - if (wrapper->script_exec_opts == NULL) - { - all_args = StringConcatenate(3, wrapper->script_path_quoted, " ", args); - args = all_args; - } - else - { - all_args = StringConcatenate(5, wrapper->script_exec_opts, " ", - wrapper->script_path_quoted, " ", args); - args = all_args; - } - } - int ret = PipeReadWriteData(wrapper->path, args, request, response, - PACKAGE_PROMISE_SCRIPT_TIMEOUT_SEC, - PACKAGE_PROMISE_TERMINATION_CHECK_SEC); - free(all_args); - - return ret; -} void UpdatePackagesCache(EvalContext *ctx, bool force_update) { @@ -278,24 +161,6 @@ static void FreePackageInfo(PackageInfo *package_info) } } -static char *ParseOptions(Rlist *options) -{ - if (RlistIsNullList(options)) - { - return SafeStringDuplicate(""); - } - - Buffer *data = BufferNew(); - for (Rlist *rp = options; rp != NULL; rp = rp->next) - { - char *value = RlistScalarValue(rp); - BufferAppendString(data, "options="); - BufferAppendString(data, value); - BufferAppendString(data, "\n"); - } - return BufferClose(data); -} - static PackageInfo *ParseAndCheckPackageDataReply(const Rlist *data) { PackageInfo * package_data = xcalloc(1, sizeof(PackageInfo)); @@ -386,35 +251,6 @@ static PackageInfo *ParseAndCheckPackageDataReply(const Rlist *data) return package_data; } -static int NegotiateSupportedAPIVersion(PackageModuleWrapper *wrapper) -{ - assert(wrapper); - - Log(LOG_LEVEL_DEBUG, "Getting supported API version."); - - int api_version = -1; - - Rlist *response = NULL; - if (PackageWrapperCommunicate(wrapper, "supports-api-version", "", - &response) != 0) - { - Log(LOG_LEVEL_INFO, - "Error occurred while getting supported API version."); - return -1; - } - - if (response) - { - if (RlistLen(response) == 1) - { - api_version = atoi(RlistScalarValue(response)); - Log(LOG_LEVEL_DEBUG, "package wrapper API version: %d", api_version); - } - RlistDestroy(response); - } - return api_version; -} - /* IMPORTANT: this might not return all the data we need like version or architecture but package name MUST be known. */ static @@ -473,55 +309,6 @@ PackageInfo *GetPackageData(const char *name, const char *version, return package_data; } -static void GetPackageModuleExecInfo(const PackageModuleBody *package_module, char **exec_path, - char **script_path, char **script_path_quoted, - char **script_exec_opts) -{ - - assert(exec_path != NULL); - assert(script_path != NULL); - assert(script_path_quoted != NULL); - - char *package_module_path = NULL; - - if (package_module->module_path != NULL && !StringEqual(package_module->module_path, "")) - { - package_module_path = xstrdup(package_module->module_path); - } - else - { - package_module_path = StringFormat("%s%c%s%c%s%c%s", GetWorkDir(), FILE_SEPARATOR, - "modules", FILE_SEPARATOR, "packages", FILE_SEPARATOR, - package_module->name); - } - - if (package_module->interpreter && !StringEqual(package_module->interpreter, "")) - { - *script_path = package_module_path; - *script_path_quoted = Path_GetQuoted(*script_path); - - if (strchr(package_module->interpreter, ' ') == NULL) - { - /* No spaces in the 'interpreter' string, easy! Just interpreter path given, no opts. */ - *exec_path = xstrdup(package_module->interpreter); - *script_exec_opts = NULL; - } - else - { - /* we need to split the interpreter command from the (potential) - * interpreter args */ - ArgGetExecutableAndArgs(package_module->interpreter, exec_path, script_exec_opts); - } - } - else - { - *exec_path = package_module_path; - *script_path = NULL; - *script_path_quoted = NULL; - *script_exec_opts = NULL; - } -} - static int IsPackageInCache(EvalContext *ctx, const PackageModuleWrapper *module_wrapper, const char *name, const char *ver, const char *arch) diff --git a/cf-agent/package_module.h b/cf-agent/package_module.h index b8372c96984..8bb797d4a55 100644 --- a/cf-agent/package_module.h +++ b/cf-agent/package_module.h @@ -26,10 +26,7 @@ #define PACKAGE_MODULE_H #include - -//TODO: use promise expireafter instead -#define PACKAGE_PROMISE_SCRIPT_TIMEOUT_SEC 4 * 3600 /* 4 hours timeout */ -#define PACKAGE_PROMISE_TERMINATION_CHECK_SEC 1 +#include #define GLOBAL_PACKAGE_PROMISE_LOCK_NAME "new_packages_promise_lock" @@ -54,17 +51,6 @@ typedef struct char *message; } PackageError; -typedef struct -{ - char *name; - char *path; - char *script_path; - char *script_path_quoted; - char *script_exec_opts; - PackageModuleBody *package_module; - int supported_api_version; -} PackageModuleWrapper; - typedef struct { CfLock g_lock; @@ -88,9 +74,6 @@ PromiseResult HandleAbsentPromiseAction(EvalContext *ctx, void UpdatePackagesCache(EvalContext *ctx, bool force_update); -PackageModuleWrapper *NewPackageModuleWrapper(PackageModuleBody *package_module); -void DeletePackageModuleWrapper(PackageModuleWrapper *wrapper); - PackagePromiseGlobalLock AcquireGlobalPackagePromiseLock(EvalContext *ctx); void YieldGlobalPackagePromiseLock(PackagePromiseGlobalLock lock); diff --git a/libpromises/package_module_wrapper.c b/libpromises/package_module_wrapper.c index d3302eabd17..edb616eb7e0 100644 --- a/libpromises/package_module_wrapper.c +++ b/libpromises/package_module_wrapper.c @@ -23,3 +23,224 @@ */ #include +#include +#include +#include +#include +#include +#include +#include +#include + +static void GetPackageModuleExecInfo(const PackageModuleBody *package_module, char **exec_path, + char **script_path, char **script_path_quoted, char **script_exec_opts); +static int NegotiateSupportedAPIVersion(PackageModuleWrapper *wrapper); + +void DeletePackageModuleWrapper(PackageModuleWrapper *wrapper) +{ + if (wrapper != NULL) + { + free(wrapper->path); + free(wrapper->script_path); + free(wrapper->script_path_quoted); + free(wrapper->script_exec_opts); + free(wrapper->name); + free(wrapper); + } +} + +PackageModuleWrapper *NewPackageModuleWrapper(PackageModuleBody *package_module) +{ + assert(package_module && package_module->name); + + PackageModuleWrapper *wrapper = xmalloc(sizeof(PackageModuleWrapper)); + + GetPackageModuleExecInfo(package_module, &(wrapper->path), + &(wrapper->script_path), &(wrapper->script_path_quoted), + &(wrapper->script_exec_opts)); + wrapper->name = SafeStringDuplicate(package_module->name); + wrapper->package_module = package_module; + + if (wrapper->path == NULL) + { + Log(LOG_LEVEL_ERR, "No executable for the package module '%s'", package_module->name); + DeletePackageModuleWrapper(wrapper); + return NULL; + } + + /* Check if the given files exist and have the required permissions. */ + if (wrapper->script_path != NULL) + { + /* we were given a path to a script to execute with the given + * interpreter */ + const char *interpreter_path = wrapper->path; + const char *script_path = wrapper->script_path; + + if (access(interpreter_path, X_OK) != 0) + { + Log(LOG_LEVEL_ERR, + "Cannot find package module interpreter at location '%s'" + " or access to the file is restricted: %s", + wrapper->path, GetErrorStr()); + DeletePackageModuleWrapper(wrapper); + return NULL; + } + if (access(script_path, R_OK) != 0) + { + Log(LOG_LEVEL_ERR, + "Cannot find package module script at location '%s'" + " or access to the file is restricted: %s", + wrapper->script_path, GetErrorStr()); + DeletePackageModuleWrapper(wrapper); + return NULL; + } + } + else if (access(wrapper->path, X_OK) != 0) + { + /* no script path specified --> the given path has to be executable */ + Log(LOG_LEVEL_ERR, + "Cannot find package module at location '%s' or access to file is restricted: %s", + wrapper->path, GetErrorStr()); + DeletePackageModuleWrapper(wrapper); + return NULL; + } + + /* Negotiate API version */ + wrapper->supported_api_version = NegotiateSupportedAPIVersion(wrapper); + if (wrapper->supported_api_version != 1) + { + Log(LOG_LEVEL_ERR, + "Unsupported package module wrapper API version: %d", + wrapper->supported_api_version); + DeletePackageModuleWrapper(wrapper); + return NULL; + } + + Log(LOG_LEVEL_DEBUG, + "Successfully created package module wrapper for '%s' package module.", + package_module->name); + + return wrapper; +} + +static int PackageWrapperCommunicate(const PackageModuleWrapper *wrapper, const char *args, + const char *request, Rlist **response) +{ + char *all_args = NULL; + if (wrapper->script_path != NULL) + { + if (wrapper->script_exec_opts == NULL) + { + all_args = StringConcatenate(3, wrapper->script_path_quoted, " ", args); + args = all_args; + } + else + { + all_args = StringConcatenate(5, wrapper->script_exec_opts, " ", + wrapper->script_path_quoted, " ", args); + args = all_args; + } + } + int ret = PipeReadWriteData(wrapper->path, args, request, response, + PACKAGE_PROMISE_SCRIPT_TIMEOUT_SEC, + PACKAGE_PROMISE_TERMINATION_CHECK_SEC); + free(all_args); + + return ret; +} + +static char *ParseOptions(Rlist *options) +{ + if (RlistIsNullList(options)) + { + return SafeStringDuplicate(""); + } + + Buffer *data = BufferNew(); + for (Rlist *rp = options; rp != NULL; rp = rp->next) + { + char *value = RlistScalarValue(rp); + BufferAppendString(data, "options="); + BufferAppendString(data, value); + BufferAppendString(data, "\n"); + } + return BufferClose(data); +} + +static int NegotiateSupportedAPIVersion(PackageModuleWrapper *wrapper) +{ + assert(wrapper); + + Log(LOG_LEVEL_DEBUG, "Getting supported API version."); + + int api_version = -1; + + Rlist *response = NULL; + if (PackageWrapperCommunicate(wrapper, "supports-api-version", "", + &response) != 0) + { + Log(LOG_LEVEL_INFO, + "Error occurred while getting supported API version."); + return -1; + } + + if (response) + { + if (RlistLen(response) == 1) + { + api_version = atoi(RlistScalarValue(response)); + Log(LOG_LEVEL_DEBUG, "package wrapper API version: %d", api_version); + } + RlistDestroy(response); + } + return api_version; +} + +static void GetPackageModuleExecInfo(const PackageModuleBody *package_module, char **exec_path, + char **script_path, char **script_path_quoted, + char **script_exec_opts) +{ + + assert(exec_path != NULL); + assert(script_path != NULL); + assert(script_path_quoted != NULL); + + char *package_module_path = NULL; + + if (package_module->module_path != NULL && !StringEqual(package_module->module_path, "")) + { + package_module_path = xstrdup(package_module->module_path); + } + else + { + package_module_path = StringFormat("%s%c%s%c%s%c%s", GetWorkDir(), FILE_SEPARATOR, + "modules", FILE_SEPARATOR, "packages", FILE_SEPARATOR, + package_module->name); + } + + if (package_module->interpreter && !StringEqual(package_module->interpreter, "")) + { + *script_path = package_module_path; + *script_path_quoted = Path_GetQuoted(*script_path); + + if (strchr(package_module->interpreter, ' ') == NULL) + { + /* No spaces in the 'interpreter' string, easy! Just interpreter path given, no opts. */ + *exec_path = xstrdup(package_module->interpreter); + *script_exec_opts = NULL; + } + else + { + /* we need to split the interpreter command from the (potential) + * interpreter args */ + ArgGetExecutableAndArgs(package_module->interpreter, exec_path, script_exec_opts); + } + } + else + { + *exec_path = package_module_path; + *script_path = NULL; + *script_path_quoted = NULL; + *script_exec_opts = NULL; + } +} diff --git a/libpromises/package_module_wrapper.h b/libpromises/package_module_wrapper.h index a8fc96792dd..f36ae674c9f 100644 --- a/libpromises/package_module_wrapper.h +++ b/libpromises/package_module_wrapper.h @@ -25,4 +25,24 @@ #ifndef CFENGINE_PACKAGE_MODULE_WRAPPER_H #define CFENGINE_PACKAGE_MODULE_WRAPPER_H +#include + +//TODO: use promise expireafter instead +#define PACKAGE_PROMISE_SCRIPT_TIMEOUT_SEC 4 * 3600 /* 4 hours timeout */ +#define PACKAGE_PROMISE_TERMINATION_CHECK_SEC 1 + +typedef struct +{ + char *name; + char *path; + char *script_path; + char *script_path_quoted; + char *script_exec_opts; + PackageModuleBody *package_module; + int supported_api_version; +} PackageModuleWrapper; + +PackageModuleWrapper *NewPackageModuleWrapper(PackageModuleBody *package_module); +void DeletePackageModuleWrapper(PackageModuleWrapper *wrapper); + #endif /* CFENGINE_PACKAGE_MODULE_WRAPPER_H */ From 16ad8db67edf0435fd5637d772efabbf0399c13a Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Thu, 29 Oct 2026 11:32:03 +0100 Subject: [PATCH 03/12] package_module_wrapper.c: Made PackageWrapperCommunicate() and ParseOptions() public Signed-off-by: Lars Erik Wik --- libpromises/package_module_wrapper.c | 6 +++--- libpromises/package_module_wrapper.h | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/libpromises/package_module_wrapper.c b/libpromises/package_module_wrapper.c index edb616eb7e0..1db66f34616 100644 --- a/libpromises/package_module_wrapper.c +++ b/libpromises/package_module_wrapper.c @@ -123,8 +123,8 @@ PackageModuleWrapper *NewPackageModuleWrapper(PackageModuleBody *package_module) return wrapper; } -static int PackageWrapperCommunicate(const PackageModuleWrapper *wrapper, const char *args, - const char *request, Rlist **response) +int PackageWrapperCommunicate(const PackageModuleWrapper *wrapper, const char *args, + const char *request, Rlist **response) { char *all_args = NULL; if (wrapper->script_path != NULL) @@ -149,7 +149,7 @@ static int PackageWrapperCommunicate(const PackageModuleWrapper *wrapper, const return ret; } -static char *ParseOptions(Rlist *options) +char *ParseOptions(Rlist *options) { if (RlistIsNullList(options)) { diff --git a/libpromises/package_module_wrapper.h b/libpromises/package_module_wrapper.h index f36ae674c9f..c6d7903e253 100644 --- a/libpromises/package_module_wrapper.h +++ b/libpromises/package_module_wrapper.h @@ -44,5 +44,8 @@ typedef struct PackageModuleWrapper *NewPackageModuleWrapper(PackageModuleBody *package_module); void DeletePackageModuleWrapper(PackageModuleWrapper *wrapper); +int PackageWrapperCommunicate(const PackageModuleWrapper *wrapper, const char *args, + const char *request, Rlist **response); +char *ParseOptions(Rlist *options); #endif /* CFENGINE_PACKAGE_MODULE_WRAPPER_H */ From f871307719b37cafc5886fa85a1e281d5702041a Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Thu, 29 Oct 2026 11:38:38 +0100 Subject: [PATCH 04/12] package_module_wrapper.h: Added doc comments Signed-off-by: Lars Erik Wik --- libpromises/package_module_wrapper.h | 37 ++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/libpromises/package_module_wrapper.h b/libpromises/package_module_wrapper.h index c6d7903e253..5abdf92a201 100644 --- a/libpromises/package_module_wrapper.h +++ b/libpromises/package_module_wrapper.h @@ -42,10 +42,47 @@ typedef struct int supported_api_version; } PackageModuleWrapper; +/** + * @brief Creates a wrapper to run a package module. + * + * Runs the package module once, to negotiate the API version. + * + * @param package_module The package module body, not copied, so it must + * outlive the wrapper + * @return The new wrapper, to be destroyed with DeletePackageModuleWrapper(), + * or NULL if the package module can't be run or doesn't support API + * version 1 + */ PackageModuleWrapper *NewPackageModuleWrapper(PackageModuleBody *package_module); + +/** + * @brief Destroys a package module wrapper. + * + * @param wrapper The wrapper, may be NULL + */ void DeletePackageModuleWrapper(PackageModuleWrapper *wrapper); + +/** + * @brief Runs the package module with a request. + * + * @param wrapper The wrapper of the package module + * @param args The arguments of the package module, e.g. "list-installed" + * @param request The request, written to the stdin of the package module + * @param[out] response The lines of the stdout of the package module, only + * set on success + * @return 0 on success, -1 if the package module couldn't be run or exited + * with a non-zero code + */ int PackageWrapperCommunicate(const PackageModuleWrapper *wrapper, const char *args, const char *request, Rlist **response); + +/** + * @brief Formats the options of a package module as request lines. + * + * @param options The options, may be NULL + * @return One "options=VALUE" line per option, or an empty string if there + * are none. To be freed by the caller. + */ char *ParseOptions(Rlist *options); #endif /* CFENGINE_PACKAGE_MODULE_WRAPPER_H */ From 6bba782e497a70c6401c4e5610fe8cc02f41b62b Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Thu, 29 Oct 2026 11:40:13 +0100 Subject: [PATCH 05/12] Renamed ParseOptions() to PackageModuleFormatOptions() Signed-off-by: Lars Erik Wik --- cf-agent/package_module.c | 8 ++++---- libpromises/package_module_wrapper.c | 2 +- libpromises/package_module_wrapper.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cf-agent/package_module.c b/cf-agent/package_module.c index 200bf620f29..09adaa66308 100644 --- a/cf-agent/package_module.c +++ b/cf-agent/package_module.c @@ -262,7 +262,7 @@ PackageInfo *GetPackageData(const char *name, const char *version, Log(LOG_LEVEL_DEBUG, "Getting package '%s' data.", name); - char *options_str = ParseOptions(options); + char *options_str = PackageModuleFormatOptions(options); char *ver = version ? StringFormat("Version=%s\n", version) : NULL; char *arch = architecture ? @@ -567,7 +567,7 @@ bool UpdateCache(Rlist* options, const PackageModuleWrapper *wrapper, Log(LOG_LEVEL_DEBUG, "Updating cache: %d", type); - char *options_str = ParseOptions(options); + char *options_str = PackageModuleFormatOptions(options); Rlist *response = NULL; const char *req_type = NULL; @@ -667,7 +667,7 @@ PromiseResult RemovePackage(const char *name, Rlist* options, Log(LOG_LEVEL_DEBUG, "Removing package '%s'", name); - char *options_str = ParseOptions(options); + char *options_str = PackageModuleFormatOptions(options); char *ver = version ? StringFormat("Version=%s\n", version) : NULL; char *arch = architecture ? @@ -712,7 +712,7 @@ static PromiseResult InstallPackageGeneric(Rlist *options, type == PACKAGE_TYPE_FILE ? "file" : "repo", packages_list_formatted); - char *options_str = ParseOptions(options); + char *options_str = PackageModuleFormatOptions(options); char *request = StringFormat("%s%s", options_str, packages_list_formatted); PromiseResult res = PROMISE_RESULT_CHANGE; diff --git a/libpromises/package_module_wrapper.c b/libpromises/package_module_wrapper.c index 1db66f34616..389eb7a30c9 100644 --- a/libpromises/package_module_wrapper.c +++ b/libpromises/package_module_wrapper.c @@ -149,7 +149,7 @@ int PackageWrapperCommunicate(const PackageModuleWrapper *wrapper, const char *a return ret; } -char *ParseOptions(Rlist *options) +char *PackageModuleFormatOptions(Rlist *options) { if (RlistIsNullList(options)) { diff --git a/libpromises/package_module_wrapper.h b/libpromises/package_module_wrapper.h index 5abdf92a201..d6b58beeef3 100644 --- a/libpromises/package_module_wrapper.h +++ b/libpromises/package_module_wrapper.h @@ -83,6 +83,6 @@ int PackageWrapperCommunicate(const PackageModuleWrapper *wrapper, const char *a * @return One "options=VALUE" line per option, or an empty string if there * are none. To be freed by the caller. */ -char *ParseOptions(Rlist *options); +char *PackageModuleFormatOptions(Rlist *options); #endif /* CFENGINE_PACKAGE_MODULE_WRAPPER_H */ From eba270c863ec865461062411d5b6e4894b62c8b6 Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Thu, 29 Oct 2026 11:42:11 +0100 Subject: [PATCH 06/12] Added timeout parameter to NewPackageModuleWrapper() Signed-off-by: Lars Erik Wik --- cf-agent/package_module.c | 3 ++- cf-agent/package_module.h | 3 +++ cf-agent/verify_new_packages.c | 3 ++- libpromises/package_module_wrapper.c | 6 ++++-- libpromises/package_module_wrapper.h | 7 ++++--- 5 files changed, 15 insertions(+), 7 deletions(-) diff --git a/cf-agent/package_module.c b/cf-agent/package_module.c index 09adaa66308..a07ae0e905d 100644 --- a/cf-agent/package_module.c +++ b/cf-agent/package_module.c @@ -76,7 +76,8 @@ void UpdatePackagesCache(EvalContext *ctx, bool force_update) continue; } - PackageModuleWrapper *module_wrapper = NewPackageModuleWrapper(module); + PackageModuleWrapper *module_wrapper = + NewPackageModuleWrapper(module, PACKAGE_PROMISE_SCRIPT_TIMEOUT_SEC); if (!module_wrapper) { diff --git a/cf-agent/package_module.h b/cf-agent/package_module.h index 8bb797d4a55..bbae854a92a 100644 --- a/cf-agent/package_module.h +++ b/cf-agent/package_module.h @@ -28,6 +28,9 @@ #include #include +//TODO: use promise expireafter instead +#define PACKAGE_PROMISE_SCRIPT_TIMEOUT_SEC 4 * 3600 /* 4 hours timeout */ + #define GLOBAL_PACKAGE_PROMISE_LOCK_NAME "new_packages_promise_lock" typedef enum diff --git a/cf-agent/verify_new_packages.c b/cf-agent/verify_new_packages.c index 82985a40f9b..3dcd9a010e3 100644 --- a/cf-agent/verify_new_packages.c +++ b/cf-agent/verify_new_packages.c @@ -100,7 +100,8 @@ PromiseResult HandleNewPackagePromiseType(EvalContext *ctx, const Promise *pp, c } PackageModuleWrapper *package_module = - NewPackageModuleWrapper(a->new_packages.module_body); + NewPackageModuleWrapper(a->new_packages.module_body, + PACKAGE_PROMISE_SCRIPT_TIMEOUT_SEC); if (package_module == NULL) { diff --git a/libpromises/package_module_wrapper.c b/libpromises/package_module_wrapper.c index 389eb7a30c9..7dc4a68086d 100644 --- a/libpromises/package_module_wrapper.c +++ b/libpromises/package_module_wrapper.c @@ -49,7 +49,8 @@ void DeletePackageModuleWrapper(PackageModuleWrapper *wrapper) } } -PackageModuleWrapper *NewPackageModuleWrapper(PackageModuleBody *package_module) +PackageModuleWrapper *NewPackageModuleWrapper(PackageModuleBody *package_module, + int timeout_secs) { assert(package_module && package_module->name); @@ -60,6 +61,7 @@ PackageModuleWrapper *NewPackageModuleWrapper(PackageModuleBody *package_module) &(wrapper->script_exec_opts)); wrapper->name = SafeStringDuplicate(package_module->name); wrapper->package_module = package_module; + wrapper->timeout_secs = timeout_secs; if (wrapper->path == NULL) { @@ -142,7 +144,7 @@ int PackageWrapperCommunicate(const PackageModuleWrapper *wrapper, const char *a } } int ret = PipeReadWriteData(wrapper->path, args, request, response, - PACKAGE_PROMISE_SCRIPT_TIMEOUT_SEC, + wrapper->timeout_secs, PACKAGE_PROMISE_TERMINATION_CHECK_SEC); free(all_args); diff --git a/libpromises/package_module_wrapper.h b/libpromises/package_module_wrapper.h index d6b58beeef3..0a6b12f0aac 100644 --- a/libpromises/package_module_wrapper.h +++ b/libpromises/package_module_wrapper.h @@ -27,8 +27,6 @@ #include -//TODO: use promise expireafter instead -#define PACKAGE_PROMISE_SCRIPT_TIMEOUT_SEC 4 * 3600 /* 4 hours timeout */ #define PACKAGE_PROMISE_TERMINATION_CHECK_SEC 1 typedef struct @@ -40,6 +38,7 @@ typedef struct char *script_exec_opts; PackageModuleBody *package_module; int supported_api_version; + int timeout_secs; /**< Timeout of each run of the package module */ } PackageModuleWrapper; /** @@ -49,11 +48,13 @@ typedef struct * * @param package_module The package module body, not copied, so it must * outlive the wrapper + * @param timeout_secs The timeout of each run of the package module * @return The new wrapper, to be destroyed with DeletePackageModuleWrapper(), * or NULL if the package module can't be run or doesn't support API * version 1 */ -PackageModuleWrapper *NewPackageModuleWrapper(PackageModuleBody *package_module); +PackageModuleWrapper *NewPackageModuleWrapper(PackageModuleBody *package_module, + int timeout_secs); /** * @brief Destroys a package module wrapper. From 6c124767fef5db4c023e9182efc0ba9295cfd726 Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Thu, 29 Oct 2026 11:56:22 +0100 Subject: [PATCH 07/12] package_module_wrapper.c: Added asserts for pointer arguments Signed-off-by: Lars Erik Wik --- libpromises/package_module_wrapper.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libpromises/package_module_wrapper.c b/libpromises/package_module_wrapper.c index 7dc4a68086d..f288396bf9f 100644 --- a/libpromises/package_module_wrapper.c +++ b/libpromises/package_module_wrapper.c @@ -52,7 +52,8 @@ void DeletePackageModuleWrapper(PackageModuleWrapper *wrapper) PackageModuleWrapper *NewPackageModuleWrapper(PackageModuleBody *package_module, int timeout_secs) { - assert(package_module && package_module->name); + assert(package_module != NULL); + assert(package_module->name != NULL); PackageModuleWrapper *wrapper = xmalloc(sizeof(PackageModuleWrapper)); @@ -128,6 +129,8 @@ PackageModuleWrapper *NewPackageModuleWrapper(PackageModuleBody *package_module, int PackageWrapperCommunicate(const PackageModuleWrapper *wrapper, const char *args, const char *request, Rlist **response) { + assert(wrapper != NULL); + char *all_args = NULL; if (wrapper->script_path != NULL) { @@ -203,6 +206,7 @@ static void GetPackageModuleExecInfo(const PackageModuleBody *package_module, ch char **script_exec_opts) { + assert(package_module != NULL); assert(exec_path != NULL); assert(script_path != NULL); assert(script_path_quoted != NULL); From cc474c0e29a4036607203422d4051566955c4516 Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Thu, 29 Oct 2026 14:11:14 +0100 Subject: [PATCH 08/12] Moved FreePackageManager() from eval_context.c to package_module_wrapper.c Signed-off-by: Lars Erik Wik --- libpromises/eval_context.c | 12 ------------ libpromises/package_module_wrapper.c | 12 ++++++++++++ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/libpromises/eval_context.c b/libpromises/eval_context.c index 0acf39b326f..5333fa7589b 100644 --- a/libpromises/eval_context.c +++ b/libpromises/eval_context.c @@ -1149,18 +1149,6 @@ static void StackFrameDestroy(StackFrame *frame) } } -static -void FreePackageManager(PackageModuleBody *manager) -{ - assert(manager != NULL); - - free(manager->name); - free(manager->interpreter); - free(manager->module_path); - RlistDestroy(manager->options); - free(manager); -} - static PackagePromiseContext *PackagePromiseConfigNew() { diff --git a/libpromises/package_module_wrapper.c b/libpromises/package_module_wrapper.c index f288396bf9f..06fbc641e1d 100644 --- a/libpromises/package_module_wrapper.c +++ b/libpromises/package_module_wrapper.c @@ -250,3 +250,15 @@ static void GetPackageModuleExecInfo(const PackageModuleBody *package_module, ch *script_exec_opts = NULL; } } + +static +void FreePackageManager(PackageModuleBody *manager) +{ + assert(manager != NULL); + + free(manager->name); + free(manager->interpreter); + free(manager->module_path); + RlistDestroy(manager->options); + free(manager); +} From 073fd6da537e9515cc2beda64e530dea50dd1a1d Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Thu, 29 Oct 2026 14:11:41 +0100 Subject: [PATCH 09/12] package_module_wrapper.c: Made FreePackageManager() public Signed-off-by: Lars Erik Wik --- libpromises/eval_context.c | 1 + libpromises/package_module_wrapper.c | 1 - libpromises/package_module_wrapper.h | 2 ++ 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/libpromises/eval_context.c b/libpromises/eval_context.c index 5333fa7589b..e0fab116b48 100644 --- a/libpromises/eval_context.c +++ b/libpromises/eval_context.c @@ -51,6 +51,7 @@ #include #include /* DataTypeIsIterable */ #include +#include /* If we need to put a scoped variable into a special scope, use the string * below to replace the original scope separator. diff --git a/libpromises/package_module_wrapper.c b/libpromises/package_module_wrapper.c index 06fbc641e1d..86599f189fb 100644 --- a/libpromises/package_module_wrapper.c +++ b/libpromises/package_module_wrapper.c @@ -251,7 +251,6 @@ static void GetPackageModuleExecInfo(const PackageModuleBody *package_module, ch } } -static void FreePackageManager(PackageModuleBody *manager) { assert(manager != NULL); diff --git a/libpromises/package_module_wrapper.h b/libpromises/package_module_wrapper.h index 0a6b12f0aac..71fc18cd3f8 100644 --- a/libpromises/package_module_wrapper.h +++ b/libpromises/package_module_wrapper.h @@ -86,4 +86,6 @@ int PackageWrapperCommunicate(const PackageModuleWrapper *wrapper, const char *a */ char *PackageModuleFormatOptions(Rlist *options); +void FreePackageManager(PackageModuleBody *manager); + #endif /* CFENGINE_PACKAGE_MODULE_WRAPPER_H */ From ca1ff8ee8473db521fd58b04b3a1fa9c035ef347 Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Thu, 29 Oct 2026 14:12:05 +0100 Subject: [PATCH 10/12] Renamed FreePackageManager() to PackageModuleBodyDestroy() Signed-off-by: Lars Erik Wik --- libpromises/eval_context.c | 2 +- libpromises/package_module_wrapper.c | 2 +- libpromises/package_module_wrapper.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libpromises/eval_context.c b/libpromises/eval_context.c index e0fab116b48..b2e2bb701e1 100644 --- a/libpromises/eval_context.c +++ b/libpromises/eval_context.c @@ -1158,7 +1158,7 @@ PackagePromiseContext *PackagePromiseConfigNew() package_promise_defaults->control_package_module = NULL; package_promise_defaults->control_package_inventory = NULL; package_promise_defaults->package_modules_bodies = - SeqNew(5, FreePackageManager); + SeqNew(5, PackageModuleBodyDestroy); return package_promise_defaults; } diff --git a/libpromises/package_module_wrapper.c b/libpromises/package_module_wrapper.c index 86599f189fb..6fa7b28423b 100644 --- a/libpromises/package_module_wrapper.c +++ b/libpromises/package_module_wrapper.c @@ -251,7 +251,7 @@ static void GetPackageModuleExecInfo(const PackageModuleBody *package_module, ch } } -void FreePackageManager(PackageModuleBody *manager) +void PackageModuleBodyDestroy(PackageModuleBody *manager) { assert(manager != NULL); diff --git a/libpromises/package_module_wrapper.h b/libpromises/package_module_wrapper.h index 71fc18cd3f8..638eadcf274 100644 --- a/libpromises/package_module_wrapper.h +++ b/libpromises/package_module_wrapper.h @@ -86,6 +86,6 @@ int PackageWrapperCommunicate(const PackageModuleWrapper *wrapper, const char *a */ char *PackageModuleFormatOptions(Rlist *options); -void FreePackageManager(PackageModuleBody *manager); +void PackageModuleBodyDestroy(PackageModuleBody *manager); #endif /* CFENGINE_PACKAGE_MODULE_WRAPPER_H */ From 52782b0b8e55bf38d7b94b03c0aa7f386bc2397a Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Thu, 29 Oct 2026 14:12:29 +0100 Subject: [PATCH 11/12] package_module_wrapper.c: PackageModuleBodyDestroy() now accepts NULL Signed-off-by: Lars Erik Wik --- libpromises/package_module_wrapper.c | 15 ++++++++------- libpromises/package_module_wrapper.h | 5 +++++ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/libpromises/package_module_wrapper.c b/libpromises/package_module_wrapper.c index 6fa7b28423b..267543ac101 100644 --- a/libpromises/package_module_wrapper.c +++ b/libpromises/package_module_wrapper.c @@ -253,11 +253,12 @@ static void GetPackageModuleExecInfo(const PackageModuleBody *package_module, ch void PackageModuleBodyDestroy(PackageModuleBody *manager) { - assert(manager != NULL); - - free(manager->name); - free(manager->interpreter); - free(manager->module_path); - RlistDestroy(manager->options); - free(manager); + if (manager != NULL) + { + free(manager->name); + free(manager->interpreter); + free(manager->module_path); + RlistDestroy(manager->options); + free(manager); + } } diff --git a/libpromises/package_module_wrapper.h b/libpromises/package_module_wrapper.h index 638eadcf274..5f0950b8a4e 100644 --- a/libpromises/package_module_wrapper.h +++ b/libpromises/package_module_wrapper.h @@ -86,6 +86,11 @@ int PackageWrapperCommunicate(const PackageModuleWrapper *wrapper, const char *a */ char *PackageModuleFormatOptions(Rlist *options); +/** + * @brief Destroys a package module body. + * + * @param manager The package module body, may be NULL + */ void PackageModuleBodyDestroy(PackageModuleBody *manager); #endif /* CFENGINE_PACKAGE_MODULE_WRAPPER_H */ From f381917a1a1176194a528b3278a4633c3a3cb740 Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Thu, 29 Oct 2026 14:12:53 +0100 Subject: [PATCH 12/12] package_module_wrapper.c: Added PackageModuleBodyCopy() Signed-off-by: Lars Erik Wik --- libpromises/package_module_wrapper.c | 14 ++++++++++++++ libpromises/package_module_wrapper.h | 8 ++++++++ 2 files changed, 22 insertions(+) diff --git a/libpromises/package_module_wrapper.c b/libpromises/package_module_wrapper.c index 267543ac101..cb94a54a221 100644 --- a/libpromises/package_module_wrapper.c +++ b/libpromises/package_module_wrapper.c @@ -251,6 +251,20 @@ static void GetPackageModuleExecInfo(const PackageModuleBody *package_module, ch } } +PackageModuleBody *PackageModuleBodyCopy(const PackageModuleBody *manager) +{ + assert(manager != NULL); + + PackageModuleBody *copy = xmalloc(sizeof(PackageModuleBody)); + copy->name = SafeStringDuplicate(manager->name); + copy->updates_ifelapsed = manager->updates_ifelapsed; + copy->installed_ifelapsed = manager->installed_ifelapsed; + copy->options = RlistCopy(manager->options); + copy->interpreter = SafeStringDuplicate(manager->interpreter); + copy->module_path = SafeStringDuplicate(manager->module_path); + return copy; +} + void PackageModuleBodyDestroy(PackageModuleBody *manager) { if (manager != NULL) diff --git a/libpromises/package_module_wrapper.h b/libpromises/package_module_wrapper.h index 5f0950b8a4e..365acd650fa 100644 --- a/libpromises/package_module_wrapper.h +++ b/libpromises/package_module_wrapper.h @@ -86,6 +86,14 @@ int PackageWrapperCommunicate(const PackageModuleWrapper *wrapper, const char *a */ char *PackageModuleFormatOptions(Rlist *options); +/** + * @brief Copies a package module body. + * + * @param manager The package module body + * @return The copy, to be destroyed with PackageModuleBodyDestroy() + */ +PackageModuleBody *PackageModuleBodyCopy(const PackageModuleBody *manager); + /** * @brief Destroys a package module body. *