[Ruff] Enable security ruff rules ("S") #97
Conversation
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
JulienArzul
commented
Jan 29, 2026
| """Download to a temporary file and return its path.""" | ||
| import urllib.request | ||
|
|
||
| artifact_url = f"https://github.com/ollama/ollama/releases/download/{artifact_version}/{artifact_name}" |
Collaborator
Author
There was a problem hiding this comment.
Since this function was always used to download an artifact and is not meant as a generic "download" function, I moved the construction of the URL directly in the function so that the linter can see that we are only opening a https URL.
JulienArzul
commented
Jan 29, 2026
| stdout = subprocess.DEVNULL | ||
|
|
||
| proc = subprocess.Popen( | ||
| proc = subprocess.Popen( # noqa: S603 We're always running Ollama |
Collaborator
Author
There was a problem hiding this comment.
Using a noqa is the only way to suppress this rule: the linter doesn't manage to figure out whether we control and trust the command run in the subprocess
JulienArzul
commented
Jan 29, 2026
| query_bytes = file.read_bytes() | ||
| query = query_bytes.decode("utf-8") | ||
| checksum = hashlib.md5(query_bytes).hexdigest() | ||
| checksum = hashlib.md5(query_bytes, usedforsecurity=False).hexdigest() |
Collaborator
Author
There was a problem hiding this comment.
From what I've read, it's fine to use MD5 for checksums (which is not a "security" usage).
We could replace it with a sha256 though, we'd just need to check the size of the DB column, to make sure it still fits
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What?
This PR enables the S ruff rules, that add linting rules checking the security of our code.
This is reproducing https://pypi.org/project/flake8-bandit
How?
I had to exclude "S608" that checks that SQL query strings should be static because we are creating a lot of SQL queries using formatted strings.
This might be something we want to look into at some point, although I think most of them should be harmless as we are using internal variables to create the string rather than any user input.