fix(installer): _extract_yaml_value corrupts '' escape on bash 3.2 (macOS) - #509
Merged
Merged
Conversation
…acOS)
A YAML single-quoted value escapes a quote by doubling it ('' -> '). The unescape
used `${line//\'\'/\'}`, whose `\'` REPLACEMENT literal is bash-version dependent:
bash 4/5 (Linux CI) yields the intended `'`, but bash 3.2 (the macOS system bash)
keeps the backslash and produces `a\'b` — a corrupted clientPassword for any macOS
user whose password contains a doubled quote. The bats test for this case has been
failing on macOS while passing on the Linux CI leg for exactly this reason.
Use a variable for the quote in the pattern + replacement (`local _sq="'"`), which
expands to a bare quote on bash 3.2 and 4/5 alike. Existing test kept; now passes on
both. No behavior change on Linux.
Contributor
|
👋 Heads-up — Code review queue is at 42 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
shujaatTracebloc
marked this pull request as ready for review
July 31, 2026 07:19
Contributor
|
👋 Heads-up — Code review queue is at 50 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
…ingle-quote-escape # Conflicts: # scripts/manifest.sha256
saadqbal
approved these changes
Jul 31, 2026
Contributor
|
/fr-pass |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix
_extract_yaml_value''unescape on bash 3.2 (macOS)A YAML single-quoted value escapes an embedded quote by doubling it (
''→'). The unescape used:line="${line//\'\'/\'}"The
\'in the replacement is bash-version dependent:'a\'bSo a macOS user whose
clientPasswordcontains a doubled quote gets a corrupted value, and the existing bats test_extract_yaml_value: single-quoted with '' escapehas been failing on macOS while passing on Linux for exactly this reason.Fix: use a variable for the quote in the pattern + replacement (
local _sq="'";${line//$_sq$_sq/$_sq}), which expands to a bare quote identically on bash 3.2 and 4/5. No behavior change on Linux.This was spotted while working an unrelated installer PR; landing it standalone (not folded into a feature PR) per convention.
Tests
The existing test is kept and now passes on both macOS bash 3.2 and Linux. Full
install-client-helm.batsgreen (64/64) on bash 3.2 locally;shellcheck --severity=error+ check-style + check-drift clean;scripts/manifest.sha256regenerated.Note
Low Risk
Small, targeted bash portability fix in YAML parsing with existing test coverage; no auth or deployment logic changes beyond correct credential reads on macOS.
Overview
Fixes YAML
''→'unescape in_extract_yaml_valueon macOS system bash (3.2).The installer reads
clientId/clientPasswordfromvalues.yamlvia_extract_yaml_value. For single-quoted YAML values, doubled quotes must collapse to one quote. The previous${line//\'\'/\'}replacement behaved correctly on bash 4/5 (Linux CI) but on bash 3.2 left a literal backslash in the value (a\'binstead ofa'b), breaking passwords that contain an apostrophe and failing the bats case_extract_yaml_value: single-quoted with '' escapeon macOS.The change uses a variable holding the quote character and
${line//$_sq$_sq/$_sq}so the replacement is a bare quote on both bash versions. Linux behavior is unchanged.scripts/manifest.sha256is updated for the touched script.Reviewed by Cursor Bugbot for commit 103e5be. Bugbot is set up for automated code reviews on this repo. Configure here.