Skip to content

Commit 325d551

Browse files
authored
Cap the commit message read from the local checkout (#367)
* 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. * 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. * 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. * Clarify ambiguous 413 scan failures
1 parent 7c95310 commit 325d551

10 files changed

Lines changed: 259 additions & 41 deletions

File tree

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,15 @@
11
# Changelog
22

3+
## 2.9.8
4+
5+
### Fixed: oversized commit messages no longer fail the scan
6+
7+
- The 200-character cap on the commit message now applies to the value read from the
8+
repository, not only to `--commit-message`. A truncated message ends in `...` and the
9+
truncation is reported at INFO.
10+
- A full scan refused for its size (HTTP 413, 414 or 431) now distinguishes possible
11+
upload-size and request-metadata causes and reports what to shorten.
12+
313
## 2.9.7
414

515
### Changed: bump pinned @coana-tech/cli to 15.10.51

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ build-backend = "hatchling.build"
66

77
[project]
88
name = "socketsecurity"
9-
version = "2.9.7"
9+
version = "2.9.8"
1010
requires-python = ">= 3.11"
1111
license = {"file" = "LICENSE"}
1212
dependencies = [

socketsecurity/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
__author__ = 'socket.dev'
2-
__version__ = '2.9.7'
2+
__version__ = '2.9.8'
33
USER_AGENT = f'SocketPythonCLI/{__version__}'

socketsecurity/config.py

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,32 @@ def get_plugin_config_from_env(prefix: str) -> dict:
1919
return {}
2020

2121

22+
# commit_message rides in the query string of POST /v0/orgs/{org}/full-scans, so an
23+
# oversized message overflows the edge proxy's request line limit before the API ever
24+
# sees it. The API itself has no length validation on the field; the rejection comes
25+
# from the proxy, which reports 413, 414 or 431 depending on which limit it checks. 200
26+
# chars is a conservative ceiling given URL encoding can 2-3x the raw character count.
27+
MAX_COMMIT_MESSAGE_LENGTH = 200
28+
29+
30+
COMMIT_MESSAGE_TRUNCATION_MARKER = "..."
31+
32+
33+
def truncate_commit_message(commit_message: Optional[str]) -> Optional[str]:
34+
"""Cap commit_message to a length the full-scan request line can carry."""
35+
if commit_message and len(commit_message) > MAX_COMMIT_MESSAGE_LENGTH:
36+
# INFO, not DEBUG: the scan keeps the truncated value, so for a CI job that does
37+
# not pass --enable-debug this line is the only explanation of why the message in
38+
# the dashboard is clipped.
39+
logging.info(
40+
f"commit_message truncated from {len(commit_message)} to "
41+
f"{MAX_COMMIT_MESSAGE_LENGTH} characters to stay within API request size limits"
42+
)
43+
keep = MAX_COMMIT_MESSAGE_LENGTH - len(COMMIT_MESSAGE_TRUNCATION_MARKER)
44+
return commit_message[:keep] + COMMIT_MESSAGE_TRUNCATION_MARKER
45+
return commit_message
46+
47+
2248
def load_cli_config_file(config_path: str) -> dict:
2349
"""
2450
Load CLI defaults from a JSON or TOML file.
@@ -201,7 +227,13 @@ class CliConfig:
201227
legal: bool = False
202228
legal_format: str = "socket"
203229
config_file: Optional[str] = None
204-
230+
231+
def __post_init__(self):
232+
# Capped on construction so that every source of commit_message -- the
233+
# --commit-message flag, a config file, the git backfill in socketcli -- lands
234+
# under the limit.
235+
self.commit_message = truncate_commit_message(self.commit_message)
236+
205237
@classmethod
206238
def from_args(cls, args_list: Optional[List[str]] = None) -> 'CliConfig':
207239
parser = create_argument_parser()
@@ -257,19 +289,6 @@ def from_args(cls, args_list: Optional[List[str]] = None) -> 'CliConfig':
257289
if commit_message and commit_message.startswith('"') and commit_message.endswith('"'):
258290
commit_message = commit_message[1:-1]
259291

260-
# Truncate to avoid 413s from oversized URL query parameters.
261-
# The API has no application-layer length validation on commit_message;
262-
# the 413 originates from an infrastructure-layer URL length limit
263-
# (nginx/Cloudflare). 200 chars chosen as a conservative ceiling given
264-
# URL encoding can 2-3x raw character count.
265-
MAX_COMMIT_MESSAGE_LENGTH = 200
266-
if commit_message and len(commit_message) > MAX_COMMIT_MESSAGE_LENGTH:
267-
logging.debug(
268-
f"commit_message truncated from {len(commit_message)} to "
269-
f"{MAX_COMMIT_MESSAGE_LENGTH} characters to avoid API request size limits"
270-
)
271-
commit_message = commit_message[:MAX_COMMIT_MESSAGE_LENGTH]
272-
273292
config_args = {
274293
'api_token': api_token,
275294
'repo': args.repo,

socketsecurity/core/__init__.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,12 @@
113113
FULL_SCAN_UPLOAD_MAX_ATTEMPTS = len(FULL_SCAN_UPLOAD_BACKOFF_SCHEDULE_SECONDS)
114114
FULL_SCAN_UPLOAD_BACKOFF_JITTER_SECONDS = 2.0
115115

116+
# Statuses that mean the request is too large to process. Scan metadata travels in the
117+
# query string of the full-scan POST, while manifests travel in its multipart body. A
118+
# 413 can refer to either part; 414 points to the URL, and some proxies report 431 when
119+
# the encoded request target exceeds their header limit. None are transient.
120+
REQUEST_TOO_LARGE_STATUS_CODES = (413, 414, 431)
121+
116122
# Diff-scan polling policy. The legacy scan comparison (fullscans.stream_diff) holds a
117123
# single HTTP connection open, fully idle, while the backend computes the diff; network
118124
# 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:
11181124
res = self.sdk.fullscans.post(upload_files, params, use_types=True, use_lazy_loading=True, max_open_files=50, base_paths=base_paths)
11191125
break
11201126
except APIFailure as error:
1127+
if error.status_code in REQUEST_TOO_LARGE_STATUS_CODES:
1128+
if error.status_code == 413:
1129+
guidance = (
1130+
"The response does not distinguish between an oversized multipart "
1131+
"upload and oversized scan metadata in the request URL. Reduce the "
1132+
"uploaded scan inputs, or pass a shorter --commit-message."
1133+
)
1134+
else:
1135+
guidance = (
1136+
"Scan metadata is sent in the request URL. Pass a shorter "
1137+
"--commit-message or shorten other scan metadata."
1138+
)
1139+
raise APIFailure(
1140+
f"Full scan request rejected as too large (HTTP {error.status_code}). "
1141+
f"{guidance}\n{error}",
1142+
status_code=error.status_code,
1143+
) from error
11211144
if backoff_seconds is None or not error.is_transient_error():
11221145
raise
11231146
wait_seconds = backoff_seconds + random.uniform(

socketsecurity/socketcli.py

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from socketdev import socketdev
1313
from socketdev.fullscans import FullScanParams
1414

15-
from socketsecurity.config import CliConfig
15+
from socketsecurity.config import CliConfig, truncate_commit_message
1616
from socketsecurity.core import Core
1717
from socketsecurity.core.classes import Diff
1818
from socketsecurity.core.cli_client import CliClient
@@ -210,6 +210,35 @@ def create_scm_scan(
210210
return diff, False
211211

212212

213+
def apply_git_context(config: CliConfig) -> Tuple[bool, Optional[Git]]:
214+
"""
215+
Fill in any repo details the caller did not pass from the checkout at target_path.
216+
217+
Returns whether target_path is a git repository, along with the Git handle when it is.
218+
"""
219+
try:
220+
git_repo = Git(config.target_path)
221+
except InvalidGitRepositoryError:
222+
log.debug("Not a git repository, setting ignore_commit_files=True")
223+
config.ignore_commit_files = True
224+
return False, None
225+
except NoSuchPathError:
226+
raise Exception(f"Unable to find path {config.target_path}")
227+
228+
if not config.repo:
229+
config.repo = git_repo.repo_name
230+
if not config.commit_sha:
231+
config.commit_sha = git_repo.commit_str
232+
if not config.branch:
233+
config.branch = git_repo.branch
234+
if not config.committers:
235+
config.committers = [git_repo.get_formatted_committer()]
236+
if not config.commit_message:
237+
# A repository's commit message is unbounded and ships in the query string.
238+
config.commit_message = truncate_commit_message(git_repo.commit_message)
239+
return True, git_repo
240+
241+
213242
def build_socket_sdk(config: CliConfig) -> socketdev:
214243
cli_user_agent_string = f"SocketPythonCLI/{config.version}"
215244
return socketdev(
@@ -402,27 +431,7 @@ def main_code():
402431
discovered_scan_files = None
403432

404433
# Git setup
405-
is_repo = False
406-
git_repo: Git
407-
try:
408-
git_repo = Git(config.target_path)
409-
is_repo = True
410-
if not config.repo:
411-
config.repo = git_repo.repo_name
412-
if not config.commit_sha:
413-
config.commit_sha = git_repo.commit_str
414-
if not config.branch:
415-
config.branch = git_repo.branch
416-
if not config.committers:
417-
config.committers = [git_repo.get_formatted_committer()]
418-
if not config.commit_message:
419-
config.commit_message = git_repo.commit_message
420-
except InvalidGitRepositoryError:
421-
is_repo = False
422-
log.debug("Not a git repository, setting ignore_commit_files=True")
423-
config.ignore_commit_files = True
424-
except NoSuchPathError:
425-
raise Exception(f"Unable to find path {config.target_path}")
434+
is_repo, git_repo = apply_git_context(config)
426435

427436
# Track whether repo/branch fell back to the default sentinels so reachability can skip
428437
# forwarding them as coana cache-bucket keys (computed before any workspace suffixing).

tests/unit/test_cli_config.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,15 @@ def test_truncated_above_limit(self):
3131
config = CliConfig.from_args(
3232
["--api-token", "test", "--commit-message", "a" * 250]
3333
)
34-
assert config.commit_message == "a" * 200
34+
assert config.commit_message == "a" * 197 + "..."
35+
assert len(config.commit_message) == 200
3536

3637
def test_quote_strip_runs_before_truncation(self):
3738
quoted = '"' + ("b" * 250) + '"'
3839
config = CliConfig.from_args(
3940
["--api-token", "test", "--commit-message", quoted]
4041
)
41-
assert config.commit_message == "b" * 200
42+
assert config.commit_message == "b" * 197 + "..."
4243

4344

4445
class TestCliConfig:
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
import subprocess
2+
3+
import pytest
4+
5+
from socketsecurity.config import (
6+
COMMIT_MESSAGE_TRUNCATION_MARKER,
7+
MAX_COMMIT_MESSAGE_LENGTH,
8+
CliConfig,
9+
truncate_commit_message,
10+
)
11+
from socketsecurity.socketcli import apply_git_context
12+
13+
14+
def _git(path, *args):
15+
return subprocess.run(
16+
["git", *args],
17+
cwd=path,
18+
check=True,
19+
capture_output=True,
20+
text=True,
21+
).stdout.strip()
22+
23+
24+
@pytest.fixture
25+
def repo_with_large_commit_message(tmp_path):
26+
"""A checkout whose HEAD commit message is far larger than the cap (~14 KB)."""
27+
path = tmp_path / "repo"
28+
path.mkdir()
29+
_git(path, "init", "-b", "main")
30+
_git(path, "config", "user.name", "Socket Test")
31+
_git(path, "config", "user.email", "socket@example.com")
32+
(path / "package.json").write_text("{}\n", encoding="utf-8")
33+
_git(path, "add", "package.json")
34+
_git(path, "commit", "-m", "Release notes\n\n" + ("- bumped a dependency\n" * 700))
35+
return path
36+
37+
38+
class TestTruncateCommitMessage:
39+
def test_none_passes_through(self):
40+
assert truncate_commit_message(None) is None
41+
42+
def test_empty_passes_through(self):
43+
assert truncate_commit_message("") == ""
44+
45+
def test_under_limit_is_unchanged(self):
46+
msg = "a normal short commit message"
47+
assert truncate_commit_message(msg) == msg
48+
49+
def test_at_limit_is_unchanged(self):
50+
msg = "a" * MAX_COMMIT_MESSAGE_LENGTH
51+
assert truncate_commit_message(msg) == msg
52+
53+
def test_over_limit_is_capped(self):
54+
capped = truncate_commit_message("a" * 14_000)
55+
assert len(capped) == MAX_COMMIT_MESSAGE_LENGTH
56+
assert capped.endswith(COMMIT_MESSAGE_TRUNCATION_MARKER)
57+
58+
def test_marker_fits_inside_the_limit(self):
59+
# The marker replaces the tail rather than extending past it, so the capped value
60+
# never grows the request line beyond what the proxy accepts.
61+
assert truncate_commit_message("a" * 201) == (
62+
"a" * (MAX_COMMIT_MESSAGE_LENGTH - len(COMMIT_MESSAGE_TRUNCATION_MARKER))
63+
+ COMMIT_MESSAGE_TRUNCATION_MARKER
64+
)
65+
66+
67+
class TestCliConfigInvariant:
68+
def test_direct_construction_is_capped(self):
69+
config = CliConfig(api_token="test", repo="widgets", commit_message="a" * 14_000)
70+
assert len(config.commit_message) == MAX_COMMIT_MESSAGE_LENGTH
71+
72+
def test_config_file_value_is_capped(self, tmp_path):
73+
config_file = tmp_path / "socketcli.json"
74+
config_file.write_text('{"commit_message": "%s"}' % ("a" * 14_000), encoding="utf-8")
75+
config = CliConfig.from_args(["--api-token", "test", "--config", str(config_file)])
76+
assert len(config.commit_message) == MAX_COMMIT_MESSAGE_LENGTH
77+
78+
79+
class TestGitBackfill:
80+
def test_message_read_from_git_is_capped(self, repo_with_large_commit_message):
81+
config = CliConfig(api_token="test", repo=None, target_path=str(repo_with_large_commit_message))
82+
assert config.commit_message is None
83+
84+
is_repo, git_repo = apply_git_context(config)
85+
86+
assert is_repo is True
87+
# The repository really does carry an oversized message; the cap is what keeps it
88+
# out of the full-scan query string.
89+
assert len(git_repo.commit_message) > 14_000
90+
assert len(config.commit_message) == MAX_COMMIT_MESSAGE_LENGTH
91+
assert config.commit_message.startswith("Release notes")
92+
assert config.commit_message.endswith(COMMIT_MESSAGE_TRUNCATION_MARKER)
93+
94+
def test_explicit_message_is_not_overwritten_by_git(self, repo_with_large_commit_message):
95+
config = CliConfig(
96+
api_token="test",
97+
repo=None,
98+
target_path=str(repo_with_large_commit_message),
99+
commit_message="explicit message",
100+
)
101+
102+
apply_git_context(config)
103+
104+
assert config.commit_message == "explicit message"
105+
106+
def test_non_repo_path_reports_no_repo(self, tmp_path):
107+
config = CliConfig(api_token="test", repo=None, target_path=str(tmp_path))
108+
109+
is_repo, git_repo = apply_git_context(config)
110+
111+
assert is_repo is False
112+
assert git_repo is None
113+
assert config.ignore_commit_files is True

tests/unit/test_full_scan_retry.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,3 +283,46 @@ def test_retry_decision_delegates_to_sdk_classification(
283283
core_with_mock_sdk.create_full_scan([str(manifest)], MagicMock())
284284

285285
assert core_with_mock_sdk.sdk.fullscans.post.call_count == expected_calls
286+
287+
288+
@pytest.mark.parametrize("status_code", [414, 431])
289+
def test_oversized_request_target_is_not_retried_and_names_the_cause(
290+
core_with_mock_sdk, tmp_path, no_sleep, status_code
291+
):
292+
"""
293+
URI and header size failures are deterministic for the same request, and the SDK's
294+
message does not say which metadata to shorten.
295+
"""
296+
manifest = tmp_path / "package.json"
297+
manifest.write_text("{}")
298+
core_with_mock_sdk.sdk.fullscans.post.side_effect = _catch_all_failure(status_code)
299+
300+
with pytest.raises(APIFailure) as exc_info:
301+
core_with_mock_sdk.create_full_scan([str(manifest)], MagicMock())
302+
303+
assert core_with_mock_sdk.sdk.fullscans.post.call_count == 1
304+
no_sleep.assert_not_called()
305+
message = str(exc_info.value)
306+
assert f"rejected as too large (HTTP {status_code})" in message
307+
assert "--commit-message" in message
308+
# The SDK's original text is kept so the proxy's own response stays available.
309+
assert f"original_status_code:{status_code}" in message
310+
assert exc_info.value.status_code == status_code
311+
312+
313+
def test_413_reports_upload_and_metadata_causes(core_with_mock_sdk, tmp_path, no_sleep):
314+
manifest = tmp_path / "package.json"
315+
manifest.write_text("{}")
316+
core_with_mock_sdk.sdk.fullscans.post.side_effect = _catch_all_failure(413)
317+
318+
with pytest.raises(APIFailure) as exc_info:
319+
core_with_mock_sdk.create_full_scan([str(manifest)], MagicMock())
320+
321+
assert core_with_mock_sdk.sdk.fullscans.post.call_count == 1
322+
no_sleep.assert_not_called()
323+
message = str(exc_info.value)
324+
assert "oversized multipart upload" in message
325+
assert "oversized scan metadata" in message
326+
assert "--commit-message" in message
327+
assert "original_status_code:413" in message
328+
assert exc_info.value.status_code == 413

uv.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)