Configurable CLI exit behavior for API errors#211
Merged
Conversation
… logging Adds a configurable exit code for API/infrastructure failures so CI pipelines can distinguish them from blocking security findings (exit 1), without changing any default behavior. - New CliConfig field exit_code_on_api_error (default 3) + --exit-code-on-api-error flag. The CLI already exited 3 on unexpected errors; this just makes that code configurable (e.g. remap to a Buildkite soft_fail code, or 0 to swallow). - New _emit_infrastructure_error helper + IS_BUILDKITE gate: emits Buildkite log section markers (^^^ +++ / ---⚠️ ) and a soft_fail hint when running in Buildkite; plain log.error elsewhere so markers don't leak as literal text. - Wire the top-level generic-exception handler in cli() through the helper and the configurable code. Deliberately NON-breaking for 2.3.x: - --disable-blocking STILL forces exit 0 for all outcomes and takes precedence over --exit-code-on-api-error (documented in the flag help so the two aren't combined by mistake). - Default exit codes are unchanged; the exit code only changes when the user explicitly passes the flag. The breaking variant (infra errors bypassing --disable-blocking, distinct RequestTimeoutExceeded handling, exit 1 -> 3 for diff API failures) is intentionally deferred to a future 3.0 release. Signed-off-by: lelia <2418071+lelia@users.noreply.github.com>
The --commit-message flag passes its value directly into the API request URL as a query parameter with no length limit. AI-generated commit messages and the common CI pattern of concatenating $BUILDKITE_BUILD_NUMBER + $BUILDKITE_MESSAGE can easily exceed URL length limits, producing HTTP 413 errors. The 413 originates from an infrastructure-layer URL length limit (nginx/Cloudflare), not application-level validation -- confirmed via inspection of the Socket API route handler, which has no constraint on commit_message (unlike committers, which enforces <= 200 chars and returns a clean 400). 200 chars chosen as a conservative defensive ceiling given URL encoding can 2-3x raw character count. No customer should ever want a 2000-character commit message in their scan metadata. A backend-side validation (returning 400 instead of 413) is filed as a follow-on for the depscan API team. Motivated by customer incidents (Plaid). Signed-off-by: lelia <2418071+lelia@users.noreply.github.com>
Signed-off-by: lelia <2418071+lelia@users.noreply.github.com>
The full-scan diff comparison ignored --exclude-license-details: the flag was applied to full-scan params and report URLs but never forwarded to the fullscans.stream_diff request, so diff comparisons always fetched license details regardless of the flag. Thread it through get_added_and_removed_packages -> stream_diff via a new include_license_details param (defaulting True to preserve current behavior). Non-breaking: the APIFailure handling at this call site is deliberately left as-is (exit 1, --disable-blocking -> 0). Re-routing diff APIFailures through the top-level exit-3 path is part of the 3.0 exit-code change, not this one. Originally from the unreleased PR #195 branch; the timeout-propagation half already landed in the preceding commit. Signed-off-by: lelia <2418071+lelia@users.noreply.github.com>
…tting tests/unit/test_cli_config.py - exit_code_on_api_error default 3 / custom / zero - commit-message truncation: passthrough under 200, truncate over 200, quote-strip-before-truncate tests/unit/test_socketcli.py - unexpected error exits 3 by default - --exit-code-on-api-error 100 remaps the failure exit code - --disable-blocking OVERRIDES --exit-code-on-api-error (-> 0): locks in the documented precedence so the soft_fail guidance can't silently regress - KeyboardInterrupt still exits 2 - _emit_infrastructure_error: BK markers + soft_fail hint only when IS_BUILDKITE; traceback gated on include_traceback Signed-off-by: lelia <2418071+lelia@users.noreply.github.com>
Minor bump for the new --exit-code-on-api-error flag and the supporting non-breaking improvements (commit-message truncation, Buildkite-aware infra error logging, --timeout / --exclude-license-details fixes). This release is intentionally NON-breaking: default exit codes are unchanged, the exit code only shifts when --exit-code-on-api-error is explicitly passed, and --disable-blocking keeps its existing precedence. The breaking exit-code behavior change (infra errors exiting non-zero even under --disable-blocking) is deferred to a future 3.0. CHANGELOG + README document the flag AND its interaction with --disable-blocking (which overrides it) to reduce user error in the Buildkite soft_fail setup. Version refs synced across pyproject.toml, socketsecurity/__init__.py, and uv.lock (per the version-incrementation CI check). Signed-off-by: lelia <2418071+lelia@users.noreply.github.com>
|
🚀 Preview package published! Install with: pip install --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple socketsecurity==2.3.0.dev10Docker image: |
5 tasks
Eric Hibbs (flowstate)
approved these changes
May 29, 2026
…xit-handling Resolve conflicts: - pyproject.toml / __init__.py / uv.lock: keep 2.3.0 (supersedes main's 2.2.93) - CHANGELOG.md: keep both — 2.3.0 on top, then 2.2.93/2.2.92/2.2.91 from main Dependency bumps from #207 (idna 3.15, urllib3, etc.) carried through; uv lock --check passes. Signed-off-by: lelia <2418071+lelia@users.noreply.github.com>
lelia
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
--exit-code-on-api-error <N>so CI pipelines can give API / infrastructure failures (timeouts, network errors, unexpected exceptions) a distinct exit code from blocking security findings (exit1). Primary motivator: Buildkite CIsoft_failsetups that want to tolerate a Socket outage without ignoring real findings.Deliberately non-breaking. The CLI already exited
3on these errors; this just makes that code configurable. Default behavior is unchanged — the exit code only moves when you pass the flag, and--disable-blockingkeeps its existing precedence.What's included
--exit-code-on-api-error <int>(default3). Remap to a Buildkitesoft_failcode, or
0to swallow infra errors.BUILDKITE=true, infrastructure errors emitlog section markers (
^^^ +++/--- :warning:) so the section auto-expands in theBK UI, plus a
soft_failhint. No effect on other CI platforms.URL query parameters (AI-generated messages,
$BUILDKITE_MESSAGE).--timeoutis now honored end-to-end (it was applied to the local client butnot the SDK instance used for the diff comparison, which defaulted to 1200s).
--exclude-license-detailsnow propagates to the full-scan diff request.--disable-blockingforces exit0for all outcomes and therefore overrides--exit-code-on-api-error. For the common "tolerate outages, still block on findings"goal, use the new flag without
--disable-blocking:Combining the two would exit
0on findings and outages — thesoft_failrule wouldnever match and real findings would stop blocking. The README "How these options
interact" section spells this out, and a regression test locks the precedence in.
Exit codes
0--disable-blocking)123--exit-code-on-api-errorTest plan
--exit-code-on-api-errordefault3/ custom /0--disable-blockingoverrides the flag (→0) — regression-lockedBUILDKITE=trueFixes: CE-198
Refs: CE-196