fix(lint): clear S-rule violations in backend/ (security bucket)

Mark commented

2026-08-24 10:05:30 +00:00

Owner

Part of #100 ruff cleanup — the bucket-security workstream (kanban t_5eecc109). Branch fix/100-ruff-security off origin/main (bd0fb0f).

Result

cd backend && ruff check . --select S --statistics → 0 errors (was 128).
cd backend && ruff check app/ --config ../pyproject.toml --select S → 0 errors.
pytest: 1297 passed, 5 skipped, 0 failed (matches wave-2 baseline).
pre-commit (ruff 0.11.0 + hooks) passes on all 42 changed files.
No new real-secret strings in the diff.

Real fixes (no suppression)

S101 (10): removed 7 dead assert case_id is not None in consolidator.py (guards above already return); assert call_hash is not None / assert result_dict is not None → explicit if x is None: raise RuntimeError(...) in orchestrator.py; isinstance assert → raise LLMParsingError in llm_entity_extraction.py (feeds existing legacy-format fallback).
S603 (4): explicit shell=False on subprocess.run in video_frame_extractor.py (x2), video_metadata.py, services/llm/tools/browser.py (list-arg calls, behavior-neutral).
S110/S112 → logging in 7 files that already wire a logger: corporate_releases.py (2), employee_scraper.py (2), social_finder.py (2), web_scraper.py (1), orchestrator.py (1), orchestrator_enhanced.py (1), orchestrator_v3_llm.py (1) — pass/continue become logger.debug(..., exc_info=True) (+continue).

Suppressions (each with one-line justification, listed per card rule)

File-level # ruff: noqa: S110 / S110, S112 on 29 no-logger best-effort files (27 modules + tests/ci_quality_gate.py + tests/integration/test_integration_resilience.py): try/except/pass is the documented project pattern — pyproject globally ignores S110/S112 common pattern; modules are best-effort OSINT fetchers with no logger; adding loggers to 40+ modules is a behavior change outside this bucket.
File-level # ruff: noqa: S311 on tests/performance/locustfile.py (load-test tool, pseudo-random payloads by design; pyproject ignores S311 false positives for rate limiting).
Inline S311 x2 in rate_limiter.py: jitter for rate limiting, non-crypto.
Inline S324 x2: virustotal.py url_id is a VT API identifier key (not a security hash); reverse_image_search.py md5 is a content-dedup/Google md5_search key — sha256 is already computed alongside.
Inline S603 x4 (same 4 subprocess sites): ruff 0.15.17 flags non-literal list args regardless of shell=False (rule docs note false-positive proneness); calls are module-internal arg lists, not user input.

Notes for verifier (t_7755ce59)

Non-S gate errors (PLC0415/F401/I001/UP042/UP047) are owned by sibling buckets and intentionally untouched.
Noqa count: base b81dc67 = 24; after this bucket ~54 — every new suppression carries a justification mirroring the project pyproject ignore rationale; none are blanket.
This branch is one of the 4 sibling fix PRs the verifier expects to merge before final gate verification.

Part of #100 ruff cleanup — the bucket-security workstream (kanban t_5eecc109). Branch fix/100-ruff-security off origin/main (bd0fb0f). ## Result - `cd backend && ruff check . --select S --statistics` → 0 errors (was 128). - `cd backend && ruff check app/ --config ../pyproject.toml --select S` → 0 errors. - pytest: 1297 passed, 5 skipped, 0 failed (matches wave-2 baseline). - pre-commit (ruff 0.11.0 + hooks) passes on all 42 changed files. - No new real-secret strings in the diff. ## Real fixes (no suppression) - S101 (10): removed 7 dead `assert case_id is not None` in consolidator.py (guards above already return); `assert call_hash is not None` / `assert result_dict is not None` → explicit `if x is None: raise RuntimeError(...)` in orchestrator.py; isinstance assert → `raise LLMParsingError` in llm_entity_extraction.py (feeds existing legacy-format fallback). - S603 (4): explicit `shell=False` on subprocess.run in video_frame_extractor.py (x2), video_metadata.py, services/llm/tools/browser.py (list-arg calls, behavior-neutral). - S110/S112 → logging in 7 files that already wire a logger: corporate_releases.py (2), employee_scraper.py (2), social_finder.py (2), web_scraper.py (1), orchestrator.py (1), orchestrator_enhanced.py (1), orchestrator_v3_llm.py (1) — `pass`/`continue` become `logger.debug(..., exc_info=True)` (+continue). ## Suppressions (each with one-line justification, listed per card rule) - File-level `# ruff: noqa: S110` / `S110, S112` on 29 no-logger best-effort files (27 modules + tests/ci_quality_gate.py + tests/integration/test_integration_resilience.py): try/except/pass is the documented project pattern — pyproject globally ignores S110/S112 common pattern; modules are best-effort OSINT fetchers with no logger; adding loggers to 40+ modules is a behavior change outside this bucket. - File-level `# ruff: noqa: S311` on tests/performance/locustfile.py (load-test tool, pseudo-random payloads by design; pyproject ignores S311 false positives for rate limiting). - Inline S311 x2 in rate_limiter.py: jitter for rate limiting, non-crypto. - Inline S324 x2: virustotal.py url_id is a VT API identifier key (not a security hash); reverse_image_search.py md5 is a content-dedup/Google md5_search key — sha256 is already computed alongside. - Inline S603 x4 (same 4 subprocess sites): ruff 0.15.17 flags non-literal list args regardless of shell=False (rule docs note false-positive proneness); calls are module-internal arg lists, not user input. ## Notes for verifier (t_7755ce59) - Non-S gate errors (PLC0415/F401/I001/UP042/UP047) are owned by sibling buckets and intentionally untouched. - Noqa count: base b81dc67 = 24; after this bucket ~54 — every new suppression carries a justification mirroring the project pyproject ignore rationale; none are blanket. - This branch is one of the 4 sibling fix PRs the verifier expects to merge before final gate verification.

Mark added 1 commit

2026-08-24 10:05:30 +00:00

fix(lint): clear S-rule violations in backend/ (security bucket for #100 )

CI / Backend Lint & Format (pull_request) Failing after 4s

Details

CI / Golden Fixture Validation (pull_request) Has been skipped

Details

CI / Backend Tests (pull_request) Has been skipped

Details

CI / Backend Security Tests (pull_request) Has been skipped

Details

CI / Frontend Lint (pull_request) Failing after 23s

Details

CI / Frontend Tests (pull_request) Has been skipped

Details

CI / Dependency Security Scan (pull_request) Has been skipped

Details

Performance Regression Tests / Performance Regression Detection (pull_request) Failing after 33s

Details

Stress Tests / Stress & Pipeline Tests (pull_request) Failing after 2m5s

Details

Stress Tests / Full Data Pipeline Validation (pull_request) Has been skipped

Details

CI / Frontend E2E Tests (pull_request) Has been skipped

Details

Performance Regression Tests / Compare with Baseline (pull_request) Has been skipped

Details

CI / All Checks Passed (pull_request) Failing after 48s

Details

148fbd82d0

Real fixes:
- S101: remove 7 dead asserts in consolidator.py (guards return earlier);
  explicit RuntimeError raises for call_hash/result_dict invariants in
  orchestrator.py; LLMParsingError for isinstance check in
  llm_entity_extraction.py (falls into existing legacy fallback).
- S603: explicit shell=False on 4 subprocess.run list-arg calls
  (video_frame_extractor x2, video_metadata, llm/tools/browser).
- S110/S112: log-and-swallow (logger.debug, exc_info=True) in the 7
  module/service files that already wire a logger.

Suppressions (one-line justification each):
- File-level noqa S110/S112 on 29 best-effort OSINT files without a
  logger (project pyproject globally ignores S110/S112 as 'common
  pattern'; converting 40+ modules to logging is out of bucket scope).
- File-level noqa S311 on locustfile (load-test payloads, non-crypto);
  inline S311 x2 in rate_limiter (jitter, non-crypto).
- Inline S324 x2: VT url_id is an API identifier key; reverse-image
  md5 is a content-dedup/Google search key (sha256 computed alongside).
- Inline S603 x4: ruff 0.15.17 flags non-literal list args regardless
  of shell=False; calls are module-internal, not user input.

Verification: ruff check . --select S = 0; CI-config S = 0; 1297 tests
pass; pre-commit ruff/ruff-format pass on all 42 changed files.