Skip to content

Add optional CVE metadata for Security entries - #95

Open
StanFromIreland wants to merge 1 commit into
python:mainfrom
StanFromIreland:cve-identifier
Open

StanFromIreland wants to merge 1 commit into
python:mainfrom
StanFromIreland:cve-identifier

Conversation

@StanFromIreland

Copy link
Copy Markdown
Member

This recently came up on Discourse, CVEs are useful information for users, however they aren't always listed in the entries. The patch adds a special field for them, and support to render them beside the issue number in the changelog.

Comment thread src/blurb/_merge.py
body = f'bpo-{issue_number}: {body}'
prefix = f'bpo-{issue_number}'
if cve := metadata.get('cve'):
prefix = f'{prefix}, {cve}' if prefix else cve

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do something like this to use the Sphinx :cve: role:

https://www.sphinx-doc.org/en/master/usage/restructuredtext/roles.html#role-cve

Suggested change
prefix = f'{prefix}, {cve}' if prefix else cve
cve_role = f':cve:`{cve.removeprefix("CVE-")}`'
prefix = f'{prefix}, {cve_role}' if prefix else cve_role

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we put the CVE before the gh-number?

That's what we did at https://blog.python.org/2026/10/python-31022-31117/ and I think the CVE is more interesting for readers?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't add the role, it would break 3.11. I have a PR in CPython to add it there, like we do for GitHub issues.

Comment thread src/blurb/_merge.py
if int(issue_number):
body = f'bpo-{issue_number}: {body}'
prefix = f'bpo-{issue_number}'
if cve := metadata.get('cve'):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bit of whitespace

Suggested change
if cve := metadata.get('cve'):
if cve := metadata.get('cve'):

Comment thread tests/test_merge.py

# Assert
text = Path('NEWS').read_text(encoding='utf-8')
assert '- gh-135661, CVE-2025-6069: Fix a security issue.\n' in text

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert '- gh-135661, CVE-2025-6069: Fix a security issue.\n' in text
assert '- gh-135661, :cve:`2025-6069`: Fix a security issue.\n' in text

Comment thread src/blurb/_blurb_file.py
Comment on lines +202 to +203
if 'cve' in metadata and metadata.get('section') != cve_section:
throw(f"'cve:' may only be used in the {cve_section!r} section!")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have sometimes included CVEs in non-security sections:

Should we not allow those? In those cases, should they have been under Security?

Comment thread src/blurb/_add.py

def _check_commented_cve(tmp_path: str) -> str:
"""Return an error message if a CVE was filled in but left commented out."""
with open(tmp_path, encoding='utf-8') as file:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This opens the file a second time. Not a big deal as it's a small file, but perhaps we could read just once and pass the contents?

Actually, I think it would be cleaner to move this parsing into the main parser at Blurbs.parse() and drop this function and second open.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants