Skip to content

Cap the commit message read from the local checkout - #367

Open
lelia wants to merge 5 commits into
mainfrom
lelia/cap-git-commit-message-length
Open

lelia wants to merge 5 commits into
mainfrom
lelia/cap-git-commit-message-length

Conversation

@lelia

@lelia lelia commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

What

commit_message is sent as a query parameter on 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 applied to --commit-message, but a run that omitted the flag backfilled the value from the checkout's HEAD commit after config parsing, uncapped. A repository whose HEAD commit message carries generated release notes (~14 KB in the report) could not be scanned at all.

The cap is now an invariant of the parsed configuration instead of a step in flag parsing, and it is applied to the git-derived value as well, so every source of commit_message lands under the limit.

Changes

  • MAX_COMMIT_MESSAGE_LENGTH and a truncate_commit_message helper move to module scope in config.py; CliConfig.__post_init__ applies the cap to every construction, which covers the flag and config-file paths.
  • The git backfill in socketcli.py routes through the same helper.
  • The git setup block moves out of main_code into apply_git_context, so the backfill is reachable from a test. Same fields, same order, same handling of a non-repository path.
  • New tests in tests/unit/test_commit_message_truncation.py, including a real checkout with a 14 KB commit message.

Notes

  • The SCM-derived values (COMMIT_MESSAGE / BUILDKITE_MESSAGE in core/scm/github.py, CI_COMMIT_MESSAGE in core/scm/gitlab.py) were also named in the report. They are read onto the SCM config objects but never assigned to config.commit_message and never reach FullScanParams, so they are not a live bypass. They stay as-is, holding the raw environment value.
  • The old comment attributed the failure to a 413 from an nginx/Cloudflare URL limit; the report observed a 431. Both come from the proxy rather than the API, so the comment now names both instead of one.

Fixes CE-478


Note

Low Risk
Changes are localized to commit-message normalization and full-scan upload error handling; they reduce scan failures without altering auth or scan logic.

Overview
Release 2.9.7 fixes full scans that failed when commit_message in the request URL was too long (e.g. huge HEAD commit messages from git backfill).

Commit message cap is now enforced everywhere via truncate_commit_message and CliConfig.__post_init__ (flags, config file, and git-derived values). Over-limit text is shortened to 200 characters with a ... suffix and an INFO log. Git setup is refactored into apply_git_context, which applies the same truncation when filling commit_message from the repo.

Full-scan upload errors for HTTP 413, 414, or 431 are treated as non-retryable and surfaced with clearer guidance (multipart size vs URL/metadata, including --commit-message).

Tests cover truncation, git backfill, and the new upload error behavior.

Reviewed by Cursor Bugbot for commit 80a3c96. Configure here.

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.
@lelia
lelia requested a review from a team as a code owner September 22, 2026 16:19
@lelia
lelia deployed to socket-firewall September 22, 2026 16:20 — with GitHub Actions Active
…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.
@lelia

lelia commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up: two safeguards added for CI runs, after confirming the lineage of the original cap.

Origin of the cap. The 200-character cap came from CE-196#211cdd3bf6 (v2.3.0). The git backfill this PR fixes predates it by a year (49156d4, 2025-02-12), so the newly reported failure is the half CE-196 never covered, not a regression from it. Without CE-196 the same report would still have happened, and the flag path would have failed the same way alongside it.

Truncation is no longer silent. The notice moves from DEBUG to INFO and the stored value ends in .... A pipeline that does not pass --enable-debug previously had no way to tell why the message in the dashboard was clipped. 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.

An oversized request now names its cause. 413, 414 and 431 all mean the proxy refused the request line, and all three 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, with the SDK's original text kept underneath. They were not retried before and are not now, since the same oversized URL would go back out. This covers any oversized query parameter, not only the commit message.

Nothing Buildkite-specific was added: _emit_infrastructure_error already wraps API failures in log-section markers and prints the soft_fail hint, and this error reaches it like any other.

One behavior change worth a reviewer's eye: the ... marker means a truncated message is now 197 characters plus the marker rather than 200 characters of content, which also affects the --commit-message path from CE-196.

@lelia
lelia deployed to socket-firewall September 22, 2026 16:31 — with GitHub Actions Active

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread CHANGELOG.md Outdated
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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHANGELOG restates the full PR

Low Severity

The 2.9.7 section of CHANGELOG.md retells the incident, the git-backfill gap, the ... marker, the INFO-vs-DEBUG change, and the 413/414/431 handling. That is the PR narrative rather than a few short bullets of what shipped.

Fix in Cursor Fix in Web

Triggered by learned rule: CHANGELOG is user-facing — short bullets only

Reviewed by Cursor Bugbot for commit 392935d. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 800836c. The 2.9.7 section is now two bullets covering what a user of a patch release will observe — the cap applies to the value read from the repository, a truncated message ends in ..., and an oversized request names what to shorten. The incident narrative, the git-backfill gap and the INFO-vs-DEBUG reasoning are gone from the changelog; they live in the commit messages and the PR description, which is where history belongs.

The same pass reworded the code comments the entry had been echoing, so each states a present-tense invariant rather than what changed, and both the cap's rationale and the upload path now name the same three statuses instead of two overlapping subsets.

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.
@lelia
lelia deployed to socket-firewall September 22, 2026 16:40 — with GitHub Actions Active
@lelia

lelia commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

bugbot run

@lelia
lelia deployed to socket-firewall September 22, 2026 16:45 — with GitHub Actions Active
@lelia
lelia deployed to socket-firewall September 22, 2026 16:46 — with GitHub Actions Active

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 80a3c96. Configure here.

This branch was successfully deployed

1 active deployment
socket-firewall e809c5e0 Deployed Sep 22, 2026 by lelia via python-sfw-smoke-enterprise #417
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant