Fix trivy detected vulnerabilities#2372
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR bumps pinned dependency versions across the monorepo (js-yaml, hono, dompurify, tmp, protobufjs, webpack-dev-server, ChangesDependency Version Bumps
MI E2E Test Reliability Fixes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
7a89090 to
8366f9e
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
workspaces/mi/mi-extension/src/test/e2e-playwright-tests/components/ProjectExplorer.ts (1)
62-62: 💤 Low valueConsider investigating the root cause before increasing timeout.
Doubling the default wait timeout from 30s to 60s may mask underlying timing or performance issues in the test environment. Before applying this change, consider:
- Are there specific conditions causing the 30s timeout to be insufficient?
- Could the project explorer initialization be optimized instead?
- Will this significantly impact CI pipeline duration when tests fail?
If this timeout increase is necessary due to legitimate slow initialization in CI environments, consider adding a comment explaining the rationale.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-extension/src/test/e2e-playwright-tests/components/ProjectExplorer.ts` at line 62, The waitTimeout assignment in the ProjectExplorer test component has been increased from 30000ms to 60000ms, which may mask underlying issues. Investigate the root cause of why the 30-second timeout is insufficient by checking for performance bottlenecks or initialization delays in the project explorer. If the timeout increase to 60000ms is truly necessary after investigation, add a clear comment explaining the specific reason for this increase and any CI environment constraints that justify it. If the root cause can be resolved through optimization, revert the timeout back to 30000ms.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@workspaces/mi/mi-extension/src/test/e2e-playwright-tests/components/ProjectExplorer.ts`:
- Line 62: The waitTimeout assignment in the ProjectExplorer test component has
been increased from 30000ms to 60000ms, which may mask underlying issues.
Investigate the root cause of why the 30-second timeout is insufficient by
checking for performance bottlenecks or initialization delays in the project
explorer. If the timeout increase to 60000ms is truly necessary after
investigation, add a clear comment explaining the specific reason for this
increase and any CI environment constraints that justify it. If the root cause
can be resolved through optimization, revert the timeout back to 30000ms.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cf3c380b-a7da-46ac-85f2-4da9d8cc86ba
⛔ Files ignored due to path filters (1)
common/config/rush/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
common/config/rush/.pnpmfile.cjscommon/config/rush/pnpm-config.jsonpackage.jsonworkspaces/api-designer/api-designer-extension/package.jsonworkspaces/api-designer/api-designer-visualizer/package.jsonworkspaces/ballerina/ballerina-extension/package.jsonworkspaces/ballerina/ballerina-low-code-diagram/package.jsonworkspaces/ballerina/ballerina-side-panel/package.jsonworkspaces/ballerina/ballerina-visualizer/package.jsonworkspaces/ballerina/bi-diagram/package.jsonworkspaces/ballerina/component-diagram/package.jsonworkspaces/ballerina/graphql/package.jsonworkspaces/ballerina/persist-layer-diagram/package.jsonworkspaces/ballerina/sequence-diagram/package.jsonworkspaces/ballerina/trace-visualizer/package.jsonworkspaces/ballerina/type-diagram/package.jsonworkspaces/choreo/choreo-extension/package.jsonworkspaces/choreo/choreo-webviews/package.jsonworkspaces/mi/mi-diagram/package.jsonworkspaces/mi/mi-extension/package.jsonworkspaces/mi/mi-extension/src/test/e2e-playwright-tests/components/ProjectExplorer.tsworkspaces/mi/mi-visualizer/package.jsonworkspaces/wso2-platform/wso2-platform-extension/package.jsonworkspaces/wso2-platform/wso2-platform-webviews/package.json
✅ Files skipped from review due to trivial changes (8)
- workspaces/mi/mi-diagram/package.json
- workspaces/ballerina/component-diagram/package.json
- workspaces/ballerina/trace-visualizer/package.json
- workspaces/ballerina/sequence-diagram/package.json
- workspaces/ballerina/bi-diagram/package.json
- workspaces/ballerina/persist-layer-diagram/package.json
- workspaces/mi/mi-visualizer/package.json
- workspaces/ballerina/ballerina-low-code-diagram/package.json
🚧 Files skipped from review as they are similar to previous changes (14)
- workspaces/ballerina/type-diagram/package.json
- common/config/rush/pnpm-config.json
- workspaces/choreo/choreo-extension/package.json
- workspaces/ballerina/ballerina-side-panel/package.json
- workspaces/wso2-platform/wso2-platform-extension/package.json
- workspaces/mi/mi-extension/package.json
- workspaces/api-designer/api-designer-visualizer/package.json
- workspaces/choreo/choreo-webviews/package.json
- workspaces/wso2-platform/wso2-platform-webviews/package.json
- package.json
- workspaces/ballerina/ballerina-extension/package.json
- workspaces/ballerina/ballerina-visualizer/package.json
- workspaces/api-designer/api-designer-extension/package.json
- common/config/rush/.pnpmfile.cjs
da1c758 to
bca5f03
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.github/workflows/build.yml (2)
245-245: ⚡ Quick winConsider upgrading
actions/checkoutfrom v2 to v4.v2 is quite old and v4 includes security fixes, improved performance, and better sparse checkout support. This is especially relevant in a security-focused PR.
- - uses: actions/checkout@v2 + - uses: actions/checkout@v4🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build.yml at line 245, Update the actions/checkout action version from v2 to v4 in the GitHub workflow file. Locate the line where uses: actions/checkout@v2 is specified and change it to uses: actions/checkout@v4. This will ensure the workflow uses the latest version with security fixes and performance improvements.
446-458: No integrity verification for downloaded VSIX.The step pins to a specific version (good), but downloads from the VS Marketplace without verifying a checksum or signature. While the download is from an official source with a pinned version, this presents a supply chain risk if the endpoint or CDN were compromised.
Mitigation options (in order of practicality):
- Compute and commit an expected SHA256 hash, then verify after download
- Vendor the VSIX in the repository
- Accept the risk given the official Marketplace source, pinned version, and test-only context
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build.yml around lines 446 - 458, The VSIX file download step lacks integrity verification, creating a supply chain risk. Add checksum verification to the VSIX download workflow step by computing the expected SHA256 hash of the pinned VSIX version, committing this hash to the repository, and then verifying the downloaded file's hash matches the expected value before proceeding with the installation. This can be implemented by storing the expected hash in the workflow or a configuration file, then using a verification step (such as a shell command with sha256sum) to validate the download integrity after fetching from the VS Marketplace.workspaces/mi/mi-extension/src/test/e2e-playwright-tests/components/ProjectExplorer.ts (1)
65-66: ⚡ Quick winReplace fixed sleeps with condition-based waits to reduce E2E flakiness.
The hardcoded
waitForTimeout(500)andwaitForTimeout(2000)can still be timing-sensitive across CI loads. Prefer waiting on a concrete explorer-ready condition (e.g., root tree visibility) immediately after focus, instead of fixed delays.Suggested diff
private async focusProjectExplorer() { await new ExtendedPage(this.page).executePaletteCommand('Focus on MI Project Explorer View'); - await this.page.waitForTimeout(500); + await this.explorer.waitFor({ state: "visible", timeout: 5000 }); } @@ } catch (error) { // If project not found, re-open the explorer view and wait again. console.warn(`Project ${projectName} not found, waiting additional time...`); - await this.page.waitForTimeout(2000); await this.focusProjectExplorer(); await projectExplorerRoot.waitFor({ state: 'visible', timeout: waitTimeout }); }Also applies to: 81-83
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-extension/src/test/e2e-playwright-tests/components/ProjectExplorer.ts` around lines 65 - 66, Replace the hardcoded waitForTimeout(500) and waitForTimeout(2000) calls in the ProjectExplorer test file (at lines 65-66 and 81-83) with condition-based waits that check for concrete explorer-ready conditions such as root tree visibility immediately after focus operations. Instead of using fixed delays, wait for a specific element or state to be available (for example, waiting for the root tree element to be visible) to reduce timing-sensitive flakiness in the E2E tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/build.yml:
- Line 245: Update the actions/checkout action version from v2 to v4 in the
GitHub workflow file. Locate the line where uses: actions/checkout@v2 is
specified and change it to uses: actions/checkout@v4. This will ensure the
workflow uses the latest version with security fixes and performance
improvements.
- Around line 446-458: The VSIX file download step lacks integrity verification,
creating a supply chain risk. Add checksum verification to the VSIX download
workflow step by computing the expected SHA256 hash of the pinned VSIX version,
committing this hash to the repository, and then verifying the downloaded file's
hash matches the expected value before proceeding with the installation. This
can be implemented by storing the expected hash in the workflow or a
configuration file, then using a verification step (such as a shell command with
sha256sum) to validate the download integrity after fetching from the VS
Marketplace.
In
`@workspaces/mi/mi-extension/src/test/e2e-playwright-tests/components/ProjectExplorer.ts`:
- Around line 65-66: Replace the hardcoded waitForTimeout(500) and
waitForTimeout(2000) calls in the ProjectExplorer test file (at lines 65-66 and
81-83) with condition-based waits that check for concrete explorer-ready
conditions such as root tree visibility immediately after focus operations.
Instead of using fixed delays, wait for a specific element or state to be
available (for example, waiting for the root tree element to be visible) to
reduce timing-sensitive flakiness in the E2E tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ae59fc28-a014-4a12-81e5-e9675ecab985
📒 Files selected for processing (4)
.github/workflows/build.ymlworkspaces/common-libs/playwright-vscode-tester/src/utils.tsworkspaces/mi/mi-extension/src/test/e2e-playwright-tests/Utils.tsworkspaces/mi/mi-extension/src/test/e2e-playwright-tests/components/ProjectExplorer.ts
50d7e58 to
24cc0e5
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/build.yml (1)
245-245: ⚡ Quick winConsider adding
persist-credentials: falsefor security hardening.Since the Trivy scan job only reads files and doesn't need git credentials after checkout, setting
persist-credentials: falseprevents the credentials from persisting in the runner's git config. This reduces the attack surface if subsequent steps are compromised.- uses: actions/checkout@v2 + with: + persist-credentials: falseNote: The workflow consistently uses
actions/checkout@v2. While v4 is available with improved performance and security features, maintaining consistency with the existing pattern (line 100) is reasonable unless a broader upgrade is planned.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build.yml at line 245, The actions/checkout@v2 step in the workflow does not include the persist-credentials parameter, which allows git credentials to remain in the runner's git config unnecessarily. Add the persist-credentials: false parameter to the actions/checkout@v2 action to prevent credential persistence after checkout, reducing the attack surface for subsequent workflow steps.Source: Linters/SAST tools
workspaces/mi/mi-extension/src/test/e2e-playwright-tests/components/ProjectExplorer.ts (1)
82-94: ⚡ Quick winGate or remove the temporary DOM diagnostic dump in the hot path.
This adds fixed delay and noisy serial logging on every
goToOverview()call; in current tests this method is invoked repeatedly.Suggested refactor
- // --- TEMP DIAGNOSTIC: dump the actual DOM so we can see real aria-labels --- - await this.page.waitForTimeout(1500); // let the view settle - const trees = await this.page.locator('div[role="tree"]').all(); - console.log(`[DIAG] role="tree" count: ${trees.length}`); - for (let i = 0; i < trees.length; i++) { - console.log(`[DIAG] tree[${i}] aria-label=${JSON.stringify(await trees[i].getAttribute('aria-label'))} visible=${await trees[i].isVisible()}`); - } - const items = await this.page.locator('div[role="treeitem"]').all(); - console.log(`[DIAG] role="treeitem" count: ${items.length}`); - for (let i = 0; i < Math.min(items.length, 25); i++) { - console.log(`[DIAG] treeitem[${i}] aria-label=${JSON.stringify(await items[i].getAttribute('aria-label'))} visible=${await items[i].isVisible()}`); - } - // --- END TEMP DIAGNOSTIC --- + if (process.env.MI_E2E_DIAG_PROJECT_EXPLORER === '1') { + await this.page.waitForTimeout(1500); + const trees = await this.page.locator('div[role="tree"]').all(); + console.log(`[DIAG] role="tree" count: ${trees.length}`); + for (let i = 0; i < trees.length; i++) { + console.log( + `[DIAG] tree[${i}] aria-label=${JSON.stringify(await trees[i].getAttribute('aria-label'))} visible=${await trees[i].isVisible()}` + ); + } + const items = await this.page.locator('div[role="treeitem"]').all(); + console.log(`[DIAG] role="treeitem" count: ${items.length}`); + for (let i = 0; i < Math.min(items.length, 25); i++) { + console.log( + `[DIAG] treeitem[${i}] aria-label=${JSON.stringify(await items[i].getAttribute('aria-label'))} visible=${await items[i].isVisible()}` + ); + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/mi/mi-extension/src/test/e2e-playwright-tests/components/ProjectExplorer.ts` around lines 82 - 94, The temporary diagnostic code block in ProjectExplorer.ts that dumps DOM information (marked between TEMP DIAGNOSTIC comments) includes a fixed delay via waitForTimeout and multiple serial console.log statements that execute on every invocation, causing performance and log noise issues. Either remove this entire diagnostic block if it is no longer needed for active debugging, or gate it behind a conditional environment variable or debug flag so it only executes when explicitly enabled for troubleshooting purposes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build.yml:
- Around line 460-464: The condition in the "Set screen resolution in Windows"
step checks for matrix.os == 'windows-latest', but the matrix definition sets
matrix.os to 'windows', not 'windows-latest', causing the condition to never
match and the step to be skipped on Windows runners. Change the if condition
from matrix.os == 'windows-latest' to matrix.os == 'windows' to match the actual
matrix value and align with the pattern used in the runner selection on line
390.
In
`@workspaces/mi/mi-extension/src/test/e2e-playwright-tests/components/ProjectExplorer.ts`:
- Around line 76-77: The timeout budget is being doubled because waitTimeout is
applied both before and after the retry logic, causing the fallback path to wait
up to 2 * waitTimeout. Additionally, the retry mechanism is swallowing all
failures instead of just timeout-related ones. Refactor the retry logic around
the waitTimeout variable to share a single timeout budget across all retry
attempts rather than applying it sequentially, and modify the retry condition to
only retry on timeout-specific failures while allowing other types of errors to
propagate immediately without retry.
---
Nitpick comments:
In @.github/workflows/build.yml:
- Line 245: The actions/checkout@v2 step in the workflow does not include the
persist-credentials parameter, which allows git credentials to remain in the
runner's git config unnecessarily. Add the persist-credentials: false parameter
to the actions/checkout@v2 action to prevent credential persistence after
checkout, reducing the attack surface for subsequent workflow steps.
In
`@workspaces/mi/mi-extension/src/test/e2e-playwright-tests/components/ProjectExplorer.ts`:
- Around line 82-94: The temporary diagnostic code block in ProjectExplorer.ts
that dumps DOM information (marked between TEMP DIAGNOSTIC comments) includes a
fixed delay via waitForTimeout and multiple serial console.log statements that
execute on every invocation, causing performance and log noise issues. Either
remove this entire diagnostic block if it is no longer needed for active
debugging, or gate it behind a conditional environment variable or debug flag so
it only executes when explicitly enabled for troubleshooting purposes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9cfa43d0-553c-48cd-90ae-0e3c8c12be9a
📒 Files selected for processing (4)
.github/workflows/build.ymlworkspaces/common-libs/playwright-vscode-tester/src/utils.tsworkspaces/mi/mi-extension/src/test/e2e-playwright-tests/Utils.tsworkspaces/mi/mi-extension/src/test/e2e-playwright-tests/components/ProjectExplorer.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- workspaces/common-libs/playwright-vscode-tester/src/utils.ts
- workspaces/mi/mi-extension/src/test/e2e-playwright-tests/Utils.ts
24cc0e5 to
a9c1c09
Compare
a9c1c09 to
4c4ad92
Compare
77d4a30 to
ed767cd
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Line 29: The protobufjs dependency override in the root package.json is pinned
to a fixed version (7.6.3) which takes precedence over the conditional
version-aware logic in .pnpmfile.cjs. This prevents the pnpmfile mapping rules
from being applied, creating maintenance risk if workspace packages request
protobufjs@8.x. Update the protobufjs override entry in package.json to use
version-aware override syntax that aligns with the conditional mapping in
.pnpmfile.cjs, allowing it to map version 8.x requests to 8.6.0 while defaulting
to 7.6.3 for other versions, thereby unifying the override policies across both
configuration files.
In
`@workspaces/mi/mi-extension/src/test/e2e-playwright-tests/components/ProjectExplorer.ts`:
- Around line 47-64: Wrap the diagnostic logging code within the catch block of
the findItem method in an inner try/catch statement. The inner try block should
contain all the console.log calls and the loops that iterate over trees and
items arrays calling getAttribute and isVisible methods. In the inner catch
block, log that the diagnostic itself failed (best-effort approach). After the
inner try/catch completes, rethrow the original error e to preserve the actual
root cause of the initial waitFor failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c0bca388-0d78-480c-b947-7667afe04b0f
⛔ Files ignored due to path filters (1)
common/config/rush/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
common/config/rush/.pnpmfile.cjscommon/config/rush/pnpm-config.jsonpackage.jsonworkspaces/api-designer/api-designer-extension/package.jsonworkspaces/api-designer/api-designer-visualizer/package.jsonworkspaces/ballerina/ballerina-extension/package.jsonworkspaces/ballerina/ballerina-low-code-diagram/package.jsonworkspaces/ballerina/ballerina-side-panel/package.jsonworkspaces/ballerina/ballerina-visualizer/package.jsonworkspaces/ballerina/bi-diagram/package.jsonworkspaces/ballerina/component-diagram/package.jsonworkspaces/ballerina/graphql/package.jsonworkspaces/ballerina/persist-layer-diagram/package.jsonworkspaces/ballerina/sequence-diagram/package.jsonworkspaces/ballerina/trace-visualizer/package.jsonworkspaces/ballerina/type-diagram/package.jsonworkspaces/choreo/choreo-extension/package.jsonworkspaces/choreo/choreo-webviews/package.jsonworkspaces/mi/mi-diagram/package.jsonworkspaces/mi/mi-extension/package.jsonworkspaces/mi/mi-extension/src/test/e2e-playwright-tests/components/ArtifactTest/ClassMediator.tsworkspaces/mi/mi-extension/src/test/e2e-playwright-tests/components/ProjectExplorer.tsworkspaces/mi/mi-visualizer/package.jsonworkspaces/wso2-platform/wso2-platform-extension/package.jsonworkspaces/wso2-platform/wso2-platform-webviews/package.json
✅ Files skipped from review due to trivial changes (6)
- workspaces/ballerina/type-diagram/package.json
- workspaces/mi/mi-diagram/package.json
- workspaces/ballerina/component-diagram/package.json
- workspaces/ballerina/persist-layer-diagram/package.json
- workspaces/ballerina/ballerina-low-code-diagram/package.json
- workspaces/ballerina/trace-visualizer/package.json
ed767cd to
5d7c144
Compare
$subject
Summary by CodeRabbit