Android: fold security patch level into OS version - #49272
Conversation
|
@coderabbitai full review |
✅ Action performedFull review finished. |
WalkthroughAndroid OS version reporting now includes the security patch level when present, using a shared formatter across host updates, operating system upserts, and new host creation. Tests cover enrollment, status reports, host updates, formatted values, persisted patch-level rows, and fallback behavior when the patch level is absent. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/mdm/android/service/pubsub_test.go (1)
1036-1061: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test case for nil input.
Consider adding a test case to verify that
androidOSVersiongracefully handles anilpointer without panicking, corresponding to the suggested nil-guard fix.🧪 Proposed test case
{ name: "empty version with security patch level does not emit a dangling patch level", sw: &androidmanagement.SoftwareInfo{SecurityPatchLevel: "2026-05-01"}, expected: "", }, + { + name: "nil software info", + sw: nil, + expected: "", + }, } { t.Run(tc.name, func(t *testing.T) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/mdm/android/service/pubsub_test.go` around lines 1036 - 1061, The TestAndroidOSVersion table currently lacks coverage for nil input. Add a test case with sw set to nil and the expected empty string, verifying androidOSVersion handles a nil pointer without panicking while preserving the existing cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@server/mdm/android/service/pubsub_test.go`:
- Around line 1036-1061: The TestAndroidOSVersion table currently lacks coverage
for nil input. Add a test case with sw set to nil and the expected empty string,
verifying androidOSVersion handles a nil pointer without panicking while
preserving the existing cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d66ae85b-0bbb-47dd-9e2c-2780907ec7e5
📥 Commits
Reviewing files that changed from the base of the PR and between 422b2bb and 61bc7c50410f6c1a0677ec8fb74055d92c398604.
📒 Files selected for processing (3)
changes/47334-android-os-version-security-patch-levelserver/mdm/android/service/pubsub.goserver/mdm/android/service/pubsub_test.go
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 35075-software-os-show-android-versions-and-vulnerabilities #49272 +/- ##
===============================================================================================
+ Coverage 67.97% 67.99% +0.01%
===============================================================================================
Files 3801 3805 +4
Lines 239965 240664 +699
Branches 12693 12693
===============================================================================================
+ Hits 163123 163634 +511
- Misses 62058 62164 +106
- Partials 14784 14866 +82
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Relates to #47334 Capture SoftwareInfo.securityPatchLevel from the Android Management API device report and fold it into the host's OS version, so Android versions read as "Android 16 (2026-05-01)" instead of just "Android 16". This makes the operating_systems row distinct per patch level, which is the vulnerability-relevant granularity for Android (AMAPI exposes no minor version). Both the enroll path (addNewHost) and the detail-report path (updateHost), plus the operating_systems upsert, now go through a shared androidOSVersion helper. Devices that don't report a patch level fall back to the bare major version, so they still enroll without error.
b64ffd6 to
2225fd2
Compare
6a32085
into
35075-software-os-show-android-versions-and-vulnerabilities
Relates to #47334
Capture SoftwareInfo.securityPatchLevel from the Android Management API device report and fold it into the host's OS version, so Android versions read as "Android 16 (2026-05-01)" instead of just "Android 16". This makes the operating_systems row distinct per patch level, which is the vulnerability-relevant granularity for Android (AMAPI exposes no minor version).
Both the enroll path (addNewHost) and the detail-report path (updateHost), plus the operating_systems upsert, now go through a shared androidOSVersion helper. Devices that don't report a patch level fall back to the bare major version, so they still enroll without error.
Checklist for submitter
If some of the following don't apply, delete the relevant line.
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Testing