PR-Review #167: 72-byte password reject sits ahead of the LDAP bind and can lock out long-passphrase AD users

[Sev7eNup]

Automated review finding for PR #167 — "Security audit follow-up: install-directory ACL, log redaction, pre-auth limits, SignalR scoping, HSTS ordering"
Author: @Sev7eNup · Status: merged

The PR is otherwise strong: the other four fixes (H-18 install-dir ACL, M-31 scheduler log redaction, M-33 SignalR subscription eviction on workflow/folder move, L-17 HSTS/forwarded-headers ordering) are correct, coherent, and each ships accompanying tests (9 backend tests + 12 PowerShell template assertions). No new API endpoints (no CLI/MCP client obligation); the only new user-visible strings are backend English BadRequest messages, consistent with existing API messages; docs updated in step. One finding rises to medium:

• [medium] src/NodePilot.Api/Controllers/AuthController.cs:327-337 (finding M-32) — The new invalidPassword gate rejects any login where UTF8.GetByteCount(password) > MaxPasswordBytes (72) and runs at the very top of Login(), before TryLdapLoginAsync (which binds with the raw password via LdapAuthenticator.AuthenticateAsync). The inline comment justifies the cap by "every password-setting path already runs ValidatePasswordPolicy, which caps at MaxPasswordBytes" — but that premise only holds for local BCrypt accounts. For LDAP/AD the password lives in the directory, NodePilot never governs it, BCrypt's 72-byte truncation never applies, and AD permits passwords up to 256 chars. An AD user with a passphrase longer than 72 bytes (~72 ASCII chars, i.e. a ~12-word passphrase — plausible exactly in the security-conscious orgs that deploy LDAP SSO) now gets a generic 401 with audit reason invalid_password_length and never reaches the bind. The added test OverlongPassword_IsRejectedBeforeLdapOrThrottleWork pins this short-circuit, so the regression is intentional but the reasoning is incomplete for the LDAP path. Suggestion: apply the strict 72-byte reject only on the local-BCrypt branch (after the LDAP fall-through), or raise the pre-LDAP cap to the AD maximum (e.g. 256 bytes). The [RequestSizeLimit(8 KiB)] already on the endpoint removes the multi-megabyte DoS amplifier, so the tight 72-byte gate buys almost no additional abuse protection while introducing the lockout.

This is an automated PR-review finding and needs triage. Low-severity notes, if any, are intentionally omitted.