Skip to content

FIX: Removing output_path paramater from video converter - #2558

Open
Richard Lundeen (rlundeen2) wants to merge 1 commit into
microsoft:mainfrom
rlundeen2:rlundeen2-secure-video-converter
Open

FIX: Removing output_path paramater from video converter#2558
Richard Lundeen (rlundeen2) wants to merge 1 commit into
microsoft:mainfrom
rlundeen2:rlundeen2-secure-video-converter

Conversation

@rlundeen2

Copy link
Copy Markdown
Contributor

Summary

  • Remove the caller-controlled output_path parameter from AddImageVideoConverter.
  • Change video_path to Path, consistent with other converter input files.
  • Write generated video bytes through data_serializer_factory and return its managed path.
  • Add registry support for coercing and exposing Path parameters to the backend UI.

Rationale

Allowing callers to select output_path lets UI-controlled input choose an arbitrary server-side write destination. A path traversal value could overwrite files outside the results store, and even valid-looking paths can cause unexpected filesystem side effects.

Selecting storage destinations is also outside converter scope. The converter should transform input into output. The serializer owns output location, storage backend selection, and persistence. Routing the result through data_serializer_factory removes the arbitrary-write surface and aligns this converter with the other file-producing converters.

This parameter is removed directly rather than deprecated because retaining it would preserve the security issue.

Validation

  • uv run --no-sync pytest tests/unit/converter/test_add_image_video_converter.py tests/unit/backend/test_converter_service.py tests/unit/models/test_parameter.py -q
  • uv run --no-sync pre-commit run --all-files

Route generated video data through the configured serializer so callers cannot select arbitrary write destinations.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 09fceba6-1124-4a88-96e8-6611959235e4
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