Skip to content

[AI Generated] Ensure correct DSC export of admin settings#6109

Merged
JohnMcPMS merged 10 commits intomicrosoft:masterfrom
Trenly:copilot/ms-5881-fix-issue-5881
Mar 31, 2026
Merged

[AI Generated] Ensure correct DSC export of admin settings#6109
JohnMcPMS merged 10 commits intomicrosoft:masterfrom
Trenly:copilot/ms-5881-fix-issue-5881

Conversation

@Trenly
Copy link
Copy Markdown
Contributor

@Trenly Trenly commented Mar 26, 2026

Copilot Summary

winget dsc export --all exports all admin settings as false even when enabled. Root cause: SecureSettingsContainer doesn't verify that verification hash writes succeed, causing subsequent reads to throw unhandled exceptions.

Changes

AdminSettings.cpp

  • Added exception handling around m_settingStream.Get() to catch verification failures
  • Falls back to default values with error logging instead of crashing

Settings.cpp

  • Changed SetVerificationData() to return bool indicating write success
  • SecureSettingsContainer::Set() now checks verification write result
  • Reverts main write if verification write fails (atomic operation)
  • Added diagnostic logging for verification failures

Technical Detail

SecureSettingsContainer stores settings in one file and SHA256 hash in another. Write path was:

bool Set(value) {
    bool result = ExchangeSettingsContainer::Set(value);
    if (result) {
        SetVerificationData(hash);  // Return value ignored - bug
    }
    return result;
}

Now enforces atomicity:

bool Set(value) {
    bool result = ExchangeSettingsContainer::Set(value);
    if (result) {
        if (!SetVerificationData(hash)) {
            Remove();  // Revert
            return false;
        }
    }
    return result;
}

Read path now handles verification exceptions gracefully rather than leaving settings at default values.

Microsoft Reviewers: Open in CodeFlow

@Trenly Trenly requested a review from a team as a code owner March 26, 2026 04:28
@Trenly Trenly changed the title Copilot/ms 5881 fix issue 5881 [AI Generated] Ensure correct DSC export of admin settings Mar 26, 2026
@JohnMcPMS
Copy link
Copy Markdown
Member

Should have test that intentionally corrupts the verification file of the admin settings to confirm that you get default values out.

@JohnMcPMS JohnMcPMS added the Needs-Author-Feedback Issue needs attention from issue or PR author label Mar 27, 2026
Agent-Logs-Url: https://github.com/Trenly/winget-cli/sessions/31c7cc4d-743a-4f9d-9ff3-38156f36f278

Co-authored-by: Trenly <12611259+Trenly@users.noreply.github.com>
@Trenly
Copy link
Copy Markdown
Contributor Author

Trenly commented Mar 27, 2026

Updated

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Attention Issue needs attention from Microsoft and removed Needs-Author-Feedback Issue needs attention from issue or PR author labels Mar 27, 2026
@JohnMcPMS
Copy link
Copy Markdown
Member

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@JohnMcPMS
Copy link
Copy Markdown
Member

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@Trenly
Copy link
Copy Markdown
Contributor Author

Trenly commented Mar 30, 2026

Huh. . . I tested this locally and had no test failures. Looking into it. . .

@Trenly
Copy link
Copy Markdown
Contributor Author

Trenly commented Mar 30, 2026

@JohnMcPMS - I'm terribly confused here, the tests are passing on my local with the exception of VerifyIsSameVolume which always fails on my machine and I expect to be failing

image

@JohnMcPMS
Copy link
Copy Markdown
Member

Packaged vs unpackaged??

Yes, when packaged the basic settings streams are not stored in files but in the app settings (which basically end up being in an offline registry hive I think...).

@JohnMcPMS
Copy link
Copy Markdown
Member

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@JohnMcPMS JohnMcPMS merged commit 650256c into microsoft:master Mar 31, 2026
9 checks passed
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Attention Issue needs attention from Microsoft label Mar 31, 2026
@Trenly Trenly deleted the copilot/ms-5881-fix-issue-5881 branch March 31, 2026 16:59
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.

winget dsc export --all has wrong values for Admin Settings

3 participants