Skip to content

chore(stravapipe): enable ruff S (flake8-bandit) security lints - #990

Merged
andy-esch merged 2 commits into
mainfrom
chore/ruff-s-stravapipe
Sep 1, 2026
Merged

chore(stravapipe): enable ruff S (flake8-bandit) security lints#990
andy-esch merged 2 commits into
mainfrom
chore/ruff-s-stravapipe

Conversation

@andy-esch

Copy link
Copy Markdown
Owner

Adds the S (flake8-bandit) ruleset to stravapipe's ruff config, closing the Python SAST gap (gosec and govulncheck already cover the Go side). No behaviour change — lint config plus per-line suppressions with justifications.

Every one of the 14 src findings was triaged individually rather than blanket-suppressed, so the rule keeps catching new code:

  • S608 (SQL injection) x7 — all safe: each interpolates only identifiers (table/column names from Final constants, or hardcoded purge() args) while every data value is a bound parameter (@activity_ids, :id, @user_id). Per-line noqa with a reason, so a new query that interpolates a value still trips it.
  • S105 x2 — name-collision false positives (_SECRET_PATH is a mount path, token_url is a public URL).
  • S311 x2 — retry-backoff jitter; non-cryptographic by design.
  • S101 x3 — type-narrowing asserts, not untrusted-input validation (the _bq_writer ones are reached only behind a caller's is not None guard; the protobuf one holds by TYPE_MESSAGE contract).

Tests are exempted, not the codebase: just py-lint runs ruff check ., and tests carry 1200+ idiomatic S findings (asserts, fake fixture secrets). S101, S105, S106 and S608 are added to the three test per-file-ignore patterns, mirroring bandit's own skip-tests default, so src/ stays gated without the noise.

py-lint, py-typecheck (125 files) and py-test (670 passed) all green with S enforced; verified the gate bites by seeding a fake token and observing S105.

@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.69%. Comparing base (b806ade) to head (e0800f9).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #990   +/-   ##
=======================================
  Coverage   82.69%   82.69%           
=======================================
  Files         313      313           
  Lines       15809    15809           
  Branches     3383     3382    -1     
=======================================
  Hits        13074    13074           
  Misses       2541     2541           
  Partials      194      194           
Flag Coverage Δ
go-apigateway 63.93% <ø> (ø)
go-apigateway-integration 91.63% <ø> (ø)
go-dispatcher 77.33% <ø> (ø)
go-firestore-integration 22.11% <ø> (ø)
go-shared 87.35% <ø> (ø)
python 93.87% <100.00%> (ø)
python-integration 44.47% <16.66%> (ø)
web 81.56% <ø> (ø)
web-integration 27.71% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...travapipe/src/stravapipe/adapters/gcp/_bigquery.py 98.03% <ø> (ø)
...e/src/stravapipe/adapters/gcp/_bigquery_storage.py 99.31% <100.00%> (ø)
...pe/src/stravapipe/adapters/postgres/_connection.py 91.13% <100.00%> (ø)
...pe/src/stravapipe/adapters/postgres/_repository.py 93.25% <ø> (ø)
...ipe/src/stravapipe/application/backfill/service.py 99.23% <100.00%> (ø)
...e/application/deletion/bq_user_deletion_service.py 100.00% <ø> (ø)
...ackages/stravapipe/src/stravapipe/config/common.py 90.00% <100.00%> (ø)
packages/stravapipe/src/stravapipe/retry.py 96.73% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Adds the `S` (flake8-bandit) ruleset to stravapipe's ruff config, closing the
Python SAST gap (gosec and govulncheck already cover the Go side). No behaviour
change — lint config plus per-line suppressions with justifications.

Every one of the 14 src findings was triaged individually rather than
blanket-suppressed, so the rule keeps catching new code:

- S608 (SQL injection) x7 — all safe: each interpolates only identifiers
  (table/column names from Final constants, or hardcoded purge() args) while
  every data value is a bound parameter (@activity_ids, :id, @user_id). Per-line
  noqa with a reason, so a new query that interpolates a value still trips it.
- S105 x2 — name-collision false positives (_SECRET_PATH is a mount path,
  token_url is a public URL).
- S311 x2 — retry-backoff jitter; non-cryptographic by design.
- S101 x3 — type-narrowing asserts, not untrusted-input validation (the
  _bq_writer ones are reached only behind a caller's `is not None` guard; the
  protobuf one holds by TYPE_MESSAGE contract).

Tests are exempted, not the codebase: `just py-lint` runs `ruff check .`, and
tests carry 1200+ idiomatic S findings (asserts, fake fixture secrets). S101,
S105, S106 and S608 are added to the three test per-file-ignore patterns,
mirroring bandit's own skip-tests default, so src/ stays gated without the noise.

py-lint, py-typecheck (125 files) and py-test (670 passed) all green with S
enforced; verified the gate bites by seeding a fake token and observing S105.
…urface

Enabling ruff S in packages/stravapipe/pyproject.toml also gates scripts/ops,
local-dev/containers and schemas/bigquery, because pants points ruff at that one
config for every Python target (pants.toml [ruff] config). The initial commit
only ran the stravapipe-scoped `just py-lint`, so five S findings in the other
roots slipped through and failed CI's `pants lint`. All five are legitimate:

- S104 — cloudevent_adapter binds 0.0.0.0; a local-dev container must, to be
  reachable from the host. Per-line noqa.
- S310 x3 — offline ops tooling (check-strava-sports, load_census_regions)
  opening known https URLs; one already carried a nosemgrep note for the same
  concern. Per-line noqa.
- S603 — a CLI test runs the tool under test via subprocess with sys.executable
  and a fixed script path. Added to the test per-file-ignores alongside the
  other test-idiomatic S rules, since it is a test_*.py.

Verified with the exact CI command: `pants lint packages/stravapipe::
schemas/bigquery:: scripts/ops:: local-dev/containers::` — all checks passed.
@andy-esch
andy-esch force-pushed the chore/ruff-s-stravapipe branch from 928f25d to e0800f9 Compare September 1, 2026 11:18
@andy-esch
andy-esch merged commit 6c30e34 into main Sep 1, 2026
21 checks passed
@andy-esch
andy-esch deleted the chore/ruff-s-stravapipe branch September 1, 2026 11:30
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