Fix *nix permissions and use certificate_logical_to_actual#27385
Merged
Conversation
TravisEz13
previously approved these changes
May 1, 2026
The tarball staging path used `Copy-Item`, which on *nix doesn't preserve the source file mode, so `pwsh` ended up 644 in the `.tar.gz`. The Debian, RPM, and macOS PKG paths explicitly `chmod` everything to 644 and then bump `pwsh` back to 755, which silently demoted `createdump` (the .NET helper that produces crash minidumps) along with it. Now we `chmod 755` both executables in all package staging paths, guarded by `Test-Path` since fxdependent builds don't bundle `createdump`. Also added regression tests which check the permissions of `pwsh` inside the Linux and macOS tarballs before we upload them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… group The `CP-…` key codes used for ESRP signing are now set from ADO via the `certificate_logical_to_actual` variable group. The templates reference the following variables instead of literal codes: - `$(authenticode_cert_id)` - `$(authenticode_test_cert_id)` - `$(nuget_cert_id)` - `$(apple_cert_id)` - `$(pgp_linux_cert_id)` - `$(pgp_release_cert_id)` `nupkg.yml`, `mac-package-build.yml`, and `linux-package-build.yml` pick up the new group import. `linux-package-build.yml` also now selects the PGP signing profile based on whether `jobName` starts with `mariner`, so `PowerShell-Packages-Stages.yml` no longer threads a `signingProfile` parameter in for the two Mariner jobs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
6bffb1b to
2237859
Compare
It's been this way for a couple years which means we've been passing...something else?
Contributor
There was a problem hiding this comment.
Pull request overview
Updates packaging and pipeline signing configuration to (1) restore executable permissions in *nix tarball outputs and (2) replace hardcoded certificate/key codes with values sourced from the certificate_logical_to_actual variable group.
Changes:
- Restore
+xonpwsh(andcreatedumpwhen present) before producing tarball artifacts and in relevant Unix packaging flows. - Add pipeline regression checks that validate
pwshis executable inside produced.tar.gzartifacts. - Replace hardcoded signing
cp_code/KeyCode/PGP profile values with variable-group-backed IDs (e.g.,$(nuget_cert_id),$(apple_cert_id),$(pgp_linux_cert_id)).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tools/packaging/packaging.psm1 |
Ensures pwsh/createdump are marked executable in tarball + Unix packaging staging/targets. |
.pipelines/templates/windows-hosted-build.yml |
Uses $(nuget_cert_id) instead of a hardcoded NuGet signing code. |
.pipelines/templates/stages/PowerShell-Packages-Stages.yml |
Removes explicit Mariner signing profile parameter (now selected inside the linux package template). |
.pipelines/templates/shouldSign.yml |
Switches authenticode/MSIX cert selection to variable-group-backed IDs. |
.pipelines/templates/nupkg.yml |
Adds certificate_logical_to_actual group and uses $(nuget_cert_id) for signing. |
.pipelines/templates/mac.yml |
Uses $(apple_cert_id) for mac signing KeyCode. |
.pipelines/templates/mac-package-build.yml |
Adds tarball content check for pwsh executable bit; adds cert variable group for signing job. |
.pipelines/templates/linux-package-build.yml |
Fixes signedDrop param name, selects PGP signing profile via variable group, and adds tarball permission regression check. |
Member
Author
|
@TravisEz13 @jshigetomi this should also be good to go! |
Member
Author
|
@adityapatwardhan any issues? |
adityapatwardhan
requested changes
May 5, 2026
| # Included .NET executable for producing crash dumps | ||
| $createdumpInStaging = Join-Path $Staging 'createdump' | ||
| if (Test-Path -LiteralPath $createdumpInStaging) { | ||
| Start-NativeExecution { chmod 755 $createdumpInStaging } |
Member
There was a problem hiding this comment.
Should this be chmod a+x, so we dont change other bits.
Member
Author
There was a problem hiding this comment.
The proposed change is consistent with the existing choice to use numbered / explicit permissions in packaging.psm1 so I don't we should differ here until if and when we want to update the whole file (and run that through a barrage of testing). That said, I don't think I recommend that, because I posit that "don't change other bits" doesn't really apply here, since the root bug being addressed is that the pipeline moves the files from *nix where they were made (with correct permisison bits) to Windows which loses all sense of file permissions, that is, all the bits have already been changed, so updating them explicitly is I think more correct.
adityapatwardhan
approved these changes
May 5, 2026
9 tasks
9 tasks
This was referenced May 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Summary
Fixes #23968 (with regression tests) and finishes the certificate code abstraction.
Redux of #27354 which got merged too soon.
PR Context
In the pipelines we for reasons have to zip up the macOS and Linux builds on Windows which strips the executable permissions. We already had logic in the package scripts to correctly restore those permissions before building the RPM, DEB, and PKG packages, but not before packaging the tarball. Per the bug, it's been sadly broken for two years. You'd download the tarball, try to run
./pwshand it wouldn't be executable. Now it is, with a regression test that checks the actual file in the tarball before we upload it.PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright header