fix(B614): suppress false positive on non-literal weights_only; add torch.jit.load regression coverage#1441
Open
DevamShah wants to merge 1 commit into
Open
fix(B614): suppress false positive on non-literal weights_only; add torch.jit.load regression coverage#1441DevamShah wants to merge 1 commit into
DevamShah wants to merge 1 commit into
Conversation
…orch.jit.load regression coverage B614 only treated literal weights_only=True/"True" as safe, so torch.load(f, weights_only=flag) with a computed value was a false positive. Suppress the finding when weights_only is a non-ast.Constant expression, giving the benefit of the doubt for statically-unresolvable values — mirroring the existing B615 (huggingface_unsafe_download) behaviour. Adds example coverage that torch.jit.load (TorchScript, not pickle; outside B614's matched set) is not flagged, locking in the behaviour reported in PyCQA#1293. The genuine-unsafe path and the weights_only==True safe path are unchanged. Refs PyCQA#1293 Signed-off-by: Devam Shah <devamshah91@gmail.com>
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
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.
Summary
Improves the B614 ("unsafe PyTorch load") check on two fronts, both reducing false positives:
weights_onlyis a non-literal expression (a variable, attribute, subscript, call, etc.). When the value cannot be resolved statically, the check now gives the caller the benefit of the doubt rather than emitting a finding — mirroring the existing behaviour of B615 (huggingface_unsafe_download), which uses the sameweights_only/ non-literal handling.torch.jit.loadis not flagged (refs B614 False Positive when using torch.jit.load #1293).torch.jit.loaduses TorchScript serialization, notpickle, and is correctly outside B614's matched set (torch.load/torch.serialization.load). The example file now documents this so the behaviour reported in B614 False Positive when using torch.jit.load #1293 is locked in by a test fixture.Problem / motivation
weights_only(the substantive fix): todaytorch.load(f, weights_only=flag)is flagged because the check only treats the literalTrue/"True"as safe. Real code frequently passesweights_onlyas a computed value (config flag, function arg), and flagging those is a false positive. B615 already solved the identical problem the same way; this brings B614 into line.torch.jit.load(B614 False Positive when using torch.jit.load #1293): users reported B614 firing ontorch.jit.load. On currentmainits qualified name (torch.jit.load) is not in B614's matched set, so it is already not flagged — this PR adds explicit example coverage so that stays true.Change
bandit/plugins/pytorch_load.py: when the matched call (torch.load/torch.serialization.load) passesweights_onlyas a non-ast.Constantvalue, return without a finding. The existingweights_only == True/"True"safe-path and the genuine-unsafe path are unchanged. Uses the samecontext._context.get("call")keyword-inspection idiom as the merged B615 plugin.examples/pytorch_load.py: adds non-triggering cases —torch.jit.load(with and withoutmap_location) and a non-literalweights_only— alongside the existing genuinely-unsafe calls.Trade-off (intentional, precedented)
Suppressing on a non-literal
weights_onlymeanstorch.load(f, weights_only=flag)whereflagisFalseat runtime will not be flagged (a false negative). This matches B615's deliberate "benefit of the doubt" stance for unresolvable arguments — favouring precision over recall for low-confidence static cases. The version-aware behaviour also requested in #1293 (treatingweights_onlyas defaulting toTrueon torch ≥ 2.6) is intentionally out of scope here and left for a follow-up, since it requires version detection.Testing / validation
tests/functional/test_functional.py::FunctionalTests::test_pytorch_loadpasses with the unchanged expectation (MEDIUM: 3,HIGH: 3) — the added cases correctly yield zero new findings.examples/pytorch_load.py: B614 fires only on the three genuinely-unsafetorch.loadcalls; it does not fire ontorch.jit.loador the non-literalweights_onlycase.