Align AGENTS.md with current review conventions

- invalid module options: fail() and return False from options(), no exit() (#1346)
- concise code, no stub variables and fail() over warning() apply to all code
- one-line call rule also covers raise and log calls
- e2e lines use the file's placeholders, never real targets or credentials
- name sanitize_path_component() and allow lazy imports of heavy deps (#1475)
- drop the duplicate ruff bullet
- take the AGENTS.md lines from #1383

AGENTS.md

@@ -7,10 +7,18 @@
 
 # Coding style
 
+- Never add a comment or docstring at the top of a Python file unless that file already has a top-of-file comment or docstring.
+- In parenthesized Python import blocks, group multiple imported names on each line instead of placing every name on its own line.
+- Keep function and method call arguments on one line when practical, including `raise` and log calls; format object literals for readability.
+- Keep comments short and precise. Use block form for docstrings and other long text.
 - Assume ordinary LDAP searches and LDAP response parsing do not raise exceptions; do not wrap them in `try`/`except`.
 - Always bind caught exceptions as `e`, not `error` or another name.
 - Do not prefix function or method names with underscores; preserve required Python special methods such as `__init__`.
 - Do not assume the host runs Linux; keep code and paths cross-platform unless the task explicitly targets one operating system.
+- Keep implementations as short and concise as practical. Prefer direct control flow and shared error handling over wrappers, redundant branches, or defensive boilerplate.
+- Avoid stub variables that merely hold a value for one immediate use. Pass data directly unless a local name improves clarity, prevents repeated work, or represents meaningful state.
+- Do not use `.warning()` logging. Use `.fail()` when logic or an operation fails.
+- If unsure about anything, ask for clarification before proceeding.
 
 # NetExec protocol structure
 
@@ -23,26 +31,21 @@
 - In files under `nxc/modules/`, do not define variables, constants, or helper functions at module scope.
 - Keep all module-specific state, constants, and helper methods inside the `NXCModule` class.
 - Module scope should contain only imports and the `NXCModule` class definition.
-- When a module must terminate for invalid options, import `exit` with `from sys import exit` and call `exit(...)`; do not use `import sys` with `sys.exit(...)`.
+- When module options are invalid, log the reason with `context.log.fail()` and `return False` from `options()`; do not call `exit()`.
 - Do not over-validate module options. Check only that required arguments are present, mutually exclusive arguments are not combined, or a value would otherwise cause the module to fail before performing its normal lookup or operation.
 - Do not validate whether user-supplied object names, identifiers, or similar values are semantically correct when the underlying protocol lookup can handle them; let a misspecified value produce the normal "not found" result.
-- Avoid stub variables that merely hold a value for one immediate use. Pass data directly unless a local name improves clarity, prevents repeated work, or represents meaningful state.
-- Keep module implementations as short and concise as practical. Prefer direct control flow and shared error handling over wrappers, redundant branches, or defensive boilerplate.
-- Do not optimize module code for line length; long lines are acceptable when they keep straightforward logic together.
-- Do not use `.warning()` logging in NetExec modules. Use `.fail()` when logic or an operation fails.
-- Run `ruff check .` after every coding session.
 
 # Development and validation
 
 - To verify that a module works, run the module's actual NetExec command instead of pytest or compilation checks.
-- Record every module-verification command in `tests/e2e_commands.txt`.
+- Record every module-verification command in `tests/e2e_commands.txt` using the file's placeholders (`TARGET_HOST`, `LOGIN_USERNAME`, `LOGIN_PASSWORD`, `KERBEROS`), never real targets or credentials.
 - Do not run compilation checks.
 - Run Ruff after completing a coding session.
 
 # Filesystem and source conventions
 
 - Assume that the underlying operating system and its storage are fully secure. Files and other data produced by NetExec may intentionally contain readable plaintext credentials; do not add encryption, redaction, restrictive permission handling, or permission warnings unless explicitly requested.
-- Before opening, creating, writing, renaming, moving, or deleting a file or directory, sanitize every path component derived from untrusted data and verify that the resulting path cannot traverse outside its intended base directory.
+- Before opening, creating, writing, renaming, moving, or deleting a file or directory, sanitize every path component derived from untrusted data with `sanitize_path_component()` from `nxc/helpers/path.py` and verify that the resulting path cannot traverse outside its intended base directory.
 - Place every file and directory produced by NetExec under `NXC_PATH` by default, using `TMP_PATH` for temporary artifacts. An explicit user-provided output path or task requirement may override this default.
 - `NXC_PATH` defaults to `~/.nxc` but may be overridden by the environment. Import and use `NXC_PATH` or its derived path constants instead of hardcoding `~/.nxc`.
-- Place imports at the top of files unless explicitly instructed otherwise.
+- Place imports at the top of files unless explicitly instructed otherwise; only a heavy, rarely used dependency on a hot path, such as protocol loading, may be imported where it is used.

nxc/protocols/rdp.py

@@ -26,10 +26,13 @@ from aardwolf.commons.iosettings import RDPIOSettings
 from aardwolf.commons.target import RDPTarget
 from aardwolf.keyboard.layoutmanager import KeyboardLayoutManager
 from aardwolf.protocol.x224.constants import SUPP_PROTOCOLS
+from aardwolf.network.x224 import X224Network
+from aardwolf.network.tpkt import TPKTPacketizer
 from asyauth.common.credentials.ntlm import NTLMCredential
 from asyauth.common.credentials.kerberos import KerberosCredential
 from asyauth.common.constants import asyauthSecret
 from asysocks.unicomm.common.target import UniTarget, UniProto
+from asysocks.unicomm.client import UniClient
 
 
 class rdp(connection):
@@ -118,40 +121,36 @@ class rdp(connection):
             except Exception as e:
                 self.logger.debug(f"Error adding host {self.host} into db: {e!s}")
 
-    def create_conn_obj(self, credentials=None, supported_protocols=SUPP_PROTOCOLS.SSL):
-        """Create self.conn. The call from proto_flow() has no credentials: it is the null-auth probe, which returns False when the host doesn't answer."""
-        if credentials is None:
-            self.target = RDPTarget(ip=self.host, domain="FAKE", port=self.port, timeout=self.args.rdp_timeout, unsafe_ssl=True)  # old servers only offer ciphers OpenSSL rejects at its default security level
-            self.auth = NTLMCredential(secret="pass", username="user", domain="FAKE", stype=asyauthSecret.PASS)
+    def _create_rdp_connection(self, credentials, supported_protocols=None):
         iosettings = self.iosettings.clone_for_connection()
-        # the probe, the enumeration and the nla screenshot pin the protocols, the login passes None so aardwolf picks the X224 flags for the credential type
+        # Explicit protocols are used by discovery and screenshot fallbacks.
+        # Authenticated connections leave this unset so aardwolf selects the
+        # X224 flags appropriate for the credential type.
         iosettings.supported_protocols = supported_protocols
-        self.conn = RDPConnection(iosettings=iosettings, target=copy.deepcopy(self.target), credentials=copy.deepcopy(credentials))
-        if credentials is not None:
-            return True
+        return RDPConnection(
+            iosettings=iosettings,
+            target=copy.deepcopy(self.target),
+            credentials=copy.deepcopy(credentials),
+        )
 
-        self.logger.debug(f"Checking NLA for {self.host}")
-        try:
-            self.runner.run(self.connect_rdp(auth_only=True))
-        except OSError as e:
-            self.logger.debug(f"No RDP answer from {self.host}:{self.port}: {e!r}")
-            return False
-        except Exception as e:
-            # a server that requires NLA refuses this SSL-only offer, else auth_only stops with a RuntimeError before any session opens
-            self.nla = not isinstance(e, RuntimeError) and "SSL_NOT_ALLOWED_BY_SERVER" not in str(e)
-        return True
+    def create_conn_obj(self):
+        self.target = RDPTarget(ip=self.host, domain="FAKE", port=self.port, timeout=self.args.rdp_timeout, unsafe_ssl=True)  # old servers only offer ciphers OpenSSL rejects at its default security level
+        self.auth = NTLMCredential(secret="pass", username="user", domain="FAKE", stype=asyauthSecret.PASS)
+
+        self.runner.run(self.check_nla())
 
-    def enum_host_info(self):
         for proto in reversed(self.protoflags):
             try:
-                self.create_conn_obj(self.auth, supported_protocols=proto)
+                self.conn = self._create_rdp_connection(
+                    self.auth, supported_protocols=proto
+                )
                 self.runner.run(self.connect_rdp_with_cleanup())
             except OSError as e:
                 if "Errno 104" not in str(e):
-                    break
+                    return False
             except Exception as e:
                 if "TCPSocket" in str(e):
-                    break
+                    return False
                 if "Reason:" not in str(e):
                     try:
                         info_domain = self.conn.get_extra_info()
@@ -175,11 +174,21 @@ class rdp(connection):
             self.kdcHost = result["host"] if result else None
             self.logger.info(f"Resolved domain: {self.domain} with dns, kdcHost: {self.kdcHost}")
 
-        self.target = RDPTarget(ip=self.host, hostname=self.hostname, port=self.port, domain=self.domain, dc_ip=self.domain, timeout=self.args.rdp_timeout, unsafe_ssl=True)
+        self.target = RDPTarget(
+            ip=self.host,
+            hostname=self.hostname,
+            port=self.port,
+            domain=self.domain,
+            dc_ip=self.domain,
+            timeout=self.args.rdp_timeout,
+            unsafe_ssl=True,
+        )
 
         # Reset so aardwolf negotiates on its own (restricted admin + HYBRID_EX), the loop above pinned it
         self.iosettings.supported_protocols = None
 
+        return True
+
     def guess_os(self, info_domain):
         build = info_domain.get("os_build")
         if build in WIN_VERSIONS:
@@ -188,9 +197,33 @@ class rdp(connection):
             return f"Windows 10.0 Build {build}"
         return f"{info_domain.get('os_guess') or 'Unknown'} Build {build or 'Unknown'}"
 
-    async def connect_rdp(self, auth_only=False):
+    async def check_nla(self):
+        self.logger.debug(f"Checking NLA for {self.host}")
+        probe_connection = None
+        try:
+            self.conn = self._create_rdp_connection(
+                None, supported_protocols=SUPP_PROTOCOLS.SSL
+            )
+            packetizer = TPKTPacketizer()
+            client = UniClient(self.target, packetizer)
+            probe_connection = await asyncio.wait_for(client.connect(), timeout=self.args.rdp_timeout)
+            self.conn._connection = probe_connection
+            self.conn._x224net = X224Network(self.conn._connection)
+            _, err = await asyncio.wait_for(self.conn._x224net.client_negotiate(0, SUPP_PROTOCOLS.SSL), timeout=self.args.rdp_timeout)
+            # If no error SSL supported if SSL_NOT_ALLOWED_BY_SERVER error, plain RDP supported
+            if err is None or "SSL_NOT_ALLOWED_BY_SERVER" in str(err):
+                self.nla = False
+                return
+        except Exception:
+            pass
+        finally:
+            # this probe socket is not owned by aardwolf, terminate() would not close it
+            if probe_connection is not None:
+                await probe_connection.close()
+
+    async def connect_rdp(self):
         """Connect to the RDP server. Does NOT clean up on exit."""
-        _, err = await asyncio.wait_for(self.conn.connect(auth_only=auth_only), timeout=self.args.rdp_timeout)
+        _, err = await asyncio.wait_for(self.conn.connect(), timeout=self.args.rdp_timeout)
         if err is not None:
             raise err
         self.logger.debug(f"RDP negotiated protocol: {self.conn.x224_protocol}, authz_result: {self.conn.authz_result}")
@@ -217,7 +250,7 @@ class rdp(connection):
         That PDU never comes when access is authorized but the logon is refused (restricted admin).
         """
         await self.terminate_conn()  # a session kept by an earlier success (--continue-on-success) would take a session slot this logon needs
-        self.create_conn_obj(self.auth, supported_protocols=None)
+        self.conn = self._create_rdp_connection(self.auth)
         await self.connect_rdp()
         try:
             await asyncio.wait_for(self.conn.logon_info_received.wait(), timeout=self.args.rdp_timeout)
@@ -706,7 +739,7 @@ Add-Type -AssemblyName System.Windows.Forms
         connected = False
         for proto in self.protoflags_nla:
             try:
-                self.create_conn_obj(self.auth, supported_protocols=proto)
+                self.conn = self._create_rdp_connection(self.auth, supported_protocols=proto)
                 await self.connect_rdp()
             except Exception as e:
                 self.logger.debug(f"Failed to connect for nla_screenshot with {proto} {e}")