Skip to content
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.

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

cli, service: better error handling for connection errors #287

Merged
merged 2 commits into from
May 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ All versions prior to 0.0.9 are untracked.
faster and more responsive
([#283](https://github.com/trailofbits/pip-audit/pull/283))

* CLI, Vulnerability sources: the error message used to report
connection failures to vulnerability sources was improved
([#287](https://github.com/trailofbits/pip-audit/pull/287))

### Fixed

* Vulnerability sources: a bug stemming from an incorrect assumption
Expand Down
46 changes: 28 additions & 18 deletions pip_audit/_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from pip_audit._fix import ResolvedFixVersion, SkippedFixVersion, resolve_fix_versions
from pip_audit._format import ColumnsFormat, CycloneDxFormat, JsonFormat, VulnerabilityFormat
from pip_audit._service import OsvService, PyPIService, VulnerabilityService
from pip_audit._service.interface import ConnectionError as VulnServiceConnectionError
from pip_audit._service.interface import ResolvedDependency, SkippedDependency
from pip_audit._state import AuditSpinner, AuditState
from pip_audit._util import assert_never
Expand Down Expand Up @@ -400,25 +401,34 @@ def audit() -> None:
skip_count = 0
vuln_ignore_count = 0
vulns_to_ignore = set(args.ignore_vulns)
for (spec, vulns) in auditor.audit(source):
if spec.is_skipped():
spec = cast(SkippedDependency, spec)
if args.strict:
_fatal(f"{spec.name}: {spec.skip_reason}")
try:
for (spec, vulns) in auditor.audit(source):
if spec.is_skipped():
spec = cast(SkippedDependency, spec)
if args.strict:
_fatal(f"{spec.name}: {spec.skip_reason}")
else:
state.update_state(f"Skipping {spec.name}: {spec.skip_reason}")
skip_count += 1
else:
state.update_state(f"Skipping {spec.name}: {spec.skip_reason}")
skip_count += 1
else:
spec = cast(ResolvedDependency, spec)
state.update_state(f"Auditing {spec.name} ({spec.version})")
if vulns_to_ignore:
filtered_vulns = [v for v in vulns if not v.has_any_id(vulns_to_ignore)]
vuln_ignore_count += len(vulns) - len(filtered_vulns)
vulns = filtered_vulns
result[spec] = vulns
if len(vulns) > 0:
pkg_count += 1
vuln_count += len(vulns)
spec = cast(ResolvedDependency, spec)
state.update_state(f"Auditing {spec.name} ({spec.version})")
if vulns_to_ignore:
filtered_vulns = [v for v in vulns if not v.has_any_id(vulns_to_ignore)]
vuln_ignore_count += len(vulns) - len(filtered_vulns)
vulns = filtered_vulns
result[spec] = vulns
if len(vulns) > 0:
pkg_count += 1
vuln_count += len(vulns)
except VulnServiceConnectionError as e:
# The most common source of connection errors is corporate blocking,
# so we offer a bit of advice.
logger.error(str(e))
_fatal(
"Tip: your network may be blocking this service. "
"Try another service with `-s SERVICE`"
)

# If the `--fix` flag has been applied, find a set of suitable fix versions and upgrade the
# dependencies at the source
Expand Down
2 changes: 2 additions & 0 deletions pip_audit/_service/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"""

from .interface import (
ConnectionError,
Dependency,
ResolvedDependency,
ServiceError,
Expand All @@ -14,6 +15,7 @@
from .pypi import PyPIService

__all__ = [
"ConnectionError",
"Dependency",
"ResolvedDependency",
"ServiceError",
Expand Down
9 changes: 9 additions & 0 deletions pip_audit/_service/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,3 +158,12 @@ class ServiceError(Exception):
"""

pass


class ConnectionError(ServiceError):
"""
A specialization of `ServiceError` specifically for cases where the
vulnerability service is unreachable or offline.
"""

pass
18 changes: 9 additions & 9 deletions pip_audit/_service/osv.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

from pip_audit._cache import caching_session
from pip_audit._service.interface import (
ConnectionError,
Dependency,
ResolvedDependency,
ServiceError,
Expand Down Expand Up @@ -56,24 +57,23 @@ def query(self, spec: Dependency) -> Tuple[Dependency, List[VulnerabilityResult]
"package": {"name": spec.canonical_name, "ecosystem": "PyPI"},
"version": str(spec.version),
}
response: requests.Response = self.session.post(
url=url,
data=json.dumps(query),
timeout=self.timeout,
)

results: List[VulnerabilityResult] = []

# Check for an unsuccessful status code
try:
response: requests.Response = self.session.post(
url=url,
data=json.dumps(query),
timeout=self.timeout,
)
response.raise_for_status()
except requests.ConnectTimeout:
raise ConnectionError("Could not connect to OSV's vulnerability feed")
except requests.HTTPError as http_error:
raise ServiceError from http_error

# If the response is empty, that means that the package/version pair doesn't have any
# associated vulnerabilities
#
# In that case, return an empty list
results: List[VulnerabilityResult] = []
response_json = response.json()
if not response_json:
return spec, results
Expand Down
11 changes: 10 additions & 1 deletion pip_audit/_service/pypi.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

from pip_audit._cache import caching_session
from pip_audit._service.interface import (
ConnectionError,
Dependency,
ResolvedDependency,
ServiceError,
Expand Down Expand Up @@ -55,9 +56,17 @@ def query(self, spec: Dependency) -> Tuple[Dependency, List[VulnerabilityResult]
spec = cast(ResolvedDependency, spec)

url = f"https://pypi.org/pypi/{spec.canonical_name}/{str(spec.version)}/json"
response: requests.Response = self.session.get(url=url, timeout=self.timeout)

try:
response: requests.Response = self.session.get(url=url, timeout=self.timeout)
response.raise_for_status()
except requests.ConnectTimeout:
# Apart from a normal network outage, this can happen for two main
# reasons:
# 1. PyPI's APIs are offline
# 2. The user is behind a firewall or corporate network that blocks
# PyPI (and they're probably using custom indices)
raise ConnectionError("Could not connect to PyPI's vulnerability feed")
except requests.HTTPError as http_error:
if response.status_code == 404:
skip_reason = (
Expand Down
13 changes: 12 additions & 1 deletion test/service/test_osv.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import pretend # type: ignore
import pytest
from packaging.version import Version
from requests.exceptions import HTTPError
from requests.exceptions import ConnectTimeout, HTTPError

import pip_audit._service as service

Expand Down Expand Up @@ -95,6 +95,17 @@ def test_osv_no_vuln():
assert len(vulns) == 0


def test_osv_connection_error(monkeypatch):
osv = service.OsvService()
monkeypatch.setattr(osv.session, "post", pretend.raiser(ConnectTimeout))

dep = service.ResolvedDependency("jinja2", Version("2.4.1"))
with pytest.raises(
service.ConnectionError, match="Could not connect to OSV's vulnerability feed"
):
dict(osv.query_all(iter([dep])))


def test_osv_error_response(monkeypatch):
def raise_for_status():
raise HTTPError
Expand Down
14 changes: 14 additions & 0 deletions test/service/test_pypi.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,20 @@ def test_pypi_multiple_pkg(cache_dir):
assert len(results[deps[1]]) > 0


def test_pypi_connection_error(monkeypatch):
session = pretend.stub(get=pretend.raiser(requests.ConnectTimeout))
caching_session = pretend.call_recorder(lambda c, **kw: session)
monkeypatch.setattr(service.pypi, "caching_session", caching_session)

cache_dir = pretend.stub()
pypi = service.PyPIService(cache_dir)

with pytest.raises(
service.ConnectionError, match="Could not connect to PyPI's vulnerability feed"
):
dict(pypi.query_all(iter([service.ResolvedDependency("fakedep", Version("1.0.0"))])))


def test_pypi_http_notfound(monkeypatch, cache_dir):
# If we get a "not found" response, that means that we're querying a package or version that
# isn't known to PyPI. If that's the case, we should just log a debug message and continue on
Expand Down