feat(pam): add KCM database connector for pam project import#1889
Closed
jlima8900 wants to merge 13 commits intoKeeper-Security:releasefrom
Closed
feat(pam): add KCM database connector for pam project import#1889jlima8900 wants to merge 13 commits intoKeeper-Security:releasefrom
jlima8900 wants to merge 13 commits intoKeeper-Security:releasefrom
Conversation
jlima8900
force-pushed
the
feat/pam-kcm-import
branch
2 times, most recently
from
6ee680b to
f01cab6
Compare
jlima8900
changed the title
…tion Adds `pam project kcm-import` command that connects to a KCM/Guacamole database, extracts connections/users/groups, maps 150+ parameters, and imports into Keeper PAM via the existing import/extend engine. Features: --db-ssl, --docker-detect, --db-password-record, --gateway (interactive selection), --max-instances (pool sizing), --dry-run, --output, --folder-mode, --skip-users, --include-disabled. Prints Docker/K8s deploy commands after gateway creation.
jlima8900
force-pushed
the
feat/pam-kcm-import
branch
from
f01cab6 to
588d1d4
Compare
Same pattern as #1873 — os.open mode only applies on creation, not existing files. Add os.fchmod(fd, 0o600) to ensure owner-only permissions regardless.
Security improvements: - Block remote connections without SSL by default (require --db-ssl or --allow-cleartext to override) - Clear connector.password and db_password after use - Clear docker inspect stdout after parsing (contains all container env vars including other secrets) - Remove db username from docker detect log output - Add os.fchmod for existing output files - Use e.__class__.__name__ instead of type(e).__name__ for consistency Add 8 tests for SSL enforcement, credential cleanup, and log sanitization.
Live E2E tests (skip when DB offline): - Real DB connection, schema validation, data extraction - Parameter mapping with real KCM data (SSH + RDP connections) - All 3 folder modes (ksm/exact/flat) against real groups - Docker detect against real guacamole-postgres container - Full execute() pipeline: dry-run, output file, skip-users, include-disabled, custom project name, all folder modes - Error paths: wrong password, wrong db name, wrong port - Remote SSL enforcement, nonexistent Docker container Unit tests: - Gateway resolution: not found, found by name, offline warning - _find_config_for_gateway: match and no-match paths - Deploy instructions output verification - _is_local_host: localhost, IPv4/IPv6, RFC1918, public IPs - Docker container name passthrough (custom + default) - DB flag passthrough: port, db_name, db_user, default ports - Error paths: connection failure, schema failure, invalid port, Docker timeout, Docker not installed
Bugs found during live E2E testing against real vault: - PAMController protobuf has no `isOnline` attribute — replaced with router_get_connected_gateways() cross-reference to determine online status - PAMController has no `version` attribute — removed from display - Unused import (pam_configurations_get_all) replaced with router_helper Verified full E2E: DB connect → extract → transform → vault import → gateway creation → folder structure → shared folders → PAM config → KSM app. 8 records created successfully (3 resources, 3 users, 1 config, 1 app).
Live E2E tests now read DB connection details from environment variables (KCM_TEST_DB_HOST, KCM_TEST_DB_PASS, etc.) instead of hardcoded values. Tests auto-skip when env vars are not set. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When using --docker-detect, explicit CLI flags (--db-host, --db-port, --db-name, --db-user) now override the auto-detected values. This fixes the common case where the container's internal hostname (e.g. "db") is unreachable from the host running Commander. Also removes hardcoded credentials from live tests — connection details now come from environment variables (KCM_TEST_DB_HOST, etc.). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When _resolve_gateway found an existing gateway with a PAM config, it returned config_uid which switched the flow to extend mode. Extend mode requires all folder paths to already exist in the vault, causing 616 "could not be resolved" errors on fresh imports. Now: new imports (no --config flag) always use the import path which creates folders, KSM app, gateway, PAM config, then records. Extend mode is only used when the user explicitly passes --config. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…port statistics Folder modes: - Add 'qualified' mode: parent-qualified flat names with collision avoidance - Add --strip-root: omit ROOT prefix from folder paths - Add --group-depth N: collapse deep groups into ancestor at depth N - Add --exclude-groups: skip specific connection groups by name/pattern - Add --preview-groups: show folder tree without importing Gateway flow: - Wire up --gateway flag to reuse existing gateway (was declared but never called) - Add --deploy-gateway: auto-deploy gateway via Docker after creation - Add --gateway-name/--gateway-image for Docker customization - Capture and display gateway access token in deploy instructions - edit.py: return project dict from execute() to expose gateway token Security hardening: - Enhanced _sanitize(): null bytes, control chars, empty strings, length limit - Cycle detection in _find_ancestor_at_depth() and print_node() - Exact match in _find_pam_config_by_title() (was substring match) - Negative --group-depth validation - Counter-based fallback for qualified mode collisions - Unmatched --exclude-groups warning - Docker env cleanup in try/finally - Plaintext credential warning for --output - Connection name sanitization (strip control chars/null bytes) Import statistics: - Add timing and throughput display after import completion - Show project name, folder mode, record counts, elapsed time, records/sec Documentation: - Add full working E2E command examples to README - Document all folder modes, gateway options, and security notes Tests: - Update two-phase import test to match Phase 1 + Phase 2 architecture - Fix folder path assertions for project_name prefix
…esource safety Exception handling: - Split isinstance(e, CommandError) re-raise into two except clauses - Include exception message in DB connection error (was class name only) - Include exception message in pool size warning - Add debug logging to silent except-continue in _find_config_for_gateway - Add exception detail to router connectivity check debug log - Remove unreachable IndexError from except tuple - Chain exceptions with 'from e' for better tracebacks Input validation: - Fix argparse defaults overriding docker-detected db_name/db_user (default=None with fallback chain instead of hardcoded defaults) - Sanitize connection names for path separators and length (was only stripping control chars; now consistent with group name _sanitize) - Warn when --deploy-gateway or --max-instances used with --gateway/--config Resource safety: - Add __enter__/__exit__ to KCMDatabaseConnector for context manager use - Replace TOCTOU os.path.exists+unlink with try/except OSError Cleanup: - Remove redundant null byte removal in _sanitize (already filtered) - Add debug logging to 'log' mapping type (was silently returning)
…cleanup - Change passphrase and public-key mappings from 'log' to null (silent drop) - Pass param_name to _apply_single_mapping so debug logs show actual param - Replace remaining os.path.exists+unlink with try/except OSError
…s validation, port check - SEC-1: Gateway token now written to temp env-file (0o600) instead of docker run -e flag (was visible in ps aux / docker inspect) - SEC-3/SEC-4: Rewrite _is_local_host using ipaddress module; empty host returns False (was bypass), hostnames like 10.evil.com no longer match - SEC-5: Add -- separator before container names in docker commands - REV-2: Add sys.stdin.isatty() check before getpass (CI/CD safety) - REV-5: Fix Docker env prefix POSTGRES -> POSTGRESQL (Guacamole convention) - BUG-1: Add null guard after _resolve_gateway to prevent NoneType errors - REV-1: Add port range validation (1-65535) after resolution - REV-3: Remove dead sftp-password from redact set (actual key is password) - CQ-5: Use exact gateway name match instead of startswith prefix
- Add ESXi auto-detection based on connection name and hostname patterns (esxi, vmhost, vsphere, vcenter, vmware, hypervisor) - Move detected ESXi resources/users to dedicated "ESXi Hosts" subfolder - Enable text session recording for ESXi connections (CLI-based hosts) - Tag ESXi resource titles with [ESXi] prefix for clarity - Show ESXi host count in import statistics - Add 16 unit tests covering detection, enrichment, mixed scenarios
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
pam project kcm-importcommand for migrating KCM/Guacamole connections into Keeper PAM.--dry-run,--output,--docker-detect,--db-ssl,--db-password-recordTest plan