sadpandajoe opened a new pull request, #42016:
URL: https://github.com/apache/superset/pull/42016
### SUMMARY
Two tests in the subdirectory-deployment suite passed for the wrong reason.
Neither
would have caught the regression it was written to guard against. Test-only
— no
production code changes.
**1. The sanctioned-callers invariant scan skipped an entire module.**
`test_get_explore_redirect_url_sanctioned_callers` scans the source tree to
assert
that `get_explore_redirect_url` is called from exactly two places. To avoid
matching
the helper's own `def` line, it skipped `superset/views/utils.py` — the
defining
module — wholesale. That also blinded it to any genuine *call* added inside
that same
module, which is precisely what the scan exists to catch.
It now excludes the definition by shape rather than by path, via a `(?<!def
)`
lookbehind, so the defining module is scanned like any other.
This was mutation-verified: injecting a call to `get_explore_redirect_url`
into
`views/utils.py` now fails the assertion, where previously it passed
silently.
**2. The unsafe-redirect test could not distinguish a safe rejection from a
crash.**
`test_protocol_relative_url_falls_through_to_warning_page_under_subdir`
asserted only
`status_code != 302`. The external-URL branch calls `render_app_template`,
which needs
a Jinja loader the minimal test app does not have — so the request was
actually raising
`TemplateNotFound` and returning `500`. `500 != 302`, so the test passed,
while
asserting nothing about the behavior it names.
`render_app_template` is now stubbed, and the test asserts the status code,
the response
body, and the absence of a `Location` header. It's parametrized over three
unsafe
shapes: protocol-relative (`//host`), backslash-prefixed (`\\host`), and an
absolute URL
on a non-configured host. This gives the warning-page branch its first real
coverage in
this module.
### TESTING INSTRUCTIONS
- `pytest tests/unit_tests/views/test_redirect_view_subdirectory.py` — 12
pass (was 10;
the one weak test became three real ones).
- `pytest
tests/integration_tests/views/test_explore_redirect.py::TestExploreRedirect::test_get_explore_redirect_url_sanctioned_callers`
— passes.
To confirm the hardened scan actually bites, temporarily add a call to
`get_explore_redirect_url` inside `superset/views/utils.py` and re-run — it
now fails
with `views/utils.py` listed among the callers.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
