Skip to content

fix: guard against a None action in ZPA isolation rule updates - #576

Open
hackerboey wants to merge 1 commit into
zscaler:masterfrom
hackerboey:fix/guard-none-action-in-isolation-rule-updates
Open

fix: guard against a None action in ZPA isolation rule updates#576
hackerboey wants to merge 1 commit into
zscaler:masterfrom
hackerboey:fix/guard-none-action-in-isolation-rule-updates

Conversation

@hackerboey

Copy link
Copy Markdown

Fixes #574

PolicySetControllerAPI.update_isolation_rule and update_isolation_rule_v2 both declare
action: str = None, but build the payload with an unguarded action.upper(). A partial
update that does not pass action therefore raises

AttributeError: 'NoneType' object has no attribute 'upper'

before any request is built, so those methods cannot be used for a partial update at all.

Why this shape of fix

The same file already guards the identical expression elsewhere, e.g.

"action": action.upper() if action else None
"action": action.upper() if action else kwargs.get("action", "").upper()

so this looks like an internal inconsistency rather than a deliberate difference. The change
makes the two unguarded sites match the guarded form already used nearby — no new pattern is
introduced, and behaviour for a caller who does pass action is unchanged.

Test

Adds tests/unit/test_zpa_policies.py, following the layout and style of the existing flat
tests/unit/test_*.py files. It mocks the request executor, so no credentials or network access
are needed — the failure occurs while building the payload.

Verified against this branch: 2 passed. Verified against master's policies.py: 2 failed
with the AttributeError above, so it is a genuine regression guard rather than a tautology.
Full tests/unit: 601 passed. ruff check, ruff check --select I and
black --check --line-length 127 are all clean.

Diff is +52/-2 across two files, and the change to policies.py is two lines. Happy to rescope
if you would prefer a different guard form, or to extend it to other optional fields.

update_isolation_rule and update_isolation_rule_v2 both declare action
as optional (action: str = None) but build their payload with an
unguarded action.upper(). A partial update that omits action raises
AttributeError: 'NoneType' object has no attribute 'upper' before any
request is made, so neither method can be used for a partial update.

Both now use the guarded form the sibling update methods in this module
already use, e.g. update_access_rule:

    "action": action.upper() if action else None

None is what the surrounding keys (name, description,
zpnIsolationProfileId) already resolve to when omitted, so the payload
stays internally consistent and nothing changes when action is supplied.

The remaining unguarded action.upper() sites in this module take action
as a required positional parameter, so omitting it raises TypeError
rather than AttributeError; those are left alone.
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.

[ISSUE] ZPA update_isolation_rule / update_isolation_rule_v2 raise AttributeError on a partial update (unguarded action.upper())

1 participant