fix(admin): harden stream_route superior_id dependency checks (#13672)

Author: shreemaan-abhishek

apisix/admin/stream_routes.lua

@@ -65,6 +65,10 @@ local function check_conf(id, conf, need_id, schema, opts)
 
     if conf.protocol and conf.protocol.superior_id and not opts.skip_references_check then
         local superior_id = conf.protocol.superior_id
+        if id and tostring(superior_id) == tostring(id) then
+            return nil, {error_msg = "stream route can not set itself as superior_id"}
+        end
+
         local key = "/stream_routes/" .. superior_id
         local res, err = core.etcd.get(key)
         if not res then
@@ -79,7 +83,12 @@ local function check_conf(id, conf, need_id, schema, opts)
 
         local superior_route = res.body.node.value
         if type(superior_route) == "string" then
-            superior_route = core.json.decode(superior_route)
+            local decoded, decode_err = core.json.decode(superior_route)
+            if not decoded then
+                return nil, {error_msg = "failed to decode stream routes[" .. superior_id
+                                         .. "]: " .. decode_err}
+            end
+            superior_route = decoded
         end
 
         if superior_route and superior_route.protocol
@@ -103,11 +112,11 @@ local function delete_checker(id)
     local key = "/stream_routes"
     local res, err = core.etcd.get(key, {prefix = true})
     if not res then
-        return nil, {error_msg = "failed to fetch stream routes: " .. err}
+        return 503, {error_msg = "failed to fetch stream routes: " .. err}
     end
 
     if res.status ~= 200 then
-        return nil, {error_msg = "failed to fetch stream routes, response code: " .. res.status}
+        return 503, {error_msg = "failed to fetch stream routes, response code: " .. res.status}
     end
 
     local nodes = res.body.list
@@ -124,7 +133,12 @@ local function delete_checker(id)
     for _, item in ipairs(nodes) do
         local route = item.value
         if type(route) == "string" then
-            route = core.json.decode(route)
+            local decoded, decode_err = core.json.decode(route)
+            if not decoded then
+                return 503, {error_msg = "failed to decode stream route [" .. tostring(item.key)
+                                         .. "]: " .. decode_err}
+            end
+            route = decoded
         end
 
         if route and route.protocol and tostring(route.protocol.superior_id) == id then

t/admin/stream-routes-subordinate.t

@@ -259,3 +259,88 @@ passed
 GET /t
 --- response_body
 passed
+
+
+
+=== TEST 8: superior_id references itself (should fail)
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local json = require("toolkit.json")
+            local code, body = t('/apisix/admin/stream_routes/10',
+                ngx.HTTP_PUT,
+                [[{
+                    "protocol": {"name": "redis", "superior_id": "10"},
+                    "upstream": {
+                        "nodes": {"127.0.0.1:6379": 1},
+                        "type": "roundrobin"
+                    }
+                }]]
+            )
+            if code ~= 400 then
+                ngx.say("failed: expected 400, got ", code)
+                return
+            end
+            local data = json.decode(body)
+            if not data or not data.error_msg then
+                ngx.say("failed: unexpected body: ", body)
+                return
+            end
+            if not string.find(data.error_msg, "itself as superior_id", 1, true) then
+                ngx.say("failed: unexpected body: ", body)
+                return
+            end
+            ngx.say("passed")
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 9: force delete a referenced superior route (should succeed)
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code = t('/apisix/admin/stream_routes/20',
+                ngx.HTTP_PUT,
+                [[{
+                    "protocol": {"name": "redis"},
+                    "upstream": {"nodes": {"127.0.0.1:6379": 1}, "type": "roundrobin"}
+                }]]
+            )
+            if code >= 300 then
+                ngx.say("failed to create superior: ", code)
+                return
+            end
+            code = t('/apisix/admin/stream_routes/21',
+                ngx.HTTP_PUT,
+                [[{
+                    "protocol": {"name": "redis", "superior_id": "20"},
+                    "upstream": {"nodes": {"127.0.0.1:6380": 1}, "type": "roundrobin"}
+                }]]
+            )
+            if code >= 300 then
+                ngx.say("failed to create subordinate: ", code)
+                return
+            end
+            -- force bypasses the reference check
+            local dcode, body = t('/apisix/admin/stream_routes/20?force=true',
+                ngx.HTTP_DELETE
+            )
+            if dcode >= 300 then
+                ngx.say("failed to force delete: ", dcode, " ", body)
+                return
+            end
+            t('/apisix/admin/stream_routes/21', ngx.HTTP_DELETE)
+            ngx.say("passed")
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed