From d74aabff556d37f6c580d3e605adaed0871672de Mon Sep 17 00:00:00 2001 From: Matt Finkel Date: Wed, 7 Oct 2026 14:02:25 +0100 Subject: [PATCH 1/5] Let policy condition expressions return a violation message Implements ADR 043. A condition expression may return a string instead of a boolean. A non-blank string records a violation with that string as message, a blank string records none. The message is stored on the violation and included in POLICY_VIOLATION and NEW_POLICY_VIOLATIONS_SUMMARY notifications. Signed-off-by: Matt Finkel --- .../model/PolicyViolation.java | 4 +- .../NotificationModelConverter.java | 9 +- .../notification/NotificationSubjectDao.java | 11 +- ...cessScheduledNotificationRuleActivity.java | 3 + .../ScheduledNotificationDao.java | 4 +- .../policy/cel/CelPolicyCompiler.java | 18 +++ .../policy/cel/CelPolicyEngine.java | 18 ++- .../policy/cel/CelPolicyProgram.java | 43 +++++- .../policy/cel/CelPolicyType.java | 33 ++++- .../policy/cel/persistence/CelPolicyDao.java | 20 ++- .../v1/vo/CreatePolicyConditionRequest.java | 6 +- .../v1/vo/UpdatePolicyConditionRequest.java | 6 +- .../NotificationSubjectDaoTest.java | 2 + ...ScheduledNotificationRuleActivityTest.java | 4 +- .../policy/cel/CelPolicyCompilerTest.java | 44 +++++++ .../policy/cel/CelPolicyEngineTest.java | 124 ++++++++++++++++++ .../cel/persistence/CelPolicyDaoTest.java | 68 ++++++++++ .../v1/PolicyConditionResourceTest.java | 50 +++++++ ...610061719__widen_policy_violation_text.sql | 4 + .../api/TestNotificationFactory.java | 2 + .../notification/v1/notification.proto | 4 + .../publishing/email/default-template.peb | 6 + .../mattermost/default-template.peb | 2 +- .../publishing/msteams/default-template.peb | 6 +- .../publishing/slack/default-template.peb | 9 +- .../email/EmailNotificationPublisherTest.java | 1 + .../slack/SlackNotificationPublisherTest.java | 7 + 27 files changed, 476 insertions(+), 32 deletions(-) create mode 100644 migration/src/main/resources/org/dependencytrack/migration/V202610061719__widen_policy_violation_text.sql diff --git a/apiserver/src/main/java/org/dependencytrack/model/PolicyViolation.java b/apiserver/src/main/java/org/dependencytrack/model/PolicyViolation.java index 7a862a2cf2..4021f9a985 100644 --- a/apiserver/src/main/java/org/dependencytrack/model/PolicyViolation.java +++ b/apiserver/src/main/java/org/dependencytrack/model/PolicyViolation.java @@ -105,8 +105,8 @@ public enum Type { private Date timestamp; @Persistent - @Column(name = "TEXT") - @Size(min = 1, max = 255) + @Column(name = "TEXT", jdbcType = "CLOB") + @Size(min = 1, max = 1024) @Pattern( regexp = RegexSequence.Definition.PRINTABLE_CHARS, message = "The text may only contain printable characters") diff --git a/apiserver/src/main/java/org/dependencytrack/notification/NotificationModelConverter.java b/apiserver/src/main/java/org/dependencytrack/notification/NotificationModelConverter.java index a8cc1a298e..a9c13a2ca9 100644 --- a/apiserver/src/main/java/org/dependencytrack/notification/NotificationModelConverter.java +++ b/apiserver/src/main/java/org/dependencytrack/notification/NotificationModelConverter.java @@ -184,12 +184,15 @@ public static PolicyCondition convert(org.dependencytrack.model.PolicyCondition } public static PolicyViolation convert(org.dependencytrack.model.PolicyViolation violation) { - return PolicyViolation.newBuilder() + final PolicyViolation.Builder builder = PolicyViolation.newBuilder() .setUuid(violation.getUuid().toString()) .setCondition(convert(violation.getPolicyCondition())) .setType(violation.getType().name()) - .setTimestamp(Timestamps.fromDate(violation.getTimestamp())) - .build(); + .setTimestamp(Timestamps.fromDate(violation.getTimestamp())); + if (violation.getText() != null) { + builder.setMessage(violation.getText()); + } + return builder.build(); } public static PolicyViolationAnalysis convert(ViolationAnalysis analysis) { diff --git a/apiserver/src/main/java/org/dependencytrack/notification/NotificationSubjectDao.java b/apiserver/src/main/java/org/dependencytrack/notification/NotificationSubjectDao.java index 2fc399beaa..c63edc2532 100644 --- a/apiserver/src/main/java/org/dependencytrack/notification/NotificationSubjectDao.java +++ b/apiserver/src/main/java/org/dependencytrack/notification/NotificationSubjectDao.java @@ -677,6 +677,7 @@ default List getForNewPolicyViolations(Collection SELECT pv."UUID" AS "violationUuid" , pv."TYPE" AS "violationType" , pv."TIMESTAMP" AS "violationTimestamp" + , pv."TEXT" AS "violationMessage" , pc."UUID" AS "conditionUuid" , pc."SUBJECT" AS "conditionSubject" , pc."OPERATOR" AS "conditionOperator" @@ -743,12 +744,16 @@ FROM UNNEST(:violationIds) AS req(violation_id) .setPolicy(policy) .build(); - final PolicyViolation violation = PolicyViolation.newBuilder() + final PolicyViolation.Builder violationBuilder = PolicyViolation.newBuilder() .setUuid(row.getColumn("violationUuid", String.class)) .setType(row.getColumn("violationType", String.class)) .setTimestamp(Timestamps.fromDate(row.getColumn("violationTimestamp", Timestamp.class))) - .setCondition(condition) - .build(); + .setCondition(condition); + final String violationMessage = row.getColumn("violationMessage", String.class); + if (violationMessage != null) { + violationBuilder.setMessage(violationMessage); + } + final PolicyViolation violation = violationBuilder.build(); return PolicyViolationSubject.newBuilder() .setProject(project) diff --git a/apiserver/src/main/java/org/dependencytrack/notification/ProcessScheduledNotificationRuleActivity.java b/apiserver/src/main/java/org/dependencytrack/notification/ProcessScheduledNotificationRuleActivity.java index 5e570fa032..38a18bea29 100644 --- a/apiserver/src/main/java/org/dependencytrack/notification/ProcessScheduledNotificationRuleActivity.java +++ b/apiserver/src/main/java/org/dependencytrack/notification/ProcessScheduledNotificationRuleActivity.java @@ -387,6 +387,9 @@ private void advanceSchedule(NotificationRule rule, Instant lastTriggeredAt) { if (violation.analysisState() != null) { entryBuilder.setAnalysisState(violation.analysisState()); } + if (violation.message() != null) { + entryBuilder.setMessage(violation.message()); + } violationsByProjectId .computeIfAbsent(violation.projectId(), k -> new ArrayList<>()) diff --git a/apiserver/src/main/java/org/dependencytrack/notification/ScheduledNotificationDao.java b/apiserver/src/main/java/org/dependencytrack/notification/ScheduledNotificationDao.java index babe2978fa..86641c3149 100644 --- a/apiserver/src/main/java/org/dependencytrack/notification/ScheduledNotificationDao.java +++ b/apiserver/src/main/java/org/dependencytrack/notification/ScheduledNotificationDao.java @@ -183,7 +183,8 @@ public record NewPolicyViolation( String violationType, Instant timestamp, String analysisState, - boolean suppressed) {} + boolean suppressed, + @Nullable String message) {} public List getNewPolicyViolationsSince( Collection projectIds, Instant sinceTimestamp, Instant beforeTimestamp) { @@ -197,6 +198,7 @@ public List getNewPolicyViolationsSince( , pv."TIMESTAMP" AS timestamp , va."STATE" AS analysis_state , COALESCE(va."SUPPRESSED", FALSE) AS suppressed + , pv."TEXT" AS message FROM "POLICYVIOLATION" pv LEFT JOIN "VIOLATIONANALYSIS" va ON va."POLICYVIOLATION_ID" = pv."ID" diff --git a/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyCompiler.java b/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyCompiler.java index bf0fefaef6..d7abb2409c 100644 --- a/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyCompiler.java +++ b/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyCompiler.java @@ -103,6 +103,7 @@ private CelPolicyProgram compile(String normalizedSrc, String scriptDigest, Cach LOGGER.debug("Compiling expression: %s".formatted(normalizedSrc)); final CelAbstractSyntaxTree ast = policyType.compiler().compile(normalizedSrc).getAst(); + validateResultType(ast); final CelRuntime.Program celProgram; try { @@ -124,6 +125,23 @@ private CelPolicyProgram compile(String normalizedSrc, String scriptDigest, Cach return program; } + private void validateResultType(CelAbstractSyntaxTree ast) throws CelValidationException { + final Set allowedTypes = policyType.resultTypes(); + if (allowedTypes.isEmpty() || allowedTypes.contains(ast.getResultType())) { + return; + } + + final String allowedTypeNames = + allowedTypes.stream().map(CelType::name).sorted().collect(Collectors.joining(" or ")); + throw new CelValidationException( + ast.getSource(), + List.of(CelIssue.formatError( + 1, + 0, + "Expression must return %s, but returns %s" + .formatted(allowedTypeNames, ast.getResultType().name())))); + } + private record AnalysisResult(Map> requirements, Set usedFunctions) {} private static AnalysisResult analyze(CelAbstractSyntaxTree ast) { diff --git a/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyEngine.java b/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyEngine.java index 96de5e6d90..8cc589ad1b 100644 --- a/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyEngine.java +++ b/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyEngine.java @@ -54,6 +54,7 @@ import org.dependencytrack.proto.policy.v1.License; import org.dependencytrack.proto.policy.v1.Project; import org.dependencytrack.proto.policy.v1.Vulnerability; +import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -295,6 +296,9 @@ public void evaluateProject(UUID uuid) throws InterruptedException { record ConditionScript(PolicyCondition condition, CelPolicyProgram script) {} + private record ViolatedCondition( + PolicyCondition condition, @Nullable String message) {} + record PolicyWithScripts(Policy policy, List conditionScripts) {} private List compilePoliciesScripts(List policies) { @@ -369,12 +373,13 @@ private void evaluateComponentAgainstPolicies( Map> violationsByComponentId) { for (final PolicyWithScripts pws : policiesWithScripts) { final Policy policy = pws.policy(); - final var violatedConditions = new ArrayList(); + final var violatedConditions = new ArrayList(); for (final ConditionScript cs : pws.conditionScripts()) { try { - if (cs.script().execute(scriptArgs)) { - violatedConditions.add(cs.condition()); + final CelPolicyProgram.Result result = cs.script().evaluate(scriptArgs); + if (result.matched()) { + violatedConditions.add(new ViolatedCondition(cs.condition(), result.message())); } } catch (CelEvaluationException e) { LOGGER.warn( @@ -393,11 +398,12 @@ private void evaluateComponentAgainstPolicies( }; if (policyViolated) { - for (final PolicyCondition condition : violatedConditions) { + for (final ViolatedCondition violatedCondition : violatedConditions) { final var violation = new PolicyViolation(); - violation.setType(condition.getViolationType()); - violation.setPolicyCondition(condition); + violation.setType(violatedCondition.condition().getViolationType()); + violation.setPolicyCondition(violatedCondition.condition()); violation.setTimestamp(new Date()); + violation.setText(violatedCondition.message()); violationsByComponentId .computeIfAbsent(componentId, _ -> new ArrayList<>()) .add(violation); diff --git a/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyProgram.java b/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyProgram.java index 47251a7804..0e6524d501 100644 --- a/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyProgram.java +++ b/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyProgram.java @@ -21,6 +21,7 @@ import dev.cel.common.types.CelType; import dev.cel.runtime.CelEvaluationException; import dev.cel.runtime.CelRuntime; +import org.jspecify.annotations.Nullable; import java.util.Map; import java.util.Set; @@ -28,6 +29,20 @@ public final class CelPolicyProgram { + /// Maximum length of a violation message produced by an expression. + /// Longer messages are truncated. + static final int MAX_MESSAGE_LENGTH = 1024; + + /// Outcome of evaluating a policy expression. + /// + /// @param matched Whether the expression matched + /// @param message Message produced by the expression, if it returned a string + record Result(boolean matched, @Nullable String message) { + + private static final Result MATCHED = new Result(true, null); + private static final Result NOT_MATCHED = new Result(false, null); + } + private final CelRuntime.Program program; private final Map> requirements; @@ -41,7 +56,33 @@ Map> getRequirements() { return requirements; } + /// Evaluate the expression. + /// + /// A boolean result is the match outcome. A string result means the expression + /// matched and the string is the violation message, unless the string is blank, + /// which is treated as no match. See ADR 043. + Result evaluate(final Map arguments) throws CelEvaluationException { + final Object result = program.eval(arguments); + return switch (result) { + case Boolean matched -> matched ? Result.MATCHED : Result.NOT_MATCHED; + case String message -> { + final String strippedMessage = message.strip(); + if (strippedMessage.isEmpty()) { + yield Result.NOT_MATCHED; + } + yield new Result( + true, + strippedMessage.length() > MAX_MESSAGE_LENGTH + ? strippedMessage.substring(0, MAX_MESSAGE_LENGTH) + : strippedMessage); + } + default -> + throw new IllegalStateException("Expression returned unexpected type %s" + .formatted(result.getClass().getName())); + }; + } + boolean execute(final Map arguments) throws CelEvaluationException { - return (Boolean) program.eval(arguments); + return evaluate(arguments).matched(); } } diff --git a/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyType.java b/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyType.java index 3780d222cc..91c789ba51 100644 --- a/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyType.java +++ b/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyType.java @@ -20,6 +20,8 @@ import com.google.protobuf.Descriptors.Descriptor; import dev.cel.common.CelVarDecl; +import dev.cel.common.types.CelType; +import dev.cel.common.types.SimpleType; import dev.cel.compiler.CelCompiler; import dev.cel.compiler.CelCompilerFactory; import dev.cel.extensions.CelExtensions; @@ -33,15 +35,31 @@ import org.dependencytrack.proto.policy.v1.VersionDistance; import org.dependencytrack.proto.policy.v1.Vulnerability; +import java.util.Set; + public enum CelPolicyType { - COMPONENT(CelPolicyVariable.COMPONENT, CelPolicyVariable.PROJECT, CelPolicyVariable.VULNS, CelPolicyVariable.NOW), + /// Component policy conditions may return a boolean, or a string + /// that doubles as violation message. See ADR 043. + COMPONENT( + Set.of(SimpleType.BOOL, SimpleType.STRING), + CelPolicyVariable.COMPONENT, + CelPolicyVariable.PROJECT, + CelPolicyVariable.VULNS, + CelPolicyVariable.NOW), + /// Vulnerability policies are not result type checked at compile time. VULNERABILITY( - CelPolicyVariable.COMPONENT, CelPolicyVariable.PROJECT, CelPolicyVariable.VULN, CelPolicyVariable.NOW); + Set.of(), + CelPolicyVariable.COMPONENT, + CelPolicyVariable.PROJECT, + CelPolicyVariable.VULN, + CelPolicyVariable.NOW); + private final Set resultTypes; private final CelCompiler compiler; private final CelRuntime runtime; - CelPolicyType(CelPolicyVariable... variables) { + CelPolicyType(Set resultTypes, CelPolicyVariable... variables) { + this.resultTypes = resultTypes; final var library = new CelPolicyLibrary(); // NB: Message types must be registered directly on the builders, @@ -49,7 +67,7 @@ public enum CelPolicyType { // creates the descriptor pool before invoking library callbacks. final var compilerBuilder = CelCompilerFactory.standardCelCompilerBuilder() .setStandardMacros(CelStandardMacro.STANDARD_MACROS) - .addLibraries(CelExtensions.strings(), library) + .addLibraries(CelExtensions.strings(), CelExtensions.bindings(), library) .addMessageTypes(messageTypes()); for (final CelPolicyVariable variable : variables) { compilerBuilder.addVarDeclarations( @@ -57,6 +75,7 @@ public enum CelPolicyType { } this.compiler = compilerBuilder.build(); + // NB: The bindings extension is a compile-time macro and has no runtime part. this.runtime = CelRuntimeFactory.standardCelRuntimeBuilder() .addLibraries(CelExtensions.strings(), library) .addMessageTypes(messageTypes()) @@ -79,6 +98,12 @@ private static Descriptor[] messageTypes() { }; } + /// Result types an expression of this type may have. + /// An empty set means the result type is not checked. + Set resultTypes() { + return resultTypes; + } + CelCompiler compiler() { return compiler; } diff --git a/apiserver/src/main/java/org/dependencytrack/policy/cel/persistence/CelPolicyDao.java b/apiserver/src/main/java/org/dependencytrack/policy/cel/persistence/CelPolicyDao.java index 332715f400..86bb5e0871 100644 --- a/apiserver/src/main/java/org/dependencytrack/policy/cel/persistence/CelPolicyDao.java +++ b/apiserver/src/main/java/org/dependencytrack/policy/cel/persistence/CelPolicyDao.java @@ -485,6 +485,7 @@ public Set reconcileViolations( final var projIds = new Long[size]; final var condIds = new Long[size]; final var types = new String[size]; + final var texts = new String[size]; int i = 0; for (final var entry : reportedViolationsByComponentId.entrySet()) { @@ -494,13 +495,14 @@ public Set reconcileViolations( projIds[i] = projectId; condIds[i] = violation.getPolicyCondition().getId(); types[i] = violation.getType().name(); + texts[i] = violation.getText(); i++; } } return jdbiHandle .createQuery(""" - WITH created AS ( + WITH upserted AS ( INSERT INTO "POLICYVIOLATION" ( "UUID" , "TIMESTAMP" @@ -508,16 +510,21 @@ WITH created AS ( , "PROJECT_ID" , "POLICYCONDITION_ID" , "TYPE" + , "TEXT" ) SELECT GEN_RANDOM_UUID() , t.* - FROM UNNEST(:timestamps, :componentIds, :projectIds, :policyConditionIds, :types) - AS t("TIMESTAMP", "COMPONENT_ID", "PROJECT_ID", "POLICYCONDITION_ID", "TYPE") + FROM UNNEST(:timestamps, :componentIds, :projectIds, :policyConditionIds, :types, :texts) + AS t("TIMESTAMP", "COMPONENT_ID", "PROJECT_ID", "POLICYCONDITION_ID", "TYPE", "TEXT") ORDER BY t."PROJECT_ID" , t."COMPONENT_ID" , t."POLICYCONDITION_ID" - ON CONFLICT DO NOTHING - RETURNING "ID" + ON CONFLICT ("COMPONENT_ID", "PROJECT_ID", "POLICYCONDITION_ID") DO UPDATE + SET "TEXT" = EXCLUDED."TEXT" + WHERE "POLICYVIOLATION"."TEXT" IS DISTINCT FROM EXCLUDED."TEXT" + -- xmax is zero for rows created by this statement, + -- and non-zero for existing rows that were updated. + RETURNING "ID", (xmax = 0) AS "CREATED" ), deleted AS ( DELETE FROM "POLICYVIOLATION" @@ -532,13 +539,14 @@ deleted AS ( FOR UPDATE ) ) - SELECT "ID" FROM created + SELECT "ID" FROM upserted WHERE "CREATED" """) .bind("timestamps", timestamps) .bind("componentIds", componentIds) .bind("projectIds", projIds) .bind("policyConditionIds", condIds) .bind("types", types) + .bind("texts", texts) .bind("projectId", projectId) .mapTo(Long.class) .set(); diff --git a/apiserver/src/main/java/org/dependencytrack/resources/v1/vo/CreatePolicyConditionRequest.java b/apiserver/src/main/java/org/dependencytrack/resources/v1/vo/CreatePolicyConditionRequest.java index 3ab2ab4d86..a133218348 100644 --- a/apiserver/src/main/java/org/dependencytrack/resources/v1/vo/CreatePolicyConditionRequest.java +++ b/apiserver/src/main/java/org/dependencytrack/resources/v1/vo/CreatePolicyConditionRequest.java @@ -39,7 +39,11 @@ public record CreatePolicyConditionRequest( requiredMode = Schema.RequiredMode.REQUIRED) PolicyCondition.Operator operator, - @NotBlank @Schema(description = "Value the subject is compared to", requiredMode = Schema.RequiredMode.REQUIRED) + @NotBlank @Schema(description = """ + Value the subject is compared to. For `EXPRESSION` subjects, a CEL expression \ + that returns either a boolean, or a string. A non-blank string means the \ + condition matched and the string is used as violation message. A blank string \ + means the condition did not match.""", requiredMode = Schema.RequiredMode.REQUIRED) String value, @Schema(description = "Violation type produced when the condition matches. Required for `EXPRESSION` subjects") diff --git a/apiserver/src/main/java/org/dependencytrack/resources/v1/vo/UpdatePolicyConditionRequest.java b/apiserver/src/main/java/org/dependencytrack/resources/v1/vo/UpdatePolicyConditionRequest.java index 9298d8b0fd..fc471aebef 100644 --- a/apiserver/src/main/java/org/dependencytrack/resources/v1/vo/UpdatePolicyConditionRequest.java +++ b/apiserver/src/main/java/org/dependencytrack/resources/v1/vo/UpdatePolicyConditionRequest.java @@ -46,7 +46,11 @@ public record UpdatePolicyConditionRequest( requiredMode = Schema.RequiredMode.REQUIRED) PolicyCondition.Operator operator, - @NotBlank @Schema(description = "Value the subject is compared to", requiredMode = Schema.RequiredMode.REQUIRED) + @NotBlank @Schema(description = """ + Value the subject is compared to. For `EXPRESSION` subjects, a CEL expression \ + that returns either a boolean, or a string. A non-blank string means the \ + condition matched and the string is used as violation message. A blank string \ + means the condition did not match.""", requiredMode = Schema.RequiredMode.REQUIRED) String value, @Schema(description = "Violation type produced when the condition matches. Required for `EXPRESSION` subjects") diff --git a/apiserver/src/test/java/org/dependencytrack/notification/NotificationSubjectDaoTest.java b/apiserver/src/test/java/org/dependencytrack/notification/NotificationSubjectDaoTest.java index d9b775f326..d989b042fc 100644 --- a/apiserver/src/test/java/org/dependencytrack/notification/NotificationSubjectDaoTest.java +++ b/apiserver/src/test/java/org/dependencytrack/notification/NotificationSubjectDaoTest.java @@ -774,6 +774,7 @@ public void shouldGetForNewPolicyViolations() { violation.setComponent(component); violation.setPolicyCondition(condition); violation.setTimestamp(new Date()); + violation.setText("violationMessage"); qm.persist(violation); final List subjects = withJdbiHandle(handle -> @@ -818,6 +819,7 @@ public void shouldGetForNewPolicyViolations() { "uuid": "${json-unit.matches:violationUuid}", "type": "OPERATIONAL", "timestamp": "${json-unit.any-string}", + "message": "violationMessage", "condition": { "uuid": "${json-unit.matches:conditionUuid}", "subject": "VERSION", diff --git a/apiserver/src/test/java/org/dependencytrack/notification/ProcessScheduledNotificationRuleActivityTest.java b/apiserver/src/test/java/org/dependencytrack/notification/ProcessScheduledNotificationRuleActivityTest.java index d0b69d2755..a1ab149053 100644 --- a/apiserver/src/test/java/org/dependencytrack/notification/ProcessScheduledNotificationRuleActivityTest.java +++ b/apiserver/src/test/java/org/dependencytrack/notification/ProcessScheduledNotificationRuleActivityTest.java @@ -291,6 +291,7 @@ void shouldDispatchPolicyViolationNotification() throws InvalidProtocolBufferExc pvA_parent.setType(PolicyViolation.Type.OPERATIONAL); pvA_parent.setTimestamp(Date.from(afterRuleLastFiredAt)); + pvA_parent.setText("Version 1.0.0 is not allowed"); qm.persist(pvA_parent); final var pvB_parent = new PolicyViolation(); pvB_parent.setPolicyCondition(policyConditionB); @@ -424,7 +425,8 @@ void shouldDispatchPolicyViolationNotification() throws InvalidProtocolBufferExc "operator": "IS", "subject": "VERSION", "value": "1.0.0", "uuid": "${json-unit.any-string}" }, "type": "OPERATIONAL", - "suppressed": false + "suppressed": false, + "message": "Version 1.0.0 is not allowed" }, { "component": { "name": "acme-lib-parent", "version": "1.0.0", "uuid": "${json-unit.any-string}" }, diff --git a/apiserver/src/test/java/org/dependencytrack/policy/cel/CelPolicyCompilerTest.java b/apiserver/src/test/java/org/dependencytrack/policy/cel/CelPolicyCompilerTest.java index 717b6c063a..59b395eda2 100644 --- a/apiserver/src/test/java/org/dependencytrack/policy/cel/CelPolicyCompilerTest.java +++ b/apiserver/src/test/java/org/dependencytrack/policy/cel/CelPolicyCompilerTest.java @@ -63,6 +63,50 @@ void shouldNotCacheWhenCacheModeIsNoCache() throws Exception { assertThat(second).isNotSameAs(first); } + @Test + void shouldAcceptStringResultType() { + final var compiler = new CelPolicyCompiler(CelPolicyType.COMPONENT); + + assertDoesNotThrow(() -> compiler.compile(""" + component.name == "foo" ? "Component foo is not allowed" : "" + """, CacheMode.NO_CACHE)); + } + + @Test + void shouldRejectResultTypeOtherThanBoolOrString() { + final var compiler = new CelPolicyCompiler(CelPolicyType.COMPONENT); + + final var exception = assertThrows( + InvalidCelExpressionException.class, () -> compiler.compile("size(vulns)", CacheMode.NO_CACHE)); + assertThat(exception.getErrors()) + .satisfiesExactly(error -> assertThat(error.message()) + .isEqualTo("Expression must return bool or string, but returns int")); + } + + @Test + void shouldNotCheckResultTypeForVulnerabilityPolicies() { + final var compiler = new CelPolicyCompiler(CelPolicyType.VULNERABILITY); + + assertDoesNotThrow(() -> compiler.compile("vuln.id", CacheMode.NO_CACHE)); + } + + @Test + void testRequirementsAnalysisWithBinding() { + final var compiler = new CelPolicyCompiler(CelPolicyType.COMPONENT); + + final CelPolicyProgram program = compiler.compile(""" + cel.bind( + matched, + vulns.filter(vuln, has(vuln.epss_score) && vuln.epss_score >= 0.5), + size(matched) > 0 + ? "EPSS >= 0.5 matched by: " + matched.map(vuln, vuln.id).join(", ") + : "" + ) + """, CacheMode.NO_CACHE); + + assertThat(program.getRequirements()).containsEntry(TYPE_VULNERABILITY, Set.of("epss_score", "id")); + } + @Test void shouldReturnDeeplyImmutableRequirements() throws Exception { final CelPolicyProgram compiledProgram = diff --git a/apiserver/src/test/java/org/dependencytrack/policy/cel/CelPolicyEngineTest.java b/apiserver/src/test/java/org/dependencytrack/policy/cel/CelPolicyEngineTest.java index 8243708b53..d9826e9db1 100644 --- a/apiserver/src/test/java/org/dependencytrack/policy/cel/CelPolicyEngineTest.java +++ b/apiserver/src/test/java/org/dependencytrack/policy/cel/CelPolicyEngineTest.java @@ -1361,6 +1361,130 @@ void testEvaluateProjectWithScriptExecutionException() throws Exception { violation -> assertThat(violation.getPolicyCondition()).isEqualTo(validCondition)); } + @Test + void testEvaluateProjectWithStringResult() throws Exception { + final var policy = qm.createPolicy("policy", Policy.Operator.ANY, Policy.ViolationState.FAIL); + final PolicyCondition condition = qm.createPolicyCondition( + policy, + PolicyCondition.Subject.EXPRESSION, + PolicyCondition.Operator.MATCHES, + """ + cel.bind( + name, + component.name, + name == "acme-lib" ? " Component " + name + " is not allowed " : "" + ) + """, + PolicyViolation.Type.OPERATIONAL); + + final var project = new Project(); + project.setName("acme-app"); + qm.persist(project); + + final var component = new Component(); + component.setProject(project); + component.setName("acme-lib"); + qm.persist(component); + + new CelPolicyEngine().evaluateProject(project.getUuid()); + + assertThat(qm.getAllPolicyViolations(component)).satisfiesExactly(violation -> { + assertThat(violation.getPolicyCondition()).isEqualTo(condition); + assertThat(violation.getText()).isEqualTo("Component acme-lib is not allowed"); + }); + } + + @Test + void testEvaluateProjectWithBlankStringResult() throws Exception { + final var policy = qm.createPolicy("policy", Policy.Operator.ANY, Policy.ViolationState.FAIL); + qm.createPolicyCondition( + policy, + PolicyCondition.Subject.EXPRESSION, + PolicyCondition.Operator.MATCHES, + """ + component.name == "acme-lib" ? " " : "Component is not acme-lib" + """, + PolicyViolation.Type.OPERATIONAL); + + final var project = new Project(); + project.setName("acme-app"); + qm.persist(project); + + final var component = new Component(); + component.setProject(project); + component.setName("acme-lib"); + qm.persist(component); + + new CelPolicyEngine().evaluateProject(project.getUuid()); + + assertThat(qm.getAllPolicyViolations(component)).isEmpty(); + } + + @Test + void testEvaluateProjectWithLongStringResult() throws Exception { + final var policy = qm.createPolicy("policy", Policy.Operator.ANY, Policy.ViolationState.FAIL); + qm.createPolicyCondition( + policy, + PolicyCondition.Subject.EXPRESSION, + PolicyCondition.Operator.MATCHES, + "\"%s\"".formatted("a".repeat(2000)), + PolicyViolation.Type.OPERATIONAL); + + final var project = new Project(); + project.setName("acme-app"); + qm.persist(project); + + final var component = new Component(); + component.setProject(project); + component.setName("acme-lib"); + qm.persist(component); + + new CelPolicyEngine().evaluateProject(project.getUuid()); + + assertThat(qm.getAllPolicyViolations(component)) + .satisfiesExactly(violation -> assertThat(violation.getText()).hasSize(1024)); + } + + @Test + void testEvaluateProjectUpdatesMessageOfExistingViolation() throws Exception { + final var policy = qm.createPolicy("policy", Policy.Operator.ANY, Policy.ViolationState.FAIL); + qm.createPolicyCondition( + policy, + PolicyCondition.Subject.EXPRESSION, + PolicyCondition.Operator.MATCHES, + """ + "Component version is " + component.version + """, + PolicyViolation.Type.OPERATIONAL); + + final var project = new Project(); + project.setName("acme-app"); + qm.persist(project); + + final var component = new Component(); + component.setProject(project); + component.setName("acme-lib"); + component.setVersion("1.0.0"); + qm.persist(component); + + new CelPolicyEngine().evaluateProject(project.getUuid()); + + final PolicyViolation initialViolation = + qm.getAllPolicyViolations(component).getFirst(); + assertThat(initialViolation.getText()).isEqualTo("Component version is 1.0.0"); + + component.setVersion("2.0.0"); + qm.persist(component); + + new CelPolicyEngine().evaluateProject(project.getUuid()); + qm.getPersistenceManager().evictAll(); + + assertThat(qm.getAllPolicyViolations(component)).satisfiesExactly(violation -> { + assertThat(violation.getUuid()).isEqualTo(initialViolation.getUuid()); + assertThat(violation.getText()).isEqualTo("Component version is 2.0.0"); + }); + } + @Test void testEvaluateProjectWithFuncProjectDependsOnComponent() throws Exception { final var policy = qm.createPolicy("policy", Policy.Operator.ANY, Policy.ViolationState.FAIL); diff --git a/apiserver/src/test/java/org/dependencytrack/policy/cel/persistence/CelPolicyDaoTest.java b/apiserver/src/test/java/org/dependencytrack/policy/cel/persistence/CelPolicyDaoTest.java index 9b010377dd..afa0d84e8d 100644 --- a/apiserver/src/test/java/org/dependencytrack/policy/cel/persistence/CelPolicyDaoTest.java +++ b/apiserver/src/test/java/org/dependencytrack/policy/cel/persistence/CelPolicyDaoTest.java @@ -32,6 +32,9 @@ import org.dependencytrack.model.Component; import org.dependencytrack.model.License; import org.dependencytrack.model.LicenseGroup; +import org.dependencytrack.model.Policy; +import org.dependencytrack.model.PolicyCondition; +import org.dependencytrack.model.PolicyViolation; import org.dependencytrack.model.Project; import org.dependencytrack.model.Severity; import org.dependencytrack.model.Vulnerability; @@ -55,6 +58,7 @@ import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.entry; +import static org.dependencytrack.persistence.jdbi.JdbiFactory.inJdbiTransaction; import static org.dependencytrack.persistence.jdbi.JdbiFactory.useJdbiHandle; import static org.dependencytrack.persistence.jdbi.JdbiFactory.useJdbiTransaction; import static org.dependencytrack.persistence.jdbi.JdbiFactory.withJdbiHandle; @@ -380,4 +384,68 @@ public void testLoadRequiredFieldsForVulnerability() throws Exception { } """); } + + @Test + public void testReconcileViolationsWithMessage() { + final var project = new Project(); + project.setName("projectName"); + qm.persist(project); + + final var component = new Component(); + component.setProject(project); + component.setName("componentName"); + qm.persist(component); + + final Policy policy = qm.createPolicy("policy", Policy.Operator.ANY, Policy.ViolationState.FAIL); + final PolicyCondition condition = qm.createPolicyCondition( + policy, + PolicyCondition.Subject.EXPRESSION, + PolicyCondition.Operator.MATCHES, + "\"message-a\"", + PolicyViolation.Type.OPERATIONAL); + + final var violation = new PolicyViolation(); + violation.setType(PolicyViolation.Type.OPERATIONAL); + violation.setPolicyCondition(condition); + violation.setTimestamp(new Date()); + violation.setText("message-a"); + + final Map> violationsByComponentId = Map.of(component.getId(), List.of(violation)); + + // First reconciliation creates the violation and reports it as new. + final Set createdIds = inJdbiTransaction( + handle -> new CelPolicyDao(handle).reconcileViolations(project.getId(), violationsByComponentId)); + assertThat(createdIds).hasSize(1); + final long violationId = createdIds.iterator().next(); + assertThat(getViolationText(violationId)).isEqualTo("message-a"); + + // Unchanged message: nothing is reported as new. + final Set unchangedIds = inJdbiTransaction( + handle -> new CelPolicyDao(handle).reconcileViolations(project.getId(), violationsByComponentId)); + assertThat(unchangedIds).isEmpty(); + assertThat(getViolationText(violationId)).isEqualTo("message-a"); + + // Changed message: the existing violation is updated, but not reported as new. + violation.setText("message-b"); + final Set changedIds = inJdbiTransaction( + handle -> new CelPolicyDao(handle).reconcileViolations(project.getId(), violationsByComponentId)); + assertThat(changedIds).isEmpty(); + assertThat(getViolationText(violationId)).isEqualTo("message-b"); + + // Message removed: the existing violation is updated, but not reported as new. + violation.setText(null); + final Set clearedIds = inJdbiTransaction( + handle -> new CelPolicyDao(handle).reconcileViolations(project.getId(), violationsByComponentId)); + assertThat(clearedIds).isEmpty(); + assertThat(getViolationText(violationId)).isNull(); + } + + private static String getViolationText(long violationId) { + return withJdbiHandle(handle -> handle.createQuery(""" + SELECT "TEXT" FROM "POLICYVIOLATION" WHERE "ID" = :id + """) + .bind("id", violationId) + .mapTo(String.class) + .one()); + } } diff --git a/apiserver/src/test/java/org/dependencytrack/resources/v1/PolicyConditionResourceTest.java b/apiserver/src/test/java/org/dependencytrack/resources/v1/PolicyConditionResourceTest.java index 0ed4bf3876..1d31ccc91b 100644 --- a/apiserver/src/test/java/org/dependencytrack/resources/v1/PolicyConditionResourceTest.java +++ b/apiserver/src/test/java/org/dependencytrack/resources/v1/PolicyConditionResourceTest.java @@ -145,6 +145,56 @@ public void testCreateConditionWithExpression() { """); } + @Test + public void testCreateConditionWithExpressionReturningString() { + initializeWithPermissions(Permissions.POLICY_MANAGEMENT_UPDATE); + + final Policy policy = qm.createPolicy("policy", Operator.ANY, ViolationState.FAIL); + + final Response response = jersey.target("%s/%s/condition".formatted(V1_POLICY, policy.getUuid())) + .request() + .header(X_API_KEY, apiKey) + .put(Entity.entity(/* language=JSON */ """ + { + "subject": "EXPRESSION", + "value": "component.name == \\"foo\\" ? \\"foo is not allowed\\" : \\"\\"", + "violationType": "SECURITY" + } + """, MediaType.APPLICATION_JSON)); + assertThat(response.getStatus()).isEqualTo(201); + } + + @Test + public void testCreateConditionWithExpressionReturningInvalidType() { + initializeWithPermissions(Permissions.POLICY_MANAGEMENT_UPDATE); + + final Policy policy = qm.createPolicy("policy", Operator.ANY, ViolationState.FAIL); + + final Response response = jersey.target("%s/%s/condition".formatted(V1_POLICY, policy.getUuid())) + .request() + .header(X_API_KEY, apiKey) + .put(Entity.entity(/* language=JSON */ """ + { + "subject": "EXPRESSION", + "value": "size(vulns)", + "violationType": "SECURITY" + } + """, MediaType.APPLICATION_JSON)); + + assertThat(response.getStatus()).isEqualTo(400); + assertThatJson(getPlainTextBody(response)).isEqualTo(/* language=JSON */ """ + { + "celErrors": [ + { + "line": 1, + "column": 0, + "message": "Expression must return bool or string, but returns int" + } + ] + } + """); + } + @Test public void testCreateConditionWithInvalidExpression() { initializeWithPermissions(Permissions.POLICY_MANAGEMENT_UPDATE); diff --git a/migration/src/main/resources/org/dependencytrack/migration/V202610061719__widen_policy_violation_text.sql b/migration/src/main/resources/org/dependencytrack/migration/V202610061719__widen_policy_violation_text.sql new file mode 100644 index 0000000000..de5bf0f502 --- /dev/null +++ b/migration/src/main/resources/org/dependencytrack/migration/V202610061719__widen_policy_violation_text.sql @@ -0,0 +1,4 @@ +-- Policy expressions may return a violation message of up to 1024 characters. +-- Changing VARCHAR to TEXT does not rewrite the table in Postgres. +-- squawk-ignore changing-column-type, prefer-robust-stmts +ALTER TABLE "POLICYVIOLATION" ALTER COLUMN "TEXT" TYPE TEXT; diff --git a/notification/api/src/main/java/org/dependencytrack/notification/api/TestNotificationFactory.java b/notification/api/src/main/java/org/dependencytrack/notification/api/TestNotificationFactory.java index b549a3103d..ce3597792a 100644 --- a/notification/api/src/main/java/org/dependencytrack/notification/api/TestNotificationFactory.java +++ b/notification/api/src/main/java/org/dependencytrack/notification/api/TestNotificationFactory.java @@ -319,6 +319,7 @@ public static Notification createNewPolicyViolationsSummaryTestNotification() { .setTimestamp(Timestamps.fromSeconds(66666)) .setAnalysisState("APPROVED") .setSuppressed(false) + .setMessage("Component is 666 days old") .build(); final var sinceTimestamp = Timestamps.fromMillis(66666); @@ -394,6 +395,7 @@ private static PolicyViolation createPolicyViolation(PolicyCondition condition) .setCondition(condition) .setType("OPERATIONAL") .setTimestamp(Timestamps.now()) + .setMessage("Package URL pkg:maven/foo/bar@1.2.3 is not allowed") .build(); } diff --git a/notification/api/src/main/proto/org/dependencytrack/notification/v1/notification.proto b/notification/api/src/main/proto/org/dependencytrack/notification/v1/notification.proto index 98ed5fe8cd..fe34ef2ebe 100644 --- a/notification/api/src/main/proto/org/dependencytrack/notification/v1/notification.proto +++ b/notification/api/src/main/proto/org/dependencytrack/notification/v1/notification.proto @@ -189,6 +189,8 @@ message PolicyViolation { string type = 2; google.protobuf.Timestamp timestamp = 3; PolicyCondition condition = 4; + // Message produced by the policy condition expression, if any. + optional string message = 5; } message PolicyCondition { @@ -362,5 +364,7 @@ message NewPolicyViolationsSummarySubject { google.protobuf.Timestamp timestamp = 5; optional string analysis_state = 6; bool suppressed = 7; + // Message produced by the policy condition expression, if any. + optional string message = 8; } } diff --git a/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/email/default-template.peb b/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/email/default-template.peb index 9ae1697e57..a2aa576f62 100644 --- a/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/email/default-template.peb +++ b/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/email/default-template.peb @@ -108,6 +108,9 @@ Component Version: {{ subject.component.version }} Component URL: {{ baseUrl }}/components/{{ subject.component.uuid }} Violated Policy: {{ subject.policyViolation.condition.policy.name }} +{% if subject.policyViolation.message is not empty -%} +Message: {{ subject.policyViolation.message }} +{% endif -%} {% elseif notification.group == "GROUP_NEW_VULNERABILITIES_SUMMARY" %} Overview: - New Vulnerabilities: {{ subject.overview.totalNewVulnerabilitiesCount }} (Suppressed: {{ subject.overview.suppressedNewVulnerabilitiesCount }}) @@ -190,6 +193,9 @@ Violation Details: Timestamp: {{ violation.timestamp | formatTimestamp }} Analysis State: {{ violation.analysisState | default("-") }} Suppressed: {{ violation.suppressed }} +{% if violation.message is not empty -%} + Message: {{ violation.message }} +{% endif -%} {% endfor -%} {% endfor -%} {% endif %} diff --git a/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/mattermost/default-template.peb b/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/mattermost/default-template.peb index 741aec4b85..e35ffa6ada 100644 --- a/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/mattermost/default-template.peb +++ b/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/mattermost/default-template.peb @@ -1,5 +1,5 @@ { "username": "Dependency Track", "icon_url": "https://raw.githubusercontent.com/DependencyTrack/branding/master/dt-logo-symbol-blue-background.png", - "text": "#### {{ notification.title | escape(strategy="json") }}\n{{ notification.content | escape(strategy="json") }}\n{% if notification.group == "GROUP_NEW_VULNERABILITY" %}**Component**: {{ subject.component | summarize | escape(strategy="json") }}\n**Vulnerability**: {{ subject.vulnerability.vulnId | escape(strategy="json") }}, {{ subject.vulnerability.severity | escape(strategy="json") }}\n[View Component]({{ baseUrl }}/components/{{ subject.component.uuid | escape(strategy="json") }}) - [View Vulnerability]({{ baseUrl }}/vulnerabilities/{{ subject.vulnerability.source | escape(strategy="json") }}/{{ subject.vulnerability.vulnId | escape(strategy="json") }}){% elseif notification.group == "GROUP_NEW_VULNERABLE_DEPENDENCY" %}**Project**: {{ subject.project | summarize | escape(strategy="json") }}\n**Component**: {{ subject.component | summarize | escape(strategy="json") }}\n[View Project]({{ baseUrl }}/projects/{{ subject.project.uuid | escape(strategy="json") }}) - [View Component]({{ baseUrl }}/components/{{ subject.component.uuid | escape(strategy="json") }}){% elseif notification.group == "GROUP_PROJECT_AUDIT_CHANGE" %}{% set isViolationAnalysis = subject.policyViolation is defined %}**Project**: {{ subject.project | summarize | escape(strategy="json") }}\n**Component**: {{ subject.component | summarize | escape(strategy="json") }}\n{% if isViolationAnalysis %}**Policy**: {{ subject.policyViolation.condition.policy.name | escape(strategy="json") }}, {{ subject.policyViolation.condition.subject | escape(strategy="json") }} {{ subject.policyViolation.condition.operator | escape(strategy="json") }} {{ subject.policyViolation.condition.value | escape(strategy="json") }}{% else %}**Vulnerability**: {{ subject.vulnerability.vulnId | escape(strategy="json") }}, {{ subject.vulnerability.severity | escape(strategy="json") }}{% endif %}\n**Analysis**: {{ subject.analysis.state | escape(strategy="json") }}, suppressed: {{ subject.analysis.suppressed | escape(strategy="json") }}\n[View Project]({{ baseUrl }}/projects/{{ subject.project.uuid | escape(strategy="json") }}) - [View Component]({{ baseUrl }}/components/{{ subject.component.uuid | escape(strategy="json") }}){% if not isViolationAnalysis %} - [View Vulnerability]({{ baseUrl }}/vulnerabilities/{{ subject.vulnerability.source | escape(strategy="json") }}/{{ subject.vulnerability.vulnId | escape(strategy="json") }}){% endif %}{% elseif notification.group == "GROUP_BOM_CONSUMED" or notification.group == "GROUP_BOM_PROCESSED" %}**Project**: {{ subject.project | summarize | escape(strategy="json") }}\n[View Project]({{ baseUrl }}/projects/{{ subject.project.uuid | escape(strategy="json") }}){% elseif notification.group == "GROUP_POLICY_VIOLATION" %}**Project**: {{ subject.project | summarize | escape(strategy="json") }}\n**Component**: {{ subject.component | summarize | escape(strategy="json") }}\n**Policy**: {{ subject.policyViolation.condition.policy.violationState | escape(strategy="json") }}, {{ subject.policyViolation.condition.policy.name | escape(strategy="json") }}\n[View Project]({{ baseUrl }}/projects/{{ subject.project.uuid | escape(strategy="json") }}) - [View Component]({{ baseUrl }}/components/{{ subject.component.uuid | escape(strategy="json") }}){% endif %}" + "text": "#### {{ notification.title | escape(strategy="json") }}\n{{ notification.content | escape(strategy="json") }}\n{% if notification.group == "GROUP_NEW_VULNERABILITY" %}**Component**: {{ subject.component | summarize | escape(strategy="json") }}\n**Vulnerability**: {{ subject.vulnerability.vulnId | escape(strategy="json") }}, {{ subject.vulnerability.severity | escape(strategy="json") }}\n[View Component]({{ baseUrl }}/components/{{ subject.component.uuid | escape(strategy="json") }}) - [View Vulnerability]({{ baseUrl }}/vulnerabilities/{{ subject.vulnerability.source | escape(strategy="json") }}/{{ subject.vulnerability.vulnId | escape(strategy="json") }}){% elseif notification.group == "GROUP_NEW_VULNERABLE_DEPENDENCY" %}**Project**: {{ subject.project | summarize | escape(strategy="json") }}\n**Component**: {{ subject.component | summarize | escape(strategy="json") }}\n[View Project]({{ baseUrl }}/projects/{{ subject.project.uuid | escape(strategy="json") }}) - [View Component]({{ baseUrl }}/components/{{ subject.component.uuid | escape(strategy="json") }}){% elseif notification.group == "GROUP_PROJECT_AUDIT_CHANGE" %}{% set isViolationAnalysis = subject.policyViolation is defined %}**Project**: {{ subject.project | summarize | escape(strategy="json") }}\n**Component**: {{ subject.component | summarize | escape(strategy="json") }}\n{% if isViolationAnalysis %}**Policy**: {{ subject.policyViolation.condition.policy.name | escape(strategy="json") }}, {{ subject.policyViolation.condition.subject | escape(strategy="json") }} {{ subject.policyViolation.condition.operator | escape(strategy="json") }} {{ subject.policyViolation.condition.value | escape(strategy="json") }}{% else %}**Vulnerability**: {{ subject.vulnerability.vulnId | escape(strategy="json") }}, {{ subject.vulnerability.severity | escape(strategy="json") }}{% endif %}\n**Analysis**: {{ subject.analysis.state | escape(strategy="json") }}, suppressed: {{ subject.analysis.suppressed | escape(strategy="json") }}\n[View Project]({{ baseUrl }}/projects/{{ subject.project.uuid | escape(strategy="json") }}) - [View Component]({{ baseUrl }}/components/{{ subject.component.uuid | escape(strategy="json") }}){% if not isViolationAnalysis %} - [View Vulnerability]({{ baseUrl }}/vulnerabilities/{{ subject.vulnerability.source | escape(strategy="json") }}/{{ subject.vulnerability.vulnId | escape(strategy="json") }}){% endif %}{% elseif notification.group == "GROUP_BOM_CONSUMED" or notification.group == "GROUP_BOM_PROCESSED" %}**Project**: {{ subject.project | summarize | escape(strategy="json") }}\n[View Project]({{ baseUrl }}/projects/{{ subject.project.uuid | escape(strategy="json") }}){% elseif notification.group == "GROUP_POLICY_VIOLATION" %}**Project**: {{ subject.project | summarize | escape(strategy="json") }}\n**Component**: {{ subject.component | summarize | escape(strategy="json") }}\n**Policy**: {{ subject.policyViolation.condition.policy.violationState | escape(strategy="json") }}, {{ subject.policyViolation.condition.policy.name | escape(strategy="json") }}\n{% if subject.policyViolation.message is not empty %}**Message**: {{ subject.policyViolation.message | escape(strategy="json") }}\n{% endif %}[View Project]({{ baseUrl }}/projects/{{ subject.project.uuid | escape(strategy="json") }}) - [View Component]({{ baseUrl }}/components/{{ subject.component.uuid | escape(strategy="json") }}){% endif %}" } \ No newline at end of file diff --git a/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/msteams/default-template.peb b/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/msteams/default-template.peb index dc9e6acf2c..1dc0b90949 100644 --- a/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/msteams/default-template.peb +++ b/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/msteams/default-template.peb @@ -113,7 +113,11 @@ { "name": "Project", "value": "{{ subject.project | summarize | escape(strategy="json") }}" - } + }{% if subject.policyViolation.message is not empty %}, + { + "name": "Message", + "value": "{{ subject.policyViolation.message | escape(strategy="json") }}" + }{% endif %} ], {% elseif notification.group == "GROUP_BOM_PROCESSING_FAILED" %} "facts": [ diff --git a/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/slack/default-template.peb b/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/slack/default-template.peb index 5e07db3ec3..19e33d8469 100644 --- a/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/slack/default-template.peb +++ b/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/slack/default-template.peb @@ -397,7 +397,14 @@ "text": "{{ subject.project.toString | escape(strategy="json") }}" } ] - }{% if baseUrl is not empty %}, + }{% if subject.policyViolation.message is not empty %}, + { + "type": "section", + "text": { + "type": "mrkdwn", + "text": "*Message*\n{{ subject.policyViolation.message | escape(strategy="json") }}" + } + }{% endif %}{% if baseUrl is not empty %}, { "type": "actions", "elements": [ diff --git a/notification/publishing/src/test/java/org/dependencytrack/notification/publishing/email/EmailNotificationPublisherTest.java b/notification/publishing/src/test/java/org/dependencytrack/notification/publishing/email/EmailNotificationPublisherTest.java index 48b49536db..c4afea8fe6 100644 --- a/notification/publishing/src/test/java/org/dependencytrack/notification/publishing/email/EmailNotificationPublisherTest.java +++ b/notification/publishing/src/test/java/org/dependencytrack/notification/publishing/email/EmailNotificationPublisherTest.java @@ -337,6 +337,7 @@ private void validateNewPolicyViolationsSummaryNotificationPublish() { Timestamp: 1970-01-01T18:31:06Z Analysis State: APPROVED Suppressed: false + Message: Component is 666 days old -------------------------------------------------------------------------------- diff --git a/notification/publishing/src/test/java/org/dependencytrack/notification/publishing/slack/SlackNotificationPublisherTest.java b/notification/publishing/src/test/java/org/dependencytrack/notification/publishing/slack/SlackNotificationPublisherTest.java index 78d7af4ad1..9c606e02cf 100644 --- a/notification/publishing/src/test/java/org/dependencytrack/notification/publishing/slack/SlackNotificationPublisherTest.java +++ b/notification/publishing/src/test/java/org/dependencytrack/notification/publishing/slack/SlackNotificationPublisherTest.java @@ -152,6 +152,13 @@ private void validatePolicyViolationNotificationPublish() { } ] }, + { + "type": "section", + "text": { + "type": "mrkdwn", + "text": "*Message*\\nPackage URL pkg:maven/foo/bar@1.2.3 is not allowed" + } + }, { "type": "actions", "elements": [ From aaf7bf4cf762a9eb94c76ae9587213bfdb50d6db Mon Sep 17 00:00:00 2001 From: Matt Finkel Date: Sun, 11 Oct 2026 17:21:23 +0100 Subject: [PATCH 2/5] Require vulnerability policy conditions to return bool A matching vulnerability policy usually applies a suppression. Treating any non-blank string as a match could suppress findings by accident, so only boolean results are accepted. This also removes the unchecked result type special case from the compiler. Signed-off-by: Matt Finkel --- .../policy/cel/CelPolicyCompiler.java | 2 +- .../policy/cel/CelPolicyType.java | 7 ++-- .../v2/VulnPoliciesResourceTest.java | 36 +++++++++++++++++++ 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyCompiler.java b/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyCompiler.java index d7abb2409c..513617f0bb 100644 --- a/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyCompiler.java +++ b/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyCompiler.java @@ -127,7 +127,7 @@ private CelPolicyProgram compile(String normalizedSrc, String scriptDigest, Cach private void validateResultType(CelAbstractSyntaxTree ast) throws CelValidationException { final Set allowedTypes = policyType.resultTypes(); - if (allowedTypes.isEmpty() || allowedTypes.contains(ast.getResultType())) { + if (allowedTypes.contains(ast.getResultType())) { return; } diff --git a/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyType.java b/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyType.java index 91c789ba51..97cdcdc1b1 100644 --- a/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyType.java +++ b/apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyType.java @@ -46,9 +46,11 @@ public enum CelPolicyType { CelPolicyVariable.PROJECT, CelPolicyVariable.VULNS, CelPolicyVariable.NOW), - /// Vulnerability policies are not result type checked at compile time. + /// Vulnerability policy conditions must return a boolean. A match usually + /// applies a suppression, so a string result is rejected to prevent + /// accidental matches. VULNERABILITY( - Set.of(), + Set.of(SimpleType.BOOL), CelPolicyVariable.COMPONENT, CelPolicyVariable.PROJECT, CelPolicyVariable.VULN, @@ -99,7 +101,6 @@ private static Descriptor[] messageTypes() { } /// Result types an expression of this type may have. - /// An empty set means the result type is not checked. Set resultTypes() { return resultTypes; } diff --git a/apiserver/src/test/java/org/dependencytrack/resources/v2/VulnPoliciesResourceTest.java b/apiserver/src/test/java/org/dependencytrack/resources/v2/VulnPoliciesResourceTest.java index de7ddae7fb..dc83a89d47 100644 --- a/apiserver/src/test/java/org/dependencytrack/resources/v2/VulnPoliciesResourceTest.java +++ b/apiserver/src/test/java/org/dependencytrack/resources/v2/VulnPoliciesResourceTest.java @@ -556,6 +556,42 @@ void shouldReturnCelErrorsWhenCreateConditionIsInvalid() { """); } + @Test + void shouldReturnCelErrorsWhenCreateConditionDoesNotReturnBool() { + initializeWithPermissions(Permissions.POLICY_MANAGEMENT_CREATE); + + final Response response = jersey.target("/vuln-policies") + .request() + .header(X_API_KEY, apiKey) + .post(Entity.json(/* language=JSON */ """ + { + "name": "test-policy", + "condition": "vuln.id", + "analysis": { + "state": "NOT_AFFECTED", + "suppress": true + } + } + """)); + + assertThat(response.getStatus()).isEqualTo(400); + assertThatJson(getPlainTextBody(response)).isEqualTo(/* language=JSON */ """ + { + "type": "about:blank", + "status": 400, + "title": "Bad Request", + "detail": "Condition is invalid.", + "errors": [ + { + "line": 1, + "column": 0, + "message": "Expression must return bool, but returns string" + } + ] + } + """); + } + @Test void shouldReturnCelErrorsWhenUpdateConditionIsInvalid() { initializeWithPermissions(Permissions.POLICY_MANAGEMENT, Permissions.POLICY_MANAGEMENT_UPDATE); From f80ad428f2d4be6652f9903b9d6488c6692390a0 Mon Sep 17 00:00:00 2001 From: Matt Finkel Date: Sun, 11 Oct 2026 17:21:23 +0100 Subject: [PATCH 3/5] Render policy violation message as plain text in Slack template Matches the other fields in the policy violation section. Signed-off-by: Matt Finkel --- .../publishing/slack/default-template.peb | 14 ++++++++++---- .../slack/SlackNotificationPublisherTest.java | 14 ++++++++++---- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/slack/default-template.peb b/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/slack/default-template.peb index 19e33d8469..d7b64d20f2 100644 --- a/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/slack/default-template.peb +++ b/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/slack/default-template.peb @@ -400,10 +400,16 @@ }{% if subject.policyViolation.message is not empty %}, { "type": "section", - "text": { - "type": "mrkdwn", - "text": "*Message*\n{{ subject.policyViolation.message | escape(strategy="json") }}" - } + "fields": [ + { + "type": "mrkdwn", + "text": "*Message*" + }, + { + "type": "plain_text", + "text": "{{ subject.policyViolation.message | escape(strategy="json") }}" + } + ] }{% endif %}{% if baseUrl is not empty %}, { "type": "actions", diff --git a/notification/publishing/src/test/java/org/dependencytrack/notification/publishing/slack/SlackNotificationPublisherTest.java b/notification/publishing/src/test/java/org/dependencytrack/notification/publishing/slack/SlackNotificationPublisherTest.java index 9c606e02cf..833c726b8e 100644 --- a/notification/publishing/src/test/java/org/dependencytrack/notification/publishing/slack/SlackNotificationPublisherTest.java +++ b/notification/publishing/src/test/java/org/dependencytrack/notification/publishing/slack/SlackNotificationPublisherTest.java @@ -154,10 +154,16 @@ private void validatePolicyViolationNotificationPublish() { }, { "type": "section", - "text": { - "type": "mrkdwn", - "text": "*Message*\\nPackage URL pkg:maven/foo/bar@1.2.3 is not allowed" - } + "fields": [ + { + "type": "mrkdwn", + "text": "*Message*" + }, + { + "type": "plain_text", + "text": "Package URL pkg:maven/foo/bar@1.2.3 is not allowed" + } + ] }, { "type": "actions", From c9506b661d3efc0bec9f856a97fdb73845040084 Mon Sep 17 00:00:00 2001 From: Matt Finkel Date: Sun, 11 Oct 2026 17:21:23 +0100 Subject: [PATCH 4/5] Remove result type tests from CelPolicyCompilerTest The same cases are covered at the resource layer by PolicyConditionResourceTest and VulnPoliciesResourceTest. Signed-off-by: Matt Finkel --- .../policy/cel/CelPolicyCompilerTest.java | 27 ------------------- 1 file changed, 27 deletions(-) diff --git a/apiserver/src/test/java/org/dependencytrack/policy/cel/CelPolicyCompilerTest.java b/apiserver/src/test/java/org/dependencytrack/policy/cel/CelPolicyCompilerTest.java index 59b395eda2..2e20d7f890 100644 --- a/apiserver/src/test/java/org/dependencytrack/policy/cel/CelPolicyCompilerTest.java +++ b/apiserver/src/test/java/org/dependencytrack/policy/cel/CelPolicyCompilerTest.java @@ -63,33 +63,6 @@ void shouldNotCacheWhenCacheModeIsNoCache() throws Exception { assertThat(second).isNotSameAs(first); } - @Test - void shouldAcceptStringResultType() { - final var compiler = new CelPolicyCompiler(CelPolicyType.COMPONENT); - - assertDoesNotThrow(() -> compiler.compile(""" - component.name == "foo" ? "Component foo is not allowed" : "" - """, CacheMode.NO_CACHE)); - } - - @Test - void shouldRejectResultTypeOtherThanBoolOrString() { - final var compiler = new CelPolicyCompiler(CelPolicyType.COMPONENT); - - final var exception = assertThrows( - InvalidCelExpressionException.class, () -> compiler.compile("size(vulns)", CacheMode.NO_CACHE)); - assertThat(exception.getErrors()) - .satisfiesExactly(error -> assertThat(error.message()) - .isEqualTo("Expression must return bool or string, but returns int")); - } - - @Test - void shouldNotCheckResultTypeForVulnerabilityPolicies() { - final var compiler = new CelPolicyCompiler(CelPolicyType.VULNERABILITY); - - assertDoesNotThrow(() -> compiler.compile("vuln.id", CacheMode.NO_CACHE)); - } - @Test void testRequirementsAnalysisWithBinding() { final var compiler = new CelPolicyCompiler(CelPolicyType.COMPONENT); From a197621951fedec14c255e8914777d374239f9b8 Mon Sep 17 00:00:00 2001 From: Matt Finkel Date: Sun, 11 Oct 2026 18:03:09 +0100 Subject: [PATCH 5/5] Drop label from Slack policy violation message The review asked only for the text type to become plain_text. A bold label would render as literal asterisks in plain text, so it is removed. Signed-off-by: Matt Finkel --- .../publishing/slack/default-template.peb | 14 ++++---------- .../slack/SlackNotificationPublisherTest.java | 14 ++++---------- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/slack/default-template.peb b/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/slack/default-template.peb index d7b64d20f2..69ca8e103f 100644 --- a/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/slack/default-template.peb +++ b/notification/publishing/src/main/resources/org/dependencytrack/notification/publishing/slack/default-template.peb @@ -400,16 +400,10 @@ }{% if subject.policyViolation.message is not empty %}, { "type": "section", - "fields": [ - { - "type": "mrkdwn", - "text": "*Message*" - }, - { - "type": "plain_text", - "text": "{{ subject.policyViolation.message | escape(strategy="json") }}" - } - ] + "text": { + "type": "plain_text", + "text": "{{ subject.policyViolation.message | escape(strategy="json") }}" + } }{% endif %}{% if baseUrl is not empty %}, { "type": "actions", diff --git a/notification/publishing/src/test/java/org/dependencytrack/notification/publishing/slack/SlackNotificationPublisherTest.java b/notification/publishing/src/test/java/org/dependencytrack/notification/publishing/slack/SlackNotificationPublisherTest.java index 833c726b8e..0cafeb0d14 100644 --- a/notification/publishing/src/test/java/org/dependencytrack/notification/publishing/slack/SlackNotificationPublisherTest.java +++ b/notification/publishing/src/test/java/org/dependencytrack/notification/publishing/slack/SlackNotificationPublisherTest.java @@ -154,16 +154,10 @@ private void validatePolicyViolationNotificationPublish() { }, { "type": "section", - "fields": [ - { - "type": "mrkdwn", - "text": "*Message*" - }, - { - "type": "plain_text", - "text": "Package URL pkg:maven/foo/bar@1.2.3 is not allowed" - } - ] + "text": { + "type": "plain_text", + "text": "Package URL pkg:maven/foo/bar@1.2.3 is not allowed" + } }, { "type": "actions",