From aeeaae8307efe74e0655a7b9b69d43d02fee2e14 Mon Sep 17 00:00:00 2001 From: erinlewis-keeper Date: Fri, 5 Jun 2026 09:17:10 -0700 Subject: [PATCH 1/4] adds pamCloudResource to supported workflow types --- keepercommander/commands/workflow/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keepercommander/commands/workflow/helpers.py b/keepercommander/commands/workflow/helpers.py index 7dede3c8d..2851c9c46 100644 --- a/keepercommander/commands/workflow/helpers.py +++ b/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): From 3d15cc59af6bf3b1b4baf40f6d92397d0a044f0e Mon Sep 17 00:00:00 2001 From: idimov-keeper <78815270+idimov-keeper@users.noreply.github.com> Date: Fri, 5 Jun 2026 15:38:30 -0500 Subject: [PATCH 2/4] pam launch: don't show guacd_error notice on normal logout (#2126) --- keepercommander/commands/pam_launch/launch.py | 26 ++++- .../pam/test_pam_launch_close_notice.py | 96 +++++++++++++++++++ 2 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 unit-tests/pam/test_pam_launch_close_notice.py diff --git a/keepercommander/commands/pam_launch/launch.py b/keepercommander/commands/pam_launch/launch.py index 503478a75..62ecb9ab4 100644 --- a/keepercommander/commands/pam_launch/launch.py +++ b/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 diff --git a/unit-tests/pam/test_pam_launch_close_notice.py b/unit-tests/pam/test_pam_launch_close_notice.py new file mode 100644 index 000000000..647cc2741 --- /dev/null +++ b/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() From 81a7e0e91ded66c69f3ecc45a93aaec0717edd54 Mon Sep 17 00:00:00 2001 From: Ivan Dimov <78815270+idimov-keeper@users.noreply.github.com> Date: Fri, 5 Jun 2026 12:40:56 -0500 Subject: [PATCH 3/4] Fix pam launch DAGPathException: UID-exact vertex lookup + guard --- .../commands/pam_import/keeper_ai_settings.py | 16 ++++++- keepercommander/keeper_dag/dag.py | 42 +++++++++++++++---- 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/keepercommander/commands/pam_import/keeper_ai_settings.py b/keepercommander/commands/pam_import/keeper_ai_settings.py index 13a767a15..1b84cf7c3 100644 --- a/keepercommander/commands/pam_import/keeper_ai_settings.py +++ b/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 @@ def list_resource_data_edges( 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 @@ def get_resource_settings( 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) diff --git a/keepercommander/keeper_dag/dag.py b/keepercommander/keeper_dag/dag.py index 2f928e7c7..b929c8610 100644 --- a/keepercommander/keeper_dag/dag.py +++ b/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,7 +675,7 @@ 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. @@ -657,7 +683,7 @@ def _load(self, sync_point: int = 0): 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, @@ -665,7 +691,7 @@ def _load(self, sync_point: int = 0): 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. From 53b8d643c2c9229786990e969be56bb4fbd26042 Mon Sep 17 00:00:00 2001 From: Sergey Kolupaev Date: Fri, 5 Jun 2026 14:43:39 -0700 Subject: [PATCH 4/4] Release 18.0.6 --- keepercommander/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keepercommander/__init__.py b/keepercommander/__init__.py index 3ad1bb2ec..f16f8c4d4 100644 --- a/keepercommander/__init__.py +++ b/keepercommander/__init__.py @@ -10,4 +10,4 @@ # Contact: commander@keepersecurity.com # -__version__ = '18.0.5' +__version__ = '18.0.6'