Skip to content

bug(cli): preserve base64 padding in environment variable parsing #881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

pgaijin66
Copy link
Contributor

Summary

This PR fixes a regression introduced when the Codefresh CLI switched from prepareKeyValueFromCLIEnvOption to prepareKeyValueObjectsFromCLIEnvOption for parsing CLI variables and secrets. The new parser broke support for values containing = characters, such as base64-encoded secrets. This update restores safe parsing by preserving everything after the first = in key=value pairs.

Description

While using the latest version of the Codefresh CLI (:latest), we recently observed failures in pipelines using environment variables that contain base64-encoded secrets. The errors surfaced as base64 decoding failures, even though the same values decoded correctly outside the pipeline.

With no changes made to our pipeline definitions or application code, we traced the issue to a change in how the CLI parses secrets passed via command-line flags.

When parsing key=value strings, the helper function prepareKeyValueObjectsFromCLIEnvOption split the string on all = characters and used only the second segment as the value. This caused any additional = characters (common in base64 padding) to be discarded, resulting in corrupted secrets.

Root Cause

A recent CLI update replaced the original parser prepareKeyValueFromCLIEnvOption with a new function prepareKeyValueObjectsFromCLIEnvOption to handle secrets and variables passed via CLI flags.

The original function correctly preserved the full value by splitting only on the first =:

const key = vars.split('=', 1)[0];
const value = vars.split('=').slice(1).join('=');

The new function, however, introduced a bug by splitting on all = characters and using only the second token:

const fields = vars.split('=');
const key = fields[0];
const value = fields[1];

This logic causes values to be truncated if they contain additional = characters, as is common with base64-encoded secrets (which often end in = or == for padding).

# Input
SECRET=YWJjZA==   # base64 for "abcd"

# Incorrect parsing
key: SECRET
value: YWJjZA    #  padding lost

# Expected
key: SECRET
value: YWJjZA==   #  full value preserved

Relevant code diff

- const variables = prepareKeyValueFromCLIEnvOption(this.argv.variable);
+ const variables = prepareKeyValueObjectsFromCLIEnvOption(this.argv.variable);
+ const secrets = prepareKeyValueObjectsFromCLIEnvOption(this.argv.secret).map((secret) => ({
+   ...secret, encrypted: true
+ }));
...
- request.options.variables = variables;
+ request.options.variables = variables.concat(secrets);

This change introduced the regression by altering how key=value pairs are parsed, causing silent corruption of any values containing multiple = characters.

Fix

The updated logic:

  1. Splits only on the first =
  2. Preserves all remaining characters (including =) in the value
  3. Avoids corrupting secrets that use encoding formats requiring padding

Impact

  1. Prevents silent corruption of base64-encoded secret values.
  2. Restores pipeline reliability when using secrets in key=value CLI input.
  3. Aligns behavior with the more robust prepareKeyValueFromCLIEnvOption function.
  4. Adds defensive validation against malformed or partial inputs.

@vadim-kharin-codefresh vadim-kharin-codefresh merged commit edd616d into codefresh-io:CR-28628-base64-padding-fix Jun 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants