Repository navigation
[v3-4-test] Hide assets the caller may not read from the UI dependencies graph (#73008) - #75390
Merged
Merged
Conversation
…ies graph (#73008) * Hide assets the caller may not read from the UI dependencies graph The dependencies graph scoped its nodes by which Dags the caller may read, but never by which assets. A caller who can read a Dag saw every asset that Dag produces or consumes, name included, even when the auth manager scopes that asset away from them; the data graph would also expand outward through such an asset. Asset names commonly encode bucket, table or dataset names, so this is the same class of gap the rest of the asset API closed. * Hide an unenriched asset leaf from the data dependencies graph An asset_expression leaf whose id the Dag processor has not filled in yet cannot be authorized, so it must be hidden like any other asset the caller may not read. The data graph instead parsed the id out of the node, which failed the request outright rather than dropping the node. (cherry picked from commit edcd4a5) Co-authored-by: Henry Chen <[email protected]>
1 task done
vatsrahul1001
marked this pull request as ready for review
October 22, 2026 09:52
vatsrahul1001
requested review from
bugraoz93,
choo121600,
ephraimbuddy,
henry3260,
jason810496,
pierrejeambrun,
rawwar and
shubhamraj-git
as code owners
October 22, 2026 09:52
The #73008 cherry-pick reintroduced patches of BaseAuthManager.get_authorized_assets, but SimpleAuthManager overrides that method, so patching the base class is a no-op and the readable-asset filtering is never exercised. v3-4-test already carries the check-auth-manager-mock-targets guard (via #75140), which also rejects the base-class target. Patch SimpleAuthManager.get_authorized_assets instead, matching main.
Contributor
Author
|
Hi maintainer, this PR was merged without a milestone set.
|
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.
The dependencies graph scoped its nodes by which Dags the caller may read,
but never by which assets. A caller who can read a Dag saw every asset that
Dag produces or consumes, name included, even when the auth manager scopes
that asset away from them; the data graph would also expand outward through
such an asset. Asset names commonly encode bucket, table or dataset names,
so this is the same class of gap the rest of the asset API closed.
An asset_expression leaf whose id the Dag processor has not filled in yet
cannot be authorized, so it must be hidden like any other asset the caller
may not read. The data graph instead parsed the id out of the node, which
failed the request outright rather than dropping the node.
(cherry picked from commit edcd4a5)
Co-authored-by: Henry Chen [email protected]