Skip to content

Commit

Permalink
SONARPY-1158 Rename ExpressionTrace to ExpressionFlow (#1225)
Browse files Browse the repository at this point in the history
  • Loading branch information
nils-werner-sonarsource authored Sep 29, 2022
1 parent 45e6c67 commit eefcf96
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@
*/
package org.sonar.python.checks.cdk;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Deque;
import java.util.LinkedList;
import java.util.Optional;
import java.util.function.Predicate;
import org.sonar.plugins.python.api.PythonCheck;
Expand Down Expand Up @@ -68,12 +67,12 @@ public static Optional<CallExpression> getCall(Expression expression, String fqn
/**
* Resolve a particular argument of a call or get an empty optional if the argument is not set.
*/
protected static Optional<ExpressionTrace> getArgument(SubscriptionContext ctx, CallExpression callExpression, String argumentName) {
protected static Optional<ExpressionFlow> getArgument(SubscriptionContext ctx, CallExpression callExpression, String argumentName) {
return callExpression.arguments().stream()
.map(RegularArgument.class::cast)
.filter(regularArgument -> regularArgument.keywordArgument() != null)
.filter(regularArgument -> argumentName.equals(regularArgument.keywordArgument().name()))
.map(regularArgument -> ExpressionTrace.build(ctx, regularArgument.expression()))
.map(regularArgument -> ExpressionFlow.build(ctx, regularArgument.expression()))
.findAny();
}

Expand All @@ -85,43 +84,43 @@ public static Optional<ResolvedKeyValuePair> getKeyValuePair(SubscriptionContext
}

/**
* The expression trace reflects the complete flow of a value across the code.
* The expression flow represents the propagation of an expression.
* It serves as a resolution path from the use of the expression up to the value assignment.
* For example, if the value of an argument expression did not occur directly in the function call, the value can be traced back.
* The trace allows on the one hand to check the assigned value
* For example, if the value of an argument expression did not occur directly in the function call, the value can be tracked back.
* The flow allows on the one hand to check the assigned value
* and on the other hand to display the assignment path of the relevant value when creating an issue.
*/
static class ExpressionTrace {
static class ExpressionFlow {

private static final String TAIL_MESSAGE = "Propagated setting.";

private final SubscriptionContext ctx;
private final List<Expression> trace;
private final Deque<Expression> locations;

private ExpressionTrace(SubscriptionContext ctx, List<Expression> trace) {
private ExpressionFlow(SubscriptionContext ctx, Deque<Expression> locations) {
this.ctx = ctx;
this.trace = Collections.unmodifiableList(trace);
this.locations = locations;
}

protected static ExpressionTrace build(SubscriptionContext ctx, Expression expression) {
List<Expression> trace = new ArrayList<>();
buildTrace(expression, trace);
return new ExpressionTrace(ctx, trace);
protected static ExpressionFlow build(SubscriptionContext ctx, Expression expression) {
Deque<Expression> locations = new LinkedList<>();
resolveLocations(expression, locations);
return new ExpressionFlow(ctx, locations);
}

static void buildTrace(Expression expression, List<Expression> trace) {
trace.add(expression);
static void resolveLocations(Expression expression, Deque<Expression> locations) {
locations.add(expression);
if (expression.is(Tree.Kind.NAME)) {
Expression singleAssignedValue = Expressions.singleAssignedValue(((Name) expression));
if (singleAssignedValue != null && !trace.contains(singleAssignedValue)) {
buildTrace(singleAssignedValue, trace);
if (singleAssignedValue != null && !locations.contains(singleAssignedValue)) {
resolveLocations(singleAssignedValue, locations);
}
}
}

public void addIssue(String primaryMessage) {
PythonCheck.PreciseIssue issue = ctx.addIssue(trace.get(0).parent(), primaryMessage);
trace.stream().skip(1).forEach(expression -> issue.secondary(expression.parent(), TAIL_MESSAGE));
PythonCheck.PreciseIssue issue = ctx.addIssue(locations.getFirst().parent(), primaryMessage);
locations.stream().skip(1).forEach(expression -> issue.secondary(expression.parent(), TAIL_MESSAGE));
}

public void addIssueIf(Predicate<Expression> predicate, String primaryMessage) {
Expand All @@ -137,15 +136,15 @@ public void addIssueIf(Predicate<Expression> predicate, String primaryMessage, C
}

public boolean hasExpression(Predicate<Expression> predicate) {
return trace.stream().anyMatch(predicate);
return locations.stream().anyMatch(predicate);
}

public Optional<Expression> getExpression(Predicate<Expression> predicate) {
return trace.stream().filter(predicate).findFirst();
return locations.stream().filter(predicate).findFirst();
}

public List<Expression> trace() {
return trace;
public Deque<Expression> locations() {
return locations;
}
}

Expand All @@ -154,16 +153,16 @@ public List<Expression> trace() {
*/
static class ResolvedKeyValuePair {

final ExpressionTrace key;
final ExpressionTrace value;
final ExpressionFlow key;
final ExpressionFlow value;

private ResolvedKeyValuePair(ExpressionTrace key, ExpressionTrace value) {
private ResolvedKeyValuePair(ExpressionFlow key, ExpressionFlow value) {
this.key = key;
this.value = value;
}

static ResolvedKeyValuePair build(SubscriptionContext ctx, KeyValuePair pair) {
return new ResolvedKeyValuePair(ExpressionTrace.build(ctx, pair.key()), ExpressionTrace.build(ctx, pair.value()));
return new ResolvedKeyValuePair(ExpressionFlow.build(ctx, pair.key()), ExpressionFlow.build(ctx, pair.value()));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,9 @@ public static List<DictionaryLiteral> getDictionaryInList(SubscriptionContext ct
.collect(Collectors.toList());
}

private static List<CdkUtils.ExpressionTrace> getListElements(SubscriptionContext ctx, ListLiteral list) {
private static List<CdkUtils.ExpressionFlow> getListElements(SubscriptionContext ctx, ListLiteral list) {
return list.elements().expressions().stream()
.map(expression -> CdkUtils.ExpressionTrace.build(ctx, expression))
.map(expression -> CdkUtils.ExpressionFlow.build(ctx, expression))
.collect(Collectors.toList());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ protected void registerFqnConsumer() {
private static BiConsumer<SubscriptionContext, CallExpression> checkDomain(String encryptionArgName, String argFqnMethod, String engine) {
return (ctx, callExpression) -> getArgument(ctx, callExpression, encryptionArgName)
.ifPresentOrElse(
argEncryptedTrace -> argEncryptedTrace.addIssueIf(isSensitiveOptionObj(ctx, argFqnMethod).or(isSensitiveOptionDict(ctx)), unencryptedMessage(engine)),
flow -> flow.addIssueIf(isSensitiveOptionObj(ctx, argFqnMethod).or(isSensitiveOptionDict(ctx)), unencryptedMessage(engine)),
() -> ctx.addIssue(callExpression.callee(), omittingMessage(encryptionArgName, engine))
);
}
Expand Down Expand Up @@ -117,6 +117,6 @@ private static Predicate<KeyValuePair> isKey(String keyName) {
}

private static Predicate<KeyValuePair> isValueFalse(SubscriptionContext ctx) {
return keyValuePair -> CdkUtils.ExpressionTrace.build(ctx, keyValuePair.value()).hasExpression(isFalse());
return keyValuePair -> CdkUtils.ExpressionFlow.build(ctx, keyValuePair.value()).hasExpression(isFalse());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ protected void registerFqnConsumer() {
}

protected void checkDatabaseArguments(SubscriptionContext ctx, CallExpression resourceConstructor) {
Optional<CdkUtils.ExpressionTrace> argEncrypted = CdkUtils.getArgument(ctx, resourceConstructor, ARG_ENCRYPTED);
Optional<CdkUtils.ExpressionTrace> argEncryptionKey = CdkUtils.getArgument(ctx, resourceConstructor, ARG_ENCRYPTION_KEY);
Optional<CdkUtils.ExpressionFlow> argEncrypted = CdkUtils.getArgument(ctx, resourceConstructor, ARG_ENCRYPTED);
Optional<CdkUtils.ExpressionFlow> argEncryptionKey = CdkUtils.getArgument(ctx, resourceConstructor, ARG_ENCRYPTION_KEY);

if (argEncrypted.isEmpty() && argEncryptionKey.isEmpty()) {
ctx.addIssue(resourceConstructor.callee(), DB_OMITTING_MESSAGE);
Expand All @@ -69,14 +69,14 @@ protected void checkDatabaseArguments(SubscriptionContext ctx, CallExpression re

protected void checkCfnDatabaseArguments(SubscriptionContext ctx, CallExpression resourceConstructor) {
CdkUtils.getArgument(ctx, resourceConstructor, ARG_ENCRYPTED).ifPresentOrElse(
argumentTrace -> argumentTrace.addIssueIf(isFalse(), UNENCRYPTED_MESSAGE),
flow -> flow.addIssueIf(isFalse(), UNENCRYPTED_MESSAGE),
() -> ctx.addIssue(resourceConstructor.callee(), CFNDB_OMITTING_MESSAGE)
);
}

protected boolean isEngineAurora(SubscriptionContext ctx, CallExpression resourceConstructor) {
return CdkUtils.getArgument(ctx, resourceConstructor, "engine")
.filter(argumentTrace -> argumentTrace.hasExpression(startsWith("aurora"))).isPresent();
.filter(flow -> flow.hasExpression(startsWith("aurora"))).isPresent();
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public class S3BucketBlockPublicAccessCheck extends AbstractS3BucketCheck {
@Override
BiConsumer<SubscriptionContext, CallExpression> visitBucketConstructor() {
return (ctx, bucket) -> {
Optional<CdkUtils.ExpressionTrace> blockPublicAccess = getArgument(ctx, bucket, "block_public_access");
Optional<CdkUtils.ExpressionFlow> blockPublicAccess = getArgument(ctx, bucket, "block_public_access");
if (blockPublicAccess.isPresent()) {
checkBlockPublicAccess(ctx, blockPublicAccess.get());
} else {
Expand All @@ -60,9 +60,9 @@ BiConsumer<SubscriptionContext, CallExpression> visitBucketConstructor() {
};
}

private static void checkBlockPublicAccess(SubscriptionContext ctx, CdkUtils.ExpressionTrace blockPublicAccess) {
private static void checkBlockPublicAccess(SubscriptionContext ctx, CdkUtils.ExpressionFlow blockPublicAccess) {
blockPublicAccess.addIssueIf(S3BucketBlockPublicAccessCheck::blocksAclsOnly, MESSAGE);
blockPublicAccess.trace().stream().filter(CallExpression.class::isInstance).map(CallExpression.class::cast)
blockPublicAccess.locations().stream().filter(CallExpression.class::isInstance).map(CallExpression.class::cast)
.filter(S3BucketBlockPublicAccessCheck::isBlockPublicAccessConstructor)
.findAny()
.ifPresent(bpaConstructor -> visitBlockPublicAccessConstructor(ctx, bpaConstructor));
Expand All @@ -74,7 +74,7 @@ private static void visitBlockPublicAccessConstructor(SubscriptionContext ctx, C
.filter(Optional::isPresent)
.map(Optional::get)
.collect(Collectors.toList())
.forEach(argumentTrace -> argumentTrace.addIssueIf(isFalse(), MESSAGE));
.forEach(flow -> flow.addIssueIf(isFalse(), MESSAGE));
}

private static boolean blocksAclsOnly(Expression expression) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.sonar.plugins.python.api.SubscriptionContext;
import org.sonar.plugins.python.api.symbols.Symbol;
import org.sonar.plugins.python.api.tree.CallExpression;
import org.sonar.plugins.python.api.tree.Expression;
import org.sonar.plugins.python.api.tree.ImportFrom;
import org.sonar.plugins.python.api.tree.Name;
import org.sonar.plugins.python.api.tree.QualifiedExpression;
Expand Down Expand Up @@ -89,10 +88,8 @@ BiConsumer<SubscriptionContext, CallExpression> visitBucketConstructor() {
.ifPresent(argument -> argument.addIssueIf(CdkPredicate.isFqnOf(S3_BUCKET_SENSITIVE_POLICIES), getSensitivePolicyMessage(argument)));
}

private static String getSensitivePolicyMessage(CdkUtils.ExpressionTrace argumentTrace){
Expression lastExpression = argumentTrace.trace()
.get(argumentTrace.trace().size()-1);
String attribute = ((QualifiedExpression) lastExpression).name().name();
private static String getSensitivePolicyMessage(CdkUtils.ExpressionFlow flow){
String attribute = ((QualifiedExpression) flow.locations().getLast()).name().name();
return String.format(MESSAGE_POLICY, attribute);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public class UnencryptedEbsVolumeCheck extends AbstractCdkResourceCheck {
protected void registerFqnConsumer() {
checkFqn("aws_cdk.aws_ec2.Volume", (ctx, volume) ->
getArgument(ctx, volume, "encrypted").ifPresentOrElse(
argumentTrace -> argumentTrace.addIssueIf(isFalse(), PRIMARY_MESSAGE),
flow -> flow.addIssueIf(isFalse(), PRIMARY_MESSAGE),
() -> ctx.addIssue(volume.callee(), OMITTING_MESSAGE)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,17 @@ protected void registerFqnConsumer() {

protected void checkQueue(SubscriptionContext ctx, CallExpression resourceConstructor) {
getArgument(ctx, resourceConstructor, "encryption").ifPresentOrElse(
argumentTrace -> {
argumentTrace.addIssueIf(isFqn("aws_cdk.aws_sqs.QueueEncryption.UNENCRYPTED"), UNENCRYPTED_MESSAGE);
argumentTrace.addIssueIf(isNone(), NONE_MESSAGE);
flow -> {
flow.addIssueIf(isFqn("aws_cdk.aws_sqs.QueueEncryption.UNENCRYPTED"), UNENCRYPTED_MESSAGE);
flow.addIssueIf(isNone(), NONE_MESSAGE);
},
() -> ctx.addIssue(resourceConstructor.callee(), OMITTING_MESSAGE)
);
}

protected void checkCfnQueue(SubscriptionContext ctx, CallExpression resourceConstructor) {
getArgument(ctx, resourceConstructor, "kms_master_key_id").ifPresentOrElse(
argumentTrace -> argumentTrace.addIssueIf(isNone(), CFN_NONE_MESSAGE),
flow -> flow.addIssueIf(isNone(), CFN_NONE_MESSAGE),
() -> ctx.addIssue(resourceConstructor.callee(), CFN_OMITTING_MESSAGE)
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,20 +63,20 @@ protected void registerFqnConsumer() {

private static BiConsumer<SubscriptionContext, CallExpression> checkDomainName(Predicate<Expression> predicateIssue) {
return (ctx, callExpression) -> CdkUtils.getArgument(ctx, callExpression, "security_policy").ifPresent(
argTrace -> argTrace.addIssueIf(predicateIssue, ENFORCE_MESSAGE)
flow -> flow.addIssueIf(predicateIssue, ENFORCE_MESSAGE)
);
}

private static BiConsumer<SubscriptionContext, CallExpression> checkDomain(Predicate<Expression> predicateIssue) {
return (ctx, callExpression) -> CdkUtils.getArgument(ctx, callExpression, TLS_SECURITY_POLICY).ifPresentOrElse(
argTrace -> argTrace.addIssueIf(predicateIssue, ENFORCE_MESSAGE),
flow -> flow.addIssueIf(predicateIssue, ENFORCE_MESSAGE),
() -> ctx.addIssue(callExpression.callee(), OMITTING_MESSAGE)
);
}

private static BiConsumer<SubscriptionContext, CallExpression> checkCfnDomain(String domainOptionName) {
return (ctx, callExpression) -> CdkUtils.getArgument(ctx, callExpression, "domain_endpoint_options").ifPresentOrElse(
argTrace -> argTrace.addIssueIf(isSensitiveOptionObj(ctx, domainOptionName)
flow -> flow.addIssueIf(isSensitiveOptionObj(ctx, domainOptionName)
.or(isSensitiveDictionaryTls(ctx)), ENFORCE_MESSAGE),
() -> ctx.addIssue(callExpression.callee(), OMITTING_MESSAGE)
);
Expand Down

0 comments on commit eefcf96

Please sign in to comment.