Skip to content

feat: add python utils and integrate into workflow#2176

Merged
taddes merged 13 commits intomasterfrom
feat/add-python-utils-STOR-519
Apr 1, 2026
Merged

feat: add python utils and integrate into workflow#2176
taddes merged 13 commits intomasterfrom
feat/add-python-utils-STOR-519

Conversation

@taddes
Copy link
Copy Markdown
Collaborator

@taddes taddes commented Mar 30, 2026

Description

This PR adds a number of important Python utilities to improve the developer experience. These increase reliability of code though regular static security checks using Bandit as well as enforcing type safety through MyPy. Additinoally, some useful linting and documentation features are added though pydocstyle and isort.

Adds Makefile targets to easily run these in a local environment (using Poetry) and integrates checks into our Python actions.

All of our integration and e2e testing is in Python, as well as our management utility scripts for Postgres and Spanner, so this updates all modules to be compliant.

Addresses lingering dependency updates in one swoop, too.

Bandit Security Linting

Code Fixes

B602 HIGHsubprocess with shell=True removed
tools/integration_tests/conftest.py:60
target_binary is resolved from hardcoded paths via os.path.exists() — never user-controlled.
Replaced Popen(target_binary, shell=True) with Popen([target_binary]).

B108 MEDIUM — hardcoded /tmp/ replaced with tempfile.mkstemp()
tools/tokenserver/test_scripts.py:152,167
Two test temp files now use fd, filename = tempfile.mkstemp(); os.close(fd).


# nosec Suppressions (false positives)

Code File Reason
B608 postgres/purge_ttl.py:97,110 DELETE f-strings built from validated enum literals; user data bound via SQLAlchemy :param
B608 spanner/purge_ttl.py:138,154 Same pattern — Spanner DML
B608 spanner/count_expired_rows.py:58 table iterates over hardcoded list ["batches", "bsos"]
B608 tokenserver/database.py:645 Internal test utility; column names from internal kwds, values bound as named params
B110 syncstorage-loadtest/storage/auth.py:81,108 Intentional broad exception swallowing in load test account tracking; failure is non-critical

Global Skips — pyproject.toml:59–67

Code Reason
B105 Test fixture hardcoded secrets ("SECRET") — never production code
B106 Test fixture hardcoded secrets ("Ted_Koppel_is_a_robot", "secret-a") — never production code
B603 Subprocess without shell=True on a trusted internal binary path — same category as already-skipped B404

B608 intentionally kept as inline # nosec (not global) so SQL injection detection remains active for future code.

Testing

New workflow steps in Python checks run, but you can check locally using the new Makefile targets.

Issue(s)

Closes STOR-519.

@taddes taddes self-assigned this Mar 30, 2026
@taddes taddes marked this pull request as ready for review March 30, 2026 19:16
@taddes taddes requested review from chenba and pjenvey March 30, 2026 19:45
@taddes taddes force-pushed the feat/add-python-utils-STOR-519 branch from 1a12a6d to 244dbff Compare March 30, 2026 21:27
Copy link
Copy Markdown
Member

@pjenvey pjenvey left a comment

Choose a reason for hiding this comment

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

I'd argue it's too strict to enforce pydocstyle on all test files, as most of them are superfluous, just repeating the test name itself. It should be easy enough to exclude them e.g. PyCQA/pydocstyle#553

run: poetry run ruff check tools

- name: Python Docstring Check
run: poetry run pydocstyle -es --count --config=pyproject.toml tools
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Apparently ruff might be able to replace pydocstyle now? It has a plugin, anyway https://docs.astral.sh/ruff/settings/#lint_pydoclint_ignore-one-line-docstrings

@taddes taddes merged commit af1c5fb into master Apr 1, 2026
23 checks passed
@taddes taddes deleted the feat/add-python-utils-STOR-519 branch April 1, 2026 20:02
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.

2 participants