Fix grant macros skipping roles with partial object coverage - #38
Merged
jonhopper-dataengineers merged 4 commits intoJul 21, 2026
Merged
Conversation
All grant, share, task, clean, and dependency macros now recognise build as a valid execution context alongside run and run-operation. Also fixes execute_task referencing undefined root_task variable. .... Generated with [Cortex Code](https://docs.snowflake.com/en/user-guide/cortex-code/cortex-code) Co-Authored-By: Cortex Code <noreply@snowflake.com>
Previously, grant macros would skip a role entirely if it had the privilege on ANY object in the schema. This meant that if a role had USAGE on 7 of 9 procedures (or OPERATE/MONITOR on 1 of N pipes/tasks), the remaining objects never received grants. Now each macro counts granted objects per role and only skips when the role already covers ALL objects (granted_count >= total_count). Affected macros: - grant_schema_procedure_usage_specific - grant_schema_operate_specific - grant_schema_monitor_specific - grant_schema_object_privileges .... Generated with [Cortex Code](https://docs.snowflake.com/en/user-guide/cortex-code/cortex-code) Co-Authored-By: Cortex Code <noreply@snowflake.com>
Member
Author
|
@sourcery-ai review |
.... Generated with [Cortex Code](https://docs.snowflake.com/en/user-guide/cortex-code/cortex-code) Co-Authored-By: Cortex Code <noreply@snowflake.com>
jonhopper-dataengineers
force-pushed
the
fix/grant-partial-coverage-skip-bug
branch
from
July 20, 2026 01:22
7580abe to
937a7ea
Compare
Contributor
Reviewer's GuideFix grant macros so roles are only skipped when they already have full object coverage, extend macros to run in dbt build contexts, and bump the package version and changelog accordingly. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The conditions in
grant_objectandgrant_object_applicationwere inverted (if flags.WHICH in [...]instead ofnot in), but the log message still indicates a skip; as written these macros will now always return early in allowed contexts, so the guard should be restored to the original polarity or the log updated to match the intended behavior. - In
grant_schema_monitor_specificandgrant_schema_operate_specific, theobject_count_querycounts PIPE/TASK objects only wheregrantor is not null, which will undercount total objects and can misclassify roles as fully granted; consider basingtotal_monitor_objects/total_operate_objectson the existingSHOWresults (or a table of objects) instead of filtering by existing privileges.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The conditions in `grant_object` and `grant_object_application` were inverted (`if flags.WHICH in [...]` instead of `not in`), but the log message still indicates a skip; as written these macros will now always return early in allowed contexts, so the guard should be restored to the original polarity or the log updated to match the intended behavior.
- In `grant_schema_monitor_specific` and `grant_schema_operate_specific`, the `object_count_query` counts PIPE/TASK objects only where `grantor is not null`, which will undercount total objects and can misclassify roles as fully granted; consider basing `total_monitor_objects`/`total_operate_objects` on the existing `SHOW` results (or a table of objects) instead of filtering by existing privileges.
## Individual Comments
### Comment 1
<location path="macros/grants/grant_object.sql" line_range="3-4" />
<code_context>
{% macro clean_objects(database=target.database, clean_targets=['local-dev', 'unit-test', 'test', 'prod'], object_types= ['schemas', 'functions_and_procedures', 'data_metric_functions', 'tasks', 'streams', 'stages', 'tables_and_views', 'alerts', 'file_formats', 'semantic_views', 'agents']) %}
{%if execute %}
- {% if flags.WHICH in ('run', 'run-operation') %}
+ {% if flags.WHICH in ['run', 'build', 'run-operation'] %}
{% if target.name in clean_targets %}
{% set dry_run = false %}
</code_context>
<issue_to_address>
**issue (bug_risk):** The context guard logic is inverted and will skip in the contexts where it should run.
With the current condition, the macro runs in all non-`run`/`build`/`run-operation` contexts and only skips in those three, which reverses the intended guard. Please restore the `not in` check (optionally adding `build`) or relocate the log/return to the `else` block so it only runs in the intended contexts.
</issue_to_address>
### Comment 2
<location path="macros/grants/grant_object_application.sql" line_range="3-4" />
<code_context>
{% macro clean_objects(database=target.database, clean_targets=['local-dev', 'unit-test', 'test', 'prod'], object_types= ['schemas', 'functions_and_procedures', 'data_metric_functions', 'tasks', 'streams', 'stages', 'tables_and_views', 'alerts', 'file_formats', 'semantic_views', 'agents']) %}
{%if execute %}
- {% if flags.WHICH in ('run', 'run-operation') %}
+ {% if flags.WHICH in ['run', 'build', 'run-operation'] %}
{% if target.name in clean_targets %}
{% set dry_run = false %}
</code_context>
<issue_to_address>
**issue (bug_risk):** The WHEN to-run guard is reversed here as well, causing the macro to run in non-run contexts.
This mirrors the issue in `grant_object`: wrapping the "Skipping ... not run/run-operation context" branch in `if flags.WHICH in ['run', 'build', 'run-operation']` reverses the logic, so the macro is skipped in allowed contexts and runs otherwise. Suggest reverting to the original `not in` condition (adding `build` if needed), or moving the skip block into the `else` branch so it only triggers outside the allowed contexts.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
The condition used 'in' but contained skip/return logic, meaning the macros were skipping in valid contexts and running in invalid ones. Restored to 'not in' to match the intended guard pattern. .... Generated with [Cortex Code](https://docs.snowflake.com/en/user-guide/cortex-code/cortex-code) Co-Authored-By: Cortex Code <noreply@snowflake.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
Test plan
.... Generated with Cortex Code
Summary by Sourcery
Fix grant and privilege macros to correctly handle roles with partial object coverage and support execution in dbt build context.
Bug Fixes:
Enhancements:
Build:
Documentation: