fix: fail action execution if invalid parameter name used as setOutpu…

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 = <ICommandOutput>{};
+        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;
 }
 

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}`.
 */

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",

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();
+    });
+
 });

packages/adk-utils/lib/index.ts

@@ -2,6 +2,8 @@ export {
     copyToFileSync,
     escape,
     isString,
+    isValidOutputVariableName,
+    outputVariableNamePattern,
     sanitizeCommand,
     writeContentToFileSync,
     unknownToBooleanOrFalse,

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);
+}

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();
+    });
 });