Introduce age verification helper functions to get the bracket#310
Merged
Introduce age verification helper functions to get the bracket#310
Conversation
Reviewer's GuideIntroduces new libutil age verification helper APIs that communicate with an external age daemon over a Unix domain socket, wires them into the build and installs corresponding manual pages and links. Sequence diagram for agev_get_age_bracket Unix socket interactionsequenceDiagram
participant App
participant libutil_agev as libutil_agev
participant aged_daemon as aged_daemon
App->>libutil_agev: agev_get_age_bracket(username)
libutil_agev->>libutil_agev: getpwnam(username)
alt user_not_found
libutil_agev-->>App: NULL (errno = EINVAL)
else user_found
libutil_agev->>aged_daemon: socket(AF_UNIX, SOCK_STREAM)
alt socket_failure
libutil_agev-->>App: NULL
else socket_ok
libutil_agev->>aged_daemon: connect(SOCKET_PATH)
alt connect_failure
libutil_agev->>aged_daemon: close(fd)
libutil_agev-->>App: NULL
else connect_ok
libutil_agev->>aged_daemon: write("GET <uid>")
libutil_agev->>aged_daemon: read(response)
alt response_contains_dash
libutil_agev->>libutil_agev: parse "min-max" to two ints
libutil_agev-->>App: int[2] ages
else invalid_response
libutil_agev-->>App: NULL
end
libutil_agev->>aged_daemon: close(fd)
end
end
end
Sequence diagram for privileged age updates via agev_set_age and agev_set_dobsequenceDiagram
participant App
participant libutil_agev as libutil_agev
participant aged_daemon as aged_daemon
rect rgb(235, 245, 255)
App->>libutil_agev: agev_set_age(username, age)
libutil_agev->>libutil_agev: geteuid()
alt not_root
libutil_agev-->>App: -1 (errno = EPERM)
else root
libutil_agev->>libutil_agev: getpwnam(username)
alt user_not_found
libutil_agev-->>App: -1 (errno = EINVAL)
else user_found
libutil_agev->>aged_daemon: socket(AF_UNIX, SOCK_STREAM)
alt socket_or_connect_failure
libutil_agev-->>App: -1
else connect_ok
libutil_agev->>aged_daemon: write("SET <uid> age <age>")
libutil_agev->>aged_daemon: close(fd)
libutil_agev-->>App: 0
end
end
end
end
rect rgb(235, 255, 235)
App->>libutil_agev: agev_set_dob(username, dob)
libutil_agev->>libutil_agev: geteuid()
alt not_root
libutil_agev-->>App: -1 (errno = EPERM)
else root
libutil_agev->>libutil_agev: getpwnam(username)
alt user_not_found
libutil_agev-->>App: -1 (errno = EINVAL)
else user_found
libutil_agev->>aged_daemon: socket(AF_UNIX, SOCK_STREAM)
alt socket_or_connect_failure
libutil_agev-->>App: -1
else connect_ok
libutil_agev->>aged_daemon: write("SET <uid> dob <dob>")
libutil_agev->>aged_daemon: close(fd)
libutil_agev-->>App: 0
end
end
end
end
Class diagram for agev age verification helper APIclassDiagram
class libutil_agev {
+int* agev_get_age_bracket(const char* username)
+int agev_set_age(const char* username, int age)
+int agev_set_dob(const char* username, const char* dob)
}
class aged_daemon {
+handle_get(uid_t uid)
+handle_set_age(uid_t uid, int age)
+handle_set_dob(uid_t uid, const char* dob)
}
libutil_agev ..> aged_daemon : uses Unix_socket
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 found 5 issues, and left some high level feedback:
- The socket I/O in agev.c does not check the return values of write(2) and assumes a single read(2) will yield a complete response; consider handling short writes/reads and propagating EIO or similar on protocol/transport errors instead of silently returning NULL/-1.
- agev_get_age_bracket() returns a heap-allocated int * without any bounds/validity checking on the parsed values or overflow handling; consider validating the parsed ages and using a small struct or explicit out-parameters to make ownership and size clearer.
- The HISTORY section of agev.3 references get_age_bracket, set_age, and set_dob, which do not match the exported function names agev_get_age_bracket, agev_set_age, and agev_set_dob; align these names to avoid confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The socket I/O in agev.c does not check the return values of write(2) and assumes a single read(2) will yield a complete response; consider handling short writes/reads and propagating EIO or similar on protocol/transport errors instead of silently returning NULL/-1.
- agev_get_age_bracket() returns a heap-allocated int * without any bounds/validity checking on the parsed values or overflow handling; consider validating the parsed ages and using a small struct or explicit out-parameters to make ownership and size clearer.
- The HISTORY section of agev.3 references get_age_bracket, set_age, and set_dob, which do not match the exported function names agev_get_age_bracket, agev_set_age, and agev_set_dob; align these names to avoid confusion.
## Individual Comments
### Comment 1
<location path="lib/libutil/agev.c" line_range="69-73" />
<code_context>
+ return NULL;
+ }
+
+ snprintf(buf, sizeof(buf), "GET %d", pw->pw_uid);
+ write(fd, buf, strlen(buf));
+
+ memset(buf, 0, sizeof(buf));
+ ssize_t n = read(fd, buf, sizeof(buf) - 1);
+
+ if (n > 0) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Handle partial/failed writes and reads when talking to the daemon.
`write()` and `read()` can both fail or transfer fewer bytes than requested (e.g., due to `EINTR`). Here the `write()` result is ignored, and `read()` is only checked for `> 0`, so requests/responses may be silently truncated or lost. Please loop until the full request is written (or an error occurs), check and handle `write()`’s return value, and treat short/negative reads as errors (propagating `errno` appropriately).
</issue_to_address>
### Comment 2
<location path="lib/libutil/agev.c" line_range="41-50" />
<code_context>
+#define SOCKET_PATH "/var/run/aged/aged.sock"
+
+int *
+agev_get_age_bracket(const char *username)
+{
+ int fd;
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Guard against NULL or empty usernames before calling getpwnam.
`username` is passed directly to `getpwnam()`, which has undefined behavior for NULL and may misbehave for an empty string. Add an explicit check for NULL/empty and return NULL with `errno = EINVAL` before calling `getpwnam()` to match the documented EINVAL semantics.
```suggestion
int *
agev_get_age_bracket(const char *username)
{
int fd;
struct sockaddr_un addr;
char buf[256] = {0};
int *ages = NULL;
struct passwd *pw;
if (username == NULL || username[0] == '\0') {
errno = EINVAL;
return NULL;
}
pw = getpwnam(username);
```
</issue_to_address>
### Comment 3
<location path="lib/libutil/agev.c" line_range="131-140" />
<code_context>
+}
+
+int
+agev_set_dob(const char *username, const char *dob)
+{
+ int fd;
+ struct sockaddr_un addr;
+ char buf[256] = {0};
+ struct passwd *pw;
+
+ if (geteuid() != 0) {
+ errno = EPERM;
+ return -1;
+ }
+
+ pw = getpwnam(username);
+ if (!pw) {
+ errno = EINVAL;
+ return -1;
+ }
+
+ if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
+ return -1;
+ }
+
+ memset(&addr, 0, sizeof(addr));
+ addr.sun_family = AF_UNIX;
+ strncpy(addr.sun_path, SOCKET_PATH, sizeof(addr.sun_path) - 1);
+
+ if (connect(fd, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
+ close(fd);
+ return -1;
+ }
+
+ snprintf(buf, sizeof(buf), "SET %d dob %s", pw->pw_uid, dob);
+ write(fd, buf, strlen(buf));
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider validating dob format and guarding against NULL/oversized strings.
The function accepts any `dob` string (including NULL) and writes it directly into the protocol message. Please validate `dob` as non-NULL, enforce a reasonable max length and strict `YYYY-MM-DD` formatting (e.g., 10 chars, digits + hyphens), and return `EINVAL` on failure to avoid malformed requests and unexpected characters in the protocol.
Suggested implementation:
```c
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <pwd.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <ctype.h>
```
```c
if (dob == NULL) {
errno = EINVAL;
return -1;
}
size_t dob_len = strlen(dob);
if (dob_len != 10 ||
!isdigit((unsigned char)dob[0]) ||
!isdigit((unsigned char)dob[1]) ||
!isdigit((unsigned char)dob[2]) ||
!isdigit((unsigned char)dob[3]) ||
dob[4] != '-' ||
!isdigit((unsigned char)dob[5]) ||
!isdigit((unsigned char)dob[6]) ||
dob[7] != '-' ||
!isdigit((unsigned char)dob[8]) ||
!isdigit((unsigned char)dob[9])) {
errno = EINVAL;
return -1;
}
if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
return -1;
}
memset(&addr, 0, sizeof(addr));
```
</issue_to_address>
### Comment 4
<location path="lib/libutil/agev.3" line_range="78-83" />
<code_context>
+.El
+.Sh SEE ALSO
+.Xr agectl 8
+.Sh HISTORY
+The
+.Fn get_age_bracket ,
+.Fn set_age ,
+and
+.Fn set_dob
+functions first appeared in MidnightBSD 4.0.
</code_context>
<issue_to_address>
**issue:** HISTORY section uses function names that don't match the public API.
This section references .Fn get_age_bracket, .Fn set_age, and .Fn set_dob, but the exported API (and the NAME section) use agev_get_age_bracket, agev_set_age, and agev_set_dob. Please update HISTORY to use the exported names to avoid confusing users.
</issue_to_address>
### Comment 5
<location path="lib/libutil/agev.c" line_range="56" />
<code_context>
+ return NULL;
+ }
+
+ if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
+ return NULL;
+ }
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting shared helpers for socket setup and common root/user lookup logic to eliminate repeated boilerplate in these functions while keeping behavior the same.
You can reduce the duplication without changing behavior by extracting two small internal helpers: one for socket setup and one for the “root + getpwnam” pattern used by the setters.
### 1. Factor out socket setup/connect
The socket code is repeated three times with identical behavior. A tiny static helper keeps the public functions focused on their command logic:
```c
static int
agev_open_socket(void)
{
int fd;
struct sockaddr_un addr;
fd = socket(AF_UNIX, SOCK_STREAM, 0);
if (fd == -1)
return -1;
memset(&addr, 0, sizeof(addr));
addr.sun_family = AF_UNIX;
strncpy(addr.sun_path, SOCKET_PATH, sizeof(addr.sun_path) - 1);
if (connect(fd, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
close(fd);
return -1;
}
return fd;
}
```
Usage in the three functions:
```c
int fd = agev_open_socket();
if (fd == -1)
return NULL; /* or -1, matching existing behavior */
/* existing snprintf/write/read/close logic stays unchanged */
```
This keeps all existing error handling semantics (just centralizes the implementation).
### 2. Factor out the “root + getpwnam” pattern
`agev_set_age` and `agev_set_dob` do identical root and user lookup checks:
```c
static int
agev_get_uid_for_update(const char *username, uid_t *uid_out)
{
struct passwd *pw;
if (geteuid() != 0) {
errno = EPERM;
return -1;
}
pw = getpwnam(username);
if (!pw) {
errno = EINVAL;
return -1;
}
*uid_out = pw->pw_uid;
return 0;
}
```
Then each setter becomes shorter and more focused:
```c
int
agev_set_age(const char *username, int age)
{
int fd;
char buf[256] = {0};
uid_t uid;
if (agev_get_uid_for_update(username, &uid) == -1)
return -1;
fd = agev_open_socket();
if (fd == -1)
return -1;
snprintf(buf, sizeof(buf), "SET %d age %d", uid, age);
write(fd, buf, strlen(buf));
close(fd);
return 0;
}
```
```c
int
agev_set_dob(const char *username, const char *dob)
{
int fd;
char buf[256] = {0};
uid_t uid;
if (agev_get_uid_for_update(username, &uid) == -1)
return -1;
fd = agev_open_socket();
if (fd == -1)
return -1;
snprintf(buf, sizeof(buf), "SET %d dob %s", uid, dob);
write(fd, buf, strlen(buf));
close(fd);
return 0;
}
```
`agev_get_age_bracket` can reuse `agev_open_socket()` and keep its current non-root `getpwnam()` behavior unchanged. This refactor removes the copy‑pasted plumbing while preserving the protocol and error semantics.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+69
to
+73
| snprintf(buf, sizeof(buf), "GET %d", pw->pw_uid); | ||
| write(fd, buf, strlen(buf)); | ||
|
|
||
| memset(buf, 0, sizeof(buf)); | ||
| ssize_t n = read(fd, buf, sizeof(buf) - 1); |
There was a problem hiding this comment.
issue (bug_risk): Handle partial/failed writes and reads when talking to the daemon.
write() and read() can both fail or transfer fewer bytes than requested (e.g., due to EINTR). Here the write() result is ignored, and read() is only checked for > 0, so requests/responses may be silently truncated or lost. Please loop until the full request is written (or an error occurs), check and handle write()’s return value, and treat short/negative reads as errors (propagating errno appropriately).
| return NULL; | ||
| } | ||
|
|
||
| if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) { |
There was a problem hiding this comment.
issue (complexity): Consider extracting shared helpers for socket setup and common root/user lookup logic to eliminate repeated boilerplate in these functions while keeping behavior the same.
You can reduce the duplication without changing behavior by extracting two small internal helpers: one for socket setup and one for the “root + getpwnam” pattern used by the setters.
1. Factor out socket setup/connect
The socket code is repeated three times with identical behavior. A tiny static helper keeps the public functions focused on their command logic:
static int
agev_open_socket(void)
{
int fd;
struct sockaddr_un addr;
fd = socket(AF_UNIX, SOCK_STREAM, 0);
if (fd == -1)
return -1;
memset(&addr, 0, sizeof(addr));
addr.sun_family = AF_UNIX;
strncpy(addr.sun_path, SOCKET_PATH, sizeof(addr.sun_path) - 1);
if (connect(fd, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
close(fd);
return -1;
}
return fd;
}Usage in the three functions:
int fd = agev_open_socket();
if (fd == -1)
return NULL; /* or -1, matching existing behavior */
/* existing snprintf/write/read/close logic stays unchanged */This keeps all existing error handling semantics (just centralizes the implementation).
2. Factor out the “root + getpwnam” pattern
agev_set_age and agev_set_dob do identical root and user lookup checks:
static int
agev_get_uid_for_update(const char *username, uid_t *uid_out)
{
struct passwd *pw;
if (geteuid() != 0) {
errno = EPERM;
return -1;
}
pw = getpwnam(username);
if (!pw) {
errno = EINVAL;
return -1;
}
*uid_out = pw->pw_uid;
return 0;
}Then each setter becomes shorter and more focused:
int
agev_set_age(const char *username, int age)
{
int fd;
char buf[256] = {0};
uid_t uid;
if (agev_get_uid_for_update(username, &uid) == -1)
return -1;
fd = agev_open_socket();
if (fd == -1)
return -1;
snprintf(buf, sizeof(buf), "SET %d age %d", uid, age);
write(fd, buf, strlen(buf));
close(fd);
return 0;
}int
agev_set_dob(const char *username, const char *dob)
{
int fd;
char buf[256] = {0};
uid_t uid;
if (agev_get_uid_for_update(username, &uid) == -1)
return -1;
fd = agev_open_socket();
if (fd == -1)
return -1;
snprintf(buf, sizeof(buf), "SET %d dob %s", uid, dob);
write(fd, buf, strlen(buf));
close(fd);
return 0;
}agev_get_age_bracket can reuse agev_open_socket() and keep its current non-root getpwnam() behavior unchanged. This refactor removes the copy‑pasted plumbing while preserving the protocol and error semantics.
laffer1
added a commit
that referenced
this pull request
Mar 18, 2026
* Introduce age verification helper functions to get the bracket
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 age verification helper functions for apps to call
Summary by Sourcery
Add age verification helper APIs and documentation to libutil for retrieving and updating user age information via a dedicated daemon.
New Features:
Build:
Documentation: