-
Notifications
You must be signed in to change notification settings - Fork 2
Refactor #290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Refactor #290
Conversation
| this.instance = axios.create({ | ||
| baseURL, | ||
| httpsAgent: new https.Agent({ | ||
| rejectUnauthorized: false |
Check failure
Code scanning / CodeQL
Disabling certificate validation High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the problem, the rejectUnauthorized: false option should be removed or set to true (the default). This will ensure that HTTPS requests made by the Axios client will validate server certificates, protecting against man-in-the-middle attacks. The best way to fix this is to remove the httpsAgent option entirely if there is no need for custom agent configuration, or to set rejectUnauthorized: true if a custom agent is still required for other reasons. The change should be made in the constructor of DigmaApiClient, specifically on lines 27–29 of src/api/DigmaApiClient.ts.
| @@ -26,5 +26,2 @@ | ||
| baseURL, | ||
| httpsAgent: new https.Agent({ | ||
| rejectUnauthorized: false | ||
| }), | ||
| headers: { |
| const response = await this.client.client.get<GetIncidentResponse>( | ||
| this.getUrl(`incidents/${data.id}`) | ||
| ); |
Check failure
Code scanning / CodeQL
Server-side request forgery Critical
URL
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix this SSRF vulnerability, we should validate and sanitize the incidentId parameter before using it to construct the outgoing request URL. The best approach is to enforce a strict allow-list of acceptable formats for incidentId. If incidentId is expected to be a UUID, we should check that it matches a UUID regex. If it is a numeric ID, we should check that it is a number. This validation should be performed as close as possible to the entry point of the tainted data, ideally in attachIncidentFileToChatContext in src/uris/handlers/context.ts, before passing the value to the API client. If the value does not match the expected format, we should throw an error or reject the request.
Required changes:
- In
src/uris/handlers/context.ts, add validation forincidentIdat the start ofattachIncidentFileToChatContext. - If the value is invalid, throw an error.
- No new imports are needed if we use a simple regex.
-
Copy modified lines R10-R14
| @@ -9,2 +9,7 @@ | ||
| export const attachIncidentFileToChatContext = async (incidentId: string) => { | ||
| // Validate incidentId: allow only UUIDs (adjust regex as needed for your format) | ||
| const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; | ||
| if (!uuidRegex.test(incidentId)) { | ||
| throw new Error("Invalid incidentId format"); | ||
| } | ||
| try { |
| const filePath = path.join(tempDir, fileName); | ||
| const content = JSON.stringify(incidentData, null, 2); | ||
|
|
||
| fs.writeFileSync(filePath, content, "utf8"); |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the problem, we need to ensure that the incidentId value cannot be used to perform path traversal or inject special characters into the filename. The best way to do this is to sanitize the incidentId before using it in the filename. We can use the well-known sanitize-filename npm package to remove any potentially dangerous characters from the incidentId. This will ensure that the resulting filename is safe and cannot be used to escape the intended directory. The changes should be made in src/uris/handlers/context.ts, specifically where the filename is constructed. We will need to import sanitize-filename and apply it to incidentId before using it in the filename.
-
Copy modified line R6 -
Copy modified lines R17-R18
| @@ -5,2 +5,3 @@ | ||
| import vscode from "vscode"; | ||
| import sanitizeFilename from "sanitize-filename"; | ||
| import { getDigmaClient } from "../../extension"; | ||
| @@ -15,3 +16,4 @@ | ||
| const tempDir = os.tmpdir(); | ||
| const fileName = `incident-${incidentId}-${formatISO(new Date(), { | ||
| const safeIncidentId = sanitizeFilename(incidentId); | ||
| const fileName = `incident-${safeIncidentId}-${formatISO(new Date(), { | ||
| format: "basic" |
-
Copy modified lines R162-R163
| @@ -161,3 +161,4 @@ | ||
| "husky": "^9.1.7", | ||
| "path-to-regexp": "^8.2.0" | ||
| "path-to-regexp": "^8.2.0", | ||
| "sanitize-filename": "^1.6.3" | ||
| } |
| Package | Version | Security advisories |
| sanitize-filename (npm) | 1.6.3 | None |
No description provided.