Add test that no AppArmor denied events are produced #3912
Conversation
As discussed in #3885, now that fixed Supervisor is in stable, we can test that no AppArmor denied events are logged during CI tests.
📝 WalkthroughWalkthroughThe pull request adds a new test function, Changes
Sequence Diagram(s)sequenceDiagram
participant Dependency as test_start_supervisor
participant Test as test_no_apparmor_denies
participant Shell as Shell Command Executor
Dependency-->Test: Dependency satisfied
Test->>Shell: Execute shell command to fetch audit logs
Shell-->>Test: Return log output
Test->>Test: Parse log for "DENIED" entries
alt "DENIED" entries found
Test-->>Test: Raise assertion error with details
else No "DENIED" entries
Test-->>Test: Complete test successfully
end
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
tests/supervisor_test/test_supervisor.py (4)
195-200: Good test implementation but consider enhancing for more specific AppArmor checking.The test successfully verifies that no AppArmor denial messages appear in the logs, which is valuable for ensuring proper configuration. However, there are a few potential enhancements that could make the test more robust:
Consider making the following improvements:
- More specifically target AppArmor denials by using a more precise grep pattern:
- output = shell.run_check("journalctl -t audit | grep DENIED || true") + output = shell.run_check("journalctl -t audit | grep 'apparmor=\"DENIED\"' || true")
- Add a time constraint to only check logs since the supervisor test started:
- output = shell.run_check("journalctl -t audit | grep DENIED || true") + output = shell.run_check("journalctl -t audit --since=\"$(systemctl show hassio_supervisor.service -p ActiveEnterTimestamp --value)\" | grep DENIED || true")
195-196: Consider moving this test earlier in the test sequence.This test depends on
test_start_supervisorbut is positioned after tests that have further dependencies. For better clarity on the logical progression of tests, consider positioning this test closer to other tests that directly depend ontest_start_supervisor.Consider moving this test to appear right after other tests that directly depend on
test_start_supervisor(like around line 46-55) to maintain a clearer dependency hierarchy.
197-197: Enhance the docstring with more context about AppArmor denials.While the docstring is clear about what the test does, adding more context about why AppArmor denials are problematic would improve documentation.
Consider expanding the docstring:
- """Check there are no AppArmor denies in the logs raised during Supervisor tests.""" + """Check there are no AppArmor denies in the logs raised during Supervisor tests. + + AppArmor denials indicate that the Supervisor is attempting operations that are blocked by + security policies, which could prevent proper functionality and should be addressed by + updating the AppArmor profiles or modifying the Supervisor's behavior. + """
199-199: Add more context to error messages for easier debugging.The current error message simply displays the raw output, which could be difficult to interpret if there are many denial entries.
Consider enhancing the error message to be more informative:
- assert not output, f"AppArmor denies found: {output}" + assert not output, f"AppArmor denies found in the system logs. This indicates the Supervisor is attempting operations blocked by security policies. Review these denials and update profiles accordingly:\n{output}"
| output = shell.run_check("journalctl -t audit | grep DENIED || true") | ||
| assert not output, f"AppArmor denies found: {output}" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider adding a verification step to ensure AppArmor is actually enabled.
The test assumes AppArmor is enabled, but it would be more robust to verify this first.
🏁 Script executed:
#!/bin/bash
# Check if AppArmor is enabled on the system
echo "Checking if AppArmor is enabled and running on the system..."
if command -v aa-status &> /dev/null; then
aa-status
echo "AppArmor profiles for the Supervisor:"
aa-status | grep -i supervisor || echo "No specific Supervisor profile found"
else
echo "AppArmor tools not installed or not in PATH"
cat /sys/module/apparmor/parameters/enabled 2>/dev/null || echo "AppArmor kernel module status not found"
fiLength of output: 385
Update Test to Verify AppArmor is Enabled
Based on the verification output, AppArmor is not active on the current system (the script showed that AppArmor tools are not installed and the kernel module is not enabled). To avoid relying on an assumption that AppArmor is enabled, please update the test in tests/supervisor_test/test_supervisor.py (lines 198–199) to first check whether AppArmor is active. For example, add a preliminary step that calls aa-status (or checks /sys/module/apparmor/parameters/enabled) and either skips the test or provides a clear warning if AppArmor is not enabled. This extra check will help prevent false negatives when running tests on systems without AppArmor.
As discussed in #3885, now that fixed Supervisor is in stable, we can test that no AppArmor denied events are logged during CI tests.
Summary by CodeRabbit