feat: add unattend script and tests for ISO build#13
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughAutounattend.xml was simplified (removed ProductKey and specialize/Extensions). A new validate-unattend.ps1 was added and is invoked from build-iso.ps1 to schema- and policy-validate the unattend file (optionally inspecting Source ISO). Tests and docs updated to reflect these changes. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Build as build-iso.ps1
participant Validate as validate-unattend.ps1
participant XML as autounattend.xml
participant Schema as Schema DLL
participant ISO as Source ISO
User->>Build: run build-iso.ps1
activate Build
Build->>Build: verify files & hashes
Build->>Validate: call validate-unattend.ps1 (-UnattendPath, -SourceISO)
activate Validate
Validate->>XML: load XML
XML-->>Validate: provide content
Validate->>Schema: locate & extract schema (DLL)
Schema-->>Validate: schema text
Validate->>Validate: validate XML against schema
Validate->>Validate: enforce policy checks (ProductKey, DiskConfiguration/InstallTo, FirstLogonCommands)
opt Source ISO provided
Validate->>ISO: mount ISO
ISO-->>Validate: confirm install.wim/install.esd
Validate->>Validate: inspect image metadata (Get-WindowsImage)
Validate->>ISO: dismount ISO
end
Validate-->>Build: validation passed
deactivate Validate
Build->>Build: continue ISO build (oscdimg etc.)
Build-->>User: ISO creation complete
deactivate Build
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a65f97aaa8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| foreach ($entry in $resourceSet) { | ||
| if ($entry.Name -like 'Unattend*') { |
There was a problem hiding this comment.
In Get-UnattendSchemaString, iterating a ResourceSet yields dictionary entries (Key/Value), so checking $entry.Name never matches and the loop falls through to Unable to extract unattend schema. That makes unattend validation fail even when the WSIM DLL is present, and because build-iso.ps1 now always invokes this script, ISO builds are blocked in normal usage. Use the entry key field when matching Unattend* resources.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
build-iso.ps1 (1)
194-195: Consider validatingvalidate-unattend.ps1existence before invocation.The script path is defined but not checked for existence like other required files. If the validation script is missing, the error at line 235 will be less clear than a dedicated file-not-found message.
🔧 Suggested fix
$validateUnattendScript = Join-Path $ScriptRoot "validate-unattend.ps1" + if (-not (Test-Path $validateUnattendScript)) { + throw "Required file not found: validate-unattend.ps1" + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build-iso.ps1` around lines 194 - 195, The script defines $validateUnattendScript but doesn't verify it exists before invocation; add a Test-Path check for $validateUnattendScript (similar to other required-file checks), and if missing call Write-Error (or throw) with a clear "validate-unattend.ps1 not found" message so the failure is explicit before you attempt to run the script.validate-unattend.ps1 (2)
94-98: Consider renaming event handler parameters to avoid analyzer warnings.The parameters
$senderand$eventArgstrigger PSScriptAnalyzer warnings because they resemble automatic variables. While functionally correct, renaming improves clarity.🔧 Suggested fix
$validationErrors = [System.Collections.Generic.List[string]]::new() - $eventHandler = [System.Xml.Schema.ValidationEventHandler]{ - param($sender, $eventArgs) - $validationErrors.Add($eventArgs.Message) + $eventHandler = [System.Xml.Schema.ValidationEventHandler]{ + param($s, $e) + $validationErrors.Add($e.Message) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@validate-unattend.ps1` around lines 94 - 98, Rename the parameters of the ValidationEventHandler block to avoid PSScriptAnalyzer automatic-variable warnings: update the event handler assigned to $eventHandler so it uses non-automatic names (e.g., $source and $args or $senderObj and $event) instead of $sender and $eventArgs, and update the handler body to reference the new name (for example, replace $eventArgs.Message with $args.Message) while leaving $validationErrors and the handler assignment unchanged.
106-112:StringReaderis not disposed.The
StringReadercreated for the schema text should be disposed. WhileStringReaderdoesn't hold unmanaged resources, following theIDisposablepattern consistently is good practice.🔧 Suggested fix
- $schemaReader = [System.Xml.XmlReader]::Create([System.IO.StringReader]::new($SchemaText)) + $schemaStringReader = [System.IO.StringReader]::new($SchemaText) + $schemaReader = [System.Xml.XmlReader]::Create($schemaStringReader) try { $settings.Schemas.Add('urn:schemas-microsoft-com:unattend', $schemaReader) | Out-Null } finally { $schemaReader.Dispose() + $schemaStringReader.Dispose() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@validate-unattend.ps1` around lines 106 - 112, The StringReader created inline for the schema text isn't disposed; assign it to a variable (e.g., $stringReader = [System.IO.StringReader]::new($SchemaText)), pass that to [System.Xml.XmlReader]::Create($stringReader) to create $schemaReader, and ensure you dispose the StringReader in the same finally block (call $schemaReader.Dispose() and $stringReader.Dispose() or use PowerShell's using/try/finally pattern) so both $schemaReader and the underlying StringReader are properly cleaned up when calling $settings.Schemas.Add('urn:schemas-microsoft-com:unattend', $schemaReader).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@build-iso.ps1`:
- Around line 194-195: The script defines $validateUnattendScript but doesn't
verify it exists before invocation; add a Test-Path check for
$validateUnattendScript (similar to other required-file checks), and if missing
call Write-Error (or throw) with a clear "validate-unattend.ps1 not found"
message so the failure is explicit before you attempt to run the script.
In `@validate-unattend.ps1`:
- Around line 94-98: Rename the parameters of the ValidationEventHandler block
to avoid PSScriptAnalyzer automatic-variable warnings: update the event handler
assigned to $eventHandler so it uses non-automatic names (e.g., $source and
$args or $senderObj and $event) instead of $sender and $eventArgs, and update
the handler body to reference the new name (for example, replace
$eventArgs.Message with $args.Message) while leaving $validationErrors and the
handler assignment unchanged.
- Around line 106-112: The StringReader created inline for the schema text isn't
disposed; assign it to a variable (e.g., $stringReader =
[System.IO.StringReader]::new($SchemaText)), pass that to
[System.Xml.XmlReader]::Create($stringReader) to create $schemaReader, and
ensure you dispose the StringReader in the same finally block (call
$schemaReader.Dispose() and $stringReader.Dispose() or use PowerShell's
using/try/finally pattern) so both $schemaReader and the underlying StringReader
are properly cleaned up when calling
$settings.Schemas.Add('urn:schemas-microsoft-com:unattend', $schemaReader).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: adbb184d-b995-416f-a431-9b865faffb0d
📒 Files selected for processing (6)
autounattend.xmlbuild-iso.ps1tests/Autounattend.Tests.ps1tests/BuildIso.Tests.ps1tests/ValidateUnattend.Tests.ps1validate-unattend.ps1
💤 Files with no reviewable changes (1)
- autounattend.xml
Timpan4
force-pushed
the
feat-addatt-script
branch
from
a65f97a to
3b0da13
Compare
Timpan4
force-pushed
the
feat-addatt-script
branch
from
3b0da13 to
9c2795e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b0da13551
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| # Validate autounattend.xml before doing any expensive ISO work | ||
| Write-Step "Validating autounattend.xml" | ||
| & $validateUnattendScript -UnattendPath $requiredFiles["autounattend.xml"] -SourceISO $SourceISO |
There was a problem hiding this comment.
build-iso.ps1 now always runs validate-unattend.ps1 before locating/downloading oscdimg, but that validator throws when the WSIM schema DLL is not installed locally (Find-UnattendSchemaDll in validate-unattend.ps1). In environments that intentionally use -OscdimgDownloadUrl without a full ADK install (documented in docs/ISO-GENERATION.md as supported), this change blocks ISO creation with no way to pass -SchemaDllPath through build-iso.ps1, so a previously supported workflow now fails.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c2795e340
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try { | ||
| Set-RegistryValueSafe -Path "HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\Communications" -Name "ConfigureChatAutoInstall" -Value 0 -Type DWord | ||
| Set-RegistryValueSafe -Path "HKLM:\SOFTWARE\Microsoft\PolicyManager\current\device\Start" -Name "ConfigureStartPins" -Value '{"pinnedList":[]}' -Type String | ||
| Set-RegistryValueSafe -Path "HKU:\.DEFAULT\Control Panel\Accessibility\StickyKeys" -Name "Flags" -Value "10" -Type String |
There was a problem hiding this comment.
Set-RegistryValueSafe is called with HKU:\.DEFAULT\..., but default Windows PowerShell sessions only mount HKLM: and HKCU: registry drives, so this path throws “drive not found.” In this function all registry writes are wrapped in one try, so that exception prevents the remaining HKCU tweaks (HideFileExt, TaskbarAl, etc.) from running, leaving the new post-install tweak step partially/non-functional on standard setups.
Useful? React with 👍 / 👎.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation