Copilot commented on code in PR #11655:
URL: https://github.com/apache/apisix/pull/11655#discussion_r2844068273
##########
apisix/secret/aws.lua:
##########
@@ -117,23 +117,29 @@ function _M.get(conf, key)
local sub_key = idx and sub(key, idx + 1) or nil
- core.log.info("main: ", main_key, sub_key and ", sub: " .. sub_key or "")
+ core.log.info("key: ", key, ", main: ", main_key, sub_key and ", sub: " ..
sub_key or "")
- local res, err = make_request_to_aws(conf, main_key)
- if not res then
- return nil, "failed to retrtive data from aws secret manager: " .. err
- end
+ -- key contains a slash, if secret is json
+ if main_key ~= key then
+ local res = make_request_to_aws(conf, main_key)
+
+ if res then
+ local data, err = core.json.decode(res)
+ if not data then
+ return nil, "failed to decode result, res: " .. res .. ", err:
" .. err
Review Comment:
In `_M.get`, when `core.json.decode(res)` fails, the returned error message
includes the raw `res` value (`"failed to decode result, res: " .. res .. ",
err: " .. err`). This message is propagated up to `secret.fetch_by_uri` and
logged via `core.log.error`, causing the full secret value from AWS Secrets
Manager (potentially credentials, keys, etc.) to be written to logs. To prevent
leaking sensitive data, avoid including the secret payload (`res`) in error
strings and logs; log only non-sensitive identifiers (like the secret key ID)
and the decode error message instead.
```suggestion
return nil, "failed to decode JSON result for key: " ..
main_key .. ", err: " .. (err or "unknown")
```
##########
apisix/secret/aws.lua:
##########
@@ -117,23 +117,29 @@ function _M.get(conf, key)
local sub_key = idx and sub(key, idx + 1) or nil
- core.log.info("main: ", main_key, sub_key and ", sub: " .. sub_key or "")
+ core.log.info("key: ", key, ", main: ", main_key, sub_key and ", sub: " ..
sub_key or "")
- local res, err = make_request_to_aws(conf, main_key)
- if not res then
- return nil, "failed to retrtive data from aws secret manager: " .. err
- end
+ -- key contains a slash, if secret is json
+ if main_key ~= key then
+ local res = make_request_to_aws(conf, main_key)
+
+ if res then
+ local data, err = core.json.decode(res)
+ if not data then
+ return nil, "failed to decode result, res: " .. res .. ", err:
" .. err
+ end
- if not sub_key then
- return res
+ return data[sub_key]
+ end
end
- local data, err = core.json.decode(res)
- if not data then
- return nil, "failed to decode result, res: " .. res .. ", err: " .. err
+ -- secret is string
+ local res, err = make_request_to_aws(conf, key)
+ if not res then
+ return nil, "failed to retrtive data from aws secret manager: " .. err
Review Comment:
Typo in user-facing error message: "retrtive" should be "retrieve".
```suggestion
return nil, "failed to retrieve data from aws secret manager: " ..
err
```
##########
apisix/secret/aws.lua:
##########
@@ -117,23 +117,29 @@ function _M.get(conf, key)
local sub_key = idx and sub(key, idx + 1) or nil
- core.log.info("main: ", main_key, sub_key and ", sub: " .. sub_key or "")
+ core.log.info("key: ", key, ", main: ", main_key, sub_key and ", sub: " ..
sub_key or "")
- local res, err = make_request_to_aws(conf, main_key)
- if not res then
- return nil, "failed to retrtive data from aws secret manager: " .. err
- end
+ -- key contains a slash, if secret is json
+ if main_key ~= key then
+ local res = make_request_to_aws(conf, main_key)
+
+ if res then
+ local data, err = core.json.decode(res)
+ if not data then
+ return nil, "failed to decode result, res: " .. res .. ", err:
" .. err
+ end
- if not sub_key then
- return res
+ return data[sub_key]
+ end
Review Comment:
The current fallback logic tries `make_request_to_aws(conf, main_key)` first
for any key containing `/`. This can return the wrong secret (if `main_key`
exists as a JSON secret with that field) or hard-fail (if `main_key` exists but
is a non-JSON string, decode fails) even when the intended secret is the full
`key` containing slashes. To avoid ambiguous lookups and potential secret
mix-ups, try fetching the exact `key` first; only if that fails should you fall
back to treating the last path segment as a JSON sub-key (and when you do,
capture/handle the error returned by `make_request_to_aws`).
--
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]
