Skip to content

Potential fix for code scanning alert no. 2: Uncontrolled command line#15

Merged
cwlacewe merged 1 commit intomainfrom
alert-autofix-2
Jan 22, 2026
Merged

Potential fix for code scanning alert no. 2: Uncontrolled command line#15
cwlacewe merged 1 commit intomainfrom
alert-autofix-2

Conversation

@cwlacewe
Copy link
Contributor

Potential fix for https://github.com/IntelLabs/Video-Curation-Sample/security/code-scanning/2

In general, to fix this, we should validate and constrain the user-controlled video parameter before using it to build the ffprobe command. The path joining is already done safely, but we should additionally ensure that the video argument is a “safe” relative file name (no path separators, not empty, optionally with a required extension). That way, even though Popen is called with shell=False, we explicitly restrict what can be passed to ffprobe.

The best targeted fix without changing existing behavior is to introduce a small helper function in video/utils.py (where safely_join_path already lives) that validates the filename portion, and then use it in InfoHandler.get before passing video down to _get_info. The helper can reject values that contain path separators or are empty/whitespace. In get, we call this validator right after reading the video argument; if validation fails, we return a 400 error. The rest of the logic, including the ffprobe invocation and response format, remains unchanged.

Concretely:

  • In video/utils.py, add a new function validate_video_name(name: str) -> str that:
    • Ensures name is a string.
    • Strips surrounding whitespace.
    • Rejects empty strings.
    • Rejects any name containing / or os.sep (to ensure it’s just a file name, not a path).
    • Optionally can be extended to enforce an extension, but we’ll keep it generic to avoid changing semantics.
  • In video/info.py, import validate_video_name from utils, and in InfoHandler.get:
    • After decoding the argument, call validate_video_name(video) inside a try block.
    • On ValueError, respond with HTTP 400 and stop processing.
    • Otherwise, use the validated video (possibly normalized) for the existing checks and for _get_info.

This keeps the behavior for valid inputs the same, but ensures that only sanitized, well-formed file names reach the Popen command.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@cwlacewe cwlacewe marked this pull request as ready for review January 22, 2026 01:55
@cwlacewe cwlacewe merged commit 799a355 into main Jan 22, 2026
4 checks passed
@sys-vdms sys-vdms deleted the alert-autofix-2 branch January 22, 2026 03:25
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.

2 participants