python: align annotations with Ruby

use `result=BAD` for expected alert
and `result=OK` on sinks where alerts are not wanted.

Author: yoff

python/ql/test/experimental/dataflow/TestUtil/DataflowQueryTest.qll

@@ -1,28 +1,43 @@
 import python
-import experimental.dataflow.TestUtil.FlowTest
+import semmle.python.dataflow.new.DataFlow
+import TestUtilities.InlineExpectationsTest
 private import semmle.python.dataflow.new.internal.PrintNode
 
-class DataFlowQueryTest extends FlowTest {
+class DataFlowQueryTest extends InlineExpectationsTest {
   DataFlowQueryTest() { this = "DataFlowQueryTest" }
 
-  override string flowTag() { result = "flow" }
+  override string getARelevantTag() { result = "result" }
 
-  override predicate relevantFlow(DataFlow::Node source, DataFlow::Node sink) {
-    exists(DataFlow::Configuration cfg | cfg.hasFlow(source, sink))
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    exists(DataFlow::Configuration cfg, DataFlow::Node fromNode, DataFlow::Node toNode |
+      cfg.hasFlow(fromNode, toNode)
+    |
+      location = toNode.getLocation() and
+      tag = "result" and
+      value = "BAD" and
+      element = toNode.toString()
+    )
   }
 }
 
 query predicate missingAnnotationOnSink(Location location, string error, string element) {
-  error = "ERROR, you should add `# $ MISSING: flow` annotation" and
+  error = "ERROR, you should add `# $ MISSING: result=BAD` or `result=OK` annotation" and
   exists(DataFlow::Node sink |
-    exists(DataFlow::Configuration cfg | cfg.isSink(sink)) and
+    exists(DataFlow::Configuration cfg | cfg.isSink(sink) or cfg.isSink(sink, _)) and
     location = sink.getLocation() and
     element = prettyExpr(sink.asExpr()) and
     not any(DataFlow::Configuration config).hasFlow(_, sink) and
     not exists(FalseNegativeExpectation missingResult |
-      missingResult.getTag() = "flow" and
+      missingResult.getTag() = "result" and
+      missingResult.getValue() = "BAD" and
       missingResult.getLocation().getFile() = location.getFile() and
       missingResult.getLocation().getStartLine() = location.getStartLine()
+    ) and
+    not exists(GoodExpectation okResult |
+      okResult.getTag() = "result" and
+      okResult.getValue() = "OK" and
+      okResult.getLocation().getFile() = location.getFile() and
+      okResult.getLocation().getStartLine() = location.getStartLine()
     )
   )
 }

python/ql/test/query-tests/Security/CWE-078-CommandInjection/command_injection.py

@@ -10,36 +10,36 @@
 def command_injection1():
     files = request.args.get('files', '')
     # Don't let files be `; rm -rf /`
-    os.system("ls " + files) # $flow="ImportMember, l:-8 -> BinaryExpr"
+    os.system("ls " + files) # $result=BAD
 
 
 @app.route("/command2")
 def command_injection2():
     files = request.args.get('files', '')
     # Don't let files be `; rm -rf /`
-    subprocess.Popen("ls " + files, shell=True) # $flow="ImportMember, l:-15 -> BinaryExpr"
+    subprocess.Popen("ls " + files, shell=True) # $result=BAD
 
 
 @app.route("/command3")
 def first_arg_injection():
     cmd = request.args.get('cmd', '')
-    subprocess.Popen([cmd, "param1"]) # $flow="ImportMember, l:-21 -> cmd"
+    subprocess.Popen([cmd, "param1"]) # $result=BAD
 
 
 @app.route("/other_cases")
 def others():
     files = request.args.get('files', '')
     # Don't let files be `; rm -rf /`
-    os.popen("ls " + files) # $flow="ImportMember, l:-28 -> BinaryExpr"
+    os.popen("ls " + files) # $result=BAD
 
 
 @app.route("/multiple")
 def multiple():
     command = request.args.get('command', '')
     # We should mark flow to both calls here, which conflicts with removing flow out of
     # a sink due to use-use flow.
-    os.system(command) # $flow="ImportMember, l:-36 -> command"
-    os.system(command) # $flow="ImportMember, l:-37 -> command"
+    os.system(command) # $result=BAD
+    os.system(command) # $result=BAD
 
 
 @app.route("/not-into-sink-impl")
@@ -52,11 +52,11 @@ def not_into_sink_impl():
     subprocess.call implementation: https://github.com/python/cpython/blob/fa7ce080175f65d678a7d5756c94f82887fc9803/Lib/subprocess.py#L341
     """
     command = request.args.get('command', '')
-    os.system(command) # $flow="ImportMember, l:-50 -> command"
-    os.popen(command) # $flow="ImportMember, l:-51 -> command"
-    subprocess.call(command) # $flow="ImportMember, l:-52 -> command"
-    subprocess.check_call(command) # $flow="ImportMember, l:-53 -> command"
-    subprocess.run(command) # $flow="ImportMember, l:-54 -> command"
+    os.system(command) # $result=BAD
+    os.popen(command) # $result=BAD
+    subprocess.call(command) # $result=BAD
+    subprocess.check_call(command) # $result=BAD
+    subprocess.run(command) # $result=BAD
 
 
 @app.route("/path-exists-not-sanitizer")
@@ -70,11 +70,11 @@ def path_exists_not_sanitizer():
     """
     path = request.args.get('path', '')
     if os.path.exists(path):
-        os.system("ls " + path) # $flow="ImportMember, l:-68 -> BinaryExpr"
+        os.system("ls " + path) # $result=BAD
 
 
 @app.route("/restricted-characters")
 def restricted_characters():
     path = request.args.get('path', '')
     if re.match(r'^[a-zA-Z0-9_-]+$', path):
-        os.system("ls " + path) # $SPURIOUS: flow="ImportMember, l:-75 -> BinaryExpr"
+        os.system("ls " + path) # $SPURIOUS: result=BAD