Copilot commented on code in PR #13833:
URL: https://github.com/apache/apisix/pull/13833#discussion_r3792034793
##########
apisix/admin/stream_routes.lua:
##########
@@ -151,12 +152,18 @@ local function delete_checker(id)
end
+local function encrypt_conf(id, conf)
+ apisix_upstream.encrypt_conf(conf.upstream)
Review Comment:
`encrypt_conf` currently returns `nil`. If the admin `resource` layer
expects the hook to return the (possibly mutated) config, this can accidentally
replace the stored config with `nil`. Consider explicitly returning `conf`
after encrypting (and optionally rename `id` to `_id` if it’s intentionally
unused) to make the contract unambiguous.
##########
t/admin/stream-routes.t:
##########
@@ -654,3 +654,51 @@ passed
GET /t
--- response_body
passed
+
+
+
+=== TEST 18: an inline upstream client key is encrypted at rest
+--- config
+ location /t {
+ content_by_lua_block {
+ local core = require("apisix.core")
+ local etcd = require("apisix.core.etcd")
+ local t = require("lib.test_admin")
+
+ local ssl_cert = t.read_file("t/certs/apisix.crt")
+ local ssl_key = t.read_file("t/certs/apisix.key")
+ local code, body = t.test('/apisix/admin/stream_routes/enc',
+ ngx.HTTP_PUT,
+ core.json.encode({
+ remote_addr = "127.0.0.1",
+ upstream = {
+ nodes = { ["127.0.0.1:8080"] = 1 },
+ type = "roundrobin",
+ scheme = "tls",
+ tls = {
+ client_cert = ssl_cert,
+ client_key = ssl_key,
+ },
+ },
+ })
+ )
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
+
+ local res = assert(etcd.get('/stream_routes/enc'))
+ local stored = res.body.node.value.upstream.tls.client_key
+ assert(stored ~= ssl_key, "the client key must be encrypted at
rest")
Review Comment:
This assertion can produce false positives: `stored ~= ssl_key` would still
pass if the key is merely normalized (e.g., trailing newline stripped) rather
than encrypted. To ensure the test fails unless encryption-at-rest is actually
applied, assert on an encryption-specific property (e.g., that `stored` does
not contain `PRIVATE KEY` / PEM markers, or matches an expected ciphertext
wrapper/prefix used by the encryptor).
##########
t/admin/stream-routes.t:
##########
@@ -654,3 +654,51 @@ passed
GET /t
--- response_body
passed
+
+
+
+=== TEST 18: an inline upstream client key is encrypted at rest
Review Comment:
There are multiple consecutive blank lines before the new test header.
Consider reducing to a single blank line to keep the test file formatting
consistent and easier to scan.
--
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]
