diff --git a/CHANGELOG.md b/CHANGELOG.md index a9437b8..a627734 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Precision +- A dominating guard makes a flow a hotspot only when it examines what the sink receives. `if request.method == 'POST':` before `request.POST['cmd']` no longer lowers a vulnerability to a hotspot. A test of the value itself, of the same attribute, of a method called on it (`form.is_valid()`), or of any part of an object the flow passes whole still does. The regression snapshots record the verdict of each taint finding (#135). - Django's `csrf.get_token` returns a token of letters and digits whatever the request holds, and `reverse`/`reverse_lazy` build a local path that attacker data in their arguments or query cannot turn into another host: the first is no longer an injection, the second no longer an open redirect. `reverse` still carries other kinds, since it leaves `'` unquoted (#132). - The SSRF sinks of Requests and httpx read the destination only: the URL (the first argument, the second after the method for `request` and `stream`, or `url=`) or the prepared request `send` takes. Attacker data in the body, the JSON, the headers or the query parameters is no longer a server-side request forgery. `Sink` gains `keywords` next to `positions`, and function summaries keep the name of each keyword argument, so a project function forwarding `url=` still reaches the sink; the cache format is bumped (#128). - `render_to_string` escapes what it renders where the analyzer can establish it: on a directory check, it reads the project's Django templates, and a template escapes when neither it nor what it extends or includes marks output safe, uses a tag, filter or library beyond Django's own, or prints a variable where HTML escaping does not protect it, and no settings turn autoescaping off. Data reaching a response through such a template is no longer `xss`; any other template keeps the flow reported. `TemplateRender` declares such rendering calls, and the escaped templates are part of the cache key (#130). diff --git a/docs/usage.md b/docs/usage.md index bff8f4f..d478584 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -118,9 +118,19 @@ a sanitizer for that kind of sink. Each flow is judged before it is reported. A dominating guard that proves the value safe (`isdigit()`, membership in a constant collection, equality with a constant, a numeric value proven by `int()`, `len()` or bounded arithmetic, a validator such as -`re.fullmatch`) refutes it and nothing is reported. A guard that only mentions the value, -or an authorization decorator such as `login_required`, makes it a hotspot reported at -medium confidence. The verdict and its evidence are in the finding's metadata. +`re.fullmatch`) refutes it and nothing is reported. A hotspot is reported at medium +confidence. Two things make a flow a hotspot: + +- A guard that examines the data without proving it safe. That covers a test of the value + itself (`if cmd:`, `len(cmd) < 10`), of the same attribute, or of a method called on + it (`form.is_valid()`). +- An authorization decorator such as `login_required`. + +A guard that reads another attribute of the object the data comes from is no guard of +that data. `if request.method == 'POST':` before `os.system(request.POST['cmd'])` leaves +a vulnerability. When the flow passes the object whole, to the sink or to a function, +a guard on any of its attributes still counts. The verdict and its evidence are in the +finding's metadata. Django's `render_to_string(name, context)` returns HTML in which autoescaping escaped every variable, so data reaching a response through it is no `xss`, if the template really diff --git a/src/coretrace_python/findings/refutation.py b/src/coretrace_python/findings/refutation.py index 8acec45..76a38e4 100644 --- a/src/coretrace_python/findings/refutation.py +++ b/src/coretrace_python/findings/refutation.py @@ -8,10 +8,13 @@ one of its arguments, a numeric value (``abstract.ranges``) cannot inject, and anything else that mentions the value is a guard that does not prove it. A proof counts for an origin when every dependence path from that origin to the sink argument goes through a -proven value. A flow is refuted when every tainted origin is proven safe or the sink is -unreachable, a hotspot when it sits behind an ``AuthorizationGuard`` (a decorator or a -dominating condition) or when an unproven guard mentions it, and a vulnerability -otherwise. +proven value. A guard mentions an origin only when it examines what the sink receives: +reading another attribute or item of an object the flow reads (``request.method`` next +to ``request.POST``) examines nothing of it, while the same attribute, or a method called +on the object (``form.is_valid()``), does. A flow is refuted when every tainted origin is +proven safe or the sink is unreachable, a hotspot when it sits behind an +``AuthorizationGuard`` (a decorator or a dominating condition) or when an unproven guard +mentions it, and a vulnerability otherwise. """ from __future__ import annotations @@ -37,6 +40,7 @@ Constant, FunctionIR, GetAttr, + GetItem, Instruction, UnaryOp, Value, @@ -267,6 +271,66 @@ def recognise(self, definition: Instruction | None) -> tuple[Value, str, bool] | return tested, "equals a constant", definition.operator == "eq" return None + def examined( + self, condition: Value, mentioned: set[Value], argument: Value, chain: frozenset[Value] + ) -> set[Value]: + """What the guard examines of the values it mentions: everything its condition is + computed from, except an object it only reads another attribute or item of than + the flow reads (``request`` in ``request.method`` when the sink receives + ``request.POST['cmd']``). A method called on the object (``form.is_valid()``), the + same attribute or item, or any part of an object the flow takes whole (passes to + the sink or to a call), examines it.""" + + reads = {self.access(value) for value in chain} - {None} + whole = {argument} | { + operand + for value in chain + if (definition := self.defs.get(value)) is not None + for operand in definition.operands() + if not (isinstance(definition, GetAttr | GetItem) and definition.object == operand) + } + found: set[Value] = set() + called: set[Value] = set() + pending = [condition] + while pending: + value = pending.pop() + if value in found: + continue + found.add(value) + definition = self.defs.get(value) + if definition is None: + continue + if isinstance(definition, Call): + called.add(definition.callee) + access = None if value in called else self.access(value) + for operand in definition.operands(): + if access is not None and access[0] == operand and operand not in whole and self.aside(access, reads): + continue + pending.append(operand) + return found & mentioned + + @staticmethod + def aside(access: tuple[Value, str, object], reads: set[tuple[Value, str, object] | None]) -> bool: + """Whether reading ``access`` reads beside what the flow reads of the same object; + an item under a key unknown on either side may be the same one.""" + + owner, kind, name = access + if kind == "item" and (name is None or (owner, "item", None) in reads): + return False + return access not in reads + + def access(self, value: Value) -> tuple[Value, str, object] | None: + """``(object, "attribute" or "item", name or constant key)`` for a read of an + attribute or an item; ``None`` for any other value, and as the key when unknown.""" + + definition = self.defs.get(value) + if isinstance(definition, GetAttr): + return definition.object, "attribute", definition.attribute + if isinstance(definition, GetItem): + key = self.defs.get(definition.key) + return definition.object, "item", key.value if isinstance(key, Constant) else None + return None + def is_constant_collection(self, value: Value) -> bool: definition = self.defs.get(value) if isinstance(definition, Constant): @@ -326,8 +390,9 @@ def judge(self, flow: TaintFlow, reachable: bool) -> Verdict: for value, reason in found.items(): if value in chain: proven.setdefault(value, reason) + examined = self.examined(guard.condition, mentioned, flow.argument, chain) for origin in origins: - if origin not in mentions and mentioned & ( + if origin not in mentions and examined & ( {origin} | {w for w in chain if origin in self.closure(w)} ): mentions[origin] = guard.line diff --git a/tests/regression/expected/Chatbot.json b/tests/regression/expected/Chatbot.json index 3c2dc8c..2081086 100644 --- a/tests/regression/expected/Chatbot.json +++ b/tests/regression/expected/Chatbot.json @@ -16,7 +16,8 @@ "file": "setup.py", "line": 48, "rule": "command-injection", - "function": "install_ollama" + "function": "install_ollama", + "verdict": "vulnerability" } ] } diff --git a/tests/regression/expected/DjangoGoat.json b/tests/regression/expected/DjangoGoat.json index e5ee470..0f0a09a 100644 --- a/tests/regression/expected/DjangoGoat.json +++ b/tests/regression/expected/DjangoGoat.json @@ -10,7 +10,8 @@ "file": "authentication/views.py", "line": 57, "rule": "sql-injection", - "function": "log_in" + "function": "log_in", + "verdict": "vulnerability" }, { "file": "djangogoat/settings.py", diff --git a/tests/regression/expected/Flask-Image-Editing-Website.json b/tests/regression/expected/Flask-Image-Editing-Website.json index 7eaa492..51c3ca9 100644 --- a/tests/regression/expected/Flask-Image-Editing-Website.json +++ b/tests/regression/expected/Flask-Image-Editing-Website.json @@ -22,7 +22,8 @@ "file": "main.py", "line": 79, "rule": "exploitable-vulnerability", - "function": "edit" + "function": "edit", + "verdict": "hotspot" }, { "file": "main.py", diff --git a/tests/regression/expected/InsecureGram.json b/tests/regression/expected/InsecureGram.json index 6e476f4..2faa39d 100644 --- a/tests/regression/expected/InsecureGram.json +++ b/tests/regression/expected/InsecureGram.json @@ -10,13 +10,15 @@ "file": "app/admin.py", "line": 45, "rule": "insecure-deserialization", - "function": "insecure_deserialize" + "function": "insecure_deserialize", + "verdict": "vulnerability" }, { "file": "app/command.py", "line": 12, "rule": "command-injection", - "function": "execute" + "function": "execute", + "verdict": "vulnerability" }, { "file": "app/config.py", @@ -34,25 +36,29 @@ "file": "app/redirect.py", "line": 11, "rule": "open-redirect", - "function": "redirect" + "function": "redirect", + "verdict": "vulnerability" }, { "file": "app/ssrf.py", "line": 12, "rule": "ssrf", - "function": "proxy" + "function": "proxy", + "verdict": "vulnerability" }, { "file": "app/upload.py", "line": 36, "rule": "path-traversal", - "function": "upload" + "function": "upload", + "verdict": "hotspot" }, { "file": "app/users.py", "line": 36, "rule": "sql-injection", - "function": "search_users" + "function": "search_users", + "verdict": "hotspot" } ] } diff --git a/tests/regression/expected/MiniBookApiServer.json b/tests/regression/expected/MiniBookApiServer.json index f7dfff8..a6d1f0f 100644 --- a/tests/regression/expected/MiniBookApiServer.json +++ b/tests/regression/expected/MiniBookApiServer.json @@ -10,13 +10,15 @@ "file": "app.py", "line": 69, "rule": "path-traversal", - "function": "get_img" + "function": "get_img", + "verdict": "vulnerability" }, { "file": "app.py", "line": 100, "rule": "exploitable-vulnerability", - "function": "new_book" + "function": "new_book", + "verdict": "hotspot" }, { "file": "config-example.json", diff --git a/tests/regression/expected/Vulnerable_fastapi.json b/tests/regression/expected/Vulnerable_fastapi.json index cfa3dab..b3e065e 100644 --- a/tests/regression/expected/Vulnerable_fastapi.json +++ b/tests/regression/expected/Vulnerable_fastapi.json @@ -10,13 +10,15 @@ "file": "app/routers/auth.py", "line": 37, "rule": "sql-injection", - "function": "login" + "function": "login", + "verdict": "vulnerability" }, { "file": "app/routers/files.py", "line": 27, "rule": "path-traversal", - "function": "download" + "function": "download", + "verdict": "hotspot" }, { "file": "exploit_scripts/brute_force_login.py", @@ -118,7 +120,8 @@ "file": "exploit_scripts/stored_xss_profile.py", "line": 24, "rule": "ssrf", - "function": "" + "function": "", + "verdict": "vulnerability" }, { "file": "exploit_scripts/stored_xss_profile.py", @@ -130,7 +133,8 @@ "file": "exploit_scripts/stored_xss_profile.py", "line": 28, "rule": "ssrf", - "function": "" + "function": "", + "verdict": "vulnerability" } ] } diff --git a/tests/regression/expected/dvpwa.json b/tests/regression/expected/dvpwa.json index 1934881..ae8d60e 100644 --- a/tests/regression/expected/dvpwa.json +++ b/tests/regression/expected/dvpwa.json @@ -34,7 +34,8 @@ "file": "sqli/views.py", "line": 57, "rule": "sql-injection", - "function": "students" + "function": "students", + "verdict": "vulnerability" } ] } diff --git a/tests/regression/expected/full-stack-fastapi-template.json b/tests/regression/expected/full-stack-fastapi-template.json index ac779dc..1fc03bd 100644 --- a/tests/regression/expected/full-stack-fastapi-template.json +++ b/tests/regression/expected/full-stack-fastapi-template.json @@ -40,7 +40,8 @@ "file": "backend/app/api/routes/login.py", "line": 121, "rule": "xss", - "function": "recover_password_html_content" + "function": "recover_password_html_content", + "verdict": "hotspot" }, { "file": "backend/tests/api/routes/test_login.py", diff --git a/tests/regression/expected/healthchecks.json b/tests/regression/expected/healthchecks.json index c3cf831..bcf1649 100644 --- a/tests/regression/expected/healthchecks.json +++ b/tests/regression/expected/healthchecks.json @@ -22,7 +22,8 @@ "file": "hc/accounts/decorators.py", "line": 44, "rule": "open-redirect", - "function": "wrapper" + "function": "wrapper", + "verdict": "hotspot" }, { "file": "hc/accounts/tests/test_createsuperuser.py", @@ -70,25 +71,29 @@ "file": "hc/accounts/views.py", "line": 115, "rule": "open-redirect", - "function": "_redirect_after_login" + "function": "_redirect_after_login", + "verdict": "hotspot" }, { "file": "hc/accounts/views.py", "line": 170, "rule": "open-redirect", - "function": "login" + "function": "login", + "verdict": "hotspot" }, { "file": "hc/accounts/views.py", "line": 281, "rule": "open-redirect", - "function": "check_token" + "function": "check_token", + "verdict": "hotspot" }, { "file": "hc/accounts/views.py", "line": 281, "rule": "open-redirect", - "function": "check_token" + "function": "check_token", + "verdict": "hotspot" }, { "file": "hc/accounts/views.py", @@ -100,13 +105,15 @@ "file": "hc/accounts/views.py", "line": 637, "rule": "open-redirect", - "function": "change_email_verify" + "function": "change_email_verify", + "verdict": "vulnerability" }, { "file": "hc/accounts/views.py", "line": 637, "rule": "open-redirect", - "function": "change_email_verify" + "function": "change_email_verify", + "verdict": "vulnerability" }, { "file": "hc/api/models.py", @@ -160,19 +167,22 @@ "file": "hc/api/views.py", "line": 645, "rule": "xss", - "function": "ping_body" + "function": "ping_body", + "verdict": "hotspot" }, { "file": "hc/api/views.py", "line": 778, "rule": "xss", - "function": "badge" + "function": "badge", + "verdict": "hotspot" }, { "file": "hc/api/views.py", "line": 811, "rule": "xss", - "function": "check_badge" + "function": "check_badge", + "verdict": "vulnerability" }, { "file": "hc/front/tests/test_webhook_validator.py", @@ -202,7 +212,8 @@ "file": "hc/front/views.py", "line": 814, "rule": "xss", - "function": "ping_body" + "function": "ping_body", + "verdict": "hotspot" }, { "file": "hc/integrations/discord/tests/test_add_complete.py", @@ -340,7 +351,8 @@ "file": "hc/integrations/po/views.py", "line": 59, "rule": "open-redirect", - "function": "add" + "function": "add", + "verdict": "hotspot" }, { "file": "hc/integrations/pushbullet/tests/test_add_complete.py", diff --git a/tests/regression/expected/httpie.json b/tests/regression/expected/httpie.json index e8d6931..7bbf6c1 100644 --- a/tests/regression/expected/httpie.json +++ b/tests/regression/expected/httpie.json @@ -16,13 +16,15 @@ "file": "docs/contributors/fetch.py", "line": 266, "rule": "command-injection", - "function": "" + "function": "", + "verdict": "vulnerability" }, { "file": "httpie/client.py", "line": 114, "rule": "ssrf", - "function": "collect_messages" + "function": "collect_messages", + "verdict": "vulnerability" }, { "file": "httpie/internal/update_warnings.py", diff --git a/tests/regression/expected/luigi.json b/tests/regression/expected/luigi.json index 3025063..16ecf57 100644 --- a/tests/regression/expected/luigi.json +++ b/tests/regression/expected/luigi.json @@ -28,7 +28,8 @@ "file": "luigi/contrib/lsf.py", "line": 263, "rule": "command-injection", - "function": "_run_job" + "function": "_run_job", + "verdict": "vulnerability" }, { "file": "luigi/contrib/mrrunner.py", @@ -160,7 +161,8 @@ "file": "luigi/contrib/sge.py", "line": 296, "rule": "command-injection", - "function": "_run_job" + "function": "_run_job", + "verdict": "vulnerability" }, { "file": "luigi/safe_extractor.py", diff --git a/tests/regression/expected/pygoat.json b/tests/regression/expected/pygoat.json index 0f77087..8cef4a8 100644 --- a/tests/regression/expected/pygoat.json +++ b/tests/regression/expected/pygoat.json @@ -10,13 +10,15 @@ "file": "challenge/views.py", "line": 50, "rule": "command-injection", - "function": "post" + "function": "post", + "verdict": "vulnerability" }, { "file": "challenge/views.py", "line": 81, "rule": "command-injection", - "function": "delete" + "function": "delete", + "verdict": "vulnerability" }, { "file": "dockerized_labs/broken_auth_lab/app.py", @@ -64,7 +66,8 @@ "file": "introduction/apis.py", "line": 38, "rule": "path-traversal", - "function": "ssrf_code_checker" + "function": "ssrf_code_checker", + "verdict": "vulnerability" }, { "file": "introduction/apis.py", @@ -112,7 +115,8 @@ "file": "introduction/mitre.py", "line": 242, "rule": "command-injection", - "function": "mitre_lab_17_api" + "function": "mitre_lab_17_api", + "verdict": "vulnerability" }, { "file": "introduction/playground/A6/soln.py", @@ -142,13 +146,15 @@ "file": "introduction/views.py", "line": 162, "rule": "sql-injection", - "function": "sql_lab" + "function": "sql_lab", + "verdict": "hotspot" }, { "file": "introduction/views.py", "line": 214, "rule": "insecure-deserialization", - "function": "insec_des_lab" + "function": "insec_des_lab", + "verdict": "vulnerability" }, { "file": "introduction/views.py", @@ -166,7 +172,8 @@ "file": "introduction/views.py", "line": 430, "rule": "command-injection", - "function": "cmd_lab" + "function": "cmd_lab", + "verdict": "vulnerability" }, { "file": "introduction/views.py", @@ -190,13 +197,15 @@ "file": "introduction/views.py", "line": 560, "rule": "exploitable-vulnerability", - "function": "a9_lab" + "function": "a9_lab", + "verdict": "vulnerability" }, { "file": "introduction/views.py", "line": 560, "rule": "insecure-deserialization", - "function": "a9_lab" + "function": "a9_lab", + "verdict": "vulnerability" }, { "file": "introduction/views.py", @@ -208,7 +217,8 @@ "file": "introduction/views.py", "line": 584, "rule": "exploitable-vulnerability", - "function": "a9_lab2" + "function": "a9_lab2", + "verdict": "vulnerability" }, { "file": "introduction/views.py", @@ -244,13 +254,15 @@ "file": "introduction/views.py", "line": 878, "rule": "sql-injection", - "function": "injection_sql_lab" + "function": "injection_sql_lab", + "verdict": "hotspot" }, { "file": "introduction/views.py", "line": 927, "rule": "path-traversal", - "function": "ssrf_lab" + "function": "ssrf_lab", + "verdict": "vulnerability" }, { "file": "introduction/views.py", @@ -262,7 +274,8 @@ "file": "introduction/views.py", "line": 963, "rule": "ssrf", - "function": "ssrf_lab2" + "function": "ssrf_lab2", + "verdict": "vulnerability" }, { "file": "introduction/views.py", diff --git a/tests/regression/expected/uploadbox-cloud-storage-solution.json b/tests/regression/expected/uploadbox-cloud-storage-solution.json index 1867454..621860c 100644 --- a/tests/regression/expected/uploadbox-cloud-storage-solution.json +++ b/tests/regression/expected/uploadbox-cloud-storage-solution.json @@ -16,31 +16,36 @@ "file": "app.py", "line": 139, "rule": "path-traversal", - "function": "upload_file" + "function": "upload_file", + "verdict": "vulnerability" }, { "file": "app.py", "line": 150, "rule": "path-traversal", - "function": "download_file" + "function": "download_file", + "verdict": "vulnerability" }, { "file": "app.py", "line": 161, "rule": "path-traversal", - "function": "delete_file" + "function": "delete_file", + "verdict": "hotspot" }, { "file": "db.py", "line": 24, "rule": "plaintext-credential-storage", - "function": "register_user" + "function": "register_user", + "verdict": "vulnerability" }, { "file": "db.py", "line": 31, "rule": "plaintext-credential-storage", - "function": "login_user" + "function": "login_user", + "verdict": "vulnerability" }, { "file": "requirements.txt", diff --git a/tests/regression/expected/vulnerable-flask.json b/tests/regression/expected/vulnerable-flask.json index d737f45..1257b75 100644 --- a/tests/regression/expected/vulnerable-flask.json +++ b/tests/regression/expected/vulnerable-flask.json @@ -22,31 +22,36 @@ "file": "app.py", "line": 44, "rule": "command-injection", - "function": "command_injection" + "function": "command_injection", + "verdict": "vulnerability" }, { "file": "app.py", "line": 50, "rule": "sql-injection", - "function": "sql_injection" + "function": "sql_injection", + "verdict": "vulnerability" }, { "file": "app.py", "line": 55, "rule": "path-traversal", - "function": "path_traversal" + "function": "path_traversal", + "verdict": "vulnerability" }, { "file": "app.py", "line": 60, "rule": "ssrf", - "function": "server_side_request_forgery" + "function": "server_side_request_forgery", + "verdict": "vulnerability" }, { "file": "app.py", "line": 66, "rule": "xss", - "function": "cross_site_scripting" + "function": "cross_site_scripting", + "verdict": "vulnerability" }, { "file": "app.py", diff --git a/tests/test_refutation.py b/tests/test_refutation.py index db18fe3..e6f090a 100644 --- a/tests/test_refutation.py +++ b/tests/test_refutation.py @@ -164,6 +164,31 @@ def test_non_validating_guards_make_a_hotspot() -> None: assert "guard" in truthy.evidence +def test_a_guard_on_another_attribute_of_the_tainted_object_is_no_guard() -> None: + # ``request.method == 'POST'`` says nothing of what ``request.POST[...]`` holds. + method = only(verdicts("def run():\n req = input()\n if req.method == 'POST':\n os.system(req.data)\n")) + user = only(verdicts("def run():\n req = input()\n if req.user.is_authenticated:\n os.system(req.data)\n")) + key = only(verdicts("def run():\n req = input()\n if req['mode'] == 'x':\n os.system(req['cmd'])\n")) + + assert method.status is user.status is key.status is Status.VULNERABILITY + + +def test_a_guard_on_what_the_sink_receives_stays_a_guard() -> None: + # Reading the same attribute again, or calling a method on the value, examines it. + reread = only(verdicts("def run():\n req = input()\n if req.data:\n os.system(req.data)\n")) + key = only(verdicts("def run():\n req = input()\n if req['cmd']:\n os.system(req['cmd'])\n")) + form = only( + verdicts("def run():\n form = input()\n if form.is_valid():\n os.system(form.cleaned_data)\n") + ) + transformed = only( + verdicts("def run():\n q = input()\n if not allowed(q.lower()):\n return\n os.system(q)\n") + ) + # Passed whole, the object may reach the sink by any attribute, the method included. + whole = only(verdicts("def run():\n req = input()\n if req.method == 'POST':\n os.system(req)\n")) + + assert reread.status is key.status is form.status is transformed.status is whole.status is Status.HOTSPOT + + def test_guards_on_unrelated_values_are_ignored() -> None: verdict = only( verdicts("def run(flag):\n cmd = input()\n if flag.isdigit():\n os.system(cmd)\n") diff --git a/tests/test_regression.py b/tests/test_regression.py index ad9062d..b3ec9ed 100644 --- a/tests/test_regression.py +++ b/tests/test_regression.py @@ -2,8 +2,9 @@ Every repository listed in ``tests/regression/repositories.toml`` is checked out at its pinned commit, analysed with the bundled plugins and compared with the snapshot recorded -in ``tests/regression/expected/.json``: the findings (file, line, rule, function) -and the coverage. A change in either is a regression, or an intended change to record +in ``tests/regression/expected/.json``: the findings (file, line, rule, function, +and the verdict of a taint finding) and the coverage. A change in either is a +regression, or an intended change to record with ``CORETRACE_REGRESSION_UPDATE=1``. Checkouts are kept under ``CORETRACE_REGRESSION_ROOT`` (default ``.regression/`` at the repository root). @@ -83,10 +84,12 @@ def snapshot(path: Path) -> dict[str, object]: "line": f.span.start_line, "rule": f.rule_id, "function": f.function, + # A verdict decides the confidence: a change of it is a change to review. + **({"verdict": f.metadata["verdict"]} if "verdict" in f.metadata else {}), } for f in analysis.findings ), - key=lambda f: (f["file"], f["line"], f["rule"], f["function"] or ""), + key=lambda f: (f["file"], f["line"], f["rule"], f["function"] or "", f.get("verdict", "")), ) return { "coverage": {