Skip to content

Fixed Admin By Request false positive CVEs - #42095

Merged
getvictor merged 2 commits into
mainfrom
victor/41586-admin-by-request
Mar 20, 2026
Merged

Fixed Admin By Request false positive CVEs#42095
getvictor merged 2 commits into
mainfrom
victor/41586-admin-by-request

Conversation

@getvictor

@getvictor getvictor commented Mar 19, 2026

Copy link
Copy Markdown
Member

Related issue: Resolves #41586

The fix is speculative since I wasn't able to find/get the "Admin By Request" app to install.

Checklist for submitter

If some of the following don't apply, delete the relevant line.

  • Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.

Testing

  • Added/updated automated tests

Summary by CodeRabbit

  • Bug Fixes
    • Fixed false-positive vulnerability reports for "Admin By Request" on macOS and Linux. Vulnerabilities CVE-2019-17201 and CVE-2019-17202 are Windows-specific and will now only be reported on Windows systems, eliminating unnecessary alerts on other platforms.

@getvictor

Copy link
Copy Markdown
Member Author

@claude review

@getvictor

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Mar 19, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Full review triggered.

@coderabbitai

coderabbitai Bot commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

This PR addresses false-positive vulnerability reports for Admin By Request on macOS and Linux by suppressing CVE-2019-17201 and CVE-2019-17202, which are Windows-specific. The changes include a new CPEMatchingRule in the NVD vulnerability matching logic that filters out these CVEs when the target software is not Windows, a corresponding test case validating platform-specific CVE matching behavior, and a changelog entry documenting the fix.

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning PR description includes issue reference and partially completes the template checklist, but omits most required sections and verification steps. Complete missing sections: input validation details, backend compatibility changes, QA verification, testing methodology, and database migration checks where applicable.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main change: adding a fix for false-positive Admin By Request CVEs by implementing platform-specific matching rules.
Linked Issues check ✅ Passed The code changes directly address the linked issue #41586 by implementing platform-specific CVE matching rules for Admin By Request, ensuring Windows-only CVEs are not reported for macOS and Linux installations.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the Admin By Request false positive issue through changes file documentation, CPE matching rules, and targeted test coverage.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch victor/41586-admin-by-request
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@server/vulnerabilities/nvd/cpe_matching_rules.go`:
- Around line 327-329: The IgnoreIf closure currently returns cpeMeta.TargetSW
!= "windows", which also suppresses CVEs when TargetSW is empty/ANY; update the
IgnoreIf in cpe_matching_rules.go so it only ignores when TargetSW is a known
non-Windows value (i.e., not empty and not "ANY" and not "windows"). Concretely,
change the predicate on IgnoreIf (the function taking cpeMeta *wfn.Attributes)
to first ensure cpeMeta.TargetSW is non-empty and not "ANY", and then check it
is not "windows" so unknown/ANY TargetSW will not be suppressed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e42f2887-9e8a-47ec-a233-f137953d9ea2

📥 Commits

Reviewing files that changed from the base of the PR and between 0b15204 and 8b6a4d6.

📒 Files selected for processing (3)
  • changes/41586-admin-by-request-false-positive
  • server/vulnerabilities/nvd/cpe_matching_rule_test.go
  • server/vulnerabilities/nvd/cpe_matching_rules.go

Comment thread server/vulnerabilities/nvd/cpe_matching_rules.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses false-positive vulnerability detections for Admin By Request on non-Windows hosts by adding an NVD CPE-matching exception rule, ensuring the affected CVEs are only considered applicable on Windows.

Changes:

  • Added a known NVD bug rule to ignore CVE-2019-17201 and CVE-2019-17202 unless the CPE target software is Windows.
  • Added unit test coverage to verify the rule behavior across macOS/Linux/Windows target_sw values.
  • Added a changelog entry documenting the false-positive fix.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File Description
server/vulnerabilities/nvd/cpe_matching_rules.go Adds a CPE matching rule to treat the ABR CVEs as Windows-only by filtering on target_sw.
server/vulnerabilities/nvd/cpe_matching_rule_test.go Adds tests asserting the ABR CVEs match only for Windows target_sw.
changes/41586-admin-by-request-false-positive Documents the user-visible fix in a changes entry.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov

codecov Bot commented Mar 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.46%. Comparing base (f19cc81) to head (fc817a1).
⚠️ Report is 43 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #42095      +/-   ##
==========================================
+ Coverage   66.40%   66.46%   +0.06%     
==========================================
  Files        2513     2516       +3     
  Lines      201457   201941     +484     
  Branches     9060     9060              
==========================================
+ Hits       133772   134225     +453     
+ Misses      55583    55570      -13     
- Partials    12102    12146      +44     
Flag Coverage Δ
backend 68.25% <100.00%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@getvictor
getvictor marked this pull request as ready for review March 19, 2026 20:20
@getvictor
getvictor requested a review from a team as a code owner March 19, 2026 20:20

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review.

@getvictor
getvictor merged commit 6d6a29a into main Mar 20, 2026
45 of 48 checks passed
@getvictor
getvictor deleted the victor/41586-admin-by-request branch March 20, 2026 15:35
@coderabbitai coderabbitai Bot mentioned this pull request Mar 24, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

False positive Admin by request for mac

3 participants