Revert "feat(taint): project-local sink wrappers are sinks where they are called"

This reverts commit 54c9dfc. The recall-pin check (p16) showed a large precision
and completion regression, although every recall target survived:
- MW WP Form: 3 -> 60 findings. Form-field renderers became sinks.
- WP Statistics: 16 -> 40.
- PMPro: 16 -> 65, 20 of them inside bundled Braintree/Stripe code. Completion fell
  to 390 of 1189 questions, because wrappers made many more regions look
  sink-bearing.
- YesWiki: completion fell from 2798/2807 to 216/2807.

The structural summary was too loose. Any identifier in a sink argument counted as
fed by a parameter if any assignment anywhere in the method mentioned it, with no
order or dominance. It also ignored vendored library code and cascaded three
levels by name. A redesign needs dataflow-confirmed, level-limited summaries that
leave the question count alone.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo

src/openultrasast/cpg/queries/taint.sc

@@ -72,7 +72,6 @@
   // Per-family answers that do not depend on the region, kept across the whole batch.
   val familyInRepoMemo = scala.collection.mutable.Map.empty[String, Boolean]
   val sinkCandidatesMemo = scala.collection.mutable.Map.empty[String, List[io.shiftleft.codepropertygraph.generated.nodes.Call]]
-  val wrapperMemo = scala.collection.mutable.Map.empty[String, Map[String, Set[Int]]]
   val reachableMemo = scala.collection.mutable.Map.empty[(String, String, Int), Set[String]]
   val methodSourceMemo = scala.collection.mutable.Map.empty[String, Boolean]
   val fedFieldsMemo = scala.collection.mutable.Map.empty[String, Map[String, String]]
@@ -1105,99 +1104,9 @@
   // for each of 2,487 requests was a large share of what made a request cost seconds -- in evidence mode,
   // which asks for no dataflow at all, it alone took a query past an hour. Scanned once per sink list and
   // kept for the batch; the per-request work is the scope filter over that list.
-  // ---- project-local sink WRAPPERS ------------------------------------------------------------------------
-  //
-  // Real code rarely calls the driver where it builds the query. YesWiki builds its INSERT in
-  // `FormManager::create($data)` and runs it through `$this->dbService->query($query)`, whose body is the
-  // `mysqli_query` the facts name -- so the one sink the facts knew sat in a helper no request ever reached
-  // directly, and the injectable value was never asked about where it entered. A method whose parameter
-  // feeds a family sink is itself a sink for that argument, reported where it is CALLED.
-  //
-  // Summarised STRUCTURALLY, not by dataflow: the parameter reaches the sink call's argument directly or
-  // through at most two local assignments, and at least one occurrence is outside every cleansing call. A
-  // flow query per parameter per method, repeated at every engine start of a portioned scan, is what this
-  // project's scale walls were made of. Up to three levels (a wrapper of a wrapper). Calls resolve by NAME,
-  // because an injected receiver (`$this->dbService`) has no type the frontend can see -- so a name counts
-  // only if every project method so named is a wrapper; one harmless `query()` elsewhere disqualifies it.
-  def underCleansing(node: io.shiftleft.codepropertygraph.generated.nodes.AstNode, root: io.shiftleft.codepropertygraph.generated.nodes.AstNode): Boolean = {
-    var current: Option[io.shiftleft.codepropertygraph.generated.nodes.AstNode] = Some(node)
-    var found = false
-    while (current.isDefined && !found && current.get.id != root.id) {
-      if (current.get.id != node.id && sanitizesHere(current.get, strictNames)) found = true
-      current = current.get._astIn.nextOption() match {
-        case Some(parent: io.shiftleft.codepropertygraph.generated.nodes.AstNode) => Some(parent)
-        case _                                                                     => None
-      }
-    }
-    found
-  }
-
-  def feeds(
-      method: io.shiftleft.codepropertygraph.generated.nodes.Method,
-      expression: io.shiftleft.codepropertygraph.generated.nodes.AstNode,
-      name: String,
-      depth: Int
-  ): Boolean =
-    depth <= 2 && {
-      val identifiers = expression.ast.isIdentifier.l
-      identifiers.exists(i => i.name == name && !underCleansing(i, expression)) ||
-      identifiers.map(_.name).distinct.filter(_ != name).exists { local =>
-        method.ast.isCall.nameExact(ASSIGNMENT).l.exists { assignment =>
-          val args = assignment.argument.l
-          args.find(_.argumentIndex == 1).exists {
-            case target: io.shiftleft.codepropertygraph.generated.nodes.Identifier => target.name == local
-            case _                                                                  => false
-          } && args.find(_.argumentIndex == 2).exists(value => feeds(method, value, name, depth + 1))
-        }
-      }
-    }
-
-  lazy val wrappers: Map[String, Set[Int]] =
-    wrapperMemo.getOrElseUpdate(
-      sinksS + "|" + sanitizersS + "|" + quotedSanitizersS, {
-        var known = Map.empty[String, Set[Int]]
-        var wrapperMethods = Set.empty[String]
-        var frontier = sinkCandidatesMemo.getOrElseUpdate(sinksS, cpg.call.filter(c => sinkNames.exists(n => sinkMatches(c, n))).l)
-        var level = 0
-        var growing = true
-        while (level < 3 && growing && frontier.nonEmpty) {
-          val found = frontier.groupBy(_.method).toList.flatMap { case (method, calls) =>
-            if (method.isExternal || method.name.startsWith("<")) Nil
-            else
-              method.parameter.l
-                .filter(p => p.index >= 1 && !objectBases.contains(p.name) && p.name != "self" && p.name != "this")
-                .filter(p => calls.exists(c => c.argument.argumentIndexGt(0).l.exists(a => feeds(method, a, p.name, 0))))
-                .map(p => (method, p.index))
-          }
-          val added = found.filterNot { case (m, index) => known.get(m.name).exists(_.contains(index)) }
-          growing = added.nonEmpty
-          added.foreach { case (m, index) =>
-            known = known.updated(m.name, known.getOrElse(m.name, Set.empty[Int]) + index)
-            wrapperMethods += m.fullName
-          }
-          val names = added.map(_._1.name).toSet
-          frontier = if (names.isEmpty) Nil else cpg.call.filter(c => !c.name.startsWith("<operator") && names.contains(c.name)).l
-          level += 1
-        }
-        known.filter { case (name, _) => cpg.method.nameExact(name).filterNot(_.isExternal).l.forall(m => wrapperMethods.contains(m.fullName)) }
-      }
-    )
-
-  def wrapperCall(c: io.shiftleft.codepropertygraph.generated.nodes.Call): Boolean =
-    !c.name.startsWith("<operator") && wrappers.contains(c.name) && !sinkNames.exists(n => sinkMatches(c, n))
-
-  // The arguments a flow must arrive through: every real argument of a fact sink, only the summarised ones of
-  // a wrapper.
-  def sinkTargets(c: io.shiftleft.codepropertygraph.generated.nodes.Call) =
-    if (wrapperCall(c)) c.argument.l.filter(a => wrappers(c.name).contains(a.argumentIndex))
-    else c.argument.argumentIndexGt(0).l
-
   def sinkCalls =
     sinkCandidatesMemo
-      .getOrElseUpdate("wrapped|" + sinksS + "|" + sanitizersS + "|" + quotedSanitizersS, {
+      .getOrElseUpdate(sinksS, cpg.call.filter(c => sinkNames.exists(n => sinkMatches(c, n))).l)
-        val facts = sinkCandidatesMemo.getOrElseUpdate(sinksS, cpg.call.filter(c => sinkNames.exists(n => sinkMatches(c, n))).l)
-        if (wrappers.isEmpty) facts else facts ++ cpg.call.filter(wrapperCall).l
-      })
       .filter(c => inScope(c.method))
 
   // ---- stage one of the two-stage join WITHOUT dataflow, for evidence mode (flow-aware-ranking, phase 2) --
@@ -1448,7 +1357,7 @@
     // no data at all, yet `res` is reachable from the request in any handler that captures it, so the flow
     // query answered yes about a call that receives nothing. Measured on NodeGoat, where `res.write(body)` is a
     // real finding and `res.end()` two lines below it was reported identically.
-    val flows = if (hasSources) sinkTargets(sink).reachableByFlows(sourceNodes).l.filter(plausible) else Nil
+    val flows = if (hasSources) sink.argument.argumentIndexGt(0).reachableByFlows(sourceNodes).l.filter(plausible) else Nil
 
     // The SHAPE of the sink call, which is what distinguishes a fix from a bug when the fix is a safe form
     // rather than a sanitizing call: `execute(sql, params)` binds where `execute(sql + x)` interpolates, and

tests/test_cpg_queries_compile.py

@@ -1417,56 +1417,3 @@ def test_an_allowlist_check_guards_its_branch_and_its_ternary_arm(tmp_path: Path
     payload = _taint_rows(tmp_path, "php2cpg", {"app.php": app}, {n: {**common, "function": n} for n in names})
     reported = {n for n in names if _unsanitized(payload, n)}
     assert reported == {"wrong_arm", "else_branch", "stored_in_else", "haystack"}, {n: payload[n] for n in names}
-
-
-_WRAPPED_PHP = (
-    "<?php\n"
-    "class DbService { public $link;\n"
-    "  function query( $query ) { return mysqli_query( $this->link, $query ); }\n"
-    "  function number( $n ) { return mysqli_query( $this->link, 'SELECT ' . intval( $n ) ); } }\n"
-    "class FormManager { public $dbService;\n"
-    "  function create( $data ) { $query = 'INSERT INTO t VALUES (' . $data['id'] . ')';\n"
-    "    return $this->dbService->query( $query ); } }\n"
-    "class FormController { public $formManager; public $dbService;\n"
-    "  function displayAll() { $value = $_POST['form']; return $this->formManager->create( $value ); }\n"
-    "  function direct() { return $this->dbService->query( 'SELECT ' . $_POST['q'] ); }\n"
-    "  function literal() { return $this->dbService->query( 'SELECT 1' ); }\n"
-    "  function cast() { return $this->dbService->number( $_POST['n'] ); } }\n"
-)
-
-
-def _wrapped_requests(spec, names):
-    common = {
-        "sources": ",".join(spec.sources),
-        "sinks": ",".join(spec.sinks),
-        "sanitizers": ",".join(spec.sanitizers),
-        "quotedSanitizers": ",".join(spec.quoted_sanitizers),
-        "file": "app.php",
-    }
-    return {n: {**common, "function": n} for n in names}
-
-
-@pytest.mark.skipif(_joern() is None, reason="joern is not installed on this machine")
-def test_a_project_sink_wrapper_is_a_sink_where_it_is_called(tmp_path: Path) -> None:
-    """YesWiki runs its INSERT through `$this->dbService->query($query)`, whose body is the `mysqli_query` the facts
-    name, and builds it in `FormManager::create($data)`. Neither receiver has a type the frontend can see. The
-    request value is reported where it enters a wrapper -- two levels up -- and not where it cannot."""
-    from openultrasast.model.specs import taint_specs
-
-    spec = taint_specs(language="php")["injection"]
-    names = ("displayAll", "direct", "literal", "cast")
-    payload = _taint_rows(tmp_path, "php2cpg", {"app.php": _WRAPPED_PHP}, _wrapped_requests(spec, names))
-    reported = {n: [r for r in _unsanitized(payload, n) if r.get("sinkMethod") == n] for n in names}
-    assert {n for n in names if reported[n]} == {"displayAll", "direct"}, {n: payload[n] for n in names}
-    assert "create" in str(reported["displayAll"][0]["sink"]), reported["displayAll"]
-
-
-@pytest.mark.skipif(_joern() is None, reason="joern is not installed on this machine")
-def test_a_wrapper_name_shared_with_a_harmless_method_is_not_a_sink(tmp_path: Path) -> None:
-    """Calls resolve by name, so a name is a sink only if every project method so named is a wrapper."""
-    from openultrasast.model.specs import taint_specs
-
-    spec = taint_specs(language="php")["injection"]
-    code = _WRAPPED_PHP + "class Cache { function query( $key ) { return strlen( $key ); } }\n"
-    payload = _taint_rows(tmp_path, "php2cpg", {"app.php": code}, _wrapped_requests(spec, ("direct",)))
-    assert not [r for r in _unsanitized(payload, "direct") if r.get("sinkMethod") == "direct"], payload["direct"]