From b551fe17325f4cbfb742eff980c833416d3208bc Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 12:33:47 +0000 Subject: [PATCH 01/11] fix: write managed Host lines in one prefix case ssh matches Host patterns case-sensitively. Blocks are recognised as ours case-insensitively, so a config carrying a stale `Host cloudx-*` from an older release was adopted as the generic block and written back unchanged, above host entries spelled `cloudX-`. The generic block then matched nothing: line 15: Applying options for cloudX-DTA-* line 19: Applying options for cloudX-DTA-unified (no "Applying options for cloudX-*") so `IdentitiesOnly yes` and `User ec2-user` silently stopped applying. ssh then offered every key in the agent, the server hit MaxAuthTries, and the connection failed with "Too many authentication failures" before reaching the key EC2 Instance Connect had just pushed - on an otherwise healthy session that had already completed KEX and verified the host key. This is a regression from 0.17.2. Compared against v0.17.1 on the same input, the old code wrote `Host cloudX-*` and the new code wrote `Host cloudx-*`. It came from two changes meeting: matching the generic block case-insensitively, and keeping the first of any duplicate generic blocks. Together, a stale lowercase block was adopted and then evicted the correct-case one that `_check_and_create_generic_config` had just appended. A managed block's Host line is now written with the prefix in the case currently in use, so a file this tool writes never disagrees with itself. Inline comments survive, only the prefix is rewritten, and a host whose own name repeats the prefix spelling is left alone. Existing broken configs are repaired by `cloudX-proxy cleanup`, which already normalised the prefix across the whole file; this makes `setup` do the same rather than leaving the file half-converted. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NiMd9TR2SJ4PAY4tmSBWzd --- cloudx_proxy/setup.py | 44 +++++++++++--- tests/test_ssh_config.py | 123 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 7 deletions(-) diff --git a/cloudx_proxy/setup.py b/cloudx_proxy/setup.py index 0db6eff..90c253a 100644 --- a/cloudx_proxy/setup.py +++ b/cloudx_proxy/setup.py @@ -1000,13 +1000,43 @@ def _requote_path_directive(cls, line: str) -> str: return f'{indent}{keyword}{gap}"{value}"' - @classmethod - def _clean_managed_lines(cls, lines: list) -> list: + def _normalize_managed_host_line(self, line: str) -> str: + """Write a managed Host line with the prefix case currently in use. + + Blocks are recognised as ours case-insensitively, but ssh matches Host + patterns case-SENSITIVELY. Writing a block back in the case it happened + to have therefore produces a file whose parts disagree: a stale + `Host cloudx-*` sitting above `Host cloudX-dev-web1` never matches it, + so `IdentitiesOnly yes` and `User ec2-user` silently stop applying - + which lets ssh offer every agent key and hit the server's MaxAuthTries + before reaching the one that was just pushed. + + Args: + line: A managed block's Host line + + Returns: + str: The line with its prefix in the canonical case + """ + match = re.match(r'^(\s*)(host)(\s+)(.*)$', line, re.IGNORECASE) + if not match: + return line + + indent, keyword, gap, value = match.groups() + normalized = re.sub( + rf'^{re.escape(self.ssh_host_prefix)}-', + f'{self.ssh_host_prefix}-', + value, + count=1, + flags=re.IGNORECASE, + ) + return f'{indent}{keyword}{gap}{normalized}' + + def _clean_managed_lines(self, lines: list) -> list: """Strip comments and blank lines from a block cloudx-proxy manages. - The header line is kept verbatim so inline comments on Host entries - survive; everything below it is normalised, since those sections are - regenerated from scratch on every write. + The header line keeps its inline comment, but has its prefix put into + the case in use; everything below it is normalised, since those + sections are regenerated from scratch on every write. Args: lines: Raw lines of the block, header first @@ -1017,14 +1047,14 @@ def _clean_managed_lines(cls, lines: list) -> list: cleaned = [] for index, line in enumerate(lines): if index == 0: - cleaned.append(line.rstrip()) + cleaned.append(self._normalize_managed_host_line(line.rstrip())) continue stripped = line.strip() if not stripped or stripped.startswith('#'): continue if '#' in line: line = line.split('#')[0].rstrip() - line = cls._requote_path_directive(line) + line = self._requote_path_directive(line) cleaned.append(line.rstrip()) return cleaned diff --git a/tests/test_ssh_config.py b/tests/test_ssh_config.py index a533153..4ca782f 100644 --- a/tests/test_ssh_config.py +++ b/tests/test_ssh_config.py @@ -468,3 +468,126 @@ def test_keeps_ordinary_comments(self, setup): lines = ["# one", "# two", "# three"] assert setup._strip_generated_banners(lines) == lines + + +class TestManagedHostLinesShareOnePrefixCase: + """ssh matches Host patterns case-sensitively. + + Blocks are recognised as ours case-insensitively, so a config carrying a + stale `Host cloudx-*` from an older release was written back unchanged and + then never matched `cloudX-dev-web1`. The generic block's `IdentitiesOnly + yes` and `User ec2-user` silently stopped applying, so ssh offered every + key in the agent and hit the server's MaxAuthTries before reaching the one + just pushed - surfacing as "Too many authentication failures" on a + connection that was otherwise healthy. + """ + + MIXED_CASE = """# SSH Configuration - Managed by cloudx-proxy v0.16.15 + +Host cloudx-* + User ec2-user + TCPKeepAlive yes + IdentitiesOnly yes + +Host cloudX-DTA-* + IdentityFile ~/.ssh/cloudX/cloudX + ProxyCommand uvx cloudX-proxy connect %h %p --profile cloudx + +Host cloudX-DTA-unified + HostName i-095f07267c26a685c +""" + + def uppercase_setup(self, tmp_path): + ssh_dir = tmp_path / "cloudX" + ssh_dir.mkdir(parents=True, exist_ok=True) + (ssh_dir / "config").write_text(self.MIXED_CASE) + return CloudXSetup( + ssh_dir=str(ssh_dir), ssh_host_prefix="cloudX", non_interactive=True + ) + + def managed_host_lines(self, setup): + return [ + line for line in setup.ssh_config_file.read_text().splitlines() + if line.startswith("Host ") + ] + + def test_adding_a_host_normalises_the_stale_generic_block(self, tmp_path): + setup = self.uppercase_setup(tmp_path) + + setup.setup_ssh_config("DTA", "i-0123456789abcdef0", "web2") + + lines = self.managed_host_lines(setup) + assert "Host cloudX-*" in lines + assert "Host cloudx-*" not in lines + + def test_cleanup_normalises_it_too(self, tmp_path): + setup = self.uppercase_setup(tmp_path) + + setup.cleanup_config() + + lines = self.managed_host_lines(setup) + assert "Host cloudX-*" in lines + assert "Host cloudx-*" not in lines + + def test_every_managed_host_line_uses_one_case(self, tmp_path): + """The invariant: a written file never disagrees with itself.""" + setup = self.uppercase_setup(tmp_path) + + setup.setup_ssh_config("DTA", "i-0123456789abcdef0", "web2") + + for line in self.managed_host_lines(setup): + assert line.startswith("Host cloudX-"), f"inconsistent prefix case: {line!r}" + + def test_the_generic_block_keeps_its_directives(self, tmp_path): + """Normalising the header must not disturb what the block contains.""" + setup = self.uppercase_setup(tmp_path) + + setup.setup_ssh_config("DTA", "i-0123456789abcdef0", "web2") + + result = setup.ssh_config_file.read_text() + for directive in ("User ec2-user", "TCPKeepAlive yes", "IdentitiesOnly yes"): + assert directive in result + + def test_lowercase_invocation_normalises_the_other_way(self, tmp_path): + ssh_dir = tmp_path / "cloudx" + ssh_dir.mkdir(parents=True) + (ssh_dir / "config").write_text("""Host cloudX-* + User ec2-user + IdentitiesOnly yes + +Host cloudx-dev-* + ProxyCommand uvx cloudx-proxy connect %h %p + +Host cloudx-dev-web1 + HostName i-0123456789abcdef0 +""") + setup = CloudXSetup( + ssh_dir=str(ssh_dir), ssh_host_prefix="cloudx", non_interactive=True + ) + + setup.cleanup_config() + + for line in self.managed_host_lines(setup): + assert line.startswith("Host cloudx-"), f"inconsistent prefix case: {line!r}" + + +class TestNormalizeManagedHostLine: + def test_wrong_case_prefix_is_corrected(self, setup): + assert setup._normalize_managed_host_line("Host cloudX-dev-web1") == "Host cloudx-dev-web1" + + def test_correct_case_is_untouched(self, setup): + assert setup._normalize_managed_host_line("Host cloudx-dev-web1") == "Host cloudx-dev-web1" + + def test_inline_comment_survives(self, setup): + assert setup._normalize_managed_host_line( + "Host cloudX-dev-web1 # erik's box" + ) == "Host cloudx-dev-web1 # erik's box" + + def test_only_the_prefix_is_touched(self, setup): + """A host whose own name contains the prefix spelling is not rewritten.""" + assert setup._normalize_managed_host_line( + "Host cloudX-dev-cloudX-thing" + ) == "Host cloudx-dev-cloudX-thing" + + def test_a_non_host_line_is_untouched(self, setup): + assert setup._normalize_managed_host_line(" User ec2-user") == " User ec2-user" From 5958bbaa4db313d0ec4cd5aa045f5a39f08a0ec3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 12:37:34 +0000 Subject: [PATCH 02/11] fix: cleanup no longer deletes ProxyCommand flags The ProxyCommand rebuild exists to drop flags that merely restate an auto-detected default, but it recomputes those defaults from the running process rather than from the line it is rewriting. Left to itself it deletes settings that are doing real work. Reported from the field: a config carrying `--profile cloudx` came out of cleanup with no --profile at all, because the default detected here is `cloudX`. AWS profile names are case-sensitive, so the next connection failed with "The config profile (cloudX) could not be found". `--region` was worse - the rebuild has no notion of it, so it was dropped outright. A flag the rebuild does not reproduce is now carried over rather than discarded. Cleanup may tidy a configuration; it may not decide that part of it was unnecessary. This does not change how a ProxyCommand is generated for a new environment - flags matching a detected default are still omitted there - only that rewriting an existing line cannot lose what it already said. This behaviour predates the previous release: v0.17.1 strips the flag identically. It is longstanding data loss in the same class as the rest of this file's recent fixes, not a regression. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NiMd9TR2SJ4PAY4tmSBWzd --- cloudx_proxy/setup.py | 59 +++++++++++++++------ tests/test_ssh_config.py | 109 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 151 insertions(+), 17 deletions(-) diff --git a/cloudx_proxy/setup.py b/cloudx_proxy/setup.py index 90c253a..0b80bdd 100644 --- a/cloudx_proxy/setup.py +++ b/cloudx_proxy/setup.py @@ -1591,6 +1591,47 @@ def _add_host_entry(self, cloudx_env: str, instance_id: str, hostname: str, curr self.print_status(f"{color_error('Error:', bold=True)} {e!s}", False, 2) return self.confirm_continue_after_error("SSH config issues") + # Flags a ProxyCommand can carry, with their values quoted or bare + _PROXY_FLAG_RE = re.compile(r'''(--[\w-]+)\s+("[^"]*"|'[^']*'|[^\s-]\S*)''') + + def _rebuild_proxy_command(self, line: str) -> str: + """Rebuild a ProxyCommand, keeping every flag it already carried. + + The rebuild exists to drop flags that merely restate an auto-detected + default, but it recomputes them from this process's own defaults, which + are not the ones the line was written with. Left to itself it deletes + settings that are doing real work: an explicit `--profile cloudx` was + dropped because the default detected here is `cloudX`, and since AWS + profile names are case-sensitive the next connection failed with "The + config profile (cloudX) could not be found". + + A flag the rebuild does not reproduce is therefore carried over rather + than discarded. Cleanup may tidy the configuration; it may not decide + that part of it was unnecessary. + + Args: + line: The existing ProxyCommand line + + Returns: + str: The rebuilt command, with nothing lost + """ + existing = dict(self._PROXY_FLAG_RE.findall(line)) + + # aws-env cannot be derived from anything here, so it is fed back in + original_aws_env = self.aws_env + aws_env = existing.get('--aws-env') + self.aws_env = aws_env.strip('"\'') if aws_env else None + try: + rebuilt = self._build_proxy_command() + finally: + self.aws_env = original_aws_env + + for flag, value in existing.items(): + if flag not in rebuilt: + rebuilt += f" {flag} {value}" + + return rebuilt + def cleanup_config(self) -> bool: """Clean up and reorganize SSH configuration file. @@ -1645,23 +1686,7 @@ def cleanup_config(self) -> bool: new_lines = [] for line in env_lines: if line.strip().startswith('ProxyCommand'): - # Extract aws-env from the existing ProxyCommand if - # present, quoted or not - aws_env = None - if '--aws-env' in line: - match = re.search( - r'''--aws-env\s+("[^"]*"|'[^']*'|\S+)''', line - ) - if match: - aws_env = match.group(1).strip('"\'') - - # Temporarily set aws_env for ProxyCommand building - original_aws_env = self.aws_env - self.aws_env = aws_env - optimized_command = self._build_proxy_command() - self.aws_env = original_aws_env - - new_lines.append(f" ProxyCommand {optimized_command}") + new_lines.append(f" ProxyCommand {self._rebuild_proxy_command(line)}") else: new_lines.append(line) diff --git a/tests/test_ssh_config.py b/tests/test_ssh_config.py index 4ca782f..3f75adf 100644 --- a/tests/test_ssh_config.py +++ b/tests/test_ssh_config.py @@ -591,3 +591,112 @@ def test_only_the_prefix_is_touched(self, setup): def test_a_non_host_line_is_untouched(self, setup): assert setup._normalize_managed_host_line(" User ec2-user") == " User ec2-user" + + +class TestCleanupKeepsProxyCommandFlags: + """cleanup rebuilt the ProxyCommand and silently deleted flags. + + The rebuild drops flags that merely restate an auto-detected default, but + it recomputes those defaults from the running process rather than from the + line it is rewriting. An explicit `--profile cloudx` was therefore removed + because the default detected here is `cloudX`; AWS profile names are + case-sensitive, so the next connection died with "The config profile + (cloudX) could not be found". `--region` was dropped outright, since the + rebuild has no notion of it at all. + """ + + def cleaned(self, tmp_path, proxy_command): + home = tmp_path / "home" + ssh_dir = home / ".ssh" / "cloudX" + ssh_dir.mkdir(parents=True) + (ssh_dir / "config").write_text(f"""Host cloudX-* + User ec2-user + +Host cloudX-dev-* + ProxyCommand {proxy_command} + +Host cloudX-dev-web1 + HostName i-0123456789abcdef0 +""") + setup = CloudXSetup( + ssh_config=str(ssh_dir / "config"), ssh_host_prefix="cloudX", + non_interactive=True, + ) + setup.home_dir = str(home) + assert setup.cleanup_config() is True + for line in setup.ssh_config_file.read_text().splitlines(): + if line.strip().startswith("ProxyCommand"): + return line.strip() + raise AssertionError("ProxyCommand disappeared entirely") + + def test_an_explicit_profile_is_kept(self, tmp_path): + result = self.cleaned( + tmp_path, "uvx cloudX-proxy connect %h %p --profile cloudx" + ) + assert "--profile cloudx" in result + + def test_an_explicit_ssh_key_is_kept(self, tmp_path): + result = self.cleaned( + tmp_path, "uvx cloudX-proxy connect %h %p --ssh-key mykey" + ) + assert "--ssh-key mykey" in result + + def test_region_is_kept(self, tmp_path): + """The rebuild never emits --region, so it used to vanish.""" + result = self.cleaned( + tmp_path, "uvx cloudX-proxy connect %h %p --region eu-central-1" + ) + assert "--region eu-central-1" in result + + def test_aws_env_is_still_kept(self, tmp_path): + result = self.cleaned( + tmp_path, "uvx cloudX-proxy connect %h %p --aws-env prod" + ) + assert "--aws-env prod" in result + + def test_several_flags_all_survive(self, tmp_path): + result = self.cleaned( + tmp_path, + "uvx cloudX-proxy connect %h %p --profile acme --ssh-key mykey " + "--region eu-central-1 --aws-env prod", + ) + for flag in ("--profile acme", "--ssh-key mykey", + "--region eu-central-1", "--aws-env prod"): + assert flag in result, f"{flag} was dropped" + + def test_no_flag_is_duplicated(self, tmp_path): + result = self.cleaned( + tmp_path, "uvx cloudX-proxy connect %h %p --aws-env prod" + ) + assert result.count("--aws-env") == 1 + + def test_a_bare_command_stays_bare(self, tmp_path): + """Nothing to preserve means nothing is invented.""" + result = self.cleaned(tmp_path, "uvx cloudX-proxy connect %h %p") + assert result == "ProxyCommand uvx cloudX-proxy connect %h %p" + + def test_repeated_cleanup_is_stable(self, tmp_path): + home = tmp_path / "home" + ssh_dir = home / ".ssh" / "cloudX" + ssh_dir.mkdir(parents=True) + (ssh_dir / "config").write_text("""Host cloudX-* + User ec2-user + +Host cloudX-dev-* + ProxyCommand uvx cloudX-proxy connect %h %p --profile cloudx --region eu-west-3 + +Host cloudX-dev-web1 + HostName i-0123456789abcdef0 +""") + setup = CloudXSetup( + ssh_config=str(ssh_dir / "config"), ssh_host_prefix="cloudX", + non_interactive=True, + ) + setup.home_dir = str(home) + + setup.cleanup_config() + once = setup.ssh_config_file.read_text() + setup.cleanup_config() + + assert setup.ssh_config_file.read_text() == once + assert once.count("--profile cloudx") == 1 From 8ae48605643daded0f40850a5e53446940da2fe4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 16:42:27 +0000 Subject: [PATCH 03/11] fix: find the 1Password SSH agent per platform ~/.1password/agent.sock was treated as the one place the agent lives. It is a Unix socket path, so on Windows the check never passed and --1password silently fell back to a plain on-disk key; had it passed, the config would have carried an IdentityAgent pointing at nothing and broken an agent that already worked. Windows needs no directive at all: 1Password serves the standard OpenSSH named pipe \\.\pipe\openssh-ssh-agent, which ssh uses by default. Check the pipe there, keep the socket (and the macOS symlink handling) elsewhere, add the Linux snap location, and write IdentityAgent only where one has to be named. Output on macOS and Linux is unchanged. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NiMd9TR2SJ4PAY4tmSBWzd --- .ai/context/ssh-config.md | 9 ++- README.md | 4 +- cloudx_proxy/setup.py | 142 ++++++++++++++++++++++++++++------- tests/test_runtime_safety.py | 78 +++++++++++++++++++ 4 files changed, 203 insertions(+), 30 deletions(-) diff --git a/.ai/context/ssh-config.md b/.ai/context/ssh-config.md index 5b6a328..ebd8d92 100644 --- a/.ai/context/ssh-config.md +++ b/.ai/context/ssh-config.md @@ -66,7 +66,14 @@ Host cloudX-Prod-* **Settings explained**: - `IdentityFile`: Path to the SSH private key for this environment -- `IdentitiesOnly yes`: Only use the specified key, don't try others from ssh-agent +- `IdentitiesOnly yes`: Only use the specified key, don't try others from ssh-agent. + With 1Password this is why `IdentityFile` names the *public* key: ssh offers only + identities named by `IdentityFile`, even when the agent holds more, so the `.pub` + is what lets the agent's copy of the key be used at all. +- `IdentityAgent`: Only written with `--1password`, and only where the agent has to be + named: `~/.1password/agent.sock` on macOS and Linux (or the snap path on snap + installs). Nothing is written on Windows, where 1Password serves the standard + OpenSSH named pipe `\\.\pipe\openssh-ssh-agent` that ssh already uses by default. - `ProxyCommand`: The cloudX-proxy connect command that: - Checks if the instance is running (starts it if needed) - Pushes the SSH public key to the instance diff --git a/README.md b/README.md index df805c6..c88f3a1 100644 --- a/README.md +++ b/README.md @@ -675,7 +675,9 @@ These permissions are required to bootstrap the instance, so that after creation - **Region mismatch** - Ensure AWS profile region matches instance location 4. **SSH Key Issues** - - If using 1Password SSH agent, verify agent is running (~/.1password/agent.sock exists) + - If using 1Password SSH agent, verify the agent is running. Where it lives depends on the platform: + * macOS/Linux: `~/.1password/agent.sock` exists (Linux snap installs: `~/snap/1password/current/.1password/agent.sock`) + * Windows: 1Password serves the standard OpenSSH named pipe `\\.\pipe\openssh-ssh-agent`, which ssh uses by default - there is no socket file and no `IdentityAgent` line is written - Check file permissions (600 for private key, 644 for public key) - Verify the public key is being successfully pushed to the instance - For 1Password-managed keys, make sure: diff --git a/cloudx_proxy/setup.py b/cloudx_proxy/setup.py index 0b80bdd..b7c8bcf 100644 --- a/cloudx_proxy/setup.py +++ b/cloudx_proxy/setup.py @@ -239,6 +239,12 @@ def __init__(self, profile: str = "cloudX", ssh_key: str = "cloudX", ssh_config: self.home_dir = str(Path.home()) self.op_agent_sock = Path(self.home_dir) / ".1password" / "agent.sock" self.op_agent_sock_macos = Path(self.home_dir) / "Library" / "Group Containers" / "2BUA8C4S2C.com.1password" / "t" / "agent.sock" + # The Linux snap package keeps its agent inside the snap's own home + # rather than at ~/.1password/agent.sock. + self.op_agent_sock_snap = Path(self.home_dir) / "snap" / "1password" / "current" / ".1password" / "agent.sock" + # Set by _check_op_agent() when the agent was found somewhere other + # than the default socket. None means "write the default". + self.op_agent_override = None self.pending_migration = False @@ -267,6 +273,95 @@ def __init__(self, profile: str = "cloudX", ssh_key: str = "cloudX", ssh_config: self.ssh_key_file = self.ssh_dir / f"{ssh_key}" self.default_env = None + # On Windows the 1Password SSH agent serves the standard OpenSSH named + # pipe, which ssh talks to by default. There is no socket file to find + # and no IdentityAgent directive to write. + OP_AGENT_PIPE_WINDOWS = r'\\.\pipe\openssh-ssh-agent' + + def _windows_agent_pipe_exists(self) -> bool: + """Check whether an SSH agent is listening on the standard OpenSSH pipe.""" + pipe_name = self.OP_AGENT_PIPE_WINDOWS.rsplit('\\', 1)[-1] + try: + return pipe_name in os.listdir(r'\\.\pipe') + except OSError: + # Enumerating the pipe namespace is not guaranteed to work; fall + # back to probing the pipe directly. + return os.path.exists(self.OP_AGENT_PIPE_WINDOWS) + + def _check_op_agent(self) -> bool: + """Check that the 1Password SSH agent is reachable on this platform. + + Sets self.op_agent_override when the agent was found somewhere other + than ~/.1password/agent.sock, so the SSH config points at the socket + that actually exists. + + Returns: + bool: True if an agent was found. + """ + system = platform.system() + + if system == 'Windows': + if not self._windows_agent_pipe_exists(): + self.print_status( + f"No SSH agent listening on {self.OP_AGENT_PIPE_WINDOWS}", False, 2 + ) + self.print_status( + "Enable the SSH agent in 1Password: Settings > Developer > Use the SSH agent", + None, + 2, + ) + self.print_status("1Password integration is not supported in this configuration", False, 2) + return False + self.print_status("1Password SSH agent pipe is available", True, 2) + return True + + if self.op_agent_sock.exists(): + if system == 'Darwin' and self.op_agent_sock.is_symlink(): + try: + current_target = self.op_agent_sock.resolve(strict=False) + except FileNotFoundError: + current_target = None + + if current_target != self.op_agent_sock_macos and self.op_agent_sock_macos.exists(): + self.print_status("Updating 1Password agent symlink to default location", None, 2) + if not self._ensure_op_agent_symlink(): + self.print_status("1Password integration is not supported in this configuration", False, 2) + return False + + self.print_status("1Password SSH agent socket is available", True, 2) + return True + + self.print_status("1Password SSH agent socket not found at ~/.1password/agent.sock", False, 2) + + if system == 'Darwin': + if self._ensure_op_agent_symlink(): + self.print_status("1Password SSH agent socket is available", True, 2) + return True + elif self.op_agent_sock_snap.exists(): + # Snap install: use the snap's socket where it is rather than + # symlinking into a home directory the snap does not see. + self.op_agent_override = "~/snap/1password/current/.1password/agent.sock" + self.print_status("Using the snap 1Password SSH agent socket", True, 2) + return True + + self.print_status("1Password SSH agent is not available", False, 2) + self.print_status("Please ensure 1Password SSH agent is enabled in 1Password settings", None, 2) + self.print_status("1Password integration is not supported in this configuration", False, 2) + return False + + def _op_identity_agent(self) -> str | None: + """The IdentityAgent value for the 1Password agent, or None to omit it. + + Nothing is written on Windows: ssh reaches the 1Password agent over the + standard OpenSSH named pipe without being told to, and pointing + IdentityAgent at a Unix socket path there would aim ssh at nothing and + break an agent that already works. + """ + if platform.system() == 'Windows': + return None + # A literal tilde, left for ssh to expand. + return self.op_agent_override or "~/.1password/agent.sock" + def _ensure_op_agent_symlink(self) -> bool: """Ensure ~/.1password/agent.sock points to the macOS agent location.""" if platform.system() != 'Darwin': @@ -581,28 +676,10 @@ def _check_op_availability(self) -> bool: self.print_status("1Password CLI is authenticated", True, 2) - # Check if 1Password SSH agent socket exists at ~/.1password/agent.sock - if not self.op_agent_sock.exists(): - self.print_status("1Password SSH agent socket not found at ~/.1password/agent.sock", False, 2) - - if not self._ensure_op_agent_symlink(): - self.print_status("1Password SSH agent is not available", False, 2) - self.print_status("Please ensure 1Password SSH agent is enabled in 1Password settings", None, 2) - self.print_status("1Password integration is not supported in this configuration", False, 2) - return False - elif platform.system() == 'Darwin' and self.op_agent_sock.is_symlink(): - try: - current_target = self.op_agent_sock.resolve(strict=False) - except FileNotFoundError: - current_target = None - - if current_target != self.op_agent_sock_macos and self.op_agent_sock_macos.exists(): - self.print_status("Updating 1Password agent symlink to default location", None, 2) - if not self._ensure_op_agent_symlink(): - self.print_status("1Password integration is not supported in this configuration", False, 2) - return False - - self.print_status("1Password SSH agent socket is available", True, 2) + # Check that the 1Password SSH agent is reachable. Where it lives is + # platform-specific, so this is not a single path test. + if not self._check_op_agent(): + return False # If using a vault other than "Private", warn the user if self.op_vault and self.op_vault != "Private": @@ -895,12 +972,21 @@ def _build_auth_config(self) -> str: """ if self.op_enabled: # When using 1Password: - # 1. Set IdentityAgent to the 1Password socket (literal tilde for SSH compatibility) - # 2. Set IdentityFile to the PUBLIC key (.pub) to limit key search - # (IdentitiesOnly is now set globally for all cloudX hosts) - return """ IdentityAgent ~/.1password/agent.sock - IdentityFile {} -""".format(self.quote_config_value(f"{self.ssh_key_file}.pub")) + # 1. Point IdentityAgent at the 1Password socket, where one has to + # be named at all - see _op_identity_agent() + # 2. Set IdentityFile to the PUBLIC key (.pub) to limit key search. + # IdentitiesOnly yes is set globally for all cloudX hosts, and + # ssh offers only identities named by IdentityFile even when the + # agent holds more - so the .pub is what lets the agent's copy of + # the key be used at all. + lines = [] + identity_agent = self._op_identity_agent() + if identity_agent: + lines.append(f" IdentityAgent {identity_agent}") + lines.append( + f" IdentityFile {self.quote_config_value(f'{self.ssh_key_file}.pub')}" + ) + return "\n".join(lines) + "\n" else: # Standard SSH key configuration # (IdentitiesOnly is now set globally for all cloudX hosts) diff --git a/tests/test_runtime_safety.py b/tests/test_runtime_safety.py index 1ccd28a..c6192f9 100644 --- a/tests/test_runtime_safety.py +++ b/tests/test_runtime_safety.py @@ -265,3 +265,81 @@ def test_does_nothing_off_macos(self, tmp_path, monkeypatch): setup = CloudXSetup(ssh_dir=str(tmp_path / "ssh"), op_vault="Private") assert setup._ensure_op_agent_symlink() is False + + +class TestOpAgentPerPlatform: + """The 1Password SSH agent does not live in the same place everywhere. + + ~/.1password/agent.sock is a Unix socket path. On Windows there is no + such socket: 1Password serves the standard OpenSSH named pipe, which ssh + already uses by default. Writing the Unix path into the SSH config there + aims ssh at nothing and breaks an agent that was working. + """ + + def _setup(self, tmp_path, system, monkeypatch): + monkeypatch.setattr("cloudx_proxy.setup.platform.system", lambda: system) + return CloudXSetup(ssh_dir=str(tmp_path / "ssh"), op_vault="Private") + + def test_windows_writes_no_identity_agent(self, tmp_path, monkeypatch): + setup = self._setup(tmp_path, "Windows", monkeypatch) + + assert setup._op_identity_agent() is None + auth = setup._build_auth_config() + assert "IdentityAgent" not in auth + assert "IdentityFile" in auth + assert auth.endswith("\n") + + def test_unix_keeps_the_default_socket(self, tmp_path, monkeypatch): + for system in ("Darwin", "Linux"): + setup = self._setup(tmp_path, system, monkeypatch) + assert setup._op_identity_agent() == "~/.1password/agent.sock" + assert " IdentityAgent ~/.1password/agent.sock\n" in setup._build_auth_config() + + def test_snap_socket_is_used_where_it_lives(self, tmp_path, monkeypatch): + setup = self._setup(tmp_path, "Linux", monkeypatch) + setup.op_agent_sock = tmp_path / "dot1password" / "agent.sock" + setup.op_agent_sock_snap = tmp_path / "snap" / "agent.sock" + setup.op_agent_sock_snap.parent.mkdir(parents=True, exist_ok=True) + setup.op_agent_sock_snap.write_text("") # stand-in for the socket + + assert setup._check_op_agent() is True + assert setup._op_identity_agent() == "~/snap/1password/current/.1password/agent.sock" + + def test_linux_without_a_socket_fails(self, tmp_path, monkeypatch): + setup = self._setup(tmp_path, "Linux", monkeypatch) + setup.op_agent_sock = tmp_path / "dot1password" / "agent.sock" + setup.op_agent_sock_snap = tmp_path / "snap" / "agent.sock" + + assert setup._check_op_agent() is False + + def test_windows_agent_found_via_the_pipe(self, tmp_path, monkeypatch): + setup = self._setup(tmp_path, "Windows", monkeypatch) + monkeypatch.setattr( + "cloudx_proxy.setup.os.listdir", + lambda path: ["openssh-ssh-agent", "chrome.sync"], + ) + + assert setup._check_op_agent() is True + # No socket file exists on Windows, so a missing one must not be + # what the check reports on. + assert setup.op_agent_override is None + + def test_windows_without_an_agent_fails(self, tmp_path, monkeypatch): + setup = self._setup(tmp_path, "Windows", monkeypatch) + monkeypatch.setattr("cloudx_proxy.setup.os.listdir", lambda path: ["chrome.sync"]) + + assert setup._check_op_agent() is False + + def test_windows_falls_back_to_probing_the_pipe(self, tmp_path, monkeypatch): + setup = self._setup(tmp_path, "Windows", monkeypatch) + + def no_enumeration(path): + raise OSError("cannot enumerate the pipe namespace") + + monkeypatch.setattr("cloudx_proxy.setup.os.listdir", no_enumeration) + monkeypatch.setattr( + "cloudx_proxy.setup.os.path.exists", + lambda path: path == setup.OP_AGENT_PIPE_WINDOWS, + ) + + assert setup._windows_agent_pipe_exists() is True From 4532ecf97d31c4cfa06181a1c19675b53bf56c31 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 07:50:06 +0000 Subject: [PATCH 04/11] fix: wildcard Host blocks match every spelling of the prefix ssh matches Host patterns case-sensitively, and its pattern syntax has only * and ? - `cloud[xX]-*` is a literal hostname, not a character class. So a block written as `Host cloudX-*` does not apply to a host entry spelled `cloudx-dev-web1`, and both spellings are out there: two command names, older releases, and hand edits all put them in the same file. When they disagree the generic block stops applying, which is how `IdentitiesOnly yes` and `User ec2-user` go missing and ssh runs into MaxAuthTries. Normalising the case (the previous fix) keeps a file we wrote consistent, but only refuses to add to a mixed one. A Host line takes any number of patterns and matches if any of them does, so wildcard blocks now list one pattern per spelling: `Host cloudX-* cloudx-*`. Host entries keep a single name in the configured case - they are what `list` reports and what VSCode offers. The parser has to recognise its own output: a multi-pattern Host line was classed as the user's, so without this a rewrite would file our own block under "not managed" and generate a duplicate. Patterns that differ by more than case still name different hosts and stay the user's. Verified against OpenSSH 9.6 with `ssh -G`: both spellings of a host entry resolve User, IdentitiesOnly, IdentityFile and ProxyCommand from the widened blocks. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NiMd9TR2SJ4PAY4tmSBWzd --- .ai/context/ssh-config.md | 20 ++++- README.md | 21 +++-- cloudx_proxy/setup.py | 136 ++++++++++++++++++++++++---- tests/test_backward_compat.py | 64 ++++++++++--- tests/test_ssh_config.py | 165 ++++++++++++++++++++++++++++++++-- 5 files changed, 362 insertions(+), 44 deletions(-) diff --git a/.ai/context/ssh-config.md b/.ai/context/ssh-config.md index ebd8d92..570bdc0 100644 --- a/.ai/context/ssh-config.md +++ b/.ai/context/ssh-config.md @@ -108,13 +108,29 @@ Host cloudX-Prod-foobar - The `Environment` tag determines the environment part - The `Name` tag (or user-specified hostname) determines the hostname part +## Prefix Case + +ssh matches `Host` **patterns** case-sensitively, and the pattern syntax supports +only `*` and `?` - `cloud[xX]-*` is a literal hostname, not a character class. A +block written as `Host cloudX-*` therefore does not apply to a host entry spelled +`cloudx-dev-web1`, and both spellings exist in the wild (two command names, older +releases, hand edits). + +Wildcard blocks are consequently written with one pattern per prefix spelling - +`Host cloudX-* cloudx-*` - since a `Host` line takes any number of patterns and +matches if any one of them does. The configured spelling always comes first. + +Host entries keep a single name in the configured case: they are what `list` +reports and what VSCode offers, and a second spelling of the prefix would not help +anyone typing a different case further along the name. + ## Configuration Inheritance SSH applies configurations from most specific to least specific. When connecting to `cloudX-Prod-foobar`: 1. **Host cloudX-Prod-foobar** matches first → sets `HostName` -2. **Host cloudX-Prod-*** matches next → sets `IdentityFile`, `IdentitiesOnly`, `ProxyCommand` -3. **Host cloudX-*** matches last → sets `User`, `TCPKeepAlive`, `Control*` +2. **Host cloudX-Prod-* cloudx-Prod-*** matches next → sets `IdentityFile`, `IdentitiesOnly`, `ProxyCommand` +3. **Host cloudX-* cloudx-*** matches last → sets `User`, `TCPKeepAlive`, `Control*` The result is a fully configured connection with all necessary settings. diff --git a/README.md b/README.md index c88f3a1..81f0cad 100644 --- a/README.md +++ b/README.md @@ -252,7 +252,7 @@ Will create a three-tier configuration structure like this: # Generic configuration (shared by all environments) # Created by cloudX-proxy v1.0.0 on 2025-03-07 09:05:23 # Configuration type: generic -Host cloudX-* +Host cloudX-* cloudx-* User ec2-user TCPKeepAlive yes ControlMaster auto @@ -262,7 +262,7 @@ Host cloudX-* # Environment configuration (specific to a single environment) # Created by cloudX-proxy v1.0.0 on 2025-03-07 09:05:23 # Configuration type: environment -Host cloudX-dev-* +Host cloudX-dev-* cloudx-dev-* IdentityFile ~/.ssh/cloudX/mykey IdentitiesOnly yes ProxyCommand uvx cloudX-proxy connect %h %p --profile myprofile --ssh-key mykey --ssh-dir ~/.ssh/cloudX @@ -429,8 +429,13 @@ Understanding the connection flow helps with troubleshooting and explains why ce ### Command Line Options > **Note:** Both `cloudX-proxy` (preferred) and `cloudx-proxy` command names are available. The command name determines the prefix case used in SSH configurations: -> - `cloudX-proxy` generates `Host cloudX-*` patterns (preferred) -> - `cloudx-proxy` generates `Host cloudx-*` patterns +> - `cloudX-proxy` generates `Host cloudX-* cloudx-*` patterns (preferred) +> - `cloudx-proxy` generates `Host cloudx-* cloudX-*` patterns +> +> ssh matches `Host` patterns case-sensitively, and its pattern syntax has only +> `*` and `?` - there is no `[xX]` character class. Wildcard blocks therefore list +> both spellings, so a config that mixes them keeps working; the first pattern is +> the one the command name chose. Host entries name one host and keep one name. > > Run `cleanup` with your preferred command to normalize existing configurations. @@ -568,8 +573,12 @@ uvx cloudX-proxy cleanup --ssh-config ~/.ssh/custom/config ``` **Prefix Normalization:** The cleanup command normalizes all host patterns and ProxyCommand references to match the command name used: -- Running `cloudX-proxy cleanup` converts `Host cloudx-*` → `Host cloudX-*` and `uvx cloudx-proxy` → `uvx cloudX-proxy` -- Running `cloudx-proxy cleanup` converts `Host cloudX-*` → `Host cloudx-*` and `uvx cloudX-proxy` → `uvx cloudx-proxy` +- Running `cloudX-proxy cleanup` converts `Host cloudx-*` → `Host cloudX-* cloudx-*` and `uvx cloudx-proxy` → `uvx cloudX-proxy` +- Running `cloudx-proxy cleanup` converts `Host cloudX-*` → `Host cloudx-* cloudX-*` and `uvx cloudX-proxy` → `uvx cloudx-proxy` + +Wildcard blocks keep both spellings so that host entries left behind in the other +case still pick up `User`, `IdentitiesOnly` and the `ProxyCommand`; host entries +themselves are renamed to the chosen case. This allows users to easily convert between naming conventions. The preferred convention is `cloudX` (uppercase X). diff --git a/cloudx_proxy/setup.py b/cloudx_proxy/setup.py index b7c8bcf..c44bb5a 100644 --- a/cloudx_proxy/setup.py +++ b/cloudx_proxy/setup.py @@ -1086,8 +1086,79 @@ def _requote_path_directive(cls, line: str) -> str: return f'{indent}{keyword}{gap}"{value}"' + def _prefix_spellings(self) -> list[str]: + """The spellings of the host prefix a managed block should answer to. + + The configured one first, then its lowercase form, then - for this + project's own prefix - the counterpart spelling, so a config written + as cloudx also answers to cloudX and the other way round. + """ + spellings = [self.ssh_host_prefix] + counterpart = 'cloudX' if self.ssh_host_prefix.lower() == 'cloudx' else None + for candidate in (self.ssh_host_prefix.lower(), counterpart): + if candidate and candidate not in spellings: + spellings.append(candidate) + return spellings + + def _host_pattern_variants(self, pattern: str) -> list[str]: + """Every spelling of a wildcard Host pattern to write, canonical first. + + ssh matches Host patterns case-SENSITIVELY, and its pattern syntax has + only '*' and '?' - there is no [xX] character class, so `cloud[xX]-*` + matches a host literally called that and nothing else. A block written + as `Host cloudX-*` therefore simply does not apply to a host entry + spelled `cloudx-dev-web1`, and both spellings exist in the wild: they + have been written by different versions of this tool, by the two + command names, and by hand. + + A Host line takes any number of patterns and matches if any one of them + does, so wildcard blocks list every spelling rather than betting on one. + + Args: + pattern: A Host pattern, e.g. 'cloudX-dev-*' + + Returns: + list[str]: The patterns to write, canonical spelling first. Only + the prefix varies; everything after it is left exactly as given. + """ + prefix_len = len(self.ssh_host_prefix) + if pattern[:prefix_len + 1].lower() != f"{self.ssh_host_prefix.lower()}-": + return [pattern] + + rest = pattern[prefix_len:] + variants = [] + for spelling in self._prefix_spellings(): + candidate = f"{spelling}{rest}" + if candidate not in variants: + variants.append(candidate) + return variants + + def _host_line_value(self, pattern: str) -> str: + """The Host line value for a pattern: every spelling, space separated.""" + return ' '.join(self._host_pattern_variants(pattern)) + + def _collapse_host_patterns(self, patterns: list[str]) -> str | None: + """Collapse a Host line's patterns to the single pattern they spell. + + Our wildcard blocks carry one pattern per prefix spelling, so their Host + lines list more than one. That is still one block - but only while the + patterns differ in case alone. A line naming genuinely different hosts + is the user's, and stays theirs. + + Args: + patterns: The whitespace-separated patterns of a Host line + + Returns: + str | None: The first pattern if they are all one pattern, else None + """ + if not patterns: + return None + if len({pattern.lower() for pattern in patterns}) != 1: + return None + return patterns[0] + def _normalize_managed_host_line(self, line: str) -> str: - """Write a managed Host line with the prefix case currently in use. + """Write a managed Host line so it matches every spelling of the prefix. Blocks are recognised as ours case-insensitively, but ssh matches Host patterns case-SENSITIVELY. Writing a block back in the case it happened @@ -1097,25 +1168,42 @@ def _normalize_managed_host_line(self, line: str) -> str: which lets ssh offer every agent key and hit the server's MaxAuthTries before reaching the one that was just pushed. + Wildcard blocks are therefore written with one pattern per prefix + spelling, which covers the mixed-case files already out there instead + of merely refusing to add to them. Host entries name one host and keep + one name, in the configured case: they are what `list` reports and what + VSCode offers, and a second spelling of the prefix would not help + anyone typing a different case further along the name anyway. + Args: line: A managed block's Host line Returns: - str: The line with its prefix in the canonical case + str: The line with its patterns in canonical form """ match = re.match(r'^(\s*)(host)(\s+)(.*)$', line, re.IGNORECASE) if not match: return line indent, keyword, gap, value = match.groups() - normalized = re.sub( - rf'^{re.escape(self.ssh_host_prefix)}-', - f'{self.ssh_host_prefix}-', - value, - count=1, - flags=re.IGNORECASE, - ) - return f'{indent}{keyword}{gap}{normalized}' + + # Split off an inline comment, keeping the spacing in front of it. + hash_pos = value.find('#') + body, tail = (value, '') if hash_pos == -1 else (value[:hash_pos], value[hash_pos:]) + gap_before_comment = body[len(body.rstrip()):] + + collapsed = self._collapse_host_patterns(body.split()) + if collapsed is None: + return line + + prefix_len = len(self.ssh_host_prefix) + if collapsed[:prefix_len + 1].lower() != f"{self.ssh_host_prefix.lower()}-": + return line + + canonical = f"{self.ssh_host_prefix}{collapsed[prefix_len:]}" + patterns = self._host_pattern_variants(canonical) if '*' in canonical else [canonical] + + return f"{indent}{keyword}{gap}{' '.join(patterns)}{gap_before_comment}{tail}" def _clean_managed_lines(self, lines: list) -> list: """Strip comments and blank lines from a block cloudx-proxy manages. @@ -1284,11 +1372,16 @@ def _parse_ssh_config(self, config_content: str) -> dict: # patterns have to be collected before host entries, because they are # what makes a hyphenated environment name unambiguous. for block in blocks: - name = block['value'].split('#')[0].strip() - - # Match blocks, multi-pattern Host lines and anything not carrying - # our prefix belong to the user, not to us. - if block['keyword'] != 'host' or not name or len(name.split()) > 1: + # A wildcard block of ours lists one pattern per prefix spelling, + # so collapse those back to the single pattern they spell. Match + # blocks, Host lines naming genuinely different hosts, and anything + # not carrying our prefix belong to the user, not to us. + name = None + if block['keyword'] == 'host': + name = self._collapse_host_patterns( + block['value'].split('#')[0].split() + ) + if not name: unmanaged.append(block) continue @@ -1344,7 +1437,9 @@ def _parse_ssh_config(self, config_content: str) -> dict: result['environments'][env_name_key] = { 'pattern': f"{prefix}-{env_name_original}-*", 'name': env_name_original, # Store original case for display - 'lines': [f"Host {prefix}-{env_name_original}-*"] + 'lines': [ + f"Host {self._host_line_value(f'{prefix}-{env_name_original}-*')}" + ] } # Add host entry @@ -1515,7 +1610,7 @@ def _build_generic_config(self) -> str: str: Generic configuration block """ # No metadata comments - handled by _organize_ssh_config - config = f"""Host {self.ssh_host_prefix}-* + config = f"""Host {self._host_line_value(f"{self.ssh_host_prefix}-*")} User ec2-user TCPKeepAlive yes IdentitiesOnly yes @@ -1546,7 +1641,7 @@ def _build_environment_config(self, cloudx_env: str) -> str: str: Environment configuration block """ # No metadata comments - handled by _organize_ssh_config - config = f"""Host {self.ssh_host_prefix}-{cloudx_env}-* + config = f"""Host {self._host_line_value(f"{self.ssh_host_prefix}-{cloudx_env}-*")} """ # Add authentication configuration config += self._build_auth_config() @@ -1626,7 +1721,10 @@ def _add_host_entry(self, cloudx_env: str, instance_id: str, hostname: str, curr parsed['environments'][env_key] = { 'pattern': env_pattern, 'name': cloudx_env, - 'lines': [f"Host {env_pattern}", *self._build_environment_config(cloudx_env).split('\n')[1:]] + 'lines': [ + f"Host {self._host_line_value(env_pattern)}", + *self._build_environment_config(cloudx_env).split('\n')[1:], + ] } self.print_status(f"Created new environment section for '{cloudx_env}'", None, 2) else: diff --git a/tests/test_backward_compat.py b/tests/test_backward_compat.py index a4b10e4..9495214 100644 --- a/tests/test_backward_compat.py +++ b/tests/test_backward_compat.py @@ -191,24 +191,60 @@ def without_version_header(content): ] +def is_version_header(line): + return line.startswith("# SSH Configuration - Managed by") + + +def widens_prefix(setup, old_line, new_line): + """True when a wildcard Host line only gained the other prefix spelling. + + ssh matches Host patterns case-sensitively, so a block written as + `Host cloudx-*` does not apply to a host entry spelled `cloudX-dev-web1`, + and both spellings are out there. Wildcard blocks are therefore rewritten + to list every spelling. That is the one content change cleanup is allowed + to make to an existing config; a host entry never changes. + """ + if not old_line.lower().startswith("host "): + return False + + old_patterns = old_line.split()[1:] + if len(old_patterns) != 1 or "*" not in old_patterns[0]: + return False + + return new_line.split()[1:] == setup._host_pattern_variants(old_patterns[0]) + + class TestExistingConfigsAreUnchanged: - """cleanup rewrites the whole file; on an untouched config it must no-op.""" + """cleanup rewrites the whole file; on an untouched config it must not + touch anything but the version stamp and the Host patterns.""" @pytest.mark.parametrize("name", sorted(FIXTURES)) - def test_cleanup_changes_nothing_but_the_version_stamp(self, tmp_path, name): + def test_cleanup_changes_nothing_it_may_not(self, tmp_path, name): template, kwargs = FIXTURES[name] setup, before = materialise(tmp_path, template, kwargs) assert setup.cleanup_config() is True after = setup.ssh_config_file.read_text() - assert without_version_header(after) == without_version_header(before), ( - f"{name}: cleanup changed an existing v0.17.1 config" - ) + + # strict=True also pins the line count: nothing added, nothing dropped. + for old_line, new_line in zip( + before.splitlines(), after.splitlines(), strict=True + ): + if old_line == new_line: + continue + allowed = ( + (is_version_header(old_line) and is_version_header(new_line)) + or widens_prefix(setup, old_line, new_line) + ) + assert allowed, ( + f"{name}: cleanup changed an existing v0.17.1 config: " + f"{old_line!r} -> {new_line!r}" + ) @pytest.mark.parametrize("name", sorted(FIXTURES)) - def test_only_the_version_header_line_differs(self, tmp_path, name): - """Pin the exception: the header is restamped, nothing else may be.""" + def test_host_entries_and_directives_are_untouched(self, tmp_path, name): + """Pin the exceptions: only the header and wildcard Host lines move.""" template, kwargs = FIXTURES[name] setup, before = materialise(tmp_path, template, kwargs) setup.cleanup_config() @@ -219,10 +255,18 @@ def test_only_the_version_header_line_differs(self, tmp_path, name): if a != b ] - assert len(differing) <= 1, f"{name}: more than the header changed: {differing}" + headers = [pair for pair in differing if is_version_header(pair[0])] + assert len(headers) == 1, f"{name}: the version stamp must be rewritten once" + for old_line, new_line in differing: - assert old_line.startswith("# SSH Configuration - Managed by") - assert new_line.startswith("# SSH Configuration - Managed by") + if is_version_header(old_line): + continue + assert "*" in old_line, ( + f"{name}: a host entry changed: {old_line!r} -> {new_line!r}" + ) + assert widens_prefix(setup, old_line, new_line), ( + f"{name}: unexpected change: {old_line!r} -> {new_line!r}" + ) @pytest.mark.parametrize("name", sorted(FIXTURES)) def test_a_backup_of_the_original_is_kept(self, tmp_path, name): diff --git a/tests/test_ssh_config.py b/tests/test_ssh_config.py index 3f75adf..0cd8595 100644 --- a/tests/test_ssh_config.py +++ b/tests/test_ssh_config.py @@ -47,6 +47,15 @@ def host_names(content): ] +def host_patterns(content): + """The canonical (first) pattern of every Host/Match header, in order. + + Wildcard blocks carry one pattern per prefix spelling, so the header line + is not the pattern; this is what to count sections by. + """ + return [line.split()[1] for line in host_names(content)] + + MANAGED = """Host cloudx-* User ec2-user @@ -210,8 +219,8 @@ def test_adding_a_host_reuses_the_existing_case(self, setup): assert setup._add_host_entry(env, "i-00000000", "web2", config) is True result = setup.ssh_config_file.read_text() - assert host_names(result).count("Host cloudx-dev-*") == 1 - assert "Host cloudx-Dev-*" not in result + assert host_patterns(result).count("cloudx-dev-*") == 1 + assert "cloudx-Dev-*" not in result assert "Host cloudx-dev-web2" in result def test_add_host_entry_normalises_case_on_its_own(self, setup): @@ -221,7 +230,7 @@ def test_add_host_entry_normalises_case_on_its_own(self, setup): assert setup._add_host_entry("DEV", "i-00000000", "web2", config) is True result = setup.ssh_config_file.read_text() - assert host_names(result).count("Host cloudx-dev-*") == 1 + assert host_patterns(result).count("cloudx-dev-*") == 1 assert "Host cloudx-dev-web2" in result def test_resolve_keeps_new_environment_untouched(self, setup): @@ -517,7 +526,7 @@ def test_adding_a_host_normalises_the_stale_generic_block(self, tmp_path): setup.setup_ssh_config("DTA", "i-0123456789abcdef0", "web2") lines = self.managed_host_lines(setup) - assert "Host cloudX-*" in lines + assert "Host cloudX-* cloudx-*" in lines assert "Host cloudx-*" not in lines def test_cleanup_normalises_it_too(self, tmp_path): @@ -526,11 +535,17 @@ def test_cleanup_normalises_it_too(self, tmp_path): setup.cleanup_config() lines = self.managed_host_lines(setup) - assert "Host cloudX-*" in lines + assert "Host cloudX-* cloudx-*" in lines assert "Host cloudx-*" not in lines - def test_every_managed_host_line_uses_one_case(self, tmp_path): - """The invariant: a written file never disagrees with itself.""" + def test_every_managed_host_line_leads_with_one_case(self, tmp_path): + """The invariant: a written file never disagrees with itself. + + Wildcard blocks additionally answer to the other spelling, so that a + host entry left behind in the other case still picks them up - but the + canonical pattern always comes first and always uses the configured + case. + """ setup = self.uppercase_setup(tmp_path) setup.setup_ssh_config("DTA", "i-0123456789abcdef0", "web2") @@ -700,3 +715,139 @@ def test_repeated_cleanup_is_stable(self, tmp_path): assert setup.ssh_config_file.read_text() == once assert once.count("--profile cloudx") == 1 + + +class TestBothPrefixSpellingsMatch: + """A config may spell the prefix either way, in the same file. + + ssh matches Host patterns case-sensitively and its pattern syntax has only + '*' and '?' - `cloud[xX]-*` is a literal hostname, not a character class - + so one spelling in a wildcard block cannot cover the other. Wildcard blocks + therefore list every spelling; a Host line matches if any of its patterns + does. + """ + + MIXED = """# SSH Configuration - Managed by cloudx-proxy v0.16.15 + +Host cloudx-* + User ec2-user + IdentitiesOnly yes + +Host cloudX-dev-* + IdentityFile ~/.ssh/cloudX/cloudX + ProxyCommand uvx cloudX-proxy connect %h %p + +Host cloudx-dev-web1 + HostName i-0123456789abcdef0 +""" + + def make(self, tmp_path, content, prefix="cloudX"): + ssh_dir = tmp_path / "cloudX" + ssh_dir.mkdir(parents=True, exist_ok=True) + (ssh_dir / "config").write_text(content) + return CloudXSetup( + ssh_dir=str(ssh_dir), ssh_host_prefix=prefix, non_interactive=True + ) + + def test_wildcard_blocks_list_both_spellings(self, tmp_path): + setup = self.make(tmp_path, self.MIXED) + + setup.cleanup_config() + + lines = host_names(setup.ssh_config_file.read_text()) + assert "Host cloudX-* cloudx-*" in lines + assert "Host cloudX-dev-* cloudx-dev-*" in lines + + def test_host_entries_keep_a_single_name(self, tmp_path): + """They are what `list` reports and what VSCode offers.""" + setup = self.make(tmp_path, self.MIXED) + + setup.cleanup_config() + + entries = [ + line for line in host_names(setup.ssh_config_file.read_text()) + if "*" not in line + ] + assert entries == ["Host cloudX-dev-web1"] + + def test_a_lowercase_config_answers_to_the_uppercase_prefix_too(self, tmp_path): + setup = self.make(tmp_path, self.MIXED, prefix="cloudx") + + setup.cleanup_config() + + lines = host_names(setup.ssh_config_file.read_text()) + assert "Host cloudx-* cloudX-*" in lines + assert "Host cloudx-dev-* cloudX-dev-*" in lines + + def test_the_widened_form_round_trips(self, tmp_path): + """Our own output must be recognised as ours, or a second rewrite would + file the block under 'not managed' and generate a duplicate.""" + setup = self.make(tmp_path, self.MIXED) + + setup.cleanup_config() + once = setup.ssh_config_file.read_text() + setup.cleanup_config() + twice = setup.ssh_config_file.read_text() + + assert twice == once + assert "NOT MANAGED" not in twice + assert host_patterns(twice).count("cloudX-*") == 1 + + def test_a_users_multi_host_line_stays_theirs(self, tmp_path): + """Patterns that differ by more than case name different hosts.""" + setup = self.make(tmp_path, self.MIXED + """ +Host buildbox releasebox + User jenkins +""") + + setup.cleanup_config() + + result = setup.ssh_config_file.read_text() + assert "NOT MANAGED" in result + assert "Host buildbox releasebox" in result + assert " User jenkins" in result + + def test_a_hand_written_both_case_block_is_recognised(self, tmp_path): + """The form users write themselves to work around the case problem.""" + setup = self.make(tmp_path, """# SSH Configuration - Managed by cloudx-proxy v0.16.15 + +Host cloudX-dev-* cloudx-dev-* + IdentityFile ~/.ssh/cloudX/cloudX + ProxyCommand uvx cloudX-proxy connect %h %p + +Host cloudX-dev-web1 + HostName i-0123456789abcdef0 +""") + + parsed = setup._parse_ssh_config(setup.ssh_config_file.read_text()) + + assert parsed["other"] == [] + assert "dev" in parsed["environments"] + + def test_an_unrelated_prefix_gains_no_spellings(self, tmp_path): + setup = self.make(tmp_path, """# SSH Configuration - Managed by cloudx-proxy v0.16.15 + +Host vscode-* + User ec2-user + +Host vscode-dev-* + IdentityFile ~/.ssh/cloudX/cloudX + ProxyCommand uvx cloudX-proxy connect %h %p + +Host vscode-dev-web1 + HostName i-0123456789abcdef0 +""", prefix="vscode") + + setup.cleanup_config() + + for line in host_names(setup.ssh_config_file.read_text()): + assert len(line.split()) == 2, f"invented a spelling: {line!r}" + + def test_variants_only_vary_the_prefix(self, tmp_path): + setup = self.make(tmp_path, self.MIXED) + + assert setup._host_pattern_variants("cloudX-Pre-Prod-*") == [ + "cloudX-Pre-Prod-*", + "cloudx-Pre-Prod-*", + ] + assert setup._host_pattern_variants("unrelated-*") == ["unrelated-*"] From 1b19d329b73484dc0601b7393968184c22dab62e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 07:58:45 +0000 Subject: [PATCH 05/11] fix: never rename a host entry to the command name's case Only the X in the prefix is ambiguous. cloudX is the product's name - the X is ten, after Cloud9 - but people who would rather not reach for shift call their instance cloudx-dev-something, and that name is theirs: it is what they type, what list reports and what VSCode offers. The environment part is fixed by whoever rolled out the environment stack, and the host part is the user's. Widening the wildcard blocks to both spellings removed the reason to rewrite anything else, so stop: - _normalize_managed_host_line leaves entries alone and only widens patterns - cleanup's prefix conversion applies to wildcard patterns and the ProxyCommand, not to entry names, so switching command name no longer renames someone's box - re-running setup for an existing host updates its HostName under the name it already has; only a brand new entry uses the configured case Verified with ssh -G: an entry left as cloudx-DTA-lower keeps its name, takes its new instance id, and still resolves User, IdentitiesOnly and ProxyCommand from the cloudX-DTA-* cloudx-DTA-* block. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NiMd9TR2SJ4PAY4tmSBWzd --- .ai/context/ssh-config.md | 14 +++++-- README.md | 14 +++++-- cloudx_proxy/setup.py | 42 ++++++++++++++------ tests/test_ssh_config.py | 81 +++++++++++++++++++++++++++++++++------ 4 files changed, 121 insertions(+), 30 deletions(-) diff --git a/.ai/context/ssh-config.md b/.ai/context/ssh-config.md index 570bdc0..deec4c5 100644 --- a/.ai/context/ssh-config.md +++ b/.ai/context/ssh-config.md @@ -120,9 +120,17 @@ Wildcard blocks are consequently written with one pattern per prefix spelling - `Host cloudX-* cloudx-*` - since a `Host` line takes any number of patterns and matches if any one of them does. The configured spelling always comes first. -Host entries keep a single name in the configured case: they are what `list` -reports and what VSCode offers, and a second spelling of the prefix would not help -anyone typing a different case further along the name. +Only the prefix varies between the patterns. The environment part is set by +whoever rolled out the environment stack and the host part is chosen by the user; +both are written exactly as given. + +Host entries are never rewritten. `cloudX` is the product's name - the X is ten, +after Cloud9 - but people who would rather not reach for shift call their instance +`cloudx-dev-something`, and that name is theirs: it is what they type, what `list` +reports and what VSCode offers. The widened wildcard blocks match it either way, so +it does not need renaming to inherit its settings. Re-running `setup` for an +existing host updates its `HostName` and keeps its name; only a brand new entry is +written in the configured case. ## Configuration Inheritance diff --git a/README.md b/README.md index 81f0cad..34e0aea 100644 --- a/README.md +++ b/README.md @@ -435,7 +435,12 @@ Understanding the connection flow helps with troubleshooting and explains why ce > ssh matches `Host` patterns case-sensitively, and its pattern syntax has only > `*` and `?` - there is no `[xX]` character class. Wildcard blocks therefore list > both spellings, so a config that mixes them keeps working; the first pattern is -> the one the command name chose. Host entries name one host and keep one name. +> the one the command name chose. +> +> Only the prefix varies. The environment part is fixed by whoever rolled out the +> environment stack, and the host part is chosen by the user - both are written +> exactly as given. An instance called `cloudx-dev-web1` keeps that name and still +> picks up its settings from `Host cloudX-dev-* cloudx-dev-*`. > > Run `cleanup` with your preferred command to normalize existing configurations. @@ -576,9 +581,10 @@ uvx cloudX-proxy cleanup --ssh-config ~/.ssh/custom/config - Running `cloudX-proxy cleanup` converts `Host cloudx-*` → `Host cloudX-* cloudx-*` and `uvx cloudx-proxy` → `uvx cloudX-proxy` - Running `cloudx-proxy cleanup` converts `Host cloudX-*` → `Host cloudx-* cloudX-*` and `uvx cloudX-proxy` → `uvx cloudx-proxy` -Wildcard blocks keep both spellings so that host entries left behind in the other -case still pick up `User`, `IdentitiesOnly` and the `ProxyCommand`; host entries -themselves are renamed to the chosen case. +Wildcard blocks keep both spellings so that host entries in either case pick up +`User`, `IdentitiesOnly` and the `ProxyCommand`. Host entries are **not** renamed: +what an instance is called belongs to whoever created it. Re-running `setup` for a +host that already exists updates its instance id and leaves its name alone. This allows users to easily convert between naming conventions. The preferred convention is `cloudX` (uppercase X). diff --git a/cloudx_proxy/setup.py b/cloudx_proxy/setup.py index c44bb5a..4ef7491 100644 --- a/cloudx_proxy/setup.py +++ b/cloudx_proxy/setup.py @@ -1170,10 +1170,14 @@ def _normalize_managed_host_line(self, line: str) -> str: Wildcard blocks are therefore written with one pattern per prefix spelling, which covers the mixed-case files already out there instead - of merely refusing to add to them. Host entries name one host and keep - one name, in the configured case: they are what `list` reports and what - VSCode offers, and a second spelling of the prefix would not help - anyone typing a different case further along the name anyway. + of merely refusing to add to them. + + Host entries are left exactly as they are. `cloudX` is the product's + name - the X is ten, after Cloud9 - but people who dislike reaching for + shift call their instance `cloudx-dev-something`, and that name is + theirs: it is what they type, what `list` reports and what VSCode + offers. Since the wildcard blocks above now answer to both spellings, + the entry does not need renaming to pick up its settings. Args: line: A managed block's Host line @@ -1200,8 +1204,11 @@ def _normalize_managed_host_line(self, line: str) -> str: if collapsed[:prefix_len + 1].lower() != f"{self.ssh_host_prefix.lower()}-": return line + if '*' not in collapsed: + return line + canonical = f"{self.ssh_host_prefix}{collapsed[prefix_len:]}" - patterns = self._host_pattern_variants(canonical) if '*' in canonical else [canonical] + patterns = self._host_pattern_variants(canonical) return f"{indent}{keyword}{gap}{' '.join(patterns)}{gap_before_comment}{tail}" @@ -1587,9 +1594,12 @@ def _normalize_prefix(self, content: str) -> str: # Determine the "other" prefix to replace other_prefix = 'cloudx' if self.ssh_host_prefix == 'cloudX' else 'cloudX' - # Replace in Host patterns: Host cloudX-* or Host cloudx-* + # Replace in wildcard Host patterns only: Host cloudX-* or Host + # cloudx-*. A host entry carries no '*', and its name belongs to + # whoever created the instance - converting the command name must not + # rename it out from under them. content = re.sub( - rf'\bHost {other_prefix}-', + rf'\bHost {other_prefix}-(?=\S*\*)', f'Host {self.ssh_host_prefix}-', content ) @@ -1652,19 +1662,22 @@ def _build_environment_config(self, cloudx_env: str) -> str: return config - def _build_host_config(self, cloudx_env: str, hostname: str, instance_id: str) -> str: + def _build_host_config(self, cloudx_env: str, hostname: str, instance_id: str, + host_name: str | None = None) -> str: """Build a host-specific configuration block. Args: cloudx_env: CloudX environment hostname: Hostname for the instance instance_id: EC2 instance ID + host_name: Full name to write, when an entry already exists under a + name of its own. New entries use the configured prefix. Returns: str: Host configuration block """ # No metadata comments - handled by _organize_ssh_config - config = f"""Host {self.ssh_host_prefix}-{cloudx_env}-{hostname} + config = f"""Host {host_name or f"{self.ssh_host_prefix}-{cloudx_env}-{hostname}"} HostName {instance_id} """ @@ -1711,8 +1724,8 @@ def _add_host_entry(self, cloudx_env: str, instance_id: str, hostname: str, curr cloudx_env = existing_env.get('name', cloudx_env) host_pattern = f"{self.ssh_host_prefix}-{cloudx_env}-{hostname}" - new_host_entry = self._build_host_config(cloudx_env, hostname, instance_id) host_existed = False + existing_host_name = None # Ensure environment section exists if existing_env is None: @@ -1739,13 +1752,20 @@ def _add_host_entry(self, cloudx_env: str, instance_id: str, hostname: str, curr skipping = entry.lower() == host_pattern.lower() if skipping: host_existed = True + existing_host_name = entry continue elif skipping: continue new_lines.append(line) existing_env['lines'] = new_lines - # Add new host entry + # Add new host entry. An entry that is already there keeps the + # name it has: cloudX is the product's name, but calling the box + # cloudx-dev-web1 is its owner's call and it is what they type - + # only the instance id is being updated here. + new_host_entry = self._build_host_config( + cloudx_env, hostname, instance_id, host_name=existing_host_name + ) parsed['environments'][env_key]['lines'].extend(new_host_entry.split('\n')) # Rebuild config with organization diff --git a/tests/test_ssh_config.py b/tests/test_ssh_config.py index 0cd8595..6041344 100644 --- a/tests/test_ssh_config.py +++ b/tests/test_ssh_config.py @@ -587,22 +587,38 @@ def test_lowercase_invocation_normalises_the_other_way(self, tmp_path): class TestNormalizeManagedHostLine: - def test_wrong_case_prefix_is_corrected(self, setup): - assert setup._normalize_managed_host_line("Host cloudX-dev-web1") == "Host cloudx-dev-web1" + """The wildcard blocks answer to both spellings; entries are left alone.""" - def test_correct_case_is_untouched(self, setup): - assert setup._normalize_managed_host_line("Host cloudx-dev-web1") == "Host cloudx-dev-web1" + def test_a_wildcard_block_gains_the_other_spelling(self, setup): + assert setup._normalize_managed_host_line( + "Host cloudX-dev-*" + ) == "Host cloudx-dev-* cloudX-dev-*" + + def test_the_widened_form_is_idempotent(self, setup): + assert setup._normalize_managed_host_line( + "Host cloudx-dev-* cloudX-dev-*" + ) == "Host cloudx-dev-* cloudX-dev-*" + + def test_a_host_entry_keeps_the_case_its_owner_gave_it(self, setup): + """cloudX is the product name, but naming an instance cloudx-dev-web1 + is the user's call - and the widened blocks above match it either way.""" + assert setup._normalize_managed_host_line( + "Host cloudX-dev-web1" + ) == "Host cloudX-dev-web1" + assert setup._normalize_managed_host_line( + "Host cloudx-dev-web1" + ) == "Host cloudx-dev-web1" def test_inline_comment_survives(self, setup): assert setup._normalize_managed_host_line( - "Host cloudX-dev-web1 # erik's box" - ) == "Host cloudx-dev-web1 # erik's box" + "Host cloudX-dev-* # erik's env" + ) == "Host cloudx-dev-* cloudX-dev-* # erik's env" def test_only_the_prefix_is_touched(self, setup): - """A host whose own name contains the prefix spelling is not rewritten.""" + """A pattern whose own body contains the prefix spelling is not rewritten.""" assert setup._normalize_managed_host_line( - "Host cloudX-dev-cloudX-thing" - ) == "Host cloudx-dev-cloudX-thing" + "Host cloudX-dev-cloudX-*" + ) == "Host cloudx-dev-cloudX-* cloudX-dev-cloudX-*" def test_a_non_host_line_is_untouched(self, setup): assert setup._normalize_managed_host_line(" User ec2-user") == " User ec2-user" @@ -758,8 +774,9 @@ def test_wildcard_blocks_list_both_spellings(self, tmp_path): assert "Host cloudX-* cloudx-*" in lines assert "Host cloudX-dev-* cloudx-dev-*" in lines - def test_host_entries_keep_a_single_name(self, tmp_path): - """They are what `list` reports and what VSCode offers.""" + def test_host_entries_keep_the_name_their_owner_gave_them(self, tmp_path): + """They are what the user types, what `list` reports and what VSCode + offers - and `cloudx-dev-web1` is a legitimate thing to call a box.""" setup = self.make(tmp_path, self.MIXED) setup.cleanup_config() @@ -768,7 +785,47 @@ def test_host_entries_keep_a_single_name(self, tmp_path): line for line in host_names(setup.ssh_config_file.read_text()) if "*" not in line ] - assert entries == ["Host cloudX-dev-web1"] + assert entries == ["Host cloudx-dev-web1"] + + def test_a_lowercase_entry_still_resolves_its_settings(self, tmp_path): + """The point of widening: the entry keeps its name and still inherits.""" + setup = self.make(tmp_path, self.MIXED) + + setup.cleanup_config() + result = setup.ssh_config_file.read_text() + + assert "Host cloudx-dev-web1" in result + assert "Host cloudX-dev-* cloudx-dev-*" in result + + def test_converting_the_command_name_does_not_rename_instances(self, tmp_path): + """cleanup run as cloudX-proxy converts the patterns, not the boxes.""" + setup = self.make(tmp_path, self.MIXED, prefix="cloudX") + + setup.cleanup_config() + result = setup.ssh_config_file.read_text() + + assert "Host cloudx-dev-web1" in result + assert "Host cloudX-dev-web1" not in result + + def test_reregistering_a_host_keeps_its_name(self, tmp_path): + """Only the instance id is being updated - not what the box is called.""" + setup = self.make(tmp_path, self.MIXED, prefix="cloudX") + + setup.setup_ssh_config("dev", "i-9999999999999999", "web1") + result = setup.ssh_config_file.read_text() + + entries = [line for line in host_names(result) if "*" not in line] + assert entries == ["Host cloudx-dev-web1"] + assert "HostName i-9999999999999999" in result + assert "HostName i-0123456789abcdef0" not in result + + def test_a_brand_new_host_uses_the_configured_case(self, tmp_path): + setup = self.make(tmp_path, self.MIXED, prefix="cloudX") + + setup.setup_ssh_config("dev", "i-9999999999999999", "web9") + result = setup.ssh_config_file.read_text() + + assert "Host cloudX-dev-web9" in result def test_a_lowercase_config_answers_to_the_uppercase_prefix_too(self, tmp_path): setup = self.make(tmp_path, self.MIXED, prefix="cloudx") From 0c49ec5202c4dda1c6e43a42c5cc15e5d10efb09 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 08:02:56 +0000 Subject: [PATCH 06/11] fix: list shows patterns in the preferred spelling A pattern has no owner; a host does. Host entries are listed under the names their owners gave them, case and all, but a pattern was being rendered in whichever case the command name implied - so cloudx-proxy list showed cloudx-dev-* for a config written by cloudX-proxy, and the other way round. Show patterns as cloudX-*: the X is ten, after Cloud9, and it is the spelling to put in front of a user. An unrelated prefix is still shown as configured. An environment with no hosts of its own already appeared only among the patterns, since the listing below them is built from host entries; that is now covered by a test rather than left to chance. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NiMd9TR2SJ4PAY4tmSBWzd --- README.md | 5 +++ cloudx_proxy/cli.py | 14 ++++++-- cloudx_proxy/setup.py | 12 +++++++ tests/test_cli_options.py | 69 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 97 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 34e0aea..9e407b0 100644 --- a/README.md +++ b/README.md @@ -442,6 +442,11 @@ Understanding the connection flow helps with troubleshooting and explains why ce > exactly as given. An instance called `cloudx-dev-web1` keeps that name and still > picks up its settings from `Host cloudX-dev-* cloudx-dev-*`. > +> `list` reflects that split: hosts appear under the names their owners gave them, +> while patterns are shown as `cloudX-*` whichever command name you type, since +> `cloudX` is the preferred spelling. An environment with no hosts of its own is +> listed only as a pattern (under `--detailed`), not as an environment. +> > Run `cleanup` with your preferred command to normalize existing configurations. #### Setup Command diff --git a/cloudx_proxy/cli.py b/cloudx_proxy/cli.py index 64c04ee..d4ca3bb 100644 --- a/cloudx_proxy/cli.py +++ b/cloudx_proxy/cli.py @@ -367,17 +367,25 @@ def list(ssh_config: str, environment: str, detailed: bool, dry_run: bool): environments = {} generic_hosts = [] + # Patterns are shown in the preferred spelling whichever command name + # was typed. A configured host keeps whatever case its owner gave it + # and is listed under that name; a pattern has no owner, so there is + # no reason to show 'cloudx-dev-*' to someone who ran cloudX-proxy. + display_prefix = setup.preferred_host_prefix + # Add global pattern if present if parsed['global']: - generic_hosts.append((f"{ssh_host_prefix}-*", "N/A")) + generic_hosts.append((f"{display_prefix}-*", "N/A")) # Process each environment for env_key, env_data in parsed['environments'].items(): # Use original case name for display, fallback to key if not present display_name = env_data.get('name', env_key) - # Add environment pattern to generic hosts - generic_hosts.append((env_data['pattern'], "N/A")) + # Add environment pattern to generic hosts. Environments with no + # hosts of their own appear here and nowhere else, since the + # listing below is built from host entries. + generic_hosts.append((f"{display_prefix}-{display_name}-*", "N/A")) # Filter by environment if specified if environment and env_key.lower() != environment.lower(): diff --git a/cloudx_proxy/setup.py b/cloudx_proxy/setup.py index 4ef7491..def7085 100644 --- a/cloudx_proxy/setup.py +++ b/cloudx_proxy/setup.py @@ -1100,6 +1100,18 @@ def _prefix_spellings(self) -> list[str]: spellings.append(candidate) return spellings + @property + def preferred_host_prefix(self) -> str: + """The spelling to show a pattern in. + + cloudX is the product's own name - the X is ten, after Cloud9 - so it + is the spelling to put in front of a user, whichever of the two command + names they happened to type. Any other prefix is shown as configured. + """ + if self.ssh_host_prefix.lower() == 'cloudx': + return 'cloudX' + return self.ssh_host_prefix + def _host_pattern_variants(self, pattern: str) -> list[str]: """Every spelling of a wildcard Host pattern to write, canonical first. diff --git a/tests/test_cli_options.py b/tests/test_cli_options.py index 984b963..5ff8d80 100644 --- a/tests/test_cli_options.py +++ b/tests/test_cli_options.py @@ -9,6 +9,7 @@ upgrade that changes it fails here rather than in the field. """ +import pytest from click.testing import CliRunner from cloudx_proxy.cli import cli @@ -63,3 +64,71 @@ def test_help_shows_the_optional_value(self, tmp_path): assert result.exit_code == 0 assert "--1password [VAULT]" in result.output + + +class TestListShowsThePreferredSpelling: + """A pattern has no owner; a host does. + + cloudX is the product's name - the X is ten, after Cloud9 - so patterns are + shown that way whichever command name was typed. A configured host keeps + the case its owner gave it and is listed under that name. + """ + + CONFIG = """# SSH Configuration - Managed by cloudX-proxy v0.17.3 + +Host cloudX-* cloudx-* + User ec2-user + IdentitiesOnly yes + +Host cloudX-DTA-* cloudx-DTA-* + IdentityFile ~/.ssh/cloudX/cloudX + ProxyCommand uvx cloudX-proxy connect %h %p + +Host cloudX-DTA-unified + HostName i-095f07267c26a685c + +Host cloudx-DTA-lower + HostName i-0aaaaaaaaaaaaaaaa + +Host cloudx-empty-* cloudX-empty-* + IdentityFile ~/.ssh/cloudX/cloudX + ProxyCommand uvx cloudx-proxy connect %h %p +""" + + def run(self, tmp_path, monkeypatch, argv0, extra_args=()): + ssh_dir = tmp_path / "cloudX" + ssh_dir.mkdir(parents=True, exist_ok=True) + config = ssh_dir / "config" + config.write_text(self.CONFIG) + monkeypatch.setattr("cloudx_proxy.cli.sys.argv", [argv0]) + + return CliRunner().invoke( + cli, ["list", "--ssh-config", str(config), *extra_args] + ) + + @pytest.mark.parametrize("argv0", ["cloudX-proxy", "cloudx-proxy"]) + def test_patterns_use_the_x_spelling_either_way(self, tmp_path, monkeypatch, argv0): + result = self.run(tmp_path, monkeypatch, argv0, ["--detailed"]) + + assert result.exit_code == 0, result.output + assert "cloudX-*" in result.output + assert "cloudX-DTA-*" in result.output + assert "cloudx-DTA-*" not in result.output + + @pytest.mark.parametrize("argv0", ["cloudX-proxy", "cloudx-proxy"]) + def test_hosts_keep_their_own_case(self, tmp_path, monkeypatch, argv0): + result = self.run(tmp_path, monkeypatch, argv0) + + assert "cloudx-DTA-lower" in result.output + assert "cloudX-DTA-unified" in result.output + # ...and are still shortened against either spelling + assert "lower (" in result.output + assert "unified (" in result.output + + def test_an_environment_without_hosts_is_not_listed(self, tmp_path, monkeypatch): + result = self.run(tmp_path, monkeypatch, "cloudX-proxy", ["--detailed"]) + + assert "Environment: DTA" in result.output + assert "Environment: empty" not in result.output + # It is still visible as a pattern, in the preferred spelling. + assert "cloudX-empty-*" in result.output From 3e24b87e55fa67dc7587909180af210f12f25bc3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 08:11:41 +0000 Subject: [PATCH 07/11] fix: stop stacking blank lines, and name the host that was written print_header prepended two newlines while the banner above it appended one, so the first section sat under three blank lines and every section after it under two. One blank line separates sections now. Banners say cloudX-proxy whichever command name was typed, matching what list does with patterns: cloudX is the product's name. The setup summary's "Connect using: ssh ..." was built from the configured prefix, so after the previous commit it could name a host that does not exist - an entry already in the config keeps its own name. It reports the entry that was actually written. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NiMd9TR2SJ4PAY4tmSBWzd --- cloudx_proxy/cli.py | 8 ++++---- cloudx_proxy/setup.py | 19 +++++++++++++++++-- tests/test_cli_options.py | 35 +++++++++++++++++++++++++++++++++++ tests/test_ssh_config.py | 9 +++++++++ 4 files changed, 65 insertions(+), 6 deletions(-) diff --git a/cloudx_proxy/cli.py b/cloudx_proxy/cli.py index d4ca3bb..ab6ea0b 100644 --- a/cloudx_proxy/cli.py +++ b/cloudx_proxy/cli.py @@ -205,9 +205,9 @@ def setup(profile: str, ssh_key: str, ssh_config: str, ssh_dir: str, aws_env: st ) if dry_run: - print(f"\n{header(f'=== {ssh_host_prefix}-proxy Setup (DRY RUN) ===')}\n") + print(f"\n{header('=== cloudX-proxy Setup (DRY RUN) ===')}") else: - print(f"\n{header(f'=== {ssh_host_prefix}-proxy Setup ===')}\n") + print(f"\n{header('=== cloudX-proxy Setup ===')}") # Report missing tooling up front rather than from inside a # ProxyCommand, where the error is easy to miss @@ -340,7 +340,7 @@ def list(ssh_config: str, environment: str, detailed: bool, dry_run: bool): config_file = cloudx_config if dry_run: - print(f"\n{header('=== cloudx-proxy List (DRY RUN) ===')}\n") + print(f"\n{header('=== cloudX-proxy List (DRY RUN) ===')}\n") print(f"[DRY RUN] Would read SSH config from: {config_file}") if environment: print(f"[DRY RUN] Would filter hosts by environment: {environment}") @@ -434,7 +434,7 @@ def list(ssh_config: str, environment: str, detailed: bool, dry_run: bool): return # Print header - print(f"\n{header('=== cloudx-proxy Configured Hosts ===')}\n") + print(f"\n{header('=== cloudX-proxy Configured Hosts ===')}\n") # Print generic patterns if any and detailed mode if generic_hosts and detailed: diff --git a/cloudx_proxy/setup.py b/cloudx_proxy/setup.py index def7085..ebab22c 100644 --- a/cloudx_proxy/setup.py +++ b/cloudx_proxy/setup.py @@ -272,6 +272,9 @@ def __init__(self, profile: str = "cloudX", ssh_key: str = "cloudX", ssh_config: self.ssh_key_file = self.ssh_dir / f"{ssh_key}" self.default_env = None + # Name of the host entry last written, which is not necessarily the + # configured case: an entry already in the config keeps its own name. + self.last_host_entry_name = None # On Windows the 1Password SSH agent serves the standard OpenSSH named # pipe, which ssh talks to by default. There is no socket file to find @@ -422,10 +425,14 @@ def _ensure_op_agent_symlink(self) -> bool: def print_header(self, text: str) -> None: """Print a section header. + One blank line separates it from what came before. The caller printing + the banner above the first section does not add one of its own, or the + two stack into a gap. + Args: text: The header text """ - print(f"\n\n{header(f'=== {text} ===')}") + print(f"\n{header(f'=== {text} ===')}") def print_status(self, message: str, status: bool | None = None, indent: int = 0) -> None: """Print a status message with optional checkmark/cross. @@ -1738,6 +1745,7 @@ def _add_host_entry(self, cloudx_env: str, instance_id: str, hostname: str, curr host_pattern = f"{self.ssh_host_prefix}-{cloudx_env}-{hostname}" host_existed = False existing_host_name = None + self.last_host_entry_name = host_pattern # Ensure environment section exists if existing_env is None: @@ -1775,6 +1783,7 @@ def _add_host_entry(self, cloudx_env: str, instance_id: str, hostname: str, curr # name it has: cloudX is the product's name, but calling the box # cloudx-dev-web1 is its owner's call and it is what they type - # only the instance id is being updated here. + self.last_host_entry_name = existing_host_name or host_pattern new_host_entry = self._build_host_config( cloudx_env, hostname, instance_id, host_name=existing_host_name ) @@ -2197,7 +2206,13 @@ def setup_ssh_config(self, cloudx_env: str, instance_id: str, hostname: str) -> self.print_status(f"System config: {format_path(str(system_config_path))}", None, 2) self.print_status(f"cloudX-proxy config: {format_path(str(self.ssh_config_file))}", None, 2) self.print_status(f"SSH key directory: {format_path(str(self.ssh_dir))}", None, 2) - self.print_status(f"Connect using: {format_command(f'ssh {self.ssh_host_prefix}-{cloudx_env}-{hostname}')}", None, 2) + # Name the entry that was actually written: an existing host keeps + # the name it has, which is not necessarily the configured case. + connect_name = ( + self.last_host_entry_name + or f"{self.ssh_host_prefix}-{cloudx_env}-{hostname}" + ) + self.print_status(f"Connect using: {format_command(f'ssh {connect_name}')}", None, 2) return True diff --git a/tests/test_cli_options.py b/tests/test_cli_options.py index 5ff8d80..bd0a02b 100644 --- a/tests/test_cli_options.py +++ b/tests/test_cli_options.py @@ -132,3 +132,38 @@ def test_an_environment_without_hosts_is_not_listed(self, tmp_path, monkeypatch) assert "Environment: empty" not in result.output # It is still visible as a pattern, in the preferred spelling. assert "cloudX-empty-*" in result.output + + +class TestOutputSpacing: + """`print_header` prepended two newlines while the banner above it appended + one, so the first section sat under three blank lines and every section + after it under two.""" + + def test_sections_are_separated_by_one_blank_line(self, tmp_path, monkeypatch): + monkeypatch.setattr("cloudx_proxy.setup.boto3.Session", lambda *a, **k: None) + monkeypatch.setattr("cloudx_proxy.cli.sys.argv", ["cloudX-proxy"]) + + result = CliRunner().invoke(cli, [ + "setup", "--dry-run", "--yes", + "--instance", "i-0123456789abcdef0", + "--hostname", "web1", "--environment", "dev", + "--ssh-config", str(tmp_path / "cloudX" / "config"), + ]) + + assert result.exit_code == 0, result.output + assert "\n\n\n" not in result.output, "blank lines are stacking up" + assert "=== Prerequisites ===" in result.output + + @pytest.mark.parametrize("argv0", ["cloudX-proxy", "cloudx-proxy"]) + def test_banners_use_the_product_spelling(self, tmp_path, monkeypatch, argv0): + monkeypatch.setattr("cloudx_proxy.setup.boto3.Session", lambda *a, **k: None) + monkeypatch.setattr("cloudx_proxy.cli.sys.argv", [argv0]) + + result = CliRunner().invoke(cli, [ + "setup", "--dry-run", "--yes", + "--instance", "i-0123456789abcdef0", + "--hostname", "web1", "--environment", "dev", + "--ssh-config", str(tmp_path / "cloudX" / "config"), + ]) + + assert "=== cloudX-proxy Setup (DRY RUN) ===" in result.output diff --git a/tests/test_ssh_config.py b/tests/test_ssh_config.py index 6041344..2d1f253 100644 --- a/tests/test_ssh_config.py +++ b/tests/test_ssh_config.py @@ -819,6 +819,15 @@ def test_reregistering_a_host_keeps_its_name(self, tmp_path): assert "HostName i-9999999999999999" in result assert "HostName i-0123456789abcdef0" not in result + def test_the_connect_hint_names_the_entry_that_was_written(self, tmp_path, capsys): + """Telling someone to `ssh cloudX-dev-web1` when the entry says + cloudx-dev-web1 sends them to a host that does not resolve.""" + setup = self.make(tmp_path, self.MIXED, prefix="cloudX") + + setup.setup_ssh_config("dev", "i-9999999999999999", "web1") + + assert "ssh cloudx-dev-web1" in capsys.readouterr().out + def test_a_brand_new_host_uses_the_configured_case(self, tmp_path): setup = self.make(tmp_path, self.MIXED, prefix="cloudX") From a103b361f97f1643585824521cb86b7e47e3d0ed Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 08:19:35 +0000 Subject: [PATCH 08/11] style: put console output on one grid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Indents had drifted: an indent of 3 in the migration preview, details at 2 with no step above them (Prerequisites, cleanup, migration, the legacy-config check), and the instance check reporting at 4 with nothing at 2 to belong to. Symbols had drifted the same way - a condition the code went on to recover from was marked ✗, so a snap 1Password install saw a cross immediately followed by a tick, and one genuine failure produced four marked lines. The grid is now 0 for a step, 2 for a detail of it, 4 for a detail of a nested operation, and nothing else; ○ for neutral, ✓ for true now, ✗ for the outcome when it is wrong. The rule is written next to print_status, where it is enforced, and in .ai/context/development.md. Also brings the raw prints that sat off the grid onto it (the credentials prompt, the 1Password vault menu), and fixes the Windows call-out box naming a host from the configured prefix rather than the entry that was written. Tests walk a full dry-run and a cleanup, asserting every status line sits on the grid, that no detail is orphaned, and that one failure shows one ✗. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NiMd9TR2SJ4PAY4tmSBWzd --- .ai/context/development.md | 26 +++++++++ cloudx_proxy/setup.py | 88 ++++++++++++++++++++--------- tests/test_cli_options.py | 110 ++++++++++++++++++++++++++++++++++++- 3 files changed, 196 insertions(+), 28 deletions(-) diff --git a/.ai/context/development.md b/.ai/context/development.md index be44c0e..8b1dc8f 100644 --- a/.ai/context/development.md +++ b/.ai/context/development.md @@ -104,6 +104,32 @@ The project uses semantic-release with GitHub Actions: - `feat:` → minor version - `fix:`, `docs:`, `style:`, etc. → patch version +## Console Output + +Three indents and three symbols, used the same way everywhere, so a run can be +skimmed down the left edge. The rule lives next to `CloudXSetup.print_status` +in `cloudx_proxy/setup.py`; `tests/test_cli_options.py` enforces it. + +``` +=== Section === a phase of the command (print_header) +○ Doing something... STEP: one operation within the section + ✓ It worked DETAIL: what that step found or did + ○ ... SUB: detail of a nested operation +``` + +- `○` neutral - about to happen, in progress, informational, or a dry-run + preview +- `✓` true now - succeeded, exists, verified +- `✗` wrong - failed, missing, invalid + +Indents are 0, 2 and 4; nothing else. A detail must have a step above it, and a +sub-detail a detail. A ✗ marks the *outcome*: a condition the code goes on to +recover from is `○`, so one failure shows as one ✗ rather than one per line on +the way there. + +`print_header` emits a single blank line before the header, and callers printing +a banner above the first section add none of their own - the two used to stack. + ## Publishing to PyPI The package is automatically published to PyPI via GitHub Actions when a new release is created. Setup: diff --git a/cloudx_proxy/setup.py b/cloudx_proxy/setup.py index ebab22c..214b87e 100644 --- a/cloudx_proxy/setup.py +++ b/cloudx_proxy/setup.py @@ -313,7 +313,7 @@ def _check_op_agent(self) -> bool: None, 2, ) - self.print_status("1Password integration is not supported in this configuration", False, 2) + self.print_status("1Password integration is not supported in this configuration", None, 2) return False self.print_status("1Password SSH agent pipe is available", True, 2) return True @@ -328,13 +328,13 @@ def _check_op_agent(self) -> bool: if current_target != self.op_agent_sock_macos and self.op_agent_sock_macos.exists(): self.print_status("Updating 1Password agent symlink to default location", None, 2) if not self._ensure_op_agent_symlink(): - self.print_status("1Password integration is not supported in this configuration", False, 2) + self.print_status("1Password integration is not supported in this configuration", None, 2) return False self.print_status("1Password SSH agent socket is available", True, 2) return True - self.print_status("1Password SSH agent socket not found at ~/.1password/agent.sock", False, 2) + self.print_status("1Password SSH agent socket not found at ~/.1password/agent.sock", None, 2) if system == 'Darwin': if self._ensure_op_agent_symlink(): @@ -349,7 +349,7 @@ def _check_op_agent(self) -> bool: self.print_status("1Password SSH agent is not available", False, 2) self.print_status("Please ensure 1Password SSH agent is enabled in 1Password settings", None, 2) - self.print_status("1Password integration is not supported in this configuration", False, 2) + self.print_status("1Password integration is not supported in this configuration", None, 2) return False def _op_identity_agent(self) -> str | None: @@ -434,13 +434,33 @@ def print_header(self, text: str) -> None: """ print(f"\n{header(f'=== {text} ===')}") + # Output grid. Three indents and three symbols, used the same way + # everywhere, so a run can be skimmed down the left edge: + # + # === Section === a phase of the command (print_header) + # ○ Doing something... STEP: one operation within the section + # ✓ It worked DETAIL: what that step found or did + # ○ ... SUB: detail of a nested operation + # + # ○ neutral - about to happen, in progress, informational, or a + # dry-run preview + # ✓ true now - succeeded, exists, verified + # ✗ wrong - failed, missing, invalid. The problem takes the ✗; its + # consequences and the advice that follows are ○, so one failure + # shows as one ✗. + INDENT_STEP = 0 + INDENT_DETAIL = 2 + INDENT_SUB = 4 + def print_status(self, message: str, status: bool | None = None, indent: int = 0) -> None: """Print a status message with optional checkmark/cross. Args: message: The message to print - status: True for success (✓), False for failure (✗), None for no symbol - indent: Number of spaces to indent + status: True for success (✓), False for failure (✗), None for + neutral (○) - see the output grid above + indent: Number of spaces to indent: 0 for a step, 2 for a detail + of it, 4 for a detail of a nested operation """ prefix = " " * indent print(f"{prefix}{status_symbol(status)} {message}") @@ -535,10 +555,12 @@ def check_prerequisites(self) -> bool: self.print_header("Prerequisites") if self.dry_run: + self.print_status("[DRY RUN] Would check for required tools") for tool in self.REQUIRED_TOOLS: self.print_status(f"[DRY RUN] Would check for {tool}", None, 2) return True + self.print_status("Checking required tools...") all_found = True for tool, install_url in self.REQUIRED_TOOLS.items(): name = f"{tool}.exe" if platform.system() == 'Windows' and tool == 'aws' else tool @@ -587,9 +609,9 @@ def setup_aws_profile(self) -> bool: session = boto3.Session(profile_name=self.profile) except Exception: # Profile doesn't exist, create it - self.print_status(f"AWS profile '{self.profile}' not found", False, 2) + self.print_status(f"AWS profile '{self.profile}' not found", None, 2) self.print_status("Setting up AWS profile...", None, 2) - print(info("Please enter your AWS credentials:")) + self.print_status(info("Please enter your AWS credentials:"), None, 2) # Use aws configure command subprocess.run([ @@ -762,9 +784,9 @@ def _create_op_key(self) -> bool: self.print_status(f"Specified vault '{self.op_vault}' not found", False, 2) # Display available vaults - print(f"\n{info('Available 1Password vaults:')}") + self.print_status(info('Available 1Password vaults:'), None, 2) for i, vault in enumerate(vaults): - print(f" {i+1}. {vault['name']}") + print(f" {i + 1}. {vault['name']}") # Let user select vault vault_num = self.prompt("Select vault number to store SSH key", "1") @@ -780,9 +802,9 @@ def _create_op_key(self) -> bool: else: # No vault specified, prompt the user self.print_status("Creating a new SSH key in 1Password", None, 2) - print(f"\n{info('Available 1Password vaults:')}") + self.print_status(info('Available 1Password vaults:'), None, 2) for i, vault in enumerate(vaults): - print(f" {i+1}. {vault['name']}") + print(f" {i + 1}. {vault['name']}") # Let user select vault vault_num = self.prompt("Select vault number to store SSH key", "1") @@ -1868,7 +1890,10 @@ def cleanup_config(self) -> bool: bool: True if cleanup was successful """ try: + self.print_header("Cleanup") + if not self.ssh_config_file.exists(): + self.print_status("Reorganizing SSH configuration...") self.print_status(f"SSH config file not found: {self.ssh_config_file}", False, 2) return False @@ -1881,6 +1906,7 @@ def cleanup_config(self) -> bool: # For dry-run, show what would be cleaned up if self.dry_run: + self.print_status("[DRY RUN] Would reorganize the SSH configuration") self.print_status("Parsing SSH config...", None, 2) parsed = self._parse_ssh_config(current_config) @@ -1899,6 +1925,7 @@ def cleanup_config(self) -> bool: return True # Parse existing config + self.print_status("Reorganizing SSH configuration...") self.print_status("Parsing SSH config...", None, 2) parsed = self._parse_ssh_config(current_config) @@ -1918,7 +1945,7 @@ def cleanup_config(self) -> bool: parsed['environments'][env_name]['lines'] = new_lines # Reorganize with proper structure - self.print_status("Reorganizing configuration...", None, 2) + self.print_status("Rebuilding the file...", None, 2) organized_config = self._organize_ssh_config( parsed['global'] or self._build_generic_config(), parsed['environments'], @@ -2232,7 +2259,7 @@ def check_instance_setup(self, instance_id: str, hostname: str, cloudx_env: str) bool: True if instance is accessible """ ssh_host = f"{self.ssh_host_prefix}-{cloudx_env}-{hostname}" - self.print_status(f"Checking SSH connection to {ssh_host}...", None, 4) + self.print_status(f"Checking SSH connection to {ssh_host}...", None, 2) try: # Try to connect with a simple command that will exit immediately. @@ -2254,24 +2281,24 @@ def check_instance_setup(self, instance_id: str, hostname: str, cloudx_env: str) ) if result.returncode == 0: - self.print_status("SSH connection successful", True, 4) + self.print_status("SSH connection successful", True, 2) return True else: - self.print_status("SSH connection failed", False, 4) + self.print_status("SSH connection failed", False, 2) if "Connection refused" in result.stderr: - self.print_status("Instance appears to be starting up. Please try again in a few minutes.", None, 4) + self.print_status("Instance appears to be starting up. Please try again in a few minutes.", None, 2) elif "Connection timed out" in result.stderr: - self.print_status("Instance may be stopped. Please start it through the appropriate channels.", None, 4) + self.print_status("Instance may be stopped. Please start it through the appropriate channels.", None, 2) else: - self.print_status(f"Error: {result.stderr.strip()}", None, 4) + self.print_status(f"Error: {result.stderr.strip()}", None, 2) return False except subprocess.TimeoutExpired: - self.print_status("SSH connection timed out", False, 4) - self.print_status("Instance may be stopped or still starting up", None, 4) + self.print_status("SSH connection timed out", False, 2) + self.print_status("Instance may be stopped or still starting up", None, 2) return False except Exception as e: - self.print_status(f"Error checking SSH connection: {e!s}", False, 4) + self.print_status(f"Error checking SSH connection: {e!s}", False, 2) return False def wait_for_setup_completion(self, instance_id: str, hostname: str, cloudx_env: str) -> bool: @@ -2293,13 +2320,19 @@ def wait_for_setup_completion(self, instance_id: str, hostname: str, cloudx_env: self.print_status("[DRY RUN] Would wait up to 5 minutes for SSH access if needed", None, 2) return True + self.print_status("Checking instance accessibility...") + # On Windows, skip the automated connection test as it may hang # Instead, provide clear instructions for manual testing if platform.system() == 'Windows': self.print_status("Skipping automated connection test on Windows", None, 2) print(f"\n{info('='*60)}") print(info("Setup completed! To test your SSH connection, run:")) - print(f"\n {format_command(f'ssh {self.ssh_host_prefix}-{cloudx_env}-{hostname}')}") + connect_name = ( + self.last_host_entry_name + or f"{self.ssh_host_prefix}-{cloudx_env}-{hostname}" + ) + print(f"\n {format_command(f'ssh {connect_name}')}") print(f"\n{info('='*60)}\n") self.print_status("Configuration files have been created successfully", True, 2) return True @@ -2352,13 +2385,15 @@ def migrate_to_cloudx(self, target_dir: Path | None = None) -> bool: # Show what would be replaced old_dir_name = vscode_dir.name new_dir_name = target_dir.name - self.print_status(f" - Replace /{old_dir_name}/ with /{new_dir_name}/", None, 3) - self.print_status(f" - Replace ~/.ssh/{old_dir_name} with ~/.ssh/{new_dir_name}", None, 3) - self.print_status(f" - Replace --ssh-key {old_dir_name} with --ssh-key {new_dir_name}", None, 3) + self.print_status(f"Replace /{old_dir_name}/ with /{new_dir_name}/", None, 4) + self.print_status(f"Replace ~/.ssh/{old_dir_name} with ~/.ssh/{new_dir_name}", None, 4) + self.print_status(f"Replace --ssh-key {old_dir_name} with --ssh-key {new_dir_name}", None, 4) self.print_status("[DRY RUN] Would update ~/.ssh/config to include new config path", None, 2) return True + self.print_status(f"Migrating to {target_dir}...") + if not vscode_dir.exists(): self.print_status(f"Source directory {vscode_dir} does not exist", False, 2) return False @@ -2456,6 +2491,7 @@ def check_and_perform_migration(self) -> bool: return False self.print_header("Migration Available") + self.print_status("Checking for a legacy ~/.ssh/vscode configuration...") self.print_status("Found existing configuration in ~/.ssh/vscode", None, 2) self.print_status("The default directory is now ~/.ssh/cloudX", None, 2) diff --git a/tests/test_cli_options.py b/tests/test_cli_options.py index bd0a02b..e1a087e 100644 --- a/tests/test_cli_options.py +++ b/tests/test_cli_options.py @@ -13,6 +13,7 @@ from click.testing import CliRunner from cloudx_proxy.cli import cli +from cloudx_proxy.setup import CloudXSetup def run_setup(tmp_path, monkeypatch, extra_args): @@ -134,10 +135,79 @@ def test_an_environment_without_hosts_is_not_listed(self, tmp_path, monkeypatch) assert "cloudX-empty-*" in result.output -class TestOutputSpacing: +SYMBOLS = ("\u25cb", "\u2713", "\u2717") # neutral, success, failure + + +def status_lines(output): + """Every status line as (indent, symbol, text).""" + parsed = [] + for line in output.splitlines(): + stripped = line.lstrip(" ") + if stripped[:1] in SYMBOLS: + parsed.append((len(line) - len(stripped), stripped[0], stripped[1:].strip())) + return parsed + + +class TestOutputGrid: """`print_header` prepended two newlines while the banner above it appended one, so the first section sat under three blank lines and every section - after it under two.""" + after it under two. Indents had drifted to 3 and to orphaned 2s and 4s. + """ + + def full_run(self, tmp_path, monkeypatch): + monkeypatch.setattr("cloudx_proxy.setup.boto3.Session", lambda *a, **k: None) + monkeypatch.setattr("cloudx_proxy.cli.sys.argv", ["cloudX-proxy"]) + + return CliRunner().invoke(cli, [ + "setup", "--dry-run", "--yes", + "--instance", "i-0123456789abcdef0", + "--hostname", "web1", "--environment", "dev", + "--ssh-config", str(tmp_path / "cloudX" / "config"), + ]) + + def test_every_status_line_sits_on_the_grid(self, tmp_path, monkeypatch): + result = self.full_run(tmp_path, monkeypatch) + + lines = status_lines(result.output) + assert lines, result.output + for indent, _symbol, text in lines: + assert indent in (0, 2, 4), f"off-grid indent {indent}: {text!r}" + + def test_no_detail_is_orphaned(self, tmp_path, monkeypatch): + """A detail belongs to a step, and a sub-detail to a detail.""" + result = self.full_run(tmp_path, monkeypatch) + + seen = set() + for line in result.output.splitlines(): + if line.startswith("==="): + seen.clear() # a header starts a new section + continue + stripped = line.lstrip(" ") + if stripped[:1] not in SYMBOLS: + continue + indent = len(line) - len(stripped) + if indent: + assert indent - 2 in seen, f"orphaned at indent {indent}: {stripped!r}" + seen.add(indent) + + def test_cleanup_sits_on_the_grid_too(self, tmp_path, monkeypatch): + monkeypatch.setattr("cloudx_proxy.cli.sys.argv", ["cloudX-proxy"]) + ssh_dir = tmp_path / "cloudX" + ssh_dir.mkdir(parents=True) + (ssh_dir / "config").write_text( + "Host cloudX-dev-*\n IdentityFile ~/.ssh/cloudX/cloudX\n\n" + "Host cloudX-dev-web1\n HostName i-0123456789abcdef0\n" + ) + + result = CliRunner().invoke( + cli, ["cleanup", "--ssh-config", str(ssh_dir / "config")] + ) + + assert result.exit_code == 0, result.output + indents = [indent for indent, _s, _t in status_lines(result.output)] + assert indents, result.output + assert set(indents) <= {0, 2, 4} + assert 0 in indents, "cleanup's details had no step above them" def test_sections_are_separated_by_one_blank_line(self, tmp_path, monkeypatch): monkeypatch.setattr("cloudx_proxy.setup.boto3.Session", lambda *a, **k: None) @@ -167,3 +237,39 @@ def test_banners_use_the_product_spelling(self, tmp_path, monkeypatch, argv0): ]) assert "=== cloudX-proxy Setup (DRY RUN) ===" in result.output + + +class TestOneFailureOneCross: + """A ✗ marks the outcome, not every observation on the way to it. + + The 1Password check reported "socket not found at ~/.1password/agent.sock" + as a failure before it had looked anywhere else, so a snap install saw a ✗ + immediately followed by a ✓, and a genuine failure produced four marked + lines for one problem. + """ + + def linux_setup(self, tmp_path, monkeypatch): + monkeypatch.setattr("cloudx_proxy.setup.platform.system", lambda: "Linux") + setup = CloudXSetup( + ssh_dir=str(tmp_path / "cloudX"), non_interactive=True, op_vault="Private" + ) + setup.op_agent_sock = tmp_path / "absent.sock" + setup.op_agent_sock_snap = tmp_path / "snap.sock" + return setup + + def test_a_recoverable_miss_is_neutral(self, tmp_path, monkeypatch, capsys): + setup = self.linux_setup(tmp_path, monkeypatch) + setup.op_agent_sock_snap.write_text("") # the snap agent is there + + assert setup._check_op_agent() is True + + marks = [symbol for _i, symbol, _t in status_lines(capsys.readouterr().out)] + assert "✗" not in marks, "a miss we recovered from was reported as a failure" + + def test_a_real_failure_is_marked_once(self, tmp_path, monkeypatch, capsys): + setup = self.linux_setup(tmp_path, monkeypatch) + + assert setup._check_op_agent() is False + + marks = [symbol for _i, symbol, _t in status_lines(capsys.readouterr().out)] + assert marks.count("✗") == 1, f"one failure, one cross: {marks}" From 27b98dc21d8b6a45ac3992c76a466264648cf5d4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 08:24:13 +0000 Subject: [PATCH 09/11] style: make counts agree with their noun "Would reorganize 1 environments" reads like a bug in the tool. A plural() helper renders a count with the noun that agrees with it, taking an explicit plural where adding 's' is wrong ("1 host entry" / "2 host entries"). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NiMd9TR2SJ4PAY4tmSBWzd --- cloudx_proxy/setup.py | 34 +++++++++++++++-- tests/test_cli_options.py | 78 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 107 insertions(+), 5 deletions(-) diff --git a/cloudx_proxy/setup.py b/cloudx_proxy/setup.py index 214b87e..b5e9c15 100644 --- a/cloudx_proxy/setup.py +++ b/cloudx_proxy/setup.py @@ -25,6 +25,22 @@ from .colors import prompt as color_prompt +def plural(count: int, singular: str, plural_form: str | None = None) -> str: + """Render a count with its noun: '1 environment', '2 environments'. + + Args: + count: How many + singular: The noun as written for one + plural_form: The noun for any other count, when adding 's' is wrong + + Returns: + str: The count and the noun that agrees with it + """ + if count == 1: + return f"{count} {singular}" + return f"{count} {plural_form or singular + 's'}" + + class CloudXSetup: # Define SSH key prefix as a constant SSH_KEY_PREFIX = "cloudX SSH Key - " @@ -1916,11 +1932,19 @@ def cleanup_config(self) -> bool: for env_data in parsed['environments'].values() ) - self.print_status(f"[DRY RUN] Would reorganize {len(parsed['environments'])} environments", None, 2) - self.print_status(f"[DRY RUN] Would reorganize {total_hosts} host entries", None, 2) + self.print_status( + f"[DRY RUN] Would reorganize {plural(len(parsed['environments']), 'environment')}", + None, 2 + ) + self.print_status( + f"[DRY RUN] Would reorganize {plural(total_hosts, 'host entry', 'host entries')}", + None, 2 + ) if parsed['other']: self.print_status( - f"[DRY RUN] Would preserve {len(parsed['other'])} unmanaged entries as-is", None, 2 + "[DRY RUN] Would preserve " + f"{plural(len(parsed['other']), 'unmanaged entry', 'unmanaged entries')}" + " as-is", None, 2 ) return True @@ -1970,7 +1994,9 @@ def cleanup_config(self) -> bool: if parsed['other']: self.print_status( - f"Preserved {len(parsed['other'])} unmanaged entries as-is", True, 2 + "Preserved " + f"{plural(len(parsed['other']), 'unmanaged entry', 'unmanaged entries')}" + " as-is", True, 2 ) self.print_status("Cleanup completed and config reorganized", True, 2) return True diff --git a/tests/test_cli_options.py b/tests/test_cli_options.py index e1a087e..1efc9cb 100644 --- a/tests/test_cli_options.py +++ b/tests/test_cli_options.py @@ -13,7 +13,7 @@ from click.testing import CliRunner from cloudx_proxy.cli import cli -from cloudx_proxy.setup import CloudXSetup +from cloudx_proxy.setup import CloudXSetup, plural def run_setup(tmp_path, monkeypatch, extra_args): @@ -273,3 +273,79 @@ def test_a_real_failure_is_marked_once(self, tmp_path, monkeypatch, capsys): marks = [symbol for _i, symbol, _t in status_lines(capsys.readouterr().out)] assert marks.count("✗") == 1, f"one failure, one cross: {marks}" + + +class TestCountsAgreeWithTheirNoun: + """`Would reorganize 1 environments` reads like a bug in the tool.""" + + def cleanup_output(self, tmp_path, monkeypatch, config, args=()): + monkeypatch.setattr("cloudx_proxy.cli.sys.argv", ["cloudX-proxy"]) + ssh_dir = tmp_path / "cloudX" + ssh_dir.mkdir(parents=True, exist_ok=True) + (ssh_dir / "config").write_text(config) + + result = CliRunner().invoke( + cli, ["cleanup", "--ssh-config", str(ssh_dir / "config"), *args] + ) + assert result.exit_code == 0, result.output + return result.output + + ONE = """Host cloudX-dev-* + IdentityFile ~/.ssh/cloudX/cloudX + +Host cloudX-dev-web1 + HostName i-0123456789abcdef0 + +Host mybox + HostName 10.0.0.1 +""" + + TWO = ONE + """ +Host cloudX-prd-* + IdentityFile ~/.ssh/cloudX/cloudX + +Host cloudX-prd-web1 + HostName i-0aaaaaaaaaaaaaaaa + +Host cloudX-prd-web2 + HostName i-0bbbbbbbbbbbbbbbb + +Host otherbox + HostName 10.0.0.2 +""" + + def test_one_of_each(self, tmp_path, monkeypatch): + output = self.cleanup_output(tmp_path, monkeypatch, self.ONE, ["--dry-run"]) + + assert "1 environment\n" in output or "1 environment " in output + assert "1 environments" not in output + assert "1 host entry" in output + assert "1 unmanaged entry as-is" in output + + def test_more_than_one_of_each(self, tmp_path, monkeypatch): + output = self.cleanup_output(tmp_path, monkeypatch, self.TWO, ["--dry-run"]) + + assert "2 environments" in output + assert "3 host entries" in output + assert "2 unmanaged entries as-is" in output + + def test_the_real_run_agrees_too(self, tmp_path, monkeypatch): + assert "1 unmanaged entry as-is" in self.cleanup_output( + tmp_path, monkeypatch, self.ONE + ) + assert "2 unmanaged entries as-is" in self.cleanup_output( + tmp_path, monkeypatch, self.TWO + ) + + +class TestPluralHelper: + def test_one(self): + assert plural(1, "environment") == "1 environment" + + def test_zero_and_many(self): + assert plural(0, "environment") == "0 environments" + assert plural(7, "environment") == "7 environments" + + def test_irregular(self): + assert plural(1, "host entry", "host entries") == "1 host entry" + assert plural(2, "host entry", "host entries") == "2 host entries" From 3f537c44c080e5eb8b7d66f02458bd2874bc379e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 08:46:22 +0000 Subject: [PATCH 10/11] fix: detect the command name on Windows too The host prefix is taken from the command that was invoked, by comparing basename(sys.argv[0]) with 'cloudX-proxy'. On Windows a console script is an .exe, so that basename is 'cloudX-proxy.exe' and never matched: every Windows user silently got the lowercase prefix whichever of the two commands they typed. Compare the stem instead. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NiMd9TR2SJ4PAY4tmSBWzd --- cloudx_proxy/cli.py | 23 +++++++++++++++++------ tests/test_cli_options.py | 31 ++++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/cloudx_proxy/cli.py b/cloudx_proxy/cli.py index ab6ea0b..ad2f542 100644 --- a/cloudx_proxy/cli.py +++ b/cloudx_proxy/cli.py @@ -31,6 +31,20 @@ def detect_ssh_defaults() -> tuple: return "cloudX", "cloudX", "~/.ssh/cloudX" +def prefix_from_command_name() -> str: + """The host prefix implied by the command name that was invoked. + + On Windows a console script is an .exe, so sys.argv[0] ends in one and a + bare basename never equals 'cloudX-proxy' - every Windows user silently + got the lowercase prefix whichever command they typed. Compare the stem. + + Returns: + str: 'cloudX' when invoked as cloudX-proxy, otherwise 'cloudx' + """ + stem = os.path.splitext(os.path.basename(sys.argv[0]))[0] + return 'cloudX' if stem == 'cloudX-proxy' else 'cloudx' + + def short_host_name(host: str, host_prefix: str) -> str: """Strip '--' off a host entry to get its short name. @@ -188,8 +202,7 @@ def setup(profile: str, ssh_key: str, ssh_config: str, ssh_dir: str, aws_env: st try: # Determine default prefix based on command name if not provided if not ssh_host_prefix: - cmd_name = os.path.basename(sys.argv[0]) - ssh_host_prefix = 'cloudX' if cmd_name == 'cloudX-proxy' else 'cloudx' + ssh_host_prefix = prefix_from_command_name() setup = CloudXSetup( profile=profile, @@ -355,8 +368,7 @@ def list(ssh_config: str, environment: str, detailed: bool, dry_run: bool): sys.exit(1) # Detect ssh_host_prefix from command name - cmd_name = os.path.basename(sys.argv[0]) - ssh_host_prefix = 'cloudX' if cmd_name == 'cloudX-proxy' else 'cloudx' + ssh_host_prefix = prefix_from_command_name() # Use shared parser from CloudXSetup setup = CloudXSetup(ssh_config=str(config_file), ssh_host_prefix=ssh_host_prefix) @@ -522,8 +534,7 @@ def cleanup(ssh_config: str, ssh_host_prefix: str, dry_run: bool): # Determine default prefix based on command name if not provided if not ssh_host_prefix: - cmd_name = os.path.basename(sys.argv[0]) - ssh_host_prefix = 'cloudX' if cmd_name == 'cloudX-proxy' else 'cloudx' + ssh_host_prefix = prefix_from_command_name() setup = CloudXSetup(ssh_config=ssh_config, ssh_host_prefix=ssh_host_prefix, dry_run=dry_run) diff --git a/tests/test_cli_options.py b/tests/test_cli_options.py index 1efc9cb..587f54b 100644 --- a/tests/test_cli_options.py +++ b/tests/test_cli_options.py @@ -9,10 +9,12 @@ upgrade that changes it fails here rather than in the field. """ +import ntpath + import pytest from click.testing import CliRunner -from cloudx_proxy.cli import cli +from cloudx_proxy.cli import cli, prefix_from_command_name from cloudx_proxy.setup import CloudXSetup, plural @@ -349,3 +351,30 @@ def test_zero_and_many(self): def test_irregular(self): assert plural(1, "host entry", "host entries") == "1 host entry" assert plural(2, "host entry", "host entries") == "2 host entries" + + +class TestPrefixFromCommandName: + """On Windows a console script is an .exe. + + sys.argv[0] therefore ends in one, a bare basename never equalled + 'cloudX-proxy', and every Windows user silently got the lowercase prefix + whichever of the two commands they typed. + """ + + def test_posix(self, monkeypatch): + for argv0, expected in ( + ("/home/erik/.local/bin/cloudX-proxy", "cloudX"), + ("/home/erik/.local/bin/cloudx-proxy", "cloudx"), + ("cloudX-proxy", "cloudX"), + ): + monkeypatch.setattr("cloudx_proxy.cli.sys.argv", [argv0]) + assert prefix_from_command_name() == expected, argv0 + + def test_windows_exe(self, monkeypatch): + monkeypatch.setattr("cloudx_proxy.cli.os.path", ntpath) + for argv0, expected in ( + (r"C:\Users\erik\AppData\Roaming\uv\tools\x\Scripts\cloudX-proxy.exe", "cloudX"), + (r"C:\Users\erik\AppData\Roaming\uv\tools\x\Scripts\cloudx-proxy.exe", "cloudx"), + ): + monkeypatch.setattr("cloudx_proxy.cli.sys.argv", [argv0]) + assert prefix_from_command_name() == expected, argv0 From d0911acdd24e80d3097654842516638ecf93382f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 08:20:14 +0000 Subject: [PATCH 11/11] fix: dry run previews the patterns it will actually write The preview showed 'cloudX-*' while the write produces 'cloudX-* cloudx-*', so the one command meant for looking before you leap did not show the change this release makes. Route the preview through the same helper as the write. Host entries stay a single name, as they are written. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NiMd9TR2SJ4PAY4tmSBWzd --- cloudx_proxy/setup.py | 8 ++++++-- tests/test_cli_options.py | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/cloudx_proxy/setup.py b/cloudx_proxy/setup.py index b5e9c15..8f21aac 100644 --- a/cloudx_proxy/setup.py +++ b/cloudx_proxy/setup.py @@ -2158,8 +2158,12 @@ def setup_ssh_config(self, cloudx_env: str, instance_id: str, hostname: str) -> if self.dry_run: self.print_status("[DRY RUN] Would set up SSH configuration with three-tier approach") - self.print_status(f"[DRY RUN] Would create generic pattern: {self.ssh_host_prefix}-*", None, 2) - self.print_status(f"[DRY RUN] Would create environment pattern: {self.ssh_host_prefix}-{cloudx_env}-*", None, 2) + # Preview the patterns as they will be written, both spellings and + # all: a dry run is how someone checks what is about to happen. + generic = self._host_line_value(f"{self.ssh_host_prefix}-*") + environment = self._host_line_value(f"{self.ssh_host_prefix}-{cloudx_env}-*") + self.print_status(f"[DRY RUN] Would create generic pattern: {generic}", None, 2) + self.print_status(f"[DRY RUN] Would create environment pattern: {environment}", None, 2) self.print_status(f"[DRY RUN] Would create host entry: {self.ssh_host_prefix}-{cloudx_env}-{hostname} -> {instance_id}", None, 2) self.print_status(f"[DRY RUN] Would write configuration to: {self.ssh_config_file}", None, 2) return True diff --git a/tests/test_cli_options.py b/tests/test_cli_options.py index 587f54b..0a33bae 100644 --- a/tests/test_cli_options.py +++ b/tests/test_cli_options.py @@ -378,3 +378,41 @@ def test_windows_exe(self, monkeypatch): ): monkeypatch.setattr("cloudx_proxy.cli.sys.argv", [argv0]) assert prefix_from_command_name() == expected, argv0 + + +class TestDryRunPreviewsWhatIsWritten: + """A dry run is how you check what is about to happen to your config. + + It previewed `cloudX-*` while the write produces `cloudX-* cloudx-*`, so + the one command meant for looking before you leap did not show the change + this release actually makes. + """ + + def preview(self, tmp_path, monkeypatch, argv0="cloudX-proxy"): + monkeypatch.setattr("cloudx_proxy.setup.boto3.Session", lambda *a, **k: None) + monkeypatch.setattr("cloudx_proxy.cli.sys.argv", [argv0]) + + result = CliRunner().invoke(cli, [ + "setup", "--dry-run", "--yes", + "--instance", "i-0123456789abcdef0", + "--hostname", "test", "--environment", "dev", + "--ssh-config", str(tmp_path / "cloudX" / "config"), + ]) + assert result.exit_code == 0, result.output + return result.output + + def test_patterns_show_both_spellings(self, tmp_path, monkeypatch): + output = self.preview(tmp_path, monkeypatch) + + assert "Would create generic pattern: cloudX-* cloudx-*" in output + assert "Would create environment pattern: cloudX-dev-* cloudx-dev-*" in output + + def test_the_host_entry_stays_a_single_name(self, tmp_path, monkeypatch): + output = self.preview(tmp_path, monkeypatch) + + assert "Would create host entry: cloudX-dev-test -> i-0123456789abcdef0" in output + + def test_the_other_command_name_leads_with_its_own(self, tmp_path, monkeypatch): + output = self.preview(tmp_path, monkeypatch, argv0="cloudx-proxy") + + assert "Would create generic pattern: cloudx-* cloudX-*" in output