Skip to content

Commit b633394

Browse files
committed
Address CodeQL review suggestions
1 parent 05933be commit b633394

1 file changed

Lines changed: 13 additions & 11 deletions

File tree

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

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ private string getUnnormalizedLocalScriptPath(LocalScriptExecutionRunStep step)
4141

4242
private class ExecutionPathInput extends NormalizableFilepath {
4343
ExecutionPathInput() {
44-
exists(LocalScriptExecutionRunStep step |
45-
this = trimQuotes(getUnnormalizedLocalScriptPath(step))
44+
exists(LocalScriptExecutionRunStep |
45+
this = trimQuotes(getUnnormalizedLocalScriptPath(_))
4646
)
4747
or
4848
exists(LocalActionUsesStep step | this = step.getCallee())
@@ -109,19 +109,21 @@ where
109109
// the checkout is followed by a known poisonable step
110110
checkout.getAFollowingStep() = poisonable and
111111
(
112-
poisonable instanceof Run and
112+
// Check if the poisonable step is a local script execution step
113+
// and the path of the command or script matches the path of the downloaded artifact
113114
(
114-
// Check if the poisonable step is a local script execution step
115-
// and the path of the command or script matches the path of the downloaded artifact
115+
poisonable instanceof LocalScriptExecutionRunStep and
116116
checkoutContainsPath(checkout,
117-
getUnnormalizedLocalScriptPath(poisonable.(LocalScriptExecutionRunStep)),
118-
poisonable.(LocalScriptExecutionRunStep).getPath())
119-
or
120-
// Checking the path for non local script execution steps is very difficult
117+
getUnnormalizedLocalScriptPath(poisonable), poisonable.getPath())
118+
)
119+
or
120+
// Checking the path for non local script execution steps is very difficult
121+
(
122+
poisonable instanceof Run and
121123
not poisonable instanceof LocalScriptExecutionRunStep
122-
// Its not easy to extract the path from a non-local script execution step so skipping this check for now
123-
// and isSubpath(poisonable.(Run).getWorkingDirectory(), checkout.getPath())
124124
)
125+
// Its not easy to extract the path from a non-local script execution step so skipping this check for now
126+
// and isSubpath(poisonable.(Run).getWorkingDirectory(), checkout.getPath())
125127
or
126128
poisonable instanceof UsesStep and
127129
(

0 commit comments

Comments
 (0)