1 Commits

Include renames

Author	SHA1	Message	Date
3 people	66f9950a30	chore(ci): add security-audit tooling scripts (workflow files pushed separately) (#414) ... * chore(ci): add security-audit workflow and plugin security-proof scripts - scripts/prove_security.py, audit_plugins.py, generate_report.py -- automated checks (dangerous eval()/exec() calls, dependency scanning, report generation) for plugins. - .github/workflows/security-audit.yml + bandit.yaml -- CI wiring for the above plus gitleaks secret scanning and bandit static analysis. - .github/workflows/tests.yml -- pytest matrix across Python 3.10-3.12. Also fixes two Codacy findings while these files are freshly landing: - prove_security.py: dropped a pointless f-string prefix with no placeholders. - security-audit.yml: pinned gitleaks/gitleaks-action to a full commit SHA (matching this repo's existing pinning convention in test.yml) instead of the floating v2 tag. Split out of the original chore/dead-code-removal commit, which had accidentally bundled this in alongside unrelated dead-code deletions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * chore: drop workflow files -- pushed separately (needs workflow OAuth scope) * fix(security-tooling): address PR review findings across bandit.yaml, audit_plugins.py, generate_report.py, prove_security.py bandit.yaml: - Removed scripts/prove_security.py's file-level exclusion. Ran bandit directly to get ground truth: the real false positive is B105 (dict key "PASS" misread as password-like), not the eval/exec pattern the old comment claimed. Added a targeted # nosec B105 there, and found+fixed the identical pattern already present in generate_report.py. - Left the repo-wide B607 skip as-is: confirmed via AST scan that properly narrowing it touches 100+ bare-name subprocess call sites across wifi_manager.py, store_manager.py, permission_utils.py, app.py, and start.py -- none of which are part of this PR. Out of proportion to fix here; flagged as a dedicated follow-up. scripts/audit_plugins.py: - SyntaxError/OSError while scanning a plugin file now report CRITICAL (blocking) instead of WARNING/INFO -- a file that couldn't be parsed or read was never actually checked for danger, so it must not silently pass the audit. - --plugin <name> now tracks whether the requested plugin was found across all PLUGIN_BASE_DIRS and exits 1 with a clear error if not, instead of silently scanning zero plugins and reporting success. - The AST visitor now tracks import aliases (import subprocess as sp; from builtins import eval as e) and resolves them before checking against dangerous APIs, closing a straightforward evasion of every PLUGIN-001 through PLUGIN-005 check. Verified against both aliased and unaliased evasion patterns. scripts/generate_report.py: - _md_table_row now escapes pipe characters and normalizes newlines in every cell, so scanner-controlled content (a matched secret, a bandit issue_text) can't corrupt the Markdown table structure. - _load now distinguishes "artifact missing/malformed" from "valid empty result": each summarizer returns an availability flag, and main() now reports INCOMPLETE (not PASSED) with exit code 1 when any artifact is unavailable, instead of silently folding it in as 0 findings. - Gitleaks suppression now uses exact-match placeholder values (pulled from the actual config_secrets.template.json) plus a template-path allowlist, replacing broad substring checks that could hide a real secret containing something like "example.com" as part of its value. scripts/prove_security.py: - T1b (dangerous plugin calls): a file that fails to parse/read now reports CRITICAL with the exception details instead of being silently swallowed by `except (SyntaxError, OSError): pass`. - T6 (Docker hardening): base images must now be pinned to an @sha256 digest; a specific tag like python:3.12 is mutable and is now correctly flagged as unpinned, not just missing tags or :latest. - T2a (API surface): no config mechanism for enforcing local-only access exists in this codebase today (app.py hardcodes host='0.0.0.0'), so the "environment-aware" check as described isn't buildable without adding new config infrastructure -- out of scope here. Applied the achievable part: upgraded from INFO to WARNING, since enforcement can never currently be confirmed. - T1a (zip-slip): replaced the whole-file substring check with an AST walk that finds every extract()/extractall() call and confirms an is_relative_to() guard + "Zip-slip detected" log precede it in the same function. Verified it still passes on the real store_manager.py (both the per-member and validate-then-bulk-extract call sites) and correctly flags a synthetic unguarded extractall(). - T3a (hardcoded secrets): violation details no longer include the matched credential text -- only file, line, pattern type, and a redacted SHA-256 fingerprint, so a real finding doesn't get published into CI logs/artifacts/PR comments with wider exposure than the original leak. Verified with a synthetic secret that no raw content reaches the output. Validated: all four files compile; bandit scans all three scripts clean (2 legitimate targeted suppressions, 0 unaddressed findings); each new/ changed code path exercised directly (alias evasion, unmatched --plugin, missing/malformed/valid-empty artifacts, digest-pinning, zip-slip guard/no-guard, secret redaction); full audit_plugins.py -> prove_security.py -> generate_report.py pipeline run end-to-end producing a correct report. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * fix(security-tooling): address follow-up review findings on PR #414 scripts/generate_report.py: - Added an explicit `object` type annotation to _md_sanitize_cell's value parameter -- it deliberately accepts any stringifiable value (calls str(value) unconditionally), so `object` reflects its actual contract more accurately than leaving it untyped. scripts/prove_security.py: - Dockerfile FROM-line parsing: renamed the comprehension variable `l` to `line` (ambiguous single-letter name). More importantly, fixed a real false-positive: `FROM --platform=<platform> <image>` was reading the --platform= flag itself as the image token, so a properly digest-pinned image behind a platform flag was incorrectly reported as unpinned. Verified against platform+digest, platform+tag-only, and digest+AS-alias Dockerfiles. scripts/audit_plugins.py: - Consolidated visit_Call's dangerous-API detection: previously, alias resolution only covered ast.Name calls for eval/exec/compile and ast.Attribute calls for subprocess/os.system, missing from-imported subprocess/os functions called as bare names (from subprocess import run as prun; prun(cmd, shell=True) or from os import system as s; s(cmd)). Added _resolve_call_target() to resolve both call shapes to a single fully-qualified target, then run all five PLUGIN-00x checks against that one resolved value. Verified against 10 evasion combinations (from-import aliases, direct/attribute calls, aliased module imports) and confirmed zero false positives on benign os/ subprocess usage without shell=True. Validated: all three files compile, bandit scans clean (same 2 legitimate suppressions as before, 0 new findings), audit_plugins.py/prove_security.py re-run against the real repo with no regressions from the prior fix pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>	2026-07-15 14:33:17 -04:00