Skip to content

Adopt strict ruff and mypy, reformat and fix the codebase #128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
zeevmoney wants to merge 1 commit into
base: per-16221/uv-migration
Choose a base branch
from
Draft

Adopt strict ruff and mypy, reformat and fix the codebase #128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .github/dependabot.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ updates:
default-days: 7
semver-major-days: 14
groups:
# Listed first, since a dependency joins the first group it matches. A new
# lint rule or type-check error in one of these must not hold up the
# runtime floor bumps grouped below, so they get a PR of their own.
lint-tools:
patterns: ["ruff", "mypy", "typos"]
minor-and-patch:
update-types: ["minor", "patch"]
ignore:
Expand All @@ -49,6 +54,31 @@ updates:
labels:
- "dependencies"

# pre-commit hook revisions in .pre-commit-config.yaml. Dependabot follows the
# `# frozen: vX` comment on SHA-pinned revs and rewrites the SHA and the
# comment together. `repo: local` hooks (ruff, mypy, typos) are skipped:
# their versions come from uv.lock, which the "uv" entry above maintains.
# Only `default-days` cooldown is supported for this ecosystem.
- package-ecosystem: "pre-commit"
directory: "/"
schedule:
interval: "weekly"
day: "monday"
open-pull-requests-limit: 5
cooldown:
default-days: 7
groups:
minor-and-patch:
update-types: ["minor", "patch"]
ignore:
# The uv-lock hook's version must equal [tool.uv] required-version in
# pyproject.toml; uv is upgraded by hand, all three places at once.
- dependency-name: "*astral-sh/uv-pre-commit"
commit-message:
prefix: "deps"
labels:
- "dependencies"

# GitHub Actions versions.
# Note: cooldown.semver-major-days is not supported for github-actions --
# Dependabot only honours it on semver-strict ecosystems like uv and npm.
Expand Down
112 changes: 77 additions & 35 deletions .github/scripts/format_audit.py
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import json
import sys
from pathlib import Path
from typing import Any, Optional
from typing import Any

MARKER = "<!-- permit-python:audit:deps -->"

Expand All @@ -49,11 +49,17 @@
# the string render as markdown/HTML in the comment and the job summary.
FENCE = "~~~~~~"

# GitHub truncates annotation text; cut it ourselves so the ellipsis is visible.
_ANNOTATION_MAX_CHARS = 200
# The Slack message is two header lines, then one line per package.
_SLACK_HEADER_LINES = 2
_SLACK_MAX_PACKAGES = 10


class Finding:
"""One vulnerability, normalized across scanners."""

def __init__(
def __init__( # noqa: PLR0917 - one field per scanner column; built positionally
self,
vuln_id: str,
package: str,
Expand All @@ -63,7 +69,7 @@ def __init__(
title: str,
url: str,
source: str,
):
) -> None:
self.id = vuln_id
self.package = package
self.installed = installed
Expand All @@ -75,6 +81,7 @@ def __init__(

@property
def key(self) -> tuple[str, str]:
"""Identity used to merge the same advisory reported by several scanners."""
return (self.package, self.id)

@property
Expand All @@ -99,7 +106,7 @@ def _md_cell(text: str) -> str:
return _truncate(text, 140).replace("|", "\\|").replace("`", "'")


def _load(path: Optional[str], label: str) -> tuple[Optional[Any], Optional[str]]:
def _load(path: str | None, label: str) -> tuple[Any | None, str | None]:
"""Return (parsed, error). Never raises -- a bad report must not kill the run."""
if not path:
return None, None
Expand All @@ -115,7 +122,7 @@ def _load(path: Optional[str], label: str) -> tuple[Optional[Any], Optional[str]
return None, f"{label}: {path} is not valid JSON: {exc}"


def trivy_scanned_nothing(doc: Any) -> bool:
def trivy_scanned_nothing(doc: object) -> bool:
"""True when Trivy produced no package Result at all.

Trivy writes {"Results": null} and exits 0 when it recognises no package
Expand All @@ -132,7 +139,8 @@ def trivy_scanned_nothing(doc: Any) -> bool:
return not any(isinstance(r, dict) and r.get("Target") for r in results)


def parse_trivy(doc: Any, source: str = "trivy") -> list[Finding]:
def parse_trivy(doc: object, source: str = "trivy") -> list[Finding]:
"""Extract the findings of a Trivy JSON report; malformed entries are skipped."""
findings: list[Finding] = []
if not isinstance(doc, dict):
return findings
Expand All @@ -158,7 +166,7 @@ def parse_trivy(doc: Any, source: str = "trivy") -> list[Finding]:
return findings


def parse_pip_audit(doc: Any) -> list[Finding]:
def parse_pip_audit(doc: object) -> list[Finding]:
"""pip-audit carries no severity at all, so everything lands in UNKNOWN.

That is why pip-audit is advisory-only here and never gates the build: it
Expand All @@ -179,7 +187,9 @@ def parse_pip_audit(doc: Any) -> list[Finding]:
if not isinstance(vuln, dict):
continue
fixes = vuln.get("fix_versions") or []
fixed = ", ".join(str(f) for f in fixes) if isinstance(fixes, list) and fixes else NO_FIX
fixed = (
", ".join(str(f) for f in fixes) if isinstance(fixes, list) and fixes else NO_FIX
)
aliases = vuln.get("aliases") or []
alias_str = ""
if isinstance(aliases, list) and aliases:
Expand Down Expand Up @@ -232,17 +242,20 @@ def _annotation_escape(text: str) -> str:
later replacements introduce.
"""
text = str(text)
text = text if len(text) <= 200 else text[:199] + "…"
text = text if len(text) <= _ANNOTATION_MAX_CHARS else text[: _ANNOTATION_MAX_CHARS - 1] + "…"
return text.replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A")


def render_annotations(findings: list[Finding]) -> str:
"""Render one GitHub `::error` workflow command per blocking finding."""
lines = []
for finding in findings:
if not finding.blocking:
continue
title = _annotation_escape(f"{finding.severity}: {finding.id} in {finding.package}")
body = _annotation_escape(f"{finding.package} {finding.installed} -- fixed in {finding.fixed}. {finding.title}")
body = _annotation_escape(
f"{finding.package} {finding.installed} -- fixed in {finding.fixed}. {finding.title}"
)
lines.append(f"::error title={title}::{body}")
return "\n".join(lines)

Expand Down Expand Up @@ -299,7 +312,9 @@ def render_slack(findings: list[Finding], errors: list[str], run_url: str, repo:
# Highest fix target across the group -- upgrading to anything lower
# would leave part of the group unresolved.
targets = sorted({f.fixed for f in group if f.fixed != NO_FIX})
target = f" — upgrade to `{_slack_escape(targets[-1])}`" if targets else " — no fix available"
target = (
f" — upgrade to `{_slack_escape(targets[-1])}`" if targets else " — no fix available"
)
installed = _slack_escape(worst.installed)
lines.append(
f">• `{_slack_escape(package)}` {installed} — "
Expand All @@ -308,21 +323,31 @@ def render_slack(findings: list[Finding], errors: list[str], run_url: str, repo:
)

# Slack truncates long messages; keep it to something a human will read.
if len(lines) > 12:
lines = lines[:12] + [f">…and {len(by_package) - 10} more packages."]
if len(lines) > _SLACK_HEADER_LINES + _SLACK_MAX_PACKAGES:
lines = [
*lines[: _SLACK_HEADER_LINES + _SLACK_MAX_PACKAGES],
f">…and {len(by_package) - _SLACK_MAX_PACKAGES} more packages.",
]

lines.append(f">{link}")
return "\n".join(lines)


def render(
def _advisory_link(finding: Finding) -> str:
if finding.url.startswith("http"):
return f"[{_md_cell(finding.id)}]({finding.url})"
return _md_cell(finding.id)


def render( # noqa: C901, PLR0915 - one linear pass appending each report section
findings: list[Finding],
errors: list[str],
context: str,
*,
blocking: bool,
warnings: Optional[list[str]] = None,
warnings: list[str] | None = None,
) -> str:
"""Render the markdown PR comment body."""
out: list[str] = [MARKER, "", "## Dependency Security Audit", ""]

if context:
Expand Down Expand Up @@ -370,7 +395,10 @@ def render(
out.append(f":x: **{len(blockers)} fixable HIGH/CRITICAL {noun}** -- {verb}.")
if unfixable:
out.append("")
out.append(f":warning: A further **{unfixable}** HIGH/CRITICAL have no fix available yet and do not block.")
out.append(
f":warning: A further **{unfixable}** HIGH/CRITICAL have no fix available yet "
"and do not block."
)
elif severe:
# Do not say "none at HIGH or CRITICAL" here: there are some, they
# just cannot be fixed by bumping a bound. Saying otherwise would
Expand All @@ -381,33 +409,39 @@ def render(
"but they are real exposure and need a decision."
)
else:
out.append(":warning: Advisories found, but none at HIGH or CRITICAL. This does not block the build.")
out.append(
":warning: Advisories found, but none at HIGH or CRITICAL. "
"This does not block the build."
)
out.append("")

out.append("| Severity | Count |")
out.append("| --- | --- |")
for severity in SEVERITY_ORDER:
if counts.get(severity):
out.append(f"| {SEVERITY_EMOJI[severity]} {severity} | {counts[severity]} |")
out.extend(
f"| {SEVERITY_EMOJI[severity]} {severity} | {counts[severity]} |"
for severity in SEVERITY_ORDER
if counts.get(severity)
)
out.append("")

out.append("| Severity | Package | Installed | Fixed in | Advisory |")
out.append("| --- | --- | --- | --- | --- |")
for finding in findings:
link = f"[{_md_cell(finding.id)}]({finding.url})" if finding.url.startswith("http") else _md_cell(finding.id)
out.append(
f"| {SEVERITY_EMOJI[finding.severity]} {finding.severity} "
f"| `{_md_cell(finding.package)}` "
f"| `{_md_cell(finding.installed)}` "
f"| `{_md_cell(finding.fixed)}` "
f"| {link} |"
)
out.extend(
f"| {SEVERITY_EMOJI[finding.severity]} {finding.severity} "
f"| `{_md_cell(finding.package)}` "
f"| `{_md_cell(finding.installed)}` "
f"| `{_md_cell(finding.fixed)}` "
f"| {_advisory_link(finding)} |"
for finding in findings
)
out.append("")

out.append("<details><summary>Advisory details</summary>")
out.append("")
for finding in findings:
out.append(f"**{finding.severity} -- {finding.id}** (`{finding.package}` {finding.installed})")
out.append(
f"**{finding.severity} -- {finding.id}** (`{finding.package}` {finding.installed})"
)
out.append("")
out.append(f"Found by: {', '.join(sorted(finding.sources))}")
out.append("")
Expand Down Expand Up @@ -439,7 +473,8 @@ def render(
return "\n".join(out) + "\n"


def main() -> int:
def main() -> int: # noqa: C901, PLR0912 - argument handling, then one pass per output mode
"""Parse the scanner reports and print the requested output; return the exit status."""
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument(
"trivy_json",
Expand Down Expand Up @@ -468,8 +503,12 @@ def main() -> int:
action="store_true",
help="emit a single-line Slack message body carrying the findings",
)
parser.add_argument("--run-url", default="", help="workflow run URL to link from the Slack message")
parser.add_argument("--repo", default="permit-python", help="repository name for the Slack message")
parser.add_argument(
"--run-url", default="", help="workflow run URL to link from the Slack message"
)
parser.add_argument(
"--repo", default="permit-python", help="repository name for the Slack message"
)
parser.add_argument(
"--gate",
action="store_true",
Expand Down Expand Up @@ -523,7 +562,8 @@ def main() -> int:
blockers = [f for f in findings if f.blocking]
for finding in blockers:
print(
f"{finding.severity} {finding.id} {finding.package} " f"{finding.installed} -> {finding.fixed}",
f"{finding.severity} {finding.id} {finding.package} "
f"{finding.installed} -> {finding.fixed}",
file=sys.stderr,
)
if errors:
Expand All @@ -537,7 +577,9 @@ def main() -> int:
print(rendered)
return 0

sys.stdout.write(render(findings, errors, args.context, blocking=args.blocking, warnings=warnings))
sys.stdout.write(
render(findings, errors, args.context, blocking=args.blocking, warnings=warnings)
)
return 0


Expand Down
Loading
Loading