fix(gitlab): make the authentication fallback actually run

_get_auth_headers guesses between Bearer and PRIVATE-TOKEN from the shape of the
token, and retries once under the other scheme on a 401 so a wrong guess does not
fail the run. That retry has never executed.

Three things had to line up and none of them did:

  - The retry caught requests.exceptions.HTTPError, but CliClient translates every
    requests error into APIFailure before it gets there.
  - CliClient discarded the HTTP status, so even a caught failure could not be
    identified as a 401. is_transient_error was equally blind for the same reason.
  - There are two APIFailure classes -- the CLI's own and the SDK's -- and they
    were independent Exception subclasses. CliClient raises the CLI's; every
    handler in socketsecurity.core imports the SDK's. None of those eight handlers
    has ever caught a CliClient failure.

The CLI's APIFailure now subclasses the SDK's, so a handler written against either
catches both, and the status code travels with the exception.

The two tests covering the fallback were skipped rather than fixed, with a reason
that no longer described the failure -- the constructor they blamed is used by the
two passing tests in the same file. They now drive the exception the way CliClient
actually raises it, and fail if any of the three links above is broken again.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: lelia

CHANGELOG.md

@@ -66,6 +66,21 @@
   access and honors the command with a warning where the provider cannot report it,
   `strict` rejects it in that case instead, and `off` performs no check.
 
+### Fixed: GitLab authentication fallback never ran
+
+- When a GitLab token's type cannot be inferred from its shape, the CLI guesses
+  between Bearer and PRIVATE-TOKEN and retries once under the other scheme on a
+  401. That retry never happened: the retry caught `requests.exceptions.HTTPError`,
+  but the HTTP client translates every request error into `APIFailure` first, so a
+  misclassified token failed the run instead of falling back.
+- API failures raised by the CLI's HTTP client now carry their HTTP status code.
+  Without it a 401 was indistinguishable from any other failure, and
+  `is_transient_error` could not classify one either.
+- The CLI's `APIFailure` now subclasses the SDK exception of the same name. They
+  were independent types, so an `except APIFailure` importing the SDK's — which is
+  what every handler in `socketsecurity.core` does — did not catch a failure raised
+  by the HTTP client.
+
 ### Fixed: pull request and merge request comment accuracy
 
 - Per-alert ignore instructions now use ecosystem-qualified package names and

socketsecurity/core/cli_client.py

@@ -56,7 +56,12 @@ def request(
 
         except requests.exceptions.RequestException as e:
             logger.error(f"API request failed: {str(e)}")
-            raise APIFailure(f"Request failed: {str(e)}")
+            # Carry the status forward. Callers that need to react to a specific
+            # code -- the GitLab auth fallback to the other token scheme, and
+            # APIFailure.is_transient_error -- have no other way to recover it
+            # once the requests exception has been translated.
+            status_code = e.response.status_code if e.response is not None else None
+            raise APIFailure(f"Request failed: {str(e)}", status_code=status_code) from e
 
     def post_telemetry_events(self, org_slug: str, events: List[Dict]) -> None:
         """Post telemetry events one at a time to the v0 telemetry API. Fire-and-forget — logs errors but never raises."""

socketsecurity/core/exceptions.py

@@ -1,3 +1,5 @@
+from socketdev.exceptions import APIFailure as SdkAPIFailure
+
 __all__ = [
     "APIFailure",
     "APIKeyMissing",
@@ -18,8 +20,15 @@ class APIKeyMissing(Exception):
     pass
 
 
-class APIFailure(Exception):
-    """Raised when there is an error using the API"""
+class APIFailure(SdkAPIFailure):
+    """Raised when there is an error using the API.
+
+    Subclasses the SDK's exception of the same name so a handler written against
+    either one catches both. They were independent Exception subclasses, so an
+    ``except APIFailure`` importing the SDK's -- which every handler in
+    socketsecurity.core does -- silently let a CliClient failure through, and the
+    status code the SDK class carries was unavailable to anything raised here.
+    """
     pass
 
 

socketsecurity/core/scm/gitlab.py

@@ -5,6 +5,7 @@
 from typing import Optional
 
 import requests
+from socketdev.exceptions import APIFailure
 
 from socketsecurity import USER_AGENT
 from socketsecurity.core import log
@@ -149,32 +150,31 @@ def __init__(
         self._member_lookup_attempted = False
 
     def _request_with_fallback(self, **kwargs):
-        """
-        Make a request with automatic fallback between Bearer and PRIVATE-TOKEN authentication.
-        This provides robustness when the initial token type detection is incorrect.
+        """Request with one retry under the other GitLab auth scheme on a 401.
+
+        _get_auth_headers guesses between Bearer and PRIVATE-TOKEN from the shape of
+        the token, and the guess can be wrong for tokens that do not match a known
+        pattern. Rather than fail the run, try the other scheme once.
+
+        Catches APIFailure, not requests.exceptions.HTTPError: CliClient translates
+        every requests error into APIFailure, which does not inherit from HTTPError,
+        so catching the latter here never fired and the fallback never ran.
         """
         try:
-            # Try the initial request with the configured headers
             return self.client.request(**kwargs)
-        except requests.exceptions.HTTPError as e:
-            # Check if this is an authentication error (401)
-            if e.response and e.response.status_code == 401:
-                log.debug("Authentication failed with initial headers, trying fallback method")
-                
-                # Determine the fallback headers
-                original_headers = kwargs.get('headers', self.config.headers)
-                fallback_headers = self._get_fallback_headers(original_headers)
-                
-                if fallback_headers and fallback_headers != original_headers:
-                    log.debug("Retrying request with fallback authentication method")
-                    kwargs['headers'] = fallback_headers
-                    return self.client.request(**kwargs)
-            
-            # Re-raise the original exception if it's not an auth error or fallback failed
-            raise
-        except Exception:
-            # Handle other types of exceptions that don't have response attribute
-            raise
+        except APIFailure as error:
+            if error.status_code != 401:
+                raise
+
+            log.debug("Authentication failed with initial headers, trying fallback method")
+            original_headers = kwargs.get('headers', self.config.headers)
+            fallback_headers = self._get_fallback_headers(original_headers)
+            if not fallback_headers or fallback_headers == original_headers:
+                raise
+
+            log.debug("Retrying request with fallback authentication method")
+            kwargs['headers'] = fallback_headers
+            return self.client.request(**kwargs)
 
     def _get_fallback_headers(self, original_headers: dict) -> dict:
         """

tests/unit/test_client.py

@@ -174,3 +174,56 @@ def test_post_telemetry_events_continues_on_failure(client):
         client.post_telemetry_events("test-org", events)
 
         assert mock_request.call_count == 2
+
+
+def test_request_preserves_the_http_status_on_failure():
+    """The status is the only thing that survives translation to APIFailure.
+
+    Callers that must react to a specific code -- the GitLab auth fallback on a
+    401, and APIFailure.is_transient_error -- have no other way to recover it once
+    the requests exception is gone.
+    """
+    config = SocketConfig(api_key="test_key")
+    client = CliClient(config)
+
+    with patch('requests.request') as mock_request:
+        mock_response = Mock()
+        mock_response.status_code = 401
+        error = requests.exceptions.HTTPError("401 Client Error")
+        error.response = mock_response
+        mock_response.raise_for_status.side_effect = error
+        mock_request.return_value = mock_response
+
+        with pytest.raises(APIFailure) as exc_info:
+            client.request("test/path")
+
+    assert exc_info.value.status_code == 401
+
+
+def test_request_tolerates_a_failure_with_no_response():
+    """A connection error never reached a server, so there is no status to carry."""
+    config = SocketConfig(api_key="test_key")
+    client = CliClient(config)
+
+    with patch('requests.request') as mock_request:
+        mock_request.side_effect = requests.exceptions.ConnectionError("no route")
+
+        with pytest.raises(APIFailure) as exc_info:
+            client.request("test/path")
+
+    assert exc_info.value.status_code is None
+
+
+def test_a_handler_written_against_the_sdk_exception_catches_client_failures():
+    """socketsecurity.core imports the SDK's APIFailure in every handler, while
+    CliClient raises the CLI's own. They must not be independent types."""
+    from socketdev.exceptions import APIFailure as SdkAPIFailure
+
+    config = SocketConfig(api_key="test_key")
+    client = CliClient(config)
+
+    with patch('requests.request') as mock_request:
+        mock_request.side_effect = requests.exceptions.ConnectionError("no route")
+
+        with pytest.raises(SdkAPIFailure):
+            client.request("test/path")

tests/unit/test_gitlab_auth_fallback.py

@@ -1,13 +1,28 @@
-"""Integration test demonstrating GitLab authentication fallback"""
+"""GitLab authentication fallback.
+
+_get_auth_headers guesses between Bearer and PRIVATE-TOKEN from the shape of the
+token. When the guess is wrong the first request comes back 401, and the CLI
+retries once under the other scheme rather than failing the run.
+
+The retry hinges on which exception type it catches: CliClient translates every
+requests error into APIFailure, which is not an HTTPError, so these tests drive
+the failure the way CliClient actually raises it.
+"""
 import os
 from unittest.mock import MagicMock, patch
 
 import pytest
+from socketdev.exceptions import APIFailure
 
 from socketsecurity.core.scm.gitlab import Gitlab
 from socketsecurity.socketcli import CliClient
 
 
+def _auth_failure() -> APIFailure:
+    """The exception CliClient raises for a 401, with the status preserved."""
+    return APIFailure("Request failed: 401 Client Error", status_code=401)
+
+
 class TestGitlabAuthFallback:
     """Test GitLab authentication fallback mechanism"""
 
@@ -18,21 +33,13 @@ class TestGitlabAuthFallback:
         'CI_MERGE_REQUEST_IID': '123',
         'CI_MERGE_REQUEST_PROJECT_ID': '456'
     })
-    @pytest.mark.skip(reason="Gitlab constructor does not accept client kwarg; needs rework to match current implementation")
     def test_fallback_from_private_token_to_bearer(self):
         """Test fallback from PRIVATE-TOKEN to Bearer authentication"""
         # Create a mock client that simulates auth failure then success
         mock_client = MagicMock(spec=CliClient)
 
-        # First call (with PRIVATE-TOKEN) fails with 401
-        auth_error = Exception()
-        auth_error.response = MagicMock()
-        auth_error.response.status_code = 401
-        
-        # Second call (with Bearer) succeeds
-        success_response = {'notes': []}
-        
-        mock_client.request.side_effect = [auth_error, success_response]
+        # First call (with PRIVATE-TOKEN) fails with 401, second (Bearer) succeeds
+        mock_client.request.side_effect = [_auth_failure(), MagicMock(json=lambda: [])]
 
         # Create GitLab instance with mock client
         gitlab = Gitlab(client=mock_client)
@@ -60,21 +67,13 @@ def test_fallback_from_private_token_to_bearer(self):
         'CI_MERGE_REQUEST_IID': '123',
         'CI_MERGE_REQUEST_PROJECT_ID': '456'
     })
-    @pytest.mark.skip(reason="Gitlab constructor does not accept client kwarg; needs rework to match current implementation")
     def test_fallback_from_bearer_to_private_token(self):
         """Test fallback from Bearer to PRIVATE-TOKEN authentication"""
         # Create a mock client that simulates auth failure then success
         mock_client = MagicMock(spec=CliClient)
 
-        # First call (with Bearer) fails with 401
-        auth_error = Exception()
-        auth_error.response = MagicMock()
-        auth_error.response.status_code = 401
-        
-        # Second call (with PRIVATE-TOKEN) succeeds
-        success_response = {'notes': []}
-        
-        mock_client.request.side_effect = [auth_error, success_response]
+        # First call (with Bearer) fails with 401, second (PRIVATE-TOKEN) succeeds
+        mock_client.request.side_effect = [_auth_failure(), MagicMock(json=lambda: [])]
 
         # Create GitLab instance with mock client
         gitlab = Gitlab(client=mock_client)
@@ -107,18 +106,16 @@ def test_non_auth_error_not_retried(self):
         # Create a mock client that simulates a non-auth error
         mock_client = MagicMock(spec=CliClient)
 
-        # Simulate a 500 error (not auth-related)
-        server_error = Exception()
-        server_error.response = MagicMock()
-        server_error.response.status_code = 500
-        
-        mock_client.request.side_effect = server_error
+        # A 500 is not recoverable by changing the auth scheme.
+        mock_client.request.side_effect = APIFailure(
+            "Request failed: 500 Server Error", status_code=500
+        )
 
         # Create GitLab instance with mock client
         gitlab = Gitlab(client=mock_client)
 
         # This should NOT trigger the fallback mechanism
-        with pytest.raises(Exception):
+        with pytest.raises(APIFailure):
             gitlab.get_comments_for_pr()
 
         # Verify only one request was made (no retry)
@@ -135,7 +132,7 @@ def test_successful_first_attempt_no_fallback(self):
         """Test that successful requests don't trigger fallback"""
         # Create a mock client that succeeds on first try
         mock_client = MagicMock(spec=CliClient)
-        mock_client.request.return_value = {'notes': []}
+        mock_client.request.return_value = MagicMock(json=lambda: [])
 
         # Create GitLab instance with mock client
         gitlab = Gitlab(client=mock_client)