From c19da4469e15ff100d007165450f21430b01ddac Mon Sep 17 00:00:00 2001
From: David Zuelke <dzuelke@salesforce.com>
Date: Fri, 1 Sep 2023 02:07:21 +0200
Subject: [PATCH 1/2] Use EXIT trap for heroku-command-exit-status
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Appending our own `; echo …` to the command has several drawbacks:

- it does not run if the command contains an `exit` statement (simple example: `heroku run -x 'exit 3'`);
- it does not run if the shell itself exits, e.g. on commands that use `set -e` or `set -u`;
- it breaks when the preceding command uses valid syntax that conflicts with the suffix (simple example: `heroku run -x 'echo hello;'`);
- it causes the wrong exit status to be reported in `heroku logs` - it's always 0 (from the `echo`), not the correct value.

Using a `trap` on `EXIT` instead works around all of these issues.
---
 packages/run-v5/lib/dyno.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/run-v5/lib/dyno.js b/packages/run-v5/lib/dyno.js
index aea630da6a..5855166f5e 100644
--- a/packages/run-v5/lib/dyno.js
+++ b/packages/run-v5/lib/dyno.js
@@ -51,7 +51,7 @@ class Dyno extends Duplex {
   }
 
   _doStart(retries = 2) {
-    let command = this.opts['exit-code'] ? `${this.opts.command}; echo "\uFFFF heroku-command-exit-status: $?"` : this.opts.command
+    let command = this.opts['exit-code'] ? `trap 'echo "\uFFFF heroku-command-exit-status: $?"' EXIT; ${this.opts.command}` : this.opts.command
     return this.heroku.post(this.opts.dyno ? `/apps/${this.opts.app}/dynos/${this.opts.dyno}` : `/apps/${this.opts.app}/dynos`, {
       headers: {
         Accept: this.opts.dyno ? 'application/vnd.heroku+json; version=3.run-inside' : 'application/vnd.heroku+json; version=3',

From 6cfffa82ad7d670f775fe93de6b90a34bc230366 Mon Sep 17 00:00:00 2001
From: David Zuelke <dzuelke@salesforce.com>
Date: Fri, 1 Sep 2023 02:35:05 +0200
Subject: [PATCH 2/2] Fix heroku-command-exit-status newline handling
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The match call uses multiline mode for no reason (it contains no anchors).

The replace call anchors at the start of the string, but this is not a guarantee, and on failure, the magic string is visible in the output (simple example: `heroku run -x 'echo "hi"; echo -n "yo"'`).

Solution is to remove the leading anchor.

It also leaves behind a stray newline after replacing - the reason is that the `\n?` is ungreedy due to the `?`, so it will not actually match the newline character. The result is that using `heroku run -x` produces an extra blank line at the end of the output.

Solution is to not have the newline optional, or to use a different trailing marker - I have re-used `\uFFFF` here, because that also allows removing multiline mode.

Ideally, we'd use a proper unique string - e.g. generate a GUID for the command, and match that exact GUID later. Could also use a different separator - `\uFFFF` is not legal unicode, but 💜 is ;) (that's `\U1F49C` in Bash, with uppercase "`U`", and `\uD83D\uDC9C` in JavaScript, expressed as a UTF-16 surrogate pair).
---
 packages/run-v5/lib/dyno.js | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/packages/run-v5/lib/dyno.js b/packages/run-v5/lib/dyno.js
index 5855166f5e..edf40dae69 100644
--- a/packages/run-v5/lib/dyno.js
+++ b/packages/run-v5/lib/dyno.js
@@ -51,7 +51,7 @@ class Dyno extends Duplex {
   }
 
   _doStart(retries = 2) {
-    let command = this.opts['exit-code'] ? `trap 'echo "\uFFFF heroku-command-exit-status: $?"' EXIT; ${this.opts.command}` : this.opts.command
+    let command = this.opts['exit-code'] ? `trap 'echo -n "\uFFFF heroku-command-exit-status: $? \uFFFF"' EXIT; ${this.opts.command}` : this.opts.command
     return this.heroku.post(this.opts.dyno ? `/apps/${this.opts.app}/dynos/${this.opts.dyno}` : `/apps/${this.opts.app}/dynos`, {
       headers: {
         Accept: this.opts.dyno ? 'application/vnd.heroku+json; version=3.run-inside' : 'application/vnd.heroku+json; version=3',
@@ -312,10 +312,10 @@ class Dyno extends Duplex {
       // eslint-disable-next-line
       if (!process.stdout.isTTY) data = data.replace(new RegExp('\r\n', 'g'), '\n')
 
-      let exitCode = data.match(/\uFFFF heroku-command-exit-status: (\d+)/m)
+      let exitCode = data.match(/\uFFFF heroku-command-exit-status: (\d+) \uFFFF/)
       if (exitCode) {
         debug('got exit code: %d', exitCode[1])
-        this.push(data.replace(/^\uFFFF heroku-command-exit-status: \d+$\n?/m, ''))
+        this.push(data.replace(/\uFFFF heroku-command-exit-status: \d+ \uFFFF/, ''))
         let code = Number.parseInt(exitCode[1])
         if (code === 0) this.resolve()
         else {