Skip to content

fix(deps): honor SSH user from dependency URL (closes #1383)#1385

Merged
danielmeppiel merged 1 commit into
mainfrom
danielmeppiel/fix-ssh-user-issue-1383
May 19, 2026
Merged

fix(deps): honor SSH user from dependency URL (closes #1383)#1385
danielmeppiel merged 1 commit into
mainfrom
danielmeppiel/fix-ssh-user-issue-1383

Conversation

@danielmeppiel
Copy link
Copy Markdown
Collaborator

Closes #1383

Problem

build_ssh_url() unconditionally prepended git@ even when the user explicitly specified a non-git SSH user in the dependency URL (e.g. EMU accounts like meppiel-microsoft or ado-username@org). The user portion of ssh://, scp-shorthand, and parsed dependency URLs was silently dropped, leaving clone attempts authenticated as the wrong identity.

Fix

Thread ssh_user from the parsed DependencyReference through to per-host backend build_clone_ssh_url callers, validated against a strict allowlist before composing the URL.

Source changes

  • utils/github_host.pyvalidate_ssh_user() with ^[a-zA-Z0-9_][a-zA-Z0-9_.+-]*$ regex, 64-char cap, error messages that hide the user value (only the length is leaked). build_ssh_url(user="git") validates before composing.
  • models/dependency/reference.pyssh_user: str | None field on DependencyReference; parsed from ssh:// and scp shorthand; percent-encoded userinfo rejected at the pre-urlparse layer (defence in depth). Kept out of to_canonical/get_identity so it cannot perturb the install-plan identity invariant.
  • deps/host_backends.py — three non-ADO backends pass user=getattr(dep_ref, "ssh_user", None) or "git". ADO is left untouched (literal git@ is required by the service).

Supply-chain review

Reviewed by the apm-review-panel supply-chain expert (verdict: APPROVE-WITH-CHANGES, all changes applied):

  • Strict allowlist with leading-dash rejection blocks SSH argument injection (-oProxyCommand=…).
  • Length cap prevents pathological inputs.
  • Error message hides the raw user value to avoid leaking secrets if a token is mistakenly pasted into a URL.
  • Identity invariant preserved: ssh_user is presentation-only, never part of canonical identity.

Tests

  • tests/unit/test_ssh_user_threading.py (new) — allowlist boundaries, length cap, percent rejection, error-message-hides-value, parser threading, identity-invariant preservation.
  • tests/integration/test_dep_url_parsing_e2e.py — end-to-end thread from URL string to backend.build_clone_ssh_url.
  • tests/acceptance/test_logging_acceptance.py — drive-by baseline fix: extend bracket pattern to cover the [~], [-], [=] plan-diff symbols (was failing on main).

All 226 SSH-related tests pass; lint clean (ruff check + ruff format --check both silent).

Deferred follow-ups

  • Lockfile ssh_user storageapm.yml is re-parsed on each install, so URL-encoded users round-trip correctly today. Persisting ssh_user in apm.lock.yaml is a separate enhancement and left for a follow-up issue.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

build_ssh_url() unconditionally prepended 'git@' even when the user
explicitly specified a non-git SSH user (e.g. EMU accounts like
'meppiel-microsoft' or 'ado-username@org'). The user portion of
ssh://, scp-shorthand, and parsed dependency URLs was silently
dropped, leaving clone attempts authenticated as the wrong identity.

Threads ssh_user from the parsed DependencyReference through to
the per-host backend build_clone_ssh_url callers and validates it
against a strict allowlist before composing the URL.

Changes:
- utils/github_host.py: add validate_ssh_user() with an
  '[a-zA-Z0-9_][a-zA-Z0-9_.+-]*' regex, 64-char cap, error
  messages that hide the user value; build_ssh_url(user='git')
  validates before composing.
- models/dependency/reference.py: add ssh_user field; parse it
  from ssh:// and scp shorthand; reject percent-encoded userinfo
  at the pre-urlparse layer for defence in depth; keep ssh_user
  out of to_canonical/get_identity so it cannot perturb the
  install-plan identity invariant.
- deps/host_backends.py: pass user=ssh_user (default 'git') from
  the three non-ADO backends; ADO is left untouched (literal
  'git@' is required by the service).

Tests:
- tests/unit/test_ssh_user_threading.py: validate_ssh_user
  allowlist, length cap, percent rejection, error hides value;
  parser threads ssh_user through DependencyReference; ssh_user
  is NOT part of identity.
- tests/integration/test_dep_url_parsing_e2e.py: end-to-end
  thread from URL to backend.build_clone_ssh_url.
- tests/acceptance/test_logging_acceptance.py: extend bracket
  pattern to cover the [~], [-], [=] plan-diff symbols (drive-by
  baseline fix).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 19, 2026 09:12
@danielmeppiel danielmeppiel merged commit 8598d10 into main May 19, 2026
20 checks passed
@danielmeppiel danielmeppiel deleted the danielmeppiel/fix-ssh-user-issue-1383 branch May 19, 2026 09:27
sergio-sisternes-epam pushed a commit that referenced this pull request May 19, 2026
build_ssh_url() unconditionally prepended 'git@' even when the user
explicitly specified a non-git SSH user (e.g. EMU accounts like
'meppiel-microsoft' or 'ado-username@org'). The user portion of
ssh://, scp-shorthand, and parsed dependency URLs was silently
dropped, leaving clone attempts authenticated as the wrong identity.

Threads ssh_user from the parsed DependencyReference through to
the per-host backend build_clone_ssh_url callers and validates it
against a strict allowlist before composing the URL.

Changes:
- utils/github_host.py: add validate_ssh_user() with an
  '[a-zA-Z0-9_][a-zA-Z0-9_.+-]*' regex, 64-char cap, error
  messages that hide the user value; build_ssh_url(user='git')
  validates before composing.
- models/dependency/reference.py: add ssh_user field; parse it
  from ssh:// and scp shorthand; reject percent-encoded userinfo
  at the pre-urlparse layer for defence in depth; keep ssh_user
  out of to_canonical/get_identity so it cannot perturb the
  install-plan identity invariant.
- deps/host_backends.py: pass user=ssh_user (default 'git') from
  the three non-ADO backends; ADO is left untouched (literal
  'git@' is required by the service).

Tests:
- tests/unit/test_ssh_user_threading.py: validate_ssh_user
  allowlist, length cap, percent rejection, error hides value;
  parser threads ssh_user through DependencyReference; ssh_user
  is NOT part of identity.
- tests/integration/test_dep_url_parsing_e2e.py: end-to-end
  thread from URL to backend.build_clone_ssh_url.
- tests/acceptance/test_logging_acceptance.py: extend bracket
  pattern to cover the [~], [-], [=] plan-diff symbols (drive-by
  baseline fix).

Co-authored-by: Daniel Meppiel <copilot-rework@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Hardcoded username when using SSH

1 participant