Skip to content

Reject bool where a plain int is expected in _is_valid#2088

Open
NishchayMahor wants to merge 1 commit into
weaviate:mainfrom
NishchayMahor:fix/validator-reject-bool-as-int
Open

Reject bool where a plain int is expected in _is_valid#2088
NishchayMahor wants to merge 1 commit into
weaviate:mainfrom
NishchayMahor:fix/validator-reject-bool-as-int

Conversation

@NishchayMahor

Copy link
Copy Markdown

What

_is_valid in weaviate/validator.py ends with return isinstance(value, expected). Because bool is a subclass of int, isinstance(True, int) is True, so a bool passes validation wherever a bare int is expected:

_is_valid(int, True)   # -> True  (should be False)

Real call sites hit this — e.g. query.py validates limit / offset / autocut as [int, None]. So:

collection.query.fetch_objects(limit=True)

silently passes validation and sends True (coerced to 1) instead of raising WeaviateInvalidInputError. This is the same class of bug that #2076 reported and #2077 fixed in config.py / util.py (... and not isinstance(x, bool)); this is the unguarded sibling site in the validator.

Fix

Guard the int case at the terminal return, so a bool is rejected where a plain int is expected:

if expected is int and isinstance(value, bool):
    return False
return isinstance(value, expected)

bool-typed expectations (_is_valid(bool, True)) and all other types are unaffected.

Testing

Added a case to the existing parametrized test_validator in test/collection/test_validator.py:

(True, [int], True),   # bool must not validate as int

It fails on main and passes with the fix (verified). The suite is fully offline (no DB/Docker), and the existing (1.0, [int], True) case establishes the same "reject the wrong numeric type" pattern.

bool is a subclass of int, so isinstance(True, int) is True and _is_valid
accepted a bool wherever a bare int was expected (e.g. query limit/offset/
autocut), silently coercing True/False to 1/0 instead of raising
WeaviateInvalidInputError. Guard the int case, mirroring the bool/int fix in
config.py from weaviate#2077. Adds a validator unit-test case.

@orca-security-eu orca-security-eu Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed SAST high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca

@weaviate-git-bot

Copy link
Copy Markdown

To avoid any confusion in the future about your contribution to Weaviate, we work with a Contributor License Agreement. If you agree, you can simply add a comment to this PR that you agree with the CLA so that we can merge.

beep boop - the Weaviate bot 👋🤖

PS:
Are you already a member of the Weaviate Forum?

@NishchayMahor

Copy link
Copy Markdown
Author

I agree with the CLA.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants