From 30ff2c5696c4efd8ab3589579001aea6d5afa877 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Tue, 22 Sep 2026 12:19:14 -0400 Subject: [PATCH 1/4] Cap the commit message read from the local checkout commit_message travels in the query string of the full scan request, so an oversized value overflows the edge proxy's request line limit and the scan fails before reaching the API. The 200-character cap already covered --commit-message, but a run that omitted the flag backfilled the value straight from the checkout's HEAD commit, uncapped, so repositories whose commit messages carry generated release notes could not be scanned at all. Make the cap an invariant of the parsed configuration rather than a step in flag parsing, and apply it to the git-derived value as well. The truncation helper and its limit move to module scope so both sites share one definition. Extract the git setup block out of main_code into apply_git_context so the backfill is reachable from a test. Behavior is unchanged: the same fields are filled in the same order, and a path that is not a repository still sets ignore_commit_files. Note that the API has no length validation on the field. The rejection comes from the proxy in front of it, which reports 413 or 431 depending on which layer answers; the comment now covers both rather than naming one. --- CHANGELOG.md | 14 +++ pyproject.toml | 2 +- socketsecurity/__init__.py | 2 +- socketsecurity/config.py | 40 +++++--- socketsecurity/socketcli.py | 54 ++++++---- tests/unit/test_commit_message_truncation.py | 101 +++++++++++++++++++ uv.lock | 2 +- 7 files changed, 176 insertions(+), 39 deletions(-) create mode 100644 tests/unit/test_commit_message_truncation.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f49dd4..513a019 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ # Changelog +## 2.9.7 + +### Fixed: cap the commit message read from the local checkout + +- `commit_message` travels in the query string of the full scan request, so an + oversized value overflows the edge proxy's request line limit and the scan + fails with HTTP 431 before reaching the API. The 200-character cap already + applied to `--commit-message`, but a run that omitted the flag backfilled the + value straight from the checkout's HEAD commit, uncapped. Repositories whose + commit messages carry generated release notes could not be scanned at all. +- The cap is now an invariant of the parsed configuration and is applied to the + git-derived value as well, so every source of `commit_message` lands under the + limit. + ## 2.9.6 ### Changed: bump pinned @coana-tech/cli to 15.10.48 diff --git a/pyproject.toml b/pyproject.toml index 832c62d..7c40375 100644 --- a/pyproject.toml +++ b/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 = [ diff --git a/socketsecurity/__init__.py b/socketsecurity/__init__.py index c0fdaa0..de72965 100644 --- a/socketsecurity/__init__.py +++ b/socketsecurity/__init__.py @@ -1,3 +1,3 @@ __author__ = 'socket.dev' -__version__ = '2.9.6' +__version__ = '2.9.7' USER_AGENT = f'SocketPythonCLI/{__version__}' diff --git a/socketsecurity/config.py b/socketsecurity/config.py index bb2ad00..30a1cea 100644 --- a/socketsecurity/config.py +++ b/socketsecurity/config.py @@ -19,6 +19,25 @@ 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, reported as either 413 or 431 depending on which one answers. 200 +# chars is a conservative ceiling given URL encoding can 2-3x the raw character count. +MAX_COMMIT_MESSAGE_LENGTH = 200 + + +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: + logging.debug( + f"commit_message truncated from {len(commit_message)} to " + f"{MAX_COMMIT_MESSAGE_LENGTH} characters to avoid API request size limits" + ) + return commit_message[:MAX_COMMIT_MESSAGE_LENGTH] + return commit_message + + def load_cli_config_file(config_path: str) -> dict: """ Load CLI defaults from a JSON or TOML file. @@ -201,7 +220,13 @@ class CliConfig: legal: bool = False legal_format: str = "socket" config_file: Optional[str] = None - + + def __post_init__(self): + # Capped here rather than at the flag-parsing site 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 +282,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, diff --git a/socketsecurity/socketcli.py b/socketsecurity/socketcli.py index f585d99..2ac5b9a 100644 --- a/socketsecurity/socketcli.py +++ b/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,36 @@ 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: + # Capped like the flag-supplied value: a repository's own commit message is + # unbounded, and it ships in the full-scan 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 +432,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). diff --git a/tests/unit/test_commit_message_truncation.py b/tests/unit/test_commit_message_truncation.py new file mode 100644 index 0000000..0737b56 --- /dev/null +++ b/tests/unit/test_commit_message_truncation.py @@ -0,0 +1,101 @@ +import subprocess + +import pytest + +from socketsecurity.config import ( + 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): + assert truncate_commit_message("a" * 14_000) == "a" * MAX_COMMIT_MESSAGE_LENGTH + + +class TestCliConfigInvariant: + def test_direct_construction_is_capped(self): + config = CliConfig(api_token="test", repo="widgets", commit_message="a" * 14_000) + assert config.commit_message == "a" * 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 config.commit_message == "a" * 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 == git_repo.commit_message[:MAX_COMMIT_MESSAGE_LENGTH] + + 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 diff --git a/uv.lock b/uv.lock index c63e4ae..dd350a1 100644 --- a/uv.lock +++ b/uv.lock @@ -1293,7 +1293,7 @@ wheels = [ [[package]] name = "socketsecurity" -version = "2.9.6" +version = "2.9.7" source = { editable = "." } dependencies = [ { name = "beautifulsoup4" }, From 392935dd0badde0ed8f6db61aa7627afc472e976 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Tue, 22 Sep 2026 12:30:42 -0400 Subject: [PATCH 2/4] Make truncation visible and name the cause when a request is refused for size Two follow-on safeguards for the same failure, both aimed at CI runs where no one is watching a terminal. Truncation was silent: the notice sat at DEBUG, which a pipeline that does not pass --enable-debug never prints, and the stored value gave no sign it had been clipped. The notice moves to INFO and the value now ends in "...". The 200-character ceiling is unchanged -- the marker replaces the tail rather than extending past it -- so the request line is no larger than before. A request line the proxy refuses comes back as 413, 414 or 431 depending on which limit it checks, carrying the proxy's own response body and nothing about what to change. Those statuses now raise with the cause and the flag to change named, keeping the SDK's original text underneath. None of them were retried before and none are now: the same oversized URL would go back out. Any oversized query parameter is covered, not only the commit message. Buildkite already gets the section markers and the soft_fail hint from _emit_infrastructure_error, which this error reaches like any other API failure, so nothing platform-specific is added here. --- CHANGELOG.md | 12 +++++++++ socketsecurity/config.py | 13 +++++++--- socketsecurity/core/__init__.py | 16 ++++++++++++ tests/unit/test_cli_config.py | 5 ++-- tests/unit/test_commit_message_truncation.py | 20 ++++++++++++--- tests/unit/test_full_scan_retry.py | 27 ++++++++++++++++++++ 6 files changed, 84 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 513a019..e6a7d76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,18 @@ - The cap is now an invariant of the parsed configuration and is applied to the git-derived value as well, so every source of `commit_message` lands under the limit. +- A truncated message now ends in `...` and the notice is logged at INFO instead + of DEBUG, so a clipped message in the dashboard is explained by the CI log of + the run that produced it. The 200-character ceiling is unchanged; the marker + replaces the tail rather than extending past it. + +### Changed: name the cause when a full scan request is refused for its size + +- Scan metadata travels in the query string of the full scan request, so an + oversized value is refused by the proxy in front of the API, which reports 413, + 414 or 431 depending on which limit it checks. Those responses previously + surfaced as the SDK's generic status-code error carrying the proxy's response + body. They now name the cause and the flag to change, and remain unretried. ## 2.9.6 diff --git a/socketsecurity/config.py b/socketsecurity/config.py index 30a1cea..7467ec4 100644 --- a/socketsecurity/config.py +++ b/socketsecurity/config.py @@ -27,14 +27,21 @@ def get_plugin_config_from_env(prefix: str) -> dict: 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: - logging.debug( + # Logged at INFO rather than DEBUG: the scan record keeps the truncated value, and + # a CI job that never passes --enable-debug would otherwise have no way to tell 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 avoid API request size limits" + f"{MAX_COMMIT_MESSAGE_LENGTH} characters to stay within API request size limits" ) - return commit_message[:MAX_COMMIT_MESSAGE_LENGTH] + keep = MAX_COMMIT_MESSAGE_LENGTH - len(COMMIT_MESSAGE_TRUNCATION_MARKER) + return commit_message[:keep] + COMMIT_MESSAGE_TRUNCATION_MARKER return commit_message diff --git a/socketsecurity/core/__init__.py b/socketsecurity/core/__init__.py index fe49bce..3899ada 100644 --- a/socketsecurity/core/__init__.py +++ b/socketsecurity/core/__init__.py @@ -113,6 +113,13 @@ 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 line itself was rejected before the API read it: the +# scan metadata (commit message, branch, committers) travels in the query string of the +# full-scan POST, so an oversized value is refused by the proxy in front of the API. The +# proxy picks the code -- 413 (payload), 414 (URI), 431 (headers) -- so all three map to +# the same cause. Not transient: every retry sends the same oversized URL. +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 +1125,15 @@ 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: + raise APIFailure( + f"Full scan request rejected as too large (HTTP {error.status_code}). " + "Scan metadata is sent in the request URL, so an oversized value -- " + "most often the commit message -- is refused by the proxy in front of " + "the API before the request is read. Pass a shorter --commit-message " + f"to work around it.\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( diff --git a/tests/unit/test_cli_config.py b/tests/unit/test_cli_config.py index f70cda2..636ee9d 100644 --- a/tests/unit/test_cli_config.py +++ b/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: diff --git a/tests/unit/test_commit_message_truncation.py b/tests/unit/test_commit_message_truncation.py index 0737b56..46d7bde 100644 --- a/tests/unit/test_commit_message_truncation.py +++ b/tests/unit/test_commit_message_truncation.py @@ -3,6 +3,7 @@ import pytest from socketsecurity.config import ( + COMMIT_MESSAGE_TRUNCATION_MARKER, MAX_COMMIT_MESSAGE_LENGTH, CliConfig, truncate_commit_message, @@ -50,19 +51,29 @@ def test_at_limit_is_unchanged(self): assert truncate_commit_message(msg) == msg def test_over_limit_is_capped(self): - assert truncate_commit_message("a" * 14_000) == "a" * MAX_COMMIT_MESSAGE_LENGTH + 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 config.commit_message == "a" * MAX_COMMIT_MESSAGE_LENGTH + 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 config.commit_message == "a" * MAX_COMMIT_MESSAGE_LENGTH + assert len(config.commit_message) == MAX_COMMIT_MESSAGE_LENGTH class TestGitBackfill: @@ -77,7 +88,8 @@ def test_message_read_from_git_is_capped(self, repo_with_large_commit_message): # 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 == git_repo.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( diff --git a/tests/unit/test_full_scan_retry.py b/tests/unit/test_full_scan_retry.py index b31bb11..c629dd7 100644 --- a/tests/unit/test_full_scan_retry.py +++ b/tests/unit/test_full_scan_retry.py @@ -283,3 +283,30 @@ 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", [413, 414, 431]) +def test_oversized_request_is_not_retried_and_names_the_cause( + core_with_mock_sdk, tmp_path, no_sleep, status_code +): + """ + A proxy that refuses the request line reports 413, 414 or 431 depending on which limit + it checks. None of them are worth a retry (the same oversized URL goes back out), and + the SDK's own message is a status code plus the proxy's response body, which does not + say what to change. + """ + 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 From 800836ca24e3dbca5a9ad2c9ffd2ce670b5ae094 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Tue, 22 Sep 2026 12:39:50 -0400 Subject: [PATCH 3/4] Trim the changelog entry and the comments it duplicated Cut the 2.9.7 section to two bullets: what a user of a patch release needs is the behavior they will see, not the mechanism behind it. Reword the comments the entry was echoing so each states a present-tense invariant, and name the same three statuses in both the cap's rationale and the upload path rather than two overlapping subsets. --- CHANGELOG.md | 30 +++++++----------------------- socketsecurity/config.py | 14 +++++++------- socketsecurity/socketcli.py | 3 +-- 3 files changed, 15 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6a7d76..5a907e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,29 +2,13 @@ ## 2.9.7 -### Fixed: cap the commit message read from the local checkout - -- `commit_message` travels in the query string of the full scan request, so an - oversized value overflows the edge proxy's request line limit and the scan - fails with HTTP 431 before reaching the API. The 200-character cap already - applied to `--commit-message`, but a run that omitted the flag backfilled the - value straight from the checkout's HEAD commit, uncapped. Repositories whose - commit messages carry generated release notes could not be scanned at all. -- The cap is now an invariant of the parsed configuration and is applied to the - git-derived value as well, so every source of `commit_message` lands under the - limit. -- A truncated message now ends in `...` and the notice is logged at INFO instead - of DEBUG, so a clipped message in the dashboard is explained by the CI log of - the run that produced it. The 200-character ceiling is unchanged; the marker - replaces the tail rather than extending past it. - -### Changed: name the cause when a full scan request is refused for its size - -- Scan metadata travels in the query string of the full scan request, so an - oversized value is refused by the proxy in front of the API, which reports 413, - 414 or 431 depending on which limit it checks. Those responses previously - surfaced as the SDK's generic status-code error carrying the proxy's response - body. They now name the cause and the flag to change, and remain unretried. +### 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 reports which value to + shorten. ## 2.9.6 diff --git a/socketsecurity/config.py b/socketsecurity/config.py index 7467ec4..d497f8e 100644 --- a/socketsecurity/config.py +++ b/socketsecurity/config.py @@ -22,7 +22,7 @@ def get_plugin_config_from_env(prefix: str) -> dict: # 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, reported as either 413 or 431 depending on which one answers. 200 +# 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 @@ -33,9 +33,9 @@ def get_plugin_config_from_env(prefix: str) -> dict: 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: - # Logged at INFO rather than DEBUG: the scan record keeps the truncated value, and - # a CI job that never passes --enable-debug would otherwise have no way to tell why - # the message in the dashboard is clipped. + # 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" @@ -229,9 +229,9 @@ class CliConfig: config_file: Optional[str] = None def __post_init__(self): - # Capped here rather than at the flag-parsing site so that every source of - # commit_message (the --commit-message flag, a config file, the git backfill in - # socketcli) lands under the limit. + # 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 diff --git a/socketsecurity/socketcli.py b/socketsecurity/socketcli.py index 2ac5b9a..2be6850 100644 --- a/socketsecurity/socketcli.py +++ b/socketsecurity/socketcli.py @@ -234,8 +234,7 @@ def apply_git_context(config: CliConfig) -> Tuple[bool, Optional[Git]]: if not config.committers: config.committers = [git_repo.get_formatted_committer()] if not config.commit_message: - # Capped like the flag-supplied value: a repository's own commit message is - # unbounded, and it ships in the full-scan query string. + # 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 From 80a3c9619e59e7b73adbc75060d203b661fabdc8 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Tue, 22 Sep 2026 12:44:56 -0400 Subject: [PATCH 4/4] Clarify ambiguous 413 scan failures --- CHANGELOG.md | 4 ++-- socketsecurity/core/__init__.py | 25 ++++++++++++++++--------- tests/unit/test_full_scan_retry.py | 28 ++++++++++++++++++++++------ 3 files changed, 40 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a907e1..89d4068 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,8 @@ - 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 reports which value to - shorten. +- 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 diff --git a/socketsecurity/core/__init__.py b/socketsecurity/core/__init__.py index 3899ada..811bf65 100644 --- a/socketsecurity/core/__init__.py +++ b/socketsecurity/core/__init__.py @@ -113,11 +113,10 @@ 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 line itself was rejected before the API read it: the -# scan metadata (commit message, branch, committers) travels in the query string of the -# full-scan POST, so an oversized value is refused by the proxy in front of the API. The -# proxy picks the code -- 413 (payload), 414 (URI), 431 (headers) -- so all three map to -# the same cause. Not transient: every retry sends the same oversized URL. +# 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 @@ -1126,12 +1125,20 @@ def create_full_scan(self, files: List[str], params: FullScanParams, 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}). " - "Scan metadata is sent in the request URL, so an oversized value -- " - "most often the commit message -- is refused by the proxy in front of " - "the API before the request is read. Pass a shorter --commit-message " - f"to work around it.\n{error}", + f"{guidance}\n{error}", status_code=error.status_code, ) from error if backoff_seconds is None or not error.is_transient_error(): diff --git a/tests/unit/test_full_scan_retry.py b/tests/unit/test_full_scan_retry.py index c629dd7..2485a7a 100644 --- a/tests/unit/test_full_scan_retry.py +++ b/tests/unit/test_full_scan_retry.py @@ -285,15 +285,13 @@ def test_retry_decision_delegates_to_sdk_classification( assert core_with_mock_sdk.sdk.fullscans.post.call_count == expected_calls -@pytest.mark.parametrize("status_code", [413, 414, 431]) -def test_oversized_request_is_not_retried_and_names_the_cause( +@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 ): """ - A proxy that refuses the request line reports 413, 414 or 431 depending on which limit - it checks. None of them are worth a retry (the same oversized URL goes back out), and - the SDK's own message is a status code plus the proxy's response body, which does not - say what to change. + 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("{}") @@ -310,3 +308,21 @@ def test_oversized_request_is_not_retried_and_names_the_cause( # 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