Skip to content

Commit cf83115

Browse files
authored
Add --base-scan-id / --base-commit-sha flags for diffing against a chosen baseline (#261)
* add --base-scan-id / --base-commit-sha diff baseline overrides Signed-off-by: lelia <2418071+lelia@users.noreply.github.com> * document baseline overrides, merge-base workflow, Buildlkite-specific guidance Signed-off-by: lelia <2418071+lelia@users.noreply.github.com> * bump to v2.5.0, add CHANGELOG entry Signed-off-by: lelia <2418071+lelia@users.noreply.github.com> --------- Signed-off-by: lelia <2418071+lelia@users.noreply.github.com>
1 parent 12f97fb commit cf83115

11 files changed

Lines changed: 437 additions & 14 deletions

File tree

CHANGELOG.md

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

3+
## 2.5.0
4+
5+
### Added: `--base-scan-id` / `--base-commit-sha` diff baseline overrides
6+
7+
- New mutually exclusive flags to control which full scan a diff is compared
8+
against, instead of always using the repository's latest head scan:
9+
- `--base-scan-id <id>` diffs against that full scan ID verbatim.
10+
- `--base-commit-sha <sha>` diffs against the most recent full scan created
11+
from that commit — e.g. the PR's merge base from
12+
`git merge-base origin/main HEAD` — so PR diffs are not polluted by
13+
default-branch commits the PR never branched from.
14+
- A `--base-commit-sha` with no matching full scan is a hard error (exit code 3,
15+
or `--exit-code-on-api-error`; exit 0 with `--disable-blocking`) rather than a
16+
silent fallback to the head scan, since diffing against the wrong baseline
17+
misreports which alerts a PR introduces.
18+
- Both flags are also settable via `--config` files (`base_scan_id`,
19+
`base_commit_sha`).
20+
- **Requirement:** `--base-commit-sha` looks up an existing scan — it does not
21+
create one. Using it requires CI to run `socketcli` on every commit that lands
22+
on the default branch; see the "Diffing against the merge base" note in
23+
`docs/cli-reference.md` for the failure modes and a backfill pattern.
24+
325
## 2.4.20
426

527
### Changed: bump pinned @coana-tech/cli to 15.8.8

README.md

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,26 @@ socketcli --target-path .
4242
socketcli --enable-gitlab-security --gitlab-security-file gl-dependency-scanning-report.json
4343
```
4444

45+
### PR scan diffed against the merge base
46+
47+
By default, PR scans are diffed against the repository's latest head scan. To diff against
48+
the exact commit your PR branched from instead, pass the merge base as the baseline:
49+
50+
```bash
51+
BASE_SHA=$(git merge-base origin/main HEAD)
52+
socketcli --pr-number 123 --base-commit-sha "$BASE_SHA"
53+
```
54+
55+
> **Requirement:** `--base-commit-sha` only works if Socket already has a full scan for that
56+
> exact commit. In practice this means your CI must run `socketcli` on **every commit that
57+
> lands on your default branch** — not just some of them. If merges can land without a scan
58+
> (skipped/canceled builds, `[skip ci]`, path-filtered pipelines), the PR scan will fail with
59+
> exit code 3 rather than silently diff against the wrong baseline. See
60+
> [`docs/cli-reference.md`](https://github.com/SocketDev/socket-python-cli/blob/main/docs/cli-reference.md)
61+
> for the full requirements and a backfill pattern that makes PR jobs self-sufficient.
62+
63+
A specific full scan ID also works: `--base-scan-id <id>`.
64+
4565
## SARIF use cases
4666

4767
### Full-scope reachable SARIF (grouped alerts)
@@ -204,8 +224,10 @@ Minimal pattern:
204224
| `3` | Infrastructure or API error (timeout, network failure, unexpected error) |
205225

206226
`--exit-code-on-api-error <N>` remaps the infrastructure-error code (`3`) to any
207-
value — e.g. a Buildkite `soft_fail` code, or `0` to swallow infra errors. Exit
208-
`3` is a Socket convention, not an industry standard.
227+
value — e.g. a Buildkite
228+
[`soft_fail`](https://buildkite.com/docs/pipelines/configure/step-types/command-step)
229+
code, or `0` to swallow infra errors. Exit `3` is a Socket convention, not an
230+
industry standard.
209231

210232
### How these options interact
211233

docs/ci-cd.md

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,54 @@ steps:
8181
SOCKET_SECURITY_API_TOKEN: "${SOCKET_SECURITY_API_TOKEN}"
8282
```
8383
84+
#### Merge-base baselines in Buildkite (dynamic pipelines)
85+
86+
Notes for using `--base-commit-sha` (see the
87+
[merge-base note in the CLI reference](cli-reference.md#pull-request-and-commit))
88+
when your steps are emitted by a
89+
[dynamic pipeline](https://buildkite.com/docs/pipelines/configure/dynamic-pipelines)
90+
generator rather than a static YAML file:
91+
92+
- **Compute the merge base at generation time, not step time.** The generator runs
93+
with a full checkout; step agents may have shallow or fresh clones where
94+
`git merge-base` fails or needs an extra fetch. Resolve it once in the generator and
95+
bake it into the emitted step's `env`. Diff against the PR's *target* branch, which
96+
isn't always the default branch (see Buildkite's
97+
[environment variables](https://buildkite.com/docs/pipelines/configure/environment-variables)):
98+
99+
```shell
100+
TARGET="${BUILDKITE_PULL_REQUEST_BASE_BRANCH:-$BUILDKITE_PIPELINE_DEFAULT_BRANCH}"
101+
BASE_SHA=$(git merge-base "origin/${TARGET}" HEAD)
102+
```
103+
104+
- **Emit the backfill step conditionally from the generator.** The generator is the
105+
natural place for the "does a baseline scan exist?" check
106+
(`GET /orgs/{org}/full-scans?repo=<repo>&commit_hash=$BASE_SHA&per_page=1`): only
107+
emit the baseline-scan step when it returns nothing. The emitted pipeline then shows
108+
in the UI whether a backfill will run.
109+
110+
- **Keep the backfill inside one command step.** The checkout-base → scan →
111+
checkout-PR sequence must not be split across steps — steps can land on different
112+
agents with different checkouts. Prefer
113+
[`git worktree`](https://git-scm.com/docs/git-worktree) over mutating the step's
114+
checkout: `git worktree add /tmp/socket-base "$BASE_SHA"` then
115+
`socketcli --target-path /tmp/socket-base --branch "$TARGET" --disable-blocking`.
116+
117+
- **Soft-fail infra errors, not findings.** A missing baseline (or any API error)
118+
exits with code 3 (`--exit-code-on-api-error` to change it); real findings exit 1.
119+
[`soft_fail: [{exit_status: 3}]`](https://buildkite.com/docs/pipelines/configure/step-types/command-step)
120+
on the PR scan step keeps infra errors from blocking merges while security findings
121+
still do.
122+
123+
- **["Cancel intermediate builds"](https://buildkite.com/docs/pipelines/configure/canceling-builds#cancel-running-intermediate-builds)
124+
on the default branch is the main source of baseline gaps.** Canceled builds never
125+
scan their commit, so merge-base lookups for PRs based on those commits fail. The
126+
conditional backfill step above is the remedy; there is no per-step exemption from
127+
build cancellation in Buildkite. If you need strict scan-once semantics for
128+
concurrent backfills of the same merge base, serialize the backfill step with a
129+
[concurrency group](https://buildkite.com/docs/pipelines/configure/workflows/controlling-concurrency)
130+
keyed on the merge-base SHA.
131+
84132
### GitLab CI
85133

86134
```yaml

docs/cli-reference.md

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ This will simultaneously generate:
146146
socketcli [-h] [--api-token API_TOKEN] [--repo REPO] [--workspace WORKSPACE] [--repo-is-public] [--branch BRANCH] [--integration {api,github,gitlab,azure,bitbucket}]
147147
[--config <path>]
148148
[--owner OWNER] [--pr-number PR_NUMBER] [--commit-message COMMIT_MESSAGE] [--commit-sha COMMIT_SHA] [--committers [COMMITTERS ...]]
149+
[--base-scan-id BASE_SCAN_ID | --base-commit-sha BASE_COMMIT_SHA]
149150
[--target-path TARGET_PATH] [--sbom-file SBOM_FILE] [--license-file-name LICENSE_FILE_NAME] [--save-submitted-files-list SAVE_SUBMITTED_FILES_LIST]
150151
[--save-manifest-tar SAVE_MANIFEST_TAR] [--files FILES] [--sub-path SUB_PATH] [--workspace-name WORKSPACE_NAME]
151152
[--excluded-ecosystems EXCLUDED_ECOSYSTEMS] [--exclude-paths EXCLUDE_PATHS] [--include-dirs INCLUDE_DIRS] [--default-branch] [--pending-head] [--generate-license] [--enable-debug]
@@ -191,6 +192,39 @@ If you don't want to provide the Socket API Token every time then you can use th
191192
| `--pr-number` | False | "0" | Pull request number |
192193
| `--commit-message` | False | *auto* | Commit message (auto-detected from git) |
193194
| `--commit-sha` | False | *auto* | Commit SHA (auto-detected from git) |
195+
| `--base-scan-id` | False | | Full scan ID to diff against, overriding the repository's head scan as the baseline. Mutually exclusive with `--base-commit-sha` |
196+
| `--base-commit-sha`| False | | Commit SHA to diff against, overriding the repository's head scan as the baseline. The most recent full scan for that commit is used; the CLI errors (exit code 3, or `--exit-code-on-api-error`) if no scan exists for it. Mutually exclusive with `--base-scan-id` |
197+
198+
> **Diffing against the merge base** — by default, PR scans are diffed against the repository's *latest* head scan, which may include newer default-branch commits than your PR branched from. To diff against the exact commit your PR is based on, compute the merge base and pass it as the baseline:
199+
>
200+
> ```shell
201+
> BASE_SHA=$(git merge-base origin/main HEAD)
202+
> socketcli --pr-number 123 --base-commit-sha "$BASE_SHA"
203+
> ```
204+
>
205+
> **Requirement: a full scan must already exist for the merge-base commit.** `--base-commit-sha` does not create a scan of that commit; it looks up an existing one. That lookup only succeeds if your CI runs `socketcli` on **every commit that lands on your default branch** — every merge and direct push, not just periodic or latest-only scans. Common ways commits slip through without a scan:
206+
>
207+
> - CI settings that cancel or skip intermediate builds when newer commits land (e.g. Buildkite's ["cancel intermediate builds"](https://buildkite.com/docs/pipelines/configure/canceling-builds#cancel-running-intermediate-builds))
208+
> - `[skip ci]` commits, path-filtered pipelines, or failed/canceled scan steps
209+
> - merge-base commits that predate your Socket rollout
210+
>
211+
> If no scan exists for the commit, the CLI **fails** (exit code 3, or your `--exit-code-on-api-error` value; exit 0 with `--disable-blocking`) instead of silently falling back to the head scan — a wrong baseline would misreport which alerts the PR introduces. Don't adopt this flag without default-branch scan coverage in place; you'll fail PR builds on lookup misses.
212+
>
213+
> **Backfill pattern** — if your default-branch coverage has gaps, the PR job can create the missing baseline itself before scanning:
214+
>
215+
> ```shell
216+
> BASE_SHA=$(git merge-base origin/main HEAD)
217+
> # Create the baseline only if Socket doesn't have one for this commit yet
218+
> # (check: GET /orgs/{org}/full-scans?repo=<repo>&commit_hash=$BASE_SHA&per_page=1)
219+
> git checkout "$BASE_SHA"
220+
> socketcli --branch main --disable-blocking
221+
> git checkout -
222+
> socketcli --pr-number 123 --base-commit-sha "$BASE_SHA"
223+
> ```
224+
>
225+
> Run the baseline step with `--disable-blocking` (findings on the default branch must not fail the PR job) and an explicit `--branch`, since branch auto-detection is unreliable at a detached HEAD.
226+
>
227+
> Buildkite users with dynamically generated pipelines: see [Merge-base baselines in Buildkite](ci-cd.md#merge-base-baselines-in-buildkite-dynamic-pipelines) for generation-time vs. step-time guidance.
194228
195229
#### Path and File
196230
| Parameter | Required | Default | Description |

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.4.20"
9+
version = "2.5.0"
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.4.20'
2+
__version__ = '2.5.0'
33
USER_AGENT = f'SocketPythonCLI/{__version__}'

socketsecurity/config.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,8 @@ class CliConfig:
119119
scm: str = "api"
120120
sbom_file: Optional[str] = None
121121
commit_sha: str = ""
122+
base_scan_id: Optional[str] = None
123+
base_commit_sha: Optional[str] = None
122124
generate_license: bool = False
123125
enable_debug: bool = False
124126
allow_unverified: bool = False
@@ -265,6 +267,8 @@ def from_args(cls, args_list: Optional[List[str]] = None) -> 'CliConfig':
265267
'scm': args.scm,
266268
'sbom_file': args.sbom_file,
267269
'commit_sha': args.commit_sha,
270+
'base_scan_id': args.base_scan_id,
271+
'base_commit_sha': args.base_commit_sha,
268272
'generate_license': args.generate_license,
269273
'enable_debug': args.enable_debug,
270274
'enable_diff': args.enable_diff,
@@ -406,6 +410,12 @@ def from_args(cls, args_list: Optional[List[str]] = None) -> 'CliConfig':
406410
logging.error("--workspace-name requires --sub-path to be specified")
407411
exit(1)
408412

413+
# argparse only enforces the mutually exclusive group for real CLI args;
414+
# this also catches both values arriving via a --config file.
415+
if args.base_scan_id and args.base_commit_sha:
416+
logging.error("--base-scan-id and --base-commit-sha are mutually exclusive")
417+
exit(1)
418+
409419
if args.sarif_scope == "full" and not args.reach:
410420
logging.error("--sarif-scope full requires --reach to be specified")
411421
exit(1)
@@ -560,6 +570,25 @@ def create_argument_parser() -> argparse.ArgumentParser:
560570
help="Committer for the commit (comma separated)",
561571
nargs="*"
562572
)
573+
base_scan_group = pr_group.add_mutually_exclusive_group()
574+
base_scan_group.add_argument(
575+
"--base-scan-id",
576+
dest="base_scan_id",
577+
metavar="<id>",
578+
default=None,
579+
help="Full scan ID to diff the new scan against, overriding the repository's "
580+
"head scan as the baseline. Mutually exclusive with --base-commit-sha."
581+
)
582+
base_scan_group.add_argument(
583+
"--base-commit-sha",
584+
dest="base_commit_sha",
585+
metavar="<sha>",
586+
default=None,
587+
help="Commit SHA to diff the new scan against, overriding the repository's head "
588+
"scan as the baseline. The most recent full scan matching this commit (e.g. "
589+
"the merge base from 'git merge-base origin/main HEAD') is used; the CLI "
590+
"errors if no scan exists for it. Mutually exclusive with --base-scan-id."
591+
)
563592

564593
# Path and File options
565594
path_group = parser.add_argument_group('Path and File')

socketsecurity/core/__init__.py

Lines changed: 94 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,6 +1154,94 @@ def get_head_scan_for_repo(self, repo_slug: str) -> str:
11541154
repo_info = self.get_repo_info(repo_slug)
11551155
return repo_info.head_full_scan_id if repo_info.head_full_scan_id else None
11561156

1157+
def get_full_scan_id_by_commit(
1158+
self,
1159+
repo_slug: str,
1160+
commit_sha: str,
1161+
workspace: Optional[str] = None,
1162+
scan_type: Optional[str] = None
1163+
) -> Optional[str]:
1164+
"""
1165+
Finds the most recent full scan for a repository + commit SHA.
1166+
1167+
Used by --base-commit-sha to resolve the diff baseline (e.g. the merge-base
1168+
commit of a PR). When the same commit was scanned more than once, the newest
1169+
scan wins.
1170+
1171+
Args:
1172+
repo_slug: Repository slug the scan belongs to
1173+
commit_sha: Commit SHA the scan was created from
1174+
workspace: Socket workspace the scan belongs to, if any
1175+
scan_type: Socket scan type to match, if any
1176+
1177+
Returns:
1178+
Full scan ID if one exists for that commit, None otherwise
1179+
"""
1180+
query_params = {
1181+
"repo": repo_slug,
1182+
"commit_hash": commit_sha,
1183+
"sort": "created_at",
1184+
"direction": "desc",
1185+
"per_page": 1,
1186+
}
1187+
if workspace:
1188+
query_params["workspace"] = workspace
1189+
if scan_type:
1190+
query_params["scan_type"] = scan_type
1191+
1192+
response = self.sdk.fullscans.get(
1193+
self.config.org_slug,
1194+
query_params,
1195+
)
1196+
results = response.get("results") if isinstance(response, dict) else None
1197+
if not results:
1198+
return None
1199+
return results[0].get("id")
1200+
1201+
def resolve_base_full_scan_id(self, params: FullScanParams) -> Optional[str]:
1202+
"""
1203+
Resolves the baseline full scan ID to diff a new scan against.
1204+
1205+
Priority: --base-scan-id (used verbatim), then --base-commit-sha (newest
1206+
full scan for that commit), then the repository's current head scan. A
1207+
--base-commit-sha with no matching full scan is a hard error rather than a
1208+
silent fallback to the head scan, because diffing against the wrong
1209+
baseline silently misreports which alerts a PR introduces.
1210+
1211+
Returns:
1212+
Full scan ID to use as the diff baseline, or None when the repository
1213+
has no head scan yet (caller creates an empty baseline scan).
1214+
"""
1215+
if self.cli_config and self.cli_config.base_scan_id:
1216+
log.info(f"Using full scan {self.cli_config.base_scan_id} as diff baseline (--base-scan-id)")
1217+
return self.cli_config.base_scan_id
1218+
1219+
if self.cli_config and self.cli_config.base_commit_sha:
1220+
commit_sha = self.cli_config.base_commit_sha
1221+
scan_id = self.get_full_scan_id_by_commit(
1222+
params.repo,
1223+
commit_sha,
1224+
workspace=params.workspace,
1225+
scan_type=params.scan_type,
1226+
)
1227+
if scan_id is None:
1228+
log.error(
1229+
f"No full scan found for commit {commit_sha} in repo {params.repo} "
1230+
"(--base-commit-sha). Ensure a scan was created for that commit "
1231+
"(e.g. the CLI runs on default-branch pushes), or pass "
1232+
"--base-scan-id instead."
1233+
)
1234+
if self.cli_config.disable_blocking:
1235+
sys.exit(0)
1236+
sys.exit(self.cli_config.exit_code_on_api_error)
1237+
log.info(f"Using full scan {scan_id} (commit {commit_sha}) as diff baseline (--base-commit-sha)")
1238+
return scan_id
1239+
1240+
try:
1241+
return self.get_head_scan_for_repo(params.repo)
1242+
except APIResourceNotFound:
1243+
return None
1244+
11571245
@staticmethod
11581246
def update_package_values(pkg: Package) -> Package:
11591247
pkg.purl = f"{pkg.name}@{pkg.version}"
@@ -1242,8 +1330,8 @@ def get_added_and_removed_packages(
12421330
OVERWRITES whatever the diff embedded, before anything reads it.
12431331
Either way the embedded license payload is dead weight, and on
12441332
large dependency trees it inflated the diff response past ~2.3MB
1245-
and truncated it mid-string, crashing ``response.json()``
1246-
(CE-224, customer: Tremendous). Defaulting to ``False`` keeps the
1333+
and truncated it mid-string, crashing ``response.json()``.
1334+
Defaulting to ``False`` keeps the
12471335
diff lean with zero change to any output artifact. The parameter
12481336
is retained as an explicit override seam, not wired to the
12491337
``--exclude-license-details`` user flag (which still governs the
@@ -1388,11 +1476,9 @@ def create_new_diff(
13881476
log.info("No supported manifest files found - creating empty scan for diff comparison")
13891477
scan_files = Core.empty_head_scan_file()
13901478

1391-
try:
1392-
# Get head scan ID
1393-
head_full_scan_id = self.get_head_scan_for_repo(params.repo)
1394-
except APIResourceNotFound:
1395-
head_full_scan_id = None
1479+
# Resolve the baseline scan: --base-scan-id / --base-commit-sha override
1480+
# the repository's head scan (None only when the repo has no head scan yet).
1481+
head_full_scan_id = self.resolve_base_full_scan_id(params)
13961482

13971483
# If no head scan exists, create an empty baseline scan
13981484
if head_full_scan_id is None:
@@ -1475,7 +1561,7 @@ def create_new_diff(
14751561
# (the --exclude-license-details user flag) into the diff request. The
14761562
# diff path never consumes embedded license data (see
14771563
# get_added_and_removed_packages docstring), so requesting it only bloats
1478-
# the response and risks the CE-224 truncation crash on large repos. The
1564+
# the response and risks the truncation crash on large repos. The
14791565
# user flag still controls the dashboard report URL below; it just no
14801566
# longer gates this internal diff payload.
14811567
(

0 commit comments

Comments
 (0)