Add regulatory region to aged(8) and agectl(8)#327
Merged
Conversation
AI-Assisted-by: gemini pro Signed-off-by: Lucas Holt <luke@foolishgames.com>
AI-Assisted-by: Gemini 2.5 pro Signed-off-by: Lucas Holt <luke@foolishgames.com>
downloads anyway) switch to strncmp Signed-off-by: Lucas Holt <luke@foolishgames.com>
Reviewer's GuideIntroduce a configurable regulatory region for the aged(8) daemon, expose a root-only region-setting interface in agectl(8), gate age lookups based on the configured region (returning -2,-2 when verification is not allowed or not configured), and document the new behavior; also simplify the cross-bootstrap-tools CI matrix. Sequence diagram for agectl region setting and aged lookup behaviorsequenceDiagram
actor Admin
participant agectl
participant aged_daemon
participant sqlite_db
Admin->>agectl: invoke with -r region
agectl->>aged_daemon: connect UNIX socket
agectl->>aged_daemon: send REG region
aged_daemon->>sqlite_db: open aged.db
aged_daemon->>sqlite_db: UPDATE settings SET value = region WHERE key = 'region'
sqlite_db-->>aged_daemon: update result
aged_daemon-->>agectl: OK
agectl-->>Admin: display success
Admin->>User: system configured for region
actor User
User->>agectl: invoke without args
agectl->>aged_daemon: connect UNIX socket
agectl->>aged_daemon: send GET
aged_daemon->>sqlite_db: open aged.db
aged_daemon->>aged_daemon: load_region_setting
aged_daemon->>sqlite_db: SELECT value FROM settings WHERE key = 'region'
sqlite_db-->>aged_daemon: region value
aged_daemon->>aged_daemon: decide region_allowed
alt region_allowed == 0 or region unset
aged_daemon-->>agectl: -2,-2
else region_allowed == 1
aged_daemon->>sqlite_db: SELECT age FROM users WHERE uid = client_uid
sqlite_db-->>aged_daemon: age or no row
aged_daemon->>aged_daemon: derive age_range
aged_daemon-->>agectl: age_range
end
agectl-->>User: print response
ER diagram for aged SQLite users and settings tableserDiagram
USERS {
INTEGER uid PK
INTEGER age
TEXT dob
}
SETTINGS {
TEXT key PK
TEXT value
}
USERS ||..|| SETTINGS : uses_global_configuration
Flow diagram for aged GET handling with regulatory region gatingflowchart TD
A["Client sends GET to aged"] --> B["Open SQLite aged.db"]
B --> C["load_region_setting from settings table"]
C --> D{current_region is non NULL and in allowed set?
US-CA, US-CO, US-IL,
BR, parental}
D -- No --> E["Return -2,-2 to client"]
D -- Yes --> F["Prepare SELECT age FROM users WHERE uid = client_uid"]
F --> G{Row found?}
G -- No --> H["age = -1"]
G -- Yes --> I["age = stored value"]
H --> J["If uid < 1000 and age == -1 then age = 18"]
I --> J
J --> K["Map age to range string via get_range"]
K --> L["Return range string to client"]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Member
Author
|
Relates to #318 request to have a -2,-2 code for folks who don't set it. |
actions Signed-off-by: Lucas Holt <luke@foolishgames.com>
![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 2 issues, and left some high level feedback:
- The list of valid regions and which ones allow GET responses diverges between the code and the man pages (e.g., code checks "parental" and not "UK" in the allow-list, while the docs mention "UK" and "other"), so it would be good to make the documentation and the
is_valid_region/region_allowedlogic consistent. is_valid_region()usesstrncmp(region, valid_regions[i], MAX_REGION_LEN), which will treat any string with a valid prefix (e.g.,US-CA-foo) as valid; consider usingstrcmpor checking length to require an exact match.- In the
REGhandler you callsqlite3_prepare_v2without checking its return code before binding and stepping, unlike inload_region_setting; it would be safer to bail out on non-SQLITE_OKand avoid using an unprepared statement.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The list of valid regions and which ones allow GET responses diverges between the code and the man pages (e.g., code checks "parental" and not "UK" in the allow-list, while the docs mention "UK" and "other"), so it would be good to make the documentation and the `is_valid_region`/`region_allowed` logic consistent.
- `is_valid_region()` uses `strncmp(region, valid_regions[i], MAX_REGION_LEN)`, which will treat any string with a valid prefix (e.g., `US-CA-foo`) as valid; consider using `strcmp` or checking length to require an exact match.
- In the `REG` handler you call `sqlite3_prepare_v2` without checking its return code before binding and stepping, unlike in `load_region_setting`; it would be safer to bail out on non-`SQLITE_OK` and avoid using an unprepared statement.
## Individual Comments
### Comment 1
<location path="usr.sbin/aged/aged.c" line_range="226-233" />
<code_context>
syslog(LOG_INFO, "User information updated for uid %d", target_uid);
write(client_fd, "OK\n", 3);
}
+ } else if (client_uid == 0 && strncmp(buf, "REG ", 4) == 0) {
+ char *region = buf + 4;
+ char *newline = strchr(region, '\n');
+ if (newline) *newline = '\0';
+
+ if (is_valid_region(region)) {
+ sqlite3_stmt *stmt;
+ sqlite3_prepare_v2(db, "UPDATE settings SET value = ? WHERE key = 'region';", -1, &stmt, 0);
+ if (strcmp(region, "null") == 0) {
+ sqlite3_bind_null(stmt, 1);
</code_context>
<issue_to_address>
**issue (bug_risk):** Handle sqlite3_prepare_v2() failures before using stmt in the REG handler.
In this REG handler, the result of sqlite3_prepare_v2() is ignored. If prepare fails, stmt is invalid and subsequent sqlite3_bind_*/sqlite3_step/sqlite3_finalize calls invoke undefined behavior. Please capture and check rc, log on error, and return an error to the client without using stmt when rc != SQLITE_OK, following the pattern used in load_region_setting().
</issue_to_address>
### Comment 2
<location path="usr.sbin/aged/aged.c" line_range="314" />
<code_context>
return age;
}
+static int
+is_valid_region(const char *region) {
+ /* places that require age verification or will be. Not all can be supported */
</code_context>
<issue_to_address>
**issue (bug_risk):** Align valid region values and allowed-region behavior with the documented interface.
The current sets are inconsistent: `valid_regions` has "null" and "parental" but not "other" (manpage says "null" and "other" only), `is_valid_region()` accepts many regions (US-AL, EU, UK, etc.), while the GET path only treats US-CA, US-CO, US-IL, BR, and "parental" as enabling age verification. Aged(8) says UK, BR, US-CA, US-CO, and US-IL allow GET and anything else returns "-2,-2". Please reconcile code and docs on: (1) the canonical list of region strings; (2) which ones actually enable age verification/GET; and (3) the intended meaning and support of "other" vs "parental".
</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
+226
to
+233
| } else if (client_uid == 0 && strncmp(buf, "REG ", 4) == 0) { | ||
| char *region = buf + 4; | ||
| char *newline = strchr(region, '\n'); | ||
| if (newline) *newline = '\0'; | ||
|
|
||
| if (is_valid_region(region)) { | ||
| sqlite3_stmt *stmt; | ||
| sqlite3_prepare_v2(db, "UPDATE settings SET value = ? WHERE key = 'region';", -1, &stmt, 0); |
There was a problem hiding this comment.
issue (bug_risk): Handle sqlite3_prepare_v2() failures before using stmt in the REG handler.
In this REG handler, the result of sqlite3_prepare_v2() is ignored. If prepare fails, stmt is invalid and subsequent sqlite3_bind_*/sqlite3_step/sqlite3_finalize calls invoke undefined behavior. Please capture and check rc, log on error, and return an error to the client without using stmt when rc != SQLITE_OK, following the pattern used in load_region_setting().
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The set of valid region strings and the regions that permit age responses are inconsistent between code and manpages (e.g., code checks for "parental" and not "other" or "UK", while the docs list "other" and say UK should get ranges); it would be good to align the constants and behavior in
is_valid_region, theregion_allowedlogic, and the documented values. - Using
strncmp(..., MAX_REGION_LEN)for region comparisons can cause unintended matches for prefixes (e.g.,US-CA-foomatchingUS-CA); consider switching to exactstrcmpfor bothis_valid_regionand theregion_allowedchecks so only precise region codes are accepted. - The daemon calls
load_region_setting(db)for every client request even though the region is global state and only changes onREGupdates; you could simplify and reduce database load by loading the region once at startup and refreshing it only when aREGcommand succeeds.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The set of valid region strings and the regions that permit age responses are inconsistent between code and manpages (e.g., code checks for "parental" and not "other" or "UK", while the docs list "other" and say UK should get ranges); it would be good to align the constants and behavior in `is_valid_region`, the `region_allowed` logic, and the documented values.
- Using `strncmp(..., MAX_REGION_LEN)` for region comparisons can cause unintended matches for prefixes (e.g., `US-CA-foo` matching `US-CA`); consider switching to exact `strcmp` for both `is_valid_region` and the `region_allowed` checks so only precise region codes are accepted.
- The daemon calls `load_region_setting(db)` for every client request even though the region is global state and only changes on `REG` updates; you could simplify and reduce database load by loading the region once at startup and refreshing it only when a `REG` command succeeds.
## Individual Comments
### Comment 1
<location path="usr.sbin/aged/aged.c" line_range="47-56" />
<code_context>
#define SOCKET_PATH "/var/run/aged/aged.sock"
#define DB_PATH "/var/db/aged/aged.db"
#define RUN_USER "aged"
+#define MAX_REGION_LEN 10
static int calculate_age(const char *);
</code_context>
<issue_to_address>
**issue (bug_risk):** Region comparison using strncmp/MAX_REGION_LEN can accept unintended prefixes.
Using `strncmp(region, valid_regions[i], MAX_REGION_LEN)` will accept any string that merely starts with a valid region (e.g., `"US-CA-TEST"`, `"BRASIL"`), which can misclassify inputs. Use an exact match instead (e.g., `strcmp(region, valid_regions[i]) == 0`), optionally preceded by a `strlen(region) > MAX_REGION_LEN` check if you need to enforce a maximum length.
</issue_to_address>
### Comment 2
<location path="usr.sbin/aged/aged.c" line_range="231-233" />
<code_context>
+ char *newline = strchr(region, '\n');
+ if (newline) *newline = '\0';
+
+ if (is_valid_region(region)) {
+ sqlite3_stmt *stmt;
+ sqlite3_prepare_v2(db, "UPDATE settings SET value = ? WHERE key = 'region';", -1, &stmt, 0);
+ if (strcmp(region, "null") == 0) {
+ sqlite3_bind_null(stmt, 1);
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Lack of error checking for sqlite3_prepare_v2/sqlite3_step in the `REG` handler may hide failures.
In this path the return values of `sqlite3_prepare_v2` and `sqlite3_step` are ignored, so failures (locked DB, bad schema, etc.) will still result in logging a successful update and returning `"OK\n"` even when nothing was written. Please check these return codes and handle errors (log via `syslog` and return an error to the client) instead of assuming success.
</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
+47
to
56
| #define MAX_REGION_LEN 10 | ||
|
|
||
| static int calculate_age(const char *); | ||
| static void get_range(int, char *); | ||
| static void init_db(void); | ||
| static int is_valid_region(const char *); | ||
| static void load_region_setting(sqlite3 *); | ||
|
|
||
| static char *current_region = NULL; | ||
|
|
There was a problem hiding this comment.
issue (bug_risk): Region comparison using strncmp/MAX_REGION_LEN can accept unintended prefixes.
Using strncmp(region, valid_regions[i], MAX_REGION_LEN) will accept any string that merely starts with a valid region (e.g., "US-CA-TEST", "BRASIL"), which can misclassify inputs. Use an exact match instead (e.g., strcmp(region, valid_regions[i]) == 0), optionally preceded by a strlen(region) > MAX_REGION_LEN check if you need to enforce a maximum length.
Comment on lines
+231
to
+233
| if (is_valid_region(region)) { | ||
| sqlite3_stmt *stmt; | ||
| sqlite3_prepare_v2(db, "UPDATE settings SET value = ? WHERE key = 'region';", -1, &stmt, 0); |
There was a problem hiding this comment.
suggestion (bug_risk): Lack of error checking for sqlite3_prepare_v2/sqlite3_step in the REG handler may hide failures.
In this path the return values of sqlite3_prepare_v2 and sqlite3_step are ignored, so failures (locked DB, bad schema, etc.) will still result in logging a successful update and returning "OK\n" even when nothing was written. Please check these return codes and handle errors (log via syslog and return an error to the client) instead of assuming success.
in a region allowing that. Update documentation to be more accurate. 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.
Allows users to set the region they're in. This changes the aged(8) behavior as follows:
Returns -2,-2 if the user has not defined the region or opted out.
In US-CA, it would return -1, -1 since they are in a region requiring it.
Also added all the places I think have laws in place or are in the process of doing so. We don't support all of them and won't be able to in some cases. We may need to handle this in the future so the daemon will have a mechanism to do so.
Scenarios:
It's likely worse to 'lie' about doing verification in regions that require more than attesting. So we default to -2,-2 in those.
EU wants to make everyone there have an app on their phone that must be queried to check their age.
UK just said that attestation is no longer valid this year. So the aged(8) isn't compliant there anyway. Use -2,-2
BR isn't fully compliant but I think it's required to "try" to do something also. keep daemon on
AI-Assisted-by: Gemini 2.5 pro
Summary by Sourcery
Add configurable regulatory regions to the age verification daemon and CLI, gating responses based on region and documenting the new behavior.
New Features:
Enhancements:
CI:
Documentation: