fix: redact encrypted fields from error log by Revolyssup · Pull Request #12629 · apache/apisix

Revolyssup · 2025-09-23T09:14:57Z

Currently sensitive encrypted fields are directly logged in plugins. This PR removes from all the places where sensitive information from plugins could be logged.

Checklist

I have explained the need for this PR and the problem it solves
I have explained the changes or the new features added to this PR
I have added tests corresponding to this change
I have updated the documentation to reflect this change
I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

nic-6443 · 2025-09-24T03:11:03Z

+    for name, conf in pairs(ctx.var._ctx.consumer.plugins) do
+        local plugin = require("apisix.plugins."..name)
+        local schema
+        if plugin.type == "auth" then
+            schema = plugin.consumer_schema
+        else
+            schema = plugin.schema
+        end
+        redacted_ctx.var._ctx.consumer.plugins[name] = redact_encrypted(conf, schema)
+        local redacted_auth = redact_encrypted(redacted_ctx.var._ctx.consumer.auth_conf, schema)
+        redacted_ctx.var._ctx.consumer.auth_conf = redacted_auth
+    end


this code snippet repeated multiple times, we should put it into a util function to reuse it.

nic-6443 · 2025-09-24T03:15:20Z

+        local redacted_auth = redact_encrypted(redacted_ctx.var._ctx.consumer.auth_conf, schema)
+        redacted_ctx.var._ctx.consumer.auth_conf = redacted_auth
+    end
+    core.log.info("ctx: ", core.json.delay_encode(redacted_ctx, true))


we should avoid to redact the log that will not be printed like json.delay_encode, my suggestion:

local max_delay_encode_items = 16 local delay_tab_idx = 0 local delay_tab_arr = {} for i = 1, max_delay_encode_items do delay_tab_arr[i] = setmetatable({data = "", force = false}, { __tostring = function(self) if self.redact_func then self.data = deepcopy(self.data) self.redact_func(self.data) end local res, err = encode(self.data, self.force) if not res then ngx.log(ngx.WARN, "failed to encode: ", err, " force: ", self.force) end return res end }) end function _M.delay_encode(data, force, redact_func) delay_tab_idx = delay_tab_idx+1 if delay_tab_idx > max_delay_encode_items then delay_tab_idx = 1 end delay_tab_arr[delay_tab_idx].data = data delay_tab_arr[delay_tab_idx].force = force delay_tab_arr[delay_tab_idx].redact_func = redact_func return delay_tab_arr[delay_tab_idx] end

only execute redact_func when this line of log be printed actually.

Revolyssup · 2025-09-29T07:27:56Z

 ## }
 ## After check, it only appears when watch recovers and returns events in bulk.
-cat logs/error.log | grep "}, {" || (echo "failed: Log case 2 unexpected"; exit 1)
+cat logs/error.log | grep "etcd events sent in bulk" || (echo "failed: Log case 2 unexpected"; exit 1)


Since the entire res object is no longer logged. We programmatically assert in code and log this specific condition to make this test case pass.

Revolyssup · 2025-09-29T07:30:30Z

    local dir_res, err = waitdir(self)
    log.info("waitdir key: ", self.key, " prev_index: ", self.prev_index + 1)
-    log.info("res: ", json.delay_encode(dir_res, true), ", err: ", err)
-


This log contained sensitive information and was being used to assert if there is a bulk operation. So the entire log is replaced with just the log about whether etcd events were sent in bulk.

membphis · 2025-09-30T05:09:47Z

 local cjson = require("cjson.safe")
 local json_encode = cjson.encode
 local clear_tab = require("table.clear")
+local table = require("apisix.core.table")


we do not need to update this library if we only remove some core.log(....)

membphis · 2025-09-30T05:10:19Z

                                                    nodes_ver, new_nodes)
    core.log.info("parse route which contain domain: ",
-                  core.json.delay_encode(route, true))
+                core.json.delay_encode(route, true, function (route)


same issue, we can remove this core.log.info

membphis · 2025-09-30T05:10:28Z

 function _M.merge_consumer_route(route_conf, consumer_conf, consumer_group_conf, api_ctx)
    core.log.info("route conf: ", core.json.delay_encode(route_conf))
-    core.log.info("consumer conf: ", core.json.delay_encode(consumer_conf))
+    core.log.info("consumer conf: ", core.json.delay_encode(consumer_conf, false,


membphis · 2025-09-30T05:10:42Z

+    end
    core.log.info("check plugin schema, name: ", name, ", configurations: ",
-        core.json.delay_encode(plugin_conf, true))
+        core.json.delay_encode(redacted_plugin_conf, true))


membphis · 2025-09-30T05:10:49Z


 function _M.rewrite(conf, ctx)
-    core.log.info("plugin access phase, conf: ", core.json.delay_encode(conf))
+    core.log.info("plugin access phase, conf: ", core.json.delay_encode(conf, false))


membphis · 2025-09-30T05:10:59Z

-        return core.schema.check(schema, conf)
+        cur_schema = schema
    end
+    core.log.info("input conf: ", core.json.delay_encode(conf_to_log, false))


membphis · 2025-09-30T05:11:06Z

    end
-    core.log.info("consumer: ", core.json.delay_encode(consumer, true))

+    core.log.info("consumer: ", core.json.delay_encode(consumer, true, function (cur_consumer)


membphis · 2025-09-30T05:11:14Z

    end
-    core.log.info("consumers: ", core.json.delay_encode(consumers))
-    return consumers[key]
+    local consumer = consumers[key]


Revolyssup · 2025-09-30T06:13:01Z

    resource.set_nodes_ver_and_nodes(route.value.upstream.resource_key,
                                                    nodes_ver, new_nodes)
+    -- remove plugin before logging to avoid logging sensitive info
+    local route_log = core.table.deepcopy(route)


this log is needed for test so only plugins are removed.

membphis

LGTM

AlinsRan · 2025-09-30T09:39:00Z



 function _M.merge_consumer_route(route_conf, consumer_conf, consumer_group_conf, api_ctx)
    core.log.info("route conf: ", core.json.delay_encode(route_conf))


Does route_conf not print sensitive data?

Before merging, consumer_conf has the sensitive data. it would have been caught in test if route conf had sensitive data,.

Signed-off-by: Nic <[email protected]> Co-authored-by: Ashish Tiwari <[email protected]>

fix: redact encrypted fields from error log

efd23d7

dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Sep 23, 2025

Revolyssup added 3 commits September 23, 2025 15:14

fix lint

27b0fd2

f

0363d3e

f

0a8d14e

dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Sep 23, 2025

Revolyssup marked this pull request as draft September 23, 2025 17:02

nic-6443 reviewed Sep 24, 2025

View reviewed changes

Revolyssup added 5 commits September 24, 2025 11:50

f

2fe210b

fix

62d1c74

f

ae15df9

f

f4ff4de

f

39a42ba

Revolyssup marked this pull request as ready for review September 24, 2025 07:27

Revolyssup added 4 commits September 24, 2025 13:02

f

0851933

jwe decrypt

7fef54a

f

479087d

nil check

9609c87

membphis reviewed Sep 24, 2025

View reviewed changes

Comment thread apisix/admin/resource.lua

Revolyssup added 8 commits September 24, 2025 19:13

remove logs

6af70f6

f

45e7deb

handle nil check

268ac27

f

62d80cf

fix key auth

a046f95

fix

035484f

fix basic auth

92d0671

remove log

1273202

Revolyssup added 5 commits September 26, 2025 13:28

fix

8fcc64e

fix lint

ffba3e2

fix

80a4dbd

fix lint

3733320

update cli test

dde1081

Revolyssup commented Sep 29, 2025

View reviewed changes

Revolyssup added 4 commits September 29, 2025 13:08

fix lint

75c359f

fix lint

e683c19

fix code

307d181

fix spaces

384d7ff

membphis requested changes Sep 30, 2025

View reviewed changes

Revolyssup added 4 commits September 30, 2025 11:04

remove redaction and just remove logs

1514913

f

e057b67

remove log

ff496b9

revert

c528cd2

Revolyssup commented Sep 30, 2025

View reviewed changes

Revolyssup requested review from membphis and nic-6443 September 30, 2025 06:15

f

503611b

membphis approved these changes Sep 30, 2025

View reviewed changes

nic-6443 approved these changes Sep 30, 2025

View reviewed changes

AlinsRan reviewed Sep 30, 2025

View reviewed changes

AlinsRan approved these changes Sep 30, 2025

View reviewed changes

Revolyssup merged commit 742fc43 into apache:master Sep 30, 2025
43 of 46 checks passed

Revolyssup deleted the revolyssup/remove-encrypt-field-from-log branch September 30, 2025 10:02

jizhuozhi pushed a commit to jizhuozhi/apisix that referenced this pull request Oct 18, 2025

fix: redact encrypted fields from error log (apache#12629)

6ab6053

shreemaan-abhishek pushed a commit to shreemaan-abhishek/apisix that referenced this pull request Jan 2, 2026

fix: redact encrypted fields from error log (apache#12629) (apache#1046)

6a16dff

Signed-off-by: Nic <[email protected]> Co-authored-by: Ashish Tiwari <[email protected]>



		function _M.merge_consumer_route(route_conf, consumer_conf, consumer_group_conf, api_ctx)
		core.log.info("route conf: ", core.json.delay_encode(route_conf))

Uh oh!

Conversation

Revolyssup commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

membphis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Revolyssup Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Revolyssup commented Sep 23, 2025 •

edited

Loading

Revolyssup Sep 30, 2025 •

edited

Loading