Skip to content

Commit 64731af

Browse files
fix(analyzers): rely on runner for example filtering in E5 and TM4 (#237)
E5 (#218) and TM4 (#220) called is_code_example() with an unconditional continue, letting a nearby example marker (e.g. "# for example") suppress findings in executable files. The shared runner already filters examples in non-executable docs and only downweights executables, so the analyzer-level call was redundant and created an attacker-controlled bypass — the same issue fixed for SC7 in #224. Remove it from both analyzers; replace TM4's analyze-level doc-exclusion test with an executable-evasion test and add the equivalent E5 regression. Signed-off-by: CharmingGroot <ohyes9711@gmail.com> Co-authored-by: Narendran Raghavan <32655573+rng1995@users.noreply.github.com>
1 parent d2f1832 commit 64731af

4 files changed

Lines changed: 27 additions & 17 deletions

File tree

‎src/skillspector/nodes/analyzers/static_patterns_data_exfiltration.py‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
get_context,
3333
get_context_from_lines,
3434
get_line_number,
35-
is_code_example,
3635
resolve_call_name,
3736
resolve_dotted_name,
3837
)
@@ -358,13 +357,9 @@ def ctx(start: int) -> str:
358357
matched_text=match.group(0)[:200],
359358
)
360359
)
361-
# E5: cloud-storage exfiltration. Filtered through is_code_example() because
362-
# upload calls commonly appear in SKILL.md docs and examples.
360+
# E5: cloud-storage exfiltration. Example filtering is delegated to the runner.
363361
for pattern, confidence in E5_PATTERNS:
364362
for match in re.finditer(pattern, content, re.IGNORECASE | re.MULTILINE):
365-
context = ctx(match.start())
366-
if is_code_example(context):
367-
continue
368363
line_num = get_line_number(content, match.start())
369364
findings.append(
370365
AnalyzerFinding(
@@ -374,7 +369,7 @@ def ctx(start: int) -> str:
374369
location=loc(line_num),
375370
confidence=confidence,
376371
tags=tag,
377-
context=context,
372+
context=ctx(match.start()),
378373
matched_text=match.group(0)[:200],
379374
)
380375
)

‎src/skillspector/nodes/analyzers/static_patterns_tool_misuse.py‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
from skillspector.state import AnalyzerNodeResponse, SkillspectorState
3333

3434
from . import static_runner
35-
from .common import get_context, get_line_number, is_code_example
35+
from .common import get_context, get_line_number
3636
from .pattern_defaults import PatternCategory
3737

3838
logger = get_logger(__name__)
@@ -322,13 +322,9 @@ def ctx(start: int) -> str:
322322
matched_text=match.group(0)[:200],
323323
)
324324
)
325-
# TM4: privileged K8s workload. Filtered through is_code_example() because
326-
# privileged/hostPath fields commonly appear in SKILL.md docs and examples.
325+
# TM4: privileged K8s workload. Example filtering is delegated to the runner.
327326
for pattern, confidence in TM4_PATTERNS:
328327
for match in re.finditer(pattern, content, re.IGNORECASE | re.MULTILINE):
329-
context_text = ctx(match.start())
330-
if is_code_example(context_text):
331-
continue
332328
line_num = get_line_number(content, match.start())
333329
findings.append(
334330
AnalyzerFinding(
@@ -338,7 +334,7 @@ def ctx(start: int) -> str:
338334
location=loc(line_num),
339335
confidence=confidence,
340336
tags=tag,
341-
context=context_text,
337+
context=ctx(match.start()),
342338
matched_text=match.group(0)[:200],
343339
)
344340
)

‎tests/nodes/analyzers/test_static_patterns.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,21 @@ def test_e5_benign_client_creation_no_finding(self):
417417
findings = static_runner.run_static_patterns(state, [data_exfiltration_module])
418418
assert not any(f.rule_id == "E5" for f in findings)
419419

420+
def test_e5_example_marker_in_executable_still_fires(self):
421+
"""An example marker near an upload in an executable .py must NOT suppress E5.
422+
423+
Example filtering belongs to the runner, which only downweights (does not
424+
skip) executables — so a nearby '# for example' cannot be used to evade E5.
425+
"""
426+
state = {
427+
"components": ["up.py"],
428+
"file_cache": {
429+
"up.py": "# for example\ns3.put_object(Bucket='x', Key='k', Body=d)",
430+
},
431+
}
432+
findings = static_runner.run_static_patterns(state, [data_exfiltration_module])
433+
assert any(f.rule_id == "E5" for f in findings)
434+
420435
def test_eval_dataset_prose_is_not_scanned_for_static_patterns(self):
421436
"""Eval datasets are test-case data, not installed skill code."""
422437
for dataset_path in ("evals/evals.json", "eval/dataset.yaml"):

‎tests/unit/test_patterns_new.py‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1316,9 +1316,13 @@ def test_tm4_benign_workload_not_flagged(self) -> None:
13161316
)
13171317
assert not any(f.rule_id == "TM4" for f in tm_mod.analyze(content, "ds.yaml", "yaml"))
13181318

1319-
def test_tm4_documentation_example_excluded(self) -> None:
1320-
content = "For example, never set privileged: true in your manifests."
1321-
assert not any(f.rule_id == "TM4" for f in tm_mod.analyze(content, "README.md", "markdown"))
1319+
def test_tm4_example_marker_not_self_filtered(self) -> None:
1320+
"""analyze() no longer self-filters on example markers — the shared runner
1321+
handles that (suppressing non-executable docs, only downweighting
1322+
executables). So a nearby '# for example' marker cannot bypass TM4; the
1323+
finding is still produced at the analyzer level."""
1324+
content = "# for example\nprivileged: true"
1325+
assert any(f.rule_id == "TM4" for f in tm_mod.analyze(content, "ds.yaml", "yaml"))
13221326

13231327
def test_safe_content_produces_no_findings(self) -> None:
13241328
findings = tm_mod.analyze(

0 commit comments

Comments
 (0)