CFE-2318: Added findlocalusers() policy function #5714
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| #if defined(HAVE_GETPWENT) && !defined(__ANDROID__) | ||
|
|
||
| static FnCallResult FnCallFindLocalUsers(EvalContext *ctx, ARG_UNUSED const Policy *policy, const FnCall *fp, const Rlist *finalargs) |
Check warning
Code scanning / CodeQL
Poorly documented large function Warning
victormlg
force-pushed
the
CFE-2318-findusers
branch
2 times, most recently
from
1403e77 to
5a252f7
Compare
victormlg
force-pushed
the
CFE-2318-findusers
branch
from
5a252f7 to
97cde55
Compare
victormlg
force-pushed
the
CFE-2318-findusers
branch
from
97cde55 to
610913d
Compare
|
@cf-bottom jenkins, please |
|
Sure, I triggered a build (WITHOUT TESTS): Jenkins: https://ci.cfengine.com/job/build-and-deploy-docs-master/9/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-build-and-deploy-docs-master-9/ Documentation: http://buildcache.cfengine.com/packages/build-documentation-pr/jenkins-build-and-deploy-docs-master-9/output/_site/ |
| users: | ||
| "test_user1" | ||
| policy => "present", | ||
| home_dir => "/tmp/test_folder1", | ||
| description => "TestUser 1", | ||
| group_primary => "users", | ||
| shell => "/bin/sh", | ||
| uid => "12345"; | ||
|
|
||
| "test_user2" | ||
| policy => "present", | ||
| home_dir => "/tmp/test_folder2", | ||
| description => "TestUser 2", | ||
| group_primary => "users", | ||
| shell => "/bin/sh", | ||
| uid => "54321"; |
There was a problem hiding this comment.
This looks like an "unsafe" test. If I run the CFEngine acceptance tests, I don't want them to start editing /etc/passwd and adding users on my system. For those kinds of tests which make significant changes to the system they run on, we put them inside of a subdirectory called ./unsafe/ and you have to run the tests with UNSAFE_TESTS=1 to trigger them.
This is also why GitHub Actions is failing currently.
After moving this test, maybe you can make a shorter safe test which just tries to call the function with uid 0, or something. You won't be able to assert so much about the result, but it will be a lot less intrusive.
|
Jenkins CI with tests: https://ci.cfengine.com/job/pr-pipeline/11854/ |
olehermanse
changed the title
Changelog: Added findlocalusers policy function that returns all the local users matching certain attributes Ticket: CFE-2318 Signed-off-by: Victor Moene <[email protected]>
victormlg
force-pushed
the
CFE-2318-findusers
branch
from
610913d to
0c0a781
Compare
| return FnFailure(); | ||
| } | ||
|
|
||
| JsonElement *parent = JsonObjectCreate(10); |
There was a problem hiding this comment.
You will need to destroy this one before every statement that returns failure. Otherwise, the memory is leaked.
| Log(LOG_LEVEL_ERR, "Couldn't convert the uid of '%s' to string in function '%s'", | ||
| pw->pw_name, fp->name); | ||
| return FnFailure(); | ||
| } |
There was a problem hiding this comment.
I would still do this as a sanity check
| } | |
| } | |
| assert((size_t) val < sizeof(uid_string)); |
| else if (StringEqual(attribute, "uid")) | ||
| { | ||
| char uid_string[PRINTSIZE(pw->pw_uid) + 1]; | ||
| int val = snprintf(uid_string, sizeof(uid_string), "%u", pw->pw_uid); |
There was a problem hiding this comment.
It's common convention to name these variables ret or rc (for return code). But val is fine too
| "check" usebundle => dcs_check_state(test, | ||
| "$(this.promise_filename).expected.json", | ||
| $(this.promise_filename)); | ||
| } |
There was a problem hiding this comment.
Please add trailing newline :)
Merge together:
cfengine/documentation#3406
#5714