diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index f50bed1..e0776c2 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -669,6 +669,11 @@ filters/handlers and mutating live handler instances at runtime: mirror image of the normal path: same "mutate live handler instances" technique, raising instead of lowering. `--silent` also forces `--progress off`, since the progress tracker renders straight to the console and bypasses logging. +- **Bounded local client-log detail.** [`local_client_logs_targets`](src/openhound_sccm/collectors/local.py) + still scans existing CCM/ccmsetup logs on any Windows host, even without the `root\CCM` namespace: + old logs can identify active infrastructure. It logs the first full URL and UNC path for each host, + then reports the number of suppressed matches once. Target discovery still considers every host; + repeated content-download lines no longer fill the console or the always-DEBUG full log. - **A per-run issues file** (`collect_issues_.log`, [`_DiagnosticFileHandler`](src/openhound_sccm/main.py)) captures every WARNING+ **with full traceback** — even when the warning was logged without one, by injecting `sys.exc_info()` if a live exception is in flight, then restoring the record so the console isn't @@ -2087,6 +2092,7 @@ took a separate, manual step outside `openhound collect sccm`. | Date | Change | |---|---| +| 2026-09-30 | **Bounded verbose output from local client logs without losing host discovery** (§7). The scraper keeps reading CCM/ccmsetup logs on Windows even when `root\CCM` is absent. It logs one full URL and UNC sample per host, counts suppressed matches in one summary, and leaves the target set unchanged. A test covers repeated references across both log directories and discovery from logs left by a former client. | | 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 `: `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 / ` 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. | diff --git a/README.md b/README.md index 02030b7..bdcdfe4 100644 --- a/README.md +++ b/README.md @@ -260,7 +260,7 @@ These resources run a single time per collection and seed the per-host work queu | Discovery resource | What it does | Status | |---|---|---| | **LDAP** ([collectors/ldap.py](src/openhound_sccm/collectors/ldap.py)) | Queries the AD **System Management** container for SCCM **sites** (`mSSMSSite`), **management points** (`mSSMSManagementPoint`), the container **DACL**, network-boot servers, devices with the `CmRcService` SPN, and computers whose names/descriptions match SCCM naming patterns (`sccm`, `mecm`, `sms`, …). Registers discovered site systems as per-host targets. | ✅ Implemented | -| **Local** ([collectors/local.py](src/openhound_sccm/collectors/local.py)) | When run on an SCCM client: reads the `root\CCM` WMI namespace (`SMS_Authority`, `SMS_LookupMP`, `CCM_Client`) and parses CCM client logs to find management points / distribution points and the local client's SMSID. **Windows-only.** | ✅ Implemented (Windows) | +| **Local** ([collectors/local.py](src/openhound_sccm/collectors/local.py)) | On a Windows SCCM client, reads the `root\CCM` WMI namespace (`SMS_Authority`, `SMS_LookupMP`, `CCM_Client`) and the client's SMSID. On any Windows host, also parses existing CCM/ccmsetup logs for management points and distribution points, including logs left by a former client. The first URL and UNC match for each host is logged at VERBOSE; later matches are counted in one summary. Discovery still considers every distinct host. | ✅ Implemented (Windows) | | **DNS** ([collectors/dns.py](src/openhound_sccm/collectors/dns.py)) | For each discovered site code, resolves the `_mssms_mp_._tcp.` SRV record (with an ADIDNS/LDAP fallback) to find management points published to DNS. | ✅ Implemented | ## Stage 2 — Per-host phases diff --git a/src/openhound_sccm/collectors/local.py b/src/openhound_sccm/collectors/local.py index 13b32ef..46c098d 100644 --- a/src/openhound_sccm/collectors/local.py +++ b/src/openhound_sccm/collectors/local.py @@ -276,20 +276,35 @@ def local_client_logs_targets(ctx: "SourceContext") -> Iterable[dict[str, Any]]: # Maps discovered hostname (lowercase) to its source type for targeted log messages. discovered: dict[str, str] = {} + logged_references: set[tuple[str, str]] = set() + suppressed_matches = 0 def _parse(path: str) -> None: + nonlocal suppressed_matches file_name = os.path.basename(path) try: with open(path, "r", encoding="utf-8", errors="ignore") as f: for line in f: for m in unc_pattern.finditer(line): - unc_path = m.group(0).strip() - logger.verbose(f"Found UNC path in {file_name}: {unc_path}") - discovered.setdefault(m.group(1).lower(), "UNC path") + host = m.group(1).lower() + discovered.setdefault(host, "UNC path") + key = (host, "UNC path") + if key not in logged_references: + logged_references.add(key) + logger.verbose(f"Found UNC path in {file_name}: {m.group(0).strip()}") + else: + # Count repeat evidence once in the summary below. + suppressed_matches += 1 for m in url_pattern.finditer(line): - full_url = m.group(0).strip() - logger.verbose(f"Found URL in {file_name}: {full_url}") - discovered.setdefault(m.group(1).lower(), "URL") + host = m.group(1).lower() + discovered.setdefault(host, "URL") + key = (host, "URL") + if key not in logged_references: + logged_references.add(key) + logger.verbose(f"Found URL in {file_name}: {m.group(0).strip()}") + else: + # Count repeat evidence once in the summary below. + suppressed_matches += 1 except Exception as ex: logger.error(f"Failed to search log file {path}: {ex}") @@ -346,4 +361,9 @@ def _parse(path: str) -> None: else: logger.debug(f"Host found in client logs resolved to non-RFC1918 IP address, skipping: {host} ({resolved_ip})") else: - logger.verbose(f"Failed to resolve hostname {host} from {discovered[host]}") \ No newline at end of file + logger.verbose(f"Failed to resolve hostname {host} from {discovered[host]}") + + logger.verbose( + "Client log scrape found %d unique host(s); suppressed %d repeated URL/UNC host reference(s)", + len(discovered), suppressed_matches, + ) diff --git a/tests/local_log_scrape_regex_test.py b/tests/local_log_scrape_regex_test.py index 2fbb813..b1b7d00 100644 --- a/tests/local_log_scrape_regex_test.py +++ b/tests/local_log_scrape_regex_test.py @@ -1,8 +1,8 @@ """Regex tests for the local client-log scrape in ``collectors/local.py``. ``local_client_logs_targets()`` scans ``CCM``/``ccmsetup`` logs for UNC paths -and URLs, logs each hit at VERBOSE, and registers the *host* portion as a probe -target. These tests lock the behaviour touched by ope-8b99: +and URLs, logs the first hit per host and kind at VERBOSE, and registers the +*host* portion as a probe target. These tests cover: * the "Found URL" VERBOSE line prints the WHOLE url -- scheme + host + port + path + query. The ported URL regex used to stop at the hostname, so its @@ -12,6 +12,8 @@ probed. * UNC paths are already logged in full (that was never the bug); kept here as a contrast and a guard against an accidental UNC regression. + * repeated matches for a host are summarized without losing discovery, even + when its only evidence is leftover logs from a former client. The resource is an ``@app.resource`` generator; ``_raw()`` reaches the undecorated function so it runs directly against a stub context and a temp log @@ -111,3 +113,46 @@ def test_found_unc_logs_the_full_path(tmp_path, monkeypatch, caplog): list(_raw(local.local_client_logs_targets)(ctx)) assert f"Found UNC path in test.log: {unc}" in _messages(caplog) + + +def test_repeated_references_keep_targets_without_flooding_logs(tmp_path, monkeypatch, caplog): + unc = r"\\MP.mayyhem.com\SMSPKGD$\content\file.txt" + _point_at_log( + tmp_path, monkeypatch, + f"GET {FULL_URL} 200 OK", + *(f"GET https://MP.MAYYHEM.COM/content/part{i} 200 OK" for i in range(100)), + f"Copying {unc}", + ) + setup_logs = tmp_path / "ccmsetup" / "Logs" + setup_logs.mkdir(parents=True) + (setup_logs / "setup.log").write_text( + f"GET {FULL_URL} 200 OK\nCopying {unc}\nGET https://dp.mayyhem.com/content 200 OK", + encoding="utf-8", + ) + ctx = _Ctx(resolve_ip={"mp.mayyhem.com": "10.1.2.3", "dp.mayyhem.com": "10.1.2.4"}) + + with caplog.at_level(logging.DEBUG, logger="openhound_sccm.collectors.local"): + list(_raw(local.local_client_logs_targets)(ctx)) + + messages = _messages(caplog) + assert [m for m in messages if m.startswith("Found URL in")] == [ + f"Found URL in test.log: {FULL_URL}", + "Found URL in setup.log: https://dp.mayyhem.com/content", + ] + assert [m for m in messages if m.startswith("Found UNC path in")] == [ + f"Found UNC path in test.log: {unc}", + ] + assert ("mp.mayyhem.com", "Local-ClientLogs", None) in ctx.register_calls + assert ("dp.mayyhem.com", "Local-ClientLogs", None) in ctx.register_calls + assert len(ctx.register_calls) == 2 + assert "Client log scrape found 2 unique host(s); suppressed 102 repeated URL/UNC host reference(s)" in messages + + +def test_leftover_logs_discover_hosts_without_ccm_namespace(tmp_path, monkeypatch): + _point_at_log(tmp_path, monkeypatch, f"GET {FULL_URL} 200 OK") + monkeypatch.setattr(local, "_wmi_ccm", lambda: None) + ctx = _Ctx(resolve_ip={"mp.mayyhem.com": "10.1.2.3"}) + + list(_raw(local.local_client_logs_targets)(ctx)) + + assert ctx.register_calls == [("mp.mayyhem.com", "Local-ClientLogs", None)]