Skip to content

fix: prevent shell injection in update-versions script#1

Merged
pproenca merged 1 commit intomasterfrom
codex/propose-fix-for-command-injection-vulnerability
Mar 7, 2026
Merged

fix: prevent shell injection in update-versions script#1
pproenca merged 1 commit intomasterfrom
codex/propose-fix-for-command-injection-vulnerability

Conversation

@pproenca
Copy link
Copy Markdown
Owner

@pproenca pproenca commented Mar 7, 2026

Motivation

  • Close a command-injection vector in scripts/update-versions.mjs where unsanitized skill directory names were interpolated into a shell command for git log, allowing command substitution from malicious directory names.

Description

  • Replace execSync with execFileSync and invoke git with an explicit argv array (["log","--oneline","--reverse","--format=%s","--", rel]) so directory names are passed as literal arguments and cannot perform shell substitution.
  • Preserve existing behavior and output formatting for deriving and updating skill metadata.json versions.

Testing

  • Ran node scripts/update-versions.mjs --dry-run successfully and observed the expected version update summary without modifying files.
  • Created a malicious directory named skills/.experimental/evil$(touch PWNED) with a metadata.json, ran the script in dry-run mode, and verified that PWNED was not created, confirming the injection was mitigated.

Codex Task

@pproenca pproenca merged commit d81ed10 into master Mar 7, 2026
1 check passed
pproenca added a commit that referenced this pull request Mar 19, 2026
Analyzed every commit from Michael Bolin (666) and jif-oai (597).
Found 24 recurring patterns not in our skills. Added the highest-impact ones:

rust-implement (270 -> 286 lines):
- Transformation 8: enum-as-disjoint-union (struct with options -> typed enum)
- Transformation 9: struct-for-long-parameter-lists (4+ params -> args struct)
- Thread-through plumbing (trace every intermediate layer)
- Drop/teardown precision (lock guards before .await, kill_on_drop)
- Defensive serialization (BEGIN IMMEDIATE for SQLite)
- Orphan event handling (events for dead entities)
- "Rewrite, don't rewire" principle

rust-write-tests (190 -> 247 lines):
- Test flake hunting protocol (Bolin's #1 pattern, 97+ refs)
- Intent-based assertions (semantic matching, not exact strings)
- Enhanced regression test naming (encode the guarded invariant)
- Stress-test repro commands (cargo nextest --stress-count 50)

rust-refactor (203 -> 240 lines):
- Two refactoring philosophies (Bolin defensive vs jif offensive)
- Diagnostic Q4: mutually exclusive optional fields -> enum
- Crate extraction with compile-time measurement
- Feature flag cleanup (full 8-step sequence)
- "Rewrite, don't rewire" principle
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant