Repository navigation
Check the URL scheme before downloading in generate_docset.py - #5803
Merged
Merged
Conversation
Codacy flagged two Bandit findings in the docset generator added in #5799: B310 (urlopen with an unchecked scheme) and B506 (yaml.load). download() now rejects anything but http(s) URLs before opening them, and the yaml.load call is marked, since its Loader derives from yaml.SafeLoader. The SHA-1 used to name downloaded files is marked as not used for security. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| req = urllib.request.Request(u.geturl(), headers={'User-Agent': USER_AGENT}) | ||
| with urllib.request.urlopen(req, timeout=20) as r: | ||
| # (the scheme is checked above) | ||
| with urllib.request.urlopen(req, timeout=20) as r: # nosec B310 |
| ext = os.path.splitext(path)[1] | ||
| if u.query or not ext or path.endswith('/'): | ||
| digest = hashlib.sha1(url.encode()).hexdigest()[:12] | ||
| digest = hashlib.sha1(url.encode(), usedforsecurity=False).hexdigest()[:12] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Codacy reports two Bandit findings in
docs/docset/generate_docset.py, added in #5799 (which was merged with this Codacy failure). Every pull request that merges the current develop inherits them; #5738 fails Codacy for this reason.Summary
urlopenwith an unchecked scheme):download()now rejects any URL whose scheme is nothttporhttpsbefore opening it. Only URLs thatis_remote()matched reach it, so nothing changes in practice; the check makes it explicit. The call is marked# nosec B310.yaml.load): theLoaderpassed toyaml.loadderives fromyaml.SafeLoader(it only ignores mkdocs.yml's custom tags), so the load is safe. The call is marked# nosec B506with that reason in a comment above it, following the# nosecstyle ofcheck_version_history.py.usedforsecurity=False. Codacy did not report it, but Bandit does locally.Tests
banditon the file: no findings (before: B310, B506, B324).python3 docs/docset/generate_docset.py list(run bymake style_check) still succeeds.ruff check(default rules): 18 findings before, 17 after; none new.Public API
No breaking changes. This only touches the documentation tooling.
Written by Claude Code.
🤖 Generated with Claude Code