rusackas opened a new pull request, #45765:
URL: https://github.com/apache/superset/pull/45765

   ### SUMMARY
   `superset/utils/hashing.py` provides `hash_from_str` / `hash_from_dict`, 
which hash through the configurable `HASH_ALGORITHM` app setting (default 
`sha256`, overridable for FIPS deployments) rather than a hardcoded algorithm. 
Several production call sites hand-rolled the exact same 
`hashlib.sha256(...).hexdigest()` pattern instead of using these helpers, 
bypassing that configurability. This PR migrates those sites to the shared 
helpers, with no behavior change under the default `HASH_ALGORITHM=sha256` 
config.
   
   Files changed:
   - `superset/datasource/api.py` — `_column_values_cache_key` and the 
`compatible-columns` cache-key builder now use `hash_from_dict` instead of 
`hashlib.sha256(json.dumps(...))`.
   - `superset/utils/rls.py` — the guest-RLS cache identity hash now uses 
`hash_from_str` (kept on `hash_from_str` rather than `hash_from_dict` since 
`rls_rules` here is a list, and `hash_from_dict`'s type hint is `dict[Any, 
Any]`).
   - `superset/sql/execution/executor.py` — the SQL executor's result cache key 
now uses `hash_from_str`.
   - `superset/utils/file.py` — the export filename disambiguator 
(`_name_hash`) now uses `hash_from_str`.
   - `superset/mcp_service/catalog/tool/get_catalog.py` — the keyset-cursor 
search digest now uses `hash_from_str`.
   - `superset/commands/deletion_retention/purge_impact.py` — the dataset purge 
impact-token fingerprint now uses `hash_from_str`.
   
   Each file's now-unused `import hashlib` was removed; `hashlib` and `json` 
imports were otherwise left alone where still used for other purposes in the 
same file (e.g. `superset/datasource/api.py` still uses `json.dumps` elsewhere).
   
   **Deliberately NOT touched** (per the codebase's hashing conventions): 
`superset/config.py` (md5 matching an external `md5 <path>` command), 
`superset/utils/oauth2.py` (PKCE/RFC 7636 mandates SHA256), and 
HMAC/token-contract call sites in `password_recovery.py`, `guest_token.py`, 
`tasks/guest.py`, `semantic_layers/execution.py`, and 
`extensions/storage/persistent_dao.py` — these are pinned to a specific 
algorithm for a protocol/security reason, not candidates for this refactor.
   
   **On persistence**: none of the migrated hashes are stored in a database 
column or otherwise persisted across a deploy. They are all either (a) 
ephemeral cache keys (datasource column-values cache, compatible-columns cache, 
SQL executor result cache, guest-RLS cache identity) recomputed on every 
lookup, or (b) a short-lived confirm-then-delete token (`purge_impact.py`'s 
`impact_token`) that's generated and verified within the same request/response 
round trip, or (c) a filename disambiguator computed fresh at export time. 
Under the default `HASH_ALGORITHM=sha256` config (confirmed as the default in 
`superset/config.py`), none of these digests change value, so there's no 
one-time cache-miss risk even on deploy.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   N/A — internal refactor, no UI change.
   
   ### TESTING INSTRUCTIONS
   - `tests/unit_tests/utils/test_hashing.py`
   - `tests/unit_tests/utils/rls_test.py`
   - `tests/unit_tests/datasource/test_query_api.py`
   - `tests/unit_tests/datasource/test_compatible_api.py`
   - `tests/unit_tests/sql/execution/test_executor.py`
   - `tests/unit_tests/mcp_service/catalog/tool/test_get_catalog.py`
   - `tests/unit_tests/commands/deletion_retention/test_purge_impact.py`
   - `tests/unit_tests/utils/test_file.py`
   
   All 512 tests across these files pass locally. `pre-commit run` (mypy, ruff, 
pylint, etc.) is clean on the changed files.
   
   ### 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
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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]

Reply via email to