fix: harden install-flex against injection, input validation, and supply-chain risks#12
Open
lgarceau768 wants to merge 5 commits intoflexfrom
Open
fix: harden install-flex against injection, input validation, and supply-chain risks#12lgarceau768 wants to merge 5 commits intoflexfrom
lgarceau768 wants to merge 5 commits intoflexfrom
Conversation
…isks Address all findings from security review: - (High/F1) Replace perl s|...|...| substitution with printf %q heredoc to eliminate Perl injection via CLONE_DIR containing | or ; characters - (High/F2) Validate all user inputs: CLONE_DIR blocked on shell metacharacters, ACCOUNT_ID enforced as exactly 12 digits, AWS_REGION validated against standard pattern; tilde expanded before use - (Medium/F4) Add --frozen-lockfile to bun install so transitive versions are pinned to bun.lock, preventing silent upgrades when BUN_CONFIG_REGISTRY bypasses Artifactory - (Medium/F5) Wrap eval + opencode invocation in a subshell so exported AWS credentials do not persist in the interactive shell after opencode exits - (Medium/F6) Guard existing-clone path: abort on uncommitted changes or wrong branch; replace git checkout with merge --ff-only; add --depth 1 to fresh clone - (Medium/F7) Replace cat >> ~/.aws/config heredoc with aws configure set calls for atomic, section-safe writes; use aws configure get for idempotency check - (Low/F8) Verify aws-cli is v2 in check_prereqs; v1 produces an upgrade hint - (Low/F9) grep -q regex on profile name replaced by aws configure get (F7) - (Low/F10) Idempotency sentinel changed from function signature to fixed header comment, guarded with grep -qF
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
tdonaworth
reviewed
Apr 23, 2026
| CLONE_DIR="${CLONE_DIR:-$HOME/opencode}" | ||
| # Expand leading ~ to $HOME before validation and use | ||
| CLONE_DIR="${CLONE_DIR/#\~/$HOME}" | ||
| # Reject shell metacharacters that could corrupt the rc file |
There was a problem hiding this comment.
I'd also add in rejection for control characters (newlines, tabs, etc)
# Reject control characters (especially newlines)
if [[ "$CLONE_DIR" =~ [[:cntrl:]] ]]; then
die "Invalid clone directory — control characters are not allowed"
fi
| } | ||
| } | ||
| } | ||
| EOF |
There was a problem hiding this comment.
current file is world readable as it's written out, can update file permissions immediately:
chmod 600 "$config_file"
or 644 if not as paranoid.
…permissions - Add [[:cntrl:]] check to block newlines, tabs, and other control characters from CLONE_DIR input (per PR review comment) - chmod 600 opencode.json after write so it is not world-readable (per PR review comment)
- Replace pre-commit with prek (Rust-based alternative) - Update CONTRIBUTING.md with prek installation instructions - Update .husky/pre-push to use prek command - Add exclude pattern for JSON5 files (tsconfig.json, .oxlintrc.json) - Remove Python 3.10+ requirement (prek is a single binary) Benefits: - Faster hook execution (Rust vs Python) - No Python runtime dependency required - Drop-in compatible with existing .pre-commit-config.yaml - Better security features (SHA validation, cooldown periods)
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
2 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.
Issue for this PR
Closes #
Type of change
What does this PR do?
Addresses 10 findings from a security review of the
install-flexinstaller script. All changes are contained to that single file.High severity (2 fixed)
CLONE_DIR_PLACEHOLDERwith a single-quoted heredoc, then substituted the real path usingperl -i -pe "s|...|${CLONE_DIR}|g". A clone directory containing|would terminate the Perls///delimiter and allow arbitrary Perl execution against~/.zshrc/~/.bashrc. Fixed by switching to a double-quoted heredoc where$(printf '%q' "$CLONE_DIR")shell-quotes the path at install time — no Perl involved.CLONE_DIR,ACCOUNT_ID, andAWS_REGIONwere accepted without any checks. Added: shell-metacharacter denylist onCLONE_DIR(plus~expansion), 12-digit enforcement onACCOUNT_IDwith re-prompt on failure, and region-format regex onAWS_REGION.Medium severity (4 fixed)
--frozen-lockfiletobun installsobun.lockversions are pinned and can't silently drift whenBUN_CONFIG_REGISTRYbypasses Artifactory.eval "$(aws configure export-credentials ...)"+ binary invocation in a subshell so exported credentials are scoped to theopencodeprocess and don't persist in the interactive shell.HEADis not on theflexbranch.git checkoutreplaced withmerge --ff-only. Fresh clones get--depth 1.cat >> ~/.aws/configheredoc (non-atomic, fragile) with fiveaws configure setcalls. Idempotency check changed from agrepregex toaws configure get.Low severity (3 fixed)
check_prereqsnow verifiesaws --versionreportsaws-cli/2.*; v1 produces an upgrade link instead of a cryptic failure later.grep -q "\[profile ...]"regex is gone entirely."opencode-work()"(which a stale comment could satisfy) togrep -qF "── Flexion opencode launcher ──".How did you verify your code works?
bash -n install-flex— syntax check passes clean.$clone_dir_qexpands at install time; all other$references in the generated function body are\-escaped and expand correctly at runtime.aws configure setidempotency path by checking thataws configure get sso_start_url --profile ClaudeCodeAccessexits non-zero on a fresh config and zero on an existing one.Screenshots / recordings
N/A — installer script only.
Checklist