Move secrets to Azure Key Vault #738
Merged
Conversation
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
mjcheetham
force-pushed
the
keyvault
branch
4 times, most recently
from
643ddbd to
22a910a
Compare
dscho
reviewed
Mar 26, 2025
There was a problem hiding this comment.
Thank you so much! This is already looking great, my only remaining concern is how to ensure that the secret values are masked in the log output (in case future tired me adds set -x for debugging, for example). Once this is resolved, I think this is good to go!
mjcheetham
force-pushed
the
keyvault
branch
5 times, most recently
from
4e81bc4 to
030b28a
Compare
dscho
previously approved these changes
Apr 1, 2025
There was a problem hiding this comment.
Very nice!
I could imagine that the resulting workflow definition would be even nicer to read if the convention was introduced that in addition to setting outputs via NAME => OUTPUT, secrets could also be downloaded via NAME ENCODING> FILENAME (e.g. ${{ secrets.LINUX_GPG_PUBLIC_SECRET_NAME }} base64> msft-git-public.asc or ${{ secrets.WIN_CODESIGN_PASS_SECRET_NAME }} ascii> .sig/codesign.pass).
This would probably make even more sense in the context of using the same Action also in other workflow definitions.
What do you think?
dscho
reviewed
Apr 2, 2025
mjcheetham
force-pushed
the
keyvault
branch
2 times, most recently
from
4354a3f to
4af03aa
Compare
dscho
reviewed
Apr 3, 2025
Comment on lines
+11
to
+13
| env: | ||
| DO_WIN_CODESIGN: ${{ secrets.WIN_CODESIGN_CERT_SECRET_NAME != '' && secrets.WIN_CODESIGN_PASS_SECRET_NAME != '' }} | ||
| DO_WIN_GPGSIGN: ${{ secrets.WIN_GPG_KEYGRIP_SECRET_NAME != '' && secrets.WIN_GPG_PRIVATE_SECRET_NAME != '' && secrets.WIN_GPG_PASSPHRASE_SECRET_NAME != '' }} |
Add a new JavaScript GitHub Action to download secrets from Azure Key Vault using the `az` CLI, mask the secret values, and store them as: * outputs, * environment variables, or * files; Values are all masked for safe consumption by other steps in a workflow. Callers of this action can optionally perform base64 decoding of secret values using the syntax: `INPUT base64> OUTPUT`. It is assumed that the `az login` command has already been run prior to this action being invoked. Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Migrate secrets to Azure Key Vault. Names of the secrets in the AKV, and the AKV and Managed Identity IDs themselves are stored in GitHub environment secrets. This indirection allows for an easy way to re-point these to different Key Vault secrets without modifying the workflow file itself. In forks, this would allow others to use their own AKV and sercrets with the same workflow. Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Continue to migrate secrets to AKV for the macOS build steps. Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Continue to migrate secrets to AKV for the Debian package build steps. Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Use the `akv-secret` action rather than `az keyvault secret show | base64 -d` for downloading the Debian package public GPG key. Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
mjcheetham
added a commit
that referenced
this pull request
Apr 9, 2025
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.
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.
We should standardise on a location (and naming convention) to store secrets used as part of the build process for
microsoft/git. Azure Key Vault is an approved store for secrets inside of Microsoft, and so we should migrate any existing secrets from GitHub environment secrets to AKV. Access to the AKV is via Managed Identity and federated authentication through GitHub Actions.The names of the secrets, and other configuration for accessing the Key Vault, remain in GitHub environment secrets. Forks of the project can therefore define the same set of environment secrets pointing at their own AKV to utilise the same build process.
For reference, the set of secrets that must be defined for the workflow are as follows:
AZURE_VAULTAZURE_CLIENT_IDAZURE_TENANT_IDAZURE_SUBSCRIPTION_IDWIN_CODESIGN_CERT_SECRET_NAMEWIN_CODESIGN_PASS_SECRET_NAMEWIN_GPG_PRIVATE_SECRET_NAMEWIN_GPG_KEYGRIP_SECRET_NAMEWIN_GPG_PASSPHRASE_SECRET_NAMEAPPLE_APPCERT_SECRET_NAMEAPPLE_APPCERT_PASS_SECRET_NAMEAPPLE_INSTCERT_SECRET_NAMEAPPLE_INSTCERT_PASS_SECRET_NAMEAPPLE_TEAM_ID_SECRET_NAMEAPPLE_DEVELOPER_ID_SECRET_NAMEAPPLE_DEVELOPER_PASSWORD_SECRET_NAMEAPPLE_APPSIGN_ID_SECRET_NAMEAPPLE_INSTSIGN_ID_SECRET_NAMELINUX_GPG_PUBLIC_SECRET_NAMELINUX_GPG_PRIVATE_SECRET_NAMELINUX_GPG_PASSPHRASE_SECRET_NAMELINUX_GPG_KEYGRIP_SECRET_NAME