Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,16 @@
"mocha": true
},
"rules": {
"node/no-missing-import": ["off"],
"node/no-missing-require": ["off"],
"node/no-missing-import": ["off"],
"node/no-missing-require": ["off"],
"node/no-unpublished-import": ["off"],
"node/no-unpublished-require": ["off"],
"node/no-unsupported-features/es-syntax": ["off"]
"node/no-unsupported-features/es-syntax": ["off"],
"n/no-missing-import": ["off"],
"n/no-missing-require": ["off"],
"n/no-unpublished-import": ["off"],
"n/no-unpublished-require": ["off"],
"n/no-unsupported-features/es-syntax": ["off"]
},
"parserOptions": {
"ecmaVersion": 2020,
Expand Down
17 changes: 11 additions & 6 deletions .github/scripts/setup-vars.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,15 @@ export default function setupVars({projectId, core, setup, serviceAccount, idTok
console.log(` ${key}: ${setup.secrets[key]}`);
}

// Set global secret for the Service Account identity token
// Set global secret for the Service Account identity token if provided
// Use in place of 'gcloud auth print-identity-token' or auth.getIdTokenClient
// usage: curl -H 'Bearer: $ID_TOKEN' https://
core.exportVariable('ID_TOKEN', idToken)
core.setSecret(idToken)
// For logging, show the source of the ID_TOKEN
console.log(` ID_TOKEN: steps.auth.outputs.id_token (from GitHub Action)`);
if (idToken) {
core.exportVariable('ID_TOKEN', idToken);
core.setSecret(idToken);
// For logging, show the source of the ID_TOKEN
console.log(` ID_TOKEN: steps.auth.outputs.id_token (from GitHub Action)`);
}

// Return env and secrets to use for further steps.
return {
Expand All @@ -65,9 +67,12 @@ export default function setupVars({projectId, core, setup, serviceAccount, idTok
}

export function substituteVars(value, env) {
if (value === null || value === undefined || typeof value !== 'string') {
return value;
}
for (const key in env) {
let re = new RegExp(`\\$(${key}\\b|\\{\\s*${key}\\s*\\})`, 'g');
value = value.replaceAll(re, env[key]);
value = value.replaceAll(re, String(env[key]));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If an environment variable in env is undefined or null, calling String(env[key]) will substitute the literal string "undefined" or "null" into the value. This can lead to unexpected and hard-to-debug values (e.g., "http://localhost:undefined"). It is safer to substitute an empty string "" instead.

Suggested change
value = value.replaceAll(re, String(env[key]));
value = value.replaceAll(re, env[key] == null ? '' : String(env[key]));

}
return value;
}
Expand Down
42 changes: 41 additions & 1 deletion .github/scripts/setup-vars.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
limitations under the License.
*/

import {deepStrictEqual} from 'assert';
import {deepStrictEqual, strictEqual} from 'assert';
import setupVars from './setup-vars.js';
import {substituteVars, uniqueId} from './setup-vars.js';

Expand Down Expand Up @@ -57,6 +57,13 @@ describe('setupVars', () => {
deepStrictEqual(vars.env, expected);
});

it('should support non-string variables without throwing', () => {
const setup = {env: {PORT: 8080, ENABLED: true, URL: 'http://localhost:$PORT'}};
const vars = setupVars({projectId, core, setup, serviceAccount}, 'run-id');
const expected = {...autovars, PORT: 8080, ENABLED: true, URL: 'http://localhost:8080'};
deepStrictEqual(vars.env, expected);
});

it('should override automatic variables', () => {
const setup = {env: {PROJECT_ID: 'custom-value', SERVICE_ACCOUNT: 'baz@foo.com'}};
const vars = setupVars({projectId, core, setup, serviceAccount}, 'run-id');
Expand All @@ -82,6 +89,32 @@ describe('setupVars', () => {
});
});

describe('idToken handling', () => {
it('should export and setSecret ID_TOKEN when idToken is provided', () => {
const exported = {};
const secrets = [];
const testCore = {
exportVariable: (k, v) => { exported[k] = v; },
setSecret: (s) => { secrets.push(s); },
};
setupVars({projectId, core: testCore, setup: {}, serviceAccount, idToken: 'test-token-123'}, 'run-id');
strictEqual(exported['ID_TOKEN'], 'test-token-123');
deepStrictEqual(secrets, ['test-token-123']);
});

it('should not export or setSecret ID_TOKEN when idToken is missing', () => {
const exported = {};
const secrets = [];
const testCore = {
exportVariable: (k, v) => { exported[k] = v; },
setSecret: (s) => { secrets.push(s); },
};
setupVars({projectId, core: testCore, setup: {}, serviceAccount}, 'run-id');
strictEqual(exported['ID_TOKEN'], undefined);
deepStrictEqual(secrets, []);
});
});

describe('secrets', () => {
it('zero secrets', () => {
const setup = {secrets: {}};
Expand Down Expand Up @@ -146,6 +179,13 @@ describe('substituteVars', () => {
const expected = '$Ab';
deepStrictEqual(got, expected);
});

it('should safely return non-string inputs', () => {
strictEqual(substituteVars(8080, {A: 'x'}), 8080);
strictEqual(substituteVars(true, {A: 'x'}), true);
strictEqual(substituteVars(null, {A: 'x'}), null);
strictEqual(substituteVars(undefined, {A: 'x'}), undefined);
});
});

describe('uniqueId', () => {
Expand Down
Loading