-
Notifications
You must be signed in to change notification settings - Fork 75
Handle missing windows home directory #1461
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
twiggler
commented
Dec 17, 2025
- Fix spliting home when there is no home
- Turn cache reader and writer into an iterator
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1461 +/- ##
==========================================
- Coverage 80.74% 80.27% -0.48%
==========================================
Files 390 392 +2
Lines 34217 34434 +217
==========================================
+ Hits 27629 27641 +12
- Misses 6588 6793 +205
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Miauwkeru
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels like the cache change should be in its own PR for the eventual git history to make more sense. As I wouldn't expect that a commit called "Handle missing windows home directory" would change the caching mechanism that hits all the plugins.
.vscode/launch.json
Outdated
| @@ -1,4 +1,5 @@ | |||
| { | |||
| "version": "0.2.0", | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this needed to be added
| assert users[1].home == windows_path("C:\\Users\\John") | ||
|
|
||
|
|
||
| def test_windows_user_registry_parsing(target_win_users: Target) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test doesn't fail if you use the previous code.
I was able to recreate the erroneous behaviour that it displays on a vm. Here i created a fake SID but it did not contain a ProfileImagePath RegistryValue. Causeing it to error and it would try to call target.resolve on home which is a None object. Shouldn't that be the behaviour this tests? / shouldn't we have a test for that specifically?