From 33d41a7e7c05b1ddc419c01e3c696a34548ea971 Mon Sep 17 00:00:00 2001 From: kshp Date: Fri, 4 Aug 2023 10:36:25 -0700 Subject: [PATCH] fix: fail action execution if invalid parameter name used as setOutput() argument --- .../lib/toolkit/sdk/core/command-wrapper.ts | 11 ++++++- .../adk-core/lib/toolkit/sdk/core/core.ts | 3 +- packages/adk-core/package.json | 3 +- packages/adk-core/test/core.test.ts | 30 +++++++++++++++++++ packages/adk-utils/lib/index.ts | 2 ++ packages/adk-utils/lib/util/util.ts | 10 +++++++ packages/adk-utils/test/util.test.ts | 19 ++++++++++++ 7 files changed, 74 insertions(+), 4 deletions(-) diff --git a/packages/adk-core/lib/toolkit/sdk/core/command-wrapper.ts b/packages/adk-core/lib/toolkit/sdk/core/command-wrapper.ts index deefa7415..3349434a2 100644 --- a/packages/adk-core/lib/toolkit/sdk/core/command-wrapper.ts +++ b/packages/adk-core/lib/toolkit/sdk/core/command-wrapper.ts @@ -2,7 +2,7 @@ import { exec } from 'shelljs'; // @ts-ignore import os from 'os'; import { ICommandOutput } from './core'; -import { sanitizeCommand } from '@aws/codecatalyst-adk-utils/lib'; +import { isValidOutputVariableName, outputVariableNamePattern, sanitizeCommand } from '@aws/codecatalyst-adk-utils/lib'; export function runCommand(cmd: string, sanitizeInput: boolean = true, @@ -26,6 +26,15 @@ export function getInputParam(inputVar: string) { } export function setOutputParam(varName: string, varValue: string) { + if (!isValidOutputVariableName(varName)) { + const errorMessage = `Invalid output parameter name, it must match the following pattern ${outputVariableNamePattern}`; + const command_output = {}; + command_output.code = 1; + command_output.stdout = errorMessage; + command_output.stderr = errorMessage; + setFailure(errorMessage, 1); + return command_output; + } return runCommand(`echo "::set-output name=${varName}::${varValue}"`, false).stdout; } diff --git a/packages/adk-core/lib/toolkit/sdk/core/core.ts b/packages/adk-core/lib/toolkit/sdk/core/core.ts index b2903c675..80224f6c0 100644 --- a/packages/adk-core/lib/toolkit/sdk/core/core.ts +++ b/packages/adk-core/lib/toolkit/sdk/core/core.ts @@ -38,9 +38,8 @@ export function getEnvironmentVariable(inputVar: string) { /** * Sets the output value for the action output parameter. * -* @param varName The name of the environment variable. +* @param varName The name of the environment variable. Must match the `^[A-Za-z0-9][A-Za-z0-9\-_]{1,30}[A-Za-z0-9]$` pattern. * @param varValue The fully resolved value of the output variable. -* @param sanitizeInput If true, all the input is sanitized. * * @return The result of running `echo ${varName}`. */ diff --git a/packages/adk-core/package.json b/packages/adk-core/package.json index 2d14b661a..0b0ad9244 100644 --- a/packages/adk-core/package.json +++ b/packages/adk-core/package.json @@ -25,7 +25,8 @@ }, "dependencies": { "@types/shelljs": "^0.8.11", - "shelljs": "^0.8.5" + "shelljs": "^0.8.5", + "@aws/codecatalyst-adk-utils": "^1.0.14" }, "devDependencies": { "jsii": "^5.0.0", diff --git a/packages/adk-core/test/core.test.ts b/packages/adk-core/test/core.test.ts index 06d22b77e..550a1dd1a 100644 --- a/packages/adk-core/test/core.test.ts +++ b/packages/adk-core/test/core.test.ts @@ -1,6 +1,7 @@ 'use strict'; import * as adkCore from '../lib/toolkit/sdk/core/core.js'; +import { outputVariableNamePattern } from '@aws/codecatalyst-adk-utils/lib'; describe('@aws/codecatalyst-adk-core', () => { @@ -73,4 +74,33 @@ describe('@aws/codecatalyst-adk-core', () => { expect(output.code === 0).toBeTruthy(); }); + it('test setOutput validation', () => { + const errorMessage = `Invalid output parameter name, it must match the following pattern ${outputVariableNamePattern}`; + const outputParamValue = 'outputParamValue'; + const validInput30Chars = 'Stack_ID_12345678910111213145'; + + const emptyInput = ''; + const invalidInput = 'Stack ID'; + const tooLongInput = 'longer_than_30_chars_123456789101112131415161718'; + const startsWithInvalidChar = '-Stack_ID'; + const endsWithInvalidChar = 'Stack_ID-'; + + expect(adkCore.setOutput(validInput30Chars, outputParamValue).code === undefined).toBeTruthy(); + + expect(adkCore.setOutput(emptyInput, outputParamValue).code === 1).toBeTruthy(); + expect(adkCore.setOutput(emptyInput, outputParamValue).stdout === errorMessage).toBeTruthy(); + + expect(adkCore.setOutput(invalidInput, outputParamValue).code === 1).toBeTruthy(); + expect(adkCore.setOutput(invalidInput, outputParamValue).stdout === errorMessage).toBeTruthy(); + + expect(adkCore.setOutput(tooLongInput, outputParamValue).code === 1).toBeTruthy(); + expect(adkCore.setOutput(tooLongInput, outputParamValue).stdout === errorMessage).toBeTruthy(); + + expect(adkCore.setOutput(startsWithInvalidChar, outputParamValue).code === 1).toBeTruthy(); + expect(adkCore.setOutput(startsWithInvalidChar, outputParamValue).stdout === errorMessage).toBeTruthy(); + + expect(adkCore.setOutput(endsWithInvalidChar, outputParamValue).code === 1).toBeTruthy(); + expect(adkCore.setOutput(endsWithInvalidChar, outputParamValue).stdout === errorMessage).toBeTruthy(); + }); + }); diff --git a/packages/adk-utils/lib/index.ts b/packages/adk-utils/lib/index.ts index 6e8788de0..f21098674 100644 --- a/packages/adk-utils/lib/index.ts +++ b/packages/adk-utils/lib/index.ts @@ -2,6 +2,8 @@ export { copyToFileSync, escape, isString, + isValidOutputVariableName, + outputVariableNamePattern, sanitizeCommand, writeContentToFileSync, unknownToBooleanOrFalse, diff --git a/packages/adk-utils/lib/util/util.ts b/packages/adk-utils/lib/util/util.ts index 721909bed..337855b78 100644 --- a/packages/adk-utils/lib/util/util.ts +++ b/packages/adk-utils/lib/util/util.ts @@ -1,5 +1,7 @@ import fs from 'fs'; +export const outputVariableNamePattern = new RegExp(/^[A-Za-z0-9][A-Za-z0-9\-_]{1,30}[A-Za-z0-9]$/); + /** * Sanitizes (escapes) special characters in the command and its arguments. * @param cmd The command to be sanitized. @@ -83,3 +85,11 @@ export function writeContentToFileSync(dest: string, content: string, overrideDe fs.writeFileSync(dest, content, 'utf8'); } } + +/** + * Validates if the output variable name is valid by checking against this pattern - https://github.com/aws/actions-dev-kit/blob/main/packages/adk/templates/codecatalyst_model_schema.json#L94 + * @param varName The destination file. + */ +export function isValidOutputVariableName(varName: string): boolean { + return outputVariableNamePattern.test(varName); +} diff --git a/packages/adk-utils/test/util.test.ts b/packages/adk-utils/test/util.test.ts index 2fa2bae07..b4e89fcd7 100644 --- a/packages/adk-utils/test/util.test.ts +++ b/packages/adk-utils/test/util.test.ts @@ -5,6 +5,7 @@ import { escape, sanitizeCommand, isString, + isValidOutputVariableName, copyToFileSync, writeContentToFileSync, unknownToStringOrUndefined, @@ -125,4 +126,22 @@ describe('ADK-Util test', () => { writeContentToFileSync('dummy', 'dummy', true); expect(fs.writeFileSync).toHaveBeenCalledTimes(1); }); + + it('test validateOutputVariableName', async () => { + const validInput = 'Stack_ID'; + const validInput30Chars = 'Stack_ID_12345678910111213145'; + const emptyInput = ''; + const invalidInput = 'Stack ID'; + const tooLongInput = 'longer_than_30_chars_123456789101112131415161718'; + const startsWithInvalidChar = '-Stack_ID'; + const endsWithInvalidChar = 'Stack_ID-'; + + expect(isValidOutputVariableName(validInput)).toBeTruthy(); + expect(isValidOutputVariableName(validInput30Chars)).toBeTruthy(); + expect(isValidOutputVariableName(emptyInput)).toBeFalsy(); + expect(isValidOutputVariableName(invalidInput)).toBeFalsy(); + expect(isValidOutputVariableName(tooLongInput)).toBeFalsy(); + expect(isValidOutputVariableName(startsWithInvalidChar)).toBeFalsy(); + expect(isValidOutputVariableName(endsWithInvalidChar)).toBeFalsy(); + }); });