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

Fix various minor bugs in vulnerability policy evaluation #503

Merged
merged 3 commits into from
Dec 21, 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
Expand Up @@ -201,12 +201,26 @@ private Set<Vulnerability> syncVulnerabilities(final QueryManager qm, final Scan
}

try {
syncedVulns.add(syncVulnerability(qm, vuln, scannerResult.getScanner()));
final Vulnerability syncedVuln = syncVulnerability(qm, vuln, scannerResult.getScanner());

// Detach vulnerabilities from JDO persistence context.
// We do not want to trigger any DB interactions by accessing their fields later.
// Note that even PersistenceManager#detachCopy will load / unload fields based
// on the current FetchPlan. But we just want to keep the data we already have,
// and #makeTransientAll does exactly that.
qm.getPersistenceManager().makeTransient(syncedVuln);

if (vuln.getAliases() != null && !vuln.getAliases().isEmpty()) {
final var syncedAliases = new ArrayList<VulnerabilityAlias>();
for (VulnerabilityAlias alias : vuln.getAliases()) {
qm.synchronizeVulnerabilityAlias(alias);
final VulnerabilityAlias syncedAlias = qm.synchronizeVulnerabilityAlias(alias);
qm.getPersistenceManager().makeTransient(syncedAlias);
syncedAliases.add(syncedAlias);
}
syncedVuln.setAliases(syncedAliases);
}

syncedVulns.add(syncedVuln);
} catch (RuntimeException e) {
// Use a broad catch here, so we can still try to process other
// vulnerabilities, even though processing one of them failed.
Expand All @@ -216,13 +230,6 @@ private Set<Vulnerability> syncVulnerabilities(final QueryManager qm, final Scan
}
}

// Detach vulnerabilities from JDO persistence context.
// We do not want to trigger any DB interactions by accessing their fields.
// Note that even PersistenceManager#detachCopy will load / unload fields based
// on the current FetchPlan. But we just want to keep the data we already have,
// and #makeTransientAll does exactly that.
qm.getPersistenceManager().makeTransientAll(syncedVulns);

return syncedVulns;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import org.projectnessie.cel.CEL;
import org.projectnessie.cel.Env;
import org.projectnessie.cel.Env.AstIssuesTuple;
import org.projectnessie.cel.EnvOption;
import org.projectnessie.cel.Program;
import org.projectnessie.cel.common.CELError;
import org.projectnessie.cel.common.Errors;
Expand Down Expand Up @@ -52,17 +51,17 @@ public enum CacheMode {
private final AbstractCacheManager cacheManager;
private final Env environment;

CelPolicyScriptHost(final AbstractCacheManager cacheManager, final List<EnvOption> envOptions) {
public CelPolicyScriptHost(final AbstractCacheManager cacheManager, final CelPolicyType policyType) {
this.locks = Striped.lock(128);
this.cacheManager = cacheManager;
this.environment = Env.newCustomEnv(
ProtoTypeRegistry.newRegistry(),
envOptions
policyType.envOptions()
);
}

public static synchronized CelPolicyScriptHost getInstance(final CelPolicyType policyType) {
return INSTANCES.computeIfAbsent(policyType, ignored -> new CelPolicyScriptHost(CacheManager.getInstance(), policyType.envOptions()));
return INSTANCES.computeIfAbsent(policyType, ignored -> new CelPolicyScriptHost(CacheManager.getInstance(), policyType));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,14 @@ public class CelVulnerabilityPolicyEvaluator implements VulnerabilityPolicyEvalu
private final CelPolicyScriptHost scriptHost;
private final AbstractCacheManager cacheManager;

@SuppressWarnings("unused") // Called by ServiceLoader
public CelVulnerabilityPolicyEvaluator() {
this(ServiceLoader.load(VulnerabilityPolicyProvider.class).findFirst().orElseThrow(),
CelPolicyScriptHost.getInstance(CelPolicyType.VULNERABILITY), CacheManager.getInstance());
}

CelVulnerabilityPolicyEvaluator(final VulnerabilityPolicyProvider policyProvider,
final CelPolicyScriptHost scriptHost, final AbstractCacheManager cacheManager) {
public CelVulnerabilityPolicyEvaluator(final VulnerabilityPolicyProvider policyProvider,
final CelPolicyScriptHost scriptHost, final AbstractCacheManager cacheManager) {
this.policyProvider = policyProvider;
this.scriptHost = scriptHost;
this.cacheManager = cacheManager;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ private static Set<String> determineFieldsToLoad(final Descriptor typeDescriptor
if (fieldDescriptor.isRepeated() && typeInstance.getRepeatedFieldCount(fieldDescriptor) == 0) {
// There's no way differentiate between repeated fields being not set or just empty.
fieldsToLoad.add(fieldName);
} else if (!typeInstance.hasField(fieldDescriptor)) {
} else if (!fieldDescriptor.isRepeated() && !typeInstance.hasField(fieldDescriptor)) {
fieldsToLoad.add(fieldName);
}
}
Expand Down
13 changes: 13 additions & 0 deletions src/test/java/org/dependencytrack/TestCacheManager.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package org.dependencytrack;

import alpine.server.cache.AbstractCacheManager;

import java.util.concurrent.TimeUnit;

public class TestCacheManager extends AbstractCacheManager {

public TestCacheManager(final long expiresAfter, final TimeUnit timeUnit, final long maxSize) {
super(expiresAfter, timeUnit, maxSize);
}

}
Loading