Merge pull request github#10999 from RasmusWL/inline-fail-tag

RasmusWL · web-flow · commit 8628ff5e5222 · 2022-10-28T10:35:49.000+02:00
InlineExpectationsTest: Fail if missing `getARelevantTag`
diff --git a/cpp/ql/test/TestUtilities/InlineExpectationsTest.qll b/cpp/ql/test/TestUtilities/InlineExpectationsTest.qll
@@ -137,6 +137,7 @@ abstract class InlineExpectationsTest extends string {
   final predicate hasFailureMessage(FailureLocatable element, string message) {
     exists(ActualResult actualResult |
       actualResult.getTest() = this and
+      actualResult.getTag() = this.getARelevantTag() and
       element = actualResult and
       (
         exists(FalseNegativeExpectation falseNegative |
@@ -150,9 +151,18 @@ abstract class InlineExpectationsTest extends string {
       )
     )
     or
+    exists(ActualResult actualResult |
+      actualResult.getTest() = this and
+      not actualResult.getTag() = this.getARelevantTag() and
+      element = actualResult and
+      message =
+        "Tag mismatch: Actual result with tag '" + actualResult.getTag() +
+          "' that is not part of getARelevantTag()"
+    )
+    or
     exists(ValidExpectation expectation |
       not exists(ActualResult actualResult | expectation.matchesActualResult(actualResult)) and
-      expectation.getTag() = getARelevantTag() and
+      expectation.getTag() = this.getARelevantTag() and
       element = expectation and
       (
         expectation instanceof GoodExpectation and
diff --git a/csharp/ql/test/TestUtilities/InlineExpectationsTest.qll b/csharp/ql/test/TestUtilities/InlineExpectationsTest.qll
@@ -137,6 +137,7 @@ abstract class InlineExpectationsTest extends string {
   final predicate hasFailureMessage(FailureLocatable element, string message) {
     exists(ActualResult actualResult |
       actualResult.getTest() = this and
+      actualResult.getTag() = this.getARelevantTag() and
       element = actualResult and
       (
         exists(FalseNegativeExpectation falseNegative |
@@ -150,9 +151,18 @@ abstract class InlineExpectationsTest extends string {
       )
     )
     or
+    exists(ActualResult actualResult |
+      actualResult.getTest() = this and
+      not actualResult.getTag() = this.getARelevantTag() and
+      element = actualResult and
+      message =
+        "Tag mismatch: Actual result with tag '" + actualResult.getTag() +
+          "' that is not part of getARelevantTag()"
+    )
+    or
     exists(ValidExpectation expectation |
       not exists(ActualResult actualResult | expectation.matchesActualResult(actualResult)) and
-      expectation.getTag() = getARelevantTag() and
+      expectation.getTag() = this.getARelevantTag() and
       element = expectation and
       (
         expectation instanceof GoodExpectation and
diff --git a/go/ql/test/TestUtilities/InlineExpectationsTest.qll b/go/ql/test/TestUtilities/InlineExpectationsTest.qll
@@ -137,6 +137,7 @@ abstract class InlineExpectationsTest extends string {
   final predicate hasFailureMessage(FailureLocatable element, string message) {
     exists(ActualResult actualResult |
       actualResult.getTest() = this and
+      actualResult.getTag() = this.getARelevantTag() and
       element = actualResult and
       (
         exists(FalseNegativeExpectation falseNegative |
@@ -150,9 +151,18 @@ abstract class InlineExpectationsTest extends string {
       )
     )
     or
+    exists(ActualResult actualResult |
+      actualResult.getTest() = this and
+      not actualResult.getTag() = this.getARelevantTag() and
+      element = actualResult and
+      message =
+        "Tag mismatch: Actual result with tag '" + actualResult.getTag() +
+          "' that is not part of getARelevantTag()"
+    )
+    or
     exists(ValidExpectation expectation |
       not exists(ActualResult actualResult | expectation.matchesActualResult(actualResult)) and
-      expectation.getTag() = getARelevantTag() and
+      expectation.getTag() = this.getARelevantTag() and
       element = expectation and
       (
         expectation instanceof GoodExpectation and
diff --git a/java/ql/test/TestUtilities/InlineExpectationsTest.qll b/java/ql/test/TestUtilities/InlineExpectationsTest.qll
@@ -137,6 +137,7 @@ abstract class InlineExpectationsTest extends string {
   final predicate hasFailureMessage(FailureLocatable element, string message) {
     exists(ActualResult actualResult |
       actualResult.getTest() = this and
+      actualResult.getTag() = this.getARelevantTag() and
       element = actualResult and
       (
         exists(FalseNegativeExpectation falseNegative |
@@ -150,9 +151,18 @@ abstract class InlineExpectationsTest extends string {
       )
     )
     or
+    exists(ActualResult actualResult |
+      actualResult.getTest() = this and
+      not actualResult.getTag() = this.getARelevantTag() and
+      element = actualResult and
+      message =
+        "Tag mismatch: Actual result with tag '" + actualResult.getTag() +
+          "' that is not part of getARelevantTag()"
+    )
+    or
     exists(ValidExpectation expectation |
       not exists(ActualResult actualResult | expectation.matchesActualResult(actualResult)) and
-      expectation.getTag() = getARelevantTag() and
+      expectation.getTag() = this.getARelevantTag() and
       element = expectation and
       (
         expectation instanceof GoodExpectation and
diff --git a/java/ql/test/library-tests/frameworks/JaxWs/JakartaRs1.java b/java/ql/test/library-tests/frameworks/JaxWs/JakartaRs1.java
@@ -41,7 +41,7 @@ void Post() { // $ ResourceMethod ResourceMethodOnResourceClass
   @Produces("text/plain") // $ ProducesAnnotation=text/plain
   @DELETE
   double Delete() { // $ ResourceMethod=text/plain ResourceMethodOnResourceClass
-    return 0.0; // $ XssSink
+    return 0.0;
   }
 
   @Produces(MediaType.TEXT_HTML) // $ ProducesAnnotation=text/html
@@ -77,7 +77,7 @@ boolean Post() { // $ ResourceMethod=text/html ResourceMethodOnResourceClass
     @Produces(MediaType.TEXT_PLAIN) // $ ProducesAnnotation=text/plain
     @DELETE
     double Delete() { // $ ResourceMethod=text/plain ResourceMethodOnResourceClass
-      return 0.0; // $ XssSink
+      return 0.0;
     }
 
     @Path("")
diff --git a/java/ql/test/library-tests/frameworks/JaxWs/JaxRs.ql b/java/ql/test/library-tests/frameworks/JaxWs/JaxRs.ql
@@ -14,7 +14,7 @@ class JaxRsTest extends InlineExpectationsTest {
         "InjectionAnnotation", "ResponseDeclaration", "ResponseBuilderDeclaration",
         "ClientDeclaration", "BeanParamConstructor", "MessageBodyReaderDeclaration",
         "MessageBodyReaderReadFromCall", "MessageBodyReaderReadCall", "ProducesAnnotation",
-        "ConsumesAnnotation"
+        "ConsumesAnnotation", "XssSink"
       ]
   }
 
diff --git a/java/ql/test/library-tests/frameworks/JaxWs/JaxRs1.java b/java/ql/test/library-tests/frameworks/JaxWs/JaxRs1.java
@@ -41,7 +41,7 @@ void Post() { // $ ResourceMethod ResourceMethodOnResourceClass
   @Produces("text/plain") // $ ProducesAnnotation=text/plain
   @DELETE
   double Delete() { // $ ResourceMethod=text/plain ResourceMethodOnResourceClass
-    return 0.0; // $ XssSink
+    return 0.0;
   }
 
   @Produces(MediaType.TEXT_HTML) // $ ProducesAnnotation=text/html
@@ -77,7 +77,7 @@ boolean Post() { // $ ResourceMethod=text/html ResourceMethodOnResourceClass
     @Produces(MediaType.TEXT_PLAIN) // $ ProducesAnnotation=text/plain
     @DELETE
     double Delete() { // $ ResourceMethod=text/plain ResourceMethodOnResourceClass
-      return 0.0; // $ XssSink
+      return 0.0;
     }
 
     @Path("")
diff --git a/python/ql/test/TestUtilities/InlineExpectationsTest.qll b/python/ql/test/TestUtilities/InlineExpectationsTest.qll
@@ -137,6 +137,7 @@ abstract class InlineExpectationsTest extends string {
   final predicate hasFailureMessage(FailureLocatable element, string message) {
     exists(ActualResult actualResult |
       actualResult.getTest() = this and
+      actualResult.getTag() = this.getARelevantTag() and
       element = actualResult and
       (
         exists(FalseNegativeExpectation falseNegative |
@@ -150,9 +151,18 @@ abstract class InlineExpectationsTest extends string {
       )
     )
     or
+    exists(ActualResult actualResult |
+      actualResult.getTest() = this and
+      not actualResult.getTag() = this.getARelevantTag() and
+      element = actualResult and
+      message =
+        "Tag mismatch: Actual result with tag '" + actualResult.getTag() +
+          "' that is not part of getARelevantTag()"
+    )
+    or
     exists(ValidExpectation expectation |
       not exists(ActualResult actualResult | expectation.matchesActualResult(actualResult)) and
-      expectation.getTag() = getARelevantTag() and
+      expectation.getTag() = this.getARelevantTag() and
       element = expectation and
       (
         expectation instanceof GoodExpectation and
diff --git a/python/ql/test/library-tests/InlineExpectationsTest/missing-relevant-tag/Test.expected b/python/ql/test/library-tests/InlineExpectationsTest/missing-relevant-tag/Test.expected
@@ -0,0 +1,2 @@
+| test.py:1:1:1:3 | foo | Tag mismatch: Actual result with tag 'foo' that is not part of getARelevantTag() |
+| test.py:4:1:4:3 | foo | Tag mismatch: Actual result with tag 'foo' that is not part of getARelevantTag() |
diff --git a/python/ql/test/library-tests/InlineExpectationsTest/missing-relevant-tag/Test.ql b/python/ql/test/library-tests/InlineExpectationsTest/missing-relevant-tag/Test.ql
@@ -0,0 +1,20 @@
+// test to illustrate what happens if you forget to put in the
+// right values for `getARelevantTag`. We want to alert on this,
+// so it gets fixed!
+import python
+import TestUtilities.InlineExpectationsTest
+
+class MissingRelevantTag extends InlineExpectationsTest {
+  MissingRelevantTag() { this = "MissingRelevantTag" }
+
+  override string getARelevantTag() { none() }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    exists(Name name | name.getId() = "foo" |
+      location = name.getLocation() and
+      element = name.toString() and
+      value = "val" and
+      tag = "foo"
+    )
+  }
+}
diff --git a/python/ql/test/library-tests/InlineExpectationsTest/missing-relevant-tag/test.py b/python/ql/test/library-tests/InlineExpectationsTest/missing-relevant-tag/test.py
@@ -0,0 +1,7 @@
+foo # $ foo=val
+
+# with wrong value
+foo # $ foo=bad-value
+
+# there is a typo here, so this result is actually missing!
+fooo # $ foo=val
diff --git a/ql/ql/src/qlpack.yml b/ql/ql/src/qlpack.yml
@@ -6,4 +6,4 @@ suites: codeql-suites
 defaultSuiteFile: codeql-suites/ql-code-scanning.qls
 extractor: ql
 dependencies:
-  codeql/typos: 0.0.3-dev
+  codeql/typos: 0.0.4-dev
diff --git a/ql/ql/test/TestUtilities/InlineExpectationsTest.qll b/ql/ql/test/TestUtilities/InlineExpectationsTest.qll
@@ -137,6 +137,7 @@ abstract class InlineExpectationsTest extends string {
   final predicate hasFailureMessage(FailureLocatable element, string message) {
     exists(ActualResult actualResult |
       actualResult.getTest() = this and
+      actualResult.getTag() = this.getARelevantTag() and
       element = actualResult and
       (
         exists(FalseNegativeExpectation falseNegative |
@@ -150,9 +151,18 @@ abstract class InlineExpectationsTest extends string {
       )
     )
     or
+    exists(ActualResult actualResult |
+      actualResult.getTest() = this and
+      not actualResult.getTag() = this.getARelevantTag() and
+      element = actualResult and
+      message =
+        "Tag mismatch: Actual result with tag '" + actualResult.getTag() +
+          "' that is not part of getARelevantTag()"
+    )
+    or
     exists(ValidExpectation expectation |
       not exists(ActualResult actualResult | expectation.matchesActualResult(actualResult)) and
-      expectation.getTag() = getARelevantTag() and
+      expectation.getTag() = this.getARelevantTag() and
       element = expectation and
       (
         expectation instanceof GoodExpectation and
diff --git a/ruby/ql/test/TestUtilities/InlineExpectationsTest.qll b/ruby/ql/test/TestUtilities/InlineExpectationsTest.qll
@@ -137,6 +137,7 @@ abstract class InlineExpectationsTest extends string {
   final predicate hasFailureMessage(FailureLocatable element, string message) {
     exists(ActualResult actualResult |
       actualResult.getTest() = this and
+      actualResult.getTag() = this.getARelevantTag() and
       element = actualResult and
       (
         exists(FalseNegativeExpectation falseNegative |
@@ -150,9 +151,18 @@ abstract class InlineExpectationsTest extends string {
       )
     )
     or
+    exists(ActualResult actualResult |
+      actualResult.getTest() = this and
+      not actualResult.getTag() = this.getARelevantTag() and
+      element = actualResult and
+      message =
+        "Tag mismatch: Actual result with tag '" + actualResult.getTag() +
+          "' that is not part of getARelevantTag()"
+    )
+    or
     exists(ValidExpectation expectation |
       not exists(ActualResult actualResult | expectation.matchesActualResult(actualResult)) and
-      expectation.getTag() = getARelevantTag() and
+      expectation.getTag() = this.getARelevantTag() and
       element = expectation and
       (
         expectation instanceof GoodExpectation and
diff --git a/ruby/ql/test/library-tests/dataflow/api-graphs/use.ql b/ruby/ql/test/library-tests/dataflow/api-graphs/use.ql
@@ -20,7 +20,7 @@ class CustomEntryPointUse extends API::EntryPoint {
 class ApiUseTest extends InlineExpectationsTest {
   ApiUseTest() { this = "ApiUseTest" }
 
-  override string getARelevantTag() { result = ["use", "def"] }
+  override string getARelevantTag() { result = ["use", "def", "call"] }
 
   private predicate relevantNode(API::Node a, DataFlow::Node n, Location l, string tag) {
     l = n.getLocation() and
diff --git a/ruby/ql/test/query-tests/experimental/improper-memoization/ImproperMemoization.ql b/ruby/ql/test/query-tests/experimental/improper-memoization/ImproperMemoization.ql
@@ -5,7 +5,7 @@ import codeql.ruby.security.ImproperMemoizationQuery
 class ImproperMemoizationTest extends InlineExpectationsTest {
   ImproperMemoizationTest() { this = "ImproperMemoizationTest" }
 
-  override string getARelevantTag() { result = "BAD" }
+  override string getARelevantTag() { result = "result" }
 
   override predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "result" and
diff --git a/ruby/ql/test/query-tests/security/cwe-300/InsecureDependency.ql b/ruby/ql/test/query-tests/security/cwe-300/InsecureDependency.ql
@@ -5,7 +5,7 @@ import codeql.ruby.security.InsecureDependencyQuery
 class InsecureDependencyTest extends InlineExpectationsTest {
   InsecureDependencyTest() { this = "InsecureDependencyTest" }
 
-  override string getARelevantTag() { result = "BAD" }
+  override string getARelevantTag() { result = "result" }
 
   override predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "result" and
diff --git a/swift/ql/test/TestUtilities/InlineExpectationsTest.qll b/swift/ql/test/TestUtilities/InlineExpectationsTest.qll
@@ -137,6 +137,7 @@ abstract class InlineExpectationsTest extends string {
   final predicate hasFailureMessage(FailureLocatable element, string message) {
     exists(ActualResult actualResult |
       actualResult.getTest() = this and
+      actualResult.getTag() = this.getARelevantTag() and
       element = actualResult and
       (
         exists(FalseNegativeExpectation falseNegative |
@@ -150,9 +151,18 @@ abstract class InlineExpectationsTest extends string {
       )
     )
     or
+    exists(ActualResult actualResult |
+      actualResult.getTest() = this and
+      not actualResult.getTag() = this.getARelevantTag() and
+      element = actualResult and
+      message =
+        "Tag mismatch: Actual result with tag '" + actualResult.getTag() +
+          "' that is not part of getARelevantTag()"
+    )
+    or
     exists(ValidExpectation expectation |
       not exists(ActualResult actualResult | expectation.matchesActualResult(actualResult)) and
-      expectation.getTag() = getARelevantTag() and
+      expectation.getTag() = this.getARelevantTag() and
       element = expectation and
       (
         expectation instanceof GoodExpectation and

Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@ class JaxRsTest extends InlineExpectationsTest {`
`14`	`14`	`"InjectionAnnotation", "ResponseDeclaration", "ResponseBuilderDeclaration",`
`15`	`15`	`"ClientDeclaration", "BeanParamConstructor", "MessageBodyReaderDeclaration",`
`16`	`16`	`"MessageBodyReaderReadFromCall", "MessageBodyReaderReadCall", "ProducesAnnotation",`
`17`		`- "ConsumesAnnotation"`
	`17`	`+ "ConsumesAnnotation", "XssSink"`
`18`	`18`	`]`
`19`	`19`	`}`
`20`	`20`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+\| test.py:1:1:1:3 \| foo \| Tag mismatch: Actual result with tag 'foo' that is not part of getARelevantTag() \|`
	`2`	`+\| test.py:4:1:4:3 \| foo \| Tag mismatch: Actual result with tag 'foo' that is not part of getARelevantTag() \|`