Skip to content

fix: clean up code scanning security alerts#608

Merged
jtn0123 merged 2 commits into
mainfrom
codex/security-code-scanning-cleanup
Apr 29, 2026
Merged

fix: clean up code scanning security alerts#608
jtn0123 merged 2 commits into
mainfrom
codex/security-code-scanning-cleanup

Conversation

@jtn0123
Copy link
Copy Markdown
Owner

@jtn0123 jtn0123 commented Apr 29, 2026

Summary

  • harden GitHub Actions shell inputs by moving GitHub context values into quoted environment variables
  • validate CDN SRI URLs, plugin/display module names, icon inputs, and benchmark schema migrations without suppressing alerts
  • adjust Docker install harnesses to default to a non-root user while still invoking privileged installer work through sudo
  • clean up scanner-flagged response/logging/JS console patterns and add regression coverage

Validation

  • PYTHONPATH=src python3 -m pytest tests/static/test_js_api_contracts.py tests/unit/test_benchmark_storage.py tests/unit/test_install_scripts.py tests/test_sri.py::TestUpdateCdnSriScript tests/unit/test_plugin_registry.py tests/unit/test_waveshare_display.py tests/unit/test_icon_utils.py tests/unit/test_request_models.py tests/integration/test_playlist_xss.py tests/unit/test_security_middleware_rate_limit.py -q
  • PYTHONPATH=src python3 -m pytest tests/unit/test_install_scripts.py -q
  • bash scripts/lint.sh
  • git diff --check

Notes

No code-scanning suppressions or alert dismissals were added. The GitHub security page should update after the PR code-scanning workflows upload fresh SARIF, and then again after merge to main.

Summary by CodeRabbit

  • Security

    • Added input sanitization to prevent path traversal attacks in icon rendering, plugin loading, and display configuration.
    • Implemented validation for CDN asset URLs; non-HTTPS URLs are now rejected.
    • Docker containers now run as non-root user for enhanced security.
  • Improvements

    • Enhanced error logging consistency across settings polling, playlist, and diagnostics features.
    • Improved rate-limiting response headers for login and token refresh endpoints.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 29, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 27db6547-fbdc-405a-94a6-994bf06e509f

📥 Commits

Reviewing files that changed from the base of the PR and between 2a7979a and a083142.

📒 Files selected for processing (21)
  • .github/workflows/ci.yml
  • .github/workflows/memory-diff.yml
  • scripts/Dockerfile.install-matrix
  • scripts/Dockerfile.sim-install
  • scripts/ci_install_matrix_verify.sh
  • scripts/update_cdn_sri.py
  • src/app_setup/security_middleware.py
  • src/benchmarks/benchmark_storage.py
  • src/blueprints/playlist.py
  • src/display/waveshare_display.py
  • src/plugins/plugin_registry.py
  • src/static/scripts/playlist/progress.js
  • src/static/scripts/settings/actions.js
  • src/static/scripts/settings/diagnostics.js
  • src/static/scripts/store.js
  • src/utils/icon_utils.py
  • tests/test_sri.py
  • tests/unit/test_benchmark_storage.py
  • tests/unit/test_icon_utils.py
  • tests/unit/test_plugin_registry.py
  • tests/unit/test_waveshare_display.py

📝 Walkthrough

Walkthrough

This PR applies security hardening and consistency improvements across CI/CD, deployment, and application code. Changes include GitHub workflow environment variable captures, Docker containers running as non-root users with passwordless sudo, input validation for CDN URLs and plugin/display identifiers, refactored error-logging patterns, and new test coverage validating sanitization and rejection of malicious inputs.

Changes

Cohort / File(s) Summary
GitHub Workflows
.github/workflows/ci.yml, .github/workflows/memory-diff.yml
Added dedicated environment variables (GITHUB_REF_VALUE, GITHUB_BASE_REF_VALUE) to capture GitHub context instead of using expressions directly in step references.
Docker Configuration
scripts/Dockerfile.install-matrix, scripts/Dockerfile.sim-install
Both Dockerfiles now create a non-root inkypi-ci user with passwordless sudo, adjust repository ownership, and invoke ./install.sh via sudo instead of as root.
Build & Installation Scripts
scripts/ci_install_matrix_verify.sh, scripts/update_cdn_sri.py
Updated installation verification to run via sudo; added URL validation in CDN SRI computation to reject non-HTTPS URLs before download.
Security & Input Validation
src/utils/icon_utils.py, src/plugins/plugin_registry.py, src/display/waveshare_display.py
Added input sanitization and strict regex validation for icon names, plugin IDs, and display types; replaced dynamic string interpolation with validated lookup-based approaches; updated import mechanisms to use validated identifiers.
Error Handling & Response Refactoring
src/app_setup/security_middleware.py, src/blueprints/playlist.py
Introduced _rate_limited_json_response helper to centralize 429 response construction with consistent Retry-After headers; updated playlist payload parsing to return error objects instead of responses directly.
Database & Storage
src/benchmarks/benchmark_storage.py
Replaced dynamic f-string SQL construction with pre-defined query lookup maps; raises ValueError for unexpected column names instead of executing unvalidated SQL.
JavaScript Error Logging
src/static/scripts/playlist/progress.js, src/static/scripts/settings/actions.js, src/static/scripts/settings/diagnostics.js, src/static/scripts/store.js
Standardized error logging by passing message strings and error/context objects as separate arguments to console.warn/console.error, replacing embedded string concatenation.
Test Coverage
tests/test_sri.py, tests/unit/test_benchmark_storage.py, tests/unit/test_icon_utils.py, tests/unit/test_plugin_registry.py, tests/unit/test_waveshare_display.py
Added tests validating CDN URL validation, rejection of unrecognized benchmark columns, sanitization of malicious icon/class inputs, unsafe plugin IDs, and unsafe display types.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 Through the code with fuzzy care,
Validating URLs everywhere—
Non-root hops and inputs kept secure,
XSS escapes make the logic pure,
A safer warren, test by test! 🛡️

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/security-code-scanning-cleanup

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 29, 2026

Memory diff vs base

Metric Base PR Delta
Peak RSS 61.27 MB 61.59 MB +324.0 KB
sys.modules count 592 592 0

Largest grouped allocator deltas

Group Base PR Delta
werkzeug 235.7 KB 2.24 MB +2.01 MB
python import system 15.38 MB 13.62 MB -1.75 MB
lark 12.78 MB 11.77 MB -1.00 MB
attr 231.5 KB 1.23 MB +1.00 MB
flask 59.2 KB 1.06 MB +1.00 MB
arrow 1.11 MB 122.3 KB -1015.5 KB
python stdlib 1.56 MB 2.52 MB +974.3 KB
Source-location detail: top 18 deltas (sampled base=500, PR=500)
# Location Base PR Delta
1 <frozen importlib._bootstrap_external>:757 8.56 MB 5.46 MB -3.10 MB
2 <frozen importlib._bootstrap>:488 5.69 MB 7.93 MB +2.24 MB
3 lark/lexer.py:215 3.00 MB 1.00 MB -2.00 MB
4 python3.12/functools.py:58 0 B 2.00 MB +2.00 MB
5 parsers/lalr_parser_state.py:105 3.00 MB 1.00 MB -2.00 MB
6 lark/tree.py:145 0 B 2.00 MB +2.00 MB
7 lark/lexer.py:389 1.00 MB 0 B -1.00 MB
8 python3.12/ast.py:78 1.00 MB 0 B -1.00 MB
9 parsers/lalr_analysis.py:285 1.00 MB 0 B -1.00 MB
10 arrow/locales.py:3873 1.00 MB 0 B -1.00 MB
11 <frozen importlib._bootstrap>:126 1.00 MB 0 B -1.00 MB
12 attr/_make.py:226 34.0 KB 1.03 MB +1.00 MB
13 parsers/lalr_analysis.py:128 0 B 1.00 MB +1.00 MB
14 sansio/response.py:488 0 B 1.00 MB +1.00 MB
15 lark/load_grammar.py:537 0 B 1.00 MB +1.00 MB
16 flask/sessions.py:337 0 B 1.00 MB +1.00 MB
17 lark/visitors.py:347 0 B 1.00 MB +1.00 MB
18 werkzeug/http.py:1211 0 B 1.00 MB +1.00 MB

JTN-610 · backend=base:memray, pr:memray · informational only, does not block merge. Hard RSS budgets are enforced separately by JTN-608. Source-location rows are sampled allocator attribution, not exact module ownership.

@jtn0123 jtn0123 changed the title [codex] fix code scanning security alerts fix: clean up code scanning security alerts Apr 29, 2026
@sonarqubecloud
Copy link
Copy Markdown

@jtn0123 jtn0123 marked this pull request as ready for review April 29, 2026 04:49
@jtn0123 jtn0123 merged commit 3a269ee into main Apr 29, 2026
33 of 34 checks passed
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.

1 participant