fix: install.sh printf format strings and e2e source documentation#2445
Merged
fix: install.sh printf format strings and e2e source documentation#2445
Conversation
louisgv
approved these changes
Mar 10, 2026
Member
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: APPROVED
Commit: c208a14
Findings
✅ No security issues found. This PR improves security posture.
Changes Reviewed
-
sh/cli/install.sh (lines 27-30, 242-244)
- Replaced variable interpolation in printf format strings with %b placeholders
- Prevents potential format string injection if color variables were user-controlled
- Single-quoted format strings prevent shell expansion
-
sh/e2e/lib/common.sh (lines 34-56)
- Same printf format string hardening as install.sh
- Improved logging security posture
-
sh/e2e/lib/common.sh (lines 62-63, 70, 80)
- Added documentation clarifying intentional use of BASH_SOURCE and source
- No behavioral change, documentation-only improvement
Security Improvements
- ✅ Eliminates format string injection vectors in logging functions
- ✅ Hardens printf calls with single-quoted format strings
- ✅ Variables passed as positional arguments are properly escaped
Tests
- bash -n install.sh: ✅ PASS
- bash -n common.sh: ✅ PASS
- curl|bash safety: ✅ OK (install.sh has no relative paths or BASH_SOURCE)
- macOS compat: ✅ OK (portable printf syntax, no echo -e)
-- security/pr-reviewer
install.sh: Replace color variable interpolation in printf format strings with %b arguments to prevent format string injection (fixes #2443). common.sh: Use %b for color escapes in logging functions. Document that BASH_SOURCE and source usage in load_cloud_driver is intentional since e2e scripts are filesystem-only, not curl|bash (fixes #2438). Agent: ux-engineer Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
c208a14 to
4791533
Compare
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
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.
Summary
printfformat strings with%barguments. Previously, ANSI escape codes were embedded directly in the format string (e.g.,printf "${GREEN}[spawn]${NC} %s\n"), which is a format string injection vector. Now usesprintf '%b[spawn]%b %s\n' "$GREEN" "$NC" "$1".%bfix to all logging functions (log_header,log_step,log_ok,log_err,log_warn,log_info). Add documentation clarifying thatBASH_SOURCEandsourceusage inload_cloud_driveris intentional since e2e scripts are always run from the filesystem, not viabash <(curl ...).Closes #2443
Closes #2438
-- refactor/ux-engineer