Skip to content

Handle parentheses in variables in commands#526

Closed
i7an wants to merge 3 commits intobkeepers:mainfrom
i7an:handle-parentheses
Closed

Handle parentheses in variables in commands#526
i7an wants to merge 3 commits intobkeepers:mainfrom
i7an:handle-parentheses

Conversation

@i7an
Copy link
Contributor

@i7an i7an commented Jan 8, 2025

Variables with parentheses are not handled correctly in commands:

FOO='passwo(rd'
BAR=$(echo '$FOO')

Result:

expected: {"BAR"=>"passwo(rd",           "FOO"=>"passwo(rd"}
     got: {"BAR"=>"$(echo 'passwo(rd')", "FOO"=>"passwo(rd"}

The solution I suggest is to parse commands first and expand variables before executing commands.

Dependencies

This change will break kamal which is included in Rails by default. This is how I want to handle it:

@mrj
Copy link

mrj commented Apr 1, 2025

Will this also fix #530 (improperly expanding double-quotes in command variables)?
This makes Kamal only able to use JSON secrets when its double-quotes are backslashed. I see you escaped quotes and braces in your secrets example. Shouldn't neither be necessary, so the secrets can be normal JSON?

@i7an
Copy link
Contributor Author

i7an commented Apr 8, 2025

@mrj Unfortunately this PR does not fix the issue you reported.

@bkeepers
Copy link
Owner

bkeepers commented Apr 8, 2025

@i7an thanks for reporting this and coming up with a fix. I'm good with going forward with this, let me know when you're ready.

Can you also add a test case that verifies that command => variable => command works?

FOO=$(echo bar)
BAR-$(echo $FOO)

Should equal

FOO=bar
BAR=bar

🐢
🐢
🐢

@i7an
Copy link
Contributor Author

i7an commented May 19, 2025

@bkeepers this PR is ready to be merged.
Context. Kamal released a new version that includes the backward compatibility fix which this PR was waiting for.

@i7an
Copy link
Contributor Author

i7an commented Jul 29, 2025

@bkeepers ping

@i7an
Copy link
Contributor Author

i7an commented Sep 21, 2025

@bkeepers ping ping

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.

3 participants