Add video export functionality for route replay videos#2275
Add video export functionality for route replay videos#2275
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ecovery, security improvements, and validations
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| body = JSON.parse(req.body) | ||
| urls = body['callback_urls'] | ||
| urls.length == 2 && | ||
| urls[0].start_with?('https://app.example.com') && |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
In general, this kind of issue is fixed by avoiding substring or prefix checks on raw URL strings when what you care about is the hostname. Instead, parse the URL and compare the host (and optionally scheme) against an explicit allowlist, or otherwise assert exact values rather than prefixes that could be extended with arbitrary content.
In this spec, we want to preserve the intent (ensuring that callback URLs use the expected hosts) without relying on start_with?('https://app.example.com'). The safest and clearest approach is: parse each URL with URI.parse, then assert that its scheme and host match the expected values. This keeps the test independent of the rest of the URL (path, query, etc.) but removes the substring-style check on the full URL string. Concretely, in spec/services/video_exports/request_render_spec.rb, lines 134–137 should be updated so that instead of urls[0].start_with?('https://app.example.com') and urls[1].start_with?('https://192.168.1.10'), we parse each URL and check uri.host. To keep requirements minimal, we can rely on Ruby’s standard URI library, adding a require 'uri' at the top of the file if it is not already present in the snippet, and use URI.parse(urls[0]).host == 'app.example.com' and URI.parse(urls[1]).host == '192.168.1.10'. Optionally, we can also assert uri.scheme == 'https' to match the intended protocol.
| @@ -1,6 +1,7 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require 'rails_helper' | ||
| require 'uri' | ||
|
|
||
| RSpec.describe VideoExports::RequestRender do | ||
| let(:user) { create(:user) } | ||
| @@ -131,9 +132,11 @@ | ||
| .with do |req| | ||
| body = JSON.parse(req.body) | ||
| urls = body['callback_urls'] | ||
| first_uri = URI.parse(urls[0]) | ||
| second_uri = URI.parse(urls[1]) | ||
| urls.length == 2 && | ||
| urls[0].start_with?('https://app.example.com') && | ||
| urls[1].start_with?('https://192.168.1.10') | ||
| first_uri.scheme == 'https' && first_uri.host == 'app.example.com' && | ||
| second_uri.scheme == 'https' && second_uri.host == '192.168.1.10' | ||
| end) | ||
| end | ||
|
|
| expect(WebMock).to(have_requested(:post, "#{video_service_url}/api/render") | ||
| .with do |req| | ||
| body = JSON.parse(req.body) | ||
| body['callback_url'].start_with?('https://app.example.com') |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High test
Copilot Autofix
AI about 1 month ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
2 participants
No description provided.