Release 18.0.6

keepercommander/__init__.py

@@ -10,4 +10,4 @@
 # Contact: commander@keepersecurity.com
 #
 
-__version__ = '18.0.5'
+__version__ = '18.0.6'

keepercommander/commands/pam_import/keeper_ai_settings.py

@@ -14,6 +14,7 @@
 
 from ...params import KeeperParams
 from ...keeper_dag import DAG, EdgeType
+from ...keeper_dag.exceptions import DAGPathException
 from ...keeper_dag.connection.commander import Connection
 from ...keeper_dag.types import PamEndpoints
 from ...vault import PasswordRecord
@@ -95,7 +96,11 @@
             read_endpoint=PamEndpoints.PAM,
             write_endpoint=PamEndpoints.PAM
         )
-        linking_dag.load()
+        try:
+            linking_dag.load()
+        except DAGPathException as e:
+            logging.debug(f"Skipping DATA edge listing for {resource_uid}: ambiguous DAG path ({e})")
+            return []
 
         # Get the resource vertex
         resource_vertex = linking_dag.get_vertex(resource_uid)
@@ -196,7 +201,14 @@
             read_endpoint=PamEndpoints.PAM,
             write_endpoint=PamEndpoints.PAM
         )
-        linking_dag.load()
+        try:
+            linking_dag.load()
+        except DAGPathException as e:
+            # The graph has duplicate/ambiguous vertices for this path. Treat it
+            # as "no settings available" rather than aborting the caller (e.g.
+            # pam launch); there is nothing we can read for this resource.
+            logging.debug(f"Skipping {dag_path} for {resource_uid}: ambiguous DAG path ({e})")
+            return None
 
         # Get the resource vertex
         resource_vertex = linking_dag.get_vertex(resource_uid)

keepercommander/commands/pam_launch/launch.py

@@ -286,6 +286,7 @@ def _print_close_reason_notice(
     reason: Optional[str],
     *,
     pending_exit_code: Optional[int],
+    session_established: bool = False,
 ) -> Optional[int]:
     """Show a user-facing notice for an involuntary remote close.
 
@@ -294,7 +295,19 @@ def _print_close_reason_notice(
     stopped and the local terminal is back in cooked mode — the message is the
     last thing the user sees before returning to Commander, so no acknowledge
     prompt is needed.
+
+    ``session_established`` reflects whether the guac session was live (≥1 sync /
+    data flowing) at the moment of close. A guacd-initiated disconnect — the user
+    typing ``exit`` / ``logout`` — is tagged ``GuacdError`` (code 14) by the
+    gateway, and ``server_disconnect`` when guacd sends an explicit ``disconnect``
+    opcode. Neither is an error once the session was running, so both are treated
+    as a normal close to avoid the misleading "Session ended (guacd_error)." line
+    on a clean logout. A ``guacd_error`` that arrives *before* the session went
+    live is a genuine connect-time fault and is still surfaced.
     """
+    if reason == 'server_disconnect' or (reason == 'guacd_error' and session_established):
+        reason = 'normal'
+
     if not reason or reason in ('normal', 'client'):
         return pending_exit_code
 
@@ -1557,6 +1570,10 @@ def _start_cli_session(
         # PyCloseConnectionReason). Set asynchronously by _on_session_disconnect
         # below; consumed in the inner finally to print a user-facing notice.
         closure_reason: Optional[str] = None
+        # Whether the guac session was live (≥1 sync) at the moment of remote
+        # close. Captured in _on_session_disconnect; lets the notice treat a
+        # guacd_error after an established session as a normal logout.
+        session_established_at_close: bool = False
         # Distinct exit code for involuntary terminations (KeeperAI, admin).
         # Raised as SystemExit at the end of the method so the inner/outer
         # finally cleanup blocks run first.
@@ -1628,9 +1645,15 @@ def _on_lease_expired():
             # admin, etc.). Runs on the rust callback thread — do not print
             # here; terminal is still in raw mode.
             def _on_session_disconnect(reason: str) -> None:
-                nonlocal closure_reason, shutdown_requested
+                nonlocal closure_reason, shutdown_requested, session_established_at_close
                 closure_reason = reason
                 shutdown_requested = True
+                # Snapshot whether data was flowing so the notice can tell a clean
+                # guacd-initiated logout from a real connect-time guacd fault.
+                try:
+                    session_established_at_close = bool(python_handler.is_data_flowing())
+                except Exception:
+                    pass
 
             python_handler.on_disconnect = _on_session_disconnect
 
@@ -2128,6 +2151,7 @@ def _remote_key_ctrl_c() -> None:
                 pending_exit_code = _print_close_reason_notice(
                     closure_reason,
                     pending_exit_code=pending_exit_code,
+                    session_established=session_established_at_close,
                 )
 
                 # Cleanup - check if connection is already closed to avoid deadlock

keepercommander/commands/workflow/helpers.py

@@ -298,7 +298,7 @@ def prompt_for_reason_ticket(needs_reason: bool, needs_ticket: bool) -> Tuple[Op
 
 class RecordResolver:
 
-    WORKFLOW_RECORD_TYPES = {'pamMachine', 'pamDirectory', 'pamDatabase', 'pamRemoteBrowser'}
+    WORKFLOW_RECORD_TYPES = {'pamCloudResource', 'pamMachine', 'pamDirectory', 'pamDatabase', 'pamRemoteBrowser'}
 
     @staticmethod
     def resolve(params, record_input, allow_missing=False):

keepercommander/keeper_dag/dag.py

@@ -441,6 +441,27 @@ def get_vertex(self, key) -> Optional[DAGVertex]:
 
         return None
 
+    def get_vertex_by_uid(self, uid: str) -> Optional[DAGVertex]:
+        """
+        Get a single vertex by its UID only.
+
+        Unlike get_vertex, this never falls back to path or name matching, so it
+        can never raise DAGPathException. Use this where the key is known to be a
+        UID (for example, the graph load and the add_vertex existence check),
+        otherwise an unloaded UID that happens to collide with a shared path
+        value would be misinterpreted as an ambiguous path.
+
+        :param uid: The UID of the vertex.
+        :return: DAGVertex instance, if it exists.
+        """
+
+        if uid is None:
+            return None
+        index = self._uid_lookup.get(uid)
+        if index is None:
+            return None
+        return self._vertices[index]
+
     @property
     def get_root(self) -> Optional[DAGVertex]:
         """
@@ -462,7 +483,12 @@ def vertex_exists(self, key: str) -> bool:
         :return:
         """
 
-        return self.get_vertex(key) is not None
+        try:
+            return self.get_vertex(key) is not None
+        except DAGPathException:
+            # The key matched multiple vertices by path value. We cannot pick a
+            # single vertex, but a matching vertex does exist.
+            return True
 
     def get_vertices_by_path_value(self, path: str, inc_deleted: bool = False) -> List[DAGVertex]:
         """
@@ -637,7 +663,7 @@ def _load(self, sync_point: int = 0):
             self.debug(f"  * edge {edge_type}, tail {tail_uid} to head {head_uid}", level=3)
 
             # We want to store this edge in the Vertex with the same value/UID as the ref.
-            if not self.vertex_exists(tail_uid):
+            if self.get_vertex_by_uid(tail_uid) is None:
                 self.debug(f"    * tail vertex {tail_uid} does not exists. create.", level=3)
                 self.add_vertex(
                     uid=tail_uid,
@@ -649,23 +675,23 @@ def _load(self, sync_point: int = 0):
                 )
 
             # Get the tail vertex.
-            tail = self.get_vertex(tail_uid)
+            tail = self.get_vertex_by_uid(tail_uid)
 
             # This most likely is a DELETION edge of a DATA edge.
             # Set the head to be the same as the tail.
             if head_uid is None:
                 head_uid = tail_uid
 
             # If the head vertex doesn't exist, we need to create.
-            if not self.vertex_exists(head_uid):
+            if self.get_vertex_by_uid(head_uid) is None:
                 self.debug(f"    * head vertex {head_uid} does not exists. create.", level=3)
                 self.add_vertex(
                     uid=head_uid,
                     name=data.parentRef.name,
                     vertex_type=RefType.GENERAL
                 )
             # Get the head vertex, which will exist now.
-            head = self.get_vertex(head_uid)
+            head = self.get_vertex_by_uid(head_uid)
             self.debug(f"    * tail {tail_uid} belongs to {head_uid}, "
                        f"edge type {edge_type}", level=3)
 
@@ -704,7 +730,7 @@ def _load(self, sync_point: int = 0):
             # Get the tail vertex.
             tail_uid = data.ref.value
             # We want to store this edge in the Vertex with the same value/UID as the ref.
-            if not self.vertex_exists(tail_uid):
+            if self.get_vertex_by_uid(tail_uid) is None:
                 self.debug(f"    * tail vertex {tail_uid} does not exists. create.", level=3)
                 self.add_vertex(
                     uid=tail_uid,
@@ -714,7 +740,7 @@ def _load(self, sync_point: int = 0):
                     # future.
                     vertex_type=RefType.find_enum(data.ref.type)
                 )
-            tail = self.get_vertex(tail_uid)
+            tail = self.get_vertex_by_uid(tail_uid)
 
             content = data.content
             if content is not None:
@@ -1264,7 +1290,7 @@ def add_vertex(self, name: Optional[str] = None, uid: Optional[str] = None,  key
             keychain=keychain,
             vertex_type=vertex_type
         )
-        if self.vertex_exists(vertex.uid):
+        if self.get_vertex_by_uid(vertex.uid) is not None:
             raise DAGVertexAlreadyExistsException(f"Vertex {vertex.uid} already exists.")
 
         # Set the UID to array index lookup.

unit-tests/pam/test_pam_launch_close_notice.py

@@ -0,0 +1,96 @@
+"""
+Unit tests for ``_print_close_reason_notice`` — the user-facing notice shown when a
+``pam launch`` guac session is closed by the remote end.
+
+Regression: a normal logout (user types ``exit`` / ``logout``) is reported by the
+gateway as ``guacd_error`` (CloseConnectionReason code 14) or ``server_disconnect``.
+The notice must NOT print the misleading "Session ended (guacd_error)." line once the
+session was actually live, while still surfacing a genuine connect-time guacd fault
+(``guacd_error`` before any data flowed).
+"""
+import importlib
+import io
+import os
+import sys
+import unittest
+from contextlib import redirect_stdout
+
+sys.path.insert(0, os.path.dirname(__file__))
+
+skip_tests = False
+skip_reason = ""
+try:
+    importlib.import_module('keepercommander.commands.pam_import.keeper_ai_settings')
+    from keepercommander.commands.pam_launch import launch as launch_mod
+    from keepercommander.commands.pam_launch.launch import (
+        _print_close_reason_notice,
+        EXIT_CODE_AI_TERMINATED,
+        EXIT_CODE_ADMIN_TERMINATED,
+    )
+except ImportError as e:  # pragma: no cover
+    skip_tests = True
+    skip_reason = f"Cannot import pam_launch.launch: {e}"
+
+
+@unittest.skipIf(skip_tests, skip_reason)
+class TestPrintCloseReasonNotice(unittest.TestCase):
+
+    def _notice(self, reason, *, session_established=False, pending_exit_code=None):
+        buf = io.StringIO()
+        with redirect_stdout(buf):
+            rc = _print_close_reason_notice(
+                reason,
+                pending_exit_code=pending_exit_code,
+                session_established=session_established,
+            )
+        return rc, buf.getvalue()
+
+    # --- the regression: clean logout must be silent -----------------------
+
+    def test_guacd_error_after_established_session_is_silent(self):
+        rc, out = self._notice('guacd_error', session_established=True)
+        self.assertEqual(out, '')
+        self.assertIsNone(rc)
+
+    def test_server_disconnect_is_silent_regardless_of_state(self):
+        for established in (True, False):
+            rc, out = self._notice('server_disconnect', session_established=established)
+            self.assertEqual(out, '')
+            self.assertIsNone(rc)
+
+    # --- genuine faults still surface --------------------------------------
+
+    def test_guacd_error_before_session_established_is_shown(self):
+        rc, out = self._notice('guacd_error', session_established=False)
+        self.assertIn('Session ended (guacd_error).', out)
+        self.assertIsNone(rc)
+
+    # --- unchanged behaviour for the other reasons -------------------------
+
+    def test_normal_and_client_are_silent(self):
+        for reason in ('normal', 'client', None):
+            rc, out = self._notice(reason, session_established=True)
+            self.assertEqual(out, '')
+
+    def test_ai_closed_returns_ai_exit_code(self):
+        rc, out = self._notice('ai_closed', session_established=True)
+        self.assertEqual(rc, EXIT_CODE_AI_TERMINATED)
+        self.assertIn('KeeperAI', out)
+
+    def test_admin_closed_returns_admin_exit_code(self):
+        rc, out = self._notice('admin_closed', session_established=True)
+        self.assertEqual(rc, EXIT_CODE_ADMIN_TERMINATED)
+        self.assertIn('administrator', out)
+
+    def test_other_reason_prints_generic_notice(self):
+        rc, out = self._notice('timeout', session_established=True)
+        self.assertIn('Session ended (timeout).', out)
+
+    def test_pending_exit_code_preserved_on_silent_close(self):
+        rc, out = self._notice('guacd_error', session_established=True, pending_exit_code=7)
+        self.assertEqual(out, '')
+        self.assertEqual(rc, 7)
+
+
+if __name__ == '__main__':
+    unittest.main()