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

Ruff: Add and fix RUF015 #11708

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Ruff: Add and fix RUF015 #11708

wants to merge 1 commit into from

Conversation

kiblik
Copy link
Contributor

@kiblik kiblik commented Feb 1, 2025

Originally proposed in #10712 but I didn't like Ruff's autofix

This PR is trying to address unnecessary-iterable-allocation-for-first-element (RUF015)

next(iter(x)) is proposed by Ruff but it is a bit hard to read it in code so I wrote a wrapper.
Plus, I dropped som conversions to a list where it is not needed.

@kiblik kiblik force-pushed the ruff_RUF015 branch 2 times, most recently from 895b39b to f83b38d Compare February 4, 2025 18:07
@kiblik kiblik marked this pull request as ready for review February 4, 2025 21:33
Copy link

dryrunsecurity bot commented Feb 4, 2025

DryRun Security Summary

The pull request implements code refactoring by introducing a first_elem() utility function while revealing minor security concerns in the Cobalt API client's handling of credentials and error messages, though no critical vulnerabilities were introduced.

Expand for full summary

The pull request introduces minor refactoring across multiple files, consistently replacing list(x.keys())[0] or list(x.values())[0] with a new utility function first_elem() from dojo.utils. Security findings include potential information exposure in API client error handling and API token management in the Cobalt API client, with no critical vulnerabilities introduced by the code changes.

Specific security observations:

  1. In dojo/tools/api_cobalt/api_client.py:
    • API credentials passed directly in headers
    • Full API response content logged in exceptions
    • Single authentication method (API key)
    • Potential information exposure in error messages

No other significant security vulnerabilities were identified across the modified files.

Code Analysis

We ran 9 analyzers against 16 files and 0 analyzers had findings. 9 analyzers had no findings.

View PR in the DryRun Dashboard.

@Maffooch
Copy link
Contributor

Maffooch commented Feb 6, 2025

Hey @kiblik I am super conflicted about this one, but have not landed on a stance I feel great about.

On one hand, I like the approach of abstracting away next(iter(x)) as it does not appear all that intuitive at first glance. On the other hand, this takes away the opportunity to learn why the use of the iterator is the safer and performant approach. I don't have much experience in iterators in python, and learned something very valuable from just this PR 😄

I have no qualms with the ruff rule introduced here, but am on the fence about the use of the utility (we have a ton of those as it is 😅 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants