Skip to content
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

JAS scanners - Remove timeout & Enhance progress bar and include additional logging #437

Merged
merged 5 commits into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.jfrog.ide.idea.scan;

import com.jfrog.ide.common.configuration.ServerConfig;
import com.jfrog.ide.common.log.ProgressIndicator;
import com.jfrog.ide.common.nodes.ApplicableIssueNode;
import com.jfrog.ide.common.nodes.FileTreeNode;
import com.jfrog.ide.common.nodes.VulnerabilityNode;
Expand Down Expand Up @@ -30,8 +30,8 @@ public ApplicabilityScannerExecutor(Log log) {
supportedPackageTypes = SUPPORTED_PACKAGE_TYPES;
}

public List<JFrogSecurityWarning> execute(ScanConfig.Builder inputFileBuilder, Runnable checkCanceled) throws IOException, InterruptedException {
return super.execute(inputFileBuilder, SCANNER_ARGS, checkCanceled);
public List<JFrogSecurityWarning> execute(ScanConfig.Builder inputFileBuilder, Runnable checkCanceled, ProgressIndicator indicator) throws IOException, InterruptedException {
return super.execute(inputFileBuilder, SCANNER_ARGS, checkCanceled, indicator);
}

@Override
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/com/jfrog/ide/idea/scan/IACScannerExecutor.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.jfrog.ide.idea.scan;

import com.jfrog.ide.common.configuration.ServerConfig;
import com.jfrog.ide.common.log.ProgressIndicator;
import com.jfrog.ide.common.nodes.FileIssueNode;
import com.jfrog.ide.common.nodes.FileTreeNode;
import com.jfrog.ide.common.nodes.subentities.SourceCodeScanType;
Expand All @@ -26,8 +26,8 @@ public IACScannerExecutor(Log log) {
super(SourceCodeScanType.IAC, log);
}

public List<JFrogSecurityWarning> execute(ScanConfig.Builder inputFileBuilder, Runnable checkCanceled) throws IOException, InterruptedException {
return super.execute(inputFileBuilder, SCANNER_ARGS, checkCanceled);
public List<JFrogSecurityWarning> execute(ScanConfig.Builder inputFileBuilder, Runnable checkCanceled, ProgressIndicator indicator) throws IOException, InterruptedException {
return super.execute(inputFileBuilder, SCANNER_ARGS, checkCanceled, indicator);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.jfrog.ide.idea.scan;

import com.jfrog.ide.common.configuration.ServerConfig;
import com.jfrog.ide.common.log.ProgressIndicator;
import com.jfrog.ide.common.nodes.FileIssueNode;
import com.jfrog.ide.common.nodes.FileTreeNode;
import com.jfrog.ide.common.nodes.SastIssueNode;
Expand Down Expand Up @@ -31,8 +31,8 @@ public SastScannerExecutor(Log log) {
super(SourceCodeScanType.SAST, log);
}

public List<JFrogSecurityWarning> execute(ScanConfig.Builder inputFileBuilder, Runnable checkCanceled) throws IOException, InterruptedException {
return super.execute(inputFileBuilder, SCANNER_ARGS, checkCanceled, RUN_WITH_NEW_CONFIG_FILE);
public List<JFrogSecurityWarning> execute(ScanConfig.Builder inputFileBuilder, Runnable checkCanceled, ProgressIndicator indicator) throws IOException, InterruptedException {
return super.execute(inputFileBuilder, SCANNER_ARGS, checkCanceled, RUN_WITH_NEW_CONFIG_FILE, indicator);
}

@Override
Expand Down
38 changes: 28 additions & 10 deletions src/main/java/com/jfrog/ide/idea/scan/ScanBinaryExecutor.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,23 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.intellij.util.EnvironmentUtil;
import com.jfrog.ide.common.configuration.ServerConfig;
import com.jfrog.ide.common.log.ProgressIndicator;
import com.jfrog.ide.common.nodes.FileTreeNode;
import com.jfrog.ide.common.nodes.subentities.SourceCodeScanType;
import com.jfrog.ide.idea.configuration.GlobalSettings;
import com.jfrog.ide.idea.configuration.ServerConfigImpl;
import com.jfrog.ide.idea.inspections.JFrogSecurityWarning;
import com.jfrog.ide.idea.log.Logger;
import com.jfrog.ide.idea.scan.data.*;
import com.jfrog.ide.idea.scan.data.Message;
import com.jfrog.ide.idea.scan.data.NewScanConfig;
import com.jfrog.ide.idea.scan.data.NewScansConfig;
import com.jfrog.ide.idea.scan.data.Output;
import com.jfrog.ide.idea.scan.data.PackageManagerType;
import com.jfrog.ide.idea.scan.data.Rule;
import com.jfrog.ide.idea.scan.data.Run;
import com.jfrog.ide.idea.scan.data.SarifResult;
import com.jfrog.ide.idea.scan.data.ScanConfig;
import com.jfrog.ide.idea.scan.data.ScansConfig;
import com.jfrog.xray.client.Xray;
import com.jfrog.xray.client.services.entitlements.Feature;
import lombok.Getter;
Expand All @@ -36,7 +46,12 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.time.LocalDateTime;
import java.util.*;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.TimeUnit;

import static com.jfrog.ide.common.utils.ArtifactoryConnectionUtils.createAnonymousAccessArtifactoryManagerBuilder;
Expand All @@ -53,7 +68,6 @@
*/
public abstract class ScanBinaryExecutor {
public static final Path BINARIES_DIR = HOME_PATH.resolve("dependencies").resolve("jfrog-security");
private static final long MAX_EXECUTION_MINUTES = 10;
private static final int UPDATE_INTERVAL = 1;
private static final int USER_NOT_ENTITLED = 31;
private static final int NOT_SUPPORTED = 13;
Expand Down Expand Up @@ -121,13 +135,13 @@ String getBinaryDownloadURL(String externalResourcesRepo) {

abstract Feature getScannerFeatureName();

abstract List<JFrogSecurityWarning> execute(ScanConfig.Builder inputFileBuilder, Runnable checkCanceled) throws IOException, InterruptedException, URISyntaxException;
abstract List<JFrogSecurityWarning> execute(ScanConfig.Builder inputFileBuilder, Runnable checkCanceled, ProgressIndicator indicator) throws IOException, InterruptedException, URISyntaxException;

protected List<JFrogSecurityWarning> execute(ScanConfig.Builder inputFileBuilder, List<String> args, Runnable checkCanceled) throws IOException, InterruptedException {
return execute(inputFileBuilder, args, checkCanceled, false);
protected List<JFrogSecurityWarning> execute(ScanConfig.Builder inputFileBuilder, List<String> args, Runnable checkCanceled, ProgressIndicator indicator) throws IOException, InterruptedException {
return execute(inputFileBuilder, args, checkCanceled, false, indicator);
}

protected List<JFrogSecurityWarning> execute(ScanConfig.Builder inputFileBuilder, List<String> args, Runnable checkCanceled, boolean newConfigFormat) throws IOException, InterruptedException {
protected List<JFrogSecurityWarning> execute(ScanConfig.Builder inputFileBuilder, List<String> args, Runnable checkCanceled, boolean newConfigFormat, ProgressIndicator indicator) throws IOException, InterruptedException {
if (!shouldExecute()) {
return List.of();
}
Expand All @@ -151,24 +165,28 @@ protected List<JFrogSecurityWarning> execute(ScanConfig.Builder inputFileBuilder
Logger log = Logger.getInstance();
// The following logging is done outside the commandExecutor because the commandExecutor log level is set to INFO.
// As it is an internal binary execution, the message should be printed for DEBUG use only.
log.debug(String.format("Executing command: %s %s", binaryTargetPath.toString(), join(" ", args)));
indicator.setText(String.format("Running %s scan at %s", scanType.toString().toLowerCase(), String.join(" ", inputParams.getRoots())));
String cmd = String.format("%s %s", binaryTargetPath.toString(), join(" ", args));
log.info(String.format("Executing JAS scanner %s with config: %s", cmd, inputParams));
CommandExecutor commandExecutor = new CommandExecutor(binaryTargetPath.toString(), createEnvWithCredentials());
CommandResults commandResults = commandExecutor.exeCommand(binaryTargetPath.toFile().getParentFile(), args,
null, new NullLog(), MAX_EXECUTION_MINUTES, TimeUnit.MINUTES);
null, new NullLog(), Long.MAX_VALUE, TimeUnit.MINUTES);

checkCanceled.run();

if (commandResults.isOk()) {
log.info(String.format("Finished successfully to run command: %s", cmd));
log.debug(commandResults.getRes());
return parseOutputSarif(outputFilePath);
}
log.info(String.format("Failed to run command: %s", cmd));
switch (commandResults.getExitValue()) {
case USER_NOT_ENTITLED -> {
log.debug("User not entitled for advance security scan");
return List.of();
}
case NOT_SUPPORTED -> {
log.debug(String.format("Scanner %s is not supported in the current Analyzer Manager version.", scanType));
log.info(String.format("Scanner %s is not supported in the current Analyzer Manager version.", scanType));
return List.of();
}
default -> {
Expand Down
6 changes: 2 additions & 4 deletions src/main/java/com/jfrog/ide/idea/scan/ScanManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,12 @@
import static com.jfrog.ide.common.utils.XrayConnectionUtils.createXrayClientBuilder;

public class ScanManager {
private final int SCAN_TIMEOUT_MINUTES = 60;
private final Project project;
private final ScannerFactory factory;
private final SourceCodeScannerManager sourceCodeScannerManager;
private Map<Integer, ScannerBase> scanners = Maps.newHashMap();
private ExecutorService executor;


private ScanManager(@NotNull Project project) {
this.project = project;
factory = new ScannerFactory(project);
Expand Down Expand Up @@ -97,8 +95,8 @@ public void startScan() {
scanner.asyncScanAndUpdateResults();
}
executor.shutdown();
if (!executor.awaitTermination(SCAN_TIMEOUT_MINUTES, TimeUnit.MINUTES)) {
logError(Logger.getInstance(), "Scan timeout of " + SCAN_TIMEOUT_MINUTES + " minutes elapsed. The scan is being canceled.", true);
if (!executor.awaitTermination(Long.MAX_VALUE, TimeUnit.MINUTES)) {
logError(Logger.getInstance(), "Scan timeout elapsed. The scan is being canceled.", true);
}
// Cache tree only if no errors occurred during scan.
if (scanners.values().stream().anyMatch(ScannerBase::isScanErrorOccurred)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.jfrog.ide.idea.scan;

import com.jfrog.ide.common.configuration.ServerConfig;
import com.jfrog.ide.common.log.ProgressIndicator;
import com.jfrog.ide.common.nodes.FileIssueNode;
import com.jfrog.ide.common.nodes.FileTreeNode;
import com.jfrog.ide.common.nodes.subentities.SourceCodeScanType;
Expand All @@ -26,8 +26,8 @@ public SecretsScannerExecutor(Log log) {
super(SourceCodeScanType.SECRETS, log);
}

public List<JFrogSecurityWarning> execute(ScanConfig.Builder inputFileBuilder, Runnable checkCanceled) throws IOException, InterruptedException {
return super.execute(inputFileBuilder, SCANNER_ARGS, checkCanceled);
public List<JFrogSecurityWarning> execute(ScanConfig.Builder inputFileBuilder, Runnable checkCanceled, ProgressIndicator indicator) throws IOException, InterruptedException {
return super.execute(inputFileBuilder, SCANNER_ARGS, checkCanceled, indicator);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,14 @@
import java.net.URISyntaxException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.*;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.atomic.AtomicBoolean;
Expand All @@ -41,7 +48,9 @@
import static com.jfrog.ide.common.utils.Utils.createYAMLMapper;
import static com.jfrog.ide.idea.scan.ScannerBase.createRunnable;
import static com.jfrog.ide.idea.scan.data.applications.JFrogApplicationsConfig.createApplicationConfigWithDefaultModule;
import static com.jfrog.ide.idea.ui.configuration.ConfigVerificationUtils.*;
import static com.jfrog.ide.idea.ui.configuration.ConfigVerificationUtils.EXCLUSIONS_PREFIX;
import static com.jfrog.ide.idea.ui.configuration.ConfigVerificationUtils.EXCLUSIONS_REGEX_PATTERN;
import static com.jfrog.ide.idea.ui.configuration.ConfigVerificationUtils.EXCLUSIONS_SUFFIX;
import static com.jfrog.ide.idea.utils.Utils.getProjectBasePath;
import static org.apache.commons.lang3.StringUtils.defaultIfEmpty;

Expand Down Expand Up @@ -90,7 +99,7 @@ public List<FileTreeNode> applicabilityScan(ProgressIndicator indicator, Collect
Set<String> directIssuesCVEs = issuesMap.keySet();
// If no direct dependencies with issues are found by Xray, the applicability scan is irrelevant.
if (!directIssuesCVEs.isEmpty()) {
List<JFrogSecurityWarning> applicabilityResults = applicability.execute(createBasicScannerInput().cves(List.copyOf(directIssuesCVEs)), checkCanceled);
List<JFrogSecurityWarning> applicabilityResults = applicability.execute(createBasicScannerInput().cves(List.copyOf(directIssuesCVEs)), checkCanceled, indicator);
scanResults.addAll(applicabilityResults);
}
}
Expand Down Expand Up @@ -183,7 +192,7 @@ private void scan(ModuleConfig moduleConfig, ProgressIndicator indicator, Runnab
}
}
try {
List<JFrogSecurityWarning> scanResults = scanner.execute(createBasicScannerInput(moduleConfig, scannerConfig), checkCanceled);
List<JFrogSecurityWarning> scanResults = scanner.execute(createBasicScannerInput(moduleConfig, scannerConfig), checkCanceled, indicator);
addSourceCodeScanResults(scanner.createSpecificFileIssueNodes(scanResults));
} catch (IOException | URISyntaxException | InterruptedException e) {
logError(log, "", e, true);
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/com/jfrog/ide/idea/scan/data/ScanConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
import com.fasterxml.jackson.annotation.JsonProperty;
import com.jfrog.ide.common.nodes.subentities.SourceCodeScanType;
import lombok.Getter;
import lombok.ToString;

import java.util.ArrayList;
import java.util.List;

@Getter
@ToString
public class ScanConfig {
@JsonProperty("type")
private SourceCodeScanType scanType;
Expand Down Expand Up @@ -95,7 +97,6 @@ public void setSkippedFolders(List<String> skippedFolders) {
this.skippedFolders = skippedFolders;
}


public static class Builder {
private SourceCodeScanType scanType;
private String language;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.jfrog.ide.idea.integration;

import com.jfrog.ide.common.log.ProgressIndicator;
import com.jfrog.ide.common.nodes.subentities.SourceCodeScanType;
import com.jfrog.ide.idea.inspections.JFrogSecurityWarning;
import com.jfrog.ide.idea.log.Logger;
Expand All @@ -9,6 +10,8 @@
import java.io.IOException;
import java.util.List;

import static org.mockito.Mockito.mock;

public class ApplicabilityScannerIntegrationTests extends BaseIntegrationTest {
private ApplicabilityScannerExecutor scanner;
private final static String TEST_PROJECT_PREFIX = "sourceCode/testProjects/";
Expand All @@ -22,7 +25,8 @@ protected void setUp() throws Exception {
public void testApplicabilityScannerJsProjectNotApplicable() throws IOException, InterruptedException {
String testProjectRoot = createTempProjectDir("npm");
ScanConfig.Builder input = new ScanConfig.Builder().roots(List.of(testProjectRoot)).cves(List.of("CVE-2021-3918", "CVE-2021-3807"));
List<JFrogSecurityWarning> results = scanner.execute(input, this::dummyCheckCanceled);
ProgressIndicator indicator = mock(ProgressIndicator.class);
List<JFrogSecurityWarning> results = scanner.execute(input, this::dummyCheckCanceled, indicator);
assertEquals(2, results.size());
// Expect all issues to be not applicable to this test project
assertFalse(results.stream().anyMatch(JFrogSecurityWarning::isApplicable));
Expand All @@ -31,7 +35,8 @@ public void testApplicabilityScannerJsProjectNotApplicable() throws IOException,
public void testApplicabilityScannerJsProject() throws IOException, InterruptedException {
String testProjectRoot = createTempProjectDir("npm");
ScanConfig.Builder input = new ScanConfig.Builder().roots(List.of(testProjectRoot)).cves(List.of("CVE-2022-25878"));
List<JFrogSecurityWarning> results = scanner.execute(input, this::dummyCheckCanceled);
ProgressIndicator indicator = mock(ProgressIndicator.class);
List<JFrogSecurityWarning> results = scanner.execute(input, this::dummyCheckCanceled, indicator);
assertEquals(2, results.size());
// Expect all issues to be applicable.
assertTrue(results.stream().allMatch(JFrogSecurityWarning::isApplicable));
Expand All @@ -49,7 +54,8 @@ public void testApplicabilityScannerJsProject() throws IOException, InterruptedE
public void testApplicabilityScannerPythonProjectNotApplicable() throws IOException, InterruptedException {
String testProjectRoot = createTempProjectDir("python");
ScanConfig.Builder input = new ScanConfig.Builder().roots(List.of(testProjectRoot)).cves(List.of("CVE-2021-3918", "CVE-2019-15605"));
List<JFrogSecurityWarning> results = scanner.execute(input, this::dummyCheckCanceled);
ProgressIndicator indicator = mock(ProgressIndicator.class);
List<JFrogSecurityWarning> results = scanner.execute(input, this::dummyCheckCanceled, indicator);
assertEquals(2, results.size());
// Expect all issues to be not applicable to this test project
assertFalse(results.stream().anyMatch(JFrogSecurityWarning::isApplicable));
Expand All @@ -58,7 +64,8 @@ public void testApplicabilityScannerPythonProjectNotApplicable() throws IOExcept
public void testApplicabilityScannerPythonProject() throws IOException, InterruptedException {
String testProjectRoot = createTempProjectDir("python");
ScanConfig.Builder input = new ScanConfig.Builder().roots(List.of(testProjectRoot)).cves(List.of("CVE-2019-20907"));
List<JFrogSecurityWarning> results = scanner.execute(input, this::dummyCheckCanceled);
ProgressIndicator indicator = mock(ProgressIndicator.class);
List<JFrogSecurityWarning> results = scanner.execute(input, this::dummyCheckCanceled, indicator);
assertEquals(1, results.size());
// Expect specific indications
assertTrue(results.get(0).isApplicable());
Expand All @@ -74,7 +81,8 @@ public void testApplicabilityScannerPythonProject() throws IOException, Interrup
public void testApplicabilityScannerJavaProject() throws IOException, InterruptedException {
String testProjectRoot = createTempProjectDir("maven");
ScanConfig.Builder input = new ScanConfig.Builder().roots(List.of(testProjectRoot)).cves(List.of("CVE-2013-7285"));
List<JFrogSecurityWarning> results = scanner.execute(input, this::dummyCheckCanceled);
ProgressIndicator indicator = mock(ProgressIndicator.class);
List<JFrogSecurityWarning> results = scanner.execute(input, this::dummyCheckCanceled, indicator);
assertEquals(2, results.size());
// Expect specific indications
assertTrue(results.get(0).isApplicable());
Expand Down
Loading
Loading