Error on multiline secret

Author: ericsciple

node/Strings/resources.resjson/en-US/resources.resjson

@@ -5,6 +5,7 @@
   "loc.messages.LIB_MkdirFailedFileExists": "Unable to create directory '%s'. Conflicting file exists: '%s'",
   "loc.messages.LIB_MkdirFailedInvalidDriveRoot": "Unable to create directory '%s'. Root directory does not exist: '%s'",
   "loc.messages.LIB_MkdirFailedInvalidShare": "Unable to create directory '%s'. Unable to verify the directory exists: '%s'. If directory is a file share, please verify the share name is correct, the share is online, and the current process has permission to access the share.",
+  "loc.messages.LIB_MultilineSecret": "Secrets cannot contain multiple lines",
   "loc.messages.LIB_ReturnCode": "Return code: %d",
   "loc.messages.LIB_ResourceFileNotExist": "Resource file doesn\\'t exist: %s",
   "loc.messages.LIB_ResourceFileAlreadySet": "Resource file has already set to: %s",

node/lib.json

@@ -6,6 +6,7 @@
     "LIB_MkdirFailedFileExists": "Unable to create directory '%s'. Conflicting file exists: '%s'",
     "LIB_MkdirFailedInvalidDriveRoot": "Unable to create directory '%s'. Root directory does not exist: '%s'",
     "LIB_MkdirFailedInvalidShare": "Unable to create directory '%s'. Unable to verify the directory exists: '%s'. If directory is a file share, please verify the share name is correct, the share is online, and the current process has permission to access the share.",
+    "LIB_MultilineSecret": "Secrets cannot contain multiple lines",
     "LIB_ReturnCode": "Return code: %d",
     "LIB_ResourceFileNotExist": "Resource file doesn\\'t exist: %s",
     "LIB_ResourceFileAlreadySet": "Resource file has already set to: %s",

node/task.ts

@@ -114,7 +114,7 @@ export function getVariables(): VariableInfo[] {
  * 
  * @param     name    name of the variable to set
  * @param     val     value to set
- * @param     secret  whether variable is secret.  optional, defaults to false
+ * @param     secret  whether variable is secret.  Multi-line secrets are not allowed.  Optional, defaults to false
  * @returns   void
  */
 export function setVariable(name: string, val: string, secret: boolean = false): void {
@@ -128,6 +128,10 @@ export function setVariable(name: string, val: string, secret: boolean = false):
     let varValue = val || '';
     debug('set ' + name + '=' + (secret && varValue ? '********' : varValue));
     if (secret) {
+        if (varValue && varValue.match(/\r|\n/) && `${process.env['SYSTEM_UNSAFEALLOWMULTILINESECRET']}`.toUpperCase() != 'TRUE') {
+            throw new Error(loc('LIB_MultilineSecret'));
+        }
+
         im._vault.storeSecret('SECRET_' + key, varValue);
         delete process.env[key];
     } else {
@@ -141,6 +145,20 @@ export function setVariable(name: string, val: string, secret: boolean = false):
     command('task.setvariable', { 'variable': name || '', 'issecret': (secret || false).toString() }, varValue);
 }
 
+/**
+ * Registers a value with the logger, so the value will be masked from the logs.  Multi-line secrets are not allowed.
+ *
+ * @param val value to register
+ */
+export function setSecret(val: string): void {
+    if (val) {
+        if (val.match(/\r|\n/) && `${process.env['SYSTEM_UNSAFEALLOWMULTILINESECRET']}`.toUpperCase() !== 'TRUE') {
+            throw new Error(loc('LIB_MultilineSecret'));
+        }
+        command('task.setsecret', {}, val);
+    }
+}
+
 /** Snapshot of a variable at the time when getVariables was called. */
 export interface VariableInfo {
     name: string;

node/test/inputtests.ts

@@ -222,6 +222,92 @@ describe('Input Tests', function () {
 
         done();
     })
+    it('does not allow setting a multi-line secret variable', function (done) {
+        this.timeout(1000);
+
+        im._loadData();
+
+        // test carriage return
+        let failed = false;
+        try {
+            tl.setVariable('my.secret', 'line1\rline2', true);
+        }
+        catch (err) {
+            failed = true;
+        }
+        assert(failed, 'Should have failed setting a secret variable with a carriage return');
+
+        // test line feed
+        failed = false;
+        try {
+            tl.setVariable('my.secret', 'line1\nline2', true);
+        }
+        catch (err) {
+            failed = true;
+        }
+        assert(failed, 'Should have failed setting a secret variable with a line feed');
+
+        done();
+    })
+    it('allows unsafe setting a multi-line secret variable', function (done) {
+        this.timeout(1000);
+
+        im._loadData();
+        try {
+            process.env['SYSTEM_UNSAFEALLOWMULTILINESECRET'] = 'true';
+            tl.setVariable('my.secret', 'line1\r\nline2', true);
+        }
+        finally {
+            delete process.env['SYSTEM_UNSAFEALLOWMULTILINESECRET'];
+        }
+
+        assert.equal(tl.getVariable('my.secret'), 'line1\r\nline2');
+
+        done();
+    })
+
+    // setSecret tests
+    it('does not allow setting a multi-line secret', function (done) {
+        this.timeout(1000);
+
+        im._loadData();
+
+        // test carriage return
+        let failed = false;
+        try {
+            tl.setSecret('line1\rline2');
+        }
+        catch (err) {
+            failed = true;
+        }
+        assert(failed, 'Should have failed setting a secret with a carriage return');
+
+        // test line feed
+        failed = false;
+        try {
+            tl.setSecret('line1\nline2');
+        }
+        catch (err) {
+            failed = true;
+        }
+        assert(failed, 'Should have failed setting a secret with a line feed');
+
+        done();
+    })
+    it('allows unsafe setting a multi-line secret', function (done) {
+        this.timeout(1000);
+
+        im._loadData();
+        try {
+            process.env['SYSTEM_UNSAFEALLOWMULTILINESECRET'] = 'true';
+            tl.setSecret('line1\r\nline2');
+        }
+        finally {
+            delete process.env['SYSTEM_UNSAFEALLOWMULTILINESECRET'];
+        }
+
+        done();
+    })
 
     // getVariables tests
     it('gets public variables from initial load', function (done) {

node/test/toolrunnertests.ts

@@ -329,7 +329,7 @@ describe('Toolrunner Tests', function () {
         }
     })
     it('Succeeds on stderr by default', function (done) {
-        this.timeout(1000);
+        this.timeout(2000);
 
         var scriptPath = path.join(__dirname, 'scripts', 'stderroutput.js');
         var ls = tl.tool(tl.which('node', true));