mirror of
https://github.com/kevinveenbirkenbach/computer-playbook.git
synced 2026-11-08 03:38:23 +00:00
kevinveenbirkenbachandClaude Opus 4.7
191bc1229a
Adds ``S`` (flake8-bandit) to the strict ruleset. ``PTH`` was scoped
out: it surfaced 1000+ violations with no auto-fix support (most PTH
sub-rules are warning-only because each ``str`` ↔ ``Path`` conversion
is context-sensitive). Reserved for its own dedicated commit.
Config tuning
=============
Global ``ignore`` additions with rationale:
- ``S603`` (subprocess call without ``shell=False``) and ``S607`` (start
process with partial path) are over-eager for a project whose CLI
invokes ``make`` / ``git`` / ``docker`` / ``ansible-playbook`` from
PATH using list-form args. The list-form already prevents shell
injection (Python's ``subprocess`` does not parse argv[0] as a shell
command), and resolving every binary's absolute path in the source
would couple the code to the host filesystem layout. The genuinely
dangerous shell-injection patterns (``shell=True``) are still caught
by S602 / S605.
New ``[tool.ruff.lint.per-file-ignores]`` for ``tests/**``:
- ``S101`` — ``assert`` is the unittest idiom.
- ``S102`` — ``exec`` is used to load script-under-test by file path.
- ``S105`` / ``S106`` — hardcoded credentials in fixtures / dummy creds.
- ``S108`` — ``/tmp`` paths in tempdir fixtures.
- ``S110`` / ``S112`` — ``try``/``except``/``pass``/``continue`` in cleanup helpers.
Hand-fixes (38 across 23 files)
================================
S101 × 3
Type-narrowing asserts (``cli/deploy/development/proc.py`` —
``p.stdout is not None`` after ``stdout=PIPE``) and an upstream-Odoo
invariant assert in ``roles/web-app-odoo/.../res_users.py``.
S105 × 1
``cli/core/app.py`` argv parser comparing ``token == "--log"`` —
``token`` is a CLI argv element, not a credential. Inline noqa.
S106 × 1
``utils/cache/users.py`` Jinja template string ``"{{ 42 |
strong_password }}"`` rendered to a real password at runtime; not a
literal credential. Inline noqa.
S108 × 6
Container-internal paths bind-mounted from the host:
- ``/tmp/infinito/ca/root-ca.crt`` and ``/tmp/infinito/bin/with-ca-trust.sh``
in ``roles/sys-svc-compose-ca/files/compose_ca.py`` and
``roles/sys-svc-container/files/container.py`` (CA-injection
contract between role and wrapper).
- ``/tmp/cache_nginx_general/`` and ``/tmp/cache_nginx_image/`` in
``plugins/lookup/nginx.py`` (well-known nginx cache dirs).
S110 × 1
``roles/web-app-bigbluebutton/filter_plugins/compose_mods.py``: filter
intentionally swallows YAML parse errors and falls back to the
unmodified text. Inline noqa.
S112 × 5
Filter plugins / inventory builders iterating over role config files
with skip-on-malformed semantics. The pattern is intentional
throughout (CLI tools / Ansible plugins shouldn't crash on a single
bad role file). Per-line noqa with rationale.
S324 × 2
``utils/cache/base.py`` uses md5 as a fast non-cryptographic
fingerprint for cache keying; ``utils/manager/value_generator.py``
uses sha1 when the caller selects ``algorithm == "sha1"`` for legacy
app compatibility. Both gained ``usedforsecurity=False`` (Python 3.9+
hashlib API) which silences the rule structurally; comments document
the non-security purpose.
S310 × 8
``urlopen`` calls to fixed HTTPS registries (``api.github.com``,
``hub.docker.com``, ``ghcr.io``). URL prefix is a literal; only the
``namespace`` / ``image`` segment varies and comes from the project's
own ``meta/services.yml``, never user input. No ``file://`` scheme
exposure. Inline noqa per call site.
S602 / S605 / S606 × 8
Role-deployed scripts that legitimately use ``shell=True`` /
``os.system`` for pipes/``&&``, or ``os.execvp`` in list-form (safe by
construction — no shell parses argv). Each call site gained an inline
noqa with a per-call rationale identifying where the command string
is sourced (Ansible-templated config, host-trusted, never untrusted
external input).
Verification
============
- ``ruff check .`` → All checks passed
- ``ruff format --check .`` → 894 files formatted
- 1621 unit tests → all pass
- Integration suite: 44 baseline failures (unchanged)
23 files changed, +97 / −38. Combined with the prior commits the
branch satisfies 21 ruff rule families end-to-end. ``PTH``
(modernise ``os.path`` to ``pathlib``) is the remaining big-impact
candidate, deferred to its own session.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
75 lines
1.7 KiB
Python
75 lines
1.7 KiB
Python
from __future__ import annotations
|
|
|
|
import subprocess
|
|
import sys
|
|
import threading
|
|
from typing import TextIO
|
|
|
|
|
|
def _drain_stream(
|
|
stream: TextIO,
|
|
*,
|
|
sink: TextIO,
|
|
) -> None:
|
|
"""
|
|
Read a text stream line-by-line and write to sink.
|
|
"""
|
|
try:
|
|
for line in iter(stream.readline, ""):
|
|
sink.write(line)
|
|
sink.flush()
|
|
finally:
|
|
try:
|
|
stream.close()
|
|
except Exception as exc:
|
|
sys.stderr.write(f"Warning: failed to close stream {stream!r}: {exc}\n")
|
|
sys.stderr.flush()
|
|
|
|
|
|
def run_streaming(
|
|
cmd: list[str],
|
|
*,
|
|
cwd,
|
|
env: dict[str, str],
|
|
text: bool = True,
|
|
) -> subprocess.CompletedProcess:
|
|
"""
|
|
Run a subprocess and stream stdout/stderr live to the terminal.
|
|
"""
|
|
p = subprocess.Popen(
|
|
cmd,
|
|
cwd=cwd,
|
|
env=env,
|
|
stdout=subprocess.PIPE,
|
|
stderr=subprocess.PIPE,
|
|
text=text,
|
|
bufsize=1, # line buffered (best effort)
|
|
)
|
|
|
|
# Type-narrowing asserts so the type checker sees the streams as
|
|
# non-None below (PIPE was passed); these never fire at runtime.
|
|
assert p.stdout is not None # noqa: S101
|
|
assert p.stderr is not None # noqa: S101
|
|
|
|
t_out = threading.Thread(
|
|
target=_drain_stream,
|
|
kwargs={"stream": p.stdout, "sink": sys.stdout},
|
|
)
|
|
t_err = threading.Thread(
|
|
target=_drain_stream,
|
|
kwargs={"stream": p.stderr, "sink": sys.stderr},
|
|
)
|
|
|
|
t_out.daemon = True
|
|
t_err.daemon = True
|
|
t_out.start()
|
|
t_err.start()
|
|
|
|
rc = p.wait()
|
|
|
|
# Process is finished; drain threads should finish quickly too.
|
|
t_out.join()
|
|
t_err.join()
|
|
|
|
return subprocess.CompletedProcess(cmd, rc)
|