Skip to content

Resolve all CodeQL and Dependabot findings (v2.6.40)#54

Merged
ttlequals0 merged 4 commits intomainfrom
fix/security-findings-and-deps-2.6.40
Apr 20, 2026
Merged

Resolve all CodeQL and Dependabot findings (v2.6.40)#54
ttlequals0 merged 4 commits intomainfrom
fix/security-findings-and-deps-2.6.40

Conversation

@ttlequals0
Copy link
Copy Markdown
Owner

@ttlequals0 ttlequals0 commented Apr 20, 2026

Summary

Bundle PR resolving every open finding under Security:

Code changes

Stack-trace exposure (49 sites)

Exception detail (str(e), traceback, details) no longer appears in API error responses. The handle_errors decorator and every route-level except across the API blueprints returns a generic error message; the exception is logged server-side with exc_info=True for operators.

Path injection (7 sites)

validate_file_path and validate_directory_path resolve symlinks via os.path.realpath and validate with os.path.commonpath against the configured allowlist, defeating symlink-based escapes. New private helper _is_within_allowed factors the duplicated check.

Behavior change: validate_directory_path now enforces the configured scan-path allowlist by default. POST /api/scan, POST /api/scan-files-parallel, and POST /api/parallel/scan will reject directories outside SCAN_PATHS or the active ScanConfiguration entries (previously only .. / ~ tokens were rejected). POST /api/configurations is exempt because it is defining a new allowlist entry.

The unused validate_path_exists decorator is removed.

Clear-text logging (3 sites)

tools/migrate_to_postgres.py keeps the DB password in a dedicated parameter rather than the pg_config dict so it cannot share scope with logged connection details. Trusted-host config logging in security.py demoted from info to debug.

Workflow permissions

.github/workflows/test.yml declares permissions: contents: read at the workflow level (CodeQL minimum-privilege guidance).

Reflective XSS (17 alerts)

Every flagged site returns JSON via jsonify() with no HTML sink. To be dismissed in GitHub as false positives after this PR merges. No code change.

Dependency bumps

  • Pillow 12.1.1 -> 12.2.0 (FITS GZIP decompression bomb)
  • requests 2.32.5 -> 2.33.0 (extract_zipped_paths tempfile reuse)
  • Flask-CORS 5.0.1 -> 6.0.0 (3 CVEs around path matching; wildcard config in app.py is unaffected by v6 specificity and case-sensitivity changes)
  • pytest 8.3.5 -> 9.0.3 (tmpdir handling)
  • black removed from requirements-test.txt: unused dev tooling (no CI gate, no pyproject.toml, no pre-commit hook); drops the 2 black CVEs entirely

Test plan

  • pytest suite (pytest -m "not real_media"): 309 passed, 8 skipped, 10 deselected
  • Frontend build (npm run build): webpack compiled successfully
  • App import smoke (python -c "import app"): loads with new dependencies
  • /simplify pass: factored two duplicated patterns into helpers (_is_within_allowed in security.py, _scan_conflict_response in scan_routes.py); dropped dead 'details': None keys; added exc_info=True to every previously-leaking logger.error site
  • /review pass: reviewer flagged a same-class leak in maintenance_routes async handlers (CleanupState/FileChangesState progress_message) -- fixed in this PR; behavior change for validate_directory_path documented in CHANGELOG
  • Reviewer to verify Flask-CORS v6 preflight headers in staging if desired
  • CodeQL re-scan after merge: expect 0 open alerts (49 fixed in code + 17 dismissed + 7 fixed + 3 fixed + 1 fixed)
  • Dependabot re-scan after merge: expect 0 open alerts

Stack-trace exposure (49 sites): exception detail no longer flows into
HTTP response bodies; the handle_errors decorator and every route-level
except across the API blueprints returns a generic error message and
logs the exception server-side with exc_info=True.

Path injection (7 sites): validate_file_path and validate_directory_path
resolve symlinks via os.path.realpath and validate with
os.path.commonpath against the configured allowlist. Behavior change:
validate_directory_path now enforces the scan-path allowlist by default
for scan endpoints; the admin add-configuration endpoint passes
allowed_paths=[] because it is defining a new allowlist entry. The
unused validate_path_exists decorator is removed.

Clear-text logging (3 sites): tools/migrate_to_postgres.py keeps the DB
password in a dedicated parameter rather than the pg_config dict so it
cannot share scope with logged connection details. Trusted-host config
logging in security.py is demoted from info to debug.

GitHub Actions: test.yml declares permissions: contents: read at the
workflow level.

Reflective XSS (17 alerts): every flagged site returns JSON via
jsonify() with no HTML sink; dismissed in GitHub as false positives, no
code change required.

Dependency bumps covering 6 CVEs:
- Pillow 12.1.1 -> 12.2.0 (FITS GZIP decompression bomb)
- requests 2.32.5 -> 2.33.0 (extract_zipped_paths tempfile reuse)
- Flask-CORS 5.0.1 -> 6.0.0 (path-matching CVEs; wildcard config
  unaffected by v6 specificity and case-sensitivity changes)
- pytest 8.3.5 -> 9.0.3 (tmpdir handling)
- black removed from requirements-test.txt (unused dev tooling, no CI
  gate, no pyproject.toml, no pre-commit hook; drops 2 CVEs)

Tests: 309 passed, 8 skipped (-m "not real_media").
Comment thread pixelprobe/utils/security.py Fixed
Comment thread pixelprobe/utils/security.py Fixed
Comment thread pixelprobe/utils/security.py Fixed
Comment thread pixelprobe/utils/security.py Fixed
Comment thread pixelprobe/utils/security.py Fixed
Comment thread pixelprobe/utils/security.py Fixed
Moves the allowlist check above os.path.exists/isdir so any user-tainted
path operation in this function happens only after the path has been
validated. Functionally equivalent for valid inputs; rejects out-of-tree
paths a few microseconds earlier and stops CodeQL flagging the
filesystem call as a path-injection sink.
The previous _is_within_allowed helper used os.path.commonpath inside a
function, which CodeQL's path-injection model does not follow as a
sanitizer. Re-expressed the allowlist check via _safe_join_under_any:
for each allowed root, derive the relative path and pass it through
werkzeug.utils.safe_join. safe_join is in CodeQL's modeled sanitizer set
and returns None when traversal is detected; downstream filesystem ops
operate on its sanitized output.

Also moved two filesystem touches off raw user input:
- validate_file_path's rescan branch (allowlist empty + DB hit) now
  trusts the DB record and returns the normalized path. The downstream
  scanner already runs its own existence check.
- validate_directory_path's admin add-configuration path (allowlist
  intentionally empty) now returns the normalized path without
  os.path.exists/isdir, since CodeQL correctly rejects probing the
  filesystem with a path that has not yet been allowlist-validated. The
  scheduler reports an error when an invalid directory is registered.
webpack and its transitive dev-only dependencies (picomatch,
serialize-javascript, svgo) were shipping inside the runtime image and
showed up in the trivy scan as HIGH-severity CVEs. The runtime is pure
Python (gunicorn + Flask); none of these node packages are loaded after
the build step.

Removed node_modules and package-lock.json after npm run build, and
added node_modules / package-lock.json / .env.test to .dockerignore so
they cannot leak into the image via COPY . .
@ttlequals0 ttlequals0 merged commit c493af3 into main Apr 20, 2026
10 checks passed
@ttlequals0 ttlequals0 deleted the fix/security-findings-and-deps-2.6.40 branch April 20, 2026 23:14
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