Skip to content

Update sac-enabler.ps1 to v1.3 - #126

Open
anmocanu wants to merge 26 commits into
Azure:mainfrom
anmocanu:patch-6
Open

Update sac-enabler.ps1 to v1.3#126
anmocanu wants to merge 26 commits into
Azure:mainfrom
anmocanu:patch-6

Conversation

@anmocanu

Copy link
Copy Markdown
Contributor

This script enables Special Administration Console (SAC) and Serial Console boot settings on an attached OS disk's BCD store. It includes enhanced logic for Gen2 disks and improved error handling.
.VERSION
v1.3: [May 2026] - Updated the script again (current)
- Fixed breaking exception when the Hyper-V module is not installed on the host.
- Added explicit checking via Get-Module before executing nested VM discovery.
v1.2: [May 2026] - Updated the script
- Included advanced Gen2 unlettered EFI fallback and dynamic drive-letter assignment.
v0.1: Initial commit. This was the version 1.0 of the script

This script enables Special Administration Console (SAC) and Serial Console boot settings on an attached OS disk's BCD store. It includes enhanced logic for Gen2 disks and improved error handling.
    .VERSION
    v1.3: [May 2026] - Updated the script again (current)
                       - Fixed breaking exception when the Hyper-V module is not installed on the host.
                       - Added explicit checking via Get-Module before executing nested VM discovery.
    v1.2: [May 2026] - Updated the script
                       - Included advanced Gen2 unlettered EFI fallback and dynamic drive-letter assignment.
    v0.1: Initial commit. This was the version 1.0 of the script
@EdwinBernal1
EdwinBernal1 self-requested a review June 16, 2026 13:58
@EdwinBernal1

EdwinBernal1 commented Jun 16, 2026

Copy link
Copy Markdown
Member

Issues Found

🟡 Potential compatibility gap: only checks winload.exe for OS detection

The script sets $isOsPath via \windows\system32\winload.exe only. Some UEFI-oriented scenarios rely on winload.efi path detection. If winload.exe is absent or not suitable in edge images, OS detection may fail even if the disk is valid.

Recommendation: mirror the pattern used in PR #130 and check both winload.exe and winload.efi.

🟢 Minor: fallback failure path could be clearer

If no default boot entry is parsed from displayorder, the script exits with generic failure state. Add explicit warning when $defaultLine/GUID extraction fails for easier troubleshooting.

anmocanu added 3 commits July 9, 2026 10:59
Updated script to enable SAC and Serial Console boot settings on attached Windows disks, improving OS detection and error handling.
Updated the script to improve logging and error handling, including better management of temporary EFI drive letters and enhanced status tracking.
@EdwinBernal1

Copy link
Copy Markdown
Member

@anmocanu could you help us with the review of these findings

Findings

Critical

File Context Issue Recommendation
None

Warning

File Context Issue Recommendation
sac-enabler.ps1 temp EFI letter diskpart assigns a temporary drive letter to the EFI partition. Cleanup is in finally (good), but a hard process kill between assign and cleanup could leave an orphaned mount on the rescue host. Acceptable; the finally guard is the right mitigation. Optionally add a startup sweep for stale temp letters.
sac-enabler.ps1 logging Desktop-only log, not the plugin dir auto-collected by az vm repair run. Dual-write for auto-collection (batch-level).
sac-enabler.ps1 Log-* wrapper Duplicated logging-wrapper pattern (see batch note). Extract shared helper.

Info

File Context Suggestion
sac-enabler.ps1 v2 helper + EFI Because Get-Disk-Partitions-v2 does not letter EFI partitions, the added diskpart logic is the correct compensation. Good Gen1+Gen2 coverage.
sac-enabler.ps1 paired diskpart assign/remove letter operations are paired across try/finally — good registry/partition hygiene.

Operational Risk Assessment

Factor Rating Notes
Scope Low-Medium Single script; adds diskpart partition manipulation.
Destructive ops Low BCD /set edits are reversible via bcdedit; temp letter add/remove only; no file/registry deletion.
Rollback possible Yes SAC/EMS settings can be reverted with bcdedit; .VERIFICATION documents checks.
Testing documented Yes (guide) .SCENARIO_RECREATION/.VERIFICATION provide a repro/validation path.
Gen compatibility Gen1+Gen2 Explicit Gen2 unlettered-EFI handling.

@glimoli

glimoli commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

VMRepair Script Update: sac-enabler.ps1

🎯 Summary

Comprehensive testing and validation of sac-enabler.ps1 with fault injection verification across multiple VM configurations.

✅ Test Results

Overall Score: 76/100 (Grade: B)

Metric Score Status
Test Success Rate 4/4 passed (100%)
Code Quality 91/100
Header Validation 62/100 ⚠️
Telemetry Coverage 0%

🔬 Testing Performed

Strategy: Representative (4 configurations)
Fault Injection: ✅ Enabled with breaker script validation
Region: westus2
Test Date: 2026-07-28

Test Configurations

  1. Modern Standard (Gen2) — Win2022Datacenter, Standard_D2s_v3, Premium_LRS
  2. Legacy Standard (Gen1) — Win2019Datacenter, Standard_D2s_v3, Standard_LRS
  3. Modern with Encryption — Win2022Datacenter, Standard_D2s_v3, Premium_LRS + Storage Encryption
  4. High-Performance — Win2022Datacenter, Standard_D4s_v3, Premium_LRS

Fault Injection Validation:

  • Breaker Script: break-win-sac-ems-corruption.ps1
  • Pre-repair state: ✅ Corruption verified
  • Post-repair state: ✅ Repair verified
  • All 4 configurations passed fault injection testing

📊 Detailed Test Artifacts

Complete test artifacts available in SME work item:

  • Interactive HTML Report: Full test execution details with fault injection validation
  • Improvement Recommendations: Actionable enhancement suggestions
  • Test Data Package: Complete logs, breaker scripts, validation outputs
  • Reproducibility Guide: JSON manifest for test replication

🔗 SME Work Item: Tooling 59896 win-sac-on

🚀 Improvements Recommended

1. ⚠️ Header Validation (62/100)

Current Issues:

  • Missing required header fields
  • Incomplete metadata

Proposed Fix:
Add/update the following header fields to meet VMRepair standards:

<#
.SYNOPSIS
    Enables Serial Admin Console (SAC) and Emergency Management Services (EMS) on Windows VMs

.DESCRIPTION
    Configures boot settings to enable SAC and EMS for out-of-band management and troubleshooting

.EXAMPLE
    .\sac-enabler.ps1

.NOTES
    Author: <name>
    Last Modified: 2026-07-28
    Version: 1.3
    
.LINK
    https://github.com/Azure/repair-script-library
#>

2. ❌ Telemetry Coverage (0%)

Critical Gap:
No telemetry instrumentation — production failures cannot be diagnosed or measured.

Proposed Fix:
Add comprehensive telemetry:

Script Start Event:

Log-Start -Message "Starting SAC/EMS enablement" -Properties @{
    OSVersion = (Get-CimInstance Win32_OperatingSystem).Version
    OSCaption = (Get-CimInstance Win32_OperatingSystem).Caption
    VMGeneration = if (Test-Path "EFI:\") { "V2" } else { "V1" }
    RepairScriptVersion = "1.3"
}

Operation Events:

# Before state capture
$beforeState = @{
    BootEms = (bcdedit /enum {bootmgr} | Select-String "bootems").ToString()
    EMS = (bcdedit /enum {default} | Select-String "ems").ToString()
}
Log-Info -Message "Before-state captured" -Properties $beforeState

# After each bcdedit operation
Log-Info -Message "Applied bcdedit command" -Properties @{
    Command = "bcdedit /set {bootmgr} bootems yes"
    ExitCode = $LASTEXITCODE
    Success = ($LASTEXITCODE -eq 0)
}

Success Event:

Log-Success -Message "SAC/EMS repair completed successfully" -Properties @{
    OperationsPerformed = $operationCount
    DurationSeconds = $executionTime
    BootEmsEnabled = $true
    EMSEnabled = $true
}

Error Handling:

try {
    # Repair logic here
} catch {
    Log-Error -Message "SAC/EMS enablement failed" -Exception $_ -Properties @{
        FailurePoint = "bcdedit operation"
        Command = $lastCommand
    }
    throw
}

🧪 Validation Evidence

Pre-Repair State (Corrupted):

EMS settings verified as disabled
Boot manager EMS: No
Default OS EMS: No
Status: CORRUPTED ✅ Verified

Post-Repair State (Fixed):

EMS settings verified as enabled
Boot manager EMS: Yes
Default OS EMS: Yes
Status: FIXED ✅ Verified

📋 Review Checklist

  • Script tested with fault injection across 4 configurations
  • Pre-repair corruption verified
  • Post-repair fix verified
  • Code quality: 91/100 (PSScriptAnalyzer clean)
  • Header validation improvements implemented
  • Telemetry instrumentation added
  • SME work item updated with test results
  • Test artifacts attached to SME work item

🔗 References

  • SME Work Item: 59896
  • Test Execution ID: sac-enabler.ps1-20260728-172934
  • Testing Framework: VMRepairMint
  • Test Subscription: d4895902-76c8-43c7-be78-f27ef254ebeb

🙏 Request for Review

@AzureVMRepairTeam — Please review the test results and improvement recommendations. The script is functionally sound (100% pass rate), but would benefit from header standardization and telemetry instrumentation for production observability.

Priority Improvements:

  1. 🔴 High: Add telemetry (0% → target 80%+)
  2. 🟡 Medium: Standardize header (62/100 → target 90+)

Generated by VMRepairMint Testing Framework
Test artifacts available in SME Work Item 59896

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.

3 participants