Skip to content

CFE-4590, ENT-13239: Override default directory permissions #5931

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
olehermanse merged 5 commits into from
Nov 17, 2025
Merged

CFE-4590, ENT-13239: Override default directory permissions #5931

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b959b5f
Added test for overriding default directory create mode
larsewi Nov 4, 2025
59827e8
Skip remaining string comparisons after successful match
larsewi Nov 4, 2025
928eb80
Added default_directory_create_mode to body agent control
larsewi Nov 4, 2025
01e1a5a
Replaced hardcoded directory create mode with DEFAULTMODE
larsewi Nov 4, 2025
3b554e8
Make sure the last configuration wins
larsewi Nov 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions cf-agent/cf-agent.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1337,6 +1337,7 @@ static void KeepControlPromises(EvalContext *ctx, const Policy *policy, GenericA
{
Log(LOG_LEVEL_VERBOSE, "SET select_end_match_eof %s", (char *) value);
EvalContextSetSelectEndMatchEof(ctx, BooleanFromString(value));
continue;

@craigcomstock craigcomstock Nov 7, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? I don't understand the context or the reason for the change and what the change does.

I noticed around line 1141 there is another block that doesn't continue I wonder if this whole block of code needs to be audited for correct logic?

if (strcmp(cp->lval, CFA_CONTROLBODY[AGENT_CONTROL_ALLCLASSESREPORT].lval) == 0)

@larsewi larsewi Nov 7, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are checking if the string matches any of the possible attributes. Once we find a match, we don't need to check if it matches the remaining attributes, because we know implicitly that they will not match.

There is no change in behavior, just less CPU cycles wasted.

@nickanderson nickanderson Nov 7, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels like i am just showing what i dont know about the code

but, @larsewi your description there made me wonder if this would be affected:

body common control
{
   default_directory_create_mode => "000";
   default_directory_create_mode => "770";
}

It's a silly example, but in some policies the same attribtue is set differently in different contexts and multiple contexts might apply.

I just wanted to be sure that not continuing to check for the attribute will not affect this (in the above simple example, expect the last one to win).

@larsewi larsewi Nov 9, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these breaks will affect multiple configurations of the default directory create mode. However, I added a line in the policy to test this, just in case. I will merge once the acceptance test workflow from GitHub Actions have passed.

}

if (strcmp(cp->lval, CFA_CONTROLBODY[AGENT_CONTROL_REPORTCLASSLOG].lval) == 0)
Expand All @@ -1362,6 +1363,27 @@ static void KeepControlPromises(EvalContext *ctx, const Policy *policy, GenericA
{
EvalContextSetAgentEvalOrder(ctx, EVAL_ORDER_CLASSIC);
}
continue;
}

if (StringEqual(cp->lval, CFA_CONTROLBODY[AGENT_CONTROL_DEFAULT_DIRECTORY_CREATE_MODE].lval))
{
assert(value_type == CF_DATA_TYPE_STRING);
const char *mode_str = value;
mode_t plus, minus;
if (ParseModeString(mode_str, &plus, &minus))
{
DEFAULTMODE |= plus;
DEFAULTMODE &= ~minus;
Log(LOG_LEVEL_VERBOSE, "Changed default directory create mode to %ju "
"(default_directory_create_mode => \"%s\")", (uintmax_t) DEFAULTMODE, mode_str);
}
else
{
Log(LOG_LEVEL_ERR, "Failed to parse mode string for overriding default directory create mode "
"(default_directory_create_mode => \"%s\")", mode_str);
}
continue;
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion cf-agent/verify_files_utils.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1019,7 +1019,7 @@ static PromiseResult SourceSearchAndCopy(EvalContext *ctx, const char *from, cha

if ((!attr->copy.collapse) && (stat(newto, &dsb) == -1))
{
if (mkdir(changes_newto, 0700) == -1)
if (mkdir(changes_newto, DEFAULTMODE) == -1)
{
RecordInterruption(ctx, pp, attr, "Can't make directory '%s'. (mkdir: %s)",
newto, GetErrorStr());
Expand Down
3 changes: 1 addition & 2 deletions libpromises/cf3.defs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,8 +344,6 @@ struct GidList_
#define CF_NOINT -678L
#define CF_UNDEFINED_ITEM (void *)0x1234

#define DEFAULTMODE ((mode_t)0700)

#define CF_DONEPASSES 4

#define CFPULSETIME 60
Expand Down Expand Up @@ -496,6 +494,7 @@ typedef enum
AGENT_CONTROL_SELECT_END_MATCH_EOF,
AGENT_CONTROL_COPYFROM_RESTRICT_KEYS,
AGENT_CONTROL_EVALUATION_ORDER,
AGENT_CONTROL_DEFAULT_DIRECTORY_CREATE_MODE,
AGENT_CONTROL_NONE
} AgentControl;

Expand Down
2 changes: 2 additions & 0 deletions libpromises/cf3.extern.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ extern char VFQNAME[CF_MAXVARSIZE];
extern char VDOMAIN[CF_MAXVARSIZE / 2];
extern char VUQNAME[CF_MAXVARSIZE / 2];

extern mode_t DEFAULTMODE;

typedef enum EvalMode {
EVAL_MODE_NORMAL = 0, /* needs to be 'false' to work for DONTDO below */
EVAL_MODE_DRY_RUN = 1,
Expand Down
7 changes: 7 additions & 0 deletions libpromises/cf3globals.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,3 +154,10 @@ char BINDINTERFACE[CF_MAXVARSIZE]; /* GLOBAL_P */
- GenericAgentLoadPolicy (ReadPolicyValidatedFile)
*/
bool MINUSF = false; /* GLOBAL_A */

/*
Can be mutated in cf-agent.c (from control body)

Used as default directory create mode.
*/
mode_t DEFAULTMODE = (mode_t) 0700;
1 change: 1 addition & 0 deletions libpromises/mod_common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ const ConstraintSyntax CFA_CONTROLBODY[] =
ConstraintSyntaxNewBool("select_end_match_eof", "Set the default behavior of select_end_match_eof in edit_line promises. Default: false", SYNTAX_STATUS_NORMAL),
ConstraintSyntaxNewStringList("copyfrom_restrict_keys", ".*", "A list of key hashes to restrict copy_from to", SYNTAX_STATUS_NORMAL),
ConstraintSyntaxNewString("evaluation_order", "(classic|top_down)", "Order of evaluation of promises of agent", SYNTAX_STATUS_NORMAL),
ConstraintSyntaxNewString("default_directory_create_mode", ".*", "Default directory create mode (defaults to 0700 if not specified)", SYNTAX_STATUS_NORMAL),
ConstraintSyntaxNewNull()
};

Expand Down
72 changes: 72 additions & 0 deletions tests/acceptance/10_files/agent_control_default_directory_create_mode.cf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
##############################################################################
#
# Test overriding default directory create mode (CFE-4590, ENT-13239).
#
##############################################################################


body common control
{
inputs => { "../default.cf.sub" };
bundlesequence => { default("$(this.promise_filename)") };
version => "1.0";
}

body agent control {
# Override the default directory create mode to 0755 (it defaults to 0700 if
# not specified)
default_directory_create_mode => "0777"; # Make sure the last one wins
default_directory_create_mode => "a+rx"; # Can also use octets 0755
}

##############################################################################

bundle agent init
{
methods:
"clean";
}

##############################################################################

bundle agent test
{
meta:
"description" -> { "CFE-4590", "ENT-13239" }
string => "Test overriding default directory create mode";

files:
"$(G.testdir)/foo/config.txt"
create => "true",
perms => m(0655);
}

##############################################################################

bundle agent check
{
vars:
"expected"
string => "[0-9]*755",
comment => "We only care about the last three octets (i.e., 755)";
"actual"
string => filestat("$(G.testdir)/foo/", "modeoct");

methods:
"Pass/FAIL"
usebundle => dcs_check_regcmp("$(expected)", "$(actual)",
"$(this.promise_filename)", "false");

reports:
"Expected $(expected), actual $(actual)";
}

##############################################################################

bundle agent clean
{
files:
"$(G.testdir)/foo/."
delete => tidy,
if => fileexist("$(this.promiser)");
}
Loading