Skip to content

fix(ai): avoid shell execution for fixture tags - #9293

Open
russellwheatley wants to merge 1 commit into
mainfrom
fix/ai-fixture-tag-shell-execution
Open

fix(ai): avoid shell execution for fixture tags#9293
russellwheatley wants to merge 1 commit into
mainfrom
fix/ai-fixture-tag-shell-execution

Conversation

@russellwheatley

@russellwheatley russellwheatley commented Sep 4, 2026

Copy link
Copy Markdown
Member

The AI test-fixture fetcher (scripts/fetch_ai_mock_responses.ts) interpolated the latest git tag from the external vertexai-sdk-test-data repo directly into an execSync shell string. A tag containing shell metacharacters (e.g. v17.0;id) would have the non-tag portion executed by the shell.

  • Switch both git invocations (ls-remote, clone) to execFileSync with argument arrays
  • Add a -- separator before positional repo/path args
  • Wrap the top-level logic in an exported fetchAiMockResponses() behind a require.main === module guard so it's testable
  • Add a regression test asserting the malicious tag never reaches shell interpretation

Reported by Oskar Eichler, who also supplied the original patch this is based on.


Maintainer note: Fixes internal CPRN-418

@russellwheatley russellwheatley changed the title fix(ai): [MEDIUM] avoid shell execution for fixture tags fix(ai): avoid shell execution for fixture tags Sep 4, 2026
@russellwheatley
russellwheatley force-pushed the fix/ai-fixture-tag-shell-execution branch from 22c934c to 71fd059 Compare September 4, 2026 10:40
The AI test-fixture fetcher interpolated the latest git tag from the
external vertexai-sdk-test-data repository directly into an execSync
shell string. A tag containing shell metacharacters (e.g. v17.0;id)
would have its non-tag portion executed by the shell.

Switch both git invocations to execFileSync with argument arrays and
add a -- separator before positional repo/path args, so an
attacker-controlled tag can never be parsed as shell syntax.

Reported by Oskar Eichler, who also supplied the fix.
@russellwheatley
russellwheatley force-pushed the fix/ai-fixture-tag-shell-execution branch from 71fd059 to 6f7acdf Compare September 4, 2026 10:50
@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.27273% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.45%. Comparing base (afaba0b) to head (6f7acdf).
⚠️ Report is 24 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #9293      +/-   ##
============================================
+ Coverage     69.45%   69.45%   +0.01%     
+ Complexity     2122     2121       -1     
============================================
  Files           443      444       +1     
  Lines         25537    25570      +33     
  Branches       4259     4264       +5     
============================================
+ Hits          17733    17756      +23     
- Misses         6404     6409       +5     
- Partials       1400     1405       +5     
Flag Coverage Δ
android-native 65.74% <ø> (-<0.01%) ⬇️
e2e-ts-android 54.09% <ø> (-0.05%) ⬇️
e2e-ts-ios 53.61% <ø> (ø)
e2e-ts-macos 49.88% <ø> (ø)
ios-ruby 100.00% <ø> (ø)
jest 47.82% <77.28%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@russellwheatley
russellwheatley marked this pull request as ready for review September 8, 2026 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant