Skip to content

Commit 392935d

Browse files
committed
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.
1 parent 30ff2c5 commit 392935d

6 files changed

Lines changed: 84 additions & 9 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,18 @@
1313
- The cap is now an invariant of the parsed configuration and is applied to the
1414
git-derived value as well, so every source of `commit_message` lands under the
1515
limit.
16+
- A truncated message now ends in `...` and the notice is logged at INFO instead
17+
of DEBUG, so a clipped message in the dashboard is explained by the CI log of
18+
the run that produced it. The 200-character ceiling is unchanged; the marker
19+
replaces the tail rather than extending past it.
20+
21+
### Changed: name the cause when a full scan request is refused for its size
22+
23+
- Scan metadata travels in the query string of the full scan request, so an
24+
oversized value is refused by the proxy in front of the API, which reports 413,
25+
414 or 431 depending on which limit it checks. Those responses previously
26+
surfaced as the SDK's generic status-code error carrying the proxy's response
27+
body. They now name the cause and the flag to change, and remain unretried.
1628

1729
## 2.9.6
1830

‎socketsecurity/config.py‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,21 @@ def get_plugin_config_from_env(prefix: str) -> dict:
2727
MAX_COMMIT_MESSAGE_LENGTH = 200
2828

2929

30+
COMMIT_MESSAGE_TRUNCATION_MARKER = "..."
31+
32+
3033
def truncate_commit_message(commit_message: Optional[str]) -> Optional[str]:
3134
"""Cap commit_message to a length the full-scan request line can carry."""
3235
if commit_message and len(commit_message) > MAX_COMMIT_MESSAGE_LENGTH:
33-
logging.debug(
36+
# Logged at INFO rather than DEBUG: the scan record keeps the truncated value, and
37+
# a CI job that never passes --enable-debug would otherwise have no way to tell why
38+
# the message in the dashboard is clipped.
39+
logging.info(
3440
f"commit_message truncated from {len(commit_message)} to "
35-
f"{MAX_COMMIT_MESSAGE_LENGTH} characters to avoid API request size limits"
41+
f"{MAX_COMMIT_MESSAGE_LENGTH} characters to stay within API request size limits"
3642
)
37-
return commit_message[:MAX_COMMIT_MESSAGE_LENGTH]
43+
keep = MAX_COMMIT_MESSAGE_LENGTH - len(COMMIT_MESSAGE_TRUNCATION_MARKER)
44+
return commit_message[:keep] + COMMIT_MESSAGE_TRUNCATION_MARKER
3845
return commit_message
3946

4047

‎socketsecurity/core/__init__.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,13 @@
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 line itself was rejected before the API read it: the
117+
# scan metadata (commit message, branch, committers) travels in the query string of the
118+
# full-scan POST, so an oversized value is refused by the proxy in front of the API. The
119+
# proxy picks the code -- 413 (payload), 414 (URI), 431 (headers) -- so all three map to
120+
# the same cause. Not transient: every retry sends the same oversized URL.
121+
REQUEST_TOO_LARGE_STATUS_CODES = (413, 414, 431)
122+
116123
# Diff-scan polling policy. The legacy scan comparison (fullscans.stream_diff) holds a
117124
# single HTTP connection open, fully idle, while the backend computes the diff; network
118125
# 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:
11181125
res = self.sdk.fullscans.post(upload_files, params, use_types=True, use_lazy_loading=True, max_open_files=50, base_paths=base_paths)
11191126
break
11201127
except APIFailure as error:
1128+
if error.status_code in REQUEST_TOO_LARGE_STATUS_CODES:
1129+
raise APIFailure(
1130+
f"Full scan request rejected as too large (HTTP {error.status_code}). "
1131+
"Scan metadata is sent in the request URL, so an oversized value -- "
1132+
"most often the commit message -- is refused by the proxy in front of "
1133+
"the API before the request is read. Pass a shorter --commit-message "
1134+
f"to work around it.\n{error}",
1135+
status_code=error.status_code,
1136+
) from error
11211137
if backoff_seconds is None or not error.is_transient_error():
11221138
raise
11231139
wait_seconds = backoff_seconds + random.uniform(

‎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:

‎tests/unit/test_commit_message_truncation.py‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import pytest
44

55
from socketsecurity.config import (
6+
COMMIT_MESSAGE_TRUNCATION_MARKER,
67
MAX_COMMIT_MESSAGE_LENGTH,
78
CliConfig,
89
truncate_commit_message,
@@ -50,19 +51,29 @@ def test_at_limit_is_unchanged(self):
5051
assert truncate_commit_message(msg) == msg
5152

5253
def test_over_limit_is_capped(self):
53-
assert truncate_commit_message("a" * 14_000) == "a" * MAX_COMMIT_MESSAGE_LENGTH
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+
)
5465

5566

5667
class TestCliConfigInvariant:
5768
def test_direct_construction_is_capped(self):
5869
config = CliConfig(api_token="test", repo="widgets", commit_message="a" * 14_000)
59-
assert config.commit_message == "a" * MAX_COMMIT_MESSAGE_LENGTH
70+
assert len(config.commit_message) == MAX_COMMIT_MESSAGE_LENGTH
6071

6172
def test_config_file_value_is_capped(self, tmp_path):
6273
config_file = tmp_path / "socketcli.json"
6374
config_file.write_text('{"commit_message": "%s"}' % ("a" * 14_000), encoding="utf-8")
6475
config = CliConfig.from_args(["--api-token", "test", "--config", str(config_file)])
65-
assert config.commit_message == "a" * MAX_COMMIT_MESSAGE_LENGTH
76+
assert len(config.commit_message) == MAX_COMMIT_MESSAGE_LENGTH
6677

6778

6879
class TestGitBackfill:
@@ -77,7 +88,8 @@ def test_message_read_from_git_is_capped(self, repo_with_large_commit_message):
7788
# out of the full-scan query string.
7889
assert len(git_repo.commit_message) > 14_000
7990
assert len(config.commit_message) == MAX_COMMIT_MESSAGE_LENGTH
80-
assert config.commit_message == git_repo.commit_message[:MAX_COMMIT_MESSAGE_LENGTH]
91+
assert config.commit_message.startswith("Release notes")
92+
assert config.commit_message.endswith(COMMIT_MESSAGE_TRUNCATION_MARKER)
8193

8294
def test_explicit_message_is_not_overwritten_by_git(self, repo_with_large_commit_message):
8395
config = CliConfig(

‎tests/unit/test_full_scan_retry.py‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,3 +283,30 @@ 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", [413, 414, 431])
289+
def test_oversized_request_is_not_retried_and_names_the_cause(
290+
core_with_mock_sdk, tmp_path, no_sleep, status_code
291+
):
292+
"""
293+
A proxy that refuses the request line reports 413, 414 or 431 depending on which limit
294+
it checks. None of them are worth a retry (the same oversized URL goes back out), and
295+
the SDK's own message is a status code plus the proxy's response body, which does not
296+
say what to change.
297+
"""
298+
manifest = tmp_path / "package.json"
299+
manifest.write_text("{}")
300+
core_with_mock_sdk.sdk.fullscans.post.side_effect = _catch_all_failure(status_code)
301+
302+
with pytest.raises(APIFailure) as exc_info:
303+
core_with_mock_sdk.create_full_scan([str(manifest)], MagicMock())
304+
305+
assert core_with_mock_sdk.sdk.fullscans.post.call_count == 1
306+
no_sleep.assert_not_called()
307+
message = str(exc_info.value)
308+
assert f"rejected as too large (HTTP {status_code})" in message
309+
assert "--commit-message" in message
310+
# The SDK's original text is kept so the proxy's own response stays available.
311+
assert f"original_status_code:{status_code}" in message
312+
assert exc_info.value.status_code == status_code

0 commit comments

Comments
 (0)