fix(pypi): check both author and maintainer email in get_email_addresses - #810
fix(pypi): check both author and maintainer email in get_email_addresses#810arpitjain099 wants to merge 1 commit into
Conversation
The PyPI get_email_addresses helper used
{author_email or maintainer_email}, so the 'or' short-circuits and
only the author email is ever returned when it is set. The three
detectors that consume it (unclaimed_maintainer_email_domain,
potentially_compromised_email_domain, deceptive_author) therefore
never look at the maintainer email when an author email is present,
so a hijackable maintainer domain is reported as clean. The npm
get_email_addresses already returns the full set of emails.
Return both addresses (dropping None and empty), and add a unit test.
Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc0e75c914
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| info = package_info.get("info", {}) | ||
|
|
||
| return {info.get("author_email") or info.get("maintainer_email")} - {None} | ||
| return {info.get("author_email"), info.get("maintainer_email")} - {None, ""} |
There was a problem hiding this comment.
PyPI core metadata allows Author-email and Maintainer-email to contain multiple comma-separated RFC-822 addresses (spec). Returning each raw field as one set entry means the shared detectors pass a value like maintainer_email='expired@old.tld, owner@gmail.com' to extract_email_address_domain(), which only examines the text after the last @; in that valid metadata shape, the newly included maintainer address with the expired domain is ignored. Parse these fields into individual addresses before returning the set.
Useful? React with 👍 / 👎.
2 participants
While reading the PyPI metadata detectors I noticed
get_email_addressesonly ever returns one address:The
orshort-circuits, so whenauthor_emailis set the maintainer email is never included. All three detectors that call it (unclaimed_maintainer_email_domain,potentially_compromised_email_domain,deceptive_author) then skip the maintainer address entirely whenever an author address is present. So a package whosemaintainer_emaildomain is expired/unregistered (hijackable) is reported clean as long as it also has a normalauthor_email, which defeats the point of a detector literally named for the maintainer email.The npm side already returns the full set (
{...}comprehension over all emails), so this looks like an oversight rather than intent.This returns both addresses and drops empty strings alongside
None:Added
tests/analyzer/metadata/test_pypi_utils.py. I verified against the current code that the old form drops the maintainer email and the new form returns both (and still dropsNone/empty). I could not run the full suite locally since my machine is on Python 3.9 and the project targets 3.10+, so please let CI confirm.Thanks for taking a look.