Ran a full review pass over both audit scripts and the HTML converter,
independently cross-checked against an automated code review, and fixed
everything that survived verification:
- ad_audit.py: connect() ignored the boolean return of conn.start_tls().
ldap3 doesn't raise on STARTTLS failure by default, so a rejected/
downgraded STARTTLS silently fell through to an unencrypted bind --
the bind DN and password would go out in cleartext with no warning.
Now aborts before calling bind() if start_tls() returns False.
- ad_audit.py: "Enabled accounts that have never logged on" detail rows
weren't filtering out disabled accounts, unlike the count used to
decide whether to show the finding at all -- so a disabled account
could appear in a list titled "enabled accounts."
- ad_audit.py: replaced deprecated datetime.datetime.utcnow() (scheduled
for removal) with timezone-aware datetime.now(timezone.utc); made
FILETIME_EPOCH aware too so the stale-account comparison keeps working
without a naive/aware TypeError.
- ad_audit.py: removed dead code (unused defaultdict import, unused
UAC_NOT_DELEGATED constant, unused scope_bit variable, import os
buried inside main() instead of at module level).
- Invoke-ADAudit.ps1: GroupCount/MemberCount used (@($x)).Count, which
PowerShell evaluates to 1 (not 0) when $x is $null -- @($null) is a
one-element array, not an empty one. This silently broke "empty group"
detection (the report's own headline feature) for any group with
genuinely zero members, and inflated GroupCount for users with no
group memberships. Added Get-SafeCount to null-check before counting.
- Invoke-ADAudit.ps1: $maxDepth came back as $null (not 0) when a
SearchBase had zero OUs, printing a blank instead of "0" in the report.
- Invoke-ADAudit.ps1 / ad_audit.py: the h4 finding-detail title wasn't
escaped through ConvertTo-MdSafe/md_escape in one of its three
occurrences, inconsistent with the other two -- fixed for consistency
even though today's titles are all fixed strings.
- md_to_html.py: badge_findings_table()'s regex matched any single-word
table cell equal to a severity name, not just the Severity column of
the findings table -- an OS name, group name, or object class that
happened to literally be "Critical"/"High"/etc (plausible under a
tiering naming scheme) would get rewritten into a colored badge
anywhere in the report. Rescoped to the one table whose header row is
literally "Severity | Finding | Count | Notes", which only this
generator ever emits.
- Added README.md covering both scripts' usage, what each collects, and
a documented known limitation (finding-detection logic is duplicated
between the Python and PowerShell implementations with no shared
source of truth).
Every fix verified with a reproduction before and after: a mocked
start_tls() failure confirms bind() is never reached; mocked $null
MemberOf/Members confirm counts are now 0; a zero-OU domain confirms
"Maximum nesting depth: 0" instead of blank; a disabled never-logged-on
account confirms it's excluded from the enabled-accounts finding; and
AD data literally named "Critical"/"High" confirms it no longer gets
badged outside the real findings table.
The previous fix escaped AD data at generation time in both scripts,
but the converter still blindly trusted its input -- a report from
before that fix, from a hand edit, or from a third-party tool would
still render live HTML unmodified.
Normalize the whole Markdown source before parsing: unescape any
existing entities, then re-escape &, <, > uniformly. The round trip
keeps already-escaped (freshly generated) reports single-escaped
instead of doubling up, while raw/legacy unescaped HTML gets
neutralized for the first time. Neither script intentionally emits
raw HTML, so this is safe across all normal report content.
Verified: a freshly-escaped report stays single-escaped (AT&T reads
as AT&T, not AT&amp;T), a hand-written report with a live
<script> tag gets neutralized, and a full report with badges/TOC/
timestamps/tables still renders identically to before.
Every table cell fed from directory content (descriptions, sAMAccountName,
OS strings, DNs, object classes) was interpolated into the Markdown report
raw. python-markdown doesn't escape inline HTML by default, so a
directory-controlled value like an OU description containing <script>...
would render live once the report was converted to HTML with md_to_html.py
-- and the underlying data is attacker-influenceable, not just
operator-authored.
Added md_escape() (Python) / ConvertTo-MdSafe (PowerShell), applied at
every table-row interpolation in both scripts. Escapes &, <, > to HTML
entities and | plus embedded newlines to keep the table structure intact.
Raw JSON dumps are left untouched -- this only affects the Markdown/HTML
presentation layer.
Verified end-to-end with <script>, <img onerror=...>, embedded &, and
embedded | payloads across every affected table in both scripts; confirmed
no live tags reach the rendered HTML and no double-escaping occurs.
Each finding's detail-list table is now its own indented nav item
under "Finding Detail Lists", one click away instead of scroll-only.
Also fixes a double-space artifact in h4 badge titles (regex wasn't
consuming the whitespace after "[Severity]" before inserting the
badge span), which was most visible once those titles started
feeding the nav labels.
badge_findings_table matched literal <h4> with no attributes; once
the toc extension started stamping id="..." on every heading (needed
for the sidebar nav), the h4 severity badges silently stopped
rendering. Match <h4[^>]*> instead.
Links to every h2/h3 section (h4 finding headers excluded -- there
can be many). Sticky on desktop, collapses to a stacked block above
the content on narrow viewports. Active section highlights via
IntersectionObserver as you scroll. Uses markdown's toc extension
for heading ids/slugs rather than hand-rolling a slugifier.
- All timestamps (last logon, password last set, whenCreated, report
generation time) now emit as RFC 3339 UTC in both the Markdown
report and raw JSON dump, for both the Python and PowerShell
scripts.
- md_to_html.py: click any table header to sort ascending/descending
(vanilla JS, numeric-aware); a "Show local time" toggle swaps every
timestamp between UTC and the viewer's local offset, still RFC 3339.
Converts an ad_audit_report_*.md (from either audit script) into a
self-contained, styled HTML page: severity badges on findings, sorted
tables, responsive layout, light/dark theme via prefers-color-scheme.
No network access needed to view -- all CSS is inlined.
Each finding in the Risk & Cleanup table now expands into a full list
of the matching accounts/computers/groups (sAMAccountName, last logon
or type, DN) so the report can be acted on directly instead of just
citing counts. Removed the now-redundant standalone stale-user and
empty-group sections since they're covered by the finding lists.
Surfaces blank-password, AS-REP roastable, unconstrained delegation,
lockout, stale-account, empty-group, and password-never-expires
counts as a severity-ranked findings table at the top of the report,
ahead of the full detail tables -- useful as a reorg-planning summary.
Read-only structural/health audit of Active Directory: OU tree, user
and computer account status/hygiene flags, and group breakdown. Two
equivalent implementations depending on available access -- raw LDAP
via ldap3, or Get-AD* cmdlets via RSAT on a domain-joined machine.
ergosteur