8000 Fix various issues with build-git-installers.yml by mjcheetham · Pull Request #741 · microsoft/git · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix various issues with build-git-installers.yml #741

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Apr 9, 2025

Conversation

mjcheetham
Copy link
Member

There are several issues that have been uncovered with the changes made in #738. Let's fix them!

  • Check out akv-secret action before it is used.
  • Log in to Azure before accessing the Key Vault.
  • Don't mask empty lines.
  • Use a buffer to fix encoding issues when writing binary data.
  • Correctly mask multi-line secret values.
  • Add missing require('path') statement.

dscho and others added 6 commits April 8, 2025 08:37
This fixes "Error: path is not defined" issues in the workflow run.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Use a buffer rather than a string when handling the output from the `az`
command. Handling of binary data (that was base64 encoded) requires that
we use a buffer and not a string, or else we will end up writing invalid
data to files/output variables.

Introduce several new helper functions for working with buffers,
including trimming the EOL bytes (CR, LF), and fix up some of the
output functions to correctly validate the value passed - it must be
something printable (UTF-8-ish).

Finally ensure that we correctly mask multi-line secret values by
emitting a `::add-mask` command for each line.

Co-authored-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Don't mask empty lines since this isn't possible!

Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Since the Azure Portal supports only single-line secrets (but those
lines can be _very_ long), the convention is to store GPG keys (which
_are_ multi-line) as base64-encoded values.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
We need to log into Azure, and check out the custom Action, before we
can download secrets from the KeyVault via `akv-secret`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Ensure that we don't clobber existing directory with the checkout,
which in this instance contains downloaded artifacts!

Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
mjcheetham and others added 11 commits April 8, 2025 16:14
Use the global `DO_WIN_CODESIGN` environment variable rather than
re-compute this for the verification of codesigning step.

Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Replace Bash script (using `signtool`) for validating executables are
code-signed correctly with a PowerShell script (which instead uses the
`Get-AuthenticodeSignature` cmdlet).

The `signtool` is only available in the Windows SDK, which isn't always
installed on self-hosted runners (e.g., for ARM64), but PowerShell is
always available on our images.

Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
The custom Action is checked out at a different location than
`.github/`...

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
We need to log into Azure before we can access the KeyVault.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
We also need to quote the Apple App identity (because it contains a
parenthesis).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
We're not using quotes around simple string values like 'git' in other
places.. let's be consistent.

Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
The `akv-secret` Action must be checked out before it can be used.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Ensure that we don't clobber existing directory with the checkout,
which in this instance contains downloaded artifacts!

Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
The `akv-secret` Action must be checked out before it can be used.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Ensure that we don't clobber existing directory with the checkout,
which in this instance contains downloaded artifacts!

Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Add semi-colon to the `require('buffer')` line to stay consistent with
the rest of the index.js file.

Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Copy link
Member
@dscho dscho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes all look good to me!

@mjcheetham mjcheetham merged commit e92e23d into microsoft:vfs-2.49.0 Apr 9, 2025
63 checks passed
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
0