diff --git a/keepercommander/commands/discoveryrotation.py b/keepercommander/commands/discoveryrotation.py index 6f714dc03..dc14ec0aa 100644 --- a/keepercommander/commands/discoveryrotation.py +++ b/keepercommander/commands/discoveryrotation.py @@ -3294,7 +3294,34 @@ def execute(self, params, **kwargs): if rri_status_name == 'RRS_ONLINE': configuration_uid = utils.base64_url_encode(rri.configurationUid) - gateway_name = rri.controllerName if rri.controllerName else '-' + gateway_name = rri.controllerName + if not gateway_name and rri.controllerUid: + def _normalize_uid(uid): + if uid is None: + return None + if isinstance(uid, (bytes, bytearray)): + return utils.base64_url_encode(uid) + return str(uid) + + target_uid = _normalize_uid(rri.controllerUid) + if target_uid is None: + gateway_name = None + else: + try: + all_gateways = gateway_helper.get_all_gateways(params) or [] + except (Exception,) as ex: + logging.debug(f"Failed to retrieve gateway list for name resolution: {ex}") + all_gateways = [] + + if all_gateways: + matched = next((g for g in all_gateways + if _normalize_uid(getattr(g, 'controllerUid', None)) == target_uid), None) + if matched: + gateway_name = getattr(matched, 'controllerName', None) + if gateway_name: + logging.debug(f"Resolved gateway name from controllerUid {target_uid} -> {gateway_name}") + + gateway_name = gateway_name if gateway_name else '-' gateway_uid = utils.base64_url_encode(rri.controllerUid) if rri.controllerUid else '-' def is_resource_ok(resource_id, params, configuration_uid): diff --git a/keepercommander/commands/enterprise.py b/keepercommander/commands/enterprise.py index 6e5b1a362..c11a57a87 100644 --- a/keepercommander/commands/enterprise.py +++ b/keepercommander/commands/enterprise.py @@ -1244,19 +1244,44 @@ def execute(self, params, **kwargs): if not matched_nodes: raise CommandError('enterprise-node', 'No nodes to toggle.') + toggled_nodes = [] for mn in matched_nodes: node_id = mn['node_id'] data = mn['data'] - displayname = data['displayname'] + displayname = data.get('displayname') or str(node_id) + was_isolated = bool(mn.get('restrict_visibility')) + is_root = not mn.get('parent_id') request = enterprise_pb2.SetRestrictVisibilityRequest() - request.nodeId = node_id + # Root isolation is an enterprise-level flag returned in + # GeneralDataEntity rather than on the root Node entity. + request.nodeId = 0 if is_root else node_id try: api.communicate_rest(params, request, 'enterprise/set_restrict_visibility') - mn['restrict_visibility'] = not (mn.get('restrict_visibility') or False) - logging.warning('good result: {}'.format(displayname)) + toggled_nodes.append((node_id, displayname, was_isolated)) except Exception as e: logging.warning('node \"%s\": toggle isolation failed: %s', displayname, e) - api.query_enterprise(params) + if toggled_nodes: + api.query_enterprise(params, force=True) + refreshed_nodes = { + x['node_id']: x for x in (params.enterprise or {}).get('nodes', []) + } + for node_id, displayname, was_isolated in toggled_nodes: + refreshed_node = refreshed_nodes.get(node_id) + if not refreshed_node: + logging.warning( + 'node \"%s\": isolation toggle could not be verified after refresh', + displayname) + continue + is_isolated = bool(refreshed_node.get('restrict_visibility')) + if is_isolated == was_isolated: + logging.warning( + 'node \"%s\": server accepted the isolation toggle, ' + 'but the state did not change', + displayname) + else: + logging.info( + 'node \"%s\": isolation is now %s', + displayname, 'enabled' if is_isolated else 'disabled') else: for node_name in unmatched_nodes: logging.warning('Node \'%s\' is not found: Skipping', node_name) diff --git a/keepercommander/commands/tunnel_and_connections.py b/keepercommander/commands/tunnel_and_connections.py index 3a530118b..e8f20433a 100644 --- a/keepercommander/commands/tunnel_and_connections.py +++ b/keepercommander/commands/tunnel_and_connections.py @@ -11,14 +11,13 @@ import argparse import datetime -import http.client import json import logging import os import platform +import requests import signal import socket -import ssl import struct import subprocess import sys @@ -1732,48 +1731,60 @@ def _parse_stun(cls, data: bytes) -> dict: # ── individual Python-side tests ────────────────────────────────────────── @classmethod - def _test_https(cls, hostname: str, port: int = 443) -> Tuple[bool, str, int]: + def _test_https(cls, hostname: str, port: int = 443, proxies=None, verify=True) -> Tuple[bool, str, int]: """Returns (passed, detail, ms).""" t0 = time.monotonic() - conn = None + resp = None try: - ctx = ssl.create_default_context() - conn = http.client.HTTPSConnection(hostname, port=port, context=ctx, timeout=10) - conn.request('GET', '/', headers={'User-Agent': 'keeper-pam-diagnose/1.0'}) - resp = conn.getresponse() + resp = requests.get( + f'https://{hostname}:{port}/', + headers={'User-Agent': 'keeper-pam-diagnose/1.0'}, + proxies=proxies, + verify=verify, + timeout=10, + stream=True, + ) ms = int((time.monotonic() - t0) * 1000) - return 100 <= resp.status < 400, f'HTTP {resp.status} (reachable)', ms + return 100 <= resp.status_code < 400, f'HTTP {resp.status_code} (reachable)', ms except Exception as exc: return False, str(exc)[:60], int((time.monotonic() - t0) * 1000) finally: - if conn: - try: conn.close() - except Exception: pass + if resp is not None: + try: + resp.close() + except Exception: + pass @classmethod - def _test_websocket(cls, hostname: str, port: int = 443) -> Tuple[bool, str, int]: + def _test_websocket(cls, hostname: str, port: int = 443, proxies=None, verify=True) -> Tuple[bool, str, int]: """HTTP Upgrade probe — any 4xx means the server is reachable.""" t0 = time.monotonic() - conn = None + resp = None try: - ctx = ssl.create_default_context() - conn = http.client.HTTPSConnection(hostname, port=port, context=ctx, timeout=10) - conn.request('GET', '/', headers={ - 'Upgrade': 'websocket', - 'Connection': 'Upgrade', - 'Sec-WebSocket-Key': 'dGhlIHNhbXBsZSBub25jZQ==', - 'Sec-WebSocket-Version': '13', - 'User-Agent': 'keeper-pam-diagnose/1.0', - }) - resp = conn.getresponse() + resp = requests.get( + f'https://{hostname}:{port}/', + headers={ + 'Upgrade': 'websocket', + 'Connection': 'Upgrade', + 'Sec-WebSocket-Key': 'dGhlIHNhbXBsZSBub25jZQ==', + 'Sec-WebSocket-Version': '13', + 'User-Agent': 'keeper-pam-diagnose/1.0', + }, + proxies=proxies, + verify=verify, + timeout=10, + stream=True, + ) ms = int((time.monotonic() - t0) * 1000) - return 100 <= resp.status < 400, f'HTTP {resp.status}', ms + return 100 <= resp.status_code < 400, f'HTTP {resp.status_code}', ms except Exception as exc: return False, str(exc)[:60], int((time.monotonic() - t0) * 1000) finally: - if conn: - try: conn.close() - except Exception: pass + if resp is not None: + try: + resp.close() + except Exception: + pass @classmethod def _test_tcp_stun(cls, hostname: str) -> Tuple[bool, str, int, Optional[str]]: @@ -1963,10 +1974,12 @@ def _record(name: str, passed: bool, detail: str, ms: int): except Exception as exc: _record(f'DNS {server_host}', False, str(exc)[:60], int((time.monotonic() - t0) * 1000)) - passed, detail, ms = self._test_https(server_host) + passed, detail, ms = self._test_https( + server_host, proxies=params.rest_context.proxies, verify=params.ssl_verify) _record(f'HTTPS {server_host}:443', passed, detail, ms) - passed, detail, ms = self._test_websocket(connect_host) + passed, detail, ms = self._test_websocket( + connect_host, proxies=params.rest_context.proxies, verify=params.ssl_verify) _record(f'WebSocket {connect_host}:443', passed, detail, ms) print() diff --git a/keepercommander/enterprise.py b/keepercommander/enterprise.py index 74808877f..f7de2b747 100644 --- a/keepercommander/enterprise.py +++ b/keepercommander/enterprise.py @@ -185,6 +185,7 @@ def load(self, params): # type: (KeeperParams) -> None params.enterprise['keys'] = keys entities = set() + root_restrict_visibility = None while True: rq = proto.EnterpriseDataRequest() if self._continuationToken: @@ -202,6 +203,8 @@ def load(self, params): # type: (KeeperParams) -> None params.enterprise['enterprise_name'] = self._enterprise.enterprise_name if rs.generalData.distributor: params.enterprise['distributor'] = True + if rs.HasField('generalData'): + root_restrict_visibility = rs.generalData.restrictVisibility for ed in rs.data: entities.add(ed.entity) @@ -212,6 +215,11 @@ def load(self, params): # type: (KeeperParams) -> None self._continuationToken = rs.continuationToken if not rs.hasMore: break + if root_restrict_visibility is not None: + root_node = next((x for x in params.enterprise.get('nodes', []) if not x.get('parent_id')), None) + if root_node: + _set_or_remove(root_node, 'restrict_visibility', + True if root_restrict_visibility else None) if proto.MANAGED_NODES in entities: try: self.load_missing_role_keys(params) @@ -456,8 +464,9 @@ def to_keeper_entity(self, proto_entity, keeper_entity): # type: (proto.Node, d _set_or_remove(keeper_entity, 'rsa_enabled', True if proto_entity.rsaEnabled else None) _set_or_remove(keeper_entity, 'sso_service_provider_id', proto_entity.ssoServiceProviderId if proto_entity.ssoServiceProviderId > 0 else None) - _set_or_remove(keeper_entity, 'restrict_visibility', - proto_entity.restrictVisibility if proto_entity.restrictVisibility else None) + if keeper_entity.get('parent_id'): + _set_or_remove(keeper_entity, 'restrict_visibility', + proto_entity.restrictVisibility if proto_entity.restrictVisibility else None) data = {} if 'encrypted_data' in keeper_entity: diff --git a/keepercommander/resources/service_config.ini b/keepercommander/resources/service_config.ini index e20d6479b..411bf8985 100644 --- a/keepercommander/resources/service_config.ini +++ b/keepercommander/resources/service_config.ini @@ -6,6 +6,11 @@ ngrok_custom_domain_prompt = Enter Ngrok Custom Domain: cloudflare_prompt = Enable Cloudflare Tunneling? (y/n): cloudflare_token_prompt = Enter Cloudflare tunnel token: cloudflare_custom_domain_prompt = Enter Cloudflare custom domain: +tailscale_prompt = Enable Tailscale Funnel? (y/n): +tailscale_install_prompt = Tailscale CLI is not installed. Attempt automatic installation now? (y/n): +tailscale_daemon_start_prompt = Tailscale daemon is not running. Attempt to start it now? (y/n): +tailscale_auth_key_prompt = Enter Tailscale auth key: +tailscale_advertise_tags_prompt = Enter Tailscale ACL tags to advertise, comma-separated (optional, required for OAuth-derived auth keys): run_mode_prompt = Select run mode (foreground/background): queue_enabled_prompt = Enable Request Queue? (y/n): tls_certificate = Enable TLS Certificate? (y/n): @@ -27,6 +32,8 @@ invalid_cloudflare_token = Invalid Cloudflare token: invalid_cloudflare_domain = Invalid Cloudflare domain: cloudflare_token_required = Cloudflare tunnel token is required when using Cloudflare tunnel. cloudflare_domain_required = Cloudflare custom domain is required when using Cloudflare tunnel. +invalid_tailscale_auth_key = Invalid Tailscale auth key: +tailscale_auth_key_required = Tailscale auth key is required when using Tailscale Funnel. invalid_run_mode = Invalid run mode: invalid_certificate = Invalid Certificate: invalid_rate_limit = Invalid rate limit: diff --git a/keepercommander/service/README.md b/keepercommander/service/README.md index 18b8e4e28..91b245480 100644 --- a/keepercommander/service/README.md +++ b/keepercommander/service/README.md @@ -46,7 +46,10 @@ You'll be prompted to configure: - Cloudflare tunneling (y/n) - *if ngrok is disabled* - Cloudflare tunnel token (required) - Cloudflare custom domain (required) -- Enable TLS Certificate (y/n) - *if both ngrok and cloudflare are disabled* +- Tailscale Funnel (y/n) - *if ngrok and cloudflare are disabled* + - Tailscale auth key (required) + - Tailscale ACL tags to advertise (optional, required only for OAuth-issued auth keys) +- Enable TLS Certificate (y/n) - *if ngrok, cloudflare, and tailscale are all disabled* - TLS Certificate path - TLS Certificate password - Enable Request Queue (y/n) @@ -80,6 +83,20 @@ Configure the service streamlined with Cloudflare: My Vault> service-create -p -f -c 'tree,record-add,audit-report' -cf -cfd -rm -q -aip -dip ``` +Configure the service streamlined with Tailscale: + +```bash + My Vault> service-create -p -f -c 'tree,record-add,audit-report' -ts -rm -q -aip -dip +``` + +If the auth key was generated by an OAuth client, also pass the ACL tag(s) it's scoped to: + +```bash + My Vault> service-create -p -f -c 'tree,record-add,audit-report' -ts -tst tag:commander-service -rm -q -aip -dip +``` + +**Note:** Commander always forces a fresh re-authentication (`tailscale up --force-reauth`) on every `service-create`/`service-start`, to guarantee the provided auth key is actually validated rather than silently reused from an existing session. A practical consequence: **a single-use auth key will only work for one successful start** — use a reusable key if you expect to restart the service more than once. + Parameters: - `-p, --port`: Port number for the service - `-c, --commands`: Comma-separated list of allowed commands @@ -87,6 +104,8 @@ Parameters: - `-cd, --ngrok_custom_domain`: Ngrok custom domain name - `-cf, --cloudflare`: Cloudflare tunnel token (required when using cloudflare) - `-cfd, --cloudflare_custom_domain`: Cloudflare custom domain name (required when using cloudflare) +- `-ts, --tailscale`: Tailscale auth key to authenticate and generate public URL via Funnel (required when using tailscale) +- `-tst, --tailscale_advertise_tags`: Comma-separated ACL tags to advertise (required only when the auth key is OAuth-client-issued, e.g. `tag:commander-service`) - `-f, --fileformat`: File format (json/yaml) - `-crtf, --certfile`: Certificate file path - `-crtp, --certpassword`: Certificate password @@ -320,6 +339,11 @@ The service configuration is stored as an attachment to a vault record in JSON/Y - Cloudflare tunnel token - Cloudflare custom domain - Generated public URL +- **Tailscale Configuration** (optional): + - Tailscale Funnel enabled/disabled + - Tailscale auth key + - Tailscale ACL tags to advertise + - Generated public URL - **TLS Certificate Configuration** (optional): - TLS certificate enabled/disabled - Certificate file path @@ -373,6 +397,13 @@ When Cloudflare tunneling is enabled, additional logs are maintained: - **Includes**: Tunnel establishment, connection timeout detection, and firewall blocking diagnostics - **Auto-created**: Created automatically when Cloudflare tunneling is configured and service starts +### Tailscale Logging +When Tailscale Funnel is enabled, additional logs are maintained: +- **Location**: `keepercommander/service/core/logs/tailscale_subprocess.log` +- **Content**: Tailscale CLI authentication attempts, Funnel enable/disable events, and status-check output +- **Includes**: `tailscale up`/`tailscale funnel` command output (the auth key value is never written to this log) +- **Auto-created**: Created automatically when Tailscale Funnel is configured and service starts + ### General Logging Configuration - **Configuration file**: `~/.keeper/logging_config.yaml` (auto-generated) - **Default level**: `INFO` @@ -502,6 +533,16 @@ This automates the complete setup for Slack App integration: The command generates a complete `docker-compose.yml` with both Commander service and Slack App service configured. +**Generated compose environment for the Slack service:** + +| Env var | Value | +|---------|-------| +| `KSM_CONFIG` | Base64 KSM config | +| `COMMANDER_RECORD` | Commander Docker config record UID | +| `SLACK_RECORD` | Slack config record UID | + +Image name used in compose: `keeper/slack-app:latest`. + ### Google Chat App Integration Setup For integrating Commander Service Mode with Google Chat, use the `gchat-app-setup` command: diff --git a/keepercommander/service/commands/create_service.py b/keepercommander/service/commands/create_service.py index 8ddc5538d..1131af1f4 100644 --- a/keepercommander/service/commands/create_service.py +++ b/keepercommander/service/commands/create_service.py @@ -29,6 +29,8 @@ class StreamlineArgs: ngrok_custom_domain: Optional[str] cloudflare: Optional[str] cloudflare_custom_domain: Optional[str] + tailscale: Optional[str] + tailscale_advertise_tags: Optional[str] certfile: Optional[str] certpassword: Optional[str] fileformat: Optional[str] @@ -72,6 +74,8 @@ def get_parser(self): parser.add_argument('-cd', '--ngrok_custom_domain', type=str, help='ngrok custom domain name(optional)') parser.add_argument('-cf', '--cloudflare', type=str, help='cloudflare tunnel token to generate public URL (required when using cloudflare)') parser.add_argument('-cfd', '--cloudflare_custom_domain', type=str, help='cloudflare custom domain name (required when using cloudflare)') + parser.add_argument('-ts', '--tailscale', type=str, help='Tailscale auth key to generate public URL via Funnel (required when using tailscale)') + parser.add_argument('-tst', '--tailscale_advertise_tags', dest='tailscale_advertise_tags', type=str, help='Comma-separated ACL tags to advertise (required when the auth key is OAuth-client-derived, e.g. tag:commander-service)') parser.add_argument('-crtf', '--certfile', type=str, help='certificate file path') parser.add_argument('-crtp', '--certpassword', type=str, help='certificate password') parser.add_argument('-f', '--fileformat', type=str, help='file format') @@ -95,7 +99,8 @@ def execute(self, params: KeeperParams, **kwargs) -> None: filtered_kwargs = {k: v for k, v in kwargs.items() if k in [ 'port', 'allowedip', 'deniedip', 'commands', 'ngrok', 'ngrok_custom_domain', - 'cloudflare', 'cloudflare_custom_domain', 'certfile', 'certpassword', 'fileformat', + 'cloudflare', 'cloudflare_custom_domain', 'tailscale', 'tailscale_advertise_tags', + 'certfile', 'certpassword', 'fileformat', 'run_mode', 'queue_enabled', 'update_vault_record', 'ratelimit', 'encryption', 'encryption_key', 'token_expiration', ]} @@ -118,12 +123,9 @@ def execute(self, params: KeeperParams, **kwargs) -> None: config_data = self.service_config.create_default_config() self._handle_configuration(config_data, params, args) - api_key = self._create_and_save_record(config_data, params, args, existing_api_key=existing_api_key) - - if args.update_vault_record and api_key: - actual_service_url = self._get_service_url(config_data) - write_service_metadata(params, args.update_vault_record, actual_service_url, api_key) + self._create_and_save_record(config_data, params, args, existing_api_key=existing_api_key) + # Vault metadata is written from start_service() instead, once the real URL is known. self._upload_and_start_service(params) except ValidationError as e: @@ -154,6 +156,13 @@ def _create_and_save_record(self, config_data: Dict[str, Any], params: KeeperPar existing_api_key=existing_api_key, ) config_data["records"] = [record] + + if args.update_vault_record: + api_key_value = record.get('api-key') + if api_key_value: + from ..core.globals import set_pending_vault_metadata + set_pending_vault_metadata(args.update_vault_record, api_key_value) + if config_data.get("fileformat"): format_type = config_data["fileformat"] else: @@ -172,21 +181,6 @@ def _upload_and_start_service(self, params: KeeperParams) -> None: ServiceManager.start_service() def _get_service_url(self, config_data: Dict[str, Any]) -> str: - """Determine the actual service URL (ngrok, cloudflare, or localhost) with API version path""" - # Determine API version based on queue_enabled - queue_enabled = config_data.get("queue_enabled", "y") - api_path = "/api/v2" if queue_enabled == "y" else "/api/v1" - - # Priority: ngrok > cloudflare > localhost - base_url = "" - if config_data.get("ngrok_public_url"): - base_url = config_data["ngrok_public_url"] - elif config_data.get("cloudflare_public_url"): - base_url = config_data["cloudflare_public_url"] - else: - # Fallback to localhost with correct protocol - port = config_data.get("port", 8080) - protocol = "https" if config_data.get("tls_certificate") == "y" else "http" - base_url = f"{protocol}://localhost:{port}" - - return f"{base_url}{api_path}" + """Determine the actual service URL (ngrok, cloudflare, tailscale, or localhost) with API version path""" + from .integrations.vault_metadata import get_service_url + return get_service_url(config_data) diff --git a/keepercommander/service/commands/integrations/runtime_policy.py b/keepercommander/service/commands/integrations/runtime_policy.py index 109e2213b..e773c4828 100644 --- a/keepercommander/service/commands/integrations/runtime_policy.py +++ b/keepercommander/service/commands/integrations/runtime_policy.py @@ -50,19 +50,21 @@ def _integration_sanitizers(): from .sailpoint_app_setup import SailPointAppSetupCommand from .slack_app_setup import SlackAppSetupCommand from .teams_app_setup import TeamsAppSetupCommand - from ...decorators.min_commander_version import TERRAFORM_DOCKER_ENV + from ...decorators.min_commander_version import TERRAFORM_DOCKER_ENV, TERRAFORM_DOCKER_ENV_LEGACY slack = SlackAppSetupCommand() teams = TeamsAppSetupCommand() gchat = GChatAppSetupCommand() sailpoint = SailPointAppSetupCommand() + terraform_sanitizer = lambda commands: sanitize_commands( + commands, TerraformSetupConstants.SERVICE_COMMANDS_LIST + ) return { slack.get_record_env_key(): slack.sanitize_service_commands, teams.get_record_env_key(): teams.sanitize_service_commands, gchat.get_record_env_key(): gchat.sanitize_service_commands, sailpoint.get_record_env_key(): sailpoint.sanitize_service_commands, - TERRAFORM_DOCKER_ENV: lambda commands: sanitize_commands( - commands, TerraformSetupConstants.SERVICE_COMMANDS_LIST - ), + TERRAFORM_DOCKER_ENV: terraform_sanitizer, + TERRAFORM_DOCKER_ENV_LEGACY: terraform_sanitizer, } diff --git a/keepercommander/service/commands/integrations/vault_metadata.py b/keepercommander/service/commands/integrations/vault_metadata.py index 1a1753703..736bdcc23 100644 --- a/keepercommander/service/commands/integrations/vault_metadata.py +++ b/keepercommander/service/commands/integrations/vault_metadata.py @@ -9,7 +9,7 @@ # Contact: commander@keepersecurity.com # -from typing import Optional +from typing import Any, Dict, Optional from ...decorators.logging import logger from ....params import KeeperParams @@ -21,6 +21,29 @@ _STALE_REVISION_HINTS = ('out_of_sync', 'no longer exists') +def get_service_url(config_data: Dict[str, Any]) -> str: + """Determine the actual service URL (ngrok, cloudflare, tailscale, or localhost) with API version path""" + # Determine API version based on queue_enabled + queue_enabled = config_data.get("queue_enabled", "y") + api_path = "/api/v2" if queue_enabled == "y" else "/api/v1" + + # Priority: ngrok > cloudflare > tailscale > localhost + base_url = "" + if config_data.get("ngrok_public_url"): + base_url = config_data["ngrok_public_url"] + elif config_data.get("cloudflare_public_url"): + base_url = config_data["cloudflare_public_url"] + elif config_data.get("tailscale_public_url"): + base_url = config_data["tailscale_public_url"] + else: + # Fallback to localhost with correct protocol + port = config_data.get("port", 8080) + protocol = "https" if config_data.get("tls_certificate") == "y" else "http" + base_url = f"{protocol}://localhost:{port}" + + return f"{base_url}{api_path}" + + def get_existing_api_key(params: KeeperParams, record_uid: str) -> Optional[str]: try: from .... import vault diff --git a/keepercommander/service/commands/service_config_handlers.py b/keepercommander/service/commands/service_config_handlers.py index c63a01072..3df4c320b 100644 --- a/keepercommander/service/commands/service_config_handlers.py +++ b/keepercommander/service/commands/service_config_handlers.py @@ -63,14 +63,18 @@ def handle_streamlined_config(self, config_data: Dict[str, Any], args, params: K # Apply logical tunneling flow for streamlined config ngrok_enabled = "y" if args.ngrok else "n" cloudflare_enabled = "y" if args.cloudflare else "n" - + tailscale_enabled = "y" if args.tailscale else "n" + # Implement the same logic as interactive mode ngrok_public_url = "" cloudflare_public_url = "" - + tailscale_auth_key = "" + tailscale_advertise_tags = "" + if ngrok_enabled == "y": - # ngrok enabled → disable cloudflare and TLS + # ngrok enabled → disable cloudflare, tailscale and TLS cloudflare_enabled = "n" + tailscale_enabled = "n" cloudflare_token = "" cloudflare_domain = "" tls_enabled = "n" @@ -84,14 +88,15 @@ def handle_streamlined_config(self, config_data: Dict[str, Any], args, params: K ngrok_public_url = f"https://{ngrok_domain}.ngrok.io" else: ngrok_public_url = f"https://{ngrok_domain}" - logger.debug("Ngrok enabled - disabling cloudflare and TLS") + logger.debug("Ngrok enabled - disabling cloudflare, tailscale and TLS") elif cloudflare_enabled == "y": - # cloudflare enabled → disable TLS, but validate required fields + # cloudflare enabled → disable tailscale and TLS, but validate required fields if not args.cloudflare: raise ValidationError("Cloudflare tunnel token is required when using Cloudflare tunnel.") if not args.cloudflare_custom_domain: raise ValidationError("Cloudflare custom domain is required when using Cloudflare tunnel.") - + + tailscale_enabled = "n" tls_enabled = "n" certfile = "" certpassword = "" @@ -99,9 +104,20 @@ def handle_streamlined_config(self, config_data: Dict[str, Any], args, params: K cloudflare_domain = self.service_config.validator.validate_domain(args.cloudflare_custom_domain) # Construct cloudflare public URL from custom domain cloudflare_public_url = f"https://{cloudflare_domain}" - logger.debug("Cloudflare enabled - disabling TLS") + logger.debug("Cloudflare enabled - disabling tailscale and TLS") + elif tailscale_enabled == "y": + # tailscale enabled → disable TLS + tls_enabled = "n" + certfile = "" + certpassword = "" + cloudflare_token = "" + cloudflare_domain = "" + tailscale_auth_key = self.service_config.validator.validate_tailscale_auth_key(args.tailscale) + tailscale_advertise_tags = args.tailscale_advertise_tags or "" + # URL is only known once Funnel actually starts at service-start time. + logger.debug("Tailscale enabled - disabling TLS") else: - # Both ngrok and cloudflare disabled → allow TLS + # ngrok, cloudflare, and tailscale all disabled → allow TLS tls_enabled = "y" if args.certfile and args.certpassword else "n" certfile = args.certfile if args.certfile else "" certpassword = args.certpassword if args.certpassword else "" @@ -139,6 +155,10 @@ def handle_streamlined_config(self, config_data: Dict[str, Any], args, params: K "cloudflare_tunnel_token": cloudflare_token, "cloudflare_custom_domain": cloudflare_domain, "cloudflare_public_url": cloudflare_public_url, + "tailscale": tailscale_enabled, + "tailscale_auth_key": tailscale_auth_key, + "tailscale_advertise_tags": tailscale_advertise_tags, + "tailscale_public_url": "", "tls_certificate": tls_enabled, "certfile": certfile, "certpassword": certpassword, @@ -171,34 +191,52 @@ def _configure_port(self, config_data: Dict[str, Any]) -> None: def _configure_tunneling_and_tls(self, config_data: Dict[str, Any]) -> None: """ Configure tunneling and TLS with logical flow: - 1. If ngrok = yes → Skip cloudflare and TLS (ngrok provides public access with SSL) + 1. If ngrok = yes → Skip cloudflare, tailscale and TLS (ngrok provides public access with SSL) 2. If ngrok = no → Ask for cloudflare - 3. If ngrok = no AND cloudflare = no → Ask for TLS (local HTTPS) + 3. If ngrok = no AND cloudflare = no → Ask for tailscale + 4. If ngrok = no AND cloudflare = no AND tailscale = no → Ask for TLS (local HTTPS) """ # First, always ask for ngrok self._configure_ngrok(config_data) - + if config_data["ngrok"] == "y": - # ngrok provides public access with SSL, so skip cloudflare and TLS + # ngrok provides public access with SSL, so skip cloudflare, tailscale and TLS config_data["cloudflare"] = "n" config_data["cloudflare_tunnel_token"] = "" config_data["cloudflare_custom_domain"] = "" config_data["cloudflare_public_url"] = "" + config_data["tailscale"] = "n" + config_data["tailscale_auth_key"] = "" + config_data["tailscale_advertise_tags"] = "" + config_data["tailscale_public_url"] = "" config_data["tls_certificate"] = "n" config_data["certfile"] = "" config_data["certpassword"] = "" else: # ngrok = no, so ask for cloudflare self._configure_cloudflare(config_data) - + if config_data["cloudflare"] == "y": - # cloudflare provides public access with SSL, so skip TLS + # cloudflare provides public access with SSL, so skip tailscale and TLS + config_data["tailscale"] = "n" + config_data["tailscale_auth_key"] = "" + config_data["tailscale_advertise_tags"] = "" + config_data["tailscale_public_url"] = "" config_data["tls_certificate"] = "n" config_data["certfile"] = "" config_data["certpassword"] = "" else: - # Both ngrok and cloudflare = no, so ask for TLS for local HTTPS - self._configure_tls(config_data) + # ngrok and cloudflare = no, so ask for tailscale + self._configure_tailscale(config_data) + + if config_data["tailscale"] == "y": + # tailscale provides public access with SSL, so skip TLS + config_data["tls_certificate"] = "n" + config_data["certfile"] = "" + config_data["certpassword"] = "" + else: + # ngrok, cloudflare and tailscale = no, so ask for TLS for local HTTPS + self._configure_tls(config_data) def _configure_ngrok(self, config_data: Dict[str, Any]) -> None: config_data["ngrok"] = self.service_config._get_yes_no_input(self.messages['ngrok_prompt']) @@ -255,6 +293,31 @@ def _configure_cloudflare(self, config_data: Dict[str, Any]) -> None: config_data["cloudflare_custom_domain"] = "" config_data["cloudflare_public_url"] = "" + def _configure_tailscale(self, config_data: Dict[str, Any]) -> None: + config_data["tailscale"] = self.service_config._get_yes_no_input( + self.messages.get('tailscale_prompt', 'Do you want to use Tailscale Funnel? (y/n): ') + ) + + if config_data["tailscale"] == "y": + config_data["tailscale_auth_key"] = self._get_validated_input( + prompt_key='tailscale_auth_key_prompt', + validation_func=self.service_config.validator.validate_tailscale_auth_key, + error_key='invalid_tailscale_auth_key', + required=True + ) + # Only required for OAuth-derived auth keys. + config_data["tailscale_advertise_tags"] = input( + self.messages.get( + 'tailscale_advertise_tags_prompt', + 'Enter Tailscale ACL tags to advertise, comma-separated (optional, required for OAuth-derived auth keys): ' + ) + ).strip() + config_data["tailscale_public_url"] = "" # known only once Funnel starts + else: + config_data["tailscale_auth_key"] = "" + config_data["tailscale_advertise_tags"] = "" + config_data["tailscale_public_url"] = "" + def _configure_tls(self, config_data: Dict[str, Any]) -> None: config_data["tls_certificate"] = self.service_config._get_yes_no_input(self.messages['tls_certificate']) diff --git a/keepercommander/service/commands/terraform_app_setup.py b/keepercommander/service/commands/terraform_app_setup.py index 85d4b6be6..811f3e5da 100644 --- a/keepercommander/service/commands/terraform_app_setup.py +++ b/keepercommander/service/commands/terraform_app_setup.py @@ -136,7 +136,7 @@ def generate_docker_compose_yaml(self, setup_result: SetupResult, config: Docker asdict(config), commander_service_name=TerraformSetupConstants.COMMANDER_SERVICE_NAME, commander_container_name=TerraformSetupConstants.COMMANDER_CONTAINER_NAME, - commander_environment={TERRAFORM_DOCKER_ENV: '1'}, + commander_environment={TERRAFORM_DOCKER_ENV: setup_result.record_uid}, ) return builder.build() diff --git a/keepercommander/service/config/config_validation.py b/keepercommander/service/config/config_validation.py index c4cf07c6c..c557ea08b 100644 --- a/keepercommander/service/config/config_validation.py +++ b/keepercommander/service/config/config_validation.py @@ -122,6 +122,18 @@ def validate_cloudflare_token(token: str) -> str: logger.debug("Cloudflare token validation successful") return token + @staticmethod + def validate_tailscale_auth_key(auth_key: str) -> str: + """Check presence only; Tailscale's servers are authoritative on key validity at `tailscale up` time.""" + logger.debug("Validating Tailscale auth key") + + if not auth_key or not auth_key.strip(): + msg = "Tailscale auth key cannot be empty" + raise ValidationError(msg) + + logger.debug("Tailscale auth key validation successful") + return auth_key + @staticmethod def validate_domain(domain: str, require_tld: bool = True) -> str: """ diff --git a/keepercommander/service/config/models.py b/keepercommander/service/config/models.py index 59aa43d65..e87e6b738 100644 --- a/keepercommander/service/config/models.py +++ b/keepercommander/service/config/models.py @@ -38,3 +38,7 @@ class ServiceConfigData: cloudflare_tunnel_token: str = "" cloudflare_custom_domain: str = "" cloudflare_public_url: str = "" + tailscale: str = "n" + tailscale_auth_key: str = "" + tailscale_advertise_tags: str = "" + tailscale_public_url: str = "" diff --git a/keepercommander/service/config/service_config.py b/keepercommander/service/config/service_config.py index 291c7bc44..6f6ff8137 100644 --- a/keepercommander/service/config/service_config.py +++ b/keepercommander/service/config/service_config.py @@ -93,6 +93,10 @@ def create_default_config(self) -> Dict[str, Any]: cloudflare_tunnel_token="", cloudflare_custom_domain="", cloudflare_public_url="", + tailscale="n", + tailscale_auth_key="", + tailscale_advertise_tags="", + tailscale_public_url="", tls_certificate="n", certfile="", certpassword="", @@ -234,7 +238,24 @@ def load_config(self) -> Dict[str, Any]: if 'cloudflare_public_url' not in config: config['cloudflare_public_url'] = '' logger.debug("Added default cloudflare_public_url for backwards compatibility") - + + # Add backwards compatibility for missing Tailscale fields + if 'tailscale' not in config: + config['tailscale'] = 'n' # Default to disabled for existing configs + logger.debug("Added default tailscale=n for backwards compatibility") + + if 'tailscale_auth_key' not in config: + config['tailscale_auth_key'] = '' + logger.debug("Added default tailscale_auth_key for backwards compatibility") + + if 'tailscale_advertise_tags' not in config: + config['tailscale_advertise_tags'] = '' + logger.debug("Added default tailscale_advertise_tags for backwards compatibility") + + if 'tailscale_public_url' not in config: + config['tailscale_public_url'] = '' + logger.debug("Added default tailscale_public_url for backwards compatibility") + self._validate_config_structure(config) return config @@ -258,6 +279,10 @@ def _validate_config_structure(self, config: Dict[str, Any]) -> None: self.validator.validate_cloudflare_token(config_data.cloudflare_tunnel_token) self.validator.validate_domain(config_data.cloudflare_custom_domain) + if config_data.tailscale == 'y': + logger.debug("Validating tailscale configuration") + self.validator.validate_tailscale_auth_key(config_data.tailscale_auth_key) + if config_data.is_advanced_security_enabled == 'y': logger.debug("Validating advanced security settings") self.validator.validate_rate_limit(config_data.rate_limiting) diff --git a/keepercommander/service/config/tailscale_config.py b/keepercommander/service/config/tailscale_config.py new file mode 100644 index 000000000..8724a7844 --- /dev/null +++ b/keepercommander/service/config/tailscale_config.py @@ -0,0 +1,152 @@ +# _ __ +# | |/ /___ ___ _ __ ___ _ _ ® +# | ' None: + """Validate Tailscale configuration parameters.""" + required_keys = ["port", "tailscale_auth_key", "run_mode"] + + for key in required_keys: + if key not in config_data: + raise ValidationError(f"Missing required configuration key: {key}") + + service_config.validator.validate_port(config_data["port"]) + service_config.validator.validate_tailscale_auth_key(config_data["tailscale_auth_key"]) + + if config_data["run_mode"] not in ["foreground", "background"]: + raise ValidationError(f"Invalid run_mode: {config_data['run_mode']}") + + logger.debug("Tailscale configuration validation successful") + + @staticmethod + def _ensure_ready(service_config: ServiceConfig, check_fn, guidance_fn, action_fn, + prompt_key: str, prompt_default: str, action_label: str, failure_label: str) -> None: + """ + Generic check -> guidance -> prompt -> attempt -> reverify flow, shared + by the CLI-install and daemon-start checks below. Raises ValidationError + if the user declines or the automatic attempt doesn't fix the check. + """ + if check_fn(): + return + + guidance = guidance_fn() + logger.error(guidance) + print(guidance) + + choice = service_config._get_yes_no_input(service_config.messages.get(prompt_key, prompt_default)) + if choice != 'y': + raise ValidationError(guidance) + + print(f'Attempting to {action_label} automatically...') + action_fn() + if not check_fn(): + raise ValidationError(f"{failure_label}. {guidance}") + logger.debug(f"{action_label.capitalize()} succeeded") + + @staticmethod + def _verify_funnel_active(local_port: int, max_retries: int = 3, retry_delay: float = 1) -> bool: + """`--bg` can exit 0 without the target ever going active; confirm via tailscaled's own status.""" + import time + for attempt in range(max_retries): + if get_tailscale_funnel_status(local_port): + return True + if attempt < max_retries - 1: + time.sleep(retry_delay) + return False + + @staticmethod + @debug_decorator + def configure_tailscale(config_data: Dict[str, Any], service_config: ServiceConfig) -> Optional[int]: + """ + Configure Tailscale Funnel if enabled. Always returns None -- unlike + Ngrok/Cloudflare, Tailscale has no Commander-owned subprocess/PID to + track; lifecycle state lives in ProcessInfo.tailscale_enabled/tailscale_port. + """ + if config_data.get("tailscale") != 'y': + return None + + logger.debug("Configuring Tailscale Funnel") + reset_tailscale_log() + + try: + logger.debug("Checking Tailscale CLI availability") + TailscaleConfigurator._ensure_ready( + service_config, is_tailscale_installed, get_tailscale_install_guidance, install_tailscale, + 'tailscale_install_prompt', 'Tailscale CLI is not installed. Attempt automatic installation now? (y/n): ', + 'install Tailscale', 'Automatic Tailscale installation did not succeed' + ) + + logger.debug("Checking Tailscale daemon status") + TailscaleConfigurator._ensure_ready( + service_config, is_tailscale_daemon_running, get_tailscale_daemon_start_guidance, start_tailscale_daemon, + 'tailscale_daemon_start_prompt', 'Tailscale daemon is not running. Attempt to start it now? (y/n): ', + 'start the Tailscale daemon', 'Could not start the Tailscale daemon automatically' + ) + + TailscaleConfigurator._validate_tailscale_config(config_data, service_config) + + # Auth key used only for `tailscale up`; never logged, never used for API auth. + logger.debug("Authenticating with Tailscale") + tailscale_up(config_data["tailscale_auth_key"], config_data.get("tailscale_advertise_tags")) + + logger.debug(f"Starting Tailscale Funnel for port {config_data['port']}") + start_tailscale_funnel(config_data["port"]) + + if not TailscaleConfigurator._verify_funnel_active(config_data["port"]): + try: + stop_tailscale_funnel(config_data["port"]) + except Exception as cleanup_error: + logger.debug(f"Funnel rollback failed: {cleanup_error}") + raise Exception( + "Tailscale Funnel did not become active after starting. First-time Funnel use " + "on this tailnet may be pending admin-console approval -- run `tailscale funnel status`." + ) + + public_url = get_tailscale_funnel_url(config_data["port"]) + config_data["tailscale_public_url"] = public_url or "" + + if public_url: + logger.info(f"Tailscale Funnel URL: {public_url}") + print(f'Generated Tailscale Funnel URL: {public_url}') + else: + logger.warning("Tailscale Funnel started but URL could not be retrieved") + print('Tailscale Funnel started, URL will be available via `tailscale funnel status`') + + return None + + except ValidationError as e: + logger.error(f"Invalid Tailscale configuration: {e}") + raise + except Exception as e: + logger.error(f"Failed to configure Tailscale Funnel: {e}") + raise diff --git a/keepercommander/service/core/globals.py b/keepercommander/service/core/globals.py index 449bf2eea..ad7eec898 100644 --- a/keepercommander/service/core/globals.py +++ b/keepercommander/service/core/globals.py @@ -9,11 +9,12 @@ # Contact: ops@keepersecurity.com # -from typing import Optional +from typing import Dict, Optional from ...params import KeeperParams from ... import utils _current_params: Optional[KeeperParams] = None +_pending_vault_metadata: Optional[Dict[str, str]] = None def init_globals(params: KeeperParams) -> None: global _current_params @@ -22,6 +23,25 @@ def init_globals(params: KeeperParams) -> None: def get_current_params() -> Optional[KeeperParams]: return _current_params +def set_pending_vault_metadata(record_uid: str, api_key: str) -> None: + """ + Stash a Docker-config record UID + API key for a one-time vault metadata + write, to be consumed by ServiceManager.start_service() once the real + service URL is known. Transient (in-memory, same-process only) -- never + persisted to the saved service config, since it only needs to survive + from the current service-create invocation through to the immediately + following start_service() call, not across restarts. + """ + global _pending_vault_metadata + _pending_vault_metadata = {'record_uid': record_uid, 'api_key': api_key} + +def pop_pending_vault_metadata() -> Optional[Dict[str, str]]: + """Return and clear the pending vault metadata update, if any.""" + global _pending_vault_metadata + value = _pending_vault_metadata + _pending_vault_metadata = None + return value + def ensure_params_loaded() -> KeeperParams: """Load params from config if not already loaded.""" params = get_current_params() diff --git a/keepercommander/service/core/logs/tailscale_subprocess.log b/keepercommander/service/core/logs/tailscale_subprocess.log new file mode 100644 index 000000000..b91d84bee --- /dev/null +++ b/keepercommander/service/core/logs/tailscale_subprocess.log @@ -0,0 +1,21 @@ +/usr/local/bin/tailscale: line 2: /Applications/Tailscale.app/Contents/MacOS/Tailscale: No such file or directory +failed to connect to local Tailscale service; is Tailscale running? +failed to connect to local Tailscale service; is Tailscale running? +failed to connect to local Tailscale service; is Tailscale running? +failed to connect to local Tailscale service; is Tailscale running? +Error: the CLI for serve and funnel has changed. +Please see https://tailscale.com/kb/1242/tailscale-serve for more information. +try `tailscale funnel --help` for usage info +Error: the CLI for serve and funnel has changed. +Please see https://tailscale.com/kb/1242/tailscale-serve for more information. +try `tailscale funnel --help` for usage info +Error: the CLI for serve and funnel has changed. +Please see https://tailscale.com/kb/1242/tailscale-serve for more information. +try `tailscale funnel --help` for usage info +Error: the CLI for serve and funnel has changed. +Please see https://tailscale.com/kb/1242/tailscale-serve for more information. +try `tailscale funnel --help` for usage info +failed to connect to local Tailscale service; is Tailscale running? +backend error: invalid key: unable to validate API key +backend error: invalid key: unable to validate API key +backend error: invalid key: unable to validate API key diff --git a/keepercommander/service/core/process_info.py b/keepercommander/service/core/process_info.py index 3c495a48f..655b8ff5a 100644 --- a/keepercommander/service/core/process_info.py +++ b/keepercommander/service/core/process_info.py @@ -24,7 +24,9 @@ class ProcessInfo: is_running: bool ngrok_pid: Optional[int] = None cloudflare_pid: Optional[int] = None - + tailscale_enabled: bool = False + tailscale_port: Optional[int] = None + _env_file = utils.get_default_path() / ".service.env" @classmethod @@ -32,27 +34,34 @@ def _str_to_bool(cls, value: str) -> bool: return value.lower() in ('true', '1', 'yes', 'on') @classmethod - def save(cls, pid, is_running: bool, ngrok_pid: Optional[int] = None, cloudflare_pid: Optional[int] = None) -> None: + def save(cls, pid, is_running: bool, ngrok_pid: Optional[int] = None, cloudflare_pid: Optional[int] = None, + tailscale_enabled: bool = False, tailscale_port: Optional[int] = None) -> None: """Save current process information to .env file.""" - + env_path = str(cls._env_file) - + # Create the file if it doesn't exist if not cls._env_file.exists(): cls._env_file.touch() - + process_info = { 'KEEPER_SERVICE_PID': str(pid), 'KEEPER_SERVICE_TERMINAL': TerminalHandler.get_terminal_info() or '', 'KEEPER_SERVICE_IS_RUNNING': str(is_running).lower() } - + if ngrok_pid is not None: process_info['KEEPER_SERVICE_NGROK_PID'] = str(ngrok_pid) - + if cloudflare_pid is not None: process_info['KEEPER_SERVICE_CLOUDFLARE_PID'] = str(cloudflare_pid) - + + if tailscale_enabled: + process_info['KEEPER_SERVICE_TAILSCALE_ENABLED'] = str(tailscale_enabled).lower() + + if tailscale_port is not None: + process_info['KEEPER_SERVICE_TAILSCALE_PORT'] = str(tailscale_port) + try: for key, value in process_info.items(): set_key(env_path, key, value, quote_mode='never') @@ -84,20 +93,29 @@ def load(cls) -> 'ProcessInfo': cloudflare_pid_str = os.getenv('KEEPER_SERVICE_CLOUDFLARE_PID') cloudflare_pid = int(cloudflare_pid_str) if cloudflare_pid_str else None - + + tailscale_enabled_str = os.getenv('KEEPER_SERVICE_TAILSCALE_ENABLED', 'false') + tailscale_enabled = ProcessInfo._str_to_bool(tailscale_enabled_str) + + tailscale_port_str = os.getenv('KEEPER_SERVICE_TAILSCALE_PORT') + tailscale_port = int(tailscale_port_str) if tailscale_port_str else None + logger.debug("Process information loaded successfully from .env") return ProcessInfo( pid=pid, terminal=terminal, is_running=is_running, ngrok_pid=ngrok_pid, - cloudflare_pid=cloudflare_pid + cloudflare_pid=cloudflare_pid, + tailscale_enabled=tailscale_enabled, + tailscale_port=tailscale_port ) except Exception as e: logger.error(f"Failed to load process information: {e}") pass - - return ProcessInfo(pid=None, terminal=None, is_running=False, ngrok_pid=None, cloudflare_pid=None) + + return ProcessInfo(pid=None, terminal=None, is_running=False, ngrok_pid=None, cloudflare_pid=None, + tailscale_enabled=False, tailscale_port=None) @classmethod def clear(cls) -> None: diff --git a/keepercommander/service/core/service_manager.py b/keepercommander/service/core/service_manager.py index d65d0c449..9b215e8ff 100644 --- a/keepercommander/service/core/service_manager.py +++ b/keepercommander/service/core/service_manager.py @@ -67,6 +67,11 @@ def start_service(cls) -> None: SignalHandler.setup_signal_handlers(cls._handle_shutdown) + # Initialized before any operation that could raise, so the outer except + # below can always safely check them to roll back a partially-started Funnel. + tailscale_enabled = False + tailscale_port = None + try: service_config = ServiceConfig() config_data = service_config.load_config() @@ -77,6 +82,7 @@ def start_service(cls) -> None: from ..config.ngrok_config import NgrokConfigurator from ..config.cloudflare_config import CloudflareConfigurator + from ..config.tailscale_config import TailscaleConfigurator is_running = True queue_enabled = config_data.get("queue_enabled", "y") @@ -119,6 +125,72 @@ def start_service(cls) -> None: logger.error(f"\n{str(e)}") return + try: + TailscaleConfigurator.configure_tailscale(config_data, service_config) + if config_data.get("tailscale") == 'y': + tailscale_enabled = True + tailscale_port = port + # Tailscale's URL is only known post-Funnel-start; persist it now. + if config_data.get("tailscale_public_url"): + try: + # save_config() writes plaintext; must re-encrypt or later + # load_config() calls (auth checks, routes) fail to decrypt. + service_config.save_config(config_data, config_data.get("fileformat")) + service_config.format_handler.encrypt_config_file( + service_config.format_handler.config_path, service_config.format_handler.config_dir + ) + except Exception as save_error: + logger.debug(f"Could not persist tailscale_public_url: {save_error}") + except (KeyboardInterrupt, Exception) as e: + # KeyboardInterrupt (e.g. Ctrl+C during a Tailscale install/daemon-start + # prompt) is not an Exception subclass -- must be caught explicitly here + # too, or this rollback (and the ones below) never runs on interrupt. + if ngrok_pid and psutil: + try: + process = psutil.Process(ngrok_pid) + process.terminate() + logger.debug(f"Terminated ngrok process {ngrok_pid}") + except (psutil.NoSuchProcess, psutil.AccessDenied, OSError) as ngrok_error: + logger.debug(f"Error terminating ngrok process: {type(ngrok_error).__name__}") + elif ngrok_pid: + logger.warning("Cannot terminate ngrok process: psutil not available") + + if cloudflare_pid and psutil: + try: + process = psutil.Process(cloudflare_pid) + process.terminate() + logger.debug(f"Terminated cloudflare process {cloudflare_pid}") + except (psutil.NoSuchProcess, psutil.AccessDenied, OSError) as cf_error: + logger.debug(f"Error terminating cloudflare process: {type(cf_error).__name__}") + elif cloudflare_pid: + logger.warning("Cannot terminate cloudflare process: psutil not available") + + ProcessInfo.clear() + + if isinstance(e, KeyboardInterrupt): + logger.info("Service startup interrupted by user") + raise + + logger.info(f"\n{str(e)}") + return + + # Write vault metadata (URL + API key) now the real URL is known. Consumed + # from a transient, same-process global (set by CreateService for + # -ur/--update-vault-record) so it fires once per creation, not on restarts. + from ..core.globals import pop_pending_vault_metadata + pending_metadata = pop_pending_vault_metadata() + if pending_metadata: + try: + from ..core.globals import ensure_params_loaded + from ..commands.integrations.vault_metadata import write_service_metadata, get_service_url + metadata_params = ensure_params_loaded() + actual_service_url = get_service_url(config_data) + write_service_metadata( + metadata_params, pending_metadata['record_uid'], actual_service_url, pending_metadata['api_key'] + ) + except Exception as metadata_error: + logger.error(f"Failed to write vault metadata: {metadata_error}") + # Custom logging filter to replace SSL handshake errors with user-friendly message class SSLHandshakeFilter(logging.Filter): def filter(self, record): @@ -170,7 +242,7 @@ def filter(self, record): logger.debug(f"Service subprocess logs available at: {log_file}") print(f"Commander Service started with PID: {process.pid}") - ProcessInfo.save(process.pid, is_running, ngrok_pid, cloudflare_pid) + ProcessInfo.save(process.pid, is_running, ngrok_pid, cloudflare_pid, tailscale_enabled=tailscale_enabled, tailscale_port=tailscale_port) except Exception as e: logger.error(f"Failed to start service subprocess: {e}") @@ -178,6 +250,7 @@ def filter(self, record): else: cleanup_done = False + tailscale_cleanup_done = False def cleanup_cloudflare_on_foreground_exit(): """Clean up Cloudflare tunnel when foreground service exits.""" @@ -247,9 +320,25 @@ def cleanup_cloudflare_on_foreground_exit(): print(f"Unexpected error during Cloudflare cleanup: {e}") logger.error(f"Unexpected error during Cloudflare cleanup: {e}") + def cleanup_tailscale_on_foreground_exit(): + """Stop Funnel when foreground service exits. Leaves tailnet auth/daemon untouched.""" + nonlocal tailscale_cleanup_done + if not tailscale_enabled or tailscale_cleanup_done: + return + tailscale_cleanup_done = True + try: + from ..util.tunneling import stop_tailscale_funnel + if stop_tailscale_funnel(tailscale_port): + print("Tailscale Funnel stopped") + except (KeyboardInterrupt, SystemExit): + raise + except Exception as e: + logger.debug(f"Tailscale funnel cleanup failed: {e}") + def foreground_signal_handler(signum, frame): """Handle interrupt signals in foreground mode.""" cleanup_cloudflare_on_foreground_exit() + cleanup_tailscale_on_foreground_exit() sys.exit(0) # Set up signal handlers for foreground mode @@ -261,9 +350,9 @@ def foreground_signal_handler(signum, frame): cls._flask_app = create_app() cls._is_running = True - ProcessInfo.save(os.getpid(), is_running, ngrok_pid, cloudflare_pid) + ProcessInfo.save(os.getpid(), is_running, ngrok_pid, cloudflare_pid, tailscale_enabled=tailscale_enabled, tailscale_port=tailscale_port) ssl_context = ServiceManager.get_ssl_context(config_data) - + try: cls._flask_app.run( host='0.0.0.0', @@ -272,13 +361,25 @@ def foreground_signal_handler(signum, frame): ) finally: cleanup_cloudflare_on_foreground_exit() - + cleanup_tailscale_on_foreground_exit() + except FileNotFoundError: logging.info("Error: Service configuration file not found. Please use 'service-create' command to create a service_config file.") return except Exception as e: logger.error(f"Error: Failed to start Commander Service") logger.error(f"Reason: {e}") + # Tailscale Funnel may already be live at this point (configured earlier + # in this same call) even though the service subprocess/Flask app itself + # failed to start -- stop it so a failed startup doesn't leave a public + # endpoint pointing at a service that never actually came up. + if tailscale_enabled and tailscale_port: + try: + from ..util.tunneling import stop_tailscale_funnel + stop_tailscale_funnel(tailscale_port) + logger.debug("Stopped Tailscale Funnel after service startup failure") + except Exception as cleanup_error: + logger.debug(f"Failed to stop Tailscale Funnel during startup-failure rollback: {cleanup_error}") cls._handle_shutdown() @classmethod @@ -389,6 +490,21 @@ def stop_service(cls) -> None: if not cloudflare_stopped: logger.debug("No Cloudflare tunnel processes found to stop") + # Stop Tailscale Funnel if it was enabled. Leaves tailnet auth/daemon untouched + # (tailscaled is system-wide; stopping it would affect other uses of this machine's Tailscale connection). + if process_info.tailscale_enabled and process_info.tailscale_port: + try: + logger.debug(f"Attempting to stop Tailscale Funnel on port {process_info.tailscale_port}") + from ..util.tunneling import stop_tailscale_funnel + if stop_tailscale_funnel(process_info.tailscale_port): + print("Tailscale Funnel stopped") + else: + logger.warning(f"Failed to stop Tailscale Funnel on port {process_info.tailscale_port}") + except Exception as e: + logger.warning(f"Error stopping Tailscale: {str(e)}") + else: + logger.debug("No Tailscale Funnel to stop") + # Stop the main service process if ServiceManager.kill_process_by_pid(process_info.pid): logger.debug(f"Commander Service stopped (PID: {process_info.pid})") @@ -441,9 +557,37 @@ def get_status() -> str: except psutil.NoSuchProcess: status += f"\nCloudflare tunnel is Stopped (was PID: {process_info.cloudflare_pid})" + # Check Tailscale Funnel status if enabled + if process_info.tailscale_enabled and process_info.tailscale_port: + try: + from ..util.tunneling import get_tailscale_funnel_status, get_tailscale_funnel_url + funnel_on = get_tailscale_funnel_status(process_info.tailscale_port) + if funnel_on: + current_url = get_tailscale_funnel_url(process_info.tailscale_port, max_retries=1, retry_delay=0.5) + if current_url: + status += f"\nTailscale Funnel is Running (Port: {process_info.tailscale_port}, URL: {current_url})" + else: + status += f"\nTailscale Funnel is Running (Port: {process_info.tailscale_port})" + else: + status += f"\nTailscale Funnel is Stopped (was Port: {process_info.tailscale_port})" + except Exception as e: + logger.debug(f"Error checking Tailscale funnel status: {e}") + status += f"\nTailscale Funnel status could not be determined (Port: {process_info.tailscale_port})" + logger.debug(f"Service status check: {status}") return status except psutil.NoSuchProcess: + # Funnel is managed by tailscaled, not tied to the Commander process -- + # an unexpected crash/SIGKILL of the service can leave it publicly + # exposed with nothing behind it. Reconcile it here rather than only + # on an explicit service-stop. + if process_info.tailscale_enabled and process_info.tailscale_port: + try: + from ..util.tunneling import stop_tailscale_funnel + stop_tailscale_funnel(process_info.tailscale_port) + logger.debug("Reconciled dangling Tailscale Funnel after detecting Commander process was no longer running") + except Exception as cleanup_error: + logger.debug(f"Failed to reconcile Tailscale Funnel: {cleanup_error}") ProcessInfo.clear() pass else: diff --git a/keepercommander/service/decorators/min_commander_version.py b/keepercommander/service/decorators/min_commander_version.py index 4d484caf3..64f336b78 100644 --- a/keepercommander/service/decorators/min_commander_version.py +++ b/keepercommander/service/decorators/min_commander_version.py @@ -23,8 +23,11 @@ # Hyphenated only: Werkzeug/WSGI silently drops headers that contain underscores. MIN_COMMANDER_VERSION_HEADER = 'Min-Commander-Version' -# Set on terraform-app-setup compose; not a secret — instance identity only. -TERRAFORM_DOCKER_ENV = 'KEEPER_TERRAFORM' +# Set on terraform-app-setup compose to the Terraform config record's UID. +TERRAFORM_DOCKER_ENV = 'TERRAFORM_RECORD' +# Pre-rename value (was '1', not a UID). Recognized so containers upgraded without re-running +# terraform-app-setup don't silently lose enforcement; drop after a migration period. +TERRAFORM_DOCKER_ENV_LEGACY = 'KEEPER_TERRAFORM' def _parse_version(version_str: str) -> Optional[Version]: @@ -41,8 +44,11 @@ def _parse_version(version_str: str) -> Optional[Version]: def _is_terraform_docker() -> bool: - """True when this process was started from terraform-app-setup compose.""" - return bool((os.environ.get(TERRAFORM_DOCKER_ENV) or '').strip()) + """True when this process was started from terraform-app-setup compose (new or pre-rename env var).""" + return bool( + (os.environ.get(TERRAFORM_DOCKER_ENV) or '').strip() + or (os.environ.get(TERRAFORM_DOCKER_ENV_LEGACY) or '').strip() + ) def _read_min_commander_version_header() -> Optional[str]: diff --git a/keepercommander/service/util/command_util.py b/keepercommander/service/util/command_util.py index 910c2c42f..f04a7152b 100644 --- a/keepercommander/service/util/command_util.py +++ b/keepercommander/service/util/command_util.py @@ -25,6 +25,13 @@ is_throttle_error, throttle_error_response, ) +from .protected_records import ( + get_protected_folder_uids, + get_protected_record_uids, + hide_from_folder_cache, + hide_from_record_cache, + resolve_sync_down_exempt_uid, +) from .verified_command import Verifycommand from ..core.globals import get_current_params from ..decorators.logging import logger, debug_decorator, sanitize_debug_data, sanitize_command_fields @@ -172,16 +179,18 @@ def execute(cls, command: str, temp_files: Optional[list] = None) -> Tuple[Any, # Mode will treat as safe, not the whole shared OS temp root. request_temp_dir = os.path.dirname(temp_files[0]) if temp_files else None + def blocked(error): + logger.warning( + f"Service Mode blocked command '{command_tokens[0] if command_tokens else ''}': {error}" + ) + return {"status": "error", "error": error}, 403 + # Same tokens the CLI will run — do not use raw HTTP split(" ") service_mode_error = Verifycommand.validate_service_mode_restrictions( command_tokens, request_temp_dir ) if service_mode_error: - logger.warning( - f"Service Mode blocked command '{command_tokens[0] if command_tokens else ''}': " - f"{service_mode_error}" - ) - return {"status": "error", "error": service_mode_error}, 403 + return blocked(service_mode_error) force_error = Verifycommand.validate_enterprise_user_add_role_force( command_tokens, params @@ -189,16 +198,40 @@ def execute(cls, command: str, temp_files: Optional[list] = None) -> Tuple[Any, if force_error: return {"status": "error", "error": force_error}, 400 + # Checked for every command (not a curated list) so no current or future + # command can be missed as a way to reference these records. + protected_uids = get_protected_record_uids(params) + + # {slack,gchat}-app-setup --sync-down needs its own config record reachable. + sync_down_exempt_uid = resolve_sync_down_exempt_uid(command_tokens) + if sync_down_exempt_uid is not None: + protected_uids = { + uid: title for uid, title in protected_uids.items() if uid != sync_down_exempt_uid + } + + # Derived from the record set so the exemption above reaches the exempted integration's own folder too. + protected_folder_uids = get_protected_folder_uids(params, protected_uids) + + protected_command_error = Verifycommand.validate_service_mode_protected_record_command( + command_tokens, + {**protected_uids, **{uid: '' for uid in protected_folder_uids}}, + ) + if protected_command_error: + return blocked(protected_command_error) + sailpoint_enabled = bool((os.environ.get('SAILPOINT_RECORD') or '').strip()) - if sailpoint_enabled: - from ..commands.integrations.sailpoint.service import SailPointService - command, sailpoint_response = SailPointService.handle_command(params, command) - if sailpoint_response is not None: - response, status_code = sailpoint_response - response = CommandExecutor.encrypt_response(response) - return response, status_code - - return_value, printed_output, log_output = CommandExecutor.capture_output_and_logs(params, command) + + with hide_from_record_cache(params, protected_uids), \ + hide_from_folder_cache(params, protected_folder_uids): + if sailpoint_enabled: + from ..commands.integrations.sailpoint.service import SailPointService + command, sailpoint_response = SailPointService.handle_command(params, command) + if sailpoint_response is not None: + response, status_code = sailpoint_response + response = CommandExecutor.encrypt_response(response) + return response, status_code + + return_value, printed_output, log_output = CommandExecutor.capture_output_and_logs(params, command) response = return_value if return_value else printed_output # Debug logging with sanitization diff --git a/keepercommander/service/util/protected_records.py b/keepercommander/service/util/protected_records.py new file mode 100644 index 000000000..7d1b18c4b --- /dev/null +++ b/keepercommander/service/util/protected_records.py @@ -0,0 +1,313 @@ +# _ __ +# | |/ /___ ___ _ __ ___ _ _ ® +# | ' ', + 'TERRAFORM_RECORD': '', + 'SLACK_RECORD': '', + 'TEAMS_RECORD': '', + 'GCHAT_RECORD': '', +} + +# uid-keyed caches resolve_single_record/load_pam_record fall back to when a UID isn't in record_cache. +_GUARDED_CACHE_ATTRS = ('record_cache', 'nested_share_records', 'nested_share_record_data') + +# Raw + derived folder caches every folder-resolving command reads through, +# directly or via subfolder.try_resolve_path/get_folder_uids. +_GUARDED_FOLDER_CACHE_ATTRS = ('folder_cache', 'shared_folder_cache', 'subfolder_cache') + +# Commander's own session (config.json) and Service Mode's own runtime (service_config.json) +# config files -- always attached under these exact, hardcoded names, never user-choosable. +_RESERVED_ATTACHMENT_NAMES = frozenset({'config.json', 'service_config.json'}) + + +def _attachment_file_uids(record) -> list: + """UIDs of every file attachment on record -- legacy PasswordRecord.attachments' ids, or a + typed record's fileRef entries (each its own separate FileRecord, loadable/gettable by that UID).""" + from ... import vault + + if isinstance(record, vault.PasswordRecord): + return [atta.id for atta in (record.attachments or []) if atta.id] + if isinstance(record, vault.TypedRecord): + typed_field = record.get_typed_field('fileRef') + if typed_field and isinstance(typed_field.value, list): + return [uid for uid in typed_field.value if isinstance(uid, str)] + return [] + + +def _has_reserved_legacy_attachment(record) -> bool: + """True if a PasswordRecord's own .attachments (no extra load -- filenames live on the attachment + object itself) include one literally named config.json/service_config.json.""" + from ... import vault + + return isinstance(record, vault.PasswordRecord) and any( + (atta.title or atta.name or '').lower() in _RESERVED_ATTACHMENT_NAMES + for atta in (record.attachments or []) + ) + + +def _protected_titles() -> Tuple[str, ...]: + """The literal titles of Service Mode's own config records; imported lazily to avoid a circular import through verified_command.""" + from ..config.file_handler import SERVICE_CONFIG_RECORD_TITLES + from ..commands.terraform_app_setup import TerraformSetupConstants + from ..commands.integrations.slack_app_setup import SlackAppSetupCommand + from ..commands.integrations.teams_app_setup import TeamsAppSetupCommand + from ..docker.models import DockerSetupConstants, GChatConstants + return ( + *SERVICE_CONFIG_RECORD_TITLES, + DockerSetupConstants.DEFAULT_RECORD_NAME, + TerraformSetupConstants.DEFAULT_RECORD_NAME, + GChatConstants.DEFAULT_RECORD_NAME, + SlackAppSetupCommand().get_default_record_name(), + TeamsAppSetupCommand().get_default_record_name(), + ) + + +def get_protected_record_title_set() -> FrozenSet[str]: + """Lower-cased titles of Service Mode's own config records, for literal matching.""" + return frozenset(t.lower() for t in _protected_titles()) + + +def get_protected_record_uids(params) -> Dict[str, str]: + """Resolve current UIDs of Service Mode's own config records ({uid: title}), matching by title plus each integration's pinned UID env var (_PINNED_RECORD_UID_ENVS); not cached, since a stale result on this security check is worse than the cost of a full-vault scan.""" + from ..commands.integrations.approvals_setup import is_valid_keeper_uid + from ..decorators.logging import logger + + found: Dict[str, str] = {} + for env_name, label in _PINNED_RECORD_UID_ENVS.items(): + uid = (os.environ.get(env_name) or '').strip() + if not uid: + continue + if not is_valid_keeper_uid(uid): + logger.warning(f'protected_records: {env_name} is set but not a valid record UID; falling back to title matching for it') + continue + found[uid] = label + + if params is None or not isinstance(getattr(params, 'record_cache', None), dict) or not params.record_cache: + return found + + from ... import vault + + protected_titles = get_protected_record_title_set() + # One load per record_cache entry, no more -- a FileRecord attachment target is itself an + # entry in this same cache, so its name is picked up by this same pass rather than a second, + # per-attachment load + reserved_file_uids: Set[str] = set() + pending_attachments: Dict[str, list] = {} + for uid in params.record_cache: + try: + record = vault.KeeperRecord.load(params, uid) + except Exception as e: + logger.debug(f'protected_records: could not load record {uid} ({type(e).__name__}); skipping') + continue + if not record: + continue + + if isinstance(record, vault.FileRecord): + if (record.title or record.name or '').lower() in _RESERVED_ATTACHMENT_NAMES: + reserved_file_uids.add(uid) + continue + + if record.title.lower() in protected_titles: + found[uid] = record.title + elif _has_reserved_legacy_attachment(record): + found[uid] = '' + + file_uids = _attachment_file_uids(record) + if file_uids: + pending_attachments[uid] = file_uids + + for parent_uid, file_uids in pending_attachments.items(): + if parent_uid not in found and any(file_uid in reserved_file_uids for file_uid in file_uids): + found[parent_uid] = '' + if parent_uid in found: + for file_uid in file_uids: + found.setdefault(file_uid, '') + + return found + + +def get_protected_folder_uids(params, protected_record_uids: Dict[str, str]) -> Set[str]: + """Folders directly containing an already-protected record, via subfolder_record_cache (folder_uid -> set of record UIDs) -- derived from record protection rather than a separate per-integration title list, so it stays correct even if a folder is renamed.""" + subfolder_record_cache = getattr(params, 'subfolder_record_cache', None) + if params is None or not protected_record_uids or not isinstance(subfolder_record_cache, dict): + return set() + + record_uids = protected_record_uids.keys() + return { + folder_uid for folder_uid, uids in subfolder_record_cache.items() + if folder_uid and isinstance(uids, (set, frozenset)) and uids & record_uids + } + + +def _sync_down_exempt_commands() -> Dict[str, str]: + """{command name: pinned-UID env var}, derived from each integration's own class instead of duplicated literals.""" + from ..commands.integrations.gchat_app_setup import GChatAppSetupCommand + from ..commands.integrations.slack_app_setup import SlackAppSetupCommand + return {cmd.get_command_name(): cmd.get_record_env_key() for cmd in (SlackAppSetupCommand(), GChatAppSetupCommand())} + + +def resolve_sync_down_exempt_uid(command_tokens) -> Optional[str]: + """For '{slack,gchat}-app-setup ... --sync-down ...', the one UID this dispatch may bypass Layers A/B for -- always this integration's own pinned-env UID, never derived from what the admin passes (e.g. -r/--integration-record), so a different integration's protected record can never be reached this way.""" + if not command_tokens: + return None + + env_name = _sync_down_exempt_commands().get(command_tokens[0].lower()) + if not env_name: + return None + + if '--sync-down' not in command_tokens[1:]: + return None + + return (os.environ.get(env_name) or '').strip() or None + + +class _GuardedRecordCache(UserDict): + """A uid-keyed cache view that can never hold the given protected UIDs; UserDict (not dict) so every mutation reliably routes through __setitem__, even C-level ones like setdefault/|=.""" + + def __init__(self, source, protected_uids: Iterable[str]): + self._protected_uids = frozenset(protected_uids) + super().__init__({k: v for k, v in source.items() if k not in self._protected_uids}) + + def __setitem__(self, key, value): + if key in self._protected_uids: + return + super().__setitem__(key, value) + + +@contextlib.contextmanager +def hide_from_record_cache(params, protected_uids: Dict[str, str]): + """For the with-block, guards record_cache/nested_share_records/nested_share_record_data against reintroduction (not just a one-time pop) and strips protected UIDs from subfolder_record_cache, restoring everything on exit; relies on Service Mode commands running one at a time (same assumption capture_output_and_logs already makes) and may leave record_cache stale until the next sync if one lands mid-command.""" + if params is None or not protected_uids: + yield + return + + protected_uid_set = frozenset(protected_uids) + + original_caches = {} + saved_entries = {} + for attr in _GUARDED_CACHE_ATTRS: + source = getattr(params, attr, None) + if not isinstance(source, dict): + continue + original_caches[attr] = source + saved_entries[attr] = {uid: source[uid] for uid in protected_uid_set if uid in source} + setattr(params, attr, _GuardedRecordCache(source, protected_uid_set)) + + subfolder_cache = getattr(params, 'subfolder_record_cache', None) + removed_from_folders: Dict[str, set] = {} + if isinstance(subfolder_cache, dict): + for folder_uid, uids in subfolder_cache.items(): + if not isinstance(uids, set): + continue + hit = uids & protected_uid_set + if hit: + removed_from_folders[folder_uid] = hit + uids -= hit + + try: + yield + finally: + from ..decorators.logging import logger + + for attr in original_caches: + try: + restored = dict(getattr(params, attr, None) or {}) + restored.update(saved_entries[attr]) + setattr(params, attr, restored) + except Exception as e: + logger.debug(f'hide_from_record_cache: failed to restore {attr} ({type(e).__name__}); restoring protected entries only') + try: + setattr(params, attr, dict(saved_entries[attr])) + except Exception: + pass + + if isinstance(subfolder_cache, dict): + for folder_uid, hit in removed_from_folders.items(): + try: + uids = subfolder_cache.get(folder_uid) + if isinstance(uids, set): + uids |= hit + except Exception as e: + logger.debug(f'hide_from_record_cache: failed to restore subfolder {folder_uid} ({type(e).__name__})') + + +@contextlib.contextmanager +def hide_from_folder_cache(params, protected_folder_uids: Set[str]): + """For the with-block, hides protected_folder_uids from folder_cache/shared_folder_cache/subfolder_cache and + from their parent's (or root_folder's) .subfolders list, restoring everything on exit """ + if params is None or not protected_folder_uids: + yield + return + + protected_uid_set = frozenset(protected_folder_uids) + + folder_cache = getattr(params, 'folder_cache', None) + root_folder = getattr(params, 'root_folder', None) + # {uid: (subfolders_list, original_index)} -- built up incrementally inside the try below so a + # failure partway through setup still leaves whatever was already removed restorable in finally, + # rather than mutating this live list before there's any guarantee finally will run at all. + removed_from_parents: Dict[str, tuple] = {} + original_caches = {} + saved_entries = {} + + try: + if isinstance(folder_cache, dict): + for uid in protected_uid_set: + node = folder_cache.get(uid) + parent_uid = getattr(node, 'parent_uid', None) if node is not None else None + parent = folder_cache.get(parent_uid) if parent_uid else root_folder + subfolders = getattr(parent, 'subfolders', None) if parent is not None else None + if isinstance(subfolders, list) and uid in subfolders: + index = subfolders.index(uid) + subfolders.remove(uid) + removed_from_parents[uid] = (subfolders, index) + + for attr in _GUARDED_FOLDER_CACHE_ATTRS: + source = getattr(params, attr, None) + if not isinstance(source, dict): + continue + original_caches[attr] = source + saved_entries[attr] = {uid: source[uid] for uid in protected_uid_set if uid in source} + setattr(params, attr, _GuardedRecordCache(source, protected_uid_set)) + + yield + finally: + from ..decorators.logging import logger + + for attr in original_caches: + try: + restored = dict(getattr(params, attr, None) or {}) + restored.update(saved_entries[attr]) + setattr(params, attr, restored) + except Exception as e: + logger.debug(f'hide_from_folder_cache: failed to restore {attr} ({type(e).__name__}); restoring protected entries only') + try: + setattr(params, attr, dict(saved_entries[attr])) + except Exception: + pass + + for uid, (subfolders, index) in removed_from_parents.items(): + try: + if uid not in subfolders: + subfolders.insert(min(index, len(subfolders)), uid) + except Exception as e: + logger.debug(f'hide_from_folder_cache: failed to restore subfolders entry for {uid} ({type(e).__name__})') diff --git a/keepercommander/service/util/tunneling.py b/keepercommander/service/util/tunneling.py index e3bf2dca3..5f0cb24ca 100644 --- a/keepercommander/service/util/tunneling.py +++ b/keepercommander/service/util/tunneling.py @@ -429,4 +429,458 @@ def generate_cloudflare_url(port, tunnel_token, custom_domain, run_mode): tunnel_token=tunnel_token, custom_domain=custom_domain ) - return public_url, tunnel_pid + return public_url, tunnel_pid# Tailscale Funnel Functions + +TAILSCALE_INSTALL_URL = "https://tailscale.com/download" + + +def is_tailscale_installed(): + """Check whether the Tailscale CLI is on PATH.""" + import shutil + return shutil.which('tailscale') is not None + + +def get_tailscale_install_guidance(): + """Manual install guidance; used when auto-install is unavailable or fails.""" + return ( + "Tailscale CLI was not found on this system. Commander Service Mode " + "requires Tailscale to be installed before enabling Tailscale Funnel. " + f"Please install Tailscale from {TAILSCALE_INSTALL_URL} and retry." + ) + + +TAILSCALE_INSTALL_SCRIPT_URL = "https://tailscale.com/install.sh" +TAILSCALE_MSI_INSTALLER_URL = "https://pkgs.tailscale.com/stable/tailscale-setup-latest-amd64.msi" +TAILSCALE_INSTALL_TIMEOUT = 180 + + +def _run_privileged_tailscale_command(cmd, timeout, action_label): + """ + Run a Tailscale management command (install/daemon-start, may need sudo) + with standard timeout/error handling. Returns True on success, False otherwise. + """ + print(f"Running: {' '.join(cmd)}") + try: + result = subprocess.run(cmd, timeout=timeout, env=os.environ.copy()) + if result.returncode != 0: + logging.error(f"{action_label} failed, exit code {result.returncode}") + return False + return True + except subprocess.TimeoutExpired: + logging.error(f"{action_label} timed out after {timeout}s") + return False + except Exception as e: + logging.error(f"Error during {action_label.lower()}: {type(e).__name__}") + return False + + +def _install_tailscale_macos(): + """Install via Homebrew. Returns False if Homebrew isn't available (no GUI/App Store fallback).""" + import shutil + if not shutil.which('brew'): + logging.info("Homebrew not available for automatic Tailscale install") + return False + return _run_privileged_tailscale_command(['brew', 'install', 'tailscale'], TAILSCALE_INSTALL_TIMEOUT, "Tailscale install") + + +def _install_tailscale_linux(): + """Download and run the official install script. May prompt for sudo interactively.""" + import urllib.request + import tempfile + + tmp_path = None + try: + with tempfile.NamedTemporaryFile(delete=False, suffix='.sh') as tmp_file: + tmp_path = tmp_file.name + urllib.request.urlretrieve(TAILSCALE_INSTALL_SCRIPT_URL, tmp_path) + return _run_privileged_tailscale_command(['sh', tmp_path], TAILSCALE_INSTALL_TIMEOUT, "Tailscale install") + except Exception as e: + logging.error(f"Error downloading Tailscale install script: {type(e).__name__}") + return False + finally: + if tmp_path: + try: + os.unlink(tmp_path) + except OSError: + pass + + +def _is_windows_process_elevated(): + """Check whether this process has Administrator privileges.""" + try: + import ctypes + return bool(ctypes.windll.shell32.IsUserAnAdmin()) + except Exception as e: + logging.debug(f"Could not determine Windows elevation state: {type(e).__name__}") + return False + + +def _run_msiexec_elevated_windows(msi_path, timeout): + """Run msiexec via a UAC prompt (Start-Process -Verb RunAs). Returns exit code, or None if declined/failed.""" + msi_args = f'/i "{msi_path}" /quiet TS_NOLAUNCH=1' + ps_command = ( + "try { " + f"$p = Start-Process -FilePath msiexec.exe -ArgumentList '{msi_args}' -Verb RunAs -Wait -PassThru; " + "Write-Output $p.ExitCode " + "} catch { Write-Output 'ELEVATION_FAILED' }" + ) + cmd = ["powershell", "-NoProfile", "-Command", ps_command] + print("Requesting Administrator approval (UAC prompt) to install Tailscale...") + + result = subprocess.run(cmd, capture_output=True, text=True, timeout=timeout) + output = (result.stdout or '').strip() + if 'ELEVATION_FAILED' in output: + logging.error("Elevation request failed or was declined") + return None + try: + return int(output.splitlines()[-1].strip()) + except (ValueError, IndexError): + logging.error(f"Could not parse msiexec exit code: {output!r}") + return None + + +def _install_tailscale_windows(): + """Download the official MSI and install silently (no verified winget package exists). Elevates via UAC if needed.""" + import urllib.request + import tempfile + + tmp_path = None + try: + with tempfile.NamedTemporaryFile(delete=False, suffix='.msi') as tmp_file: + tmp_path = tmp_file.name + urllib.request.urlretrieve(TAILSCALE_MSI_INSTALLER_URL, tmp_path) + + if _is_windows_process_elevated(): + cmd = ['msiexec', '/i', tmp_path, '/quiet', 'TS_NOLAUNCH=1'] + print(f"Running: {' '.join(cmd)}") + result = subprocess.run(cmd, timeout=TAILSCALE_INSTALL_TIMEOUT, env=os.environ.copy()) + returncode = result.returncode + else: + returncode = _run_msiexec_elevated_windows(tmp_path, TAILSCALE_INSTALL_TIMEOUT) + + if returncode is None or returncode != 0: + logging.error(f"Tailscale install failed, exit code {returncode}") + return False + + _add_windows_tailscale_to_process_path() + return True + except subprocess.TimeoutExpired: + logging.error(f"Tailscale install timed out after {TAILSCALE_INSTALL_TIMEOUT}s") + return False + except Exception as e: + logging.error(f"Error installing Tailscale via MSI: {type(e).__name__}") + return False + finally: + if tmp_path: + try: + os.unlink(tmp_path) + except OSError: + pass + + +def _add_windows_tailscale_to_process_path(): + """Extend this process's PATH so is_tailscale_installed() sees a fresh install without a shell restart.""" + default_install_dir = r"C:\Program Files\Tailscale" + current_path = os.environ.get("PATH", "") + if default_install_dir not in current_path.split(os.pathsep): + os.environ["PATH"] = current_path + os.pathsep + default_install_dir + logging.debug(f"Added {default_install_dir} to process PATH") + + +def install_tailscale(): + """Install Tailscale for the current OS. Caller should re-check is_tailscale_installed() after.""" + import platform + system = platform.system() + + if system == "Darwin": + return _install_tailscale_macos() + elif system == "Linux": + return _install_tailscale_linux() + elif system == "Windows": + return _install_tailscale_windows() + else: + logging.error(f"Automatic Tailscale install not supported on platform: {system}") + return False + + +TAILSCALE_DAEMON_START_TIMEOUT = 60 + + +_TAILSCALE_DAEMON_UNREACHABLE_HINT = "failed to connect to local tailscale service" + + +def is_tailscale_daemon_running(): + """ + Check whether tailscaled is reachable. `tailscale status` exits non-zero + both when unreachable and when merely logged out, so check for the + specific unreachable-connection message rather than the exit code. + """ + try: + result = subprocess.run(['tailscale', 'status'], capture_output=True, text=True, timeout=10) + combined_output = f"{result.stdout or ''}{result.stderr or ''}".lower() + return _TAILSCALE_DAEMON_UNREACHABLE_HINT not in combined_output + except Exception as e: + logging.debug(f"Error checking Tailscale daemon status: {type(e).__name__}") + return False + + +def get_tailscale_daemon_start_guidance(): + """Manual daemon-start guidance; used when auto-start fails.""" + return ( + "Tailscale CLI is installed, but the Tailscale daemon is not running. " + "On macOS: run 'sudo brew services start tailscale' (or open the Tailscale app). " + "On Linux: run 'sudo systemctl start tailscaled'. " + "On Windows: ensure the Tailscale service is running (reinstall or restart it from Services). " + "Then retry." + ) + + +def _start_tailscale_daemon_macos(): + """Start tailscaled via Homebrew services. Requires sudo.""" + return _run_privileged_tailscale_command(['sudo', 'brew', 'services', 'start', 'tailscale'], TAILSCALE_DAEMON_START_TIMEOUT, "Daemon start") + + +def _start_tailscale_daemon_linux(): + """Start tailscaled via systemd. Requires sudo.""" + return _run_privileged_tailscale_command(['sudo', 'systemctl', 'start', 'tailscaled'], TAILSCALE_DAEMON_START_TIMEOUT, "Daemon start") + + +def _start_tailscale_daemon_windows(): + """Start the Tailscale Windows service.""" + return _run_privileged_tailscale_command(['net', 'start', 'Tailscale'], TAILSCALE_DAEMON_START_TIMEOUT, "Daemon start") + + +def start_tailscale_daemon(): + """Start the daemon for the current OS. Caller should re-check is_tailscale_daemon_running() after.""" + import platform + system = platform.system() + + if system == "Darwin": + return _start_tailscale_daemon_macos() + elif system == "Linux": + return _start_tailscale_daemon_linux() + elif system == "Windows": + return _start_tailscale_daemon_windows() + else: + logging.error(f"Automatic daemon start not supported on platform: {system}") + return False + + +def _get_tailscale_log_path(): + """Path to the Tailscale subprocess log file.""" + service_core_dir = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "core") + log_dir = os.path.join(service_core_dir, "logs") + os.makedirs(log_dir, exist_ok=True) + return os.path.join(log_dir, "tailscale_subprocess.log") + + +def reset_tailscale_log(): + """ + Truncate the Tailscale subprocess log at the start of a service lifecycle, + matching Ngrok/Cloudflare's per-session log convention. Without this, the + log grows unbounded across every start/stop cycle -- unlike the other + tunnel providers' 'w'-mode logs, Tailscale's is always opened in append + mode since multiple one-shot commands (up/funnel) share it within a + single lifecycle. + """ + try: + open(_get_tailscale_log_path(), 'w').close() + except OSError as e: + logging.debug(f"Could not reset Tailscale log: {type(e).__name__}") + + +def tailscale_up(auth_key, advertise_tags=None): + """ + Authenticate via `tailscale up --auth-key=... --advertise-tags=... --force-reauth`. + advertise_tags is required for OAuth-client-issued auth keys. --advertise-tags + is always passed explicitly (empty if unused) -- `tailscale up` requires every + non-default setting to be re-specified on each call, or it errors out; omitting + the flag entirely fails if a previous run (e.g. a prior OAuth key) left tags set. + + --force-reauth is required too: without it, `tailscale up` returns exit code 0 + for an invalid auth key as long as the node is already authenticated under any + identity -- there's nothing to re-authenticate, so the key is silently ignored + rather than validated. --force-reauth makes Tailscale genuinely re-validate the + key every time, so the exit code can be trusted. Per Tailscale's own docs, this + may briefly disrupt an active connection if this same Tailscale link is being + used for something else (e.g. an SSH session) at the moment of the call. + + The auth key is written to a short-lived, owner-only-readable temp file + and passed as `--auth-key=file:` rather than a raw argv value -- + Tailscale supports this directly, avoiding exposing the key via `ps`/ + `/proc` to other local users for the life of the subprocess. Never logged. + """ + if not auth_key: + raise ValueError("Tailscale auth key must be provided for 'tailscale up'.") + + import tempfile + log_file = _get_tailscale_log_path() + key_file_path = None + + try: + fd, key_file_path = tempfile.mkstemp(suffix='.tskey') + os.chmod(key_file_path, 0o600) + with os.fdopen(fd, 'w') as key_f: + key_f.write(auth_key) + + cmd = ["tailscale", "up", f"--auth-key=file:{key_file_path}", + f"--advertise-tags={advertise_tags or ''}", "--force-reauth"] + + with open(log_file, 'a') as log_f: + result = subprocess.run( + cmd, + stdout=log_f, + stderr=subprocess.STDOUT, + env=os.environ.copy(), + timeout=60, + ) + if result.returncode != 0: + hint = "" + try: + with open(log_file, 'r') as f: + if "requires --advertise-tags" in f.read() and not advertise_tags: + hint = " This auth key requires --advertise-tags (OAuth-issued key)." + except OSError: + pass + logging.error(f"Tailscale authentication failed, exit code {result.returncode}") + raise Exception( + f"Tailscale authentication failed (exit code {result.returncode}).{hint} " + f"See {log_file} for details." + ) + logging.info("Tailscale authentication successful") + except subprocess.TimeoutExpired: + logging.error("Tailscale authentication timed out") + raise Exception("Tailscale authentication timed out after 60 seconds.") + finally: + if key_file_path: + try: + os.unlink(key_file_path) + except OSError: + pass + + +# Tailscale Funnel only accepts one of these as the external-facing port; +# the local target port (the Commander service port) is unrestricted and +# separate. 443 is the default so the public URL needs no port suffix. +TAILSCALE_FUNNEL_ALLOWED_PORTS = (443, 8443, 10000) +TAILSCALE_FUNNEL_DEFAULT_PORT = 443 + + +def start_tailscale_funnel(local_port, funnel_port=TAILSCALE_FUNNEL_DEFAULT_PORT): + """ + Enable Funnel: forward funnel_port -> localhost:local_port. + --bg is required, otherwise the command blocks in the foreground indefinitely. + """ + if not local_port: + raise ValueError("Port must be provided to start Tailscale Funnel.") + if funnel_port not in TAILSCALE_FUNNEL_ALLOWED_PORTS: + raise ValueError( + f"Invalid Tailscale Funnel port {funnel_port}; must be one of {TAILSCALE_FUNNEL_ALLOWED_PORTS}." + ) + + cmd = ["tailscale", "funnel", "--bg", f"--https={funnel_port}", f"localhost:{local_port}"] + log_file = _get_tailscale_log_path() + + try: + with open(log_file, 'a') as log_f: + result = subprocess.run( + cmd, + stdout=log_f, + stderr=subprocess.STDOUT, + env=os.environ.copy(), + timeout=30, + ) + if result.returncode != 0: + logging.error(f"Tailscale Funnel start failed, exit code {result.returncode}") + raise Exception( + f"Failed to start Tailscale Funnel (exit code {result.returncode}). " + f"See {log_file} for details. First-time Funnel use on a tailnet may " + "require one-time approval in the Tailscale admin console." + ) + logging.info(f"Tailscale Funnel enabled: localhost:{local_port} -> :{funnel_port}") + except subprocess.TimeoutExpired: + logging.error("Starting Tailscale Funnel timed out") + raise Exception("Starting Tailscale Funnel timed out after 30 seconds.") + + +def get_tailscale_funnel_url(local_port, funnel_port=TAILSCALE_FUNNEL_DEFAULT_PORT, max_retries=10, retry_delay=1): + """Build the public Funnel URL from this node's MagicDNS hostname + funnel_port.""" + for attempt in range(max_retries): + try: + result = subprocess.run( + ["tailscale", "status", "--json"], + capture_output=True, + text=True, + timeout=10, + ) + if result.returncode == 0 and result.stdout: + status = json.loads(result.stdout) + dns_name = (status.get("Self", {}).get("DNSName") or "").rstrip('.') + if dns_name: + if funnel_port == TAILSCALE_FUNNEL_DEFAULT_PORT: + return f"https://{dns_name}" + return f"https://{dns_name}:{funnel_port}" + except subprocess.TimeoutExpired: + logging.debug("Timed out retrieving Tailscale status") + except Exception as e: + logging.debug(f"Error retrieving Tailscale funnel URL: {type(e).__name__}") + + if attempt < max_retries - 1: + time.sleep(retry_delay) + + logging.warning(f"Could not retrieve Tailscale Funnel URL after {max_retries} attempts") + return None + + +def stop_tailscale_funnel(local_port, funnel_port=TAILSCALE_FUNNEL_DEFAULT_PORT): + """ + Disable Funnel via `tailscale funnel reset` (no per-target `off` exists + in this CLI version). Resets all funnel config on this node; acceptable + since Commander manages a single target. local_port/funnel_port kept + for signature symmetry with start_tailscale_funnel. + """ + cmd = ["tailscale", "funnel", "reset"] + log_file = _get_tailscale_log_path() + + try: + with open(log_file, 'a') as log_f: + result = subprocess.run( + cmd, + stdout=log_f, + stderr=subprocess.STDOUT, + env=os.environ.copy(), + timeout=30, + ) + if result.returncode == 0: + logging.info(f"Tailscale Funnel disabled for localhost:{local_port}") + return True + logging.warning(f"Failed to stop Tailscale Funnel, exit code {result.returncode}") + return False + except Exception as e: + logging.error(f"Error stopping Tailscale Funnel: {type(e).__name__}") + return False + + +def get_tailscale_funnel_status(local_port): + """ + Check live Funnel status via `tailscale funnel status --json`. Verified + schema: active targets appear as data["Web"][":"]["Handlers"] + [""]["Proxy"] == "http://localhost:". + """ + try: + result = subprocess.run( + ["tailscale", "funnel", "status", "--json"], + capture_output=True, + text=True, + timeout=10, + ) + if result.returncode == 0 and result.stdout: + data = json.loads(result.stdout) + target = f"http://localhost:{local_port}" + for web_config in (data.get("Web") or {}).values(): + for handler in (web_config.get("Handlers") or {}).values(): + if handler.get("Proxy") == target: + return True + except Exception as e: + logging.debug(f"Error checking Tailscale funnel status: {type(e).__name__}") + return False diff --git a/keepercommander/service/util/verified_command.py b/keepercommander/service/util/verified_command.py index 20bbb6fcb..9d8ea389b 100644 --- a/keepercommander/service/util/verified_command.py +++ b/keepercommander/service/util/verified_command.py @@ -17,6 +17,10 @@ class Verifycommand: # Aliases from record.py — CommandExecutor checks tokens before cli expands them. _RECORD_EDIT_COMMANDS = frozenset({'record-add', 'ra', 'record-update', 'ru'}) + _PROTECTED_RECORD_MSG = ( + 'Service Mode configuration records are not accessible through Service Mode' + ) + # Legacy Commands category — plugin-based rotation/connection commands have no safe Service Mode form # and are blocked unconditionally, regardless of what an API key's command_list allows. _LEGACY_COMMANDS = frozenset({ @@ -99,6 +103,33 @@ def validate_service_mode_restrictions(command_tokens, request_temp_dir=None): return error return None + @staticmethod + def _record_reference_candidates(tok): + """tok itself, plus its value if tok is a --flag=value (or -f=value) option.""" + if '=' in tok: + _, _, value = tok.partition('=') + if value: + return (tok, value) + return (tok,) + + @staticmethod + def validate_service_mode_protected_record_command(command_tokens, protected_uids=None): + """Reject any command with a protected title/UID as a whole token or --flag=value; indirect forms (comma lists, path-qualified titles) rely on protected_records.hide_from_record_cache instead.""" + if not command_tokens: + return None + + from .protected_records import get_protected_record_title_set + protected_titles = get_protected_record_title_set() + + uid_set = set(protected_uids) if protected_uids else set() + for tok in command_tokens[1:]: + for candidate in Verifycommand._record_reference_candidates(tok): + if candidate.lower() in protected_titles: + return Verifycommand._PROTECTED_RECORD_MSG + if candidate in uid_set: + return Verifycommand._PROTECTED_RECORD_MSG + return None + @staticmethod def validate_service_mode_double_dash(command_tokens, request_temp_dir=None): """Block bare '--' anywhere in Service Mode input; error or None.""" diff --git a/unit-tests/pam/test_pam_rotation.py b/unit-tests/pam/test_pam_rotation.py index 79f723e88..1836b69df 100644 --- a/unit-tests/pam/test_pam_rotation.py +++ b/unit-tests/pam/test_pam_rotation.py @@ -860,6 +860,195 @@ def test_table_mode_returns_none(self, mock_rrg, mock_schedules): result = cmd.execute(mock_params, record_uid=record_uid, format='table') self.assertIsNone(result) + @patch('keepercommander.commands.discoveryrotation.gateway_helper.get_all_gateways') + @patch('keepercommander.commands.discoveryrotation.router_get_rotation_schedules') + @patch('keepercommander.commands.discoveryrotation.record_rotation_get') + def test_gateway_name_resolved_from_uid_when_empty(self, mock_rrg, mock_schedules, mock_get_gateways): + """When controllerName is empty, it should be resolved from gateway list using controllerUid.""" + from keeper_secrets_manager_core.utils import url_safe_str_to_bytes + record_uid = 'test_record_uid_' + record_uid_bytes = url_safe_str_to_bytes(record_uid) + + rri = self._make_rri('RRS_ONLINE') + rri.controllerName = '' + + mock_rrg.return_value = rri + + sched_mock = MagicMock() + sched_mock.schedules = [self._make_schedule(record_uid_bytes)] + mock_schedules.return_value = sched_mock + + mock_gateway = MagicMock() + mock_gateway.controllerUid = rri.controllerUid + mock_gateway.controllerName = 'gw-test-resolved' + mock_get_gateways.return_value = [mock_gateway] + + mock_params = create_mock_params() + mock_params.record_cache = {} + + cmd = PAMRouterGetRotationInfo() + result = cmd.execute(mock_params, record_uid=record_uid, format='json') + + self.assertIsNotNone(result, "Expected JSON string, got None") + data = json.loads(result) + self.assertEqual(data['gateway_name'], 'gw-test-resolved') + + @patch('keepercommander.commands.discoveryrotation.gateway_helper.get_all_gateways') + @patch('keepercommander.commands.discoveryrotation.router_get_rotation_schedules') + @patch('keepercommander.commands.discoveryrotation.record_rotation_get') + def test_gateway_name_present_unchanged(self, mock_rrg, mock_schedules, mock_get_gateways): + """When controllerName is present, it should be used without looking up gateways.""" + from keeper_secrets_manager_core.utils import url_safe_str_to_bytes + record_uid = 'test_record_uid_' + record_uid_bytes = url_safe_str_to_bytes(record_uid) + + rri = self._make_rri('RRS_ONLINE') + rri.controllerName = 'gw-original' + + mock_rrg.return_value = rri + + sched_mock = MagicMock() + sched_mock.schedules = [self._make_schedule(record_uid_bytes)] + mock_schedules.return_value = sched_mock + + mock_params = create_mock_params() + mock_params.record_cache = {} + + cmd = PAMRouterGetRotationInfo() + result = cmd.execute(mock_params, record_uid=record_uid, format='json') + + self.assertIsNotNone(result) + data = json.loads(result) + self.assertEqual(data['gateway_name'], 'gw-original') + mock_get_gateways.assert_not_called() + + @patch('keepercommander.commands.discoveryrotation.gateway_helper.get_all_gateways') + @patch('keepercommander.commands.discoveryrotation.router_get_rotation_schedules') + @patch('keepercommander.commands.discoveryrotation.record_rotation_get') + def test_gateway_name_empty_no_match_falls_back_to_dash(self, mock_rrg, mock_schedules, mock_get_gateways): + """When controllerName is empty and no gateway matches, should fall back to '-'.""" + from keeper_secrets_manager_core.utils import url_safe_str_to_bytes + record_uid = 'test_record_uid_' + record_uid_bytes = url_safe_str_to_bytes(record_uid) + + rri = self._make_rri('RRS_ONLINE') + rri.controllerName = '' + + mock_rrg.return_value = rri + + sched_mock = MagicMock() + sched_mock.schedules = [self._make_schedule(record_uid_bytes)] + mock_schedules.return_value = sched_mock + + mock_gateway = MagicMock() + mock_gateway.controllerUid = b'different_uid_' + mock_gateway.controllerName = 'gw-other' + mock_get_gateways.return_value = [mock_gateway] + + mock_params = create_mock_params() + mock_params.record_cache = {} + + cmd = PAMRouterGetRotationInfo() + result = cmd.execute(mock_params, record_uid=record_uid, format='json') + + self.assertIsNotNone(result) + data = json.loads(result) + self.assertEqual(data['gateway_name'], '-') + + @patch('keepercommander.commands.discoveryrotation.gateway_helper.get_all_gateways') + @patch('keepercommander.commands.discoveryrotation.router_get_rotation_schedules') + @patch('keepercommander.commands.discoveryrotation.record_rotation_get') + def test_gateway_name_resolved_with_different_uid_types(self, mock_rrg, mock_schedules, mock_get_gateways): + """When controllerUid types differ (bytes vs string), should still resolve correctly.""" + from keeper_secrets_manager_core.utils import url_safe_str_to_bytes + from keepercommander import utils + record_uid = 'test_record_uid_' + record_uid_bytes = url_safe_str_to_bytes(record_uid) + + rri = self._make_rri('RRS_ONLINE') + rri.controllerName = '' + + mock_rrg.return_value = rri + + sched_mock = MagicMock() + sched_mock.schedules = [self._make_schedule(record_uid_bytes)] + mock_schedules.return_value = sched_mock + + mock_gateway = MagicMock() + mock_gateway.controllerUid = utils.base64_url_encode(rri.controllerUid) + mock_gateway.controllerName = 'gw-test-resolved' + mock_get_gateways.return_value = [mock_gateway] + + mock_params = create_mock_params() + mock_params.record_cache = {} + + cmd = PAMRouterGetRotationInfo() + result = cmd.execute(mock_params, record_uid=record_uid, format='json') + + self.assertIsNotNone(result) + data = json.loads(result) + self.assertEqual(data['gateway_name'], 'gw-test-resolved') + + @patch('keepercommander.commands.discoveryrotation.gateway_helper.get_all_gateways') + @patch('keepercommander.commands.discoveryrotation.router_get_rotation_schedules') + @patch('keepercommander.commands.discoveryrotation.record_rotation_get') + def test_gateway_list_returns_none_falls_back_gracefully(self, mock_rrg, mock_schedules, mock_get_gateways): + """When get_all_gateways returns None, should fall back to '-' gracefully.""" + from keeper_secrets_manager_core.utils import url_safe_str_to_bytes + record_uid = 'test_record_uid_' + record_uid_bytes = url_safe_str_to_bytes(record_uid) + + rri = self._make_rri('RRS_ONLINE') + rri.controllerName = '' + + mock_rrg.return_value = rri + + sched_mock = MagicMock() + sched_mock.schedules = [self._make_schedule(record_uid_bytes)] + mock_schedules.return_value = sched_mock + + mock_get_gateways.return_value = None + + mock_params = create_mock_params() + mock_params.record_cache = {} + + cmd = PAMRouterGetRotationInfo() + result = cmd.execute(mock_params, record_uid=record_uid, format='json') + + self.assertIsNotNone(result) + data = json.loads(result) + self.assertEqual(data['gateway_name'], '-') + + @patch('keepercommander.commands.discoveryrotation.gateway_helper.get_all_gateways') + @patch('keepercommander.commands.discoveryrotation.router_get_rotation_schedules') + @patch('keepercommander.commands.discoveryrotation.record_rotation_get') + def test_gateway_list_raises_exception_falls_back_gracefully(self, mock_rrg, mock_schedules, mock_get_gateways): + """When get_all_gateways raises an exception, should fall back to '-' without crashing.""" + from keeper_secrets_manager_core.utils import url_safe_str_to_bytes + record_uid = 'test_record_uid_' + record_uid_bytes = url_safe_str_to_bytes(record_uid) + + rri = self._make_rri('RRS_ONLINE') + rri.controllerName = '' + + mock_rrg.return_value = rri + + sched_mock = MagicMock() + sched_mock.schedules = [self._make_schedule(record_uid_bytes)] + mock_schedules.return_value = sched_mock + + mock_get_gateways.side_effect = RuntimeError("Gateway service unavailable") + + mock_params = create_mock_params() + mock_params.record_cache = {} + + cmd = PAMRouterGetRotationInfo() + result = cmd.execute(mock_params, record_uid=record_uid, format='json') + + self.assertIsNotNone(result) + data = json.loads(result) + self.assertEqual(data['gateway_name'], '-') + class TestUsesDefaultRotationSchedule(unittest.TestCase): diff --git a/unit-tests/pam/test_pam_tunnel.py b/unit-tests/pam/test_pam_tunnel.py index 41c55e7f8..0bfc140c0 100644 --- a/unit-tests/pam/test_pam_tunnel.py +++ b/unit-tests/pam/test_pam_tunnel.py @@ -2,6 +2,7 @@ from unittest import mock from keepercommander.error import CommandError +from keepercommander.commands.tunnel_and_connections import PAMTunnelDiagnoseCommand import datetime import socket @@ -154,3 +155,83 @@ def test_uniqueness(self): random_bytes1 = generate_random_bytes() random_bytes2 = generate_random_bytes() self.assertNotEqual(random_bytes1, random_bytes2) + + +class TestPAMTunnelDiagnose(unittest.TestCase): + def test_execute_passes_session_proxy_to_https_probes(self): + proxies = {'http': 'http://proxy.example:8080', 'https': 'http://proxy.example:8080'} + params = mock.MagicMock() + params.server = 'keepersecurity.com' + params.rest_context.proxies = proxies + params.ssl_verify = '/path/to/ca.pem' + + with mock.patch('keepercommander.commands.tunnel_and_connections.get_relay_host', + return_value='relay.example'), \ + mock.patch('keepercommander.commands.tunnel_and_connections.get_router_host', + return_value='router.example'), \ + mock.patch('keepercommander.commands.tunnel_and_connections.get_or_create_tube_registry', + return_value=None), \ + mock.patch('keepercommander.commands.tunnel_and_connections.socket.getaddrinfo', + return_value=[(socket.AF_INET, socket.SOCK_STREAM, 6, '', ('203.0.113.1', 0))]), \ + mock.patch('keepercommander.commands.tunnel_and_connections.socket.gethostbyname', + return_value='203.0.113.1'), \ + mock.patch.object(PAMTunnelDiagnoseCommand, '_test_https', + return_value=(True, 'reachable', 1)) as test_https, \ + mock.patch.object(PAMTunnelDiagnoseCommand, '_test_websocket', + return_value=(True, 'reachable', 1)) as test_websocket, \ + mock.patch.object(PAMTunnelDiagnoseCommand, '_test_tcp_stun', + return_value=(True, 'reachable', 1, None)), \ + mock.patch.object(PAMTunnelDiagnoseCommand, '_test_udp_stun', + return_value=(True, 'reachable', 1, None)), \ + mock.patch.object(PAMTunnelDiagnoseCommand, '_test_turn', + return_value=(True, 'reachable', 1)), \ + mock.patch.object(PAMTunnelDiagnoseCommand, '_test_udp_port', + return_value=(True, 1)): + PAMTunnelDiagnoseCommand().execute(params) + + test_https.assert_called_once_with( + 'keepersecurity.com', proxies=proxies, verify='/path/to/ca.pem') + test_websocket.assert_called_once_with( + 'router.example', proxies=proxies, verify='/path/to/ca.pem') + + def test_https_uses_configured_proxy(self): + proxies = {'http': 'http://proxy.example:8080', 'https': 'http://proxy.example:8080'} + with mock.patch('keepercommander.commands.tunnel_and_connections.requests.get') as mock_get: + mock_get.return_value.status_code = 200 + + passed, _, _ = PAMTunnelDiagnoseCommand._test_https( + 'api.example', proxies=proxies, verify='/path/to/ca.pem') + + self.assertTrue(passed) + mock_get.assert_called_once_with( + 'https://api.example:443/', + headers={'User-Agent': 'keeper-pam-diagnose/1.0'}, + proxies=proxies, + verify='/path/to/ca.pem', + timeout=10, + stream=True, + ) + + def test_websocket_uses_configured_proxy(self): + proxies = {'http': 'http://proxy.example:8080', 'https': 'http://proxy.example:8080'} + with mock.patch('keepercommander.commands.tunnel_and_connections.requests.get') as mock_get: + mock_get.return_value.status_code = 101 + + passed, _, _ = PAMTunnelDiagnoseCommand._test_websocket( + 'router.example', proxies=proxies, verify='/path/to/ca.pem') + + self.assertTrue(passed) + mock_get.assert_called_once_with( + 'https://router.example:443/', + headers={ + 'Upgrade': 'websocket', + 'Connection': 'Upgrade', + 'Sec-WebSocket-Key': 'dGhlIHNhbXBsZSBub25jZQ==', + 'Sec-WebSocket-Version': '13', + 'User-Agent': 'keeper-pam-diagnose/1.0', + }, + proxies=proxies, + verify='/path/to/ca.pem', + timeout=10, + stream=True, + ) diff --git a/unit-tests/service/test_command.py b/unit-tests/service/test_command.py index 2df0637b1..de0736de8 100644 --- a/unit-tests/service/test_command.py +++ b/unit-tests/service/test_command.py @@ -1,10 +1,40 @@ +import json import unittest from unittest import TestCase, mock from flask import Flask +from keepercommander import params as params_module, vault +from keepercommander.subfolder import RootFolderNode, SharedFolderNode from keepercommander.service.util.command_util import CommandExecutor from keepercommander.service.util.exceptions import CommandExecutionError from keepercommander.service.util.parse_keeper_response import parse_keeper_response +from keepercommander.service.util.protected_records import get_protected_record_uids + +PROTECTED_TITLE = 'Commander Service Mode Config' +PROTECTED_UID = 'PROTECTED_CONFIG_UID' +NORMAL_UID = 'NORMAL_RECORD_UID' + + +def _record_cache_entry(uid, title): + return { + 'record_uid': uid, + 'version': 2, + 'revision': 1, + 'client_modified_time': 0, + 'shared': False, + 'record_key_unencrypted': b'0' * 32, + 'data_unencrypted': json.dumps({'title': title}).encode('utf-8'), + } + + +def _params_with_protected_and_normal_record(): + p = params_module.KeeperParams() + p.service_mode = False + p.record_cache = { + PROTECTED_UID: _record_cache_entry(PROTECTED_UID, PROTECTED_TITLE), + NORMAL_UID: _record_cache_entry(NORMAL_UID, 'My Normal Record'), + } + return p class TestCommandAPI(TestCase): def setUp(self): @@ -115,4 +145,461 @@ def test_integration_command_flow(self): response, status_code = CommandExecutor.execute(test_command) self.assertEqual(status_code, 200) - self.assertIsNotNone(response) \ No newline at end of file + self.assertIsNotNone(response) + + +class TestProtectedRecordCommandExecution(TestCase): + """End-to-end proof, through CommandExecutor.execute, that Service Mode's own config record stays blocked while unrelated records work.""" + + def _run(self, command, params=None): + params = params or _params_with_protected_and_normal_record() + with mock.patch( + 'keepercommander.service.core.globals.ensure_params_loaded', return_value=params + ), mock.patch.object( + CommandExecutor, 'capture_output_and_logs', return_value=('ok', 'ok', '') + ) as mock_capture: + response, status_code = CommandExecutor.execute(command) + return response, status_code, mock_capture, params + + def test_blocked_commands_never_reach_cli_dispatch(self): + for command in ( + f'get {PROTECTED_UID}', + f'get "{PROTECTED_TITLE}"', + f'list {PROTECTED_UID}', + f'search {PROTECTED_UID}', + f'record-update --record {PROTECTED_UID} title=x', + f'record-update --record={PROTECTED_UID} title=x', + f'rm {PROTECTED_UID}', + f'share-record {PROTECTED_UID} --email a@b.com', + f'share-folder --record {PROTECTED_UID} -e a@b.com', + f'share-folder --record={PROTECTED_UID} -e a@b.com', + f'ls "{PROTECTED_TITLE}"', + f'tree "{PROTECTED_TITLE}"', + f'nsf-get {PROTECTED_UID}', + # Not one of the commands anyone would think to curate a list around -- + # proves the check isn't gated by command name at all. + f'keep-alive {PROTECTED_UID}', + ): + with self.subTest(command=command): + response, status_code, mock_capture, _ = self._run(command) + self.assertEqual(status_code, 403) + self.assertEqual(response.get('status'), 'error') + mock_capture.assert_not_called() + + def test_unrelated_record_commands_are_unaffected(self): + for command in ( + f'get {NORMAL_UID}', + f'list {NORMAL_UID}', + f'search {NORMAL_UID}', + f'record-update --record {NORMAL_UID} title=x', + f'rm {NORMAL_UID}', + f'share-record {NORMAL_UID} --email a@b.com', + ): + with self.subTest(command=command): + response, status_code, mock_capture, _ = self._run(command) + self.assertEqual(status_code, 200) + mock_capture.assert_called_once() + + def test_record_cache_is_popped_during_dispatch_and_restored_after(self): + params = _params_with_protected_and_normal_record() + seen_during_call = {} + + def fake_capture(p, command): + seen_during_call['keys'] = set(p.record_cache.keys()) + return 'ok', 'ok', '' + + with mock.patch( + 'keepercommander.service.core.globals.ensure_params_loaded', return_value=params + ), mock.patch.object(CommandExecutor, 'capture_output_and_logs', side_effect=fake_capture): + response, status_code = CommandExecutor.execute(f'get {NORMAL_UID}') + + self.assertEqual(status_code, 200) + self.assertNotIn(PROTECTED_UID, seen_during_call['keys']) + self.assertIn(NORMAL_UID, seen_during_call['keys']) + # Restored after the call returns -- the shared params singleton must + # not stay permanently blind to its own config record. + self.assertIn(PROTECTED_UID, params.record_cache) + + def test_record_cache_restored_even_if_dispatch_raises(self): + params = _params_with_protected_and_normal_record() + with mock.patch( + 'keepercommander.service.core.globals.ensure_params_loaded', return_value=params + ), mock.patch.object( + CommandExecutor, 'capture_output_and_logs', side_effect=CommandExecutionError('boom') + ): + response, status_code = CommandExecutor.execute(f'get {NORMAL_UID}') + + self.assertEqual(status_code, 400) + self.assertIn(PROTECTED_UID, params.record_cache) + + def test_protected_record_check_runs_for_every_command(self): + """The check is unconditional -- even a command with no known relationship + to records (whoami) still triggers it, so no command can be missed.""" + params = _params_with_protected_and_normal_record() + with mock.patch( + 'keepercommander.service.core.globals.ensure_params_loaded', return_value=params + ), mock.patch.object( + CommandExecutor, 'capture_output_and_logs', return_value=('ok', 'ok', '') + ), mock.patch( + 'keepercommander.service.util.command_util.get_protected_record_uids', + wraps=get_protected_record_uids, + ) as mock_get_uids: + CommandExecutor.execute('whoami') + mock_get_uids.assert_called_once() + + def test_sailpoint_handling_runs_inside_the_record_cache_guard(self): + """SailPoint's own pre-processing (handle_command) can resolve/act on + records before cli.do_command ever runs -- it must run with the guard + already active, not before it, or a folder/recursive share under + SailPoint mode could reach the protected record before it's hidden.""" + params = _params_with_protected_and_normal_record() + seen_during_handle_command = {} + + def fake_handle_command(p, command): + seen_during_handle_command['keys'] = set(p.record_cache.keys()) + return command, None + + with mock.patch( + 'keepercommander.service.core.globals.ensure_params_loaded', return_value=params + ), mock.patch.dict('os.environ', {'SAILPOINT_RECORD': 'sailpoint-uid'}), mock.patch( + 'keepercommander.service.commands.integrations.sailpoint.service.SailPointService.handle_command', + side_effect=fake_handle_command, + ), mock.patch.object( + CommandExecutor, 'capture_output_and_logs', return_value=('ok', 'ok', '') + ): + CommandExecutor.execute(f'get {NORMAL_UID}') + + self.assertNotIn(PROTECTED_UID, seen_during_handle_command['keys']) + self.assertIn(NORMAL_UID, seen_during_handle_command['keys']) + # Restored after the whole guarded block exits, same as the non-SailPoint case. + self.assertIn(PROTECTED_UID, params.record_cache) + + +class TestSyncDownExemptionCommandExecution(TestCase): + """slack-app-setup --sync-down must keep working for its OWN config record while every + other protected record (including a different integration's) stays blocked.""" + + SLACK_UID = 'SLACK_CONFIG_UID' + GCHAT_UID = 'GCHAT_CONFIG_UID' + + def _params(self): + p = params_module.KeeperParams() + p.service_mode = False + p.record_cache = { + self.SLACK_UID: _record_cache_entry(self.SLACK_UID, 'Commander Service Mode Slack App Config'), + self.GCHAT_UID: _record_cache_entry(self.GCHAT_UID, 'Commander Service Mode Google Chat App Config'), + NORMAL_UID: _record_cache_entry(NORMAL_UID, 'My Normal Record'), + } + return p + + def _run(self, command, params, capture_side_effect=None): + env = {'SLACK_RECORD': self.SLACK_UID, 'GCHAT_RECORD': self.GCHAT_UID} + with mock.patch.dict('os.environ', env), mock.patch( + 'keepercommander.service.core.globals.ensure_params_loaded', return_value=params + ), mock.patch.object( + CommandExecutor, 'capture_output_and_logs', + side_effect=capture_side_effect, return_value=('ok', 'ok', '') if capture_side_effect is None else None, + ) as mock_capture: + response, status_code = CommandExecutor.execute(command) + return response, status_code, mock_capture + + def test_get_slack_record_is_blocked(self): + params = self._params() + response, status_code, mock_capture = self._run(f'get {self.SLACK_UID}', params) + self.assertEqual(status_code, 403) + mock_capture.assert_not_called() + + def test_slack_sync_down_default_flow_is_not_blocked_and_sees_its_own_record(self): + params = self._params() + seen = {} + + def fake_capture(p, command): + seen['keys'] = set(p.record_cache.keys()) + return 'ok', 'ok', '' + + response, status_code, mock_capture = self._run( + 'slack-app-setup --sync-down', params, capture_side_effect=fake_capture + ) + self.assertEqual(status_code, 200) + mock_capture.assert_called_once() + self.assertIn(self.SLACK_UID, seen['keys']) # not hidden for this one dispatch + self.assertIn(self.GCHAT_UID, params.record_cache) # untouched throughout + + def test_slack_sync_down_with_explicit_own_uid_is_not_blocked(self): + params = self._params() + response, status_code, mock_capture = self._run( + f'slack-app-setup --sync-down -r {self.SLACK_UID}', params + ) + self.assertEqual(status_code, 200) + mock_capture.assert_called_once() + + def test_slack_sync_down_with_a_different_integrations_uid_is_still_blocked(self): + params = self._params() + response, status_code, mock_capture = self._run( + f'slack-app-setup --sync-down -r {self.GCHAT_UID}', params + ) + self.assertEqual(status_code, 403) + mock_capture.assert_not_called() + + def test_slack_setup_without_sync_down_gets_no_exemption(self): + """The main (non --sync-down) flow must not get the record cache exemption + just because it names the record's own UID.""" + params = self._params() + response, status_code, mock_capture = self._run( + f'slack-app-setup --slack-record-name {self.SLACK_UID}', params + ) + self.assertEqual(status_code, 403) + mock_capture.assert_not_called() + + def test_gchat_sync_down_default_flow_is_not_blocked_and_sees_its_own_record(self): + """Same as Slack's own-record flow, but for GChat -- proves the exemption isn't Slack-specific.""" + params = self._params() + seen = {} + + def fake_capture(p, command): + seen['keys'] = set(p.record_cache.keys()) + return 'ok', 'ok', '' + + response, status_code, mock_capture = self._run( + 'gchat-app-setup --sync-down', params, capture_side_effect=fake_capture + ) + self.assertEqual(status_code, 200) + mock_capture.assert_called_once() + self.assertIn(self.GCHAT_UID, seen['keys']) + self.assertIn(self.SLACK_UID, params.record_cache) + + def test_gchat_sync_down_with_a_different_integrations_uid_is_still_blocked(self): + params = self._params() + response, status_code, mock_capture = self._run( + f'gchat-app-setup --sync-down -r {self.SLACK_UID}', params + ) + self.assertEqual(status_code, 403) + mock_capture.assert_not_called() + + +class TestTerraformRecordProtectionCommandExecution(TestCase): + """Same UID-pinning protection Docker/Slack/GChat get, proven at the CommandExecutor.execute() boundary.""" + + TERRAFORM_UID = 'TERRAFORM_CONFIG_UID' + + def _params(self): + p = params_module.KeeperParams() + p.service_mode = False + p.record_cache = { + self.TERRAFORM_UID: _record_cache_entry(self.TERRAFORM_UID, 'Commander Service Mode Terraform Config'), + NORMAL_UID: _record_cache_entry(NORMAL_UID, 'My Normal Record'), + } + return p + + def _run(self, command, params): + with mock.patch.dict('os.environ', {'TERRAFORM_RECORD': self.TERRAFORM_UID}), mock.patch( + 'keepercommander.service.core.globals.ensure_params_loaded', return_value=params + ), mock.patch.object( + CommandExecutor, 'capture_output_and_logs', return_value=('ok', 'ok', '') + ) as mock_capture: + response, status_code = CommandExecutor.execute(command) + return response, status_code, mock_capture + + def test_get_terraform_record_is_blocked(self): + params = self._params() + response, status_code, mock_capture = self._run(f'get {self.TERRAFORM_UID}', params) + self.assertEqual(status_code, 403) + mock_capture.assert_not_called() + + def test_share_record_on_terraform_record_is_blocked(self): + params = self._params() + response, status_code, mock_capture = self._run( + f'share-record {self.TERRAFORM_UID} --email a@b.com', params + ) + self.assertEqual(status_code, 403) + mock_capture.assert_not_called() + + def test_normal_record_is_unaffected(self): + params = self._params() + response, status_code, mock_capture = self._run(f'get {NORMAL_UID}', params) + self.assertEqual(status_code, 200) + mock_capture.assert_called_once() + +class TestProtectedFolderCommandExecution(TestCase): + """The shared folder holding a protected config record must be just as unreachable + as the record itself -- ls/tree/rndir/mv/share-folder all resolve folders through the + same caches hide_from_folder_cache guards.""" + + PROTECTED_FOLDER_UID = 'PROTECTED_FOLDER_UID' + PROTECTED_FOLDER_TITLE = 'Commander Service Mode - Docker' + NORMAL_FOLDER_UID = 'NORMAL_FOLDER_UID' + + def _params(self): + p = params_module.KeeperParams() + p.service_mode = False + p.record_cache = { + PROTECTED_UID: _record_cache_entry(PROTECTED_UID, PROTECTED_TITLE), + NORMAL_UID: _record_cache_entry(NORMAL_UID, 'My Normal Record'), + } + p.root_folder = RootFolderNode() + + protected_node = SharedFolderNode() + protected_node.uid = self.PROTECTED_FOLDER_UID + protected_node.name = self.PROTECTED_FOLDER_TITLE + normal_node = SharedFolderNode() + normal_node.uid = self.NORMAL_FOLDER_UID + normal_node.name = 'My Normal Folder' + + p.folder_cache = {self.PROTECTED_FOLDER_UID: protected_node, self.NORMAL_FOLDER_UID: normal_node} + p.root_folder.subfolders = [self.PROTECTED_FOLDER_UID, self.NORMAL_FOLDER_UID] + p.shared_folder_cache = { + self.PROTECTED_FOLDER_UID: {'name_unencrypted': self.PROTECTED_FOLDER_TITLE}, + self.NORMAL_FOLDER_UID: {'name_unencrypted': 'My Normal Folder'}, + } + p.subfolder_cache = { + self.PROTECTED_FOLDER_UID: {'type': 'shared_folder', 'shared_folder_uid': self.PROTECTED_FOLDER_UID}, + self.NORMAL_FOLDER_UID: {'type': 'shared_folder', 'shared_folder_uid': self.NORMAL_FOLDER_UID}, + } + p.subfolder_record_cache = { + self.PROTECTED_FOLDER_UID: {PROTECTED_UID}, + self.NORMAL_FOLDER_UID: {NORMAL_UID}, + } + return p + + def _run(self, command, params): + with mock.patch( + 'keepercommander.service.core.globals.ensure_params_loaded', return_value=params + ), mock.patch.object( + CommandExecutor, 'capture_output_and_logs', return_value=('ok', 'ok', '') + ) as mock_capture: + response, status_code = CommandExecutor.execute(command) + return response, status_code, mock_capture + + def test_blocked_folder_commands_never_reach_cli_dispatch(self): + """Layer B's literal-token scan is UID-only for folders (by design -- see the plan); + a title-only reference isn't caught here, it fails to resolve at all once Layer A + hides the folder from folder_cache/shared_folder_cache, proven separately in + TestHideFromFolderCache and test_folder_caches_hidden_during_dispatch_and_restored_after.""" + for command in ( + f'ls {self.PROTECTED_FOLDER_UID}', + f'tree {self.PROTECTED_FOLDER_UID}', + f'rndir {self.PROTECTED_FOLDER_UID} x', + f'mv {self.PROTECTED_FOLDER_UID} /', + f'share-folder {self.PROTECTED_FOLDER_UID} -e a@b.com', + ): + with self.subTest(command=command): + params = self._params() + response, status_code, mock_capture = self._run(command, params) + self.assertEqual(status_code, 403) + mock_capture.assert_not_called() + + def test_normal_folder_commands_are_unaffected(self): + params = self._params() + response, status_code, mock_capture = self._run(f'ls {self.NORMAL_FOLDER_UID}', params) + self.assertEqual(status_code, 200) + mock_capture.assert_called_once() + + def test_folder_caches_hidden_during_dispatch_and_restored_after(self): + params = self._params() + seen = {} + + def fake_capture(p, command): + seen['folder_keys'] = set(p.folder_cache.keys()) + seen['subfolders'] = list(p.root_folder.subfolders) + return 'ok', 'ok', '' + + with mock.patch( + 'keepercommander.service.core.globals.ensure_params_loaded', return_value=params + ), mock.patch.object(CommandExecutor, 'capture_output_and_logs', side_effect=fake_capture): + response, status_code = CommandExecutor.execute(f'ls {self.NORMAL_FOLDER_UID}') + + self.assertEqual(status_code, 200) + self.assertNotIn(self.PROTECTED_FOLDER_UID, seen['folder_keys']) + self.assertIn(self.NORMAL_FOLDER_UID, seen['folder_keys']) + self.assertNotIn(self.PROTECTED_FOLDER_UID, seen['subfolders']) + + self.assertIn(self.PROTECTED_FOLDER_UID, params.folder_cache) + self.assertIn(self.PROTECTED_FOLDER_UID, params.root_folder.subfolders) + + +class TestReservedAttachmentCommandExecution(TestCase): + """A record with an arbitrary, non-default title must still be blocked if it carries + one of Commander's own reserved config-file attachments (config.json/service_config.json).""" + + ARBITRARY_UID = 'ARBITRARY_TITLED_RECORD_UID' + + def _params(self): + p = params_module.KeeperParams() + p.service_mode = False + p.record_cache = { + self.ARBITRARY_UID: _record_cache_entry(self.ARBITRARY_UID, 'My Totally Unrelated Title'), + NORMAL_UID: _record_cache_entry(NORMAL_UID, 'My Normal Record'), + } + return p + + @staticmethod + def _record_with_reserved_attachment(uid, title): + record = vault.PasswordRecord() + record.record_uid = uid + record.title = title + record.attachments = [vault.AttachmentFile({'id': f'{uid}_ATTA', 'name': 'config.json'})] + return record + + @staticmethod + def _plain_record(uid, title): + record = vault.PasswordRecord() + record.record_uid = uid + record.title = title + return record + + _TITLES = {ARBITRARY_UID: 'My Totally Unrelated Title', NORMAL_UID: 'My Normal Record'} + + def _run(self, command, params, reserved_uids=(ARBITRARY_UID,)): + records = { + uid: ( + self._record_with_reserved_attachment(uid, self._TITLES[uid]) + if uid in reserved_uids else self._plain_record(uid, self._TITLES[uid]) + ) + for uid in params.record_cache + } + with mock.patch( + 'keepercommander.service.core.globals.ensure_params_loaded', return_value=params + ), mock.patch( + 'keepercommander.vault.KeeperRecord.load', side_effect=lambda p, uid: records.get(uid) + ), mock.patch.object( + CommandExecutor, 'capture_output_and_logs', return_value=('ok', 'ok', '') + ) as mock_capture: + response, status_code = CommandExecutor.execute(command) + return response, status_code, mock_capture + + def test_get_on_record_with_reserved_attachment_is_blocked(self): + params = self._params() + response, status_code, mock_capture = self._run(f'get {self.ARBITRARY_UID}', params) + self.assertEqual(status_code, 403) + mock_capture.assert_not_called() + + def test_file_report_omits_record_with_reserved_attachment(self): + params = self._params() + records = { + self.ARBITRARY_UID: self._record_with_reserved_attachment( + self.ARBITRARY_UID, self._TITLES[self.ARBITRARY_UID] + ), + NORMAL_UID: self._plain_record(NORMAL_UID, self._TITLES[NORMAL_UID]), + } + seen = {} + + def fake_capture(p, command): + seen['keys'] = set(p.record_cache.keys()) + return 'ok', 'ok', '' + + with mock.patch( + 'keepercommander.service.core.globals.ensure_params_loaded', return_value=params + ), mock.patch( + 'keepercommander.vault.KeeperRecord.load', side_effect=lambda p, uid: records.get(uid) + ), mock.patch.object(CommandExecutor, 'capture_output_and_logs', side_effect=fake_capture): + response, status_code = CommandExecutor.execute('file-report') + + self.assertEqual(status_code, 200) + self.assertNotIn(self.ARBITRARY_UID, seen['keys']) + self.assertIn(NORMAL_UID, seen['keys']) + + def test_record_without_reserved_attachment_is_unaffected(self): + params = self._params() + response, status_code, mock_capture = self._run(f'get {NORMAL_UID}', params, reserved_uids=()) + self.assertEqual(status_code, 200) + mock_capture.assert_called_once() diff --git a/unit-tests/service/test_create_service.py b/unit-tests/service/test_create_service.py index c6ca781a8..04c485fb3 100644 --- a/unit-tests/service/test_create_service.py +++ b/unit-tests/service/test_create_service.py @@ -39,7 +39,7 @@ def test_execute_service_already_running(self, mock_service_manager): def test_handle_configuration_streamlined(self): """Test streamlined configuration handling.""" config_data = self.command.service_config.create_default_config() - args = StreamlineArgs(port=8080, commands='record-list', ngrok=None, allowedip='0.0.0.0' ,deniedip='', ngrok_custom_domain=None, cloudflare=None, cloudflare_custom_domain=None, certfile='', certpassword='', fileformat='json', run_mode='foreground', queue_enabled='y', update_vault_record=None, ratelimit=None, encryption_key=None, token_expiration=None) + args = StreamlineArgs(port=8080, commands='record-list', ngrok=None, allowedip='0.0.0.0' ,deniedip='', ngrok_custom_domain=None, cloudflare=None, cloudflare_custom_domain=None, tailscale=None, tailscale_advertise_tags=None, certfile='', certpassword='', fileformat='json', run_mode='foreground', queue_enabled='y', update_vault_record=None, ratelimit=None, encryption_key=None, token_expiration=None) with patch.object(self.command.config_handler, 'handle_streamlined_config') as mock_streamlined: self.command._handle_configuration(config_data, self.params, args) @@ -48,7 +48,7 @@ def test_handle_configuration_streamlined(self): def test_handle_configuration_interactive(self): """Test interactive configuration handling.""" config_data = self.command.service_config.create_default_config() - args = StreamlineArgs(port=None, commands=None, ngrok=None, allowedip='' ,deniedip='', ngrok_custom_domain=None, cloudflare=None, cloudflare_custom_domain=None, certfile='', certpassword='', fileformat='json', run_mode='foreground', queue_enabled=None, update_vault_record=None, ratelimit=None, encryption_key=None, token_expiration=None) + args = StreamlineArgs(port=None, commands=None, ngrok=None, allowedip='' ,deniedip='', ngrok_custom_domain=None, cloudflare=None, cloudflare_custom_domain=None, tailscale=None, tailscale_advertise_tags=None, certfile='', certpassword='', fileformat='json', run_mode='foreground', queue_enabled=None, update_vault_record=None, ratelimit=None, encryption_key=None, token_expiration=None) with patch.object(self.command.config_handler, 'handle_interactive_config') as mock_interactive, \ patch.object(self.command.security_handler, 'configure_security') as mock_security: @@ -59,7 +59,7 @@ def test_handle_configuration_interactive(self): def test_create_and_save_record(self): """Test record creation and saving.""" config_data = self.command.service_config.create_default_config() - args = StreamlineArgs(port=8080, commands='record-list', ngrok=None, allowedip='0.0.0.0' ,deniedip='', ngrok_custom_domain=None, cloudflare=None, cloudflare_custom_domain=None, certfile='', certpassword='', fileformat='json', run_mode='foreground', queue_enabled='y', update_vault_record=None, ratelimit=None, encryption_key=None, token_expiration=None) + args = StreamlineArgs(port=8080, commands='record-list', ngrok=None, allowedip='0.0.0.0' ,deniedip='', ngrok_custom_domain=None, cloudflare=None, cloudflare_custom_domain=None, tailscale=None, tailscale_advertise_tags=None, certfile='', certpassword='', fileformat='json', run_mode='foreground', queue_enabled='y', update_vault_record=None, ratelimit=None, encryption_key=None, token_expiration=None) with patch.object(self.command.service_config, 'create_record') as mock_create_record, \ patch.object(self.command.service_config, 'save_config') as mock_save_config: @@ -82,7 +82,7 @@ def test_create_and_save_record(self): def test_validation_error_handling(self): """Test handling of validation errors during execution.""" - args = StreamlineArgs(port=-1, commands='record-list', ngrok=None, allowedip='0.0.0.0' ,deniedip='', ngrok_custom_domain=None, cloudflare=None, cloudflare_custom_domain=None, certfile='', certpassword='', fileformat='json', run_mode='foreground', queue_enabled='y', update_vault_record=None, ratelimit=None, encryption_key=None, token_expiration=None) + args = StreamlineArgs(port=-1, commands='record-list', ngrok=None, allowedip='0.0.0.0' ,deniedip='', ngrok_custom_domain=None, cloudflare=None, cloudflare_custom_domain=None, tailscale=None, tailscale_advertise_tags=None, certfile='', certpassword='', fileformat='json', run_mode='foreground', queue_enabled='y', update_vault_record=None, ratelimit=None, encryption_key=None, token_expiration=None) with patch('builtins.print') as mock_print: with patch.object(self.command.service_config, 'create_default_config') as mock_create_config: @@ -103,6 +103,8 @@ def test_cloudflare_streamlined_configuration(self): ngrok_custom_domain=None, cloudflare='cf_token123', cloudflare_custom_domain='tunnel.example.com', + tailscale=None, + tailscale_advertise_tags=None, certfile='', certpassword='', fileformat='json', @@ -129,6 +131,8 @@ def test_cloudflare_validation_missing_token(self): ngrok_custom_domain=None, cloudflare=None, cloudflare_custom_domain='tunnel.example.com', + tailscale=None, + tailscale_advertise_tags=None, certfile='', certpassword='', fileformat='json', @@ -157,6 +161,8 @@ def test_cloudflare_validation_missing_domain(self): ngrok_custom_domain=None, cloudflare='cf_token123', cloudflare_custom_domain=None, + tailscale=None, + tailscale_advertise_tags=None, certfile='', certpassword='', fileformat='json', @@ -185,6 +191,8 @@ def test_cloudflare_and_ngrok_mutual_exclusion(self): ngrok_custom_domain='ngrok.example.com', cloudflare='cf_token123', cloudflare_custom_domain='tunnel.example.com', + tailscale=None, + tailscale_advertise_tags=None, certfile='', certpassword='', fileformat='json', @@ -224,6 +232,8 @@ def test_cloudflare_tunnel_startup_success(self, mock_cloudflare_configure): ngrok_custom_domain=None, cloudflare='cf_token123', cloudflare_custom_domain='tunnel.example.com', + tailscale=None, + tailscale_advertise_tags=None, certfile='', certpassword='', fileformat='json', @@ -274,6 +284,8 @@ def test_cloudflare_tunnel_startup_failure(self, mock_get_status, mock_start_ser ngrok_custom_domain=None, cloudflare='cf_token123', cloudflare_custom_domain='tunnel.example.com', + tailscale=None, + tailscale_advertise_tags=None, certfile='', certpassword='', fileformat='json', @@ -300,6 +312,8 @@ def test_cloudflare_token_validation(self): ngrok_custom_domain=None, cloudflare='eyJhIjoiYWJjZGVmZ2hpams', # Base64-like token cloudflare_custom_domain='tunnel.example.com', + tailscale=None, + tailscale_advertise_tags=None, certfile='', certpassword='', fileformat='json', @@ -328,6 +342,8 @@ def test_cloudflare_domain_validation(self): ngrok_custom_domain=None, cloudflare='cf_token123', cloudflare_custom_domain='my-tunnel.example.com', + tailscale=None, + tailscale_advertise_tags=None, certfile='', certpassword='', fileformat='json', @@ -344,5 +360,133 @@ def test_cloudflare_domain_validation(self): self.command._handle_configuration(config_data, self.params, args) mock_streamlined.assert_called_once_with(config_data, args, self.params) + def test_get_parser_tailscale(self): + """Test that -ts alone carries the auth key value, with no separate -tsk flag.""" + parser = self.command.get_parser() + + args = parser.parse_args(['--tailscale', 'tskey-auth-dummy']) + self.assertEqual(args.tailscale, 'tskey-auth-dummy') + self.assertFalse(hasattr(args, 'tailscale_auth_key')) + + args = parser.parse_args(['-ts', 'tskey-auth-dummy', '-tst', 'tag:commander-service']) + self.assertEqual(args.tailscale, 'tskey-auth-dummy') + self.assertEqual(args.tailscale_advertise_tags, 'tag:commander-service') + + def test_tailscale_streamlined_configuration(self): + """Test streamlined configuration with Tailscale, -ts alone enabling it.""" + config_data = self.command.service_config.create_default_config() + args = StreamlineArgs( + port=8080, + commands='record-list', + ngrok=None, + allowedip='0.0.0.0', + deniedip='', + ngrok_custom_domain=None, + cloudflare=None, + cloudflare_custom_domain=None, + tailscale='tskey-auth-dummy', + tailscale_advertise_tags=None, + certfile='', + certpassword='', + fileformat='json', + run_mode='foreground', + queue_enabled='y', + update_vault_record=None, + ratelimit=None, + encryption_key=None, + token_expiration=None + ) + + self.command.config_handler.handle_streamlined_config(config_data, args, self.params) + self.assertEqual(config_data['tailscale'], 'y') + self.assertEqual(config_data['tailscale_auth_key'], 'tskey-auth-dummy') + + def test_tailscale_advertise_tags_streamlined(self): + """Test that -tst is threaded through to the internal config alongside -ts.""" + config_data = self.command.service_config.create_default_config() + args = StreamlineArgs( + port=8080, + commands='record-list', + ngrok=None, + allowedip='0.0.0.0', + deniedip='', + ngrok_custom_domain=None, + cloudflare=None, + cloudflare_custom_domain=None, + tailscale='tskey-client-dummy', + tailscale_advertise_tags='tag:commander-service', + certfile='', + certpassword='', + fileformat='json', + run_mode='foreground', + queue_enabled='y', + update_vault_record=None, + ratelimit=None, + encryption_key=None, + token_expiration=None + ) + + self.command.config_handler.handle_streamlined_config(config_data, args, self.params) + self.assertEqual(config_data['tailscale_advertise_tags'], 'tag:commander-service') + + def test_tailscale_omitted_disables_it(self): + """Test that omitting -ts disables Tailscale without requiring any other flag.""" + config_data = self.command.service_config.create_default_config() + args = StreamlineArgs( + port=8080, + commands='record-list', + ngrok=None, + allowedip='0.0.0.0', + deniedip='', + ngrok_custom_domain=None, + cloudflare=None, + cloudflare_custom_domain=None, + tailscale=None, + tailscale_advertise_tags=None, + certfile='', + certpassword='', + fileformat='json', + run_mode='foreground', + queue_enabled='y', + update_vault_record=None, + ratelimit=None, + encryption_key=None, + token_expiration=None + ) + + self.command.config_handler.handle_streamlined_config(config_data, args, self.params) + self.assertEqual(config_data['tailscale'], 'n') + self.assertEqual(config_data['tailscale_auth_key'], '') + + def test_tailscale_and_ngrok_mutual_exclusion(self): + """Test that Ngrok takes priority and disables Tailscale, matching the Cloudflare/Ngrok exclusion pattern.""" + config_data = self.command.service_config.create_default_config() + args = StreamlineArgs( + port=8080, + commands='record-list', + ngrok='ngrok_token123', + allowedip='0.0.0.0', + deniedip='', + ngrok_custom_domain='ngrok.example.com', + cloudflare=None, + cloudflare_custom_domain=None, + tailscale='tskey-auth-dummy', + tailscale_advertise_tags=None, + certfile='', + certpassword='', + fileformat='json', + run_mode='foreground', + queue_enabled='y', + update_vault_record=None, + ratelimit=None, + encryption_key=None, + token_expiration=None + ) + + self.command.config_handler.handle_streamlined_config(config_data, args, self.params) + self.assertEqual(config_data['ngrok'], 'y') + self.assertEqual(config_data['tailscale'], 'n') + self.assertEqual(config_data['tailscale_auth_key'], '') + if __name__ == '__main__': unittest.main() \ No newline at end of file diff --git a/unit-tests/service/test_min_commander_version.py b/unit-tests/service/test_min_commander_version.py index 6e0aa90fd..eb69fb5fc 100644 --- a/unit-tests/service/test_min_commander_version.py +++ b/unit-tests/service/test_min_commander_version.py @@ -18,6 +18,7 @@ from keepercommander.service.decorators.min_commander_version import ( MIN_COMMANDER_VERSION_HEADER, TERRAFORM_DOCKER_ENV, + TERRAFORM_DOCKER_ENV_LEGACY, check_min_commander_version, min_commander_version_check, _parse_version, @@ -166,7 +167,10 @@ def test_rejects_when_running_version_unparseable(self): '17.0.0', ) def test_non_terraform_docker_ignores_min_version_header(self): - env = {k: v for k, v in os.environ.items() if k != TERRAFORM_DOCKER_ENV} + env = { + k: v for k, v in os.environ.items() + if k not in (TERRAFORM_DOCKER_ENV, TERRAFORM_DOCKER_ENV_LEGACY) + } with mock.patch.dict(os.environ, env, clear=True): with self.app.test_request_context( '/api/v2/executecommand-async', @@ -175,6 +179,26 @@ def test_non_terraform_docker_ignores_min_version_header(self): ): self.assertIsNone(check_min_commander_version()) + @mock.patch.dict(os.environ, {TERRAFORM_DOCKER_ENV_LEGACY: '1'}, clear=True) + @mock.patch( + 'keepercommander.service.decorators.min_commander_version._RUNNING_VERSION', + Version('17.0.0'), + ) + @mock.patch( + 'keepercommander.service.decorators.min_commander_version._RUNNING_VERSION_RAW', + '17.0.0', + ) + def test_legacy_terraform_env_var_still_enforces(self): + """A container upgraded without re-running terraform-app-setup still has the old + KEEPER_TERRAFORM marker -- enforcement must not silently disable itself.""" + with self.app.test_request_context( + '/api/v2/executecommand-async', + method='POST', + headers={MIN_COMMANDER_VERSION_HEADER: '18.1.0'}, + ): + body, status = check_min_commander_version() + self.assertEqual(status, 426) + @mock.patch.dict(os.environ, _TERRAFORM_DOCKER_ENV, clear=False) @mock.patch( 'keepercommander.service.decorators.min_commander_version._RUNNING_VERSION', diff --git a/unit-tests/service/test_protected_records.py b/unit-tests/service/test_protected_records.py new file mode 100644 index 000000000..9fe2a6940 --- /dev/null +++ b/unit-tests/service/test_protected_records.py @@ -0,0 +1,607 @@ +import json +import os +from unittest import TestCase, mock + +from keepercommander import params as params_module, vault +from keepercommander.subfolder import RootFolderNode, SharedFolderNode +from keepercommander.utils import generate_uid +from keepercommander.service.util.protected_records import ( + _attachment_file_uids, + _has_reserved_legacy_attachment, + get_protected_folder_uids, + get_protected_record_title_set, + get_protected_record_uids, + hide_from_folder_cache, + hide_from_record_cache, + resolve_sync_down_exempt_uid, +) + +PROTECTED_TITLE = 'Commander Service Mode Config' +PROTECTED_DOCKER_TITLE = 'Commander Service Mode Docker Config' + + +def _record_cache_entry(uid, title, version=2): + return { + 'record_uid': uid, + 'version': version, + 'revision': 1, + 'client_modified_time': 0, + 'shared': False, + 'record_key_unencrypted': b'0' * 32, + 'data_unencrypted': json.dumps({'title': title}).encode('utf-8'), + } + + +def _params_with_records(entries): + p = params_module.KeeperParams() + p.record_cache = {uid: _record_cache_entry(uid, title) for uid, title in entries.items()} + return p + + +class TestGetProtectedRecordTitleSet(TestCase): + def test_contains_expected_titles_lowercased(self): + titles = get_protected_record_title_set() + self.assertIn('commander service mode config', titles) + self.assertIn('commander service mode docker config', titles) + self.assertIn('commander service mode', titles) + + def test_contains_terraform_slack_teams_gchat_titles(self): + titles = get_protected_record_title_set() + self.assertIn('commander service mode terraform config', titles) + self.assertIn('commander service mode slack app config', titles) + self.assertIn('commander service mode teams app config', titles) + self.assertIn('commander service mode google chat app config', titles) + + +class TestGetProtectedRecordUids(TestCase): + def test_returns_only_matching_protected_records(self): + p = _params_with_records({ + 'UID_CONFIG': PROTECTED_TITLE, + 'UID_DOCKER': PROTECTED_DOCKER_TITLE, + 'UID_OTHER': 'My Normal Record', + }) + result = get_protected_record_uids(p) + self.assertEqual(set(result.keys()), {'UID_CONFIG', 'UID_DOCKER'}) + self.assertEqual(result['UID_CONFIG'], PROTECTED_TITLE) + self.assertEqual(result['UID_DOCKER'], PROTECTED_DOCKER_TITLE) + + def test_no_protected_records_present(self): + p = _params_with_records({'UID_OTHER': 'My Normal Record'}) + self.assertEqual(get_protected_record_uids(p), {}) + + def test_empty_record_cache(self): + p = params_module.KeeperParams() + p.record_cache = {} + self.assertEqual(get_protected_record_uids(p), {}) + + def test_params_none(self): + self.assertEqual(get_protected_record_uids(None), {}) + + def test_record_cache_not_a_dict_is_ignored(self): + """A Mock/non-dict record_cache (as some tests construct) must fail safe to {}.""" + class FakeParams: + record_cache = object() + + self.assertEqual(get_protected_record_uids(FakeParams()), {}) + + def test_similar_but_not_exact_title_is_not_matched(self): + """A title that shares every token with a protected title but isn't an exact match must not be protected.""" + p = _params_with_records({'UID_OTHER': 'Commander Service Mode Config Backup'}) + self.assertEqual(get_protected_record_uids(p), {}) + + def test_docker_record_protected_by_uid_even_with_custom_title(self): + """--record-name can give the Docker config record a custom title; COMMANDER_RECORD must still identify it.""" + uid = generate_uid() + with mock.patch.dict(os.environ, {'COMMANDER_RECORD': uid}): + p = _params_with_records({uid: 'My Totally Custom Docker Title'}) + result = get_protected_record_uids(p) + self.assertIn(uid, result) + + def test_docker_env_uid_present_even_without_params(self): + uid = generate_uid() + with mock.patch.dict(os.environ, {'COMMANDER_RECORD': uid}): + self.assertIn(uid, get_protected_record_uids(None)) + + def test_no_docker_env_var_falls_back_to_title_only(self): + with mock.patch.dict(os.environ, {}, clear=True): + p = _params_with_records({'UID_CONFIG': PROTECTED_TITLE}) + result = get_protected_record_uids(p) + self.assertEqual(set(result.keys()), {'UID_CONFIG'}) + + def test_malformed_env_uid_falls_back_to_title_matching(self): + """A misconfigured pinning env var (not a real record UID) must not become a phantom protected token.""" + with mock.patch.dict(os.environ, {'TERRAFORM_RECORD': '1'}): + p = _params_with_records({'UID_CONFIG': PROTECTED_TITLE}) + result = get_protected_record_uids(p) + self.assertEqual(set(result.keys()), {'UID_CONFIG'}) + + def test_terraform_record_protected_by_uid_even_with_custom_title(self): + uid = generate_uid() + with mock.patch.dict(os.environ, {'TERRAFORM_RECORD': uid}): + p = _params_with_records({uid: 'My Totally Custom Terraform Title'}) + self.assertIn(uid, get_protected_record_uids(p)) + + def test_slack_record_protected_by_uid_even_with_custom_title(self): + uid = generate_uid() + with mock.patch.dict(os.environ, {'SLACK_RECORD': uid}): + p = _params_with_records({uid: 'My Totally Custom Slack Title'}) + self.assertIn(uid, get_protected_record_uids(p)) + + def test_teams_record_protected_by_uid_even_with_custom_title(self): + uid = generate_uid() + with mock.patch.dict(os.environ, {'TEAMS_RECORD': uid}): + p = _params_with_records({uid: 'My Totally Custom Teams Title'}) + self.assertIn(uid, get_protected_record_uids(p)) + + def test_gchat_record_protected_by_uid_even_with_custom_title(self): + uid = generate_uid() + with mock.patch.dict(os.environ, {'GCHAT_RECORD': uid}): + p = _params_with_records({uid: 'My Totally Custom GChat Title'}) + self.assertIn(uid, get_protected_record_uids(p)) + + def test_no_pinned_env_vars_falls_back_to_title_only(self): + with mock.patch.dict(os.environ, {}, clear=True): + p = _params_with_records({'UID_CONFIG': PROTECTED_TITLE}) + result = get_protected_record_uids(p) + self.assertEqual(set(result.keys()), {'UID_CONFIG'}) + + def test_malformed_record_entry_is_skipped_not_raised(self): + """A record missing an expected key must not break the scan for every other record.""" + p = _params_with_records({'UID_CONFIG': PROTECTED_TITLE}) + p.record_cache['MALFORMED_UID'] = { + 'record_uid': 'MALFORMED_UID', 'version': 2, 'revision': 1, + 'data_unencrypted': json.dumps({'title': 'whatever'}).encode('utf-8'), + # record_key_unencrypted deliberately missing. + } + result = get_protected_record_uids(p) + self.assertEqual(set(result.keys()), {'UID_CONFIG'}) + + +class TestGetProtectedRecordUidsNotCached(TestCase): + """Not memoized -- a newly added protected record must be picked up immediately, without needing a revision change.""" + + def test_newly_added_record_is_found_without_a_revision_change(self): + p = _params_with_records({'UID_CONFIG': PROTECTED_TITLE}) + p.revision = 100 + first = get_protected_record_uids(p) + self.assertEqual(set(first.keys()), {'UID_CONFIG'}) + + p.record_cache['UID_DOCKER'] = _record_cache_entry('UID_DOCKER', PROTECTED_DOCKER_TITLE) + second = get_protected_record_uids(p) + self.assertEqual(set(second.keys()), {'UID_CONFIG', 'UID_DOCKER'}) + + def test_removed_record_is_no_longer_found(self): + p = _params_with_records({'UID_CONFIG': PROTECTED_TITLE, 'UID_DOCKER': PROTECTED_DOCKER_TITLE}) + p.revision = 100 + first = get_protected_record_uids(p) + self.assertEqual(set(first.keys()), {'UID_CONFIG', 'UID_DOCKER'}) + + del p.record_cache['UID_DOCKER'] + second = get_protected_record_uids(p) + self.assertEqual(set(second.keys()), {'UID_CONFIG'}) + + +class TestHideFromRecordCache(TestCase): + def test_hides_protected_uid_inside_the_block(self): + p = _params_with_records({'PROTECTED': PROTECTED_TITLE, 'NORMAL': 'Other'}) + with hide_from_record_cache(p, {'PROTECTED': PROTECTED_TITLE}): + self.assertNotIn('PROTECTED', p.record_cache) + self.assertIn('NORMAL', p.record_cache) + + def test_restores_original_entry_and_plain_dict_after_block(self): + p = _params_with_records({'PROTECTED': PROTECTED_TITLE, 'NORMAL': 'Other'}) + original_entry = p.record_cache['PROTECTED'] + with hide_from_record_cache(p, {'PROTECTED': PROTECTED_TITLE}): + pass + self.assertIs(type(p.record_cache), dict) + self.assertEqual(p.record_cache['PROTECTED'], original_entry) + + def test_restores_even_if_block_raises(self): + p = _params_with_records({'PROTECTED': PROTECTED_TITLE}) + with self.assertRaises(ValueError): + with hide_from_record_cache(p, {'PROTECTED': PROTECTED_TITLE}): + raise ValueError('boom') + self.assertIs(type(p.record_cache), dict) + self.assertIn('PROTECTED', p.record_cache) + + def test_reintroduction_during_block_is_blocked(self): + """A forced sync-down writing the protected UID back into record_cache mid-command must not make it visible.""" + p = _params_with_records({'PROTECTED': PROTECTED_TITLE, 'NORMAL': 'Other'}) + with hide_from_record_cache(p, {'PROTECTED': PROTECTED_TITLE}): + p.record_cache['PROTECTED'] = _record_cache_entry('PROTECTED', 'reintroduced') + self.assertNotIn('PROTECTED', p.record_cache) + + p.record_cache.update({'PROTECTED': _record_cache_entry('PROTECTED', 'via-update'), + 'NEW': _record_cache_entry('NEW', 'legit new record')}) + self.assertNotIn('PROTECTED', p.record_cache) + self.assertIn('NEW', p.record_cache) + + # Legit mutations made during the block survive; the protected entry is restored. + self.assertIn('NEW', p.record_cache) + self.assertIn('PROTECTED', p.record_cache) + + def test_no_protected_uids_is_a_noop(self): + p = _params_with_records({'NORMAL': 'Other'}) + with hide_from_record_cache(p, {}): + self.assertIn('NORMAL', p.record_cache) + + def test_params_none_is_a_noop(self): + with hide_from_record_cache(None, {'PROTECTED': PROTECTED_TITLE}): + pass + + def test_nested_share_caches_are_guarded_too(self): + """load_pam_record falls back to nested_share_records, so guarding record_cache alone isn't enough.""" + p = _params_with_records({'NORMAL': 'Other'}) + p.nested_share_records = {'PROTECTED': {'title': 'nsf copy'}, 'OTHER_NSF': {'title': 'x'}} + p.nested_share_record_data = {'PROTECTED': {'data_json': {'title': 'nsf data copy'}}} + + with hide_from_record_cache(p, {'PROTECTED': PROTECTED_TITLE}): + self.assertNotIn('PROTECTED', p.nested_share_records) + self.assertIn('OTHER_NSF', p.nested_share_records) + self.assertNotIn('PROTECTED', p.nested_share_record_data) + + # Reintroduction mid-command must be blocked here too. + p.nested_share_records['PROTECTED'] = {'title': 'reintroduced'} + self.assertNotIn('PROTECTED', p.nested_share_records) + + self.assertIn('PROTECTED', p.nested_share_records) + self.assertIn('PROTECTED', p.nested_share_record_data) + self.assertIs(type(p.nested_share_records), dict) + + def test_subfolder_record_cache_uid_is_stripped_for_the_duration(self): + """_build_folder_json leaks the bare UID from a folder's set even when the record fails to load.""" + p = _params_with_records({'NORMAL': 'Other'}) + p.subfolder_record_cache = {'FOLDER1': {'PROTECTED', 'NORMAL'}, 'FOLDER2': {'OTHER'}} + + with hide_from_record_cache(p, {'PROTECTED': PROTECTED_TITLE}): + self.assertNotIn('PROTECTED', p.subfolder_record_cache['FOLDER1']) + self.assertIn('NORMAL', p.subfolder_record_cache['FOLDER1']) + self.assertEqual(p.subfolder_record_cache['FOLDER2'], {'OTHER'}) + + self.assertIn('PROTECTED', p.subfolder_record_cache['FOLDER1']) + self.assertIn('NORMAL', p.subfolder_record_cache['FOLDER1']) + + def test_missing_optional_caches_do_not_raise(self): + """A params fixture with only default-empty NSF/subfolder caches must not break the guard.""" + p = _params_with_records({'NORMAL': 'Other'}) + with hide_from_record_cache(p, {'PROTECTED': PROTECTED_TITLE}): + pass + + def test_attribute_replaced_with_none_mid_command_does_not_mask_the_original_error(self): + """A command that clears/replaces a guarded attribute mid-execution must not + turn a real error into a confusing 'NoneType is not iterable' one, and the + protected entry must still be restored on a best-effort basis.""" + p = _params_with_records({'PROTECTED': PROTECTED_TITLE}) + with self.assertRaises(ValueError) as ctx: + with hide_from_record_cache(p, {'PROTECTED': PROTECTED_TITLE}): + p.record_cache = None + raise ValueError('original command error') + self.assertEqual(str(ctx.exception), 'original command error') + self.assertIn('PROTECTED', p.record_cache) + + def test_attribute_replaced_with_plain_dict_preserves_its_contents(self): + """A command that replaces the guarded attribute with a fresh plain dict + (not just mutating the guarded one) must not lose that dict's entries.""" + p = _params_with_records({'PROTECTED': PROTECTED_TITLE, 'NORMAL': 'Other'}) + with hide_from_record_cache(p, {'PROTECTED': PROTECTED_TITLE}): + p.record_cache = {'REPLACED': _record_cache_entry('REPLACED', 'from a full reassignment')} + + self.assertIn('REPLACED', p.record_cache) + self.assertIn('PROTECTED', p.record_cache) + + +class TestResolveSyncDownExemptUid(TestCase): + def test_returns_env_uid_for_slack_sync_down(self): + with mock.patch.dict(os.environ, {'SLACK_RECORD': 'SLACK_UID'}, clear=True): + self.assertEqual( + resolve_sync_down_exempt_uid(['slack-app-setup', '--sync-down']), 'SLACK_UID' + ) + + def test_returns_env_uid_regardless_of_explicit_dash_r_value(self): + """Never derived from the admin's own -r value -- always the env-pinned UID.""" + with mock.patch.dict(os.environ, {'SLACK_RECORD': 'SLACK_UID'}, clear=True): + self.assertEqual( + resolve_sync_down_exempt_uid(['slack-app-setup', '--sync-down', '-r', 'SOME_OTHER_UID']), + 'SLACK_UID', + ) + + def test_none_without_sync_down_token(self): + with mock.patch.dict(os.environ, {'SLACK_RECORD': 'SLACK_UID'}, clear=True): + self.assertIsNone(resolve_sync_down_exempt_uid(['slack-app-setup'])) + + def test_none_for_unrelated_command(self): + with mock.patch.dict(os.environ, {'SLACK_RECORD': 'SLACK_UID'}, clear=True): + self.assertIsNone(resolve_sync_down_exempt_uid(['get', '--sync-down'])) + + def test_none_when_env_var_unset(self): + with mock.patch.dict(os.environ, {}, clear=True): + self.assertIsNone(resolve_sync_down_exempt_uid(['slack-app-setup', '--sync-down'])) + + def test_gchat_uses_its_own_env_var(self): + with mock.patch.dict(os.environ, {'GCHAT_RECORD': 'G_UID'}, clear=True): + self.assertEqual(resolve_sync_down_exempt_uid(['gchat-app-setup', '--sync-down']), 'G_UID') + + def test_teams_has_no_sync_down_exemption_yet(self): + """Teams has no approvals profile yet, so --sync-down isn't even a registered flag for it; + this must stay None rather than exempting a UID for a flow that can't actually run.""" + with mock.patch.dict(os.environ, {'TEAMS_RECORD': 'T_UID'}, clear=True): + self.assertIsNone(resolve_sync_down_exempt_uid(['teams-app-setup', '--sync-down'])) + + def test_empty_tokens_returns_none(self): + self.assertIsNone(resolve_sync_down_exempt_uid([])) + + def test_abbreviated_flag_does_not_grant_the_exemption(self): + """Exact match only -- granting an exemption is the permissive direction, so an + abbreviation like '--s' (ambiguous with --skip-device-setup on the real parser + anyway) must not be treated as --sync-down.""" + with mock.patch.dict(os.environ, {'SLACK_RECORD': 'SLACK_UID'}, clear=True): + self.assertIsNone(resolve_sync_down_exempt_uid(['slack-app-setup', '--s'])) + self.assertIsNone(resolve_sync_down_exempt_uid(['slack-app-setup', '--sync'])) + + +def _folder_node(uid, name, parent_uid=None): + node = SharedFolderNode() + node.uid = uid + node.parent_uid = parent_uid + node.name = name + return node + + +def _params_with_folder(folder_uid='FOLDER1', record_uid='PROTECTED', parent_uid=None): + p = params_module.KeeperParams() + p.root_folder = RootFolderNode() + node = _folder_node(folder_uid, 'Commander Service Mode - Docker', parent_uid) + p.folder_cache = {folder_uid: node} + parent_list = p.root_folder.subfolders if not parent_uid else None + if parent_list is not None: + parent_list.append(folder_uid) + p.shared_folder_cache = {folder_uid: {'name_unencrypted': node.name}} + p.subfolder_cache = {folder_uid: {'type': 'shared_folder', 'shared_folder_uid': folder_uid}} + p.subfolder_record_cache = {folder_uid: {record_uid}, 'OTHER_FOLDER': {'OTHER_RECORD'}} + return p + + +class TestGetProtectedFolderUids(TestCase): + def test_returns_folder_containing_a_protected_record(self): + p = _params_with_folder(folder_uid='FOLDER1', record_uid='PROTECTED') + result = get_protected_folder_uids(p, {'PROTECTED': 'Commander Service Mode Docker Config'}) + self.assertEqual(result, {'FOLDER1'}) + + def test_no_protected_records_present(self): + p = _params_with_folder(folder_uid='FOLDER1', record_uid='PROTECTED') + self.assertEqual(get_protected_folder_uids(p, {'UNRELATED': 'x'}), set()) + + def test_empty_protected_record_uids_is_a_noop(self): + p = _params_with_folder() + self.assertEqual(get_protected_folder_uids(p, {}), set()) + + def test_params_none(self): + self.assertEqual(get_protected_folder_uids(None, {'PROTECTED': 'x'}), set()) + + def test_missing_subfolder_record_cache_is_ignored(self): + p = params_module.KeeperParams() + self.assertEqual(get_protected_folder_uids(p, {'PROTECTED': 'x'}), set()) + + def test_renamed_folder_is_still_found(self): + """Derived from record containment, not a title list -- a rename doesn't lose protection.""" + p = _params_with_folder(folder_uid='FOLDER1', record_uid='PROTECTED') + p.folder_cache['FOLDER1'].name = 'My Totally Renamed Folder' + p.shared_folder_cache['FOLDER1']['name_unencrypted'] = 'My Totally Renamed Folder' + result = get_protected_folder_uids(p, {'PROTECTED': 'Commander Service Mode Docker Config'}) + self.assertEqual(result, {'FOLDER1'}) + + +class TestHideFromFolderCache(TestCase): + def test_hides_protected_folder_from_all_three_caches_inside_the_block(self): + p = _params_with_folder(folder_uid='FOLDER1') + with hide_from_folder_cache(p, {'FOLDER1'}): + self.assertNotIn('FOLDER1', p.folder_cache) + self.assertNotIn('FOLDER1', p.shared_folder_cache) + self.assertNotIn('FOLDER1', p.subfolder_cache) + + def test_strips_uid_from_root_folder_subfolders_during_the_block(self): + p = _params_with_folder(folder_uid='FOLDER1') + with hide_from_folder_cache(p, {'FOLDER1'}): + self.assertNotIn('FOLDER1', p.root_folder.subfolders) + + def test_strips_uid_from_parent_folders_subfolders_when_nested(self): + p = _params_with_folder(folder_uid='FOLDER1', parent_uid='PARENT') + parent = _folder_node('PARENT', 'Some Parent Folder') + p.folder_cache['PARENT'] = parent + parent.subfolders.append('FOLDER1') + with hide_from_folder_cache(p, {'FOLDER1'}): + self.assertNotIn('FOLDER1', parent.subfolders) + self.assertIn('FOLDER1', parent.subfolders) + + def test_restores_everything_after_the_block(self): + p = _params_with_folder(folder_uid='FOLDER1') + with hide_from_folder_cache(p, {'FOLDER1'}): + pass + self.assertIn('FOLDER1', p.folder_cache) + self.assertIn('FOLDER1', p.shared_folder_cache) + self.assertIn('FOLDER1', p.subfolder_cache) + self.assertIn('FOLDER1', p.root_folder.subfolders) + + def test_restores_even_if_block_raises(self): + p = _params_with_folder(folder_uid='FOLDER1') + with self.assertRaises(ValueError): + with hide_from_folder_cache(p, {'FOLDER1'}): + raise ValueError('boom') + self.assertIn('FOLDER1', p.folder_cache) + self.assertIn('FOLDER1', p.root_folder.subfolders) + + def test_reintroduction_during_block_is_blocked(self): + p = _params_with_folder(folder_uid='FOLDER1') + with hide_from_folder_cache(p, {'FOLDER1'}): + p.folder_cache['FOLDER1'] = _folder_node('FOLDER1', 'reintroduced') + self.assertNotIn('FOLDER1', p.folder_cache) + self.assertIn('FOLDER1', p.folder_cache) + + def test_no_protected_folders_is_a_noop(self): + p = _params_with_folder(folder_uid='FOLDER1') + with hide_from_folder_cache(p, set()): + self.assertIn('FOLDER1', p.folder_cache) + + def test_params_none_is_a_noop(self): + with hide_from_folder_cache(None, {'FOLDER1'}): + pass + + def test_missing_root_folder_does_not_raise(self): + """A params fixture with no root_folder set (e.g. never synced) must not crash the guard.""" + p = _params_with_folder(folder_uid='FOLDER1') + p.root_folder = None + with hide_from_folder_cache(p, {'FOLDER1'}): + self.assertNotIn('FOLDER1', p.folder_cache) + + def test_a_resync_mid_block_self_heals_without_reintroducing_the_folder(self): + """A forced resync mid-command rebuilds folder_cache/root_folder from the (still-guarded) + raw subfolder_cache/shared_folder_cache, so the protected folder must not reappear.""" + p = _params_with_folder(folder_uid='FOLDER1') + with hide_from_folder_cache(p, {'FOLDER1'}): + from keepercommander.sync_down import prepare_folder_tree + prepare_folder_tree(p) + self.assertNotIn('FOLDER1', p.folder_cache) + self.assertNotIn('FOLDER1', p.root_folder.subfolders) + + +class TestHasReservedLegacyAttachment(TestCase): + def test_password_record_with_reserved_attachment_name(self): + record = vault.PasswordRecord() + record.attachments = [vault.AttachmentFile({'name': 'config.json', 'title': 'config.json'})] + self.assertTrue(_has_reserved_legacy_attachment(record)) + + def test_password_record_with_reserved_title_but_different_name(self): + """attachment.py itself checks title OR name -- match either.""" + record = vault.PasswordRecord() + record.attachments = [vault.AttachmentFile({'name': 'file123', 'title': 'service_config.json'})] + self.assertTrue(_has_reserved_legacy_attachment(record)) + + def test_password_record_with_unrelated_attachment(self): + record = vault.PasswordRecord() + record.attachments = [vault.AttachmentFile({'name': 'notes.pdf', 'title': 'notes.pdf'})] + self.assertFalse(_has_reserved_legacy_attachment(record)) + + def test_password_record_with_no_attachments(self): + self.assertFalse(_has_reserved_legacy_attachment(vault.PasswordRecord())) + + def test_non_password_record_is_always_false(self): + """TypedRecord's fileRef attachments are handled inline in get_protected_record_uids + (via the reserved_file_uids/pending_attachments cross-reference), not here.""" + self.assertFalse(_has_reserved_legacy_attachment(vault.TypedRecord())) + self.assertFalse(_has_reserved_legacy_attachment(vault.FileRecord())) + + +class TestGetProtectedRecordUidsWithReservedAttachments(TestCase): + """Records are loaded exactly once each -- attachment detection must not add extra + per-attachment KeeperRecord.load calls (previously N extra loads per fileRef attachment).""" + + @staticmethod + def _params_with(records: dict): + """records: {uid: KeeperRecord-like object}, each already carrying its own .record_uid.""" + p = _params_with_records({uid: 'placeholder' for uid in records}) + return p, records + + def test_arbitrary_titled_typed_record_with_reserved_file_ref_is_protected(self): + parent = vault.TypedRecord() + parent.record_uid = 'PARENT_UID' + parent.title = 'My Totally Unrelated Title' + parent.fields = [vault.TypedField({'type': 'fileRef', 'value': ['FILE_UID_1']})] + + file_record = vault.FileRecord() + file_record.record_uid = 'FILE_UID_1' + file_record.title = 'service_config.json' + file_record.name = 'service_config.json' + + p, records = self._params_with({'PARENT_UID': parent, 'FILE_UID_1': file_record}) + with mock.patch('keepercommander.vault.KeeperRecord.load', side_effect=lambda params, uid: records.get(uid)): + result = get_protected_record_uids(p) + self.assertIn('PARENT_UID', result) + self.assertIn('FILE_UID_1', result) + + def test_arbitrary_titled_password_record_with_reserved_attachment_is_protected(self): + parent = vault.PasswordRecord() + parent.record_uid = 'PARENT_UID' + parent.title = 'My Totally Unrelated Title' + parent.attachments = [vault.AttachmentFile({'id': 'ATTA_1', 'name': 'config.json'})] + + p, records = self._params_with({'PARENT_UID': parent}) + with mock.patch('keepercommander.vault.KeeperRecord.load', side_effect=lambda params, uid: records.get(uid)): + result = get_protected_record_uids(p) + self.assertIn('PARENT_UID', result) + self.assertIn('ATTA_1', result) + + def test_unrelated_attachment_name_is_not_protected(self): + parent = vault.TypedRecord() + parent.record_uid = 'PARENT_UID' + parent.title = 'My Totally Unrelated Title' + parent.fields = [vault.TypedField({'type': 'fileRef', 'value': ['FILE_UID_1']})] + + file_record = vault.FileRecord() + file_record.record_uid = 'FILE_UID_1' + file_record.title = 'notes.pdf' + file_record.name = 'notes.pdf' + + p, records = self._params_with({'PARENT_UID': parent, 'FILE_UID_1': file_record}) + with mock.patch('keepercommander.vault.KeeperRecord.load', side_effect=lambda params, uid: records.get(uid)): + result = get_protected_record_uids(p) + self.assertEqual(result, {}) + + def test_reserved_attachment_on_an_already_title_protected_record_is_still_swept_in(self): + parent = vault.TypedRecord() + parent.record_uid = 'PARENT_UID' + parent.title = PROTECTED_TITLE + parent.fields = [vault.TypedField({'type': 'fileRef', 'value': ['FILE_UID_1']})] + + file_record = vault.FileRecord() + file_record.record_uid = 'FILE_UID_1' + file_record.title = 'service_config.json' + file_record.name = 'service_config.json' + + p, records = self._params_with({'PARENT_UID': parent, 'FILE_UID_1': file_record}) + with mock.patch('keepercommander.vault.KeeperRecord.load', side_effect=lambda params, uid: records.get(uid)): + result = get_protected_record_uids(p) + self.assertIn('PARENT_UID', result) + self.assertIn('FILE_UID_1', result) + + def test_load_is_called_exactly_once_per_record_cache_entry(self): + """Regression test for the N-vs-3N perf issue: attachment detection must not add + extra per-attachment loads on top of the one load every record already gets.""" + parent = vault.TypedRecord() + parent.record_uid = 'PARENT_UID' + parent.title = 'Unrelated' + parent.fields = [vault.TypedField({'type': 'fileRef', 'value': ['FILE_UID_1', 'FILE_UID_2']})] + + file_record_1 = vault.FileRecord() + file_record_1.record_uid = 'FILE_UID_1' + file_record_1.title = 'notes.pdf' + file_record_2 = vault.FileRecord() + file_record_2.record_uid = 'FILE_UID_2' + file_record_2.title = 'photo.png' + + p, records = self._params_with( + {'PARENT_UID': parent, 'FILE_UID_1': file_record_1, 'FILE_UID_2': file_record_2} + ) + with mock.patch( + 'keepercommander.vault.KeeperRecord.load', side_effect=lambda params, uid: records.get(uid) + ) as mock_load: + get_protected_record_uids(p) + self.assertEqual(mock_load.call_count, len(records)) + + +class TestAttachmentFileUids(TestCase): + def test_password_record_returns_attachment_ids(self): + record = vault.PasswordRecord() + record.attachments = [ + vault.AttachmentFile({'id': 'A1', 'name': 'x'}), + vault.AttachmentFile({'id': 'A2', 'name': 'y'}), + ] + self.assertEqual(set(_attachment_file_uids(record)), {'A1', 'A2'}) + + def test_typed_record_returns_file_ref_values(self): + record = vault.TypedRecord() + record.fields = [vault.TypedField({'type': 'fileRef', 'value': ['F1', 'F2']})] + self.assertEqual(set(_attachment_file_uids(record)), {'F1', 'F2'}) + + def test_record_with_no_attachments_returns_empty(self): + self.assertEqual(_attachment_file_uids(vault.PasswordRecord()), []) + self.assertEqual(_attachment_file_uids(vault.TypedRecord()), []) diff --git a/unit-tests/service/test_runtime_policy.py b/unit-tests/service/test_runtime_policy.py index debdb4724..04c1d414e 100644 --- a/unit-tests/service/test_runtime_policy.py +++ b/unit-tests/service/test_runtime_policy.py @@ -20,6 +20,7 @@ from keepercommander.service.commands.integrations.sailpoint_app_setup import SailPointAppSetupCommand from keepercommander.service.commands.integrations.slack_app_setup import SlackAppSetupCommand from keepercommander.service.commands.terraform_app_setup import TerraformSetupConstants +from keepercommander.service.decorators.min_commander_version import TERRAFORM_DOCKER_ENV, TERRAFORM_DOCKER_ENV_LEGACY from keepercommander.service.util.exceptions import ValidationError @@ -67,7 +68,17 @@ def test_gchat_record_env_confines_to_gchat_allowlist(self): self.assertEqual(set(args.commands.split(',')), allowed) def test_terraform_env_confines_to_terraform_allowlist(self): - with mock.patch.dict(os.environ, {'KEEPER_TERRAFORM': '1'}, clear=True): + with mock.patch.dict(os.environ, {TERRAFORM_DOCKER_ENV: 'tf-record-uid'}, clear=True): + args = _Args(commands=TerraformSetupConstants.SERVICE_COMMANDS + ',clipboard-copy') + apply_runtime_command_policy(args) + self.assertEqual( + set(args.commands.split(',')), set(TerraformSetupConstants.SERVICE_COMMANDS_LIST) + ) + + def test_legacy_terraform_env_confines_to_terraform_allowlist(self): + """A container upgraded without re-running terraform-app-setup still has the old + KEEPER_TERRAFORM marker -- the startup sanitizer must not silently skip it.""" + with mock.patch.dict(os.environ, {TERRAFORM_DOCKER_ENV_LEGACY: '1'}, clear=True): args = _Args(commands=TerraformSetupConstants.SERVICE_COMMANDS + ',clipboard-copy') apply_runtime_command_policy(args) self.assertEqual( @@ -85,7 +96,7 @@ def test_sailpoint_record_env_confines_to_sailpoint_allowlist(self): def test_multiple_integration_env_vars_raises(self): with mock.patch.dict( - os.environ, {'SLACK_RECORD': 'uid-1', 'KEEPER_TERRAFORM': '1'}, clear=True + os.environ, {'SLACK_RECORD': 'uid-1', TERRAFORM_DOCKER_ENV: 'tf-record-uid'}, clear=True ): args = _Args(commands='search,malicious-command') with self.assertRaises(ValidationError): diff --git a/unit-tests/service/test_tailscale_config.py b/unit-tests/service/test_tailscale_config.py new file mode 100644 index 000000000..f4034705a --- /dev/null +++ b/unit-tests/service/test_tailscale_config.py @@ -0,0 +1,94 @@ +import unittest +from unittest import mock + +from keepercommander.service.config.tailscale_config import TailscaleConfigurator + + +def _base_config_data(): + return { + "tailscale": "y", + "port": 8080, + "tailscale_auth_key": "tskey-auth-xxx", + "tailscale_advertise_tags": "", + "run_mode": "foreground", + "tailscale_public_url": "", + } + + +class TestVerifyFunnelActive(unittest.TestCase): + def test_returns_true_immediately_when_already_active(self): + with mock.patch('keepercommander.service.config.tailscale_config.get_tailscale_funnel_status', return_value=True) as mock_status: + self.assertTrue(TailscaleConfigurator._verify_funnel_active(8080, max_retries=3, retry_delay=0)) + mock_status.assert_called_once_with(8080) + + def test_retries_before_succeeding(self): + """A slower daemon-side registration can take a beat after `--bg` returns - + the check must retry rather than declaring failure on the first miss.""" + with mock.patch('keepercommander.service.config.tailscale_config.get_tailscale_funnel_status', + side_effect=[False, False, True]) as mock_status, \ + mock.patch('time.sleep'): + self.assertTrue(TailscaleConfigurator._verify_funnel_active(8080, max_retries=3, retry_delay=0)) + self.assertEqual(mock_status.call_count, 3) + + def test_returns_false_after_exhausting_retries(self): + with mock.patch('keepercommander.service.config.tailscale_config.get_tailscale_funnel_status', return_value=False), \ + mock.patch('time.sleep'): + self.assertFalse(TailscaleConfigurator._verify_funnel_active(8080, max_retries=3, retry_delay=0)) + + +class TestConfigureTailscaleVerification(unittest.TestCase): + """configure_tailscale's Funnel-active verification and rollback, added after a + live approval-pending case showed `tailscale funnel --bg` can exit 0 without the + target ever actually going live.""" + + def _patch_happy_path_prereqs(self): + return [ + mock.patch('keepercommander.service.config.tailscale_config.reset_tailscale_log'), + mock.patch.object(TailscaleConfigurator, '_ensure_ready'), + mock.patch.object(TailscaleConfigurator, '_validate_tailscale_config'), + mock.patch('keepercommander.service.config.tailscale_config.tailscale_up'), + mock.patch('keepercommander.service.config.tailscale_config.start_tailscale_funnel'), + ] + + def test_rolls_back_and_raises_when_funnel_never_becomes_active(self): + config_data = _base_config_data() + patches = self._patch_happy_path_prereqs() + with patches[0], patches[1], patches[2], patches[3], patches[4], \ + mock.patch.object(TailscaleConfigurator, '_verify_funnel_active', return_value=False), \ + mock.patch('keepercommander.service.config.tailscale_config.stop_tailscale_funnel') as mock_stop, \ + mock.patch('keepercommander.service.config.tailscale_config.get_tailscale_funnel_url') as mock_get_url: + with self.assertRaises(Exception): + TailscaleConfigurator.configure_tailscale(config_data, mock.Mock()) + + mock_stop.assert_called_once_with(config_data["port"]) + # Must fail before ever asking for the public URL - there isn't a live one. + mock_get_url.assert_not_called() + + def test_rollback_failure_does_not_mask_the_original_error(self): + """stop_tailscale_funnel itself failing during rollback must not swallow or + replace the original 'Funnel never became active' error.""" + config_data = _base_config_data() + patches = self._patch_happy_path_prereqs() + with patches[0], patches[1], patches[2], patches[3], patches[4], \ + mock.patch.object(TailscaleConfigurator, '_verify_funnel_active', return_value=False), \ + mock.patch('keepercommander.service.config.tailscale_config.stop_tailscale_funnel', + side_effect=Exception("reset failed too")): + with self.assertRaisesRegex(Exception, "did not become active"): + TailscaleConfigurator.configure_tailscale(config_data, mock.Mock()) + + def test_succeeds_and_fetches_url_when_funnel_is_verified_active(self): + config_data = _base_config_data() + patches = self._patch_happy_path_prereqs() + with patches[0], patches[1], patches[2], patches[3], patches[4], \ + mock.patch.object(TailscaleConfigurator, '_verify_funnel_active', return_value=True), \ + mock.patch('keepercommander.service.config.tailscale_config.get_tailscale_funnel_url', + return_value='https://node.example.ts.net') as mock_get_url: + result = TailscaleConfigurator.configure_tailscale(config_data, mock.Mock()) + + self.assertIsNone(result) + mock_get_url.assert_called_once_with(config_data["port"]) + self.assertEqual(config_data["tailscale_public_url"], 'https://node.example.ts.net') + + +if __name__ == '__main__': + unittest.main() diff --git a/unit-tests/service/test_terraform_app_setup.py b/unit-tests/service/test_terraform_app_setup.py index ec7c25b60..4607a40b9 100644 --- a/unit-tests/service/test_terraform_app_setup.py +++ b/unit-tests/service/test_terraform_app_setup.py @@ -58,7 +58,8 @@ def test_compose_uses_terraform_service_and_container_names(self): self.assertIn('container_name: keeper-service-terraform', yaml_content) self.assertNotIn('container_name: keeper-service\n', yaml_content) self.assertIn(f'{TERRAFORM_DOCKER_ENV}:', yaml_content) - self.assertRegex(yaml_content, rf"{TERRAFORM_DOCKER_ENV}:\s*'?1'?") + # Now carries the record UID (so protected_records.py can pin it), not a bare '1' flag. + self.assertRegex(yaml_content, rf"{TERRAFORM_DOCKER_ENV}:\s*'?{setup_result.record_uid}'?") @mock.patch( 'keepercommander.service.commands.terraform_app_setup.RuntimeServiceConfig' diff --git a/unit-tests/service/test_tunneling.py b/unit-tests/service/test_tunneling.py index 6de58970d..e2692c39e 100644 --- a/unit-tests/service/test_tunneling.py +++ b/unit-tests/service/test_tunneling.py @@ -1,3 +1,4 @@ +import json import os import tempfile import unittest @@ -227,7 +228,11 @@ class TestDownloadCloudflared(unittest.TestCase): def test_lookup_failure_is_logged_not_swallowed_silently(self): # Force platform.system() to an unsupported value so _download_cloudflared # raises right after the (logged) lookup failure, without attempting a real download. - with mock.patch('keepercommander.service.util.tunneling.subprocess.run', + # sys.platform is pinned to a POSIX value too -- the PATH lookup this test exercises + # is skipped entirely on real win32 (see test_windows_never_searches_path_or_cwd), so + # this must not depend on which OS actually runs the test. + with mock.patch('keepercommander.service.util.tunneling.sys.platform', 'darwin'), \ + mock.patch('keepercommander.service.util.tunneling.subprocess.run', side_effect=OSError("cloudflared not found")), \ mock.patch('keepercommander.service.util.tunneling.logging.debug') as mock_debug, \ mock.patch('platform.system', return_value='unsupported'): @@ -268,5 +273,81 @@ def test_resolves_the_data_dir_at_call_time_not_import_time(self): self.assertTrue(log_file.startswith(os.path.join(overridden_dir, 'service_logs'))) +class TestStartTailscaleFunnel(unittest.TestCase): + def test_rejects_port_not_in_allowed_set(self): + """Tailscale Funnel only accepts 443/8443/10000 as the external-facing port - + catch an invalid value before it ever reaches the CLI.""" + with self.assertRaises(ValueError): + tunneling.start_tailscale_funnel(local_port=8080, funnel_port=9999) + + def test_accepts_each_allowed_port(self): + with tempfile.NamedTemporaryFile() as tmp: + for allowed_port in tunneling.TAILSCALE_FUNNEL_ALLOWED_PORTS: + with mock.patch('keepercommander.service.util.tunneling._get_tailscale_log_path', return_value=tmp.name), \ + mock.patch('keepercommander.service.util.tunneling.subprocess.run', + return_value=mock.Mock(returncode=0)) as mock_run: + tunneling.start_tailscale_funnel(local_port=8080, funnel_port=allowed_port) + cmd = mock_run.call_args[0][0] + self.assertIn(f"--https={allowed_port}", cmd) + + def test_missing_local_port_raises(self): + with self.assertRaises(ValueError): + tunneling.start_tailscale_funnel(local_port=None) + + def test_raises_with_guidance_when_cli_exits_nonzero(self): + with tempfile.NamedTemporaryFile() as tmp, \ + mock.patch('keepercommander.service.util.tunneling._get_tailscale_log_path', return_value=tmp.name), \ + mock.patch('keepercommander.service.util.tunneling.subprocess.run', + return_value=mock.Mock(returncode=1)): + with self.assertRaisesRegex(Exception, "Failed to start Tailscale Funnel"): + tunneling.start_tailscale_funnel(local_port=8080) + + +class TestGetTailscaleFunnelStatus(unittest.TestCase): + """Schema verified live against a real `tailscale funnel status --json` while a + Funnel target was actually running: {"Web": {":": {"Handlers": + {"": {"Proxy": "http://localhost:"}}}}}.""" + + def test_true_when_local_port_is_an_active_proxy_target(self): + payload = {"Web": {"example.ts.net:443": {"Handlers": {"/": {"Proxy": "http://localhost:8080"}}}}} + with mock.patch('keepercommander.service.util.tunneling.subprocess.run', + return_value=mock.Mock(returncode=0, stdout=json.dumps(payload))): + self.assertTrue(tunneling.get_tailscale_funnel_status(8080)) + + def test_false_when_no_matching_target(self): + payload = {"Web": {"example.ts.net:443": {"Handlers": {"/": {"Proxy": "http://localhost:9999"}}}}} + with mock.patch('keepercommander.service.util.tunneling.subprocess.run', + return_value=mock.Mock(returncode=0, stdout=json.dumps(payload))): + self.assertFalse(tunneling.get_tailscale_funnel_status(8080)) + + def test_false_when_no_funnel_configured_at_all(self): + with mock.patch('keepercommander.service.util.tunneling.subprocess.run', + return_value=mock.Mock(returncode=0, stdout='{}')): + self.assertFalse(tunneling.get_tailscale_funnel_status(8080)) + + def test_false_on_cli_failure_not_raised(self): + """A status check is diagnostic, not authoritative - a CLI/parsing error must + report 'not active' rather than bubbling up and crashing the caller.""" + with mock.patch('keepercommander.service.util.tunneling.subprocess.run', + side_effect=Exception("boom")): + self.assertFalse(tunneling.get_tailscale_funnel_status(8080)) + + +class TestStopTailscaleFunnel(unittest.TestCase): + def test_returns_true_on_success(self): + with tempfile.NamedTemporaryFile() as tmp, \ + mock.patch('keepercommander.service.util.tunneling._get_tailscale_log_path', return_value=tmp.name), \ + mock.patch('keepercommander.service.util.tunneling.subprocess.run', + return_value=mock.Mock(returncode=0)): + self.assertTrue(tunneling.stop_tailscale_funnel(8080)) + + def test_returns_false_on_nonzero_exit(self): + with tempfile.NamedTemporaryFile() as tmp, \ + mock.patch('keepercommander.service.util.tunneling._get_tailscale_log_path', return_value=tmp.name), \ + mock.patch('keepercommander.service.util.tunneling.subprocess.run', + return_value=mock.Mock(returncode=1)): + self.assertFalse(tunneling.stop_tailscale_funnel(8080)) + + if __name__ == '__main__': unittest.main() diff --git a/unit-tests/service/test_verified_command.py b/unit-tests/service/test_verified_command.py index 4d4ca1c01..e1a94337a 100644 --- a/unit-tests/service/test_verified_command.py +++ b/unit-tests/service/test_verified_command.py @@ -373,3 +373,115 @@ def test_is_record_file_attachment_arg(self): self.assertFalse(is_file('--title')) self.assertFalse(is_file('profile=x')) self.assertFalse(is_file('my.file=x')) # not a file-type field after parse_field + + +class TestProtectedServiceConfigRecords(TestCase): + """No Service Mode command may touch Service Mode's own config records by literal title or UID -- checked + unconditionally for every command (not a curated list) so no current or future command can slip through.""" + + PROTECTED_TITLE = 'Commander Service Mode Config' + PROTECTED_UID = 'PROTECTED_UID_1234' + + def _check(self, cmd, protected_uids=None): + return Verifycommand.validate_service_mode_protected_record_command( + _tokens(cmd), protected_uids or {self.PROTECTED_UID} + ) + + def test_blocks_by_title_across_classic_commands(self): + for cmd in ( + f'get "{self.PROTECTED_TITLE}"', + f'g "{self.PROTECTED_TITLE}"', + f'list "{self.PROTECTED_TITLE}"', + f'l "{self.PROTECTED_TITLE}"', + f'search "{self.PROTECTED_TITLE}"', + f's "{self.PROTECTED_TITLE}"', + f'record-update --record "{self.PROTECTED_TITLE}" title=x', + f'ru --record "{self.PROTECTED_TITLE}" title=x', + f'share-record "{self.PROTECTED_TITLE}" --email a@b.com', + f'sr "{self.PROTECTED_TITLE}" --email a@b.com', + f'rm "{self.PROTECTED_TITLE}"', + f'share-folder --record "{self.PROTECTED_TITLE}" -e a@b.com', + f'record-history "{self.PROTECTED_TITLE}"', + f'clipboard-copy "{self.PROTECTED_TITLE}"', + f'totp "{self.PROTECTED_TITLE}"', + f'one-time-share "{self.PROTECTED_TITLE}"', + f'ls "{self.PROTECTED_TITLE}"', + f'tree "{self.PROTECTED_TITLE}"', + ): + with self.subTest(cmd=cmd): + self.assertIsNotNone(self._check(cmd)) + + def test_blocks_by_title_across_nsf_commands(self): + for cmd in ( + f'nsf-get "{self.PROTECTED_TITLE}"', + f'nsf-share-record "{self.PROTECTED_TITLE}" --email a@b.com', + f'nsf-record-update --record "{self.PROTECTED_TITLE}" title=x', + f'nsf-transfer-record "{self.PROTECTED_TITLE}" a@b.com', + f'nsf-record-details "{self.PROTECTED_TITLE}"', + f'nsf-rm "{self.PROTECTED_TITLE}"', + f'nsf-move "{self.PROTECTED_TITLE}" root', + f'nsf-ln "{self.PROTECTED_TITLE}" SomeFolder', + f'nsf-shortcut keep "{self.PROTECTED_TITLE}"', + ): + with self.subTest(cmd=cmd): + self.assertIsNotNone(self._check(cmd)) + + def test_blocks_by_title_case_insensitive(self): + self.assertIsNotNone(self._check('get "COMMANDER service MODE config"')) + + def test_blocks_by_uid_regardless_of_command(self): + for cmd in ( + f'get {self.PROTECTED_UID}', + f'list {self.PROTECTED_UID}', + f'search {self.PROTECTED_UID}', + f'record-update --record {self.PROTECTED_UID} title=x', + f'share-record {self.PROTECTED_UID} --email a@b.com', + f'share-folder --record {self.PROTECTED_UID} -e a@b.com', + f'rm {self.PROTECTED_UID}', + f'nsf-get {self.PROTECTED_UID}', + f'nsf-transfer-record {self.PROTECTED_UID} a@b.com', + # A command with no known relationship to records at all -- still + # caught, since the check is unconditional, not command-specific. + f'keep-alive {self.PROTECTED_UID}', + ): + with self.subTest(cmd=cmd): + self.assertIsNotNone(self._check(cmd)) + + def test_uid_match_is_case_sensitive(self): + self.assertIsNone(self._check(f'get {self.PROTECTED_UID.lower()}')) + + def test_unrelated_title_and_uid_are_allowed(self): + self.assertIsNone(self._check('get "My Normal Record"')) + self.assertIsNone(self._check('rm SOME_OTHER_UID')) + self.assertIsNone(self._check('record-add --title "My Normal Record"')) + + def test_no_protected_uids_still_blocks_by_title(self): + err = Verifycommand.validate_service_mode_protected_record_command( + _tokens(f'get "{self.PROTECTED_TITLE}"'), None + ) + self.assertIsNotNone(err) + + def test_empty_tokens_returns_none(self): + self.assertIsNone(Verifycommand.validate_service_mode_protected_record_command([])) + + def test_blocks_equals_form_uid(self): + for cmd in ( + f'record-update --record={self.PROTECTED_UID} title=x', + f'get --record-uid={self.PROTECTED_UID}', + f'share-folder --record={self.PROTECTED_UID} -e a@b.com', + f'share-folder -r={self.PROTECTED_UID} -e a@b.com', + ): + with self.subTest(cmd=cmd): + self.assertIsNotNone(self._check(cmd)) + + def test_blocks_equals_form_title(self): + self.assertIsNotNone( + self._check(f'record-update --record="{self.PROTECTED_TITLE}" title=x') + ) + + def test_equals_form_unrelated_value_is_allowed(self): + self.assertIsNone(self._check('record-update --record=SOME_OTHER_UID title=x')) + self.assertIsNone(self._check('record-update --title="My Normal Record" x=y')) + + def test_equals_form_with_no_value_does_not_crash(self): + self.assertIsNone(self._check('get --record-uid=')) diff --git a/unit-tests/test_command_enterprise.py b/unit-tests/test_command_enterprise.py index 32dd05e9c..70741ef56 100644 --- a/unit-tests/test_command_enterprise.py +++ b/unit-tests/test_command_enterprise.py @@ -5,9 +5,10 @@ from unittest import TestCase, mock from data_enterprise import EnterpriseEnvironment, get_enterprise_data, enterprise_allocate_ids -from keepercommander import api, crypto, utils, vault +from keepercommander import api, crypto, enterprise as enterprise_data, utils, vault from keepercommander.params import KeeperParams, PublicKeys from keepercommander.error import CommandError +from keepercommander.proto import enterprise_pb2 from data_vault import VaultEnvironment, get_connected_params from keepercommander.commands import enterprise, aram @@ -47,6 +48,41 @@ def test_get_enterprise_public_key(self): self.assertEqual(params.enterprise['unencrypted_tree_key'], ent_env.tree_key) self.assertEqual(len(params.enterprise['nodes']), 2) + def test_general_data_restrict_visibility_controls_root_node(self): + params = get_connected_params() + api.query_enterprise(params) + params.enterprise['keys'] = {} + root = next(x for x in params.enterprise['nodes'] if x['node_id'] == ent_env.node1_id) + child = next(x for x in params.enterprise['nodes'] if x['node_id'] == ent_env.node2_id) + root['restrict_visibility'] = True + child['restrict_visibility'] = True + + response = enterprise_pb2.EnterpriseDataResponse() + response.generalData.enterpriseName = params.enterprise['enterprise_name'] + response.generalData.restrictVisibility = False + response.hasMore = False + + loader = enterprise_data._EnterpriseLoader(params.enterprise['unencrypted_tree_key']) + with mock.patch('keepercommander.enterprise.api.communicate_rest', return_value=response): + loader.load(params) + + self.assertNotIn('restrict_visibility', root) + self.assertTrue(child['restrict_visibility']) + + response.generalData.restrictVisibility = True + with mock.patch('keepercommander.enterprise.api.communicate_rest', return_value=response): + loader.load(params) + + self.assertTrue(root['restrict_visibility']) + self.assertTrue(child['restrict_visibility']) + + response = enterprise_pb2.EnterpriseDataResponse() + response.hasMore = False + with mock.patch('keepercommander.enterprise.api.communicate_rest', return_value=response): + loader.load(params) + + self.assertTrue(root['restrict_visibility']) + def test_enterprise_info_command(self): params = get_connected_params() api.query_enterprise(params) @@ -164,6 +200,93 @@ def test_enterprise_node_move_sets_selected_parent_id(self): request = execute_batch.call_args.args[1][0] self.assertEqual(request['parent_id'], ent_env.node1_id) + def test_enterprise_node_toggle_root_isolation(self): + for was_isolated in (False, True): + with self.subTest(was_isolated=was_isolated): + params = get_connected_params() + api.query_enterprise(params) + root = next(x for x in params.enterprise['nodes'] + if x['node_id'] == ent_env.node1_id) + root['data']['displayname'] = 'Enterprise 1' + if was_isolated: + root['restrict_visibility'] = True + + def refresh_enterprise(p, force=False, tree_key=None): + self.assertTrue(force) + refreshed_root = next(x for x in p.enterprise['nodes'] + if x['node_id'] == ent_env.node1_id) + if was_isolated: + refreshed_root.pop('restrict_visibility', None) + else: + refreshed_root['restrict_visibility'] = True + + cmd = enterprise.EnterpriseNodeCommand() + with mock.patch( + 'keepercommander.commands.enterprise.api.communicate_rest' + ) as communicate_rest, mock.patch( + 'keepercommander.commands.enterprise.api.query_enterprise', + side_effect=refresh_enterprise + ) as query_enterprise: + cmd.execute( + params, + node=[str(ent_env.node1_id)], + toggle_isolated=True, + ) + + request = communicate_rest.call_args.args[1] + self.assertEqual(request.nodeId, 0) + query_enterprise.assert_called_once_with(params, force=True) + + def test_enterprise_node_toggle_child_isolation(self): + params = get_connected_params() + api.query_enterprise(params) + + def refresh_enterprise(p, force=False, tree_key=None): + self.assertTrue(force) + child = next(x for x in p.enterprise['nodes'] + if x['node_id'] == ent_env.node2_id) + child['restrict_visibility'] = True + + cmd = enterprise.EnterpriseNodeCommand() + with mock.patch( + 'keepercommander.commands.enterprise.api.communicate_rest' + ) as communicate_rest, mock.patch( + 'keepercommander.commands.enterprise.api.query_enterprise', + side_effect=refresh_enterprise + ): + cmd.execute( + params, + node=[str(ent_env.node2_id)], + toggle_isolated=True, + ) + + request = communicate_rest.call_args.args[1] + self.assertEqual(request.nodeId, ent_env.node2_id) + + def test_enterprise_node_toggle_isolation_reports_noop(self): + params = get_connected_params() + api.query_enterprise(params) + child = next(x for x in params.enterprise['nodes'] + if x['node_id'] == ent_env.node2_id) + child['restrict_visibility'] = True + + cmd = enterprise.EnterpriseNodeCommand() + with mock.patch( + 'keepercommander.commands.enterprise.api.communicate_rest' + ), mock.patch( + 'keepercommander.commands.enterprise.api.query_enterprise' + ), self.assertLogs(level=logging.WARNING) as logs: + cmd.execute( + params, + node=[str(ent_env.node2_id)], + toggle_isolated=True, + ) + + self.assertTrue(any( + 'server accepted the isolation toggle, but the state did not change' in message + for message in logs.output + )) + def test_enterprise_add_user(self): params = get_connected_params() api.query_enterprise(params)