Enforce ruff lint + format in CI and pre-commit, clear the baseline, and fix the CodeQL findings

[lelia]

Ruff already ran in CI, but as a job inside the Unit Tests workflow, so it inherited that workflow's paths: filter and never saw .hooks/, benchmarks/, or tests/e2e/. This moves it to its own unconditional Lint workflow, adds a pre-commit hook, bounds function complexity, and clears every resulting violation so the baseline is clean rather than suppressed.

ruff check and ruff format --check are both green across all 94 files. 523 tests pass.

Enforcement

• Lint workflow, unconditional on every PR. Not path-filtered: a filtered workflow reports "not run" rather than "passed", which blocks any PR that doesn't touch the filtered paths if it's made a required check.
• Pre-commit hook running ruff out of the project environment rather than the astral-sh/ruff-pre-commit mirror. Dependabot has no pre-commit ecosystem and will never update a mirror's rev:, so a mirror would drift from the pinned ruff==0.16.4 and produce the worst hook failure mode — clean locally, red on the PR.
• make lint now mirrors CI exactly.

Complexity

C901 at max 12, PLR0913 at max 8.

PLR0912 and PLR0915 were evaluated and rejected on evidence: PLR0912 duplicates C901 on 18 of 22 hits, and PLR0915's only unique catch is create_argument_parser — 116 statements but perfectly flat. max-args = 8 sits at the real gap in this codebase: everything is ≤7 arguments except run_reachability_analysis at 27.

The 20 functions over the limit today carry an explicit # noqa rather than a blanket per-file-ignores, so new complex functions in the same files are still caught. RUF100 fails the build once a suppression goes stale, so the backlog can only shrink — it already fired once during this work, when a refactor dropped _build_reachability_index under the limit. The worst remaining is main_code at complexity 109 / 424 statements.

Bugs fixed

These are behaviour changes, not style:

• Package.created_at was truncating timestamps. str.strip(" (Coordinated Universal Time)") treats its argument as a set of characters, not a suffix. "Tue Jan 15 ..." lost its leading T, and any timestamp without that suffix lost a trailing T. Now uses removesuffix.
• Notification calls could hang indefinitely. Slack, Teams, Jira, generic webhook and GitLab commit-status requests were sent with no timeout. requests blocks forever by default, so an unresponsive endpoint could hold a run open until the CI job itself timed out. All now pass an explicit 30s timeout. The existing GitLab tests caught this as a signature change and were updated.
• Two guards did nothing under python -O. assert is stripped in optimised mode. One was a real check on the org slug and now raises; the other was dead weight and was removed.
• A debug print was writing to stdout, which also carries SARIF. Now log.debug.
• config.py logged through the root logger, so its messages ignored the CLI's configured level and format. Now uses the socketcli logger like the rest of the package.
• Closures defined inside loops in alert_selection.py and messages.py captured loop variables by reference. Not live bugs — they were called within the same iteration — but they were hoisted and now take their inputs explicitly.

Formatting

ruff format is enforced from this PR onward. The formatting pass is bundled into the same commit as the lint fixes because they touch the same lines and can't be cleanly separated after the fact, so .git-blame-ignore-revs is added with instructions but no SHA — blame-ignoring this commit would also hide the real fixes above.

---

CodeQL: py/incomplete-url-substring-sanitization — fixed

CodeQL flagged "github.com" in diff_url at messages.py:80. The alert pre-exists on main; it surfaced here because the formatter touched that line.

Digging in turned up something more useful than a sanitization gap. diff_url is always a Socket dashboard link, built in Core as https://socket.dev/dashboard/org/{org_slug}/diff/.... Its host is always socket.dev and it carries no SCM information — which is exactly what the comment three lines below the check already said. The only variable part is the org slug, so the sniff could only fire when a Socket org slug contained "github", "gitlab" or "bitbucket". Those orgs got a link to a repository host they may not use; everyone else fell through to the Socket file view.

The branch was also nearly unreachable: CliConfig declares scm with a default of "api", so hasattr(config, "scm") is true for every real config and the elif never runs. It was observable only for a config carrying repo but no scm — with config=None the sniffed value was computed and then discarded, since every URL builder also requires a truthy config.

Now scm_type = (getattr(config, "scm", None) or "api").lower(), which covers a missing config, a config without the attribute, and an empty value.

get_manifest_file_url had no test coverage. Added 18 cases: GitHub, GitHub Enterprise, GitLab, self-hosted GitLab, Bitbucket, the Socket fallback, build-agent prefix stripping and multi-manifest paths. The three org-slug cases are regression guards — I verified they fail against the old implementation rather than assuming they would.

Removing the dead branch dropped the function under the complexity limit, so RUF100 required its # noqa: C901 be deleted. Complexity backlog: 20 → 19.

Judgment calls — resolved

All four were confirmed. TRY400/TRY401 moved from merely unselected into ignore in pyproject.toml, so the decision now survives a future family-wide "TRY" selection rather than silently reverting.

• TRY400 — rejected. log.exception() would dump tracebacks into customer CI logs for expected conditions (a missing config file, a handled APIFailure). To answer the config question directly: ruff offers three levels — [tool.ruff.lint] ignore (repo-wide), [tool.ruff.lint.per-file-ignores] (per path glob), and # noqa: TRY400 (per line). This is now in ignore, which is the durable form; "not selected" would have quietly come back the moment anyone added "TRY".
• E501 + W291/W293 — left to ruff format, and your steer settles it. Those trailing double-spaces in the PR-comment markup are Markdown hard line breaks. Rendering the real template both ways: 2 <br> with them, 0 without — the ❗️ Caution banner collapses into the body text and the comment becomes a run-on paragraph. Enforcing W291/W293 would strip them, and E501 would force hand-wrapping of that same customer-facing Markdown and the argparse help text. Whitespace inside a string is content.
• N (naming) — this is not about the smoke test. To clarify: the smoke test importing APIFailure was only evidence that these names are part of the public import surface; the constraint is external.
  • N815 — topLevelAncestors, diffType, manifestFiles, supplyChain are bound directly to Socket API JSON keys on both read and write: topLevelAncestors=data["topLevelAncestors"] when parsing, and the same key when serialising. Renaming the Python attributes would need alias plumbing on both sides, and would change the key names in the CLI's own --json output — a breaking change to a documented output contract, not a rename.
  • N818 — APIFailure, APIResourceNotFound and friends are importable public API. Renaming them breaks any consumer importing them and would need deprecation aliases. Happy to do it as its own PR with aliases if you want the suffix convention.
• SIM108 / PERF401 / S603 / S607 — confirmed, staying in ignore with reasons in pyproject.toml.

CodeQL: credential logging — fixed, plus 3 real leaks

The reported alert (py/clear-text-logging-sensitive-data at output.py:127) is a false positive: neither build_json_report nor build_fossa_report_payload copies a credential into the payload — both pick named fields. CodeQL taints anything derived from self.config because CliConfig declares api_token.

Chasing it found three real leaks:

Site	Leak	Reaches
socketcli.py	config.to_dict() is asdict(), so --debug printed the Socket API token in clear text	CI job log
plugins/slack.py ×2	full Slack webhook URL, one of them unconditional at debug level	CI job log and uploaded to Socket
output.py	configured Slack webhook URL in the Slack debug block	CI job log and uploaded to Socket

The Slack ones are the worse pair: they execute inside the StreamingLogs context, whose upload handler has no level filter and whose loggers are forced to DEBUG — so those records leave the machine. The token line runs before streaming attaches, so it stayed local. A Slack webhook URL is a bearer credential.

New socketsecurity/redaction.py. redact_mapping masks values by field-name pattern, so a github_token added later is covered without anyone editing it — there's a test that asserts exactly that by iterating CliConfig fields. redact_url keeps scheme and host (which is what the debug line was for) and drops the secret path plus any user:pass@. Unset values pass through, since "no token configured" is useful and isn't a secret. to_dict() still returns real values; to_redacted_dict() is the logging view — a serialiser that silently dropped the token would be its own bug.

Operational note: if a Socket API token or Slack webhook URL may already have been exposed in CI logs, rotate it.

CodeQL: 6 × actions/cache-poisoning — inert, recommend dismissal

Not fixed, because there is nothing to fix: no actions/cache step and no cache: input exists anywhere in this repository, so there is no cache to poison. CodeQL flags the pip install steps heuristically.

The underlying hazard is real but latent: under workflow_dispatch, pr-preview.yml's build job executes untrusted PR code in the default branch's cache scope, so if caching were ever added there a malicious branch could plant an entry every workflow on main then restores. What keeps it safe today is the privilege split — build holds only contents: read with no secrets and persist-credentials: false, while publish-package (which holds id-token: write) never checks out code and only downloads the built artifact.

I've documented that invariant in the workflow at the exact place someone would violate it. Dismissing these six is a security-posture call, so I left it to you — I'd dismiss as "won't fix / no cache in use". Verified my change adds no new zizmor findings (10 findings, 2 low — identical to main).

Merged main + ruff 0.16.5

Pulled in the ruff 0.16.4 → 0.16.5 bump, uv 0.12.8, and brotlicffi 1.2.0.2. 0.16.5 is a clean no-op here — ruff check and ruff format --check both pass unchanged, with no rule deprecations or renames against our select list.

main released 2.7.2 (#347) while this branch was open, so this is now 2.7.3 and the changelog is split accordingly.

Public Changelog

N/A

Ref: CE-451

---

Note

Medium Risk
Touches CI required checks, broad lint/format churn, and several CLI/runtime paths (HTTP notifications, config exit/logging, package timestamps) that affect customer pipelines.

Overview
Release 2.7.2 ships alongside a full Ruff enforcement story: linting moves out of the path-filtered Unit Tests workflow into a dedicated, unconditional Lint workflow (ruff check + ruff format --check), with matching pre-commit hooks and make lint / make hooks targets. pyproject.toml expands the rule set (bugbear, bandit, complexity C901/PLR0913, RUF100, etc.), documents intentional ignores, and clears the baseline across the repo.

Runtime fixes (not just style): Package.created_at now uses removesuffix instead of strip so timestamps are not mangled; outbound Slack/Teams/Jira/webhook/GitLab calls get a 30s requests timeout so CI cannot hang forever; config.py logs via the socketcli logger and sys.exit; duplicate SBOM packages log at debug instead of stdout; and guards that relied on assert are replaced or removed so python -O still behaves correctly.

Docs (CONTRIBUTING.md, CHANGELOG.md) and .git-blame-ignore-revs (placeholder for future format-only SHAs) support the new workflow.

^{Reviewed by Cursor Bugbot for commit ea36905. Configure here.}