Disclaimer
The analysis below was done by a coding agent. I tried to verify it up to my knowledge, but there could be inaccuracies. Nevertheless, the false positive is real and it has been affecting jsonargparse for a long time. Just decided that since I have access to a good coding agent I could analyze the issue and report it here.
Description of the false positive
py/call-to-non-callable and py/hash-unhashable-value fire on correct code purely because of the filename of the module the code lives in.
The cause is the second disjunct of isType in python/ql/lib/semmle/python/objects/TObject.qll#L215-L220:
predicate isType(ObjectInternal t) {
t.isClass() = true
or
t.getOrigin().getEnclosingModule().getName().matches("%typing")
}
isType gates the construction of TSubscriptedType (TObject.qll#L200-L205), the model of a subscripted generic such as List[int]. Because the disjunct matches on module name rather than on anything about the object, every object defined in a module whose dotted name ends in typing is classified as a type. Subscripting an ordinary module-level dict in such a module therefore produces a SubscriptedTypeInternal, whose getClass() returns the generic's class (Classes.qll#L332):
override ObjectInternal getClass() { result = this.getGeneric().getClass() }
So my_registry[key] is inferred to be an instance of dict. Any function that returns such a lookup is then inferred to return a dict, and every downstream use of its result is flagged.
Note the alert message is quite misleading in this situation. On a class returned from a registry lookup, the user sees "Call to a non-callable of builtin-class dict" — the dict is the registry, not anything related to the value, and nothing in the message text says so. The origin in relatedLocations is the only clue, and it points at the registry lookup, often in a different file from the alert.
Code samples or links to source code
Self-contained reproduction. Four files; pkg/other.py is byte-identical to pkg/typing.py apart from the identifier names, and is there purely to isolate the filename as the trigger.
pkg/__init__.py (empty)
pkg/typing.py:
registry = {}
def make_type(name):
if name in registry:
return registry[name]
class Created(str):
pass
registry[name] = Created
return Created
InTyping = make_type("InTyping")
pkg/other.py: identical, with InTyping renamed to InOther.
main.py:
from typing import Optional
from pkg.other import InOther
from pkg.typing import InTyping
InTyping("a") # py/call-to-non-callable <- false positive
Optional[InTyping] # py/hash-unhashable-value <- false positive
InOther("b") # no alert
Optional[InOther] # no alert
Running NonCallableCalled.ql and HashedButNoHash.ql from codeql/python-queries 1.6.8 (CLI 2.23.5) gives:
main.py:6 Non-callable called Call to a non-callable of builtin-class dict.
related location: pkg/typing.py:6:16:6:29
main.py:7 Unhashable object hashed This instance of dict is unhashable.
related location: pkg/typing.py:6:16:6:29
Both alerts are on uses of InTyping; the identical InOther uses produce nothing. The relatedLocation on both is pkg/typing.py:6, the registry[name] lookup.
This is not hypothetical. It is the cause of a long-standing batch of dismissed alerts in jsonargparse, which has a public jsonargparse.typing module:
Every py/call-to-non-callable and py/hash-unhashable-value alert in that repository traces, via relatedLocations, to that single line — including alerts in test files that merely call a path type such as Path_fr(...) or write Optional[Path_fr].
One further observation that may be useful when triaging severity: whether the corruption surfaces as an alert depends on unrelated downstream details. The same project has a second lookup of the same registry at typing.py#L248-L252 that produces the same bogus Dict[...] value, but it never escapes the function: the next statement reads registered_type.__name__, and since SubscriptedTypeInternal declares both attribute() { none() } and attributesUnknown() { none() }, the attribute lookup definitively fails and the following return becomes unreachable. Probing that return yields no points-to tuples at all. So the bad inference is present at both sites and happens to be masked at one of them.
Suggested fix
Restricting the disjunct to the actual typing modules removes the false positives:
predicate isType(ObjectInternal t) {
t.isClass() = true
or
exists(string name | name = t.getOrigin().getEnclosingModule().getName() |
name = "typing" or name = "typing_extensions" or name.matches("typing.%")
)
}
With that change, on the reproduction above both alerts go to zero. On a database built from jsonargparse at the commit linked above, py/call-to-non-callable goes from 9 results to 0 and py/hash-unhashable-value from 11 to 0. Those before-counts are exactly what code scanning itself reported for that commit on main (9 and 11), so the narrowing clears the full set of false positives in that project.
Open question
I could not construct a case where the current disjunct actually helps, so I may be missing its intended purpose. In every database I built the stdlib typing module is not extracted at all — querying for modules matching %typing returned only the project's own jsonargparse.typing and jsonargparse_tests.test_typing. If the disjunct exists to model typing.List and friends via extracted stdlib source, a maintainer will know whether the narrowing above preserves that; if it only ever matches user modules that happen to be named *typing, it may be removable outright.
URL to the alert on GitHub code scanning (optional)
The affected alerts are dismissed as false positives in a public repository, e.g. https://github.com/mauvilsa/jsonargparse/security/code-scanning/617 — raised on path = Path_fr(file_r), where Path_fr is a class. (Code scanning alerts are only visible to users with write access even on public repositories, so that link may not be usable from outside the project; the reproduction above stands on its own.)
Environment
codeql-bundle-v2.23.5 — CodeQL CLI 2.23.5, codeql/python-all 4.1.0, codeql/python-queries 1.6.8
- Also present on
main as of 7a367c9
Disclaimer
The analysis below was done by a coding agent. I tried to verify it up to my knowledge, but there could be inaccuracies. Nevertheless, the false positive is real and it has been affecting jsonargparse for a long time. Just decided that since I have access to a good coding agent I could analyze the issue and report it here.
Description of the false positive
py/call-to-non-callableandpy/hash-unhashable-valuefire on correct code purely because of the filename of the module the code lives in.The cause is the second disjunct of
isTypeinpython/ql/lib/semmle/python/objects/TObject.qll#L215-L220:isTypegates the construction ofTSubscriptedType(TObject.qll#L200-L205), the model of a subscripted generic such asList[int]. Because the disjunct matches on module name rather than on anything about the object, every object defined in a module whose dotted name ends intypingis classified as a type. Subscripting an ordinary module-leveldictin such a module therefore produces aSubscriptedTypeInternal, whosegetClass()returns the generic's class (Classes.qll#L332):So
my_registry[key]is inferred to be an instance ofdict. Any function that returns such a lookup is then inferred to return adict, and every downstream use of its result is flagged.Note the alert message is quite misleading in this situation. On a class returned from a registry lookup, the user sees "Call to a non-callable of builtin-class dict" — the
dictis the registry, not anything related to the value, and nothing in the message text says so. The origin inrelatedLocationsis the only clue, and it points at the registry lookup, often in a different file from the alert.Code samples or links to source code
Self-contained reproduction. Four files;
pkg/other.pyis byte-identical topkg/typing.pyapart from the identifier names, and is there purely to isolate the filename as the trigger.pkg/__init__.py(empty)pkg/typing.py:pkg/other.py: identical, withInTypingrenamed toInOther.main.py:Running
NonCallableCalled.qlandHashedButNoHash.qlfromcodeql/python-queries1.6.8 (CLI 2.23.5) gives:Both alerts are on uses of
InTyping; the identicalInOtheruses produce nothing. TherelatedLocationon both ispkg/typing.py:6, theregistry[name]lookup.This is not hypothetical. It is the cause of a long-standing batch of dismissed alerts in jsonargparse, which has a public
jsonargparse.typingmodule:jsonargparse/typing.py#L70jsonargparse/typing.py#L414-L415Every
py/call-to-non-callableandpy/hash-unhashable-valuealert in that repository traces, viarelatedLocations, to that single line — including alerts in test files that merely call a path type such asPath_fr(...)or writeOptional[Path_fr].One further observation that may be useful when triaging severity: whether the corruption surfaces as an alert depends on unrelated downstream details. The same project has a second lookup of the same registry at
typing.py#L248-L252that produces the same bogusDict[...]value, but it never escapes the function: the next statement readsregistered_type.__name__, and sinceSubscriptedTypeInternaldeclares bothattribute() { none() }andattributesUnknown() { none() }, the attribute lookup definitively fails and the followingreturnbecomes unreachable. Probing thatreturnyields no points-to tuples at all. So the bad inference is present at both sites and happens to be masked at one of them.Suggested fix
Restricting the disjunct to the actual typing modules removes the false positives:
With that change, on the reproduction above both alerts go to zero. On a database built from jsonargparse at the commit linked above,
py/call-to-non-callablegoes from 9 results to 0 andpy/hash-unhashable-valuefrom 11 to 0. Those before-counts are exactly what code scanning itself reported for that commit onmain(9 and 11), so the narrowing clears the full set of false positives in that project.Open question
I could not construct a case where the current disjunct actually helps, so I may be missing its intended purpose. In every database I built the stdlib
typingmodule is not extracted at all — querying for modules matching%typingreturned only the project's ownjsonargparse.typingandjsonargparse_tests.test_typing. If the disjunct exists to modeltyping.Listand friends via extracted stdlib source, a maintainer will know whether the narrowing above preserves that; if it only ever matches user modules that happen to be named*typing, it may be removable outright.URL to the alert on GitHub code scanning (optional)
The affected alerts are dismissed as false positives in a public repository, e.g. https://github.com/mauvilsa/jsonargparse/security/code-scanning/617 — raised on
path = Path_fr(file_r), wherePath_fris a class. (Code scanning alerts are only visible to users with write access even on public repositories, so that link may not be usable from outside the project; the reproduction above stands on its own.)Environment
codeql-bundle-v2.23.5— CodeQL CLI 2.23.5,codeql/python-all4.1.0,codeql/python-queries1.6.8mainas of 7a367c9