Skip to content

Commit d5fed3d

Browse files
ziadhanyziadhany
committed
Add an unidiff dependency to pyproject.toml file
Fix the test Signed-off-by: ziad hany <ziadhany2016@gmail.com>
1 parent 983afd1 commit d5fed3d

3 files changed

Lines changed: 92 additions & 112 deletions

File tree

‎pyproject.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ dependencies = [
7878
"openpyxl==3.1.5",
7979
"requests==2.33.1",
8080
"GitPython==3.1.46",
81+
"unidiff==0.7.5",
8182
# Profiling
8283
"pyinstrument==5.1.2",
8384
# CycloneDX

‎scanpipe/pipes/reachability.py‎

Lines changed: 18 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,14 @@ class ReachabilityStatus(str, Enum):
5555
def api_mocker():
5656
"""TODO: Remove this once the API patch url is done"""
5757
return [
58-
# {
59-
# "vcs_url": "https://github.com/pallets/flask",
60-
# "commit_hash": "089cb86dd22bff589a4eafb7ab8e42dc357623b4",
61-
# },
6258
{
63-
"vcs_url": "https://github.com/aio-libs/aiohttp",
64-
"commit_hash": "0c2e9da51126238a421568eb7c5b53e5b5d17b36",
65-
}
59+
"vcs_url": "https://github.com/pallets/flask",
60+
"commit_hash": "089cb86dd22bff589a4eafb7ab8e42dc357623b4",
61+
},
62+
# {
63+
# "vcs_url": "https://github.com/aio-libs/aiohttp",
64+
# "commit_hash": "0c2e9da51126238a421568eb7c5b53e5b5d17b36",
65+
# }
6666
]
6767

6868

@@ -245,6 +245,7 @@ def get_changed_lines(diff_text, file_path):
245245

246246
return removed, added
247247

248+
248249
def diff_changed_symbols(vuln_meta, fixed_meta):
249250
"""
250251
Keep only symbols whose body actually differs between vulnerable and fixed
@@ -382,10 +383,8 @@ def collect_and_store_symbol_reachability_results(project, logger=None):
382383
vcs_url = patch["vcs_url"]
383384
commit_hash = patch["commit_hash"]
384385
try:
385-
# repo_path = clone_repo(vcs_url, commit_hash)
386-
# repo = Repo("/home/ziad-hany/PycharmProjects/flask/")
387-
repo = Repo("/home/ziad-hany/PycharmProjects/aiohttp")
388-
386+
repo_path = clone_repo(vcs_url, commit_hash)
387+
repo = Repo(repo_path)
389388
patch_symbols_by_language = collect_patch_symbols(repo, commit_hash)
390389

391390
if not patch_symbols_by_language:
@@ -413,6 +412,7 @@ def collect_and_store_symbol_reachability_results(project, logger=None):
413412
patch_symbols["vulnerable"],
414413
resource_index,
415414
)
415+
416416
fixed_evidence = match_symbols_against_resource(
417417
patch_symbols["fixed"],
418418
resource_index,
@@ -447,6 +447,7 @@ def collect_and_store_symbol_reachability_results(project, logger=None):
447447
# cleanup_repo(repo_path)
448448
pass
449449

450+
450451
def build_resource_index(resource_text, language):
451452
if not is_supported_language(language) or not resource_text:
452453
return None
@@ -484,8 +485,6 @@ def match_symbols_against_resource(patch_symbols_metadata, resource_index):
484485

485486
call_graph = resource_index.get("call_graph") or {}
486487
imports = call_graph.get("imports", {})
487-
488-
# Set of fully-qualified names the resource imports, e.g. "aiohttp.ClientSession"
489488
imported_fq_names = set(imports.values())
490489

491490
target_qualified_names = {
@@ -511,9 +510,6 @@ def match_symbols_against_resource(patch_symbols_metadata, resource_index):
511510
fingerprint and fingerprint in resource_index.get("fingerprints", set())
512511
)
513512

514-
# Does the resource *import* this symbol?
515-
# Match either the bare name (import key) or any fq import target
516-
# that ends with ".<qualified_name>".
517513
imported = (
518514
qualified_name in imports
519515
or qualified_name in imported_fq_names
@@ -539,7 +535,6 @@ def match_symbols_against_resource(patch_symbols_metadata, resource_index):
539535
"imported": False,
540536
"fingerprint": None,
541537
"reachable_from": [],
542-
"external": False,
543538
},
544539
)
545540

@@ -548,14 +543,10 @@ def match_symbols_against_resource(patch_symbols_metadata, resource_index):
548543

549544
if imported:
550545
entry["imported"] = True
551-
if not defined:
552-
entry["external"] = True
553546

554547
if called:
555548
entry["called"] = True
556549
entry["reachable_from"] = sorted(reachable_callers)
557-
if not defined:
558-
entry["external"] = True
559550

560551
if fingerprint_hit:
561552
entry["fingerprint"] = fingerprint
@@ -572,14 +563,14 @@ def classify_reachability(evidence):
572563
for item in evidence.values():
573564
is_called = bool(item.get("called"))
574565
has_path = bool(item.get("reachable_from"))
575-
is_exact = "exact_match_fingerprint" in item
576566
is_defined = bool(item.get("defined"))
577567
is_imported = bool(item.get("imported"))
568+
is_exact = bool(item.get("fingerprint"))
578569

579-
if is_called or has_path or is_imported:
570+
if is_exact or (is_imported and (is_called or has_path)):
580571
return ReachabilityStatus.REACHABLE
581572

582-
if is_exact or is_defined:
573+
if (is_imported or is_defined) and not is_exact:
583574
highest_status = ReachabilityStatus.POTENTIALLY_REACHABLE
584575

585576
return highest_status
@@ -839,12 +830,14 @@ def compute_reachable_symbols(call_graph, target_qualified_names):
839830

840831
def collect_imports(root_node, language: str):
841832
"""
842-
Returns a dict mapping local names/aliases to their absolute import path.
833+
Return a dict mapping local names/aliases to their absolute import path.
834+
843835
Examples:
844836
'from django.db import models' -> {'models': 'django.db.models'}
845837
'import os.path' -> {'os.path': 'os.path'}
846838
'import numpy as np' -> {'np': 'numpy'}
847839
'from a.b import c as d' -> {'d': 'a.b.c'}
840+
848841
"""
849842
import_map = {}
850843
query = get_query(language, "imports")

‎scanpipe/tests/pipes/test_symbols_reachability.py‎

Lines changed: 73 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,15 @@
2727

2828
from scanpipe.models import Project
2929
from scanpipe.pipes import collect_and_create_codebase_resources
30-
from scanpipe.pipes.reachability import ReachabilityStatus, collect_imports, extract_direct_calls
30+
from scanpipe.pipes.reachability import ReachabilityStatus
3131
from scanpipe.pipes.reachability import analyze_patched_file
3232
from scanpipe.pipes.reachability import build_symbol_metadata
3333
from scanpipe.pipes.reachability import classify_reachability
3434
from scanpipe.pipes.reachability import collect_and_store_symbol_reachability_results
35+
from scanpipe.pipes.reachability import collect_imports
3536
from scanpipe.pipes.reachability import compute_reachable_symbols
3637
from scanpipe.pipes.reachability import diff_changed_symbols
38+
from scanpipe.pipes.reachability import extract_direct_calls
3739
from scanpipe.pipes.reachability import get_changed_lines
3840
from scanpipe.pipes.symbols import collect_definitions
3941
from scanpipe.pipes.symbols import extract_definitions
@@ -104,28 +106,25 @@ def test_collect_and_store_symbol_reachability_results(
104106

105107
self.assertEqual(
106108
results,
107-
[
108-
{
109-
"patch": {
110-
"vcs_url": "https://github.com/aboutcode-org/test",
111-
"commit_hash": "07ec0de1964b14bf085a1c9a27ece2b61ab6105c",
112-
},
113-
"summary": {"call_paths": {}},
114-
"evidence": {
115-
"serve_report": {
116-
"called": False,
117-
"defined": True,
118-
"reachable_from": [],
119-
"exact_match_fingerprint": (
120-
"e341b914f9823915e0685396a730d421ec9e3635"
121-
),
122-
}
123-
},
124-
"fixed_symbols": ["serve_report"],
125-
"vulnerable_symbols": ["serve_report"],
126-
"reachability_status": "POTENTIALLY_REACHABLE",
127-
}
128-
],
109+
{
110+
"patch": {
111+
"vcs_url": "https://github.com/aboutcode-org/test",
112+
"commit_hash": "07ec0de1964b14bf085a1c9a27ece2b61ab6105c",
113+
},
114+
"evidence": {
115+
"serve_report": {
116+
"called": False,
117+
"defined": True,
118+
"imported": False,
119+
"fingerprint": "d7675efb263896da2a3c0067951183"
120+
"3553907e7e6ea619115a6dfc8625c3457e",
121+
"reachable_from": [],
122+
}
123+
},
124+
"fixed_symbols": ["serve_report"],
125+
"vulnerable_symbols": ["serve_report"],
126+
"reachability_status": "REACHABLE",
127+
},
129128
)
130129

131130
def test_extract_definitions(self):
@@ -210,35 +209,20 @@ def test_classify_reachability(self):
210209
self.assertEqual(classify_reachability(None), ReachabilityStatus.NOT_REACHABLE)
211210
self.assertEqual(classify_reachability({}), ReachabilityStatus.NOT_REACHABLE)
212211
self.assertEqual(
213-
classify_reachability(
214-
{"sym1": {"exact_match_fingerprint": "hash123", "called": True}}
215-
),
212+
classify_reachability({"evidence": {"fingerprint": "hash123"}}),
216213
ReachabilityStatus.REACHABLE,
217214
)
218215

219216
self.assertEqual(
220-
classify_reachability(
221-
{
222-
"sym1": {
223-
"called": True,
224-
"reachable_from": ["main_function", "api_handler"],
225-
}
226-
}
227-
),
217+
classify_reachability({"evidence": {"imported": True, "called": True}}),
228218
ReachabilityStatus.REACHABLE,
229219
)
230220
self.assertEqual(
231-
classify_reachability({"sym1": {"defined": True, "called": False}}),
232-
ReachabilityStatus.POTENTIALLY_REACHABLE,
233-
)
234-
self.assertEqual(
235-
classify_reachability(
236-
{"sym1": {"exact_match_fingerprint": "hash123", "called": False}}
237-
),
221+
classify_reachability({"evidence": {"imported": True, "called": False}}),
238222
ReachabilityStatus.POTENTIALLY_REACHABLE,
239223
)
240224
self.assertEqual(
241-
classify_reachability({"sym1": {"file_path": "src/vulnerable.py"}}),
225+
classify_reachability({"evidence": {"imported": False, "called": False}}),
242226
ReachabilityStatus.NOT_REACHABLE,
243227
)
244228

@@ -275,15 +259,17 @@ def process_data(payload):
275259
" def inner_helper():\n"
276260
" return True\n"
277261
" return payload.strip()",
278-
"fingerprint": "0000000888014a04b037189a42b238a2c50f218c",
262+
"fingerprint": "b0d0ad9a92209a6d79b84e932ce302"
263+
"a8bc9054a405131adf7dc21e06e2e7c0c1",
279264
"start_line": 3,
280265
"end_line": 6,
281266
"node_type": "function_definition",
282267
},
283268
"process_data": {
284269
"qualified_name": "process_data",
285270
"text": "def process_data(payload):\n return payload",
286-
"fingerprint": "000000022020300e882a900807880d0300010000",
271+
"fingerprint": "9b2797712c9ab60ea8452a441396"
272+
"5c94d1b2f63739cab7de695e7b1dc0cf439a",
287273
"start_line": 9,
288274
"end_line": 10,
289275
"node_type": "function_definition",
@@ -373,26 +359,24 @@ def test_analyze_patched_file(self):
373359
{
374360
"serve_report": {
375361
"qualified_name": "serve_report",
376-
"text": "def serve_report(request_payload):\n "
377-
' """Top-level function handling a request."""\n '
378-
' generator = ReportGenerator("/var/reports")\n '
379-
' requested_file = request_payload.get("file")\n\n '
380-
"# Helper function nested inside serve_report\n "
381-
"def build_file_path(filename):\n "
382-
" # VULNERABLE: Direct concatenation allows Path Traversal\n "
383-
' # An attacker passing "../../etc/passwd" '
384-
"could read system files.\n "
385-
" return os.path.join(generator.base_dir, filename)\n\n "
386-
" if not requested_file:\n "
387-
' return "Error: No file specified"\n\n '
388-
" target_path = build_file_path(requested_file)\n\n"
389-
" "
390-
" "
391-
"if os.path.exists(target_path):\n"
392-
" "
393-
' return f"Serving content of {target_path}"\n\n '
394-
' return "Error: File not found"',
395-
"fingerprint": "000000556d322a47595af353274b000aa324e014",
362+
"text": "def serve_report(request_payload):\n"
363+
' """Top-level function handling a request."""\n'
364+
' generator = ReportGenerator("/var/reports")\n'
365+
' requested_file = request_payload.get("file")\n\n'
366+
" # Helper function nested inside serve_report\n"
367+
" def build_file_path(filename):\n"
368+
" # VULNERABLE: Direct concatenation allows Path Traversal\n"
369+
' # An attacker passing "../../etc/passwd"'
370+
" could read system files.\n"
371+
" return os.path.join(generator.base_dir, filename)\n\n"
372+
" if not requested_file:\n"
373+
' return "Error: No file specified"\n\n'
374+
" target_path = build_file_path(requested_file)\n\n"
375+
" if os.path.exists(target_path):\n"
376+
' return f"Serving content of {target_path}"\n\n'
377+
' return "Error: File not found"',
378+
"fingerprint": "d7675efb263896da2a3c0067951183"
379+
"3553907e7e6ea619115a6dfc8625c3457e",
396380
"start_line": 11,
397381
"end_line": 30,
398382
"node_type": "function_definition",
@@ -405,27 +389,30 @@ def test_analyze_patched_file(self):
405389
{
406390
"serve_report": {
407391
"qualified_name": "serve_report",
408-
"text": "def serve_report(request_payload):\n "
409-
' """Top-level function handling a request."""\n '
410-
' generator = ReportGenerator("/var/reports")\n '
411-
' requested_file = request_payload.get("file")\n\n '
412-
" # Helper function nested inside serve_report\n "
413-
" def build_file_path(filename):\n "
414-
" # FIXED: Validate that the resolved "
415-
"path stays within the base_dir\n "
416-
" base = os.path.abspath(generator.base_dir)\n "
417-
" target = os.path.abspath(os.path.join(base, filename))\n "
418-
" if not target.startswith(base):\n "
419-
' raise ValueError("Path Traversal Detected")\n '
420-
" return target\n\n if not requested_file:\n "
421-
' return "Error: No file specified"\n\n try:\n '
422-
" target_path = build_file_path(requested_file)\n "
423-
" except ValueError:\n "
424-
' return "Error: Invalid path"\n\n'
425-
" if os.path.exists(target_path):\n "
426-
' return f"Serving content of {target_path}"\n\n '
427-
' return "Error: File not found"',
428-
"fingerprint": "0000006cceea8aedf1da91830f67b64927086d24",
392+
"text": "def serve_report(request_payload):\n"
393+
' """Top-level function handling a request."""\n'
394+
' generator = ReportGenerator("/var/reports")\n'
395+
' requested_file = request_payload.get("file")\n\n'
396+
" # Helper function nested inside serve_report\n"
397+
" def build_file_path(filename):\n"
398+
" # FIXED: Validate that the resolved"
399+
" path stays within the base_dir\n"
400+
" base = os.path.abspath(generator.base_dir)\n"
401+
" target = os.path.abspath(os.path.join(base, filename))\n"
402+
" if not target.startswith(base):\n"
403+
' raise ValueError("Path Traversal Detected")\n'
404+
" return target\n\n"
405+
" if not requested_file:\n "
406+
' return "Error: No file specified"\n\n'
407+
" try:\n"
408+
" target_path = build_file_path(requested_file)\n"
409+
" except ValueError:\n"
410+
' return "Error: Invalid path"\n\n '
411+
" if os.path.exists(target_path):\n"
412+
' return f"Serving content of {target_path}"\n\n'
413+
' return "Error: File not found"',
414+
"fingerprint": "2deedb21d5f9b1409c59f0b1e5512d7"
415+
"3d9afdfc3f469ccf86e8835915d240e76",
429416
"start_line": 11,
430417
"end_line": 36,
431418
"node_type": "function_definition",
@@ -510,7 +497,6 @@ def test_extract_direct(self):
510497
source_code = """
511498
def hello():
512499
return 10
513-
514500
def clean_function():
515501
x = 10
516502
y = 20
@@ -521,4 +507,4 @@ def clean_function():
521507
functions = extract_definitions(tree, "Python", kinds=("functions",))
522508

523509
result = extract_direct_calls(functions[1], "Python", [])
524-
self.assertEqual(result, [(None, 'hello')])
510+
self.assertEqual(result, [(None, "hello")])

0 commit comments

Comments
 (0)