Skip to content

fix: validate command input to prevent execution of untrusted input#2336

Draft
vikassaini77 wants to merge 10 commits into
roboflow:developfrom
vikassaini77:fix/stream-validation
Draft

fix: validate command input to prevent execution of untrusted input#2336
vikassaini77 wants to merge 10 commits into
roboflow:developfrom
vikassaini77:fix/stream-validation

Conversation

@vikassaini77

Copy link
Copy Markdown
Contributor
Before submitting
  • Self-reviewed the code
  • Updated documentation, follow Google-style
  • Added docs entry for autogeneration (if new functions/classes)
  • Added/updated tests
  • All tests pass locally

Description

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📝 Documentation update
  • 🧪 Test update
  • 🔨 Refactoring (no functional changes)
  • ⚡ Performance improvement
  • 🔧 Chore (dependencies, configs, etc.)

Motivation and Context

Closes #(issue)

Changes Made

Testing

  • I have tested this code locally
  • I have added unit tests that prove my fix is effective or that my feature works
  • All new and existing tests pass

Google Colab (optional)

Colab link:

Screenshots/Videos (optional)

Additional Notes

@vikassaini77 vikassaini77 requested a review from SkalskiP as a code owner June 17, 2026 13:09
@CLAassistant

CLAassistant commented Jun 17, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@Borda

Borda commented Jun 17, 2026

Copy link
Copy Markdown
Member

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

@vikassaini77 mind sign it pls otherwise there is nothing I can do to land it... 🦝

@Borda Borda marked this pull request as draft June 17, 2026 20:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the examples/time_in_zone streaming helper by adding an allowlist check before executing external commands, reducing the risk of running unexpected executables when run_command() is called.

Changes:

  • Added an allowlist (docker, ffmpeg) validation in run_command() before invoking subprocess.run.
  • Replaced the prior “TODO validate command input” note with a concrete validation step.

Comment on lines +87 to +91
allowed_commands = ["docker", "ffmpeg"]
if not command or command[0] not in allowed_commands:
raise ValueError(
f"Command '{command[0] if command else ''}' is not allowed. Only {allowed_commands} are permitted."
)
@Borda Borda marked this pull request as ready for review June 18, 2026 13:36
@Borda

Borda commented Jun 18, 2026

Copy link
Copy Markdown
Member

@vikassaini77 pls follow the contribution guidelines. In particular, it means you shall write PR description why it does and how it fixes or improves this package, even I see based on your changes a value in your contribution 🦝

@Borda Borda marked this pull request as draft June 18, 2026 13:38
@codecov

codecov Bot commented Jun 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 40.00000% with 66 lines in your changes missing coverage. Please review.
✅ Project coverage is 81%. Comparing base (07d182b) to head (c974c71).
⚠️ Report is 14 commits behind head on develop.

❌ Your patch check has failed because the patch coverage (40%) is below the target coverage (95%). You can increase the patch coverage or adjust the target coverage.
❌ Your project check has failed because the head coverage (81%) is below the target coverage (95%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@           Coverage Diff           @@
##           develop   #2336   +/-   ##
=======================================
- Coverage       81%     81%   -0%     
=======================================
  Files           66      66           
  Lines         9077    9127   +50     
=======================================
+ Hits          7377    7378    +1     
- Misses        1700    1749   +49     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vikassaini77 vikassaini77 marked this pull request as ready for review June 18, 2026 16:16
@Borda Borda marked this pull request as draft June 18, 2026 16:39
@vikassaini77 vikassaini77 marked this pull request as ready for review June 19, 2026 15:17
@Borda Borda marked this pull request as draft June 19, 2026 17:59
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.

4 participants