Cap the commit message read from the local checkout

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changelog
 
+## 2.9.7
+
+### Fixed: oversized commit messages no longer fail the scan
+
+- The 200-character cap on the commit message now applies to the value read from the
+  repository, not only to `--commit-message`. A truncated message ends in `...` and the
+  truncation is reported at INFO.
+- A full scan refused for its size (HTTP 413, 414 or 431) now distinguishes possible
+  upload-size and request-metadata causes and reports what to shorten.
+
 ## 2.9.6
 
 ### Changed: bump pinned @coana-tech/cli to 15.10.48

pyproject.toml

@@ -6,7 +6,7 @@ build-backend = "hatchling.build"
 
 [project]
 name = "socketsecurity"
-version = "2.9.6"
+version = "2.9.7"
 requires-python = ">= 3.11"
 license = {"file" = "LICENSE"}
 dependencies = [

socketsecurity/__init__.py

@@ -1,3 +1,3 @@
 __author__ = 'socket.dev'
-__version__ = '2.9.6'
+__version__ = '2.9.7'
 USER_AGENT = f'SocketPythonCLI/{__version__}'

socketsecurity/config.py

@@ -19,6 +19,32 @@ def get_plugin_config_from_env(prefix: str) -> dict:
         return {}
 
 
+# commit_message rides in the query string of POST /v0/orgs/{org}/full-scans, so an
+# oversized message overflows the edge proxy's request line limit before the API ever
+# sees it. The API itself has no length validation on the field; the rejection comes
+# from the proxy, which reports 413, 414 or 431 depending on which limit it checks. 200
+# chars is a conservative ceiling given URL encoding can 2-3x the raw character count.
+MAX_COMMIT_MESSAGE_LENGTH = 200
+
+
+COMMIT_MESSAGE_TRUNCATION_MARKER = "..."
+
+
+def truncate_commit_message(commit_message: Optional[str]) -> Optional[str]:
+    """Cap commit_message to a length the full-scan request line can carry."""
+    if commit_message and len(commit_message) > MAX_COMMIT_MESSAGE_LENGTH:
+        # INFO, not DEBUG: the scan keeps the truncated value, so for a CI job that does
+        # not pass --enable-debug this line is the only explanation of why the message in
+        # the dashboard is clipped.
+        logging.info(
+            f"commit_message truncated from {len(commit_message)} to "
+            f"{MAX_COMMIT_MESSAGE_LENGTH} characters to stay within API request size limits"
+        )
+        keep = MAX_COMMIT_MESSAGE_LENGTH - len(COMMIT_MESSAGE_TRUNCATION_MARKER)
+        return commit_message[:keep] + COMMIT_MESSAGE_TRUNCATION_MARKER
+    return commit_message
+
+
 def load_cli_config_file(config_path: str) -> dict:
     """
     Load CLI defaults from a JSON or TOML file.
@@ -201,7 +227,13 @@ class CliConfig:
     legal: bool = False
     legal_format: str = "socket"
     config_file: Optional[str] = None
-
+
+    def __post_init__(self):
+        # Capped on construction so that every source of commit_message -- the
+        # --commit-message flag, a config file, the git backfill in socketcli -- lands
+        # under the limit.
+        self.commit_message = truncate_commit_message(self.commit_message)
+
     @classmethod
     def from_args(cls, args_list: Optional[List[str]] = None) -> 'CliConfig':
         parser = create_argument_parser()
@@ -257,19 +289,6 @@ def from_args(cls, args_list: Optional[List[str]] = None) -> 'CliConfig':
         if commit_message and commit_message.startswith('"') and commit_message.endswith('"'):
             commit_message = commit_message[1:-1]
 
-        # Truncate to avoid 413s from oversized URL query parameters.
-        # The API has no application-layer length validation on commit_message;
-        # the 413 originates from an infrastructure-layer URL length limit
-        # (nginx/Cloudflare). 200 chars chosen as a conservative ceiling given
-        # URL encoding can 2-3x raw character count.
-        MAX_COMMIT_MESSAGE_LENGTH = 200
-        if commit_message and len(commit_message) > MAX_COMMIT_MESSAGE_LENGTH:
-            logging.debug(
-                f"commit_message truncated from {len(commit_message)} to "
-                f"{MAX_COMMIT_MESSAGE_LENGTH} characters to avoid API request size limits"
-            )
-            commit_message = commit_message[:MAX_COMMIT_MESSAGE_LENGTH]
-
         config_args = {
             'api_token': api_token,
             'repo': args.repo,

socketsecurity/core/__init__.py

@@ -113,6 +113,12 @@
 FULL_SCAN_UPLOAD_MAX_ATTEMPTS = len(FULL_SCAN_UPLOAD_BACKOFF_SCHEDULE_SECONDS)
 FULL_SCAN_UPLOAD_BACKOFF_JITTER_SECONDS = 2.0
 
+# Statuses that mean the request is too large to process. Scan metadata travels in the
+# query string of the full-scan POST, while manifests travel in its multipart body. A
+# 413 can refer to either part; 414 points to the URL, and some proxies report 431 when
+# the encoded request target exceeds their header limit. None are transient.
+REQUEST_TOO_LARGE_STATUS_CODES = (413, 414, 431)
+
 # Diff-scan polling policy. The legacy scan comparison (fullscans.stream_diff) holds a
 # single HTTP connection open, fully idle, while the backend computes the diff; network
 # middleboxes with TCP idle timeouts (notably Azure NAT gateways, which default to
@@ -1118,6 +1124,23 @@ def create_full_scan(self, files: List[str], params: FullScanParams, base_paths:
                     res = self.sdk.fullscans.post(upload_files, params, use_types=True, use_lazy_loading=True, max_open_files=50, base_paths=base_paths)
                     break
                 except APIFailure as error:
+                    if error.status_code in REQUEST_TOO_LARGE_STATUS_CODES:
+                        if error.status_code == 413:
+                            guidance = (
+                                "The response does not distinguish between an oversized multipart "
+                                "upload and oversized scan metadata in the request URL. Reduce the "
+                                "uploaded scan inputs, or pass a shorter --commit-message."
+                            )
+                        else:
+                            guidance = (
+                                "Scan metadata is sent in the request URL. Pass a shorter "
+                                "--commit-message or shorten other scan metadata."
+                            )
+                        raise APIFailure(
+                            f"Full scan request rejected as too large (HTTP {error.status_code}). "
+                            f"{guidance}\n{error}",
+                            status_code=error.status_code,
+                        ) from error
                     if backoff_seconds is None or not error.is_transient_error():
                         raise
                     wait_seconds = backoff_seconds + random.uniform(

socketsecurity/socketcli.py

@@ -12,7 +12,7 @@
 from socketdev import socketdev
 from socketdev.fullscans import FullScanParams
 
-from socketsecurity.config import CliConfig
+from socketsecurity.config import CliConfig, truncate_commit_message
 from socketsecurity.core import Core
 from socketsecurity.core.classes import Diff
 from socketsecurity.core.cli_client import CliClient
@@ -210,6 +210,35 @@ def create_scm_scan(
     return diff, False
 
 
+def apply_git_context(config: CliConfig) -> Tuple[bool, Optional[Git]]:
+    """
+    Fill in any repo details the caller did not pass from the checkout at target_path.
+
+    Returns whether target_path is a git repository, along with the Git handle when it is.
+    """
+    try:
+        git_repo = Git(config.target_path)
+    except InvalidGitRepositoryError:
+        log.debug("Not a git repository, setting ignore_commit_files=True")
+        config.ignore_commit_files = True
+        return False, None
+    except NoSuchPathError:
+        raise Exception(f"Unable to find path {config.target_path}")
+
+    if not config.repo:
+        config.repo = git_repo.repo_name
+    if not config.commit_sha:
+        config.commit_sha = git_repo.commit_str
+    if not config.branch:
+        config.branch = git_repo.branch
+    if not config.committers:
+        config.committers = [git_repo.get_formatted_committer()]
+    if not config.commit_message:
+        # A repository's commit message is unbounded and ships in the query string.
+        config.commit_message = truncate_commit_message(git_repo.commit_message)
+    return True, git_repo
+
+
 def build_socket_sdk(config: CliConfig) -> socketdev:
     cli_user_agent_string = f"SocketPythonCLI/{config.version}"
     return socketdev(
@@ -402,27 +431,7 @@ def main_code():
         discovered_scan_files = None
 
         # Git setup
-        is_repo = False
-        git_repo: Git
-        try:
-            git_repo = Git(config.target_path)
-            is_repo = True
-            if not config.repo:
-                config.repo = git_repo.repo_name
-            if not config.commit_sha:
-                config.commit_sha = git_repo.commit_str
-            if not config.branch:
-                config.branch = git_repo.branch
-            if not config.committers:
-                config.committers = [git_repo.get_formatted_committer()]
-            if not config.commit_message:
-                config.commit_message = git_repo.commit_message
-        except InvalidGitRepositoryError:
-            is_repo = False
-            log.debug("Not a git repository, setting ignore_commit_files=True")
-            config.ignore_commit_files = True
-        except NoSuchPathError:
-            raise Exception(f"Unable to find path {config.target_path}")
+        is_repo, git_repo = apply_git_context(config)
 
         # Track whether repo/branch fell back to the default sentinels so reachability can skip
         # forwarding them as coana cache-bucket keys (computed before any workspace suffixing).

tests/unit/test_cli_config.py

@@ -31,14 +31,15 @@ def test_truncated_above_limit(self):
         config = CliConfig.from_args(
             ["--api-token", "test", "--commit-message", "a" * 250]
         )
-        assert config.commit_message == "a" * 200
+        assert config.commit_message == "a" * 197 + "..."
+        assert len(config.commit_message) == 200
 
     def test_quote_strip_runs_before_truncation(self):
         quoted = '"' + ("b" * 250) + '"'
         config = CliConfig.from_args(
             ["--api-token", "test", "--commit-message", quoted]
         )
-        assert config.commit_message == "b" * 200
+        assert config.commit_message == "b" * 197 + "..."
 
 
 class TestCliConfig:

tests/unit/test_commit_message_truncation.py

@@ -0,0 +1,113 @@
+import subprocess
+
+import pytest
+
+from socketsecurity.config import (
+    COMMIT_MESSAGE_TRUNCATION_MARKER,
+    MAX_COMMIT_MESSAGE_LENGTH,
+    CliConfig,
+    truncate_commit_message,
+)
+from socketsecurity.socketcli import apply_git_context
+
+
+def _git(path, *args):
+    return subprocess.run(
+        ["git", *args],
+        cwd=path,
+        check=True,
+        capture_output=True,
+        text=True,
+    ).stdout.strip()
+
+
+@pytest.fixture
+def repo_with_large_commit_message(tmp_path):
+    """A checkout whose HEAD commit message is far larger than the cap (~14 KB)."""
+    path = tmp_path / "repo"
+    path.mkdir()
+    _git(path, "init", "-b", "main")
+    _git(path, "config", "user.name", "Socket Test")
+    _git(path, "config", "user.email", "socket@example.com")
+    (path / "package.json").write_text("{}\n", encoding="utf-8")
+    _git(path, "add", "package.json")
+    _git(path, "commit", "-m", "Release notes\n\n" + ("- bumped a dependency\n" * 700))
+    return path
+
+
+class TestTruncateCommitMessage:
+    def test_none_passes_through(self):
+        assert truncate_commit_message(None) is None
+
+    def test_empty_passes_through(self):
+        assert truncate_commit_message("") == ""
+
+    def test_under_limit_is_unchanged(self):
+        msg = "a normal short commit message"
+        assert truncate_commit_message(msg) == msg
+
+    def test_at_limit_is_unchanged(self):
+        msg = "a" * MAX_COMMIT_MESSAGE_LENGTH
+        assert truncate_commit_message(msg) == msg
+
+    def test_over_limit_is_capped(self):
+        capped = truncate_commit_message("a" * 14_000)
+        assert len(capped) == MAX_COMMIT_MESSAGE_LENGTH
+        assert capped.endswith(COMMIT_MESSAGE_TRUNCATION_MARKER)
+
+    def test_marker_fits_inside_the_limit(self):
+        # The marker replaces the tail rather than extending past it, so the capped value
+        # never grows the request line beyond what the proxy accepts.
+        assert truncate_commit_message("a" * 201) == (
+            "a" * (MAX_COMMIT_MESSAGE_LENGTH - len(COMMIT_MESSAGE_TRUNCATION_MARKER))
+            + COMMIT_MESSAGE_TRUNCATION_MARKER
+        )
+
+
+class TestCliConfigInvariant:
+    def test_direct_construction_is_capped(self):
+        config = CliConfig(api_token="test", repo="widgets", commit_message="a" * 14_000)
+        assert len(config.commit_message) == MAX_COMMIT_MESSAGE_LENGTH
+
+    def test_config_file_value_is_capped(self, tmp_path):
+        config_file = tmp_path / "socketcli.json"
+        config_file.write_text('{"commit_message": "%s"}' % ("a" * 14_000), encoding="utf-8")
+        config = CliConfig.from_args(["--api-token", "test", "--config", str(config_file)])
+        assert len(config.commit_message) == MAX_COMMIT_MESSAGE_LENGTH
+
+
+class TestGitBackfill:
+    def test_message_read_from_git_is_capped(self, repo_with_large_commit_message):
+        config = CliConfig(api_token="test", repo=None, target_path=str(repo_with_large_commit_message))
+        assert config.commit_message is None
+
+        is_repo, git_repo = apply_git_context(config)
+
+        assert is_repo is True
+        # The repository really does carry an oversized message; the cap is what keeps it
+        # out of the full-scan query string.
+        assert len(git_repo.commit_message) > 14_000
+        assert len(config.commit_message) == MAX_COMMIT_MESSAGE_LENGTH
+        assert config.commit_message.startswith("Release notes")
+        assert config.commit_message.endswith(COMMIT_MESSAGE_TRUNCATION_MARKER)
+
+    def test_explicit_message_is_not_overwritten_by_git(self, repo_with_large_commit_message):
+        config = CliConfig(
+            api_token="test",
+            repo=None,
+            target_path=str(repo_with_large_commit_message),
+            commit_message="explicit message",
+        )
+
+        apply_git_context(config)
+
+        assert config.commit_message == "explicit message"
+
+    def test_non_repo_path_reports_no_repo(self, tmp_path):
+        config = CliConfig(api_token="test", repo=None, target_path=str(tmp_path))
+
+        is_repo, git_repo = apply_git_context(config)
+
+        assert is_repo is False
+        assert git_repo is None
+        assert config.ignore_commit_files is True

tests/unit/test_full_scan_retry.py

@@ -283,3 +283,46 @@ def test_retry_decision_delegates_to_sdk_classification(
             core_with_mock_sdk.create_full_scan([str(manifest)], MagicMock())
 
     assert core_with_mock_sdk.sdk.fullscans.post.call_count == expected_calls
+
+
+@pytest.mark.parametrize("status_code", [414, 431])
+def test_oversized_request_target_is_not_retried_and_names_the_cause(
+    core_with_mock_sdk, tmp_path, no_sleep, status_code
+):
+    """
+    URI and header size failures are deterministic for the same request, and the SDK's
+    message does not say which metadata to shorten.
+    """
+    manifest = tmp_path / "package.json"
+    manifest.write_text("{}")
+    core_with_mock_sdk.sdk.fullscans.post.side_effect = _catch_all_failure(status_code)
+
+    with pytest.raises(APIFailure) as exc_info:
+        core_with_mock_sdk.create_full_scan([str(manifest)], MagicMock())
+
+    assert core_with_mock_sdk.sdk.fullscans.post.call_count == 1
+    no_sleep.assert_not_called()
+    message = str(exc_info.value)
+    assert f"rejected as too large (HTTP {status_code})" in message
+    assert "--commit-message" in message
+    # The SDK's original text is kept so the proxy's own response stays available.
+    assert f"original_status_code:{status_code}" in message
+    assert exc_info.value.status_code == status_code
+
+
+def test_413_reports_upload_and_metadata_causes(core_with_mock_sdk, tmp_path, no_sleep):
+    manifest = tmp_path / "package.json"
+    manifest.write_text("{}")
+    core_with_mock_sdk.sdk.fullscans.post.side_effect = _catch_all_failure(413)
+
+    with pytest.raises(APIFailure) as exc_info:
+        core_with_mock_sdk.create_full_scan([str(manifest)], MagicMock())
+
+    assert core_with_mock_sdk.sdk.fullscans.post.call_count == 1
+    no_sleep.assert_not_called()
+    message = str(exc_info.value)
+    assert "oversized multipart upload" in message
+    assert "oversized scan metadata" in message
+    assert "--commit-message" in message
+    assert "original_status_code:413" in message
+    assert exc_info.value.status_code == 413