Fix unreachable target-properties guard in LAPSio.set() (Fixes #8)

[p0dalirius]

Linked Issue

Closes #8

Root Cause

Two independent defects made the same guard dead code.

The filtering loop at L269–L272 wrote its result back into results, the very name it was iterating. Python evaluates the iterable once and iterates the original list object, so the rebind did not disturb the loop — but it did destroy the value the guard was written to test. After the loop results was the last searchResEntry response dict, and len(results) was that dict's key count (5: dn, raw_dn, attributes, raw_attributes, type), never 0. In the one case the guard was actually meant to catch — a properties search that returns no entries, only searchResRef referrals — the loop continued every iteration, results stayed bound to the non-empty response list, and len(results) == 0 was false again. The guard could not fire either way.

Separately, the not-found branch at L261–L262 printed [!] Target computer does not exist! and fell through. dn was still None, so L265 called paged_search() with None as its search base and raised TypeError: cannot convert 'NoneType' object to bytes — an unhandled crash before the guard was even reached, in exactly the scenario the guard exists for.

Fix Description

Collect the searchResEntry responses into a separate list, entries, with a comprehension, and test len(entries) == 0, so the guard reads the value it is meant to read and fires when the properties search yields no entries. Add the missing return to the not-found branch, and narrow its condition from dn is None and sid is None to dn is None, since dn is what the code downstream requires and is what modify() is called with.

sid is left in place. It was already unused before this change apart from that compound condition, and the format_sid(...) call it wraps can raise on an entry without objectSid; removing it would be a behaviour change beyond this fix.

Rejected alternative: keeping the loop and adding a separate flag or break. The comprehension expresses "the entries among the responses" directly and removes the rebinding that caused the defect, rather than working around it.

How Verified

Runtime, ldap3 2.9.1 / Python 3.12, driving LAPSio.set() against an ldap3 MOCK_SYNC directory across the three reachable paths. The third stubs paged_search() to return a single searchResRef and no entries.

Before (on main):

=== set on missing computer ===
[!] Target computer does not exist! (wrong domain?)
EXCEPTION: TypeError: cannot convert 'NoneType' object to bytes
=== set on existing computer ===
[+] Target computer found: CN=DC01,OU=Domain Controllers,DC=domain,DC=local
[+] Successfully updated the ms-Mcs-AdmPwd attribute of the target DC01$ to Passw0rd123
=== properties search returns only a referral ===
[+] Target computer found: CN=DC01,OU=Domain Controllers,DC=domain,DC=local
[+] Successfully updated the ms-Mcs-AdmPwd attribute of the target DC01$ to Passw0rd123

After:

=== set on missing computer ===
[!] Computer not found in LDAP: NOPE$
[!] Target computer does not exist! (wrong domain?)
=== set on existing computer ===
[+] Target computer found: CN=DC01,OU=Domain Controllers,DC=domain,DC=local
[+] Successfully updated the ms-Mcs-AdmPwd attribute of the target DC01$ to Passw0rd123
=== properties search returns only a referral ===
[+] Target computer found: CN=DC01,OU=Domain Controllers,DC=domain,DC=local
[!] Could not query target computer properties.

The missing-computer path now exits cleanly instead of raising, the referral-only path reaches the guard and returns instead of issuing the modify(), and the working path is unchanged — the password is still written.

Static. len(entries) at L272 reads the comprehension built at L271, which nothing reassigns, so the guard's operand is the entry list on every path. results is not read after L271.

CI: python3 -m py_compile pyLAPS.py and python3 pyLAPS.py --help both pass.

Test Coverage

None. The repository has no test suite and CI runs python3 pyLAPS.py --help only. Covering this would mean introducing a test harness plus an ldap3 mock-directory fixture and a referral stub, which is out of proportion to the fix; the driver used above is available if a suite is added.

Scope of Change

• Files changed: pyLAPS.py (LAPSio.set(), 3 insertions / 5 deletions)
• Submodule pointer updated: no
• Behavioral changes outside the bug fix: none. The successful --action set path is byte-identical in output; the two changed paths are the ones that previously crashed or wrongly continued.

Risk and Rollout

Confined to LAPSio.set(). The change can only stop execution earlier than before, on paths that previously crashed with TypeError or proceeded on data they had failed to retrieve, so no working --action set invocation is affected. Safe to merge directly.

Notes

Independent of #5 / PR #6 and of #7 / PR #9, though all three touch LAPSio. Verified that this branch merges cleanly into main alongside both of the others in any order.

The paged_search() call at L265 still resolves the module-level ldap_session global rather than self.ldap_session; that is #7, fixed in PR #9, and is deliberately left alone here.