fix(tools/looker): enforce critical safety annotations for agent tools#2967
Conversation
…agent tools Address review feedback from duwenxin99 by: 1. Hardcoding ReadOnlyHint and DestructiveHint in agent tools to prevent user override. 2. Adding unit tests to verify annotation enforcement. 3. Keeping Go version at 1.25.7 to match main while omitting toolchain update.
![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 updates the initialization logic for tool annotations across several Looker agent tools (create, delete, get, list, and update) to ensure consistent application of ReadOnlyHint and DestructiveHint flags. The review feedback identifies a consistent issue where the input configuration struct is modified directly via a pointer, which could lead to unintended side effects if the caller reuses the configuration. It is recommended to create a new annotations instance and copy values from the configuration instead of modifying the original object.
…nnotations - Create a new ToolAnnotations instance in Initialize to avoid modifying the cfg.Annotations pointer directly. - Ensure ReadOnlyHint and DestructiveHint are enforced on the new instance.
|
@duwenxin99 Updated the code to avoid direct modification of the |
drstrangelooker
merged commit b8f37da
into
googleapis:looker-agent
3 participants
Description
This PR addresses the security and safety concerns raised in #2830 regarding critical annotations in Looker agent tools.
Changes
readOnlyHintanddestructiveHintin theInitializemethod for the following tools to ensure they cannot be overridden by user configuration:create_agent(readOnly: false)update_agent(readOnly: false)delete_agent(readOnly: false, destructive: true)get_agent(readOnly: true)list_agents(readOnly: true)TestAnnotationsto each tool's test suite to verify that these hints are strictly enforced even when a user attempts to override them in the configuration.1.25.7to match the currentmainbranch state while ensuring no unwanted toolchain updates were introduced.Verification Results
Ran unit tests for all 5 tools and confirmed that all tests, including the new annotation enforcement checks, passed successfully.