feat(runner): VOD upload handling #1719
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds VOD (Video on Demand) upload handling functionality to the runner service. The changes introduce a new gRPC endpoint for handling VOD requests, add codec checking capabilities to determine if re-encoding is needed, and update the VOD creation process to conditionally transcode video based on codec and bitrate requirements.
Key changes:
- Added
HandleVODRPC endpoint to the runner service for processing VOD uploads - Implemented codec checking logic to determine if video re-encoding is necessary based on codec type and bitrate
- Updated VOD conversion to support both copy and transcode modes with configurable codecs
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| runner/runner.proto | Added HandleVOD RPC method and request/response message definitions |
| runner/protobuf/runner_grpc.pb.go | Generated gRPC client/server code for HandleVOD endpoint |
| runner/protobuf/runner.pb.go | Generated protobuf message types for HandleVODRequest/Response |
| runner/protobuf/notifications.pb.go | Updated protoc version references |
| runner/protobuf/commons.pb.go | Updated protoc version references |
| runner/pkg/ffmpeg/ffmpeg.go | Added Streams() method to retrieve codec information from ffprobe results |
| runner/pkg/actions/mkvod.go | Updated to support conditional re-encoding based on codec requirements |
| runner/pkg/actions/mkthumb.go | Minor formatting cleanup moving comment position |
| runner/pkg/actions/checkcodec.go | New action to probe video files and determine if re-encoding is needed |
| runner/handlers.go | Implemented HandleVOD handler to orchestrate VOD processing pipeline |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
joschahenningsen
force-pushed
the
feat/better-vod
branch
from
136617e to
92724c3
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| // Create directory structure in Ceph: mass/streamID/videoType/ | ||
| streamDir := filepath.Join(tools.Cfg.Paths.Mass, fmt.Sprintf("%d", stream.ID), string(req.VideoType)) | ||
| err = os.MkdirAll(streamDir, os.ModePerm) |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 18 days ago
To fix the vulnerability, ensure that user-controlled data (req.VideoType) cannot contain directory traversals or path separators when used in file paths. The safest way is to only allow known-safe values: either check that the input matches an allow list, or sanitize it by validating against a regular expression that permits only expected characters (such as alphanumerics and underscores). In this case, since req.VideoType appears to be an enum-like field with a Valid() check, we should also ensure that its string representation does not contain path separators or "..". Thus, before constructing streamDir, check if string(req.VideoType) contains '/', '\\', or "..". If so, return an error. This edit should be added just after the Valid() check and before constructing streamDir on line 491. No additional imports are necessary (we already import "strings"), and no modifications to files other than api/courses.go are required.
| @@ -465,6 +465,16 @@ | ||
| return | ||
| } | ||
|
|
||
| // Ensure videoType is a safe path component | ||
| videoTypeStr := string(req.VideoType) | ||
| if strings.Contains(videoTypeStr, "/") || strings.Contains(videoTypeStr, "\\") || strings.Contains(videoTypeStr, "..") { | ||
| _ = c.Error(tools.RequestError{ | ||
| Status: http.StatusBadRequest, | ||
| CustomMessage: "invalid video type (unsafe characters)", | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| stream, err := r.StreamsDao.GetStreamByID(context.Background(), req.StreamID) | ||
| if err != nil { | ||
| _ = c.Error(tools.RequestError{ |
|
|
||
| // Save file to Ceph | ||
| destPath := filepath.Join(streamDir, header.Filename) | ||
| destFile, err := os.Create(destPath) |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 18 days ago
To fix this issue, ensure that user-provided values used as path components (header.Filename and req.VideoType) are sanitized and/or validated before being used to construct file paths.
- For
header.Filename, only allow safe filenames: reject any string that contains path separators (/,\) or..sequences, or use a whitelist approach (e.g., allow only alphanumerics,_,-, few extensions). - For
req.VideoType, even though there’s aValid()function, ensure it only allows known, safe subdirectory names, and isn’t attacker-controlled. - This can be performed by adding checks after parsing the input and before constructing the file path. If invalid, return an error. Otherwise, proceed as before.
- The validation code should be directly added within the
uploadVODMediahandler, just before the file path construction.
No new helper methods or significant refactoring is necessary, but strings may be needed for processing (already imported).
| @@ -487,6 +487,15 @@ | ||
| } | ||
| defer file.Close() | ||
|
|
||
| // Validate uploaded filename to prevent directory traversal and unsafe paths | ||
| if strings.Contains(header.Filename, "/") || strings.Contains(header.Filename, "\\") || strings.Contains(header.Filename, "..") || len(header.Filename) == 0 { | ||
| _ = c.Error(tools.RequestError{ | ||
| Status: http.StatusBadRequest, | ||
| CustomMessage: "invalid filename", | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| // Create directory structure in Ceph: mass/streamID/videoType/ | ||
| streamDir := filepath.Join(tools.Cfg.Paths.Mass, fmt.Sprintf("%d", stream.ID), string(req.VideoType)) | ||
| err = os.MkdirAll(streamDir, os.ModePerm) |
No description provided.