[9.0](backport #8503) fix(7794): update Windows re-enrollment logic and rename test#8571
Merged
[9.0](backport #8503) fix(7794): update Windows re-enrollment logic and rename test#8571
Conversation
* fix(7794): update Windows re-enrollment logic and rename test - Updated enroll.go to add a Windows-specific condition for root/Administrator privilege checks. - Removed Windows-specific file owner check implementation and its test. - Renamed integration test from enroll_unprivileged_test.go to re-enroll_test.go to better reflect its purpose. * fix(7794): implement no-op ownership check for Windows - Adds a Windows-specific implementation of isOwnerExec that always returns true (no-op), addressing build and re-enrollment issues. - Retains platform-specific files for clarity and maintainability. - Adds a Windows-specific test to ensure code coverage is maintained. - Removes the Windows OS check from enroll.go, relying on the no-op for Windows. * changelog: add fragment for Windows re-enrollment bugfix * fix(7794): remove unnecessary build tag * fix(7794): enhance re-enrollment tests with structured test cases - Renamed the test function to TestRenEnroll for clarity. - Introduced structured test cases for unprivileged and privileged agent re-enrollment scenarios. - Removed Windows-specific checks and consolidated assertions into the test cases. - Updated the test logic to improve readability and maintainability. * fix(7794): enhance re-enrollment test structure and OS handling * fix(7794): reverted to individual test functions because ci enforces where in the test function define is called * fix(7794): removed unnecessary var definition * fix(7794): fix bool value for privileged test case * fix(7794): added status check after second enroll, moved file owner error to enroll.go * fix(7794): remove unenroll step from integration tests * fix(7794): refactored test code * fix(7794): added comment explaining why windows is excluded from unprivileged test case (cherry picked from commit 00f1662)
github-actions
bot
added
the
Team:Elastic-Agent-Control-Plane
Contributor
|
Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane) |
kaanyalti
approved these changes
Jun 17, 2025
|
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
3 participants
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.
Type of change:
What does this PR do?
This PR updates the Windows enrollment logic in Elastic Agent, specifically addressing the overly strict file ownership checks that were introduced in a previous fix. Instead of removing the Windows-specific logic, this change implements the file ownership check as a no-op for Windows. The platform-specific files remain, but the check is now always allowed on Windows. This is a partial revert of the earlier logic, with the intent to restore expected behavior for privileged users on Windows.
Why is it important?
The original bug (see issue #4889, fixed in PR #6144) was that if the agent was installed unprivileged, running
enrollas a privileged user would break the agent. The fix at the time was to disallow re-enrollment as root if the agent was installed unprivileged, using a strict file owner check. This worked for Unix, where impersonation is easy, but on Windows, due to how theelastic-agent-useris set up, impersonation is not feasible.The strict SID check on Windows turned out to be too restrictive: even if the agent was installed as an admin, other admin users could not re-enroll, leading to the current issue (#7794). The workaround (changing file ownership to SYSTEM) was not reliable.
The changes in this PR implement the ownership check as a no-op for Windows, allowing any admin to re-enroll. The platform-specific files are retained for clarity and maintainability. This change also reintroduces the original risk: if the agent is installed unprivileged, a privileged user can still break the agent by re-enrolling. The platform-specific files are retained for clarity and maintainability.
Related issues
Checklist
[ ] I have commented my code, particularly in hard-to-understand areas[ ] I have made corresponding changes to the documentation[ ] I have made corresponding change to the default configuration files./changelog/fragmentsusing the changelog toolDisruptive User Impact
How to test this PR locally
re-enroll_test.go) to verify correct behavior.Related issues
This is an automatic backport of pull request #8503 done by [Mergify](https://mergify.com).