mikebridge opened a new pull request, #44319:
URL: https://github.com/apache/superset/pull/44319
### SUMMARY
Version history and the activity stream are **edit**-gated on the path
entity (#44021): the full change log — author identities, timestamps,
field-level before/after diffs — is for principals who may alter the entity.
Related-entity records inside that stream were gated only for **presence**,
by the read-level access filters in
`superset/versioning/activity/visibility.py`. The records themselves still
carried `changed_by` and the field-level `from_value` / `to_value` / `path`
diff. So an editor of a dashboard who holds only READ on a dataset that
dashboard's charts use could read that dataset's field-level change detail, and
the identity of whoever made each change — an entity they may not edit.
Redaction already existed in `apply_record_decoration`, but keyed on
tombstone state rather than on editorship of a live entity, so it never covered
this case.
**The fix.** For a live related entity the requester cannot edit, drop
`changed_by`, `from_value`, `to_value` and `path`. Presence, `entity_name`,
`summary` and `impact` remain: the name and presence are already read-gated by
the visibility filter, and `impact` is dashboard-scoped, kept under the same
explicit decision recorded for the tombstone branch. The requester still learns
*that* a related entity changed and which one — which read access entitles them
to — without the change content or the editor identity. Self records and the
tombstone redaction are unchanged.
Two details worth calling out for review:
* **The summary is rebuilt after the fields are dropped, not kept as
built.** `_build_summary` derives the restore headline from
`to_value["version_number"]` ("Dataset restored to version 37"), so keeping it
verbatim would have left a diff-derived value in place after redacting the
diff. Rebuilding makes the summary a function only of fields that survive
redaction — which holds for any future headline without re-auditing this.
* **The editorship predicate is not reimplemented.** `resolve_editorship`
batch-loads the live rows per kind with `editors` eager-loaded and delegates to
`security_manager.is_editor`, so the guest deny, the admin grant and the
subject-id rules keep exactly one definition and cannot drift. Admin and guest
principals short-circuit without querying at all. Unwired kinds, failed loads
and predicate errors all deny.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A (API response shape). For a related record whose entity the requester
can only read:
```
before: changed_by {id, first_name, last_name}, from_value, to_value, path
all present
summary "Dataset restored to version 37: Sales"
after: changed_by null, from_value null, to_value null, path null
summary "Dataset updated: Sales" (presence, name and impact
unchanged)
```
### TESTING INSTRUCTIONS
```
pytest tests/unit_tests/versioning/
pytest tests/integration_tests/versioning/activity_view_tests.py
```
Unit tests cover: a related record for a reader vs an editor; self records
unaffected; the tombstone redaction still winning; a fail-closed default for an
entity missing from the map; guest and admin short-circuits; and that
editorship resolves once per distinct entity rather than per record. The
integration test walks the end-to-end case — a dashboard the requester edits
whose chart uses a dataset they can only read — and then grants editorship and
asserts the detail returns.
Both leaks were verified red-first: with the redaction branch removed the
suite fails showing the exposed `changed_by` and the version-bearing summary.
### ADDITIONAL INFORMATION
- [x] Has associated issue: sc-120470 (residual from #44021, follow-up to
#43838)
- [ ] 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)
https://claude.ai/code/session_01267VBWbvWTNZUg9GvXKgkC
--
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]
