Skip to content

Commit e92e23d

Browse files
authored
Fix various issues with build-git-installers.yml (#741)
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.
2 parents 88e9410 + 6a7e382 commit e92e23d

File tree

2 files changed

+152
-54
lines changed

2 files changed

+152
-54
lines changed

.github/actions/akv-secret/index.js

Lines changed: 95 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
const { spawnSync } = require('child_process');
22
const fs = require('fs');
33
const os = require('os');
4+
const path = require('path');
5+
const { isUtf8 } = require("buffer");
46

57
// Note that we are not using the `@actions/core` package as it is not available
68
// without either committing node_modules/ to the repository, or using something
@@ -14,6 +16,35 @@ const escapeData = (s) => {
1416
.replace(/\n/g, '%0A')
1517
}
1618

19+
const stringify = (value) => {
20+
if (typeof value === 'string') return value;
21+
if (Buffer.isBuffer(value) && isUtf8(value)) return value.toString('utf-8');
22+
return undefined;
23+
}
24+
25+
const trimEOL = (buf) => {
26+
let l = buf.length
27+
if (l > 0 && buf[l - 1] === 0x0a) {
28+
l -= l > 1 && buf[l - 2] === 0x0d ? 2 : 1
29+
}
30+
return buf.slice(0, l)
31+
}
32+
33+
const writeBufToFile = (buf, file) => {
34+
out = fs.createWriteStream(file)
35+
out.write(buf)
36+
out.end()
37+
}
38+
39+
const logInfo = (message) => {
40+
process.stdout.write(`${message}${os.EOL}`);
41+
}
42+
43+
const setFailed = (error) => {
44+
process.stdout.write(`::error::${escapeData(error.message)}${os.EOL}`);
45+
process.exitCode = 1;
46+
}
47+
1748
const writeCommand = (file, name, value) => {
1849
// Unique delimiter to avoid conflicts with actual values
1950
let delim;
@@ -28,24 +59,38 @@ const writeCommand = (file, name, value) => {
2859
}
2960

3061
const setSecret = (value) => {
31-
process.stdout.write(`::add-mask::${escapeData(value)}${os.EOL}`);
62+
value = stringify(value);
63+
64+
// Masking a secret that is not a valid UTF-8 string or buffer is not useful
65+
if (value === undefined) return;
66+
67+
process.stdout.write(
68+
value
69+
.split(/\r?\n/g)
70+
.filter(line => line.length > 0) // Cannot mask empty lines
71+
.map(
72+
value => `::add-mask::${escapeData(value)}${os.EOL}`
73+
)
74+
.join('')
75+
);
3276
}
3377

3478
const setOutput = (name, value) => {
79+
value = stringify(value);
80+
if (value === undefined) {
81+
throw new Error(`Output value '${name}' is not a valid UTF-8 string or buffer`);
82+
}
83+
3584
writeCommand(process.env.GITHUB_OUTPUT, name, value);
3685
}
3786

3887
const exportVariable = (name, value) => {
39-
writeCommand(process.env.GITHUB_ENV, name, value);
40-
}
41-
42-
const logInfo = (message) => {
43-
process.stdout.write(`${message}${os.EOL}`);
44-
}
88+
value = stringify(value);
89+
if (value === undefined) {
90+
throw new Error(`Environment variable '${name}' is not a valid UTF-8 string or buffer`);
91+
}
4592

46-
const setFailed = (error) => {
47-
process.stdout.write(`::error::${escapeData(error.message)}${os.EOL}`);
48-
process.exitCode = 1;
93+
writeCommand(process.env.GITHUB_ENV, name, value);
4994
}
5095

5196
(async () => {
@@ -67,9 +112,7 @@ const setFailed = (error) => {
67112

68113
// Fetch secrets from Azure Key Vault
69114
for (const { input: secretName, encoding, output } of secretMappings) {
70-
let secretValue = '';
71-
72-
const az = spawnSync('az',
115+
let az = spawnSync('az',
73116
[
74117
'keyvault',
75118
'secret',
@@ -92,10 +135,12 @@ const setFailed = (error) => {
92135
if (az.error) throw new Error(az.error, { cause: az.error });
93136
if (az.status !== 0) throw new Error(`az failed with status ${az.status}`);
94137

95-
secretValue = az.stdout.toString('utf-8').trim();
138+
// az keyvault secret show --output tsv returns a buffer with trailing \n
139+
// (or \r\n on Windows), so we need to trim it specifically.
140+
let secretBuf = trimEOL(az.stdout);
96141

97142
// Mask the raw secret value in logs
98-
setSecret(secretValue);
143+
setSecret(secretBuf);
99144

100145
// Handle encoded values if specified
101146
// Sadly we cannot use the `--encoding` parameter of the `az keyvault
@@ -105,31 +150,46 @@ const setFailed = (error) => {
105150
if (encoding) {
106151
switch (encoding.toLowerCase()) {
107152
case 'base64':
108-
secretValue = Buffer.from(secretValue, 'base64').toString();
153+
secretBuf = Buffer.from(secretBuf.toString('utf-8'), 'base64');
154+
break;
155+
case 'ascii':
156+
case 'utf8':
157+
case 'utf-8':
158+
// No need to decode the existing buffer from the az command
109159
break;
110160
default:
111-
// No decoding needed
112-
}
161+
throw new Error(`Unsupported encoding: ${encoding}`);
162+
}
113163

114-
setSecret(secretValue); // Mask the decoded value as well
164+
// Mask the decoded value
165+
setSecret(secretBuf);
115166
}
116167

117-
if (output.startsWith('$env:')) {
118-
// Environment variable
119-
const envVarName = output.replace('$env:', '').trim();
120-
exportVariable(envVarName, secretValue);
121-
logInfo(`Secret set as environment variable: ${envVarName}`);
122-
} else if (output.startsWith('$output:')) {
123-
// GitHub Actions output variable
124-
const outputName = output.replace('$output:', '').trim();
125-
setOutput(outputName, secretValue);
126-
logInfo(`Secret set as output variable: ${outputName}`);
127-
} else {
128-
// File output
129-
const filePath = output.trim();
130-
fs.mkdirSync(path.dirname(filePath), { recursive: true });
131-
fs.writeFileSync(filePath, secretValue);
132-
logInfo(`Secret written to file: ${filePath}`);
168+
const outputType = output.startsWith('$env:')
169+
? 'env'
170+
: output.startsWith('$output:')
171+
? 'output'
172+
: 'file';
173+
174+
switch (outputType) {
175+
case 'env':
176+
const varName = output.replace('$env:', '').trim();
177+
exportVariable(varName, secretBuf);
178+
logInfo(`Secret set as environment variable: ${varName}`);
179+
break;
180+
181+
case 'output':
182+
const outputName = output.replace('$output:', '').trim();
183+
setOutput(outputName, secretBuf);
184+
logInfo(`Secret set as output variable: ${outputName}`);
185+
break;
186+
187+
case 'file':
188+
const filePath = output.trim();
189+
fs.mkdirSync(path.dirname(filePath), { recursive: true });
190+
writeBufToFile(secretBuf, filePath);
191+
logInfo(`Secret written to file: ${filePath}`);
192+
break;
133193
}
134194
}
135195
})().catch(setFailed);

.github/workflows/build-git-installers.yml

Lines changed: 57 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,9 @@ jobs:
132132
with:
133133
vault: ${{ secrets.AZURE_VAULT }}
134134
secrets: |
135-
${{ secrets.WIN_GPG_KEYGRIP_SECRET_NAME }} > $output:keygrip
136-
${{ secrets.WIN_GPG_PRIVATE_SECRET_NAME }} > $output:private-key
137-
${{ secrets.WIN_GPG_PASSPHRASE_SECRET_NAME }} > $output:passphrase
135+
${{ secrets.WIN_GPG_KEYGRIP_SECRET_NAME }} > $output:keygrip
136+
${{ secrets.WIN_GPG_PRIVATE_SECRET_NAME }} base64> $output:private-key
137+
${{ secrets.WIN_GPG_PASSPHRASE_SECRET_NAME }} > $output:passphrase
138138
- name: Prepare home directory for GPG signing
139139
if: ${{ steps.gpg-secrets.outputs.keygrip != '' && steps.gpg-secrets.outputs.private-key != '' }}
140140
shell: bash
@@ -218,10 +218,22 @@ jobs:
218218
shell: bash
219219
run: |
220220
git clone --filter=blob:none --single-branch -b main https://github.com/git-for-windows/build-extra /usr/src/build-extra
221+
- name: Log in to Azure
222+
uses: azure/login@v2
223+
if: env.DO_WIN_CODESIGN == 'true'
224+
with:
225+
client-id: ${{ secrets.AZURE_CLIENT_ID }}
226+
tenant-id: ${{ secrets.AZURE_TENANT_ID }}
227+
subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
228+
- name: Check out repository (for akv-secret Action)
229+
if: env.DO_WIN_CODESIGN == 'true'
230+
uses: actions/checkout@v4
231+
with:
232+
path: git
221233
- name: Download code signing secrets
222234
id: codesign-secrets
223235
if: env.DO_WIN_CODESIGN == 'true'
224-
uses: ./.github/actions/akv-secret
236+
uses: ./git/.github/actions/akv-secret
225237
with:
226238
vault: ${{ secrets.AZURE_VAULT }}
227239
secrets: |
@@ -344,13 +356,22 @@ jobs:
344356
fi &&
345357
openssl dgst -sha256 artifacts/${{matrix.type.fileprefix}}-*.exe | sed "s/.* //" >artifacts/sha-256.txt
346358
- name: Verify that .exe files are code-signed
347-
env:
348-
DO_CODE_SIGN: ${{ secrets.WIN_CODESIGN_CERT_SECRET_NAME != '' }}
349-
if: env.DO_CODE_SIGN == 'true'
350-
shell: bash
359+
if: env.DO_WIN_CODESIGN == 'true'
360+
shell: pwsh
351361
run: |
352-
PATH=$PATH:"/c/Program Files (x86)/Windows Kits/10/App Certification Kit/" \
353-
signtool verify //pa artifacts/${{matrix.type.fileprefix}}-*.exe
362+
$ret = 0
363+
$files = Get-ChildItem -Path artifacts -Filter "${{matrix.type.fileprefix}}-*.exe"
364+
foreach ($file in $files) {
365+
$signature = Get-AuthenticodeSignature -FilePath $file.FullName
366+
if ($signature.Status -eq 'Valid') {
367+
Write-Host "[ VALID ] $($file.FullName)"
368+
} else {
369+
Write-Host "[INVALID] $($file.FullName)"
370+
Write-Host " Message: $($signature.StatusMessage)"
371+
$ret = 1
372+
}
373+
}
374+
exit $ret
354375
- name: Publish ${{matrix.type.name}}-${{matrix.arch.name}}
355376
uses: actions/upload-artifact@v4
356377
with:
@@ -369,7 +390,7 @@ jobs:
369390
- name: Check out repository
370391
uses: actions/checkout@v4
371392
with:
372-
path: 'git'
393+
path: git
373394

374395
- name: Install Git dependencies
375396
run: |
@@ -386,9 +407,16 @@ jobs:
386407
# Make universal gettext library
387408
lipo -create -output libintl.a /usr/local/opt/gettext/lib/libintl.a /opt/homebrew/opt/gettext/lib/libintl.a
388409
410+
- name: Log in to Azure
411+
uses: azure/login@v2
412+
with:
413+
client-id: ${{ secrets.AZURE_CLIENT_ID }}
414+
tenant-id: ${{ secrets.AZURE_TENANT_ID }}
415+
subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
416+
389417
- name: Download signing secrets
390418
id: signing-secrets
391-
uses: ./.github/actions/akv-secret
419+
uses: ./git/.github/actions/akv-secret
392420
with:
393421
vault: ${{ secrets.AZURE_VAULT }}
394422
secrets: |
@@ -399,8 +427,8 @@ jobs:
399427
${{ secrets.APPLE_DEVELOPER_PASSWORD_SECRET_NAME }} > $output:dev-pass
400428
${{ secrets.APPLE_APPCERT_PASS_SECRET_NAME }} > $output:appcert-pass
401429
${{ secrets.APPLE_INSTCERT_PASS_SECRET_NAME }} > $output:instcert-pass
402-
${{ secrets.APPLE_APPCERT_SECRET_NAME }} base64> appcert.p12
403-
${{ secrets.APPLE_INSTCERT_SECRET_NAME }} base64> instcert.p12
430+
${{ secrets.APPLE_APPCERT_SECRET_NAME }} base64> ${{ runner.temp }}/appcert.p12
431+
${{ secrets.APPLE_INSTCERT_SECRET_NAME }} base64> ${{ runner.temp }}/instcert.p12
404432
405433
- name: Set up signing/notarization infrastructure
406434
run: |
@@ -411,7 +439,7 @@ jobs:
411439
# Prevent re-locking
412440
security set-keychain-settings $RUNNER_TEMP/buildagent.keychain
413441
414-
security import appcert.p12 \
442+
security import $RUNNER_TEMP/appcert.p12 \
415443
-k $RUNNER_TEMP/buildagent.keychain \
416444
-P '${{ steps.signing-secrets.outputs.appcert-pass }}' \
417445
-T /usr/bin/codesign
@@ -420,7 +448,7 @@ jobs:
420448
-s -k pwd \
421449
$RUNNER_TEMP/buildagent.keychain
422450
423-
security import instcert.p12 \
451+
security import $RUNNER_TEMP/instcert.p12 \
424452
-k $RUNNER_TEMP/buildagent.keychain \
425453
-P '${{ steps.signing-secrets.outputs.instcert-pass }}' \
426454
-T /usr/bin/pkgbuild
@@ -496,7 +524,7 @@ jobs:
496524
cp -R stage/git-universal-$VERSION/ \
497525
git/.github/macos-installer/build-artifacts
498526
make -C git/.github/macos-installer V=1 codesign \
499-
APPLE_APP_IDENTITY=${{ steps.signing-secrets.outputs.appsign-id }} || die "Creating signed payload failed"
527+
APPLE_APP_IDENTITY='${{ steps.signing-secrets.outputs.appsign-id }}' || die "Creating signed payload failed"
500528
501529
# Build and sign pkg
502530
make -C git/.github/macos-installer V=1 pkg \
@@ -636,9 +664,14 @@ jobs:
636664
tenant-id: ${{ secrets.AZURE_TENANT_ID }}
637665
subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
638666

667+
- name: Check out repository (for akv-secret Action)
668+
uses: actions/checkout@v4
669+
with:
670+
path: git
671+
639672
- name: Download GPG secrets
640673
id: gpg-secrets
641-
uses: ./.github/actions/akv-secret
674+
uses: ./git/.github/actions/akv-secret
642675
with:
643676
vault: ${{ secrets.AZURE_VAULT }}
644677
secrets: |
@@ -809,8 +842,13 @@ jobs:
809842
tenant-id: ${{ secrets.AZURE_TENANT_ID }}
810843
subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
811844

845+
- name: Check out repository (for akv-secret Action)
846+
uses: actions/checkout@v4
847+
with:
848+
path: git
849+
812850
- name: Download Linux GPG public key signature file
813-
uses: ./.github/actions/akv-secret
851+
uses: ./git/.github/actions/akv-secret
814852
with:
815853
vault: ${{ secrets.AZURE_VAULT }}
816854
secrets: |

0 commit comments

Comments
 (0)