test: prove the non-root AgentX path in the distribution containers - #23
Conversation
The user documentation and the manual page named agentXPerms as the supported way to run the subagent without root. Nobody had run it, so an operator was following an untested instruction. A shared scenario now runs in every distribution container. It starts a real master, creates a system user and group, runs the packaged binary as that user with the packaged configuration, and walks ifStackTable. Both distribution test scripts call it after their install checks and before their removal checks. A packaging policy test finds every distribution script and requires the call, so a new distribution cannot skip it. The recipe in the first draft did not work. net-snmp applies chmod and chown to the AgentX socket only. It never changes the group of the parent directory, so a directory mode of 0770 left /var/agentx owned by root and group root, and the subagent could not traverse it. The directive now uses 0755 for the directory, which grants traverse, and the socket carries the access control as mode 0660 owned by root and the subagent group. net-snmp also creates the directory only when the bind fails with ENOENT, so an existing /var/agentx keeps its mode and the directory mode in the directive is ignored. The documents now say that, because an operator with a previous snmpd installation meets exactly that case. The scenario proves what it claims. It walks the table before the subagent starts and requires no rows, so the rows can only come from the child it started. It reads the real and effective user IDs of that child from /proc and rejects zero. It reads the container interfaces from /sys/class/net and requires the boundary rows of exactly those interfaces, so a fabricated walk cannot pass. It waits for the socket mode and ownership together with the socket itself, because net-snmp creates the socket before it applies them. Cleanup checks that the user and the group still exist before it removes them. On Debian, userdel removes an empty primary group with the same name, so the following groupdel failed and aborted the package test after the scenario reported success. The documents state the evidence boundary. The containers run no service manager, so the systemd drop-in, the supplementary group through systemd, and the unit sandbox are not exercised. The shipped unit keeps User=root. The distribution test scripts no longer install iproute, which no command in them used. Closes #19
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThe change documents non-root AgentX operation, adds a shared end-to-end permission and table-validation scenario, runs it from Debian and RPM package tests, and adds policy checks for the documentation, safety checks, process identity, and diagnostic output. ChangesNon-root AgentX operation
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Other Sequence Diagram(s)sequenceDiagram
participant PackageTest
participant Snmpd
participant AgentxIfstack
participant Snmpwalk
PackageTest->>Snmpd: start with AgentX socket permissions
Snmpd-->>PackageTest: create /var/agentx/master
PackageTest->>AgentxIfstack: start with setpriv as agentx-ifstack
AgentxIfstack->>Snmpd: register ifStackTable
PackageTest->>Snmpwalk: walk ifStackTable
Snmpwalk-->>PackageTest: return validated boundary rows
Merge Risk: ⚪ Minimal · up to The non-root package scenario is wired into both distribution test paths, and its RPM dependency is present. No concrete merge-blocking issue remains. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation Issue Resolution Parse and validate the installed manual page before creating the master configuration. Use the argument order obtained from that manual page, and fail the scenario when the manual is missing or does not provide the expected directive syntax. Full details: Docstring CoverageExplanation Docstring coverage is 63.64% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 4 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit reads each line, Comment |
1 participant
The user documentation and the manual page named agentXPerms as the
supported way to run the subagent without root. Nobody had run it, so an
operator was following an untested instruction.
A shared scenario now runs in every distribution container. It starts a
real master, creates a system user and group, runs the packaged binary as
that user with the packaged configuration, and walks ifStackTable. Both
distribution test scripts call it after their install checks and before
their removal checks. A packaging policy test finds every distribution
script and requires the call, so a new distribution cannot skip it.
The recipe in the first draft did not work. net-snmp applies chmod and
chown to the AgentX socket only. It never changes the group of the parent
directory, so a directory mode of 0770 left /var/agentx owned by root and
group root, and the subagent could not traverse it. The directive now uses
0755 for the directory, which grants traverse, and the socket carries the
access control as mode 0660 owned by root and the subagent group.
net-snmp also creates the directory only when the bind fails with ENOENT,
so an existing /var/agentx keeps its mode and the directory mode in the
directive is ignored. The documents now say that, because an operator with
a previous snmpd installation meets exactly that case.
The scenario proves what it claims. It walks the table before the subagent
starts and requires no rows, so the rows can only come from the child it
started. It reads the real and effective user IDs of that child from /proc
and rejects zero. It reads the container interfaces from /sys/class/net and
requires the boundary rows of exactly those interfaces, so a fabricated
walk cannot pass. It waits for the socket mode and ownership together with
the socket itself, because net-snmp creates the socket before it applies
them.
Cleanup checks that the user and the group still exist before it removes
them. On Debian, userdel removes an empty primary group with the same name,
so the following groupdel failed and aborted the package test after the
scenario reported success.
The documents state the evidence boundary. The containers run no service
manager, so the systemd drop-in, the supplementary group through systemd,
and the unit sandbox are not exercised. The shipped unit keeps User=root.
The distribution test scripts no longer install iproute, which no command
in them used.
Closes #19
Summary by CodeRabbit
Documentation
Testing