Merge pull request #1678 from github/henrymercer/default-setup-safeguarding

Flag up functionality that may not exist in default setup workflows

lib/actions-util.js

@@ -490,7 +490,7 @@ async function isAnalyzingDefaultBranch() {
     const event = getWorkflowEvent();
     let defaultBranch = event?.repository?.default_branch;
     if (process.env.GITHUB_EVENT_NAME === "schedule") {
-        defaultBranch = removeRefsHeadsPrefix((0, util_1.getRequiredEnvParam)("GITHUB_REF"));
+        defaultBranch = removeRefsHeadsPrefix(getRefFromEnv());
     }
     return currentRef === defaultBranch;
 }

queries/codeql-pack.lock.yml

@@ -0,0 +1,14 @@
+---
+lockVersion: 1.0.0
+dependencies:
+  codeql-javascript:
+    version: 0.6.1
+  codeql/regex:
+    version: 0.0.12
+  codeql/tutorial:
+    version: 0.0.9
+  codeql/util:
+    version: 0.0.9
+  codeql/yaml:
+    version: 0.0.1
+compiled: false

queries/codeql-pack.yml

@@ -1,4 +1,4 @@
 name: codeql-action-custom-queries-javascript
 version: 0.0.0
-libraryPathDependencies: codeql-javascript
+dependencies:
-
+  codeql/javascript-all: 0.6.1

queries/default-setup-environment-variables.ql

@@ -0,0 +1,52 @@
+/**
+ * @name Some environment variables may not exist in default setup workflows
+ * @id javascript/codeql-action/default-setup-env-vars
+ * @kind problem
+ * @severity warning
+ */
+
+import javascript
+
+bindingset[envVar]
+predicate isSafeForDefaultSetup(string envVar) {
+  // Ignore internal Code Scanning environment variables
+  envVar.matches("CODE_SCANNING_%") or
+  envVar.matches("CODEQL_%") or
+  envVar.matches("CODESCANNING_%") or
+  envVar.matches("LGTM_%") or
+  // We flag up usage of potentially unsafe parts of the GitHub event in `default-setup-event-context.ql`.
+  envVar = "GITHUB_EVENT_PATH" or
+  // The following environment variables are known to be safe for use with default setup
+  envVar =
+    [
+      "GITHUB_ACTION_REF", "GITHUB_ACTION_REPOSITORY", "GITHUB_ACTOR", "GITHUB_API_URL",
+      "GITHUB_BASE_REF", "GITHUB_EVENT_NAME", "GITHUB_JOB", "GITHUB_RUN_ATTEMPT", "GITHUB_RUN_ID",
+      "GITHUB_SHA", "GITHUB_REPOSITORY", "GITHUB_SERVER_URL", "GITHUB_TOKEN", "GITHUB_WORKFLOW",
+      "GITHUB_WORKSPACE", "GOFLAGS", "JAVA_TOOL_OPTIONS", "RUNNER_ARCH", "RUNNER_NAME", "RUNNER_OS",
+      "RUNNER_TEMP", "RUNNER_TOOL_CACHE"
+    ]
+}
+
+predicate envVarRead(DataFlow::Node node, string envVar) {
+  node =
+    any(DataFlow::PropRead read |
+      read = NodeJSLib::process().getAPropertyRead("env").getAPropertyRead() and
+      envVar = read.getPropertyName()
+    ) or
+  node =
+    any(DataFlow::CallNode call |
+      call.getCalleeName().matches("get%EnvParam") and
+      envVar = call.getArgument(0).getStringValue()
+    )
+}
+
+from DataFlow::Node read, string envVar
+where
+  envVarRead(read, envVar) and
+  not isSafeForDefaultSetup(envVar)
+select read,
+  "The environment variable " + envVar +
+    " may not exist in default setup workflows. If all uses are safe, add it to the list of " +
+    "environment variables that are known to be safe in " +
+    "'queries/default-setup-environment-variables.ql'. If this use is safe but others are not, " +
+    "dismiss this alert as a false positive."

queries/default-setup-event-context.ql

@@ -0,0 +1,58 @@
+/**
+ * @name Some context properties may not exist in default setup workflows
+ * @id javascript/codeql-action/default-setup-context-properties
+ * @kind path-problem
+ * @severity warning
+ */
+
+import javascript
+import DataFlow::PathGraph
+
+class NotParsedLabel extends DataFlow::FlowLabel {
+  NotParsedLabel() { this = "not-parsed" }
+}
+
+class ParsedLabel extends DataFlow::FlowLabel {
+  ParsedLabel() { this = "parsed" }
+}
+
+class EventContextAccessConfiguration extends DataFlow::Configuration {
+  EventContextAccessConfiguration() { this = "EventContextAccessConfiguration" }
+
+  override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel lbl) {
+    source = NodeJSLib::process().getAPropertyRead("env").getAPropertyRead("GITHUB_EVENT_PATH") and
+    lbl instanceof NotParsedLabel
+  }
+
+  override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel lbl) {
+    sink instanceof DataFlow::PropRead and
+    lbl instanceof ParsedLabel and
+    not exists(DataFlow::PropRead n | sink = n.getBase()) and
+    not sink.asExpr().getFile().getBaseName().matches("%.test.ts")
+  }
+
+  override predicate isAdditionalFlowStep(
+    DataFlow::Node src, DataFlow::Node trg, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl
+  ) {
+    src = trg.(FileSystemReadAccess).getAPathArgument() and inlbl = outlbl
+    or
+    exists(JsonParserCall c |
+      src = c.getInput() and
+      trg = c.getOutput() and
+      inlbl instanceof NotParsedLabel and
+      outlbl instanceof ParsedLabel
+    )
+    or
+    (
+      TaintTracking::sharedTaintStep(src, trg) or
+      DataFlow::SharedFlowStep::step(src, trg) or
+      DataFlow::SharedFlowStep::step(src, trg, _, _)
+    ) and
+    inlbl = outlbl
+  }
+}
+
+from EventContextAccessConfiguration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasFlowPath(source, sink)
+select sink.getNode(), source, sink,
+  "This event context property may not exist in default setup workflows."

queries/inconsistent-action-input.ql

@@ -4,7 +4,7 @@
  *   must be defined in an identical way to avoid confusion for the user.
  *   This also makes writing queries like required-action-input.ql easier.
  * @kind problem
- * @problem.severity error
+ * @severity error
  * @id javascript/codeql-action/inconsistent-action-input
  */
 
@@ -15,7 +15,9 @@ import javascript
  */
 class ActionDeclaration extends File {
   ActionDeclaration() {
-    getRelativePath().matches("%/action.yml")
+    getRelativePath().matches("%/action.yml") and
+    // Ignore internal Actions
+    not getRelativePath().matches(".github/actions/%")
   }
 
   /**
@@ -25,19 +27,19 @@ class ActionDeclaration extends File {
     result = getRelativePath().regexpCapture("(.*)/action.yml", 1)
   }
 
-  YAMLDocument getRootNode() {
+  YamlDocument getRootNode() {
     result.getFile() = this
   }
 
-  YAMLValue getInput(string inputName) {
+  YamlValue getInput(string inputName) {
-    result = getRootNode().(YAMLMapping).lookup("inputs").(YAMLMapping).lookup(inputName)
+    result = getRootNode().(YamlMapping).lookup("inputs").(YamlMapping).lookup(inputName)
   }
 }
 
-predicate areNotEquivalent(YAMLValue x, YAMLValue y) {
+predicate areNotEquivalent(YamlValue x, YamlValue y) {
   x.getTag() != y.getTag()
   or
-  x.(YAMLScalar).getValue() != y.(YAMLScalar).getValue()
+  x.(YamlScalar).getValue() != y.(YamlScalar).getValue()
   or
   x.getNumChild() != y.getNumChild()
   or

src/actions-util.ts

@@ -648,7 +648,7 @@ export async function isAnalyzingDefaultBranch(): Promise<boolean> {
   let defaultBranch = event?.repository?.default_branch;
 
   if (process.env.GITHUB_EVENT_NAME === "schedule") {
-    defaultBranch = removeRefsHeadsPrefix(getRequiredEnvParam("GITHUB_REF"));
+    defaultBranch = removeRefsHeadsPrefix(getRefFromEnv());
   }
 
   return currentRef === defaultBranch;