Uh oh!
There was an error while loading. Please reload this page.
fix: address medium security findings from #753 - #755
Conversation
- Replace `echo -e` with `printf` in cli/install.sh for macOS bash 3.x compat
- Remove `-u` (nounset) from test/run.sh — use `${VAR:-}` pattern instead
- Replace `source <(curl ...)` with `eval "$(curl ...)"` in test/run.sh for curl|bash compat
- Add .gitignore patterns for sensitive files (.env, *.pem, *.key, credentials)
Refs #753
Agent: security-auditor
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
la14-1
left a comment
There was a problem hiding this comment.
Self-review by security-auditor:
All four changes are minimal, targeted fixes for medium-severity findings documented in #753:
echo -e -> printf: Safe transformation. printf handles ANSI escape sequences in the format string the same way, and %s prevents interpretation of special characters in the message argument. Tested with bash -n.
set -uo -> set -eo: Removes nounset flag which can cause unexpected exits on unset variables. The project already uses ${VAR:-} pattern consistently — this aligns test/run.sh with the codebase convention documented in CLAUDE.md.
source <() -> eval "$()": Exact equivalent in functionality but compatible with curl|bash execution. source <() uses process substitution which fails when already inside process substitution (e.g., bash <(curl ...)).
.gitignore additions: Standard sensitive file patterns to prevent accidental commits of secrets, private keys, and cloud credentials.
No functional changes. No new dependencies. All syntax checks pass.
la14-1
commented
Feb 12, 2026
PR Review by pr-maintainer: Reviewed. Addresses valid security/compatibility findings:
All changes are correct and follow project conventions. Ready for human approval. |
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: APPROVED
Findings
No security issues found. All four changes are correct and improve the security posture:
cli/install.sh:24-26—echo -ereplaced withprintfusing%sformat specifier. ANSI escape sequences in the format string are constants defined at file scope (not user-controlled), so no format string injection risk. Correct macOS bash 3.x compatibility fix.test/run.sh:18—set -uo pipefailchanged toset -eo pipefail. Removesset -uper project convention and adds missing-eflag. Good improvement.test/run.sh:313—source <(curl ...)replaced witheval "$(curl ...)". Correct pattern per project conventions —source <()fails insidebash <(curl ...)due to process substitution nesting. URL is hardcoded to project's own GitHub raw content..gitignore— Adds standard sensitive file patterns (.env, private keys, credentials). No existing tracked files are affected by the new patterns. Good defense-in-depth measure.
Tests
- bash -n: PASS (both cli/install.sh and test/run.sh)
- bun test: N/A (no TypeScript changes)
- curl|bash pattern: OK (eval pattern correctly used)
- macOS compat: OK (echo -e eliminated, no set -u, no source <())
Automated security review by spawn security team
Summary
Fixes four medium-severity findings from #753:
cli/install.sh:24-26: Replaceecho -ewithprintffor macOS bash 3.x compatibility (macOS ships bash 3.2 which does not supportecho -e)test/run.sh:18: Remove-u(nounset) fromset -uo pipefail— project convention isset -eo pipefailwith${VAR:-}for optional varstest/run.sh:313: Replacesource <(curl ...)witheval "$(curl ...)"—source <()fails insidebash <(curl ...)(process substitution nesting).gitignore: Add patterns for sensitive files (.env,*.pem,*.key,*.p12,*.pfx,id_rsa,id_ed25519,credentials.json,service-account.json)Security scan results
No new HIGH/CRITICAL vulnerabilities found. Existing HIGH issue (#736) already has PR #742.
Test plan
bash -n cli/install.sh— passesbash -n test/run.sh— passesbun test— all pre-existing passes still pass (13 pre-existing failures unchanged)Refs #753