Skip to content

fix: use single-quoted shell escaping for --format env output#3856

Merged
ss1909 merged 3 commits into
mainfrom
sneha/A-1153
Apr 27, 2026
Merged

fix: use single-quoted shell escaping for --format env output#3856
ss1909 merged 3 commits into
mainfrom
sneha/A-1153

Conversation

@ss1909
Copy link
Copy Markdown
Contributor

@ss1909 ss1909 commented Apr 27, 2026

Description

Extracts the secretGet logic from the CLI action into a testable function, and adds a unit test suite for it. Also fixes the env format output to use single-quoted shell escaping (via shellQuote) instead of Go's %q double-quoting, so values are safe to source in shell without unintended interpolation.

Context

The secretGet logic previously lived inside the cli.Command action, making it impossible to unit test without going through the CLI framework. The env format also used %q which produces Go-style double-quoted strings — not correct shell quoting for use with source or declare -x.

Changes

  • Extracted secretGet into a standalone function accepting

  • Fixed env format to use shellQuote (single quotes with ''' escaping) instead of %q

  • Added clicommand/secret_get_test.go

Testing

  • Tests have run locally (with go test ./...). Buildkite employees may check this if the pipeline has run automatically.
  • Code is formatted (with go tool gofumpt -extra -w .)

Disclosures / Credits

Claude Code wrote some of the unit tests, then I changed most of it 😄

@ss1909 ss1909 force-pushed the sneha/A-1153 branch 2 times, most recently from 2ef769e to 2b441f2 Compare April 27, 2026 04:22
@ss1909 ss1909 marked this pull request as ready for review April 27, 2026 05:19
@ss1909 ss1909 requested review from a team as code owners April 27, 2026 05:19
Copy link
Copy Markdown
Contributor

@zhming0 zhming0 left a comment

Choose a reason for hiding this comment

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

There might be a blocker re \n handling but otherwise I think this PR is in the right direction 👍🏿

Comment thread clicommand/secret_get_test.go
Comment thread clicommand/secret_get_test.go
Comment thread clicommand/secret_get.go
Comment thread clicommand/secret_get_test.go
Copy link
Copy Markdown
Contributor

@zhming0 zhming0 left a comment

Choose a reason for hiding this comment

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

LGTM, I hallucinated about the newline issue 😅

@ss1909 ss1909 merged commit ecd11dd into main Apr 27, 2026
3 checks passed
@ss1909 ss1909 deleted the sneha/A-1153 branch April 27, 2026 08:23
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