fix(registry): lowercase target_hosts_by_hostname lookups

ARCHITECTURE.md

@@ -383,17 +383,20 @@ so it can't stop early.
 
 Two more pieces wire discovery into this loop:
 
-- [`SourceContext.register_target`](src/openhound_sccm/context.py#L260-L358) is the single funnel every
+- [`SourceContext.register_target`](src/openhound_sccm/context.py#L356-L455) is the single funnel every
   discovery path calls. It resolves the identifier against AD (best-effort), applies the allow-list (see
   [§4](#4-targeted-collection-an-include-only-allow-list)), dedups by SID then hostname (with an FQDN
   upgrade path), merges sources/site-codes onto an existing `TargetEntry`, and — for a genuinely new host —
-  calls `work_queue.submit(...)` ([context.py:355-356](src/openhound_sccm/context.py#L355-L356)). Both
+  calls `work_queue.submit(...)` ([context.py:453-454](src/openhound_sccm/context.py#L453-L454)). Both
   Stage-1 discovery resources (which call it from inside `collectors/*`) and the CLI's `--computers`
   seeds ([main.py:963-965](src/openhound_sccm/main.py#L963-L965)) go through this *same* funnel, so dedup
   and filtering are identical.
-- [`target_hosts_snapshot`](src/openhound_sccm/context.py#L361-L369) lets phases read the *current* target
+- [`target_hosts_snapshot`](src/openhound_sccm/context.py#L459-L471) lets phases read the *current* target
   set at iteration time, so a host discovered after a phase started is still picked up — the OpenHound
   equivalent of CMBP's "iterate the updated list."
+  When a short hostname is upgraded to an FQDN, the old name remains an index alias for a queued worker,
+  while the snapshot returns the target only once. If a RemoteRegistry worker has no registered target,
+  it warns and emits the available rows without a host AD SID.
 
 One discovery path recurses on its own, one level below `register_target`:
 [`_expand_group_targets`](src/openhound_sccm/collectors/ldap.py#L618-L661) (ope-e191). When
@@ -2084,6 +2087,7 @@ took a separate, manual step outside `openhound collect sccm`.
 
 | Date | Change |
 |---|---|
+| 2026-09-30 | **Preserved target identity across hostname upgrades and incomplete RemoteRegistry discovery** (§3). A queued short hostname remains an alias when its target is upgraded to an FQDN; snapshots deduplicate the aliases. Registry lookups use normalized hostnames and emit partial rows with a warning if a target is genuinely absent, instead of failing the phase. Regression tests cover mixed-case names, the queued-name upgrade, and missing targets. |
 | 2026-08-02 | **A healthy low-privilege run stopped emitting 146 WARNINGs** (§7; con-81c2). Every one of them was the collector working as designed — the same failure mode the 2026-08-01 entry fixed one severity level up, because an operator who learns to ignore the file cannot find the 23 real entries in it. Measured against `out/ab-lowpriv-pe-off/collect_issues_20260801_230714.log` (nine-host mayyhem.com lab, plain domain user, `--run-all`). **(1) Transport tables that were never collected.** `_sccm_expected_miss` gained a third case: a missing `wmi_`/`adminservice_` table whose sibling is *also* absent is DEBUG when no privileged table holds any **rows**, because then the AdminService/WMI phases collected nothing. That was **106 of the 146**. Rows and not mere existence, which the first cut of this got wrong and the lab caught: dlt writes a resource's schema even when it yields zero rows, so a lowpriv run into a *reused* output directory carried `adminservice_client_devices` and `adminservice_site_definitions` at 0 rows, an existence check read that as "a privileged transport ran", and 98 expected misses stayed at WARNING. `--clean` runs hid the bug because a fresh catalog has no privileged tables at all. `_privileged_transport_ran` is pre-existing code shared with the `http_`/`smb_` fallback branch, which had the same blindness, so the fix can legitimately surface `http_`/`smb_` warnings that were previously swallowed — two tests that seeded an *empty* privileged table to mean "a transport ran" now insert a row. Across all 18 lab collections the split is binary (privileged: 13 tables, 13 populated; unprivileged: none populated), so partial privilege — where some classes return rows and others are ACL-denied — has never occurred and is deliberately still a WARNING. It stays a WARNING when some privileged tables did land — the case the old `test_safe_no_sibling_logs_warning` was really protecting. That test asserted the opposite for the no-transport case and was inverted deliberately, and a new test pins the privileged-transport-ran half. **(2) Probe negatives.** A host answering the AdminService probe with 404 is not an SMS Provider, and a host refusing the HTTP probe does not serve that endpoint; both are discovery results, not faults. The 404 drops to VERBOSE because `privileged.py` already logs an INFO conclusion (`is not a reachable AdminService provider; skipping`) immediately after it, so INFO would state the same fact twice per host. The HTTP connect failure drops to INFO rather than VERBOSE because `http.py` has **no** conclusion line — below INFO a host that served nothing would read as a clean collection. 401/403/500 are untouched: a provider that exists and refuses you is a finding. **(3) The RemoteRegistry roll-up lists keys, superseding the capability names introduced 2026-08-01.** `_DENIED_CAPABILITIES` and `_capability_for` are deleted; the message is the distinct denied key paths one per line, counted by distinct key so the number always matches the list (several values can be refused under one key — `_read_mssql_service_state` reads `Start` and `ObjectName` under one service key). The "requires local Administrators, re-run as admin" prose moved to README's low-privilege section, which said it already. Both RemoteRegistry warnings and the three per-read lines also dropped the trailing `on <host>`: `LogContextFilter` prefixes `[target][phase]` on every record a per-host phase emits, so it was always duplication. **(4) impacket's Python 3.14 SyntaxWarning**, from a `return` inside a `finally` at `mssql/version.py:182`, is silenced in `openhound-collector-common` 0.1.4 by a scoped `catch_warnings()` around the `from impacket import ntlm, tds` that triggers the compile — scoped, so a return-in-finally in our own code still reports. `module=` filters do **not** work on compile-time warnings: CPython calls `warn_explicit()` with `module=None` and `warnings.py` derives the module from the file path, not the dotted name. The same release shortened the skipped-transform log from a five-line DuckDB exception dump (spell-check guess + SQL echo + caret) to one line, with the full text kept on a companion DEBUG only when the miss is unexpected. Floor raised to `>=0.1.4`. |
 | 2026-08-01 | **The `--run-all` summary now names the archive that was written, not one rebuilt from the name we asked for** (§12; con-a4ec, closing the shared-library gap con-8a28 opened). Adopts `openhound-collector-common` **0.1.3** (floor raised `>=0.1.2` → `>=0.1.3`, cap unchanged), whose `run_end_to_end` returns the archive `zip_graph_output` produced on the new `StagePaths.graph_zip` instead of discarding it — `None` when convert emitted no `*.json`. `_log_all_output_locations` loses its `graph_zip_name` parameter and reads the field; the name now travels one way only, from `collect_sccm` into the chain. The reconstruction it replaces (`graph_out / <requested name>` plus an existence test) could disagree with reality — a stale archive from an earlier run into the same output directory is the obvious case, and it is exactly the shape `--clean` exists to manage — and only the returned path is evidence. Two tests pin the distinction by putting a *differently named* archive in the graph dir alongside the reported one. Stale floor references in ARCHITECTURE (`>=0.1.0`, and a §12 note saying the 0.1.1 bump was still deferred), README and PUBLISHING corrected to `>=0.1.3` at the same time. |
 | 2026-08-01 | **The registry arm could not see a named SQL Server instance, and the warning that said so was camouflaged by six false ones** (con-ab59). `get_mssql_settings` probed eight hard-coded `SuperSocketNetLib` paths, every one ending `.MSSQLSERVER` — so a *named* instance could never match. The lab's `ps1-sec` runs the SEC site database as `CONFIGMGRSEC` (SCCM's own site definition: `SQLDatabaseName 'CONFIGMGRSEC\CM_SEC'`, port 1433) and was missed **on a fully privileged run**: eight `not found`s, then "Could not access any MSSQL registry paths". That line was a true positive sitting among six identical ones from hosts with genuinely no SQL, which is why it read as noise — and is why the fix was not to quieten it. Paths are now **derived from SQL Server's own inventory** (`Instance Names\SQL`), whose value *data* is the instance's subkey (`MSSQL16.CONFIGMGRSEC`); the settings path is therefore exact, with no version prefix to guess and no default-instance assumption. Eight hard-coded strings collapse to one derivation plus a single pre-2000 fallback (`SOFTWARE\Microsoft\MSSQLServer\...`, which predates the inventory key and had only a default instance), and the duplicate second read of `Instance Names\SQL` further down the function goes with them — one read now serves both the candidate paths and the reported `instance_names`. Severity was moderate rather than urgent because the separate MSSQL phase covers a *reachable* instance (the same run logs `port 1433 is open` / `EPA testing … via SSPI`); the gap bit a firewalled instance, and `service_start_type` / `service_account_name`, which the live probe does not supply — note `_mssql_service_name` already handled `MSSQL$<name>` correctly and was simply unreachable behind the early return. The warning now fires only on the contradiction worth hearing: the inventory lists instances *and* none of their settings keys can be read. No inventory at all means no SQL Server, the normal case (seven of nine lab hosts, privileged), and logs at verbose. **A test fixture was complicit:** `_mssql_probe` seeded the inventory as `[(instance, instance)]`, putting the instance *name* where the registry stores the *subkey*, so `test_mssql_named_instance_uses_the_dollar_service_name` passed against code that could not reach a named instance — a fake mirroring the code's belief rather than the system's behaviour. Fixture corrected, 8 tests added. Suite 1025 passed / 5 skipped. |

README.md

@@ -702,6 +702,11 @@ readable after the fact without re-running. `collect_issues_<timestamp>.log` hol
 errors, each with a traceback, and is not created at all by a clean run. `--debug` additionally folds
 the `dlt`/`ldap3` internals into the full log.
 
+If a RemoteRegistry host is missing from the target index, the issues log warns that its rows will
+lack host AD identity. The collector still writes the registry values it can read, but rows without
+a host SID cannot support identity-based edges such as `HasSession`. A short hostname that is later
+resolved to an FQDN stays indexed under both names, so normal discovery retains that identity.
+
 #### What a low-privilege run looks like
 
 Collecting as a plain domain user is a supported, first-class mode — see

src/openhound_sccm/collectors/registry.py

@@ -465,9 +465,10 @@ def collect_registry(target: str, ctx: "SourceContext") -> Iterable[tuple[str, d
                 # server name only when AD resolution failed (ad_object
                 # is None or lacks a name).
                 logger.info("Found %s, this target is a site server", SCCM_REG_KEYS["component_servers"])
-                target_entry = ctx.target_hosts_by_hostname[target]
+                target_entry = ctx.target_hosts_by_hostname.get(target.lower())
+                ad_object = target_entry.ad_object if target_entry else None
                 row = {
-                    **(target_entry.ad_object or {}),
+                    **(ad_object or {}),
                     "source": "RemoteRegistry-ComponentServers",
                     "sccm_infra": True,
                     "sccm_site_system_roles": _roles(["SMS Site Server"], site_code),
@@ -529,9 +530,10 @@ def collect_registry(target: str, ctx: "SourceContext") -> Iterable[tuple[str, d
                     "database lives elsewhere. Emitting the SQL Server role as assumed "
                     "pending corroboration in preprocess.", target,
                 )
-                target_entry = ctx.target_hosts_by_hostname[target]
+                target_entry = ctx.target_hosts_by_hostname.get(target.lower())
+                ad_object = target_entry.ad_object if target_entry else None
                 row = {
-                    **(target_entry.ad_object or {}),
+                    **(ad_object or {}),
                     "source": "RemoteRegistry-MultisiteComponentServers",
                     "sccm_infra": True,
                     "sccm_site_system_roles": _roles(["SMS SQL Server", "SMS Site Server"], site_code),
@@ -600,7 +602,7 @@ def get_current_user(probe: _RegistryProbe, ctx: SourceContext) -> Iterable[tupl
         current_user_ad_object = ctx.resolve_principal(current_user_sid)
         if current_user_ad_object:
             logger.info("Found current user: %s (%s)", current_user_ad_object.get("sam_account_name"), current_user_sid)
-            target_entry = ctx.target_hosts_by_hostname.get(probe.hostname)
+            target_entry = ctx.target_hosts_by_hostname.get(probe.hostname.lower())
             host_sid = target_entry.ad_object.get("object_sid") if (target_entry and target_entry.ad_object) else None
             if host_sid is None:
                 # No resolved host AD object — HasSession can't be built for this row downstream; keep the row but log.
@@ -663,16 +665,19 @@ def get_ntlm_settings(probe: _RegistryProbe, ctx: SourceContext) -> Iterable[tup
         disable_loopback_check = disable_loopback_reg == 1
         logger.verbose(f"DisableLoopbackCheck is {'enabled' if disable_loopback_check else 'disabled'}")
 
-    target_entry = ctx.target_hosts_by_hostname[probe.hostname]
+    target_entry = ctx.target_hosts_by_hostname.get(probe.hostname.lower())
+    if target_entry is None:
+        logger.warning("No registered target for %s; RemoteRegistry rows will lack host AD identity", probe.hostname)
+    ad_object = target_entry.ad_object if target_entry else None
     row = {
-        **(target_entry.ad_object or {}),
+        **(ad_object or {}),
         "source": "RemoteRegistry-NTLMSettings",
         "smb_signing_required": signing_required,
         "smb_signing_source": signing_source,
         "restrict_receiving_ntlm_traffic": restrict_receiving_ntlm_traffic,
         "disable_loopback_check": disable_loopback_check,
     }
-    row.setdefault("name", target_entry.ad_object.get("name") if target_entry.ad_object else probe.hostname)
+    row.setdefault("name", ad_object.get("name") if ad_object else probe.hostname)
     yield "remoteregistry_computers", row
 
 
@@ -869,16 +874,17 @@ def get_mssql_settings(probe: _RegistryProbe, ctx: SourceContext) -> Iterable[tu
         probe, instance_names
     )
 
-    target_entry = ctx.target_hosts_by_hostname[probe.hostname]
+    target_entry = ctx.target_hosts_by_hostname.get(probe.hostname.lower())
+    ad_object = target_entry.ad_object if target_entry else None
 
     yield "remoteregistry_mssql_servers", {
         "source": "RemoteRegistry-MSSQL",
         "force_encryption": force_encryption if force_encryption is not None else None,
         "extended_protection": extended_protection if extended_protection is not None else None,
-        "name": target_entry.ad_object.get("name") if target_entry.ad_object else probe.hostname,
-        "domain_computer_sid": target_entry.ad_object.get("object_sid") if target_entry.ad_object else None,
+        "name": ad_object.get("name") if ad_object else probe.hostname,
+        "domain_computer_sid": ad_object.get("object_sid") if ad_object else None,
         "port": port if port else None,
         "instance_names": instance_names if instance_names else None,
         "service_start_type": service_start_type,
         "service_account_name": service_account_name,
-    }
+    }

src/openhound_sccm/context.py

@@ -99,7 +99,7 @@ class SourceContext:
     # at iteration time so late additions are picked up.
     #
     # Two parallel indexes:
-    #   target_hosts_by_hostname — always populated; key = lowercased canonical hostname
+    #   target_hosts_by_hostname — lowercased canonical hostname and prior short-name aliases
     #   _target_hosts_by_sid      — only when SID available; key = objectSid string
     # Both dicts hold references to the same entry dicts, so a mutation via
     # either index is immediately visible via the other.
@@ -413,10 +413,10 @@ def register_target(
 
             if existing is not None:
                 logger.verbose(f"Already registered target: {existing.ad_object.get('dns_host_name') if existing.ad_object else existing.hostname}")
-                # FQDN upgrade: re-key target_hosts_by_hostname
+                # Keep the queued short name as an alias when upgrading to an FQDN.
+                # A worker may still receive that name after this entry is upgraded.
                 if "." in canonical_lower and "." not in existing.hostname.lower():
                     logger.verbose("Upgrading hostname %r -> %r", existing.hostname, canonical)
-                    del self.target_hosts_by_hostname[existing.hostname.lower()]
                     existing.hostname = canonical
                     self.target_hosts_by_hostname[canonical_lower] = existing
                 # Backfill ad_object + SID index if we now have one
@@ -457,11 +457,15 @@ def register_target(
 
 
     def target_hosts_snapshot(self) -> list:
-        """Return the current list of probe targets (TargetEntry objects) as a copy.
+        """Return each current probe target once, even when it has a short-name alias.
 
         Per-host resources iterate this so late-registered hosts are picked
         up by phases that haven't started yet. Thread-safe against concurrent
         ``register_target`` mutations.
         """
         with self._ensure_target_lock():
-            return list(self.target_hosts_by_hostname.values())
+            unique = {
+                entry.hostname.lower(): entry
+                for entry in self.target_hosts_by_hostname.values()
+            }
+            return list(unique.values())

tests/per_host_wiring_test.py

@@ -72,6 +72,24 @@ def test_register_existing_target_does_not_resubmit():
     assert wq.submitted == ["hostA"]  # submitted exactly once
 
 
+def test_fqdn_upgrade_keeps_queued_short_name_as_alias():
+    wq = FakeWorkQueue()
+    ctx = _ctx(work_queue=wq)
+    short = ctx.register_target(
+        "HOST", source="first", ad_object={"name": "HOST", "object_sid": "S-1-2"},
+    )
+    fqdn = ctx.register_target(
+        "HOST.example.com", source="second",
+        ad_object={"name": "HOST", "dns_host_name": "HOST.example.com", "object_sid": "S-1-2"},
+    )
+
+    assert fqdn is short
+    assert wq.submitted == ["HOST"]
+    assert ctx.target_hosts_by_hostname[wq.submitted[0].lower()] is short
+    assert ctx.target_hosts_by_hostname["host.example.com"] is short
+    assert ctx.target_hosts_snapshot() == [short]
+
+
 def test_allow_list_blocks_non_listed_discoveries():
     wq = FakeWorkQueue()
     ctx = _ctx(allowed={"hosta"}, work_queue=wq)