fix(security): address open CodeQL alerts (path-injection, url-redirection, stack-trace-exposure)#2884
Merged
Merged
Conversation
…py/stack-trace-exposure * `assets_upload` now drops any ``src_ext`` that isn't a plain ``.<alnum>`` extension before joining it onto ``assetdir``, so a hostile Content-Disposition filename can't smuggle ``/`` or ``..`` into the destination path (py/path-injection on ``app/views.py``). * `login` runs ``url_has_allowed_host_and_scheme`` inline at the ``redirect`` site so CodeQL sees the canonical Django sanitiser guarding the target (py/url-redirection on ``app/views.py``). The since-only-internal ``_safe_login_next`` helper is dropped because the check now lives at the call site. * `DeviceSettingsViewV2.patch` logs the exception via ``logger.exception`` and returns a generic error string instead of interpolating the exception into the 400 body (py/stack-trace-exposure on ``api/views/v2.py``). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses three actionable Python CodeQL alerts by tightening input handling in the upload path, hardening login redirects against open-redirect abuse, and avoiding exception detail leakage in an API response.
Changes:
- Sanitize uploaded asset filename extensions to prevent path injection via
Content-Disposition. - Inline
url_has_allowed_host_and_schemeguarding at the loginredirect()call site and remove the helper. - Replace API error interpolation with server-side exception logging and a generic client-facing error message.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/test_login_view.py | Updates test commentary to reflect the redirect check living in the view (behavior unchanged). |
| src/anthias_server/app/views.py | Adds extension validation for uploads and inlines the safe-redirect allowlist logic in login. |
| src/anthias_server/api/views/v2.py | Prevents stack-trace/detail exposure by logging exceptions server-side and returning a generic 400 error. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…e regex Copilot review on PR #2884 noticed that the original patch dropped any ``src_ext`` failing the ``\.<alnum>`` check without re-running the MIME-subtype fallback. A file uploaded with a filename like ``foo.`` (``path.splitext`` → ``'.'``) would land extensionless even when the ``image/<subtype>`` mapping would have recovered a valid extension — which means ``needs_image_normalisation`` returns False and a HEIC upload bypasses the pipeline. Lift the regex into ``_SAFE_EXT_RE`` and apply ``_safe_ext()`` to each candidate as it's tried. The control flow stays the same (short-circuit on the first accepted value), but a candidate that fails the regex no longer wedges ``src_ext = ''`` past the MIME fallback. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the three currently-actionable CodeQL alerts on
masterHEAD:py/path-injection(src/anthias_server/app/views.py:393) —assets_uploadnow drops anysrc_extthat isn't a plain.<alnum>extension before it's joined ontoassetdir, so a hostileContent-Disposition: filename=can't smuggle/or..into the on-disk path. The random-UUID basename was already the safe component; this is defence-in-depth on the extension.py/url-redirection(src/anthias_server/app/views.py:1014) —loginrunsdjango.utils.http.url_has_allowed_host_and_schemeinline at theredirect()site so CodeQL sees the canonical Django sanitiser guarding the target. The now-only-internal_safe_login_nexthelper is removed because the check lives at the call site.py/stack-trace-exposure(src/anthias_server/api/views/v2.py:665) —DeviceSettingsViewV2.patchlogs the failure vialogger.exceptionand returns a generic 400 instead of interpolating the exception into the response body.Other open alerts (not addressed in code)
py/csrf-protection-disabled(django_project/settings.py:121) — false positive:SameHostOriginCsrfMiddlewaresubclassesdjango.middleware.csrf.CsrfViewMiddleware. CodeQL doesn't follow class hierarchy; the existingNOSONARcomment already documents this for SonarQube's S4502. Will dismiss viagh code-scanning alert updateonce the rest land.auth.py,lib/auth.py,server.py,static/js/moment.js,static/js/bootstrap-datepicker.js) that no longer exist after the React→Django (e9738288),src/layout (133ec78f), anddjango.contrib.auth(8e2f38b1) refactors. They should auto-transition tofixedafter the next master scan; any stragglers can be dismissed.Test plan
uv run ruff check— green locally for the changed files.py/path-injection,py/url-redirection, andpy/stack-trace-exposureno longer flagged on these files.tests/test_login_view.py(off-hostnextrejection) andtests/test_template_views.py(HEIC/JPEG/video upload extension preservation) still pass.🤖 Generated with Claude Code