-
Notifications
You must be signed in to change notification settings - Fork 228
[comp] Production Deploy #1983
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
[comp] Production Deploy #1983
Conversation
* feat(aikido): add Aikido security integration with vulnerability checks * feat(aikido): enhance OAuth and variable controllers with improved error handling and logging * feat(aikido): improve repository fetching and error handling in checks * fix(aikido): improve error handling for code repository scanning * fix(aikido): improve error handling for code repository scanning * fix(aikido): enhance error handling and update fetch logic for repositories * fix(aikido): update OAuth client credential handling and improve error logging --------- Co-authored-by: Lewis Carhart <lewis@trycomp.ai> Co-authored-by: Mariano Fuentes <marfuen98@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
PR SummaryIntroduces a new security integration and refines automation, OAuth, and UI flows across the platform.
Written by Cursor Bugbot for commit 90b0c49. This will update automatically on new commits. Configure here. |
|
|
| return repositories.map((repo) => ({ | ||
| value: String(repo.id), | ||
| label: `${repo.name} (${repo.provider})`, | ||
| })); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent API response handling causes potential crash
The targetRepositoriesVariable.fetchOptions function in variables.ts assumes ctx.fetch always returns an array and directly calls .map() on the response. However, code-repository-scanning.ts defensively handles both array responses and wrapped responses (like { repositories: [...] }) using Array.isArray(response) ? response : (response?.repositories ?? []). If the Aikido API returns a wrapped response, the variable fetching will crash with a "map is not a function" error, preventing users from selecting target repositories during integration setup.
Additional Locations (1)
|
|
||
| const diffDays = (Date.now() - lastScanMs) / (1000 * 60 * 60 * 24); | ||
| return diffDays > SCAN_STALE_DAYS; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invalid date strings cause staleness check to pass incorrectly
The isStale function parses date strings using new Date(lastScannedAt).getTime() without validating the result. If the API returns an invalid date string for last_scan_at, getTime() returns NaN, causing diffDays to be NaN. Since NaN > SCAN_STALE_DAYS is always false, repositories with invalid date data would incorrectly be marked as not stale, causing the security check to pass when it cannot actually determine when the repo was last scanned. A fail-safe approach would treat unparseable dates as stale.
…ion (#1984) Co-authored-by: Tofik Hasanov <annexcies@gmail.com>
* fix(sync): enhance error logging for Google API response * fix(google-workspace): update domain scope to role management readonly --------- Co-authored-by: Mariano Fuentes <marfuen98@gmail.com>
| return counts.critical + counts.high + counts.medium; | ||
| case 'low': | ||
| default: | ||
| return counts.all; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent severity counting logic between two checks
Medium Severity
The countAtOrAboveSeverity function is implemented differently in the two Aikido check files. In issue-count-threshold.ts, the 'low' case returns counts.all, while in open-security-issues.ts, it returns counts.critical + counts.high + counts.medium + counts.low. If the API's all field ever includes additional unmapped severity categories or differs from the explicit sum, these two security checks will produce inconsistent results for the same severity threshold setting.
Additional Locations (1)
| }, | ||
| }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security checks pass silently when API unavailable
Medium Severity
Both Aikido security checks call ctx.pass() when the API request fails, implementing a "fail-open" pattern. For security-critical checks designed to detect vulnerabilities, this means that API outages, credential issues, or rate limiting will cause the security checks to pass silently rather than flagging the inability to verify. This could mask real security issues during API unavailability.
Additional Locations (1)
|
|
||
| // Log details about updated tasks | ||
| overdueTasks.forEach((task) => { | ||
| tasksToTodo.forEach((task) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing await on notification causes unhandled promise rejection
Medium Severity
The novu.triggerBulk() call is missing await. The previous code used await novu.trigger.broadcast(), but the new code calls novu.triggerBulk() synchronously. This means notification failures will result in unhandled promise rejections, the task will report success before notifications complete, and any errors from the notification service won't be caught by the surrounding try/catch block.
* feat(evidence): add EvidenceJsonView component for JSON evidence display * chore: remove unused react-json-view dependency and related code --------- Co-authored-by: Tofik Hasanov <annexcies@gmail.com>
| const arrayValue = Object.values(data).find((v) => | ||
| Array.isArray(v), | ||
| ) as T[] | undefined; | ||
| items = arrayValue ?? []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First-array heuristic may select wrong data in pagination
Low Severity
The fetchAllPages heuristic uses Object.values(data).find(v => Array.isArray(v)) to find data in wrapped responses. This selects the first array property by definition order, which could pick an unintended array if the response contains multiple arrays (e.g., { errors: [], items: [...] } would select the empty errors array). This causes pagination to terminate early with items.length === 0, silently returning no data. This could cause security checks to incorrectly pass if they find no resources to evaluate.
|
🎉 This PR is included in version 1.74.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This is an automated pull request to release the candidate branch into production, which will trigger a deployment.
It was created by the [Production PR] action.