Surface actionable error when git remote redirects to sign-in or requires credentials (#1138)

[spoorcc]

Previously, GitRemote.is_git() silently swallowed any git ls-remote failure
that wasn't "Could not resolve host", returning False and letting dfetch
fall through to SVN or fail with "vcs type unsupported" — giving the user
no hint that authentication was needed.

Now two additional error patterns are detected in stderr and surfaced as
RuntimeError with an actionable message:

• "unable to update url base from redirection" (git/git:http.c): server
  redirected to a login page
• "terminal prompts disabled" / "could not read Username"
  (git/git:credential.c): server returned 401 but prompts are disabled

Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01XDsvuqHVthjgZdNWRLjeXo

Summary by CodeRabbit

• Bug Fixes
  • Improved Git remote detection when git ls-remote fails for authentication-related reasons (e.g., redirect-to sign-in or non-interactive credential/prompt failures). Auth-required remotes are now recognized as Git instead of being misclassified, so the auth error is surfaced on the following fetch.
• Tests
  • Added new coverage to verify remote detection returns success for specific authentication error messages.

[coderabbitai]

Warning

Review limit reached

@spoorcc, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 28 minutes and 22 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 020d9711-125c-42f9-9d15-e13e650dadfa

📥 Commits

Reviewing files that changed from the base of the PR and between ac7a6e6 and 6005e0a.

📒 Files selected for processing (3)

• AGENTS.md
• CHANGELOG.rst
• dfetch/vcs/git.py

Walkthrough

GitRemote.is_git() now recognizes two authentication-related subprocess failures as indicating a valid Git remote: HTTP redirect-to-login responses (extracting the redirected URL from stderr) and non-interactive terminal prompt failures. Both cases return True instead of falling through to non-Git detection. A parametrized test covers both scenarios, and the changelog documents the corrected behavior.

Changes

Authentication Error Detection in GitRemote.is_git()

Layer / File(s)

Summary

Auth failure detection implementation and tests dfetch/vcs/git.py, tests/test_git_vcs.py, CHANGELOG.rst

GitRemote.is_git() refactors SubprocessCommandError handling through a new private _handle_ls_remote_error() helper that inspects exc.stderr for redirect-to-login markers (extracting and logging the redirected URL) and terminal-prompt-disabled failures, returning True in both cases to recognize the remote as Git-based. A parametrized test patches run_on_cmdline with matching stderr payloads and asserts is_git() returns True. The changelog documents this behavior correction under an unreleased 0.14.2 entry.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• dfetch-org/dfetch#908: Both PRs modify GitRemote.is_git() and git ls-remote error handling in non-interactive mode, with the main PR extending SubprocessCommandError detection to treat auth/redirect stderr patterns as successful Git remote indicators.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly describes the main change: improving error handling to surface actionable errors when git remotes redirect to sign-in or require credentials, which matches the core objective of issue #1138.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/dfetch-issue-1138-ym55k3

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[ben-edna]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_git_vcs.py`:
- Line 258: The test directly mutates the global process environment by
assigning to os.environ["GIT_SSH_COMMAND"], which can cause test state to leak
and make subsequent tests order-dependent. Replace the direct environment
variable assignment with pytest's monkeypatch.setenv() method instead. Ensure
the test function accepts monkeypatch as a parameter (pytest provides this
fixture automatically), then use monkeypatch.setenv("GIT_SSH_COMMAND", "ssh")
instead of os.environ["GIT_SSH_COMMAND"] = "ssh" so the environment state is
automatically restored after the test completes.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6f271b6a-c385-420e-acff-5cd9e3a8d60d

📥 Commits

Reviewing files that changed from the base of the PR and between ef32ad6 and 63901d1.

📒 Files selected for processing (3)

• CHANGELOG.rst
• dfetch/vcs/git.py
• tests/test_git_vcs.py

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@dfetch/vcs/git.py`:
- Around line 117-122: The logger.debug call in this block logs the full
redirect_url verbatim, which can expose sensitive query parameters and
authentication tokens in debug logs. Extract only the safe parts of the
redirect_url (origin and path) by parsing it using URL parsing utilities,
removing the query string and fragment components, and then use this sanitized
URL in the logger.debug call instead of the full redirect_url variable to
prevent credential leakage in logs.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: dae9d01c-51cf-4f6a-81e0-b85e4ba3c198

📥 Commits

Reviewing files that changed from the base of the PR and between 63901d1 and 32b2abe.

📒 Files selected for processing (3)

• CHANGELOG.rst
• dfetch/vcs/git.py
• tests/test_git_vcs.py

[coderabbitai]

♻️ Duplicate comments (1)

dfetch/vcs/git.py (1)

128-133: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Redact redirected URL before logging.

At Line 128, redirect_url is logged verbatim; query/fragment values can include sensitive tokens from auth flows. Log only scheme/host/path (or redact query+fragment) before emitting debug logs.

Proposed fix

+            parsed = urllib.parse.urlsplit(redirect_url)
+            safe_redirect = urllib.parse.urlunsplit(
+                (parsed.scheme, parsed.netloc, parsed.path, "", "")
+            )
             logger.debug(
                 "'%s' appears to be a git remote but was redirected to '%s' — "
                 "authentication may be required",
                 self._remote,
-                redirect_url,
+                safe_redirect or "unknown",
             )

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@dfetch/vcs/git.py` around lines 128 - 133, The redirect_url variable is being
logged directly in the debug message at this location, but URLs can contain
sensitive authentication tokens in their query or fragment components. To fix
this, parse the redirect_url before logging it and either extract only the
scheme/host/path components for logging, or explicitly redact the query and
fragment portions. Ensure the sanitized URL is passed to the logger.debug call
instead of the original redirect_url to prevent sensitive credentials from
appearing in debug logs.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@dfetch/vcs/git.py`:
- Around line 128-133: The redirect_url variable is being logged directly in the
debug message at this location, but URLs can contain sensitive authentication
tokens in their query or fragment components. To fix this, parse the
redirect_url before logging it and either extract only the scheme/host/path
components for logging, or explicitly redact the query and fragment portions.
Ensure the sanitized URL is passed to the logger.debug call instead of the
original redirect_url to prevent sensitive credentials from appearing in debug
logs.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: d27cddcf-3794-49ad-bcf2-88807225ac69

📥 Commits

Reviewing files that changed from the base of the PR and between 32b2abe and ac7a6e6.

📒 Files selected for processing (1)

• dfetch/vcs/git.py