Open mcmitch opened 1 month ago
From my reading of the pyyaml
docs, your rationale makes sense to me. I'm not a security expert, however, and it looks like we match bandit's original behaviour here (and it looks like they've had this behaviour for a long time).
Digging into the source code for pyyaml
a bit:
BaseLoader
is defined here, and SafeLoader
here. They're the same, except that BaseLoader
has BaseConstructor
and BaseResolver
in its bases list, whereas SafeLoader
has SafeConstructor
and Resolver
in its bases list.BaseConstructor
is here, BaseResolver
is here, SafeConstructor
is here, Resolver
is here. SafeConstructor
is a subclass of BaseConstructor
; Resolver
is a subclass of BaseResolver
. SafeConstructor
overrides many methods from BaseConstructor
, but Resolver
seems to be something of a pointless subclass that doesn't override anything from BaseResolver
.SafeConstructor
seem mainly to extend the capabilities provided in the BaseConstructor
superclass, rather than overriding anything to make it safer -- i.e., exactly as you and the pyyaml
docs state.@ericwb, sorry for the ping -- I don't suppose you'd be able to shed light on this behaviour from bandit, would you? Is there a reason why SafeLoader
would be safer than BaseLoader
?
There's a SO post here with some useful info, though not much clarity in Bandit repo issues on the topic.
From https://pyyaml.org/wiki/PyYAMLDocumentation
For our project we are using the Baseloader, and do not want to use safeLoader, as this would not leave integer values as strings. The baseloader is not the unsafe FullLoader, and should not be flagged as an exception to S506.
Code to reproduce:
Ruff setting: [select = "S506"] Ruff version: 0.6.8