Skip to content

Fix GCL_ array env vars not splitting semicolon-separated values#1778

Open
firecow wants to merge 7 commits intomasterfrom
fix/semicolon-split-array-env-vars
Open

Fix GCL_ array env vars not splitting semicolon-separated values#1778
firecow wants to merge 7 commits intomasterfrom
fix/semicolon-split-array-env-vars

Conversation

@firecow
Copy link
Copy Markdown
Owner

@firecow firecow commented Feb 26, 2026

  • Fix GCL_VARIABLE and other array-type env vars not splitting semicolon-separated values (e.g. GCL_VARIABLE="VAR1=hello;VAR2=world")
  • Array keys derived from yargs at runtime — no hardcoded list to maintain
  • Simplify argv.ts: getStringArray() helper reduces getter boilerplate, early return in injectDotenv()
  • Fix regex backtracking vulnerability in variable getter (SonarCloud S5852)

Fixes #992

@firecow firecow self-assigned this Feb 26, 2026
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/argv.ts">

<violation number="1" location="src/argv.ts:115">
P1: Bug: `flatMap` splitting on array elements corrupts CLI values containing semicolons. When multiple CLI flags are used (e.g., `--variable "MY_VAR=host=db;port=5432"`), the CLI framework already parses them into individual array elements. Splitting those elements on `;` corrupts values that legitimately contain semicolons (connection strings, CSS, etc.). The old code correctly left arrays untouched — only string inputs (from env vars) should be split.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@firecow firecow force-pushed the fix/semicolon-split-array-env-vars branch from ee5745e to 591c423 Compare February 26, 2026 14:19
@firecow firecow marked this pull request as draft February 27, 2026 15:47
When array options (--variable, --volume, etc.) are set via GCL_*
environment variables with semicolon-separated values, yargs wraps
the entire string as a single array element. This adds semicolon
splitting for all array-type options derived from yargs at runtime.

Also simplifies argv.ts: extracts getStringArray() helper to reduce
repeated array/string normalization in getters, and uses early return
in injectDotenv().
@firecow firecow force-pushed the fix/semicolon-split-array-env-vars branch from 15f82ae to 7e8fb51 Compare February 28, 2026 14:03
@firecow firecow changed the title Fix array env vars not splitting multiple semicolon-separated values Fix GCL_ array env vars not splitting semicolon-separated values Feb 28, 2026
Replace super-linear regex flagged by SonarCloud (S5852) with
a simpler equivalent that avoids overlapping alternatives.
@firecow firecow marked this pull request as ready for review February 28, 2026 14:26
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 5 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/test-cases/gcl-env-variable-split/integration.test.ts">

<violation number="1" location="tests/test-cases/gcl-env-variable-split/integration.test.ts:170">
P2: This test doesn't verify what its name claims. It passes already-split values (`["VAR1=hello", "VAR2=world"]`) directly to `handler()` rather than providing a semicolon-joined `GCL_VARIABLE` env var. The `splitSemicolonEnvVars` middleware is never exercised. To actually test env-split → handler integration, set `process.env.GCL_VARIABLE` (or mock it) and pass an unsplit argv.</violation>
</file>

<file name="src/argv.ts">

<violation number="1" location="src/argv.ts:125">
P2: Potential `TypeError` crash if a non-string, non-array value is stored in the map (e.g. `VOLUME=true` in a `.env` file gets stored as boolean `true` by `injectDotenv`). The old getters checked `typeof val == "string"` before calling `.split()`. Consider adding a type guard.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

firecow and others added 5 commits February 28, 2026 16:32
The test claimed to verify that env-split variables reach the handler,
but passed pre-split values directly. Now passes the unsplit value and
calls splitSemicolonEnvVars before handler, actually testing the middleware.
Resolves SonarCloud S3358 (nested ternary operation).
@sonarqubecloud
Copy link
Copy Markdown

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.

GCL_VARIABLE multiple key/value pairs not working

2 participants