Ecliipse Plugin | Feature | Vuln Detection - Setting State - MCP - Problem Winodw - #263
Conversation
- Remove McpInstallService from PreferencesPage (common-lib) - Create AuthenticationListener in devassist-lib configuration - Register listener in McpInstallService static block - Create IProjectLifecycleListener interface in common-lib - ProjectLifecycleListener implements interface - Update PluginStartup.getProjectListener() to return interface - MCP auto-install now triggered by authentication event (devassist-lib) - Workspace scan triggered after login via interface Architecture: common-lib has no devassist imports, clean separation.
- Create IAuthenticationSuccessHandler interface in common-lib - Move welcome dialog logic to AuthenticationSuccessHandler in devassist-lib - PreferencesPage delegates to handler via Preferences registry - Removes WelcomeDialog import from common-lib PreferencesPage - Handlers registered in McpInstallService static block Architecture: common-lib has NO devassist imports, clean separation.
- Create ISettingsChangeNotifier interface in common-lib - Create SettingsChangeNotifier implementation in main plugin - Register notifier in PluginStartup static block - Remove PluginStartup and PluginUtils imports from common-lib PreferencesPage - Use notifier instead of direct event broker calls Architecture: common-lib has NO main plugin imports, clean separation.
devassist-lib should depend on common-lib for JAR access, not duplicate them. - Remove lib/ references from devassist-lib MANIFEST.MF and build.properties - Remove lib/ references from devassist-lib .classpath - devassist-lib Require-Bundle: common-lib provides JAR access - Revert .gitignore to only track main plugin lib/
Original alert (resolved)Security Policy Alert: Actions Policy ViolationThis workflow run has been blocked by StepSecurity's actions policy. Disallowed Actions:
To fix this issue, please modify the workflow to use only allowed actions. Contact your organization administrator to request changes to the allowed actions list if needed. For more information, see StepSecurity's Actions Policy documentation. |
| return IMarker.SEVERITY_WARNING; | ||
| } | ||
|
|
||
| switch (severity.toLowerCase()) { |
There was a problem hiding this comment.
Bug: No "Malicious" case in severity-to-marker mapping downgrades the most severe findings to Warning
SeverityLevel.MALICIOUS is defined as strictly more severe than CRITICAL, and ProblemDecorator already special-cases it — but calculateMarkerSeverity()'s switch has no "malicious" case and falls through to IMarker.SEVERITY_WARNING, understating the single most dangerous finding class (e.g. malicious packages/secrets).
Suggested fix: Add an explicit "malicious" → SEVERITY_ERROR case.
Evidence: MarkerIssueMapper.java:156-172; SeverityLevel.java:8; ProblemDecorator.java:205-206.
| return new Object[0]; | ||
| } | ||
|
|
||
| private Image getFileIcon(String fileName) { |
There was a problem hiding this comment.
New native Image created per file node on every tree refresh, never disposed
getElements() calls imageDescriptor.createImage() fresh for every file on every setInput() call; CxFindingsView.refreshTreeWithFilter() calls setInput() repeatedly (including from an async real-time-scan trigger), and dispose() is a no-op. SWT Images are OS handles, not GC'd — this leaks one handle per distinct file per refresh, eventually causing "no more handles" crashes in long-running sessions given how often real-time scanning refreshes the tree.
Suggested fix: Cache file-type icons by extension (like SeverityImageComposer's cache) and dispose superseded ones in dispose().
Evidence: FindingsContentProvider.java:36,58,108-111; CxFindingsView.java:1417,1422,1514.
| } | ||
|
|
||
| @Override | ||
| public Object getParent(Object element) { |
There was a problem hiding this comment.
getParent() always returns null, breaking the ITreeContentProvider contract implied by getChildren()/hasChildren()
A real two-level hierarchy exists, but getParent() unconditionally returns null ("would need to track in the model"). Any future targeted-viewer operation (reveal, update) on a leaf will fail to locate the tree item. Latent today because the view always does full setInput() refreshes.
Suggested fix: Track the owning FileNodeLabel on each child, or maintain a child→parent map.
Evidence: FindingsContentProvider.java:70-95,81-86.
| @@ -0,0 +1,106 @@ | |||
| //package com.checkmarx.eclipse.devassist.ui.findings.realtime; | |||
There was a problem hiding this comment.
(CheckmarxDocumentListener.java, CheckmarxEditorListener.java, RealTimeScanJob.java)
Entire package is a ~750-line, fully-unreferenced duplicate of the live real-time-scan implementation
devassist.ui.findings.realtime duplicates devassist.backend.listener (same class names, same purpose) but is imported by nothing (confirmed by grep), not exported in MANIFEST.MF, and every line of all three files is literally commented out including the package declaration itself. The canonical, wired implementation is backend.listener (held via a static strong reference in PluginStartup). Shipping ~750 lines of orphaned, plausible-looking duplicate source risks a future maintainer "fixing" the wrong, disconnected copy.
Suggested fix: Delete the three files under ui/findings/realtime/ entirely.
Evidence: grep for the package returns only its own files; every line begins with //; PluginStartup.java:12,48 imports the backend.listener version instead.
| * - IAC: Represents scanning for Infrastructure as Code issues and misconfigurations. | ||
| * - ASCA: Represents scanning for Application Security Code Analysis. | ||
| */ | ||
| public enum ScanEngine { |
There was a problem hiding this comment.
duplicate of model/ScanEngine.java
utils.ScanEngine (used only by BaseScannerCommand/AscaScannerService) and model.ScanEngine (used by everything else — adaptors, ProblemDecorator, markers, remediation) are structurally different types for the same concept. BaseScannerCommand.getScannerType() bridges them via ScanEngine.valueOf(config.getEngineName().toUpperCase()) — a string round-trip with no compiler-enforced link. A future rename or new engine applied to only one enum breaks the bridge at runtime (IllegalArgumentException), not compile time.
Suggested fix: Consolidate to model.ScanEngine (the more complete, more widely used type) and delete utils.ScanEngine.
Evidence: utils/ScanEngine.java:14-21 vs model/ScanEngine.java:6-11; BaseScannerCommand.java:4,87-88; AscaScannerService.java:9,45.
a73ef2b to
66ad330
Compare
0e10be5
into
feature/devassist_integration
2 participants
By submitting a PR to this repository, you agree to the terms within the Checkmarx Code of Conduct. Please see the contributing guidelines for how to create and submit a high-quality PR for this repo.
Description
References
Testing
Checklist