fix(gem4gov): stop update-compliance stripping the Model Armor binding from customerPolicy - #205
Open
AloysJehwin wants to merge 1 commit into
Open
Conversation
The FedRAMP High assistant PATCH named customerPolicy in the updateMask while sending a body whose customerPolicy held only an empty bannedPhrases list. A masked field is replaced wholesale, so this silently discarded the modelArmorConfig deploy.sh had bound, leaving the template in place but referenced by nothing. Dropped customerPolicy from both the mask and the body. The empty bannedPhrases list was inert anyway — the API drops it — so nothing is lost and the binding survives. Fixes google#184 Signed-off-by: Aloys Jehwin <aloysjehwin@gmail.com>
aghassemlouei
added
bug
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
2 participants
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.
deploy.shbinds the Model Armor template by mergingmodelArmorConfiginto the assistant'scustomerPolicyand widening its mask.gem4gov app update-compliancethen PATCHes the same assistant withcustomerPolicystill named in the updateMask but a body whosecustomerPolicycontains only an emptybannedPhraseslist. A field named in a mask is replaced wholesale, not merged, so the binding is silently dropped — the template stays in the project, nothing references it, and no prompt or response is screened.Dropped
customerPolicyfrom both the mask and the body. The emptybannedPhraseslist wasn't doing anything — the API discards it, which is why the assistant reads back ascustomerPolicy: {}— so removing it costs nothing and leaves whatever deploy.sh configured intact. The alternative would have been reading the assistant back and re-mergingmodelArmorConfig, but that adds a round trip to preserve a value the command has no reason to touch.Only FedRAMP High is affected; the IL4 and IL5 masks don't name
customerPolicy.One thing I left alone: the mask also names
agentConfigsand the body has noagentConfigseither, so that field looks like it gets cleared the same way. I wasn't sure whether that one is deliberate for compliance, so I haven't touched it — happy to fold it in if it's the same oversight.Fixes #184