local account discovery

[x032205]

Context

Screenshots

Steps to verify the change

Type

• Fix
• Feature
• Improvement
• Breaking
• Docs
• Chore

Checklist

• Title follows the conventional commit format: type(scope): short description (scope is optional, e.g., fix: prevent crash on sync or fix(api): handle null response).
• Tested locally
• Updated docs (if needed)
• Read the contributing guide

[maidul98]

⛔ Snyk checks have failed. 3 issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (3)
⛔	Open Source Security	1	0	2	0	3 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[greptile-apps]

Greptile Summary

This PR implements local Windows account discovery for the PAM AD discovery pipeline. It adds WinRM-based enumeration (via Get-LocalUser PowerShell over NTLM), custom DNS-over-TCP hostname resolution through the gateway proxy, and minor UI improvements (WinRM warning banner and an "AD" badge for domain accounts). The backend changes are substantial — three new helper functions and a new dependency chain.

Key concerns before merging:

• Unencrypted WinRM (port 5985): The hardcoded WINRM_PORT = 5985 uses the HTTP WinRM endpoint. Domain admin credentials and PowerShell output are sent in cleartext on the internal network segment between the gateway and each Windows Server. The secure port is 5986 (HTTPS).
• Unbounded buffer in resolveDnsTcp: Incoming DNS response bytes are concatenated without a size cap; a misbehaving or malicious DNS server could exhaust heap memory.
• Deprecated request package via winrm-client: winrm-client@0.0.11 (pre-release) pulls in ntlm-client → request@2.88.2, which is officially deprecated and no longer maintained, along with tough-cookie@2.5.0 and har-validator@5.1.5.
• Local account metadata not fully stored: LastLogon, PasswordLastSet, Enabled, SID, and Description fields returned by Get-LocalUser are silently discarded, causing the "Last Logon" column to always show "-" for local accounts and making disabled accounts indistinguishable from active ones.
• JSON.parse lacks a dedicated catch: Non-JSON WinRM output (e.g., PS warnings) causes a generic enumeration-failure log rather than a descriptive parse-error message.

Confidence Score: 2/5

• Not safe to merge — domain admin credentials are transmitted in cleartext over WinRM HTTP, and the dependency chain includes deprecated, unmaintained packages.
• The unencrypted WinRM port (5985) is a clear security regression for a feature that transmits domain administrator credentials. Combined with the pre-release winrm-client dependency pulling in a deprecated NTLM/HTTP stack, this needs remediation before production deployment.
• active-directory-discovery-factory.ts (WinRM port, buffer growth, metadata gaps) and package.json (deprecated dependency chain via winrm-client) require the most attention.

Important Files Changed

Filename	Overview
backend/src/ee/services/pam-discovery/active-directory/active-directory-discovery-factory.ts	Implements WinRM-based local account discovery: adds DNS-over-TCP hostname resolution, executeWinRmLocalAccountEnumeration via PowerShell, and upsertLocalAccount. Key issues: WinRM uses unencrypted HTTP port 5985, unbounded buffer growth in DNS response handling, local account metadata fields (LastLogon, SID, Enabled) are discarded, and JSON.parse lacks a dedicated catch for non-JSON WinRM output.
backend/package.json	Adds winrm-client@0.0.11 (production) and dns-packet@5.6.1. The winrm-client package is pre-release and pulls in the deprecated request@2.88.2 library via ntlm-client, introducing an unmaintained NTLM/HTTP stack with known-deprecated transitive dependencies.
frontend/src/pages/pam/PamDiscoveryDetailPage/PamDiscoveryDetailPage.tsx	Adds an informational WinRM warning banner in the Machine Errors section and an "AD" badge for Active Directory accounts in the Accounts tab. Straightforward UI changes with no logic issues.
backend/package-lock.json	Lockfile reflects added winrm-client, dns-packet, and their dependency trees. Notably includes deprecated packages: request@2.88.2, har-validator@5.1.5, tough-cookie@2.5.0, and uuid@3.4.0 (all transitive from ntlm-client → request).

_{Last reviewed commit: 58b8950}

[greptile-apps]

Unencrypted WinRM (HTTP port 5985)

WINRM_PORT = 5985 is the plain-HTTP WinRM port. All traffic — including NTLM authentication handshakes and PowerShell output — is transmitted in cleartext between the gateway endpoint and the Windows Server. An attacker with access to the internal network segment can capture domain credentials and command output through passive sniffing.

The secure port for WinRM is 5986 (HTTPS/SSL). The runPowershell call should be made over the encrypted channel to avoid credential exposure on the wire.

Suggested change

	const WINRM_PORT = 5985;
	const WINRM_PORT = 5986;

If HTTP is intentionally supported for development/lab environments, this should be configurable per-discovery-source (similar to how configuration.port is used for LDAP), not hardcoded.

[greptile-apps]

Unbounded buffer growth — potential DoS

The responseData buffer grows without any size cap. A malicious or misbehaving DNS server behind the gateway could stream an arbitrarily large TCP payload, causing the Node.js process to exhaust heap memory.

DNS-over-TCP messages are framed with a 2-byte length prefix, so the maximum legitimate DNS response is 65 535 bytes. Adding a guard before concatenating prevents memory exhaustion:

Suggested change

	let responseData = Buffer.alloc(0);
	socket.on("data", (chunk: Buffer) => {
	responseData = Buffer.concat([responseData, chunk]);
	// DNS-over-TCP frames each message with a 2-byte big-endian length prefix
	// wait until we have the full frame before decoding
	if (responseData.length >= 2) {
	const msgLen = responseData.readUInt16BE(0);
	if (responseData.length >= 2 + msgLen) {
	const response = dnsPacket.streamDecode(responseData);
	const aRecord = response.answers?.find((a) => a.type === "A");
	socket.destroy();
	resolve(aRecord && "data" in aRecord ? (aRecord.data as string) : null);
	}
	}
	socket.on("data", (chunk: Buffer) => {
	responseData = Buffer.concat([responseData, chunk]);
	// Enforce a reasonable upper bound (DNS-over-TCP max is 65 535 bytes + 2-byte length prefix)
	if (responseData.length > 65537) {
	socket.destroy();
	resolve(null);
	return;
	}
	// DNS-over-TCP frames each message with a 2-byte big-endian length prefix
	// wait until we have the full frame before decoding
	if (responseData.length >= 2) {
	const msgLen = responseData.readUInt16BE(0);
	if (responseData.length >= 2 + msgLen) {
	const response = dnsPacket.streamDecode(responseData);
	const aRecord = response.answers?.find((a) => a.type === "A");
	socket.destroy();
	resolve(aRecord && "data" in aRecord ? (aRecord.data as string) : null);
	}
	}
	});

[greptile-apps]

JSON.parse without dedicated error handling

If PowerShell emits warnings, progress output, or non-JSON error text before the actual JSON payload (common when RSAT is not installed or the user count is exactly zero), JSON.parse(stdout) will throw a SyntaxError. The outer try/catch (lines ~870-919) will surface this as a generic WinRM enumeration failure, losing the actual parse-error detail.

A targeted catch gives better diagnostics:

Suggested change

	}
	const parsed = JSON.parse(stdout) as TWinRmLocalUser | TWinRmLocalUser[];
	return Array.isArray(parsed) ? parsed : [parsed];
	let parsed: TWinRmLocalUser | TWinRmLocalUser[];
	try {
	parsed = JSON.parse(stdout) as TWinRmLocalUser | TWinRmLocalUser[];
	} catch (parseErr) {
	logger.warn({ hostname: computer.dNSHostName || computer.cn, stdout }, "WinRM: failed to parse Get-LocalUser output as JSON");
	return [];
	}
	return Array.isArray(parsed) ? parsed : [parsed];

[greptile-apps]

Local account metadata fields not stored

The TWinRmLocalUser type carries LastLogon, PasswordLastSet, Description, Enabled, and SID.Value, but the internalMetadata stored for each local account only includes accountType. As a result:

• The "Last Logon" column in the Accounts tab always shows "-" for local accounts.
• Enabled state is not tracked, so disabled accounts appear indistinguishable from active ones.
• The SID.Value field is useful as a stable fingerprint but is discarded.

Consider extending TWindowsAccountMetadata (or creating a dedicated local-account metadata type) to preserve at least enabled, lastLogon, and sid for display and policy purposes.

[greptile-apps]

Deprecated transitive dependencies via winrm-client

winrm-client@0.0.11 is a pre-release (0.0.x) package that pulls in ntlm-client@0.1.1, which in turn depends on request@2.88.2. The request library is officially deprecated and explicitly notes it is no longer supported (see its own changelog). It also brings in tough-cookie@2.5.0 and har-validator@5.1.5, both of which are flagged as deprecated in the generated package-lock.json.

Using an unmaintained NTLM/HTTP stack at this level exposes the authentication channel to unpatched CVEs. Before shipping, it's worth evaluating:

1. Whether winrm-client has an alternative that uses a maintained HTTP client (e.g., one built on undici or node-fetch).
2. Pinning winrm-client to a specific integrity hash and regularly auditing with npm audit given the unmaintained transitive tree.