add or remove users from group memberships based on their age.#317
Merged
add or remove users from group memberships based on their age.#317
Conversation
AI-Assisted-by: gemini 2.5 pro Signed-off-by: Lucas Holt <luke@foolishgames.com>
Reviewer's GuideUpdate the aged daemon so that when a user’s age is updated, it synchronizes their membership in age-based system groups (age4p, age13p, age16p, age18p) by calling pw(8) to add or remove them from the corresponding groups and logging results via syslog, while silencing pw output. Sequence diagram for age update and group synchronizationsequenceDiagram
actor Client
participant AgedDaemon
participant SQLiteDB
participant PwTool as pw
participant Syslog
Client->>AgedDaemon: send age update request (uid, dob)
AgedDaemon->>SQLiteDB: UPDATE user age
SQLiteDB-->>AgedDaemon: update complete
AgedDaemon->>AgedDaemon: getpwuid(target_uid) -> username
alt username found
AgedDaemon->>AgedDaemon: update_age_groups(username, age)
loop for each age group
AgedDaemon->>AgedDaemon: getgrnam(group)
alt group exists
AgedDaemon->>AgedDaemon: check membership in grp->gr_mem
alt age >= min_age and not in_group
AgedDaemon->>PwTool: run_pw_command(group, username, -m)
PwTool->>PwTool: fork, execve(/usr/sbin/pw groupmod ...)
PwTool-->>AgedDaemon: exit status
alt success
AgedDaemon->>Syslog: LOG_INFO added user to group
else failure
AgedDaemon->>Syslog: LOG_ERR failed to add user
end
else age < min_age and in_group
AgedDaemon->>PwTool: run_pw_command(group, username, -d)
PwTool->>PwTool: fork, execve(/usr/sbin/pw groupmod ...)
PwTool-->>AgedDaemon: exit status
alt success
AgedDaemon->>Syslog: LOG_INFO removed user from group
else failure
AgedDaemon->>Syslog: LOG_ERR failed to remove user
end
end
else group missing
AgedDaemon->>Syslog: LOG_WARNING group not found
end
end
else username not found
AgedDaemon->>Syslog: LOG_ERR could not find username for uid
end
AgedDaemon-->>Client: OK
Class-style diagram for aged daemon functions related to age groupsclassDiagram
class AgedDaemon {
+main() int
+calculate_age(date_string const) int
+get_range(age int, buffer char*) void
+init_db() void
+update_age_groups(username const char*, age int) void
+run_pw_command(group const char*, user const char*, action const char*) int
}
class PwTool {
+groupmod(group const char*, action const char*, user const char*) int
}
class SystemDB {
+getpwuid(uid_t) struct passwd*
+getgrnam(group const char*) struct group*
}
class SyslogService {
+syslog(priority int, message const char*) void
}
AgedDaemon --> PwTool : uses
AgedDaemon --> SystemDB : queries
AgedDaemon --> SyslogService : logs
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
waitpidcall inrun_pw_commandshould handleEINTR(e.g., loop until it returns a child or a non-EINTR error) to avoid spuriously returning-1when the wait is interrupted by a signal. - In
update_age_groups, consider defensively checking thatgrp->gr_memis non-NULL before iterating over it, so that a group with no members (or an unusualgetgrnamimplementation) cannot lead to a NULL dereference.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `waitpid` call in `run_pw_command` should handle `EINTR` (e.g., loop until it returns a child or a non-EINTR error) to avoid spuriously returning `-1` when the wait is interrupted by a signal.
- In `update_age_groups`, consider defensively checking that `grp->gr_mem` is non-NULL before iterating over it, so that a group with no members (or an unusual `getgrnam` implementation) cannot lead to a NULL dereference.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
groups[]andmin_ages[]arrays are kept in parallel and must remain in sync; consider consolidating them into a single array of structs (e.g.,{name, min_age}) to make future changes less error-prone. - In
run_pw_command, whenexecvefails the parent only sees a generic non-zero status; you might want to log the failing command anderrnoin the child before_exit(127)to aid debugging of pw invocation issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `groups[]` and `min_ages[]` arrays are kept in parallel and must remain in sync; consider consolidating them into a single array of structs (e.g., `{name, min_age}`) to make future changes less error-prone.
- In `run_pw_command`, when `execve` fails the parent only sees a generic non-zero status; you might want to log the failing command and `errno` in the child before `_exit(127)` to aid debugging of pw invocation issues.
## Individual Comments
### Comment 1
<location path="usr.sbin/aged/aged.c" line_range="343" />
<code_context>
+ run_pw_command(const char *group, const char *user, const char *action)
</code_context>
<issue_to_address>
**suggestion:** Consider redirecting stdin to /dev/null as well when invoking `pw`.
Right now only stdout and stderr are redirected. If `pw groupmod` (or future `pw` calls) ever read from stdin, they’ll inherit the daemon’s stdin and may block or interfere with it. In the child, also dup2 `STDIN_FILENO` to `/dev/null` before `execve` so the subprocess is fully detached from the daemon’s stdio.
</issue_to_address>
### Comment 2
<location path="usr.sbin/aged/aged.c" line_range="374-381" />
<code_context>
+ int num_groups = sizeof(groups) / sizeof(groups[0]);
+
+ for (int i = 0; i < num_groups; i++) {
+ struct group *grp = getgrnam(groups[i]);
+ if (grp == NULL) {
+ syslog(LOG_WARNING, "Group %s not found.", groups[i]);
+ continue;
+ }
+
+ int in_group = 0;
+ for (int j = 0; grp->gr_mem[j] != NULL; j++) {
+ if (strcmp(grp->gr_mem[j], username) == 0) {
+ in_group = 1;
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against a potential NULL `gr_mem` pointer from `getgrnam` before iterating.
Some libc implementations set `gr_mem` to NULL for groups without members. In that case, `for (int j = 0; grp->gr_mem[j] != NULL; j++)` will dereference a NULL pointer. Please guard for `grp->gr_mem == NULL` before entering the loop and skip the iteration (e.g., set `in_group = 0` and continue) to avoid a potential crash.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| syslog(LOG_ERR, "fork failed: %m"); | ||
| return -1; | ||
| } else if (pid == 0) { | ||
| int fd = open("/dev/null", O_WRONLY); |
There was a problem hiding this comment.
suggestion: Consider redirecting stdin to /dev/null as well when invoking pw.
Right now only stdout and stderr are redirected. If pw groupmod (or future pw calls) ever read from stdin, they’ll inherit the daemon’s stdin and may block or interfere with it. In the child, also dup2 STDIN_FILENO to /dev/null before execve so the subprocess is fully detached from the daemon’s stdio.
Comment on lines
+374
to
+381
| struct group *grp = getgrnam(groups[i]); | ||
| if (grp == NULL) { | ||
| syslog(LOG_WARNING, "Group %s not found.", groups[i]); | ||
| continue; | ||
| } | ||
|
|
||
| int in_group = 0; | ||
| for (int j = 0; grp->gr_mem[j] != NULL; j++) { |
There was a problem hiding this comment.
issue (bug_risk): Guard against a potential NULL gr_mem pointer from getgrnam before iterating.
Some libc implementations set gr_mem to NULL for groups without members. In that case, for (int j = 0; grp->gr_mem[j] != NULL; j++) will dereference a NULL pointer. Please guard for grp->gr_mem == NULL before entering the loop and skip the iteration (e.g., set in_group = 0 and continue) to avoid a potential crash.
laffer1
added a commit
that referenced
this pull request
Mar 26, 2026
AI-Assisted-by: gemini 2.5 pro Signed-off-by: Lucas Holt <luke@foolishgames.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds to groups based on age range. This is for mport package manager use.
AI-Assisted-by: gemini 2.5 pro
Summary by Sourcery
Update aged daemon to maintain age-based Unix group memberships when user ages are updated.
New Features:
Enhancements: