Repository: akuity/kargo

Issue:
yaml-update step isn't correctly quoting number like fields
# Checklist

* [x] I've searched the issue queue to verify this is not a duplicate bug report.
* [x] I've included steps to reproduce the bug.
* [x] I've pasted the output of `kargo version`.
* [x] I've pasted logs, if applicable.

# Description
When passing through a number that is quoted it doesn't end up quoted in the yaml output

# Steps to Reproduce
Create a pipeline with a step similar to:
```yaml
steps:
    - uses: yaml-update
      as: update-image
      vars:
      - name: testValue
        value: main-123456
      config:
        path: values.yaml
        updates:
        - key: version
          value: ${{ vars.testValue | split("-") | last() | quote() }}
    - uses: helm-template
      config:
        path: ./src/helm
        releaseName: test
        outPath: ./out/manifests.yaml
        valuesFiles:
        - values.yaml
```
If you have a helm chart using the version key it will be expressed in scientific notation indicating the value is unquoted when it is put in the yaml file, even though it is quoted in the value.

# Version
Kargo version 1.4.2

# Logs

```
Paste any relevant application logs here.
```


Discussion:

Solution patch:
diff --git a/docs/docs/50-user-guide/60-reference-docs/30-promotion-steps/git-open-pr.md b/docs/docs/50-user-guide/60-reference-docs/30-promotion-steps/git-open-pr.md
index 80dafe37..a3d9cd11 100644
--- a/docs/docs/50-user-guide/60-reference-docs/30-promotion-steps/git-open-pr.md
+++ b/docs/docs/50-user-guide/60-reference-docs/30-promotion-steps/git-open-pr.md
@@ -24,6 +24,7 @@ GitLab pull/merge requests.
 | `targetBranch` | `string` | N | The branch to which the changes should be merged. |
 | `createTargetBranch` | `boolean` | N | Indicates whether a new, empty orphaned branch should be created and pushed to the remote if the target branch does not already exist there. Default is `false`. |
 | `title` | `string` | N | The title for the pull request. Kargo generates a title based on the commit messages if it is not explicitly specified. |
+| `description` | `string` | N | The description for the pull request. Kargo generates a title based on the commit messages if it is not explicitly specified. |
 | `labels` | `[]string` | N | Labels to add to the pull request. |
 
 ## Output
diff --git a/docs/docs/50-user-guide/60-reference-docs/30-promotion-steps/http.md b/docs/docs/50-user-guide/60-reference-docs/30-promotion-steps/http.md
index dcd613c6..ec490f4a 100644
--- a/docs/docs/50-user-guide/60-reference-docs/30-promotion-steps/http.md
+++ b/docs/docs/50-user-guide/60-reference-docs/30-promotion-steps/http.md
@@ -20,7 +20,7 @@ with a wide variety of external services.
 | `queryParams` | `[]object` | N | A list of query parameters to include in the request. |
 | `queryParams[].name` | `string` | Y | The name of the query parameter. |
 | `queryParams[].value` | `string` | Y | The value of the query parameter. The provided value will automatically be URL-encoded if necessary. |
-| `body` | `string` | N | The body of the request. __Note:__ As this field is a `string`, take care to utilize [`quote()`](../40-expressions.md#quote) if the body is a valid JSON `object`. Refer to the example below of posting a message to a Slack channel. |
+| `body` | `string` | N | The body of the request. __Note:__ As this field is a `string`, take care to utilize [`quote()`](../40-expressions.md#quotevalue) if the body is a valid JSON `object`. Refer to the example below of posting a message to a Slack channel. |
 | `insecureSkipTLSVerify` | `boolean` | N | Indicates whether to bypass TLS certificate verification when making the request. Setting this to `true` is highly discouraged. |
 | `timeout` | `string` | N | A string representation of the maximum time interval to wait for a request to complete. _This is the timeout for an individual HTTP request. If a request is retried, each attempt is independently subject to this timeout._ See Go's [`time` package docs](https://pkg.go.dev/time#ParseDuration) for a description of the accepted format. |
 | `successExpression` | `string` | N | An [expr-lang] expression that can evaluate the response to determine success. If this is left undefined and `failureExpression` _is_ defined, the default success criteria will be the inverse of the specified failure criteria. If both are left undefined, success is `true` when the HTTP status code is `2xx`. If `successExpression` and `failureExpression` are both defined and both evaluate to `true`, the failure takes precedence. Note that this expression should _not_ be offset by `${{` and `}}`. See examples for more details. |
diff --git a/docs/docs/50-user-guide/60-reference-docs/40-expressions.md b/docs/docs/50-user-guide/60-reference-docs/40-expressions.md
index 5877e9bb..537615f0 100644
--- a/docs/docs/50-user-guide/60-reference-docs/40-expressions.md
+++ b/docs/docs/50-user-guide/60-reference-docs/40-expressions.md
@@ -258,14 +258,36 @@ one required argument:
 - `value` (Required): A value of any type to be converted to a string.
 
 This is useful for scenarios where an expression evaluates to a non-`string`
-JSON type, but you wish to treat it as a `string` regardless.
+JSON type, but you wish to treat it as a `string` regardless. For string inputs,
+it produces clean output without visible quotation marks.
 
 Example:
 
 ```yaml
 config:
-  numField: ${{ 40 + 2 }} # Will be treated as a number
-  strField: ${{ quote(40 + 2) }} # Will be treated as a string
+  numField: ${{ 40 + 2 }} # Will be treated as a number (42)
+  strField: ${{ quote(40 + 2) }} # Will be treated as a string ("42")
+  rawField: ${{ quote("string") }} # Will result in "string"
+```
+
+### `unsafeQuote(value)`
+
+The `unsafeQuote()` function converts any value to its string representation. It
+has one required argument:
+
+- `value` (Required): A value of any type to be converted to a string.
+
+Compared to [`quote()`](#quotevalue) this function is considered "unsafe"
+because it adds escaped quotes around input values that are already considered
+strings. For non-string values, it behaves similarly to `quote()`.
+
+Example:
+
+```yaml
+config:
+  numField: ${{ 40 + 2 }} # Will be treated as a number (42)
+  strField: ${{ unsafeQuote(40 + 2) }} # Will result in "42"
+  rawField: ${{ unsafeQuote("string") }} # Will result in "\"string\""
 ```
 
 ### `configMap(name)`
diff --git a/internal/expressions/json_templates.go b/internal/expressions/json_templates.go
index 8e639be4..823b9be3 100644
--- a/internal/expressions/json_templates.go
+++ b/internal/expressions/json_templates.go
@@ -108,22 +108,21 @@ func EvaluateTemplate(template string, env map[string]any, exprOpts ...expr.Opti
 		return template, nil
 	}
 	if exprOpts == nil {
-		exprOpts = make([]expr.Option, 0, 1)
+		exprOpts = make([]expr.Option, 0, 2)
 	}
-	exprOpts = append(exprOpts, expr.Function(
-		"quote",
-		func(a ...any) (any, error) {
-			if str, ok := a[0].(string); ok {
-				return fmt.Sprintf("%q", str), nil
-			}
-			jsonBytes, err := json.Marshal(a[0])
-			if err != nil {
-				return nil, fmt.Errorf("error applying quote() function: %w", err)
-			}
-			return fmt.Sprintf(`"%s"`, jsonBytes), nil
-		},
-		new(func(any) string),
-	))
+	exprOpts = append(
+		exprOpts,
+		expr.Function(
+			"quote",
+			quoteFunc,
+			new(func(any) string),
+		),
+		expr.Function(
+			"unsafeQuote",
+			unsafeQuoteFunc,
+			new(func(any) string),
+		),
+	)
 	t, err := fasttemplate.NewTemplate(template, "${{", "}}")
 	if err != nil {
 		return nil, fmt.Errorf("error parsing template: %w", err)
@@ -209,3 +208,24 @@ func getExpressionEvaluator(env map[string]any, exprOpts ...expr.Option) fasttem
 		return out.Write(resJSON)
 	}
 }
+
+// quoteFunc formats a value as a quoted string, with special handling for
+// string inputs. For string inputs, it produces a clean string without visible
+// quotation marks. For non-string inputs, it delegates to unsafeQuoteFunc.
+func quoteFunc(a ...any) (any, error) {
+	if str, ok := a[0].(string); ok {
+		return fmt.Sprintf("%q", str), nil
+	}
+	return unsafeQuoteFunc(a...)
+}
+
+// unsafeQuoteFunc converts any value to JSON and preserves the escape sequences.
+// Unlike quoteFunc, this function retains the literal escape sequences in the
+// output, producing a string with visible escaped quotation marks.
+func unsafeQuoteFunc(a ...any) (any, error) {
+	jsonBytes, err := json.Marshal(a[0])
+	if err != nil {
+		return nil, fmt.Errorf("error applying quote() function: %w", err)
+	}
+	return fmt.Sprintf(`"%s"`, jsonBytes), nil
+}
diff --git a/internal/promotion/runner/builtin/git_pr_opener.go b/internal/promotion/runner/builtin/git_pr_opener.go
index 00cd698b..ccd645c4 100644
--- a/internal/promotion/runner/builtin/git_pr_opener.go
+++ b/internal/promotion/runner/builtin/git_pr_opener.go
@@ -10,11 +10,12 @@ import (
 	"github.com/xeipuuv/gojsonschema"
 
 	kargoapi "github.com/akuity/kargo/api/v1alpha1"
+	"github.com/akuity/kargo/pkg/promotion"
+	"github.com/akuity/kargo/pkg/x/promotion/runner/builtin"
+
 	"github.com/akuity/kargo/internal/controller/git"
 	"github.com/akuity/kargo/internal/credentials"
 	"github.com/akuity/kargo/internal/gitprovider"
-	"github.com/akuity/kargo/pkg/promotion"
-	"github.com/akuity/kargo/pkg/x/promotion/runner/builtin"
 
 	_ "github.com/akuity/kargo/internal/gitprovider/azure"     // Azure provider registration
 	_ "github.com/akuity/kargo/internal/gitprovider/bitbucket" // Bitbucket provider registration
@@ -186,13 +187,21 @@ func (g *gitPROpener) run(
 	title := cfg.Title
 	description := commitMsg
 
+	if cfg.Description != "" {
+		description = cfg.Description
+	}
+
 	if title == "" {
 		parts := strings.SplitN(commitMsg, "\n", 2)
 		title = parts[0]
-		description = ""
 
-		if len(parts) > 1 {
-			description = parts[1]
+		// Only override the description if it has not been set in the config.
+		if cfg.Description == "" {
+			if len(parts) > 1 {
+				description = parts[1]
+			} else {
+				description = "" // The commit message is just a title.
+			}
 		}
 	}
 
diff --git a/internal/promotion/runner/builtin/helm_chart_updater.go b/internal/promotion/runner/builtin/helm_chart_updater.go
index 7aad0d99..87935ceb 100644
--- a/internal/promotion/runner/builtin/helm_chart_updater.go
+++ b/internal/promotion/runner/builtin/helm_chart_updater.go
@@ -102,7 +102,7 @@ func (h *helmChartUpdater) run(
 		return promotion.StepResult{Status: kargoapi.PromotionStepStatusErrored}, err
 	}
 
-	if err = intyaml.SetStringsInFile(chartFilePath, changes); err != nil {
+	if err = intyaml.SetValuesInFile(chartFilePath, changes); err != nil {
 		return promotion.StepResult{
 			Status: kargoapi.PromotionStepStatusErrored,
 		}, fmt.Errorf("failed to update chart dependencies in %q: %w", chartFilePath, err)
diff --git a/internal/promotion/runner/builtin/schemas/git-open-pr-config.json b/internal/promotion/runner/builtin/schemas/git-open-pr-config.json
index 02087e23..93833c20 100644
--- a/internal/promotion/runner/builtin/schemas/git-open-pr-config.json
+++ b/internal/promotion/runner/builtin/schemas/git-open-pr-config.json
@@ -39,6 +39,11 @@
       "description": "The title for the pull request. Kargo generates a title based on the commit messages if it is not explicitly specified.",
       "minLength": 1
     },
+    "description": {
+      "type": "string",
+      "description": "The description of the pull request. Kargo generates a description based on the commit messages if it is not explicitly specified.",
+      "minLength": 1
+    },
     "labels": {
       "type": "array",
       "description": "Labels to add to the pull request.",
diff --git a/internal/promotion/runner/builtin/schemas/yaml-update-config.json b/internal/promotion/runner/builtin/schemas/yaml-update-config.json
index a61fdeb0..ae0c250e 100644
--- a/internal/promotion/runner/builtin/schemas/yaml-update-config.json
+++ b/internal/promotion/runner/builtin/schemas/yaml-update-config.json
@@ -14,7 +14,6 @@
           "minLength": 1
         },
         "value": {
-          "type": "string",
           "description": "The new value for the specified key."
         }
       },
diff --git a/internal/promotion/runner/builtin/yaml_updater.go b/internal/promotion/runner/builtin/yaml_updater.go
index 9950a095..42e5a55d 100644
--- a/internal/promotion/runner/builtin/yaml_updater.go
+++ b/internal/promotion/runner/builtin/yaml_updater.go
@@ -9,6 +9,7 @@ import (
 	"github.com/xeipuuv/gojsonschema"
 
 	kargoapi "github.com/akuity/kargo/api/v1alpha1"
+	"github.com/akuity/kargo/internal/yaml"
 	intyaml "github.com/akuity/kargo/internal/yaml"
 	"github.com/akuity/kargo/pkg/promotion"
 	"github.com/akuity/kargo/pkg/x/promotion/runner/builtin"
@@ -92,7 +93,7 @@ func (y *yamlUpdater) updateFile(workDir string, path string, updates []intyaml.
 	if err != nil {
 		return fmt.Errorf("error joining path %q: %w", path, err)
 	}
-	if err := intyaml.SetStringsInFile(absValuesFile, updates); err != nil {
+	if err := intyaml.SetValuesInFile(absValuesFile, updates); err != nil {
 		return fmt.Errorf("error updating image references in values file %q: %w", path, err)
 	}
 	return nil
@@ -106,7 +107,13 @@ func (y *yamlUpdater) generateCommitMessage(path string, updates []builtin.YAMLU
 	var commitMsg strings.Builder
 	_, _ = commitMsg.WriteString(fmt.Sprintf("Updated %s\n", path))
 	for _, update := range updates {
-		_, _ = commitMsg.WriteString(fmt.Sprintf("\n- %s: %q", update.Key, update.Value))
+		_, _ = commitMsg.WriteString(
+			fmt.Sprintf(
+				"\n- %s: %v",
+				update.Key,
+				yaml.QuoteIfNecessary(update.Value),
+			),
+		)
 	}
 
 	return commitMsg.String()
diff --git a/internal/yaml/quote.go b/internal/yaml/quote.go
new file mode 100644
index 00000000..b1bccdc2
--- /dev/null
+++ b/internal/yaml/quote.go
@@ -0,0 +1,37 @@
+package yaml
+
+import (
+	"encoding/json"
+	"fmt"
+	"strconv"
+)
+
+// QuoteIfNecessary takes a value and returns it as-is if it is not a string. If
+// it is a string, it ascertains whether a YAML parser would interpret it as
+// another type. If so, it returns the string with additional quotes around it.
+// If not, it returns the string as-is.
+func QuoteIfNecessary(val any) any {
+	valStr, ok := val.(string)
+	if !ok {
+		return val
+	}
+	// If valStr is parseable as a float64, return that. float64 is used because
+	// it can represent all JSON numbers.
+	//
+	// NB: This is attempted prior to attempting to parse valStr as a boolean so
+	// that "0" and "1" will be interpreted as numbers.
+	if _, err := strconv.ParseFloat(valStr, 64); err == nil {
+		return fmt.Sprintf("%q", valStr)
+	}
+	// If valStr is parseable as a bool return that.
+	if _, err := strconv.ParseBool(valStr); err == nil {
+		return fmt.Sprintf("%q", valStr)
+	}
+	// If valStr is valid JSON, return its unmarshaled value. This could be an
+	// object, array, or null.
+	if json.Valid([]byte(valStr)) {
+		return fmt.Sprintf("%q", valStr)
+	}
+	// If we get to here, just return the string.
+	return val
+}
diff --git a/internal/yaml/yaml.go b/internal/yaml/yaml.go
index 488cf484..175b095b 100644
--- a/internal/yaml/yaml.go
+++ b/internal/yaml/yaml.go
@@ -16,16 +16,16 @@ type Update struct {
 	// Key is the dot-separated path to the field to update.
 	Key string
 	// Value is the new value to set for the field.
-	Value string
+	Value any
 }
 
-// SetStringsInFile overwrites the specified file with the changes specified by
+// SetValuesInFile overwrites the specified file with the changes specified by
 // the the list of Updates. Keys are of the form <key 0>.<key 1>...<key n>.
 // Integers may be used as keys in cases where a specific node needs to be
 // selected from a sequence. An error is returned for any attempted update to a
 // key that does not exist or does not address a scalar node. Importantly, all
 // comments and style choices in the input bytes are preserved in the output.
-func SetStringsInFile(file string, updates []Update) error {
+func SetValuesInFile(file string, updates []Update) error {
 	inBytes, err := os.ReadFile(file)
 	if err != nil {
 		return fmt.Errorf(
@@ -34,7 +34,7 @@ func SetStringsInFile(file string, updates []Update) error {
 			err,
 		)
 	}
-	outBytes, err := SetStringsInBytes(inBytes, updates)
+	outBytes, err := SetValuesInBytes(inBytes, updates)
 	if err != nil {
 		return fmt.Errorf("error mutating bytes: %w", err)
 	}
@@ -53,13 +53,13 @@ func SetStringsInFile(file string, updates []Update) error {
 	return nil
 }
 
-// SetStringsInBytes returns a copy of the provided bytes with the changes
+// SetValuesInBytes returns a copy of the provided bytes with the changes
 // specified by Updates applied. Keys are of the form <key 0>.<key 1>...<key n>.
 // Integers may be used as keys in cases where a specific node needs to be
 // selected from a sequence. An error is returned for any attempted update to a
 // key that does not exist or does not address a scalar node. Importantly, all
 // comments and style choices in the input bytes are preserved in the output.
-func SetStringsInBytes(inBytes []byte, updates []Update) ([]byte, error) {
+func SetValuesInBytes(inBytes []byte, updates []Update) ([]byte, error) {
 	doc := &yaml.Node{}
 	if err := yaml.Unmarshal(inBytes, doc); err != nil {
 		return nil, fmt.Errorf("error unmarshaling input: %w", err)
@@ -67,7 +67,7 @@ func SetStringsInBytes(inBytes []byte, updates []Update) ([]byte, error) {
 
 	type change struct {
 		col   int
-		value string
+		value any
 	}
 	changesByLine := map[int]change{}
 	for _, update := range updates {
@@ -107,7 +107,9 @@ func SetStringsInBytes(inBytes []byte, updates []Update) ([]byte, error) {
 					return nil, fmt.Errorf("%s: %w", errMsg, err)
 				}
 			}
-			if _, err := outBuf.WriteString(change.value); err != nil {
+			if _, err := outBuf.WriteString(
+				fmt.Sprintf("%v", QuoteIfNecessary(change.value)),
+			); err != nil {
 				return nil, fmt.Errorf("%s: %w", errMsg, err)
 			}
 			if _, err := outBuf.WriteString("\n"); err != nil {
diff --git a/pkg/x/promotion/runner/builtin/zz_config_types.go b/pkg/x/promotion/runner/builtin/zz_config_types.go
index 3df28bd6..9331046b 100644
--- a/pkg/x/promotion/runner/builtin/zz_config_types.go
+++ b/pkg/x/promotion/runner/builtin/zz_config_types.go
@@ -174,6 +174,9 @@ type GitOpenPRConfig struct {
 	// Indicates whether a new, empty orphan branch should be created and pushed to the remote
 	// if the target branch does not already exist there. Default is false.
 	CreateTargetBranch bool `json:"createTargetBranch,omitempty"`
+	// The description of the pull request. Kargo generates a description based on the commit
+	// messages if it is not explicitly specified.
+	Description string `json:"description,omitempty"`
 	// Indicates whether to skip TLS verification when cloning the repository. Default is false.
 	InsecureSkipTLSVerify bool `json:"insecureSkipTLSVerify,omitempty"`
 	// Labels to add to the pull request.
@@ -432,7 +435,7 @@ type YAMLUpdate struct {
 	// The key whose value needs to be updated. For nested values, use a YAML dot notation path.
 	Key string `json:"key"`
 	// The new value for the specified key.
-	Value string `json:"value"`
+	Value interface{} `json:"value"`
 }
 
 // The name of the Git provider to use. Currently 'azure', 'bitbucket', 'gitea', 'github',
diff --git a/ui/src/gen/directives/git-open-pr-config.json b/ui/src/gen/directives/git-open-pr-config.json
index 772272c7..2c0f3c37 100644
--- a/ui/src/gen/directives/git-open-pr-config.json
+++ b/ui/src/gen/directives/git-open-pr-config.json
@@ -44,6 +44,11 @@
    "description": "The title for the pull request. Kargo generates a title based on the commit messages if it is not explicitly specified.",
    "minLength": 1
   },
+  "description": {
+   "type": "string",
+   "description": "The description of the pull request. Kargo generates a description based on the commit messages if it is not explicitly specified.",
+   "minLength": 1
+  },
   "labels": {
    "type": "array",
    "description": "Labels to add to the pull request.",
diff --git a/ui/src/gen/directives/yaml-update-config.json b/ui/src/gen/directives/yaml-update-config.json
index 18ef87e0..cef63d2b 100644
--- a/ui/src/gen/directives/yaml-update-config.json
+++ b/ui/src/gen/directives/yaml-update-config.json
@@ -12,7 +12,6 @@
      "minLength": 1
     },
     "value": {
-     "type": "string",
      "description": "The new value for the specified key."
     }
    }
@@ -39,7 +38,6 @@
       "minLength": 1
      },
      "value": {
-      "type": "string",
       "description": "The new value for the specified key."
      }
     }
diff --git a/ui/src/plugins/pr-plugin/deep-link/promotion-step.tsx b/ui/src/plugins/pr-plugin/deep-link/promotion-step.tsx
index 500581d1..aef4f0f7 100644
--- a/ui/src/plugins/pr-plugin/deep-link/promotion-step.tsx
+++ b/ui/src/plugins/pr-plugin/deep-link/promotion-step.tsx
@@ -38,7 +38,7 @@ const plugin: DeepLinkPluginsInstallation['PromotionStep'] = {
       );
     }
 
-    const url = getPullRequestLink(props.step, props.output || {});
+    const url = getPullRequestLink(props.output || {});
 
     nodes.push(
       <a href={url} target='_blank'>
diff --git a/ui/src/plugins/pr-plugin/deep-link/promotion.tsx b/ui/src/plugins/pr-plugin/deep-link/promotion.tsx
index 9ccb30fd..81ba9c39 100644
--- a/ui/src/plugins/pr-plugin/deep-link/promotion.tsx
+++ b/ui/src/plugins/pr-plugin/deep-link/promotion.tsx
@@ -22,7 +22,7 @@ const plugin: DeepLinkPluginsInstallation['Promotion'] = {
         const alias = getPromotionStepAlias(step, idx);
 
         try {
-          const deepLink = getPullRequestLink(step, promotionState[alias]);
+          const deepLink = getPullRequestLink(promotionState[alias]);
 
           deepLinks.push([alias, deepLink]);
         } catch {
diff --git a/ui/src/plugins/pr-plugin/get-pr-link.ts b/ui/src/plugins/pr-plugin/get-pr-link.ts
index 1a911760..883f2c4f 100644
--- a/ui/src/plugins/pr-plugin/get-pr-link.ts
+++ b/ui/src/plugins/pr-plugin/get-pr-link.ts
@@ -1,40 +1,2 @@
-import gitUrlParse from 'git-url-parse';
-
-import { PromotionStep } from '@ui/gen/api/v1alpha1/generated_pb';
-import { getPromotionStepConfig } from '@ui/plugins/atoms/plugin-helper';
-
-export const getPullRequestLink = (
-  promotionStep: PromotionStep,
-  promotionStepOutput: Record<string, unknown>
-) => {
-  const stepConfig = getPromotionStepConfig(promotionStep);
-
-  // plugins responsibility to keep up to date with actual promotion step config
-  const provider =
-    (stepConfig?.provider as 'github' | 'gitlab') || 'github'; /* default provider is github */
-
-  const repoURL = stepConfig?.repoURL as string;
-
-  const prNumber = promotionStepOutput?.prNumber;
-
-  if (typeof prNumber !== 'number' || !repoURL) {
-    // TODO: Throw plugin identified error to handle it at plugin level
-    throw new Error(
-      `cannot generate pull request link, either promotion step didn't return proper output or promotion step config is corrupted`
-    );
-  }
-
-  const parsedUrl = gitUrlParse(repoURL);
-
-  const url = `${parsedUrl.protocol}://${parsedUrl.resource}/${parsedUrl.full_name}`;
-
-  if (provider === 'github') {
-    return `${url}/pull/${prNumber}`;
-  }
-
-  if (provider === 'gitlab') {
-    return `${url}/merge_requests/${prNumber}`;
-  }
-
-  throw new Error(`provider ${provider} not supported by this plugin`);
-};
+export const getPullRequestLink = (promotionStepOutput: Record<string, unknown>) =>
+  (promotionStepOutput as { pr: { url: string } }).pr.url;



Test patch:
diff --git a/internal/expressions/json_templates_test.go b/internal/expressions/json_templates_test.go
index ae3d9f21..5fb5b9f5 100644
--- a/internal/expressions/json_templates_test.go
+++ b/internal/expressions/json_templates_test.go
@@ -230,6 +230,16 @@ func TestEvaluateJSONTemplate(t *testing.T) {
 				require.Equal(t, "foo", parsed.AString)
 			},
 		},
+		{
+			name:         "unsafeQuote function does double quote string input",
+			jsonTemplate: `{ "AString": "${{ unsafeQuote('foo') }}" }`,
+			assertions: func(t *testing.T, jsonOutput []byte, err error) {
+				require.NoError(t, err)
+				parsed := testStruct{}
+				require.NoError(t, json.Unmarshal(jsonOutput, &parsed))
+				require.Equal(t, "\"foo\"", parsed.AString)
+			},
+		},
 		{
 			name: "a variety of tricky cases dealing with YAML and quotes",
 			yamlTemplate: `
diff --git a/internal/promotion/runner/builtin/git_pr_opener_test.go b/internal/promotion/runner/builtin/git_pr_opener_test.go
index 0dacd682..d9eaf201 100644
--- a/internal/promotion/runner/builtin/git_pr_opener_test.go
+++ b/internal/promotion/runner/builtin/git_pr_opener_test.go
@@ -102,6 +102,42 @@ func Test_gitPROpener_validate(t *testing.T) {
 				"title":        "custom title",
 			},
 		},
+		{
+			name: "invalid with empty title",
+			config: promotion.Config{
+				"provider":     "github",
+				"repoURL":      "https://github.com/example/repo.git",
+				"sourceBranch": "fake-branch",
+				"targetBranch": "another-fake-branch",
+				"title":        "",
+			},
+			expectedProblems: []string{
+				"title: String length must be greater than or equal to 1",
+			},
+		},
+		{
+			name: "valid with custom description",
+			config: promotion.Config{
+				"provider":     "github",
+				"repoURL":      "https://github.com/example/repo.git",
+				"sourceBranch": "fake-branch",
+				"targetBranch": "another-fake-branch",
+				"description":  "custom description",
+			},
+		},
+		{
+			name: "invalid with empty description",
+			config: promotion.Config{
+				"provider":     "github",
+				"repoURL":      "https://github.com/example/repo.git",
+				"sourceBranch": "fake-branch",
+				"targetBranch": "another-fake-branch",
+				"description":  "",
+			},
+			expectedProblems: []string{
+				"description: String length must be greater than or equal to 1",
+			},
+		},
 	}
 
 	r := newGitPROpener(nil)
@@ -207,6 +243,7 @@ func Test_gitPROpener_run(t *testing.T) {
 			CreateTargetBranch: true,
 			Provider:           ptr.To(builtin.Provider(fakeGitProviderName)),
 			Title:              "kargo",
+			Description:        "kargo description",
 		},
 	)
 	require.NoError(t, err)
diff --git a/internal/promotion/runner/builtin/yaml_updater_test.go b/internal/promotion/runner/builtin/yaml_updater_test.go
index 8d88d4b7..4b82c01d 100644
--- a/internal/promotion/runner/builtin/yaml_updater_test.go
+++ b/internal/promotion/runner/builtin/yaml_updater_test.go
@@ -95,6 +95,10 @@ func Test_yamlUpdater_validate(t *testing.T) {
 						"key":   "another-fake-key",
 						"value": "another-fake-value",
 					},
+					{
+						"key":   "yet-another-fake-key",
+						"value": 5,
+					},
 				},
 			},
 		},
@@ -135,22 +139,35 @@ func Test_yamlUpdater_run(t *testing.T) {
 				Path: "values.yaml",
 				Updates: []builtin.YAMLUpdate{
 					{Key: "image.tag", Value: "fake-tag"},
+					{Key: "image.numStr", Value: "42"},
+					{Key: "image.realNum", Value: 42},
 				},
 			},
 			files: map[string]string{
-				"values.yaml": "image:\n  tag: oldtag\n",
+				"values.yaml": `
+image:
+  tag: oldtag
+  numStr: "5"
+  realNum: 5
+`,
 			},
 			assertions: func(t *testing.T, workDir string, result promotion.StepResult, err error) {
 				assert.NoError(t, err)
 				assert.Equal(t, promotion.StepResult{
 					Status: kargoapi.PromotionStepStatusSucceeded,
 					Output: map[string]any{
-						"commitMessage": "Updated values.yaml\n\n- image.tag: \"fake-tag\"",
+						"commitMessage": `Updated values.yaml
+
+- image.tag: fake-tag
+- image.numStr: "42"
+- image.realNum: 42`,
 					},
 				}, result)
 				content, err := os.ReadFile(path.Join(workDir, "values.yaml"))
 				require.NoError(t, err)
-				assert.Contains(t, string(content), "tag: fake-tag")
+				assert.Contains(t, string(content), "  tag: fake-tag")
+				assert.Contains(t, string(content), `  numStr: "42"`)
+				assert.Contains(t, string(content), "  realNum: 42")
 			},
 		},
 		{
@@ -272,7 +289,7 @@ func Test_yamlUpdater_generateCommitMessage(t *testing.T) {
 			assertions: func(t *testing.T, result string) {
 				assert.Equal(t, `Updated values.yaml
 
-- image: "repo/image:tag1"`, result)
+- image: repo/image:tag1`, result)
 			},
 		},
 		{
@@ -281,12 +298,20 @@ func Test_yamlUpdater_generateCommitMessage(t *testing.T) {
 			updates: []builtin.YAMLUpdate{
 				{Key: "image1", Value: "repo1/image1:tag1"},
 				{Key: "image2", Value: "repo2/image2:tag2"},
+				{Key: "numStr", Value: "42"},
+				{Key: "realNum", Value: 42},
 			},
 			assertions: func(t *testing.T, result string) {
-				assert.Equal(t, `Updated chart/values.yaml
+				assert.Equal(
+					t,
+					`Updated chart/values.yaml
 
-- image1: "repo1/image1:tag1"
-- image2: "repo2/image2:tag2"`, result)
+- image1: repo1/image1:tag1
+- image2: repo2/image2:tag2
+- numStr: "42"
+- realNum: 42`,
+					result,
+				)
 			},
 		},
 	}
diff --git a/internal/yaml/quote_test.go b/internal/yaml/quote_test.go
new file mode 100644
index 00000000..7b820b4d
--- /dev/null
+++ b/internal/yaml/quote_test.go
@@ -0,0 +1,33 @@
+package yaml
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestQuoteIfNecessary(t *testing.T) {
+	tests := []struct {
+		input    any
+		expected any
+	}{
+		{123, 123},
+		{123.0, 123.0},
+		{true, true},
+		{false, false},
+		{nil, nil},
+		{"123", `"123"`},
+		{"true", `"true"`},
+		{"false", `"false"`},
+		{"null", `"null"`},
+		{"[1, 2, 3]", `"[1, 2, 3]"`},
+		{`{"key": "value"}`, `"{\"key\": \"value\"}"`},
+		{"string", "string"},
+	}
+	for i, test := range tests {
+		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+			require.Equal(t, test.expected, QuoteIfNecessary(test.input))
+		})
+	}
+}
diff --git a/internal/yaml/yaml_test.go b/internal/yaml/yaml_test.go
index 1783f5fd..5cf2a7bd 100644
--- a/internal/yaml/yaml_test.go
+++ b/internal/yaml/yaml_test.go
@@ -8,7 +8,7 @@ import (
 	yaml "sigs.k8s.io/yaml/goyaml.v3"
 )
 
-func TestSetStringsInBytes(t *testing.T) {
+func TestSetValuesInBytes(t *testing.T) {
 	testCases := []struct {
 		name       string
 		inBytes    []byte
@@ -28,6 +28,58 @@ characters:
 				require.Nil(t, bytes)
 			},
 		},
+		{
+			name: "no extra quotes around true number",
+			inBytes: []byte(`
+characters:
+- name: Arthur Dent
+  answer: idk
+`),
+			updates: []Update{
+				{
+					Key:   "characters.0.answer",
+					Value: 42,
+				},
+			},
+			assertions: func(t *testing.T, bytes []byte, err error) {
+				require.NoError(t, err)
+				require.Equal(
+					t,
+					[]byte(`
+characters:
+- name: Arthur Dent
+  answer: 42
+`),
+					bytes,
+				)
+			},
+		},
+		{
+			name: "extra quotes around string containing a valid number",
+			inBytes: []byte(`
+characters:
+- name: Arthur Dent
+  answer: idk
+`),
+			updates: []Update{
+				{
+					Key:   "characters.0.answer",
+					Value: "42",
+				},
+			},
+			assertions: func(t *testing.T, bytes []byte, err error) {
+				require.NoError(t, err)
+				require.Equal(
+					t,
+					[]byte(`
+characters:
+- name: Arthur Dent
+  answer: "42"
+`),
+					bytes,
+				)
+			},
+		},
 		{
 			name: "success",
 			inBytes: []byte(`
@@ -57,7 +109,7 @@ characters:
 	}
 	for _, testCase := range testCases {
 		t.Run(testCase.name, func(t *testing.T) {
-			b, err := SetStringsInBytes(testCase.inBytes, testCase.updates)
+			b, err := SetValuesInBytes(testCase.inBytes, testCase.updates)
 			testCase.assertions(t, b, err)
 		})
 	}