fix: harden Sprite exec against command injection#2446
Merged
Conversation
louisgv
approved these changes
Mar 10, 2026
Member
louisgv
left a comment
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: APPROVED
Commit: 54a9409
Findings
NONE - All security improvements validated.
Security Improvements in This PR:
- Command injection via org name (HIGH → FIXED): Lines 111-114 validate org name against
^[A-Za-z0-9_-]+$before use - Word-splitting injection (MEDIUM → FIXED): Replaced unquoted
$(_sprite_org_flags)with array-based_sprite_cmd() - Grep regex injection (LOW → FIXED): Lines 164, 239 use
-qF(fixed string) instead of-q - Predictable temp file (LOW → FIXED): Line 194 uses
mktempwith random suffix
Tests
- bash -n: PASS
- bun test: PASS (1497 tests, 0 failures)
- curl|bash: N/A (library component, sourced by common.sh)
- macOS compat: OK (bash 3.2 compatible, no prohibited syntax)
-- security/pr-reviewer
…erns - Replace word-split _sprite_org_flags() call sites with _sprite_cmd() helper that uses a proper bash array for the -o flag, eliminating injection risk from org names with spaces or shell metacharacters - Validate _SPRITE_ORG against [A-Za-z0-9_-]+ in _sprite_validate_env - Use grep -qF (fixed-string) instead of grep -q for app name matching to prevent regex metacharacters in names from causing false matches - Use mktemp for _stderr_tmp in _sprite_exec instead of predictable PID-based path (/tmp/sprite-exec-err.$$) to prevent symlink attacks Closes #2436 Agent: complexity-hunter
louisgv
force-pushed
the
fix/sprite-injection
branch
from
54a9409 to
d2cc475
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
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.
Why: Issue #2436 flags direct command embedding in Sprite exec functions in sprite.sh that may allow injection if inputs are not properly sanitized.
Changes:
_sprite_cmd()helper that builds org flags as a proper bash array ("-o" "${_SPRITE_ORG}") instead of relying on word-splitting from_sprite_org_flags(). All 5 call sites (_sprite_provision_verify,_sprite_exec,_sprite_teardown,_sprite_cleanup_stale) now use the safe helper._SPRITE_ORGagainst[A-Za-z0-9_-]+in_sprite_validate_envto reject crafted org names.grep -q "${app}"togrep -qF "${app}"for fixed-string matching, preventing regex metacharacters in app names from causing false matches./tmp/sprite-exec-err.$$) withmktempto prevent symlink attacks.Closes #2436
-- refactor/complexity-hunter