Protect setup files and avoid publisher trust prompts - #103
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Signature verification currently fails on supported PowerShell versions before 7.4, and existing install roots may prevent reboot resume.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Secures Calm OS setup files and replaces AllSigned launches with verified, prompt-free RemoteSigned execution.
Changes:
- Adds signature and ACL validation.
- Installs under protected
%ProgramData%\CalmOS. - Updates bootstrap, relaunch behavior, and documentation.
File summaries
| File | Description |
|---|---|
src/windows-dev-config/steps/_security.ps1 |
Adds signature and permission checks. |
src/windows-dev-config/steps/_elevation.ps1 |
Uses RemoteSigned for relaunches. |
src/windows-dev-config/dev-config.ps1 |
Validates files before loading helpers. |
src/windows-dev-config/bootstrap.ps1 |
Adds protected download, installation, and elevation. |
src/windows-dev-config/README.md |
Documents the new security model. |
src/docs/development.md |
Updates contributor guidance. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
JohnMcPMS
left a comment
JohnMcPMS
left a comment
There was a problem hiding this comment.
Given the lack of support for the memory-based signature check, I think the path is:
- First run creates directory under install root, copies all scripts there.
- Changes owner and ACLs to admin only write with everyone read/execute
- Verifies signature on the entry point script one more time
- Invokes the entry point script with signature validation elevated
- Elevated script revalidates signatures for all files
- Continue with actual work
ranm-msft
left a comment
ranm-msft
left a comment
There was a problem hiding this comment.
Read this end to end against the head commit. The hardening looks solid, and several details are clearly deliberate: rejecting reparse points, walking every ancestor with DeleteSubdirectoriesAndFiles in the ancestor mask, creating the root atomically with its ACL instead of create-then-ACL, and having dev-config.ps1 verify and execute the same in-memory text of _security.ps1 so there is no file-swap window. I also confirmed the earlier AllSigned concern is resolved in 8c21cce - the pre-rejection is gone, _elevation.ps1 relaunches with RemoteSigned, and only an org-enforced Restricted still blocks.
Two things I would like your read on:
-
An existing install root never gets its ACL reconciled.
New-DevConfigProtectedDirectoryapplies the hardened ACL only insideif (-not (Test-Path -LiteralPath $Path)), andAssert-DevConfigProtectedTreeonly proves non-admins lack dangerous rights - never that the resume identity has traverse/read/execute. So a pre-existing Administrators-only%ProgramData%\CalmOSpasses every check and the elevated run finishes normally. But the resume task is registered-RunLevel Limited, and after the reboot PowerShell has to opendev-config.ps1before any in-script elevation runs, so it can fail before it ever reaches UAC or the setup log. Could the existing-root path reconcile a canonical ACL, or otherwise verify effective RX for the resume principal? (This expands the still-open Copilot comment on_security.ps1.) -
Resume identity under over-the-shoulder elevation.
Suspend-DevConfigForRebootreads[WindowsIdentity]::GetCurrent().Namewhile already elevated, so if a standard user starts setup and supplies a different administrator's credentials, the-AtLogOntrigger is registered for that administrator and will not fire when the original user signs back in. Is that scenario in scope?
Brings in bf74ae3 (microsoft#103, microsoft#105, microsoft#106, microsoft#107). .gitattributes keeps both sides: the fork's *.sh eol=lf rule and upstream's -text rules for the signed release copies.
- Last merged upstream commit is now bf74ae3. - The signed flow no longer depends on AllSigned; since microsoft#103 it checks signatures and an Administrator/SYSTEM-only folder instead. Both are skipped with -AllowUnsigned, so apply-ps-flow.ps1 keeps working (verified with -NoLaunch against the merged tree). - New caveat: upstream's -text rule checks out the Workloads\ and wsl-comfort\ signed copies as LF, so they are NotSigned in a fresh clone or ZIP.
No description provided.