Skip to content

parseRemoteAuthority: parse right-to-left to handle all safeHost shapes uniformly #937

@EhabY

Description

@EhabY

Follow-up from the review thread on #930 (#930 (comment)).

Problem

parseRemoteAuthority (src/util.ts) splits the SSH host name on --, which conflates the field separator with -- that legitimately appears inside <safeHost> (Punycode xn--abc, apex Punycode xn--p1ai, triple-dash Punycode xn--test---8o4..., Latin double-dash test--domain.com). The current parts[0].endsWith(".xn") merge loop added in #930 only fixes one of these shapes.

Approach

Don't change the wire format. Parse from the right.

The encoder writes coder-vscode.<safeHost>--<user>--<ws>(.<agent>)?. After split("--"), the last element is always <ws> or <ws>.<agent> and the one before it is always <user>. Everything earlier rejoins with -- to give back coder-vscode.<safeHost>, no matter how many -- it contains.

Algorithm

  1. Split on --. Require parts[0] starts with "coder-vscode.", else return null.
  2. Require parts.length >= 3 and no empty parts, else throw Invalid Coder SSH authority.
  3. parts[-1] is <ws> or <ws>.<agent> (split on .). parts[-2] is <user>.
  4. Rejoin parts.slice(0, -2) with -- and strip the coder-vscode. prefix to get safeHostname.

Single code path, no Unicode awareness.

What we lose

Two old authority shapes the current parser still accepts:

  1. No safeHost (coder-vscode--<user>--<ws>(--|.)<agent?>): produced before Add support for connections to multiple deployments #292 (Jun 2024). About 23 months ago.
  2. SafeHost with --<agent> (coder-vscode.<safeHost>--<user>--<ws>--<agent>): produced between Add support for connections to multiple deployments #292 (Jun 2024) and Use coder ssh in place of coder vscodessh #420 (Feb 2025). Window ended about 15 months ago.

Both can still surface in VS Code's Recent menu for users who never reopened the workspace through the Coder panel since. Failure mode is a "not logged in" prompt or a null parse, not a crash. Reopening from the panel writes a fresh authority in the current shape and they're back to working.

Scope

Parser

  • src/util.ts: replace parseRemoteAuthority body per the algorithm.

Tests (test/unit/util.test.ts)

  • Move coder-vscode--foo--bar and coder-vscode--foo--bar--baz (lines 44-54 and 55-65) into ignores unrelated authority (now return null).
  • Remove with hostname and -- agent (lines 77-87).
  • Add test--domain.com, xn--test---8o4..., and a regression case with random -- in the middle of a safeHost.

Dead helper cleanup (empty-hostname branches that were only reachable via the dropped shapes)

  • src/core/pathResolver.ts: remove empty-hostname branch in getGlobalConfigDir and the related doc comments on the get*Path helpers.
  • src/remote/sshConfig.ts: simplify startBlockComment / endBlockComment to always include the hostname; simplify the malformed-config error messages.
  • src/core/secretsManager.ts: remove the || "<legacy>" fallback in buildKey.
  • src/extension.ts:113: skip the getSessionAuth call when deployment is undefined instead of falling back to "".
  • test/unit/core/pathResolver.test.ts: remove should use base path for empty labels (lines 22-29).
  • test/unit/remote/sshConfig.test.ts: remove creates a new file and adds config with empty label (lines 48-84) and the update("") case at line 447. Keep does not remove deployment-unaware SSH config (line 242); it still passes since getBlock("dev.coder.com") does not match the legacy block format.

Why not change the encoding

Escaping -- in <safeHost> at toRemoteAuthority time would also work, but it creates the inverse backward-incompat (older extension versions can't parse newer authorities) and adds a custom escape format. The right-to-left parser needs no encoding change.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions