Skip to content

Commit 9d147bc

Browse files
JarLobaschackmull
andauthored
Apply batched suggestions from code review
Co-authored-by: Anders Schack-Mulligen <aschackmull@users.noreply.github.com>
1 parent 43d485a commit 9d147bc

3 files changed

Lines changed: 8 additions & 23 deletions

File tree

actions/ql/lib/codeql/actions/security/UntrustedCheckoutQuery.qll

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,6 @@ class GhSHACheckout extends SHACheckoutStep instanceof Run {
386386
private predicate isRunCheckoutReference(
387387
PRHeadCheckoutStep checkout, Expression reference, string variable
388388
) {
389-
checkout instanceof Run and
390389
reference = checkout.(Run).getInScopeEnvVarExpr(variable) and
391390
(
392391
checkout instanceof SHACheckoutStep and containsHeadSHA(reference.getExpression())
@@ -405,13 +404,10 @@ private predicate isRunCheckoutReference(
405404

406405
/** Gets the expression that controls the untrusted checkout, if one can be identified. */
407406
AstNode getCheckoutReference(PRHeadCheckoutStep checkout) {
408-
exists(UsesStep uses |
409-
checkout = uses and
410-
(
411-
result = uses.getArgumentExpr("ref")
412-
or
413-
not exists(uses.getArgumentExpr("ref")) and result = uses.getArgumentExpr("repository")
414-
)
407+
exists(UsesStep uses | uses = checkout |
408+
result = uses.getArgumentExpr("ref")
409+
or
410+
not exists(uses.getArgumentExpr("ref")) and result = uses.getArgumentExpr("repository")
415411
)
416412
or
417413
isRunCheckoutReference(checkout, result, _)
@@ -430,9 +426,6 @@ string getCheckoutReferenceText(AstNode reference) {
430426

431427
/** Adds checkout-reference provenance before the checkout step in path queries. */
432428
predicate checkoutReferenceEdge(AstNode predecessor, AstNode successor) {
433-
exists(PRHeadCheckoutStep checkout |
434-
predecessor = getCheckoutReference(checkout) and
435-
successor = checkout and
436-
not predecessor = successor
437-
)
429+
predecessor = getCheckoutReference(successor) and
430+
not predecessor = successor
438431
}

actions/ql/src/Security/CWE-349/CachePoisoningViaPoisonableStep.ql

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,7 @@ import codeql.actions.security.PoisonableSteps
1919
import codeql.actions.security.ControlChecks
2020

2121
query predicate edges(AstNode predecessor, AstNode successor) {
22-
exists(Step previous, Step next |
23-
predecessor = previous and
24-
successor = next and
25-
previous.getNextStep() = next
26-
)
22+
predecessor.(Step).getNextStep() = successor
2723
or
2824
checkoutReferenceEdge(predecessor, successor)
2925
}

actions/ql/src/Security/CWE-829/UntrustedCheckoutCritical.ql

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,7 @@ import codeql.actions.security.PoisonableSteps
1919
import codeql.actions.security.ControlChecks
2020

2121
query predicate edges(AstNode predecessor, AstNode successor) {
22-
exists(Step previous, Step next |
23-
predecessor = previous and
24-
successor = next and
25-
previous.getNextStep() = next
26-
)
22+
predecessor.(Step).getNextStep() = successor
2723
or
2824
checkoutReferenceEdge(predecessor, successor)
2925
}

0 commit comments

Comments
 (0)