Performance and minor fixes for prod release #138
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Performance indices and also updates for handling TG Task Submissions of URL only
Minor tweak for better support of the upload panel in CA
Use legacy scorecard ID as a fallback for legacy data
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const isFileSubmission = hasUploadedFile; | ||
| const hasS3Url = | ||
| typeof body.url === 'string' && | ||
| body.url.includes('https://s3.amazonaws.com'); |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 13 days ago
The fix is to properly parse body.url and check its host rather than searching for a substring. Use the standard Node.js URL class to parse the URL. Then, check whether the host is one of the allowed hosts for S3 (e.g., s3.amazonaws.com or region-specific variants like s3.<region>.amazonaws.com).
In this code, to replace the substring check, introduce host checks such as:
- host exactly equals
s3.amazonaws.com - host ends with
.s3.amazonaws.com - host matches common region patterns (e.g.,
my-bucket.s3.eu-west-1.amazonaws.com)
To implement this:
- Replace line 1654 with: check that
body.urlis a string, parse it withnew URL(body.url), and test the host. - Add try/catch to guard against invalid URLs.
- Use a helper function or inline code for host matching; avoid changing broader logic outside this region.
No external packages are needed, as the built-in URL class suffices.
Suggested changeset
1
src/api/submission/submission.service.ts
| @@ -1649,9 +1649,25 @@ | ||
| !!file && | ||
| ((typeof file.size === 'number' && file.size > 0) || | ||
| (file.buffer && file.buffer.length > 0)); | ||
| const hasS3Url = | ||
| typeof body.url === 'string' && | ||
| body.url.includes('https://s3.amazonaws.com'); | ||
| let hasS3Url = false; | ||
| if (typeof body.url === 'string') { | ||
| try { | ||
| const urlObj = new URL(body.url); | ||
| // Accept s3.amazonaws.com and any subdomain of s3.amazonaws.com | ||
| const s3Hosts = [ | ||
| 's3.amazonaws.com', | ||
| ]; | ||
| // Accept region pattern: *.s3.amazonaws.com or *.s3.<region>.amazonaws.com | ||
| const host = urlObj.host; | ||
| hasS3Url = | ||
| s3Hosts.includes(host) || | ||
| host.endsWith('.s3.amazonaws.com') || | ||
| /^s3\.[a-z0-9-]+\.amazonaws\.com$/.test(host) || | ||
| /^[^\.]+\.s3\.[a-z0-9-]+\.amazonaws\.com$/.test(host); | ||
| } catch (e) { | ||
| hasS3Url = false; | ||
| } | ||
| } | ||
| const isFileSubmission = hasUploadedFile || hasS3Url; | ||
|
|
||
| // Derive common metadata if available |
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.