Copilot commented on code in PR #13525:
URL: https://github.com/apache/apisix/pull/13525#discussion_r3395420565
##########
apisix/admin/resource.lua:
##########
@@ -421,6 +422,13 @@ function _M:patch(id, conf, sub_path, args)
local node_value = res_old.body.node.value
local modified_index = res_old.body.node.modifiedIndex
+ -- the encrypt_fields of the stored plugin conf are ciphertext, decrypt
+ -- them before merging, otherwise the check_conf below will encrypt them
+ -- again, resulting in fields that are encrypted multiple times
+ if apisix_plugin.enable_gde() then
+ utils.decrypt_params(apisix_plugin.decrypt_conf, res_old.body)
+ end
Review Comment:
Decrypting the stored plugin configs here makes `node_value` plaintext until
`check_conf()` re-encrypts it. This changes the security properties of later
log statements in this function (e.g. the `core.log.info("new conf: ...")`
log), which will now include decrypted secrets from plugins' `encrypt_fields`
when log level includes `info`. Please avoid logging the merged plaintext
config; move that log after `check_conf()` (so values are encrypted again),
remove it, or log a redacted/encrypted copy.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
