diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e5c6bb..948e889 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ # Changelog +## Unreleased + +### Compatibility and migration + +- The cache format changes (18): caches built by an earlier version miss once. +- SARIF: each rule carries a title (`shortDescription`) and a description (`fullDescription`) + instead of repeating its id (#270), and each result carries its confidence as the + `confidence` property (#271). A flow reaching its sink inside a project function points at + that sink as a related location, which the result's message links to (#271). +- Plugin API, additive: `Plugin.rules` maps the rules a plugin reports to their + `RuleDescription`, optional, so that third-party plugins describe their own rules; + `Finding.related` lists `RelatedLocation`s (a span and a message). `Report.rules` carries the + descriptions to the reporters. + ## 0.23.0 (2026-10-16) ### Compatibility and migration diff --git a/docs/plugins.md b/docs/plugins.md index 757e318..0f7b4d3 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -341,6 +341,32 @@ JSON pointer of a value) for a value of a structured file whose line cannot be: that reads a finding's line checks which it has, and never invents a line for a `FileLocation` (`coretrace_python.source.positions` places JSON and TOML values). +A plugin describes the rules it reports in its optional `rules` attribute: each rule id to a +`RuleDescription`, a title and what the rule detects. The SARIF report gives them to its +consumers (`shortDescription` and `fullDescription`), so that GitHub code scanning names and +explains the alert; a rule the plugin does not describe is listed by its id alone. + +```python +from collections.abc import Mapping + +from coretrace_python.findings import RuleDescription + + +class SensitiveLoggingPlugin(SymbolCallDetector): + ... + rules: ClassVar[Mapping[str, RuleDescription]] = { + "sensitive-logging": RuleDescription( + "Sensitive data logged", + "A logging call at debug level that may write sensitive data to the logs.", + ), + } +``` + +A finding may also point at other places it involves, each a `RelatedLocation` (a span and a +short message) in its `related` tuple, as a taint flow reaching its sink inside a project +function points at that sink. The SARIF report lists them as `relatedLocations` and links to +them from the message. + ## Testing a plugin Load the plugin directory with the engine and check one source or one project: diff --git a/docs/usage.md b/docs/usage.md index e43f262..4e7460d 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -417,7 +417,14 @@ several sources define the same advisory, it names the one whose definition is u the next section describes. `--format sarif` prints a SARIF 2.1.0 log, one run with the tool, its rules and one -result per finding. The root is declared once as the `SRCROOT` original URI base and +result per finding. Each rule carries a title (`shortDescription`) and what it detects +(`fullDescription`), so that a code scanning service can explain a finding: the plugin that +reports a rule describes it, the engine its own notes, and a rule a plugin does not describe +keeps its id alone. Each result carries the finding's confidence as the `confidence` +property. A flow that reaches its sink inside a project function is reported at the call, and +points at the sink as a related location (`relatedLocations`), with the message +`sink , reached via `, which the result's message links to. The root is +declared once as the `SRCROOT` original URI base and every location under it is relative to that base, which is what code scanning services need to attach results to files. The components of a directory check are the tool's `extensions`, their kind and digest in `properties`: diff --git a/src/coretrace_python/bundled/dependency/dependency_policy/dependency_policy.py b/src/coretrace_python/bundled/dependency/dependency_policy/dependency_policy.py index ba2929e..70cf063 100644 --- a/src/coretrace_python/bundled/dependency/dependency_policy/dependency_policy.py +++ b/src/coretrace_python/bundled/dependency/dependency_policy/dependency_policy.py @@ -2,17 +2,27 @@ from __future__ import annotations -from collections.abc import Sequence +from collections.abc import Mapping, Sequence from typing import ClassVar from coretrace_python.analysis import AnyAnalysis from coretrace_python.dependency import DependencyAnalysis -from coretrace_python.findings import Confidence, Finding, Severity +from coretrace_python.findings import Confidence, Finding, RuleDescription, Severity from coretrace_python.plugins import ProjectContext, ProjectPlugin class DependencyPolicyPlugin(ProjectPlugin): name: ClassVar[str] = "dependency-policy" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "denied-dependency": RuleDescription( + "Denied dependency", + "A requirement on a package the policy denies.", + ), + "unpinned-dependency": RuleDescription( + "Unpinned dependency", + "A requirement without an exact pin, when the policy requires pins.", + ), + } requires: ClassVar[frozenset[AnyAnalysis]] = frozenset({DependencyAnalysis}) def analyze_project(self, ctx: ProjectContext) -> Sequence[Finding]: diff --git a/src/coretrace_python/bundled/dependency/reachable_vulnerability/reachable_vulnerability.py b/src/coretrace_python/bundled/dependency/reachable_vulnerability/reachable_vulnerability.py index 53e7ead..02bfa3f 100644 --- a/src/coretrace_python/bundled/dependency/reachable_vulnerability/reachable_vulnerability.py +++ b/src/coretrace_python/bundled/dependency/reachable_vulnerability/reachable_vulnerability.py @@ -4,7 +4,7 @@ from __future__ import annotations -from collections.abc import Iterator, Sequence +from collections.abc import Iterator, Mapping, Sequence from typing import ClassVar from coretrace_python.analysis import AnyAnalysis @@ -17,7 +17,7 @@ evidence, suffix_index, ) -from coretrace_python.findings import Confidence, Finding +from coretrace_python.findings import Confidence, Finding, RuleDescription from coretrace_python.interprocedural import CallGraphAnalysis, ExternalSymbol from coretrace_python.plugins import ProjectContext, ProjectPlugin from coretrace_python.semantic.symbols import SymbolId @@ -26,6 +26,12 @@ class ReachableVulnerabilityPlugin(ProjectPlugin): name: ClassVar[str] = "reachable-vulnerability" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "reachable-vulnerability": RuleDescription( + "Reachable dependency vulnerability", + "The project calls an API that an advisory of a vulnerable requirement affects.", + ), + } requires: ClassVar[frozenset[AnyAnalysis]] = frozenset({DependencyAnalysis, CallGraphAnalysis}) def analyze_project(self, ctx: ProjectContext) -> Sequence[Finding]: diff --git a/src/coretrace_python/bundled/dependency/vulnerable_dependency/vulnerable_dependency.py b/src/coretrace_python/bundled/dependency/vulnerable_dependency/vulnerable_dependency.py index f5751d5..e0eaea7 100644 --- a/src/coretrace_python/bundled/dependency/vulnerable_dependency/vulnerable_dependency.py +++ b/src/coretrace_python/bundled/dependency/vulnerable_dependency/vulnerable_dependency.py @@ -2,7 +2,7 @@ from __future__ import annotations -from collections.abc import Sequence +from collections.abc import Mapping, Sequence from pathlib import PurePath from typing import ClassVar @@ -15,13 +15,19 @@ ruled_out, suffix_index, ) -from coretrace_python.findings import Confidence, Finding +from coretrace_python.findings import Confidence, Finding, RuleDescription from coretrace_python.interprocedural import CallGraphAnalysis, ExternalSymbol from coretrace_python.plugins import ProjectContext, ProjectPlugin class VulnerableDependencyPlugin(ProjectPlugin): name: ClassVar[str] = "vulnerable-dependency" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "vulnerable-dependency": RuleDescription( + "Vulnerable dependency", + "A requirement allows a version that an advisory affects.", + ), + } requires: ClassVar[frozenset[AnyAnalysis]] = frozenset({DependencyAnalysis, CallGraphAnalysis}) def analyze_project(self, ctx: ProjectContext) -> Sequence[Finding]: diff --git a/src/coretrace_python/bundled/security/code_injection/code_injection.py b/src/coretrace_python/bundled/security/code_injection/code_injection.py index 247ab54..891d476 100644 --- a/src/coretrace_python/bundled/security/code_injection/code_injection.py +++ b/src/coretrace_python/bundled/security/code_injection/code_injection.py @@ -18,11 +18,12 @@ from __future__ import annotations +from collections.abc import Mapping from typing import ClassVar from coretrace_python.abstract.strings import ModuleStringsAnalysis, leading_text from coretrace_python.analysis import AnyAnalysis -from coretrace_python.findings import Severity +from coretrace_python.findings import RuleDescription, Severity from coretrace_python.findings.refutation import Status, Verdict from coretrace_python.hir import nodes from coretrace_python.ir.ssa import SSAAnalysis @@ -42,6 +43,12 @@ class CodeInjectionPlugin(TaintDetector): name: ClassVar[str] = "code-injection" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "code-injection": RuleDescription( + "Code injection", + "Attacker-controlled data chooses code that runs: the code eval or exec runs, the module an import loads, the script runpy runs, or the attribute getattr selects and the program calls.", + ), + } rule_id: ClassVar[str] = "code-injection" kind: ClassVar[TaintKind] = TaintKind.CODE severity: ClassVar[Severity] = Severity.HIGH diff --git a/src/coretrace_python/bundled/security/command_injection/command_injection.py b/src/coretrace_python/bundled/security/command_injection/command_injection.py index fcf9570..e40fe35 100644 --- a/src/coretrace_python/bundled/security/command_injection/command_injection.py +++ b/src/coretrace_python/bundled/security/command_injection/command_injection.py @@ -36,7 +36,7 @@ from coretrace_python.abstract.strings import ModuleStringsAnalysis, leading_text from coretrace_python.analysis import AnyAnalysis -from coretrace_python.findings import Severity +from coretrace_python.findings import RuleDescription, Severity from coretrace_python.findings.refutation import Status, Verdict from coretrace_python.hir import nodes from coretrace_python.interprocedural import Arguments @@ -398,6 +398,12 @@ class _Operands: class CommandInjectionPlugin(TaintDetector): name: ClassVar[str] = "command-injection" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "command-injection": RuleDescription( + "Command injection", + "Attacker-controlled data reaches a shell or process execution (os.system, subprocess, ...) where it can run or choose a command.", + ), + } rule_id: ClassVar[str] = "command-injection" kind: ClassVar[TaintKind] = TaintKind.COMMAND severity: ClassVar[Severity] = Severity.HIGH diff --git a/src/coretrace_python/bundled/security/insecure_deserialization/insecure_deserialization.py b/src/coretrace_python/bundled/security/insecure_deserialization/insecure_deserialization.py index 6700bfb..75c74af 100644 --- a/src/coretrace_python/bundled/security/insecure_deserialization/insecure_deserialization.py +++ b/src/coretrace_python/bundled/security/insecure_deserialization/insecure_deserialization.py @@ -2,15 +2,22 @@ from __future__ import annotations +from collections.abc import Mapping from typing import ClassVar -from coretrace_python.findings import Severity +from coretrace_python.findings import RuleDescription, Severity from coretrace_python.plugins import TaintDetector from coretrace_python.taint import TaintKind class InsecureDeserializationPlugin(TaintDetector): name: ClassVar[str] = "insecure-deserialization" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "insecure-deserialization": RuleDescription( + "Insecure deserialization", + "Attacker-controlled data reaches a deserializer that can build arbitrary objects or run code: pickle, marshal, dill, jsonpickle, shelve, or a YAML loader without a safe loader.", + ), + } rule_id: ClassVar[str] = "insecure-deserialization" kind: ClassVar[TaintKind] = TaintKind.DESERIALIZATION severity: ClassVar[Severity] = Severity.CRITICAL diff --git a/src/coretrace_python/bundled/security/nosql_injection/nosql_injection.py b/src/coretrace_python/bundled/security/nosql_injection/nosql_injection.py index fda5452..29b859f 100644 --- a/src/coretrace_python/bundled/security/nosql_injection/nosql_injection.py +++ b/src/coretrace_python/bundled/security/nosql_injection/nosql_injection.py @@ -5,15 +5,22 @@ from __future__ import annotations +from collections.abc import Mapping from typing import ClassVar -from coretrace_python.findings import Severity +from coretrace_python.findings import RuleDescription, Severity from coretrace_python.plugins import TaintDetector from coretrace_python.taint import TaintKind class NoSqlInjectionPlugin(TaintDetector): name: ClassVar[str] = "nosql-injection" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "nosql-injection": RuleDescription( + "NoSQL injection", + "Attacker input that may hold a structure, such as a JSON body, reaches a NoSQL query, where an operator such as {\"$ne\": null} matches any value.", + ), + } rule_id: ClassVar[str] = "nosql-injection" kind: ClassVar[TaintKind] = TaintKind.NOSQL severity: ClassVar[Severity] = Severity.HIGH diff --git a/src/coretrace_python/bundled/security/open_redirect/open_redirect.py b/src/coretrace_python/bundled/security/open_redirect/open_redirect.py index 37c83d3..687aa86 100644 --- a/src/coretrace_python/bundled/security/open_redirect/open_redirect.py +++ b/src/coretrace_python/bundled/security/open_redirect/open_redirect.py @@ -10,11 +10,12 @@ from __future__ import annotations +from collections.abc import Mapping from typing import ClassVar from coretrace_python.abstract.strings import ModuleStringsAnalysis from coretrace_python.analysis import AnyAnalysis -from coretrace_python.findings import Severity +from coretrace_python.findings import RuleDescription, Severity from coretrace_python.findings.refutation import Status, Verdict from coretrace_python.hir import nodes from coretrace_python.ir.ssa import SSAAnalysis @@ -25,6 +26,12 @@ class OpenRedirectPlugin(TaintDetector): name: ClassVar[str] = "open-redirect" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "open-redirect": RuleDescription( + "Open redirect", + "Attacker-controlled data chooses the target of an HTTP redirect, which can send the user to another site.", + ), + } rule_id: ClassVar[str] = "open-redirect" kind: ClassVar[TaintKind] = TaintKind.REDIRECT severity: ClassVar[Severity] = Severity.MEDIUM diff --git a/src/coretrace_python/bundled/security/path_traversal/path_traversal.py b/src/coretrace_python/bundled/security/path_traversal/path_traversal.py index f43edbc..f2c305f 100644 --- a/src/coretrace_python/bundled/security/path_traversal/path_traversal.py +++ b/src/coretrace_python/bundled/security/path_traversal/path_traversal.py @@ -2,15 +2,22 @@ from __future__ import annotations +from collections.abc import Mapping from typing import ClassVar -from coretrace_python.findings import Severity +from coretrace_python.findings import RuleDescription, Severity from coretrace_python.plugins import TaintDetector from coretrace_python.taint import TaintKind class PathTraversalPlugin(TaintDetector): name: ClassVar[str] = "path-traversal" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "path-traversal": RuleDescription( + "Path traversal", + "Attacker-controlled data reaches a file system path (open, send_file, os.remove, ...).", + ), + } rule_id: ClassVar[str] = "path-traversal" kind: ClassVar[TaintKind] = TaintKind.PATH severity: ClassVar[Severity] = Severity.HIGH diff --git a/src/coretrace_python/bundled/security/plaintext_credentials/plaintext_credentials.py b/src/coretrace_python/bundled/security/plaintext_credentials/plaintext_credentials.py index 885e2f0..acd7a96 100644 --- a/src/coretrace_python/bundled/security/plaintext_credentials/plaintext_credentials.py +++ b/src/coretrace_python/bundled/security/plaintext_credentials/plaintext_credentials.py @@ -5,15 +5,22 @@ from __future__ import annotations +from collections.abc import Mapping from typing import ClassVar -from coretrace_python.findings import Confidence, Severity +from coretrace_python.findings import Confidence, RuleDescription, Severity from coretrace_python.plugins import TaintDetector from coretrace_python.taint import TaintKind class PlaintextCredentialsPlugin(TaintDetector): name: ClassVar[str] = "plaintext-credentials" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "plaintext-credential-storage": RuleDescription( + "Plaintext credential storage", + "A parameter named like a password is stored in a database without hashing.", + ), + } rule_id: ClassVar[str] = "plaintext-credential-storage" kind: ClassVar[TaintKind] = TaintKind.CREDENTIAL severity: ClassVar[Severity] = Severity.HIGH diff --git a/src/coretrace_python/bundled/security/sql_injection/sql_injection.py b/src/coretrace_python/bundled/security/sql_injection/sql_injection.py index d3b7179..c50e047 100644 --- a/src/coretrace_python/bundled/security/sql_injection/sql_injection.py +++ b/src/coretrace_python/bundled/security/sql_injection/sql_injection.py @@ -2,15 +2,22 @@ from __future__ import annotations +from collections.abc import Mapping from typing import ClassVar -from coretrace_python.findings import Severity +from coretrace_python.findings import RuleDescription, Severity from coretrace_python.plugins import TaintDetector from coretrace_python.taint import TaintKind class SqlInjectionPlugin(TaintDetector): name: ClassVar[str] = "sql-injection" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "sql-injection": RuleDescription( + "SQL injection", + "Attacker-controlled data reaches a database statement (cursor.execute, SQLAlchemy text, Django raw, ...) rather than a query parameter.", + ), + } rule_id: ClassVar[str] = "sql-injection" kind: ClassVar[TaintKind] = TaintKind.SQL severity: ClassVar[Severity] = Severity.HIGH diff --git a/src/coretrace_python/bundled/security/ssrf/ssrf.py b/src/coretrace_python/bundled/security/ssrf/ssrf.py index aa4ed5a..2b4b87b 100644 --- a/src/coretrace_python/bundled/security/ssrf/ssrf.py +++ b/src/coretrace_python/bundled/security/ssrf/ssrf.py @@ -10,11 +10,12 @@ from __future__ import annotations +from collections.abc import Mapping from typing import ClassVar from coretrace_python.abstract.strings import ModuleStringsAnalysis from coretrace_python.analysis import AnyAnalysis -from coretrace_python.findings import Severity +from coretrace_python.findings import RuleDescription, Severity from coretrace_python.findings.refutation import Status, Verdict from coretrace_python.hir import nodes from coretrace_python.ir.ssa import SSAAnalysis @@ -25,6 +26,12 @@ class SsrfPlugin(TaintDetector): name: ClassVar[str] = "ssrf" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "ssrf": RuleDescription( + "Server-side request forgery", + "Attacker-controlled data reaches the destination of an HTTP client request (Requests, httpx, urllib).", + ), + } rule_id: ClassVar[str] = "ssrf" kind: ClassVar[TaintKind] = TaintKind.SSRF severity: ClassVar[Severity] = Severity.HIGH diff --git a/src/coretrace_python/bundled/security/xss/xss.py b/src/coretrace_python/bundled/security/xss/xss.py index 81aabd8..6588b87 100644 --- a/src/coretrace_python/bundled/security/xss/xss.py +++ b/src/coretrace_python/bundled/security/xss/xss.py @@ -2,15 +2,22 @@ from __future__ import annotations +from collections.abc import Mapping from typing import ClassVar -from coretrace_python.findings import Severity +from coretrace_python.findings import RuleDescription, Severity from coretrace_python.plugins import TaintDetector from coretrace_python.taint import TaintKind class XssPlugin(TaintDetector): name: ClassVar[str] = "xss" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "xss": RuleDescription( + "Cross-site scripting", + "Attacker-controlled data reaches an HTTP response body built without escaping.", + ), + } rule_id: ClassVar[str] = "xss" kind: ClassVar[TaintKind] = TaintKind.HTML severity: ClassVar[Severity] = Severity.HIGH diff --git a/src/coretrace_python/bundled/syntax/dangerous_eval/dangerous_eval.py b/src/coretrace_python/bundled/syntax/dangerous_eval/dangerous_eval.py index 7b428fa..5c5c04e 100644 --- a/src/coretrace_python/bundled/syntax/dangerous_eval/dangerous_eval.py +++ b/src/coretrace_python/bundled/syntax/dangerous_eval/dangerous_eval.py @@ -2,15 +2,22 @@ from __future__ import annotations +from collections.abc import Mapping from typing import ClassVar -from coretrace_python.findings import Severity +from coretrace_python.findings import RuleDescription, Severity from coretrace_python.plugins import SymbolCallDetector from coretrace_python.semantic.symbols import SymbolId class DangerousEvalPlugin(SymbolCallDetector): name: ClassVar[str] = "dangerous-eval" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "dangerous-eval": RuleDescription( + "Dynamic code execution", + "A call to eval or exec, whatever name the file gives them.", + ), + } rule_id: ClassVar[str] = "dangerous-eval" symbols: ClassVar[frozenset[SymbolId]] = frozenset( {SymbolId("python.builtins.eval"), SymbolId("python.builtins.exec")} diff --git a/src/coretrace_python/bundled/syntax/flask_debug/flask_debug.py b/src/coretrace_python/bundled/syntax/flask_debug/flask_debug.py index 137d2ff..1644464 100644 --- a/src/coretrace_python/bundled/syntax/flask_debug/flask_debug.py +++ b/src/coretrace_python/bundled/syntax/flask_debug/flask_debug.py @@ -2,11 +2,11 @@ from __future__ import annotations -from collections.abc import Iterator, Sequence +from collections.abc import Iterator, Mapping, Sequence from typing import ClassVar from coretrace_python.analysis import AnyAnalysis -from coretrace_python.findings import Confidence, Finding, Severity +from coretrace_python.findings import Confidence, Finding, RuleDescription, Severity from coretrace_python.hir import nodes from coretrace_python.hir.visitors import Node, children from coretrace_python.plugins import Plugin, PluginContext @@ -18,6 +18,12 @@ class FlaskDebugPlugin(Plugin): name: ClassVar[str] = "flask-debug" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "debug-enabled": RuleDescription( + "Debug mode enabled", + "A Flask application run with debug=True, which exposes an interactive debugger.", + ), + } requires: ClassVar[frozenset[AnyAnalysis]] = frozenset({ScopeAnalysis, SymbolAnalysis}) def analyze(self, ctx: PluginContext) -> Sequence[Finding]: diff --git a/src/coretrace_python/bundled/syntax/insecure_tempfile/insecure_tempfile.py b/src/coretrace_python/bundled/syntax/insecure_tempfile/insecure_tempfile.py index 217155d..f3efb01 100644 --- a/src/coretrace_python/bundled/syntax/insecure_tempfile/insecure_tempfile.py +++ b/src/coretrace_python/bundled/syntax/insecure_tempfile/insecure_tempfile.py @@ -3,10 +3,11 @@ from __future__ import annotations +from collections.abc import Mapping from typing import ClassVar from coretrace_python.analysis import AnyAnalysis -from coretrace_python.findings import Severity +from coretrace_python.findings import RuleDescription, Severity from coretrace_python.interprocedural import CallGraphAnalysis from coretrace_python.ir.ssa import SSAAnalysis from coretrace_python.plugins import SymbolCallDetector @@ -15,6 +16,12 @@ class InsecureTempFilePlugin(SymbolCallDetector): name: ClassVar[str] = "insecure-temp-file" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "insecure-temp-file": RuleDescription( + "Insecure temporary file", + "tempfile.mktemp, which names a file without creating it, so that another process can create it first.", + ), + } rule_id: ClassVar[str] = "insecure-temp-file" requires: ClassVar[frozenset[AnyAnalysis]] = frozenset({SSAAnalysis, CallGraphAnalysis}) symbols: ClassVar[frozenset[SymbolId]] = frozenset({SymbolId("python.tempfile.mktemp")}) diff --git a/src/coretrace_python/bundled/syntax/insecure_tls/insecure_tls.py b/src/coretrace_python/bundled/syntax/insecure_tls/insecure_tls.py index d602203..1b76014 100644 --- a/src/coretrace_python/bundled/syntax/insecure_tls/insecure_tls.py +++ b/src/coretrace_python/bundled/syntax/insecure_tls/insecure_tls.py @@ -7,7 +7,7 @@ from typing import ClassVar from coretrace_python.analysis import AnyAnalysis -from coretrace_python.findings import Severity +from coretrace_python.findings import RuleDescription, Severity from coretrace_python.interprocedural import CallGraphAnalysis from coretrace_python.ir.model import Call, Constant, Instruction, Value from coretrace_python.ir.ssa import SSAAnalysis @@ -24,6 +24,12 @@ class InsecureTlsPlugin(SymbolCallDetector): name: ClassVar[str] = "insecure-tls" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "insecure-tls": RuleDescription( + "TLS certificate verification disabled", + "verify=False on a Requests or httpx call, or ssl._create_unverified_context(), which accept any certificate.", + ), + } rule_id: ClassVar[str] = "insecure-tls" requires: ClassVar[frozenset[AnyAnalysis]] = frozenset({SSAAnalysis, CallGraphAnalysis}) symbols: ClassVar[frozenset[SymbolId]] = HTTP_CALLS | UNVERIFIED diff --git a/src/coretrace_python/bundled/syntax/missing_timeout/missing_timeout.py b/src/coretrace_python/bundled/syntax/missing_timeout/missing_timeout.py index f1a585a..10d0de7 100644 --- a/src/coretrace_python/bundled/syntax/missing_timeout/missing_timeout.py +++ b/src/coretrace_python/bundled/syntax/missing_timeout/missing_timeout.py @@ -2,11 +2,11 @@ from __future__ import annotations -from collections.abc import Sequence +from collections.abc import Mapping, Sequence from typing import ClassVar from coretrace_python.analysis import AnyAnalysis -from coretrace_python.findings import Confidence, Finding, Severity +from coretrace_python.findings import Confidence, Finding, RuleDescription, Severity from coretrace_python.interprocedural import CallGraphAnalysis from coretrace_python.ir.model import Call from coretrace_python.ir.ssa import SSAAnalysis @@ -20,6 +20,12 @@ class MissingTimeoutPlugin(Plugin): name: ClassVar[str] = "missing-timeout" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "missing-timeout": RuleDescription( + "HTTP request without a timeout", + "A Requests or httpx call without a timeout, which can wait forever.", + ), + } requires: ClassVar[frozenset[AnyAnalysis]] = frozenset({SSAAnalysis, CallGraphAnalysis}) def analyze(self, ctx: PluginContext) -> Sequence[Finding]: diff --git a/src/coretrace_python/bundled/syntax/unsafe_archive/unsafe_archive.py b/src/coretrace_python/bundled/syntax/unsafe_archive/unsafe_archive.py index 097e598..67785e0 100644 --- a/src/coretrace_python/bundled/syntax/unsafe_archive/unsafe_archive.py +++ b/src/coretrace_python/bundled/syntax/unsafe_archive/unsafe_archive.py @@ -7,7 +7,7 @@ from typing import ClassVar from coretrace_python.analysis import AnyAnalysis -from coretrace_python.findings import Severity +from coretrace_python.findings import RuleDescription, Severity from coretrace_python.interprocedural import CallGraphAnalysis from coretrace_python.ir.model import Call, Instruction, Value from coretrace_python.ir.ssa import SSAAnalysis @@ -20,6 +20,12 @@ class UnsafeArchivePlugin(SymbolCallDetector): name: ClassVar[str] = "unsafe-archive-extraction" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "unsafe-archive-extraction": RuleDescription( + "Unsafe archive extraction", + "A tarfile or zipfile extraction, or shutil.unpack_archive, without a member filter: a member can be written outside the destination.", + ), + } rule_id: ClassVar[str] = "unsafe-archive-extraction" requires: ClassVar[frozenset[AnyAnalysis]] = frozenset({SSAAnalysis, CallGraphAnalysis}) symbols: ClassVar[frozenset[SymbolId]] = frozenset( diff --git a/src/coretrace_python/bundled/syntax/unsafe_xml/unsafe_xml.py b/src/coretrace_python/bundled/syntax/unsafe_xml/unsafe_xml.py index c682d04..d32368f 100644 --- a/src/coretrace_python/bundled/syntax/unsafe_xml/unsafe_xml.py +++ b/src/coretrace_python/bundled/syntax/unsafe_xml/unsafe_xml.py @@ -19,7 +19,7 @@ from coretrace_python.abstract.strings import ModuleStringsAnalysis from coretrace_python.analysis import AnyAnalysis -from coretrace_python.findings import Confidence, Severity +from coretrace_python.findings import Confidence, RuleDescription, Severity from coretrace_python.interprocedural import CallGraphAnalysis from coretrace_python.ir.model import Call, Constant, Global, Instruction, Value from coretrace_python.ir.ssa import SSAAnalysis @@ -64,6 +64,12 @@ def _resolve_entities(call: Call, defs: Mapping[Value, Instruction]) -> object: class UnsafeXmlPlugin(SymbolCallDetector): name: ClassVar[str] = "unsafe-xml" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "unsafe-xml": RuleDescription( + "Unsafe XML parser", + "An XML parser of the standard library or of lxml, exposed to the entities a document declares; defusedxml is the safe replacement.", + ), + } rule_id: ClassVar[str] = "unsafe-xml" requires: ClassVar[frozenset[AnyAnalysis]] = frozenset({SSAAnalysis, CallGraphAnalysis, ModuleStringsAnalysis}) symbols: ClassVar[frozenset[SymbolId]] = _STANDARD_LIBRARY | _LXML | _LXML_UNTIL_6_1 diff --git a/src/coretrace_python/bundled/syntax/weak_crypto/weak_crypto.py b/src/coretrace_python/bundled/syntax/weak_crypto/weak_crypto.py index 30e365b..cf216fc 100644 --- a/src/coretrace_python/bundled/syntax/weak_crypto/weak_crypto.py +++ b/src/coretrace_python/bundled/syntax/weak_crypto/weak_crypto.py @@ -2,15 +2,22 @@ from __future__ import annotations +from collections.abc import Mapping from typing import ClassVar -from coretrace_python.findings import Severity +from coretrace_python.findings import RuleDescription, Severity from coretrace_python.plugins import SymbolCallDetector from coretrace_python.semantic.symbols import SymbolId class WeakCryptoPlugin(SymbolCallDetector): name: ClassVar[str] = "weak-crypto" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "weak-crypto": RuleDescription( + "Weak cryptographic algorithm", + "A broken algorithm such as hashlib.md5 or hashlib.sha1.", + ), + } rule_id: ClassVar[str] = "weak-crypto" symbols: ClassVar[frozenset[SymbolId]] = frozenset( {SymbolId("python.hashlib.md5"), SymbolId("python.hashlib.sha1")} diff --git a/src/coretrace_python/bundled/syntax/weak_random/weak_random.py b/src/coretrace_python/bundled/syntax/weak_random/weak_random.py index d69dff7..97c1579 100644 --- a/src/coretrace_python/bundled/syntax/weak_random/weak_random.py +++ b/src/coretrace_python/bundled/syntax/weak_random/weak_random.py @@ -7,11 +7,11 @@ from __future__ import annotations import re -from collections.abc import Iterator, Sequence +from collections.abc import Iterator, Mapping, Sequence from typing import ClassVar from coretrace_python.analysis import AnyAnalysis -from coretrace_python.findings import Confidence, Finding, Severity +from coretrace_python.findings import Confidence, Finding, RuleDescription, Severity from coretrace_python.hir import nodes from coretrace_python.hir.visitors import Node, children from coretrace_python.plugins import Plugin, PluginContext @@ -25,6 +25,12 @@ class WeakRandomPlugin(Plugin): name: ClassVar[str] = "weak-random" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "weak-random": RuleDescription( + "Weak random value for a credential", + "A value of the random module used as a credential; random is not cryptographically secure.", + ), + } rule_id: ClassVar[str] = "weak-random" requires: ClassVar[frozenset[AnyAnalysis]] = frozenset({ScopeAnalysis, SymbolAnalysis}) diff --git a/src/coretrace_python/cache.py b/src/coretrace_python/cache.py index e8fc550..879daaf 100644 --- a/src/coretrace_python/cache.py +++ b/src/coretrace_python/cache.py @@ -21,7 +21,7 @@ from types import MappingProxyType from typing import Any -from coretrace_python.findings import Confidence, Finding, Severity +from coretrace_python.findings import Confidence, Finding, RelatedLocation, Severity from coretrace_python.interprocedural import ( Arguments, CallSite, @@ -44,7 +44,8 @@ # Cached summaries from earlier versions preserve unstable external-call # insertion order. They must be recomputed with canonical ordering (#261). -CACHE_FORMAT = 17 +# Findings carry their related locations from format 18 on (#271). +CACHE_FORMAT = 18 @dataclass(frozen=True) @@ -229,6 +230,7 @@ def _encode_finding(finding: Finding) -> dict[str, Any]: "span": _encode_location(finding.span), "function": finding.function, "metadata": dict(finding.metadata), + "related": [[_encode_span(r.span), r.message] for r in finding.related], } @@ -242,6 +244,7 @@ def _decode_finding(data: Mapping[str, Any]) -> Finding: _decode_location(data["span"]), None if function is None else _string(function), {_string(k): _string(v) for k, v in data["metadata"].items()}, + tuple(RelatedLocation(_decode_span(span), _string(message)) for span, message in data["related"]), ) diff --git a/src/coretrace_python/cli.py b/src/coretrace_python/cli.py index bdd53f5..0a0d5f9 100644 --- a/src/coretrace_python/cli.py +++ b/src/coretrace_python/cli.py @@ -280,6 +280,7 @@ def main(argv: list[str] | None = None) -> int: coverage = analysis.coverage suppressed = analysis.suppressed components = analysis.components + rules = analysis.rules if args.sbom is not None: args.sbom.write_text( render_sbom( @@ -307,6 +308,7 @@ def main(argv: list[str] | None = None) -> int: findings, coverage = file_analysis.findings, file_analysis.coverage suppressed = file_analysis.suppressed components = () + rules = file_analysis.rules baselined = None recorded = None if previous is not None: @@ -317,7 +319,7 @@ def main(argv: list[str] | None = None) -> int: elif args.baseline is not None: recorded = Baseline.of(findings, root, key) findings, baselined = (), findings - report = engine.report(findings, coverage, root, suppressed, baselined, components) + report = engine.report(findings, coverage, root, suppressed, baselined, components, rules) text = render(args.format or "text", report) # Recorded once the report is rendered, so that a failed check keeps the # previous baseline. diff --git a/src/coretrace_python/engine.py b/src/coretrace_python/engine.py index 9952e6e..911606d 100644 --- a/src/coretrace_python/engine.py +++ b/src/coretrace_python/engine.py @@ -70,6 +70,7 @@ Coverage, FileCoverage, Finding, + RuleDescription, Severity, ) from coretrace_python.findings.context import code_context, code_origin, path_in_project @@ -148,6 +149,43 @@ # checks. They are loaded by default; ``--plugins`` adds directories on top. BUNDLED_PLUGINS = Path(__file__).resolve().parent / "bundled" + +# The rules the engine reports itself, beside those of the plugins: notes on what it could not +# analyse, and the correlation of a dependency's advisory with the project's taint flows. +ENGINE_RULES: Mapping[str, RuleDescription] = MappingProxyType( + { + "syntax-error": RuleDescription( + "File that could not be read", + "A source, requirements or policy file the analyzer could not read or parse: " + "nothing in it is checked.", + ), + "ambiguous-module": RuleDescription( + "Ambiguous module name", + "A file whose import name another file of the project shares: calls through that " + "name may not be followed to the right function.", + ), + "unsupported-syntax": RuleDescription( + "Unsupported syntax", + "A function the analyzer could not analyze, because of syntax it does not support: its code is not checked.", + ), + "exploitable-vulnerability": RuleDescription( + "Exploitable dependency vulnerability", + "Attacker-controlled data reaches an API that an advisory of a vulnerable requirement affects.", + ), + } +) + + +def rule_descriptions(registry: PluginRegistry) -> Mapping[str, RuleDescription]: + """What every rule of a check detects, for its report: the rules the engine reports + itself, and those each loaded plugin describes, the first declaration of an id kept.""" + + described: dict[str, RuleDescription] = dict(ENGINE_RULES) + for loaded in registry: + for rule_id, description in loaded.plugin.rules.items(): + described.setdefault(rule_id, description) + return MappingProxyType(described) + ALL_ANALYSES: tuple[AnyAnalysis, ...] = ( *SEMANTIC_ANALYSES, CFGAnalysis, @@ -224,6 +262,10 @@ def run(cls, ctx: AnalysisContext) -> None: pass +def _no_rules() -> Mapping[str, RuleDescription]: + return MappingProxyType({}) + + def _no_keys() -> Mapping[str, str]: return MappingProxyType({}) @@ -244,6 +286,8 @@ class ProjectAnalysis: accepted: tuple[Finding, ...] = () # The plugins and advisory files the result was produced with. components: tuple[Component, ...] = () + # What each rule of the check detects, for its report. + rules: Mapping[str, RuleDescription] = field(default_factory=_no_rules) @dataclass(frozen=True) @@ -251,6 +295,8 @@ class FileAnalysis: findings: tuple[Finding, ...] coverage: Coverage suppressed: tuple[Finding, ...] = () + # What each rule of the check detects, for its report. + rules: Mapping[str, RuleDescription] = field(default_factory=_no_rules) def _text_of(sources: SourceManager) -> Callable[[SourceId], str | None]: @@ -349,7 +395,7 @@ def analyze_file(source: SourceFile, plugin_roots: Sequence[Path], root: Path | functions = len(analyzable_functions(manager.module)) coverage = Coverage((FileCoverage(str(source.source_id), "analysed", functions, len(supported)),)) kept, suppressed = partition(findings, lambda sid: source.text if sid == source.source_id else None) - return FileAnalysis(kept, coverage, suppressed) + return FileAnalysis(kept, coverage, suppressed, rule_descriptions(registry)) def resolve_dependencies(root: Path, sources: SourceManager) -> DependencyGraph: @@ -635,6 +681,7 @@ def analyze_project( suppressed, accepted, components, + rule_descriptions(registry), ) @@ -1134,6 +1181,7 @@ def report( suppressed: Sequence[Finding] = (), baselined: Sequence[Finding] | None = None, components: Sequence[Component] = (), + rules: Mapping[str, RuleDescription] | None = None, ) -> Report: """The report of a check; paths under ``root`` are rendered relative to it. ``baselined`` is the findings a baseline accounts for, ``None`` when none applied.""" @@ -1148,4 +1196,5 @@ def report( tuple(baselined or ()), baselined is not None, tuple(components), + MappingProxyType(dict(rules or {})), ) diff --git a/src/coretrace_python/findings/__init__.py b/src/coretrace_python/findings/__init__.py index c263417..ec3a20f 100644 --- a/src/coretrace_python/findings/__init__.py +++ b/src/coretrace_python/findings/__init__.py @@ -1,8 +1,15 @@ """Normalized findings produced by plugins and consumed by reporters.""" from coretrace_python.findings.coverage import Coverage, FileCoverage -from coretrace_python.findings.model import FINDING_SCHEMA_VERSION, Confidence, Finding, Severity +from coretrace_python.findings.model import ( + FINDING_SCHEMA_VERSION, + Confidence, + Finding, + RelatedLocation, + Severity, +) from coretrace_python.findings.provenance import ADVISORIES, PLUGIN, Component +from coretrace_python.findings.rules import RuleDescription __all__ = [ "ADVISORIES", @@ -13,5 +20,7 @@ "Coverage", "FileCoverage", "Finding", + "RelatedLocation", + "RuleDescription", "Severity", ] diff --git a/src/coretrace_python/findings/model.py b/src/coretrace_python/findings/model.py index 5bb5b4e..2860cc7 100644 --- a/src/coretrace_python/findings/model.py +++ b/src/coretrace_python/findings/model.py @@ -12,7 +12,7 @@ from enum import Enum from types import MappingProxyType -from coretrace_python.source import Location +from coretrace_python.source import Location, SourceSpan FINDING_SCHEMA_VERSION = 1 @@ -44,6 +44,15 @@ def _no_metadata() -> Mapping[str, str]: return MappingProxyType({}) +@dataclass(frozen=True) +class RelatedLocation: + """Another place a finding involves, such as the sink a flow reaches inside a project + function, with a short message saying what happens there.""" + + span: SourceSpan + message: str + + @dataclass(frozen=True) class Finding: rule_id: str @@ -54,6 +63,7 @@ class Finding: span: Location function: str | None = None metadata: Mapping[str, str] = field(default_factory=_no_metadata) + related: tuple[RelatedLocation, ...] = () def __post_init__(self) -> None: object.__setattr__(self, "metadata", MappingProxyType(dict(self.metadata))) diff --git a/src/coretrace_python/findings/rules.py b/src/coretrace_python/findings/rules.py new file mode 100644 index 0000000..af8db6a --- /dev/null +++ b/src/coretrace_python/findings/rules.py @@ -0,0 +1,16 @@ +"""What a rule checks, for a report to explain its findings. + +A title and a description, as a SARIF consumer shows them (``shortDescription`` and +``fullDescription``). Whoever defines a rule describes it: a plugin those it reports +(``Plugin.rules``), the engine its own (``engine.ENGINE_RULES``). +""" + +from __future__ import annotations + +from dataclasses import dataclass + + +@dataclass(frozen=True) +class RuleDescription: + title: str + description: str diff --git a/src/coretrace_python/plugins/api.py b/src/coretrace_python/plugins/api.py index ca6370c..a13db2e 100644 --- a/src/coretrace_python/plugins/api.py +++ b/src/coretrace_python/plugins/api.py @@ -12,6 +12,7 @@ from abc import ABC, abstractmethod from collections.abc import Iterable, Mapping, Sequence from pathlib import Path +from types import MappingProxyType from typing import Any, ClassVar, overload from coretrace_python.analysis import ( @@ -23,7 +24,7 @@ ) from coretrace_python.analysis.provider import R from coretrace_python.dependency import Advisory, DependencyGraph, Policy -from coretrace_python.findings import Finding +from coretrace_python.findings import Finding, RuleDescription from coretrace_python.findings.context import path_in_project from coretrace_python.hir import nodes from coretrace_python.interprocedural import ( @@ -47,6 +48,9 @@ class Plugin(ABC): requires: ClassVar[frozenset[AnyAnalysis]] = frozenset() models: ClassVar[tuple[Model, ...]] = () advisories: ClassVar[tuple[Advisory, ...]] = () + # What each rule the plugin reports detects, for a report to explain its findings (SARIF + # shortDescription and fullDescription). Optional: a rule left out is listed by id alone. + rules: ClassVar[Mapping[str, RuleDescription]] = MappingProxyType({}) @abstractmethod def analyze(self, ctx: PluginContext) -> Sequence[Finding]: diff --git a/src/coretrace_python/plugins/detectors.py b/src/coretrace_python/plugins/detectors.py index 476bddb..77df899 100644 --- a/src/coretrace_python/plugins/detectors.py +++ b/src/coretrace_python/plugins/detectors.py @@ -15,7 +15,7 @@ from typing import ClassVar from coretrace_python.analysis import AnyAnalysis -from coretrace_python.findings import Confidence, Finding, Severity +from coretrace_python.findings import Confidence, Finding, RelatedLocation, Severity from coretrace_python.findings.refutation import RefutationAnalysis, Status, Verdict from coretrace_python.hir import nodes from coretrace_python.interprocedural import CallGraphAnalysis @@ -80,10 +80,14 @@ def analyze(self, ctx: PluginContext) -> Sequence[Finding]: "evidence": verdict.evidence, **assessment.metadata, } + related: tuple[RelatedLocation, ...] = () if flow.through is not None and flow.sink_location is not None: message += f" through {flow.through}" metadata["through"] = flow.through metadata["sink_line"] = str(flow.sink_location.start_line) + related = ( + RelatedLocation(flow.sink_location, f"sink {flow.sink.symbol}, reached via {flow.through}"), + ) confidence = ( self.confidence if verdict.status is Status.VULNERABILITY else _lower(self.confidence) ) @@ -96,6 +100,7 @@ def analyze(self, ctx: PluginContext) -> Sequence[Finding]: span=flow.location, function=function.name, metadata=metadata, + related=related, ) ) return findings diff --git a/src/coretrace_python/plugins/secrets.py b/src/coretrace_python/plugins/secrets.py index 16af8ab..ec97761 100644 --- a/src/coretrace_python/plugins/secrets.py +++ b/src/coretrace_python/plugins/secrets.py @@ -23,12 +23,12 @@ import tomllib from bisect import bisect_right from collections import Counter -from collections.abc import Iterator, Sequence +from collections.abc import Iterator, Mapping, Sequence from dataclasses import dataclass, replace from pathlib import Path from typing import ClassVar -from coretrace_python.findings import Confidence, Finding, Severity +from coretrace_python.findings import Confidence, Finding, RuleDescription, Severity from coretrace_python.findings.context import is_test_code from coretrace_python.hir import nodes from coretrace_python.hir.visitors import Node, children @@ -495,6 +495,20 @@ def is_placeholder(value: str) -> bool: class SecretDetector(Plugin): """Report hardcoded secrets among the module's string literals.""" + rules: ClassVar[Mapping[str, RuleDescription]] = { + "hardcoded-secret": RuleDescription( + "Hard-coded secret", + "A secret in a provider-specific format (AWS, GitHub, Slack, Stripe, Google, a private key, a JWT, ...) written in a source or configuration file.", + ), + "hardcoded-credential": RuleDescription( + "Hard-coded credential", + "A credential-like name (password, token, api_key, ...) bound to a real value in a source or configuration file.", + ), + "high-entropy-string": RuleDescription( + "High-entropy string", + "An opaque high-entropy token that may be a secret.", + ), + } patterns: ClassVar[tuple[SecretPattern, ...]] = () credential_names: ClassVar[tuple[str, ...]] = () entropy_threshold: ClassVar[float] = 4.5 diff --git a/src/coretrace_python/reporters/report.py b/src/coretrace_python/reporters/report.py index 01d37a0..b560a1d 100644 --- a/src/coretrace_python/reporters/report.py +++ b/src/coretrace_python/reporters/report.py @@ -2,10 +2,12 @@ from __future__ import annotations -from dataclasses import dataclass +from collections.abc import Mapping +from dataclasses import dataclass, field from pathlib import Path, PurePosixPath +from types import MappingProxyType -from coretrace_python.findings import Component, Coverage, Finding +from coretrace_python.findings import Component, Coverage, Finding, RuleDescription from coretrace_python.source import FileLocation @@ -34,6 +36,8 @@ class Report: with_baseline: bool = False # The plugins and advisory files the result was produced with. components: tuple[Component, ...] = () + # What each rule detects, as the plugin reporting it or the engine describes it. + rules: Mapping[str, RuleDescription] = field(default_factory=lambda: MappingProxyType({})) def __post_init__(self) -> None: for name in ("findings", "suppressed", "baselined"): diff --git a/src/coretrace_python/reporters/sarif.py b/src/coretrace_python/reporters/sarif.py index cf7a6c2..ede71f9 100644 --- a/src/coretrace_python/reporters/sarif.py +++ b/src/coretrace_python/reporters/sarif.py @@ -3,8 +3,9 @@ from __future__ import annotations import json +from collections.abc import Mapping -from coretrace_python.findings import Component, Finding, Severity +from coretrace_python.findings import Component, Finding, RuleDescription, Severity from coretrace_python.reporters.report import Report from coretrace_python.source import SourceSpan @@ -44,11 +45,7 @@ def _result( physical: dict[str, object] = {"artifactLocation": _artifact(report, str(span.source_id))} location: dict[str, object] = {"physicalLocation": physical} if isinstance(span, SourceSpan): - region: dict[str, int] = {"startLine": span.start_line, "startColumn": span.start_column} - if span.end_line is not None and span.end_column is not None: - region["endLine"] = span.end_line - region["endColumn"] = span.end_column - physical["region"] = region + physical["region"] = _region(span) else: # No region: the line is not established; the pointer says where in the file. location["properties"] = {"pointer": span.pointer} @@ -56,10 +53,10 @@ def _result( "ruleId": finding.rule_id, "ruleIndex": rule_index, "level": _LEVELS[finding.severity], - "message": {"text": finding.message}, + "message": {"text": _message(finding)}, "locations": [location], } - properties: dict[str, str] = {} + properties: dict[str, str] = {"confidence": finding.confidence.value} injection = finding.metadata.get("injection") if injection in _INJECTIONS: properties["injection"] = injection @@ -69,8 +66,19 @@ def _result( origin = finding.metadata.get("origin") if origin in _ORIGINS: properties["origin"] = origin - if properties: - result["properties"] = properties + result["properties"] = properties + if finding.related: + result["relatedLocations"] = [ + { + "id": index, + "physicalLocation": { + "artifactLocation": _artifact(report, str(related.span.source_id)), + "region": _region(related.span), + }, + "message": {"text": related.message}, + } + for index, related in enumerate(finding.related) + ] if suppressed: result["suppressions"] = [{"kind": "inSource"}] if baseline_state is not None: @@ -88,6 +96,44 @@ def _extension(component: Component) -> dict[str, object]: return extension +def _escaped(text: str) -> str: + """``text`` as literal SARIF message text, beside embedded links (SARIF 2.1.0 ยง3.11.6).""" + + return text.replace("\\", "\\\\").replace("[", "\\[").replace("]", "\\]") + + +def _message(finding: Finding) -> str: + """The finding's message, ending with an embedded link to each related location, which + GitHub code scanning shows as a link; the other formats keep the message as it is.""" + + if not finding.related: + return finding.message + links = ", ".join(f"[{_escaped(related.message)}]({index})" for index, related in enumerate(finding.related)) + return f"{_escaped(finding.message)} ({links})" + + +def _region(span: SourceSpan) -> dict[str, int]: + region = {"startLine": span.start_line, "startColumn": span.start_column} + if span.end_line is not None and span.end_column is not None: + region["endLine"] = span.end_line + region["endColumn"] = span.end_column + return region + + +def _rule(rule_id: str, rules: Mapping[str, RuleDescription]) -> dict[str, object]: + """A rule as the log lists it: its title and description when the report has them, its + id alone otherwise, never the id repeated as a title.""" + + described = rules.get(rule_id) + if described is None: + return {"id": rule_id} + return { + "id": rule_id, + "shortDescription": {"text": described.title}, + "fullDescription": {"text": described.description}, + } + + def render_sarif(report: Report) -> str: rule_ids: list[str] = [] for finding in (*report.findings, *report.suppressed, *report.baselined): @@ -103,10 +149,7 @@ def render_sarif(report: Report) -> str: "driver": { "name": report.tool_name, "version": report.tool_version, - "rules": [ - {"id": rule_id, "shortDescription": {"text": rule_id}} - for rule_id in rule_ids - ], + "rules": [_rule(rule_id, report.rules) for rule_id in rule_ids], }, **({"extensions": [_extension(c) for c in report.components]} if report.components else {}), }, diff --git a/tests/test_code_injection.py b/tests/test_code_injection.py index f9860cc..9c20180 100644 --- a/tests/test_code_injection.py +++ b/tests/test_code_injection.py @@ -61,6 +61,35 @@ def test_dangerous_eval_still_reports_the_call_beside_the_injection() -> None: assert rules == ["code-injection", "dangerous-eval"] +def test_the_sink_inside_a_project_function_is_a_related_location() -> None: + text = "def evaluate(expression):\n return eval(expression)\n\ndef run():\n return evaluate(input())\n" + + (finding,) = [f for f in check(text) if f.rule_id == "code-injection"] + (related,) = finding.related + assert (str(related.span.source_id), related.span.start_line) == ("app.py", 2) + assert related.message == "sink python.builtins.eval, reached via evaluate" + + +def test_a_sink_two_calls_away_is_reached_via_the_first_one() -> None: + text = ( + "def evaluate(expression):\n return eval(expression)\n\n" + "def relay(value):\n return evaluate(value)\n\n" + "def run():\n return relay(input())\n" + ) + + (finding,) = [f for f in check(text) if f.rule_id == "code-injection"] + (related,) = finding.related + # The sink is in evaluate; relay is the call the flow goes through, not where the sink is. + assert related.span.start_line == 2 + assert related.message == "sink python.builtins.eval, reached via relay" + + +def test_a_sink_in_the_function_itself_has_no_related_location() -> None: + (finding,) = [f for f in check("def run():\n return eval(input())\n") if f.rule_id == "code-injection"] + + assert finding.related == () + + def test_code_reaching_eval_through_a_project_function_is_reported_at_the_call() -> None: text = "def evaluate(expression):\n return eval(expression)\n\ndef run():\n return evaluate(input())\n" diff --git a/tests/test_reporters.py b/tests/test_reporters.py index f3f354b..74c564e 100644 --- a/tests/test_reporters.py +++ b/tests/test_reporters.py @@ -11,11 +11,19 @@ import ast import json +from dataclasses import replace from pathlib import Path import pytest -from coretrace_python.findings import FINDING_SCHEMA_VERSION, Confidence, Finding, Severity +from coretrace_python.findings import ( + FINDING_SCHEMA_VERSION, + Confidence, + Finding, + RelatedLocation, + RuleDescription, + Severity, +) from coretrace_python.source import SourceId, SourceSpan try: @@ -170,6 +178,84 @@ def test_sarif_reporter_emits_a_single_run_with_rules_and_results() -> None: ] +def test_sarif_rules_carry_the_description_the_report_gives_them() -> None: + described = Report( + findings=(finding("dangerous-eval", 2),), + tool_name="coretrace-python-analyzer", + tool_version="0.1.0", + rules={"dangerous-eval": RuleDescription("Dynamic code execution", "A call to eval or exec.")}, + ) + + assert json.loads(render_sarif(described))["runs"][0]["tool"]["driver"]["rules"] == [ + { + "id": "dangerous-eval", + "shortDescription": {"text": "Dynamic code execution"}, + "fullDescription": {"text": "A call to eval or exec."}, + } + ] + + +def test_a_rule_without_a_description_keeps_its_id_alone() -> None: + document = json.loads(render_sarif(report(finding("from-a-third-party-plugin", 1)))) + + assert document["runs"][0]["tool"]["driver"]["rules"] == [{"id": "from-a-third-party-plugin"}] + + +def test_sarif_results_carry_their_confidence() -> None: + def confidence(level: Confidence) -> object: + document = json.loads(render_sarif(report(replace(finding("r", 1), confidence=level)))) + return document["runs"][0]["results"][0]["properties"]["confidence"] + + assert confidence(Confidence.HIGH) == "high" + assert confidence(Confidence.MEDIUM) == "medium" + assert confidence(Confidence.LOW) == "low" + + +def test_the_confidence_sits_beside_the_other_properties() -> None: + document = json.loads(render_sarif(report(finding("command-injection", 1, injection="option")))) + + assert document["runs"][0]["results"][0]["properties"] == {"confidence": "high", "injection": "option"} + + +def test_sarif_results_point_at_the_other_places_a_finding_involves() -> None: + sink = SourceSpan(SourceId("app/helpers.py"), 12, 9, 12, 21) + involved = replace(finding("code-injection", 3), related=(RelatedLocation(sink, "eval runs the input here"),)) + + (result,) = json.loads(render_sarif(report(involved)))["runs"][0]["results"] + + assert result["relatedLocations"] == [ + { + "id": 0, + "physicalLocation": { + "artifactLocation": {"uri": "app/helpers.py"}, + "region": {"startLine": 12, "startColumn": 9, "endLine": 12, "endColumn": 21}, + }, + "message": {"text": "eval runs the input here"}, + } + ] + + +def test_the_message_links_to_each_related_location() -> None: + sink = SourceSpan(SourceId("app/helpers.py"), 12, 9, 12, 21) + involved = replace( + finding("code-injection", 3), + message="a[0] reaches eval", + related=(RelatedLocation(sink, "sink eval"), RelatedLocation(sink, "sink [exec]")), + ) + + (result,) = json.loads(render_sarif(report(involved)))["runs"][0]["results"] + + # SARIF embedded links: [text](id) points at relatedLocations[id], which GitHub code + # scanning shows as a link; literal brackets and backslashes are escaped around them. + assert result["message"] == {"text": "a\\[0\\] reaches eval ([sink eval](0), [sink \\[exec\\]](1))"} + + +def test_a_result_without_other_places_has_no_related_locations() -> None: + (result,) = json.loads(render_sarif(report(finding("code-injection", 3))))["runs"][0]["results"] + + assert "relatedLocations" not in result + + def test_sarif_levels_follow_severity() -> None: def level(severity: Severity) -> str: document = json.loads(render_sarif(report(finding("r", 1, severity=severity)))) diff --git a/tests/test_rule_descriptions.py b/tests/test_rule_descriptions.py new file mode 100644 index 0000000..71f2320 --- /dev/null +++ b/tests/test_rule_descriptions.py @@ -0,0 +1,157 @@ +"""Acceptance tests: the SARIF log describes every rule it lists (#270). + +A rule's title and description belong to whoever defines the rule: a plugin declares those +of the rules it reports, bundled or third-party; the engine those of its own notes. They +travel with the analysis to the report. A plugin that declares nothing still loads and +reports, its rules listed by id alone. +""" + +from __future__ import annotations + +import ast +import contextlib +import io +import json +from collections.abc import Mapping +from pathlib import Path + +import pytest + +from coretrace_python import engine +from coretrace_python.cli import main +from coretrace_python.findings import RuleDescription +from coretrace_python.source import SourceManager + +MANIFEST = ( + 'name = "{name}"\nversion = "1.0.0"\nplugin_api = ">=1,<2"\n' + 'requires = ["ir.ssa"]\nprovides = []\n\n' + '[entrypoint]\nmodule = "{module}"\nclass = "{cls}"\n' +) + +DESCRIBED = """ +from typing import ClassVar + +from coretrace_python.findings import Severity +from coretrace_python.findings.rules import RuleDescription +from coretrace_python.plugins import SymbolCallDetector +from coretrace_python.semantic.symbols import SymbolId + + +class ForbiddenCall(SymbolCallDetector): + name: ClassVar[str] = "forbidden-call" + rule_id: ClassVar[str] = "forbidden-call" + symbols: ClassVar[frozenset[SymbolId]] = frozenset({SymbolId("python.tempfile.gettempdir")}) + severity: ClassVar[Severity] = Severity.LOW + message_template: ClassVar[str] = "call to {symbol} is forbidden here" + rules = {"forbidden-call": RuleDescription("Forbidden call", "A call to a function the project forbids.")} +""" + +UNDESCRIBED = """ +from typing import ClassVar + +from coretrace_python.findings import Severity +from coretrace_python.plugins import SymbolCallDetector +from coretrace_python.semantic.symbols import SymbolId + + +class QuietCall(SymbolCallDetector): + name: ClassVar[str] = "quiet-call" + rule_id: ClassVar[str] = "quiet-call" + symbols: ClassVar[frozenset[SymbolId]] = frozenset({SymbolId("python.tempfile.gettempprefix")}) + severity: ClassVar[Severity] = Severity.LOW + message_template: ClassVar[str] = "call to {symbol}" +""" + + +def plugin(root: Path, name: str, module: str, cls: str, code: str) -> None: + directory = root / name + directory.mkdir(parents=True) + (directory / "plugin.toml").write_text(MANIFEST.format(name=name, module=module, cls=cls)) + (directory / f"{module}.py").write_text(code) + + +def sarif_rules(tmp_path: Path, source: str) -> dict[str, dict[str, object]]: + plugins = tmp_path / "plugins" + plugin(plugins, "forbidden-call", "forbidden", "ForbiddenCall", DESCRIBED) + plugin(plugins, "quiet-call", "quiet", "QuietCall", UNDESCRIBED) + path = tmp_path / "app.py" + path.write_text(source) + out = io.StringIO() + with contextlib.redirect_stdout(out): + main(["--check", str(path), "--format", "sarif", "--plugins", str(plugins)]) + rules = json.loads(out.getvalue())["runs"][0]["tool"]["driver"]["rules"] + return {rule["id"]: rule for rule in rules} + + +def test_a_plugin_describes_the_rules_it_reports(tmp_path: Path) -> None: + rules = sarif_rules(tmp_path, "import tempfile\n\ndef run():\n return tempfile.gettempdir()\n") + + assert rules["forbidden-call"] == { + "id": "forbidden-call", + "shortDescription": {"text": "Forbidden call"}, + "fullDescription": {"text": "A call to a function the project forbids."}, + } + + +def test_a_plugin_that_describes_nothing_still_reports_by_id(tmp_path: Path) -> None: + rules = sarif_rules(tmp_path, "import tempfile\n\ndef run():\n return tempfile.gettempprefix()\n") + + assert rules["quiet-call"] == {"id": "quiet-call"} + + +def bundled_rules() -> Mapping[str, RuleDescription]: + """What the bundled plugins and the engine describe, as a check carries it to its report.""" + + return engine.analyze_file(SourceManager().add_source("app.py", ""), [engine.BUNDLED_PLUGINS]).rules + + +def documented_rules() -> list[str]: + """The rule ids the Rules section of the usage guide lists, and ``unsupported-syntax``.""" + + usage = (Path(__file__).resolve().parent.parent / "docs" / "usage.md").read_text(encoding="utf-8") + section = usage.split("\n## Rules\n", 1)[1].split("\n## ", 1)[0] + rules = {line.split("`")[1] for line in section.splitlines() if line.startswith("| `")} + return sorted(rules | {"unsupported-syntax"}) + + +@pytest.mark.parametrize("rule", documented_rules()) +def test_every_documented_rule_is_described(rule: str) -> None: + described = bundled_rules().get(rule) + + assert described is not None + assert described.title not in ("", rule) + assert described.description + + +def reportable_rules() -> set[str]: + """Every rule id the code gives a finding literally, the engine's notes included: a + rule_id class attribute, a rule_id argument, or the first argument of Finding or of the + engine's _note.""" + + source = Path(__file__).resolve().parent.parent / "src" / "coretrace_python" + found: set[str] = set() + for path in source.rglob("*.py"): + for node in ast.walk(ast.parse(path.read_text(encoding="utf-8"))): + if isinstance(node, ast.AnnAssign) and getattr(node.target, "id", None) == "rule_id": + value = node.value + elif isinstance(node, ast.Call) and getattr(node.func, "id", None) in ("Finding", "_note"): + keyword = next((k.value for k in node.keywords if k.arg == "rule_id"), None) + value = keyword or (node.args[0] if node.args else None) + else: + continue + if isinstance(value, ast.Constant) and isinstance(value.value, str): + found.add(value.value) + return found + + +def test_the_rules_are_read_from_the_code() -> None: + assert {"dangerous-eval", "hardcoded-secret", "unsupported-syntax", "syntax-error"} <= reportable_rules() + + +@pytest.mark.parametrize("rule", sorted(reportable_rules())) +def test_every_rule_the_code_reports_is_described(rule: str) -> None: + described = bundled_rules().get(rule) + + assert described is not None + assert described.title not in ("", rule) + assert described.description diff --git a/tests/test_sarif_injection.py b/tests/test_sarif_injection.py index ffc5a8f..c44ca10 100644 --- a/tests/test_sarif_injection.py +++ b/tests/test_sarif_injection.py @@ -47,12 +47,14 @@ def results( def test_a_result_carries_the_injection_its_rule_established( results: dict[str, dict[str, object]], ) -> None: - assert results["option.py"]["properties"] == {"injection": "option"} - assert results["command.py"]["properties"] == {"injection": "command"} + # Beside the confidence every result carries (#271), the injection is the only property. + assert results["option.py"]["properties"] == {"confidence": "high", "injection": "option"} + assert results["command.py"]["properties"] == {"confidence": "high", "injection": "command"} def test_a_result_whose_injection_is_not_established_has_none( results: dict[str, dict[str, object]], ) -> None: - assert "properties" not in results["unknown.py"] - assert "properties" not in results["shell.py"] + # Every result carries its confidence (#271): nothing else, and no injection. + assert set(results["unknown.py"]["properties"]) == {"confidence"} + assert set(results["shell.py"]["properties"]) == {"confidence"} diff --git a/tests/test_vendored_context.py b/tests/test_vendored_context.py index 72e6e40..169c80e 100644 --- a/tests/test_vendored_context.py +++ b/tests/test_vendored_context.py @@ -122,9 +122,10 @@ def test_the_sarif_result_carries_both_facts(tmp_path: Path) -> None: results = json.loads(render_sarif(engine.report(analysis.findings, root=root)))["runs"][0]["results"] + # Test code is reported at low confidence, which every result carries (#271). assert {r["ruleId"]: r["properties"] for r in results} == { - "hardcoded-credential": {"context": "test", "origin": "vendored"}, - "unsafe-xml": {"context": "test", "origin": "vendored"}, + "hardcoded-credential": {"confidence": "low", "context": "test", "origin": "vendored"}, + "unsafe-xml": {"confidence": "low", "context": "test", "origin": "vendored"}, }