feat(bigquery): support authorized views with dataset restrictions#2561
feat(bigquery): support authorized views with dataset restrictions#2561Genesis929 wants to merge 12 commits intogoogleapis:mainfrom
Conversation
Refactors BigQuery query validation to support Authorized Views by combining behavioral analysis (Dry Run) with explicit reference auditing. ### Working Logic: The validation process uses a coordinated check: 1. Behavioral Detection (Dry Run): Identifies physical tables accessed by the query. If tables are found and all are allowed, the query is permitted immediately. 2. Explicit Reference Audit (Lexical Search): If the dry run identifies disallowed tables, a lexical scanner checks the SQL text for explicit mentions. Access is permitted if the disallowed tables are absent from the SQL, which correctly identifies Authorized View scenarios. 3. Fallback Intent Check: If no tables are found (e.g. DDL/Dynamic SQL) or to confirm the final intent, a local SQL parser validates the query to catch restricted operations and disallowed identifiers. ### Limitations: - Structural vs. Lexical Gap: SQL parsing complexity makes it difficult to understand all structural nuances; lexical search provides a safer layer to detect user intent where the structural parser might fall short. - False Positives: Queries may be blocked if non-table identifiers (like column aliases) exactly match a restricted table name. This prioritizes data security over permissive access. ### Changes: - Added IsAnyTableExplicitlyReferenced using a lexical state machine. - Reorganized ValidateQueryAgainstAllowedDatasets to optimize for performance (early return) and safety (coordinated checks). - Simplified tool code by removing redundant mock indirection. - Added tests for Authorized Views and complex table ID formats.
Summary of ChangesHello @Genesis929, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances BigQuery query validation by introducing robust support for authorized views when dataset restrictions are in place. The refactored validation process now intelligently combines BigQuery's dry run capabilities with a new lexical analysis step. This ensures that queries accessing data through authorized views are correctly permitted, while direct access to restricted underlying tables remains blocked, improving both security and flexibility. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for authorized views in BigQuery dataset restrictions through a multi-layered validation approach, refactoring core logic into ValidateQueryAgainstAllowedDatasets and enhancing the SQL parser. While the feature's intent is sound, the current implementation has significant security vulnerabilities: the lexical scanner and validation logic can be bypassed using SQL comments within identifiers, and restricted operations like EXECUTE IMMEDIATE can be circumvented via multi-statement queries. These critical bypasses must be addressed to ensure robust dataset restriction enforcement. Additionally, the review identified high-severity regressions due to removed statement type validation, potentially leading to less clear error messages, and medium-severity performance concerns in the SQL parsing logic from repeated string allocations in loops.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a sophisticated, multi-layered validation mechanism to support authorized views in BigQuery while enforcing dataset restrictions. The changes include a significant refactoring of the SQL parsing logic, centralizing validation, and adding comprehensive integration tests. The approach of combining a dry run, lexical analysis, and a fallback parser is robust. My feedback includes one suggestion to improve the user experience by providing more comprehensive error messages.
| if explicitlyReferenced { | ||
| return nil, fmt.Errorf("access to dataset '%s' is not allowed", strings.Join(strings.Split(violatingTables[0], ".")[:2], ".")) | ||
| } |
There was a problem hiding this comment.
The current implementation reports only the first dataset that violates the access policy. This could be frustrating for a user if their query accesses multiple disallowed datasets, as they would have to fix them one by one. Consider reporting all unique violating datasets in the error message to improve the user experience. Note that this change would also require updating the corresponding integration tests that check for this error message.
if explicitlyReferenced {
violatingDatasets := []string{}
seenDatasets := make(map[string]struct{})
for _, tableID := range violatingTables {
datasetFQN := strings.Join(strings.Split(tableID, ".")[:2], ".")
if _, seen := seenDatasets[datasetFQN]; !seen {
violatingDatasets = append(violatingDatasets, fmt.Sprintf("'%s'", datasetFQN))
seenDatasets[datasetFQN] = struct{}{}
}
}
plural := ""
if len(violatingDatasets) > 1 {
plural = "s"
}
return nil, fmt.Errorf("access to dataset%s %s is not allowed", plural, strings.Join(violatingDatasets, ", "))
}
duwenxin99
added
the
priority: p2
2 participants
Refactors BigQuery query validation to support Authorized Views by combining behavioral analysis (Dry Run) with explicit reference auditing.
Working Logic:
The validation process uses a coordinated check:
Limitations:
Changes:
Description
PR Checklist
CONTRIBUTING.md
bug/issue
before writing your code! That way we can discuss the change, evaluate
designs, and agree on the general idea
!if this involve a breaking change🛠️ Fixes #<issue_number_goes_here>