Skip to content

Commit 99d0945

Browse files
dschomjcheetham
authored andcommitted
fixup! .github/actions/akv-secret: add action to get secrets
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 <[email protected]> Signed-off-by: Matthew John Cheetham <[email protected]>
1 parent 2394117 commit 99d0945

File tree

1 file changed

+93
-35
lines changed

1 file changed

+93
-35
lines changed

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

Lines changed: 93 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ const { spawnSync } = require('child_process');
22
const fs = require('fs');
33
const os = require('os');
44
const path = require('path');
5+
const { isUtf8 } = require("buffer")
56

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

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+
1848
const writeCommand = (file, name, value) => {
1949
// Unique delimiter to avoid conflicts with actual values
2050
let delim;
@@ -29,24 +59,37 @@ const writeCommand = (file, name, value) => {
2959
}
3060

3161
const setSecret = (value) => {
32-
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+
.map(
71+
value => `::add-mask::${escapeData(value)}${os.EOL}`
72+
)
73+
.join('')
74+
);
3375
}
3476

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

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

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

5295
(async () => {
@@ -68,9 +111,7 @@ const setFailed = (error) => {
68111

69112
// Fetch secrets from Azure Key Vault
70113
for (const { input: secretName, encoding, output } of secretMappings) {
71-
let secretValue = '';
72-
73-
const az = spawnSync('az',
114+
let az = spawnSync('az',
74115
[
75116
'keyvault',
76117
'secret',
@@ -93,10 +134,12 @@ const setFailed = (error) => {
93134
if (az.error) throw new Error(az.error, { cause: az.error });
94135
if (az.status !== 0) throw new Error(`az failed with status ${az.status}`);
95136

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

98141
// Mask the raw secret value in logs
99-
setSecret(secretValue);
142+
setSecret(secretBuf);
100143

101144
// Handle encoded values if specified
102145
// Sadly we cannot use the `--encoding` parameter of the `az keyvault
@@ -106,31 +149,46 @@ const setFailed = (error) => {
106149
if (encoding) {
107150
switch (encoding.toLowerCase()) {
108151
case 'base64':
109-
secretValue = Buffer.from(secretValue, 'base64').toString();
152+
secretBuf = Buffer.from(secretBuf.toString('utf-8'), 'base64');
153+
break;
154+
case 'ascii':
155+
case 'utf8':
156+
case 'utf-8':
157+
// No need to decode the existing buffer from the az command
110158
break;
111159
default:
112-
// No decoding needed
113-
}
160+
throw new Error(`Unsupported encoding: ${encoding}`);
161+
}
114162

115-
setSecret(secretValue); // Mask the decoded value as well
163+
// Mask the decoded value
164+
setSecret(secretBuf);
116165
}
117166

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

0 commit comments

Comments
 (0)