From cd108f809331273bb12afccea311c1585667861e Mon Sep 17 00:00:00 2001 From: JianFeeeee Date: Thu, 22 Oct 2026 23:27:51 +0800 Subject: [PATCH] =?UTF-8?q?fix(admin):=20requireToken=20=E5=8E=BB=E6=8E=89?= =?UTF-8?q?=20fail-open=20=E6=AD=BB=E5=88=86=E6=94=AF=EF=BC=8C=E7=A9=BA=20?= =?UTF-8?q?expected=20=E4=B8=80=E5=BE=8B=E6=8B=92=E7=BB=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 审计第 1 条的「另一半」。第 1 步把 history-server 的 requireToken 改成了 fail-closed，但 admin-server 的同类函数还留着原来的形状： if expected == "" { return true // 未配置 token 则不校验 } 它今天打不到（唯一调用点 requireAdminToken 开头有 adminToken != "" 守卫）， 但形状本身就是风险：任何人新写一行 requireToken(w, r, someOptionalToken) 就会把 fail-open 复活，且没有任何测试会报红 —— 第 1 步的静态护栏只钉了 history-server 一半仓库。 现在 admin-server 的 requireToken 与 history-server 完全对齐： - expected 为空 → 401 + JSON {"error": "...set NAVI_ADMIN_TOKEN / NAVI_SYNC_TOKEN..."}， 绝不 return true；拒绝信息带修复指引（全站 401 时运维最需要的就是这句）， 且不回显任何 token 值。 - 「没有 token 也允许访问」的决策只允许出现在明确语义的函数里 （requireAdminToken 的 allowUnauthenticated 分支），不再藏在通用校验函数里。 requireAdminToken 的三段判定顺序一行未动： ① 配了 adminToken → requireToken；② 未配 + allowUnauthenticated → 放行； ③ 其它 → 拒绝。security_test.go 的既有用例（TestAdminAPIFailsClosedByDefault、 TestAdminAPIRejectsWithoutTokenEvenIfFlagUnset 等）全部继续绿。 新增 admin-server/token_failclosed_test.go： 1. 直接调 requireToken(rec, req, "") 的四种请求形态 → 全部 false + 401 + JSON + 含修复指引 + 不回显 token； 2. 配了 token 时正常路径不变（无头/错 token/缺 Bearer 前缀/对 token）， 外加逐条断言 requireAdminToken 的三段顺序（含「配了 token 时不因 allowUnauthenticated 而放行错误 token」这条最容易改坏的）； 3. 静态护栏：admin-server 的 main.go / gateway.go / security_test.go 都不得再出现 「未配置 token 则不校验」或 `expected == "" { return true`，并正向断言 requireToken 函数体里有 StatusUnauthorized。 反向验证：把空 expected 分支改回 `return true` 后， TestRequireTokenRejectsEmptyExpected（4 个子用例全红）与 TestNoFailOpenShortCircuitAcrossServers 报红；恢复后 admin-server 全集 ok。 --- admin-server/main.go | 17 ++- admin-server/token_failclosed_test.go | 195 ++++++++++++++++++++++++++ 2 files changed, 211 insertions(+), 1 deletion(-) create mode 100644 admin-server/token_failclosed_test.go diff --git a/admin-server/main.go b/admin-server/main.go index 81e0d8c..376f489 100644 --- a/admin-server/main.go +++ b/admin-server/main.go @@ -313,9 +313,24 @@ func requireAdminToken(w http.ResponseWriter, r *http.Request) bool { return false } +// requireToken 校验 Bearer token（fail-closed）。 +// +// 历史背景：早期实现在 expected 为空时直接 return true，等于「没配 token 就没有任何 +// 鉴权」。今天它打不到（唯一调用点 requireAdminToken 有 adminToken != "" 守卫）， +// 但那个空分支就是审计第 1 条点名的形状：任何人新写一行 +// requireToken(w, r, someOptionalToken) 就会把 fail-open 复活。 +// +// 现在与 history-server 的 requireToken 完全对齐： +// - 配了 token → 必须带对的 Bearer token +// - expected 为空 → **一律拒绝**（401 + JSON），绝不 return true +// 放行「没有 token 也允许访问」的决策只允许出现在明确语义的函数里 +// （requireAdminToken 的 allowUnauthenticated 分支），不允许藏在通用校验函数里。 func requireToken(w http.ResponseWriter, r *http.Request, expected string) bool { if expected == "" { - return true // 未配置 token 则不校验 + jsonErr(w, http.StatusUnauthorized, + "API is locked: this endpoint requires a configured token "+ + "(set NAVI_ADMIN_TOKEN / NAVI_SYNC_TOKEN and send Authorization: Bearer )") + return false } auth := r.Header.Get("Authorization") if strings.HasPrefix(auth, "Bearer ") && strings.TrimPrefix(auth, "Bearer ") == expected { diff --git a/admin-server/token_failclosed_test.go b/admin-server/token_failclosed_test.go new file mode 100644 index 0000000..cfb12ac --- /dev/null +++ b/admin-server/token_failclosed_test.go @@ -0,0 +1,195 @@ +package main + +import ( + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" +) + +// 本文件把审计第 1 条的「另一半」钉死：admin-server 自己的 requireToken +// 也不能留着「expected 为空就放行」的死分支。 +// +// 现状是那个分支今天打不到（唯一调用点 requireAdminToken 有 adminToken != "" 守卫）， +// 但形状本身就是风险：任何人新写一行 requireToken(w, r, someOptionalToken) +// 就会把 fail-open 复活，而这里不会有任何测试报红。 +// +// 语义与 history-server 的 requireToken 对齐：空 expected 一律拒绝（401 + JSON）。 + +// 用例 1（反向验证的咬点）：直接调 requireToken(rec, req, "") → 必须 false 且 401。 +func TestRequireTokenRejectsEmptyExpected(t *testing.T) { + cases := []struct { + name string + header string + }{ + {"无 Authorization 头", ""}, + {"带任意 Bearer", "Bearer anything"}, + {"带空 Bearer", "Bearer "}, + {"带 raw token（无 Bearer 前缀）", "anything"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/api/gateway/routes", nil) + if tc.header != "" { + req.Header.Set("Authorization", tc.header) + } + rec := httptest.NewRecorder() + if requireToken(rec, req, "") { + t.Fatal("expected 为空时 requireToken 必须返回 false（fail-closed）；" + + "返回 true 意味着任何「可选 token」调用点都会变成免鉴权后门") + } + if rec.Code != http.StatusUnauthorized { + t.Fatalf("拒绝时应返回 401，实际 %d", rec.Code) + } + // 拒绝响应必须是 JSON（客户端/部署脚本靠它区分「打到错误服务」） + if ct := rec.Header().Get("Content-Type"); !strings.Contains(ct, "application/json") { + t.Fatalf("拒绝响应的 Content-Type 应为 application/json，实际 %q（body=%s）", + ct, rec.Body.String()) + } + body := rec.Body.String() + if !strings.Contains(body, `"error"`) { + t.Fatalf("拒绝响应应带 error 字段，实际: %s", body) + } + // 修复指引：告诉运维该配哪个变量（这正是「全站 401」时最需要的信息） + if !strings.Contains(body, "NAVI_ADMIN_TOKEN") && !strings.Contains(body, "NAVI_SYNC_TOKEN") { + t.Fatalf("拒绝响应应给出修复指引（提示配置 NAVI_ADMIN_TOKEN），实际: %s", body) + } + // 不得把 expected 的值（哪怕是空）或任何令牌内容回显 + if tc.header != "" && strings.Contains(body, strings.TrimPrefix(tc.header, "Bearer ")) && tc.header != "Bearer " { + t.Fatalf("拒绝响应不得回显客户端提交的 token 值，实际: %s", body) + } + }) + } +} + +// 用例 2：配了 adminToken 时正常路径不变 —— 对 token 放行、错 token 拒绝。 +// （改 fail-closed 不允许把这条路径改坏；security_test.go 里的同名意图用例也仍在跑。） +func TestRequireTokenNormalPathUnchanged(t *testing.T) { + const secret = "admin-secret-value" + oldAdmin := adminToken + adminToken = secret + t.Cleanup(func() { adminToken = oldAdmin }) + + cases := []struct { + name string + header string + want bool + }{ + {"无头", "", false}, + {"错误 token", "Bearer wrong", false}, + {"缺 Bearer 前缀", secret, false}, + {"正确 token", "Bearer " + secret, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/api/history", nil) + if tc.header != "" { + req.Header.Set("Authorization", tc.header) + } + rec := httptest.NewRecorder() + got := requireToken(rec, req, secret) + if got != tc.want { + t.Fatalf("requireToken = %v; want %v (status %d)", got, tc.want, rec.Code) + } + if !tc.want && rec.Code != http.StatusUnauthorized { + t.Fatalf("拒绝时应 401，实际 %d", rec.Code) + } + if !tc.want && strings.Contains(rec.Body.String(), secret) { + t.Fatalf("拒绝响应不得回显 token 值: %s", rec.Body.String()) + } + }) + } + + // requireAdminToken 的三段判定顺序一行都不能变： + // ① 配了 adminToken → 走 requireToken（不管 allowUnauthenticated） + // ② 没配 + allowUnauthenticated → 放行 + // ③ 其它 → 拒绝 + t.Run("顺序①：配了 token 时不因 allowUnauthenticated 而放行错误 token", func(t *testing.T) { + oldAllow := allowUnauthenticated + allowUnauthenticated = true + t.Cleanup(func() { allowUnauthenticated = oldAllow }) + + rec := httptest.NewRecorder() + if requireAdminToken(rec, httptest.NewRequest(http.MethodGet, "/api/history", nil)) { + t.Fatal("配了 adminToken 时必须校验：不能因为 allowUnauthenticated 就放行无 token 请求") + } + if rec.Code != http.StatusUnauthorized { + t.Fatalf("应 401，实际 %d", rec.Code) + } + }) + + t.Run("顺序②：无 token + allowUnauthenticated → 放行", func(t *testing.T) { + oldAdmin2, oldAllow2 := adminToken, allowUnauthenticated + adminToken, allowUnauthenticated = "", true + t.Cleanup(func() { adminToken, allowUnauthenticated = oldAdmin2, oldAllow2 }) + + rec := httptest.NewRecorder() + if !requireAdminToken(rec, httptest.NewRequest(http.MethodGet, "/api/history", nil)) { + t.Fatalf("显式允许匿名时应放行，实际 %d: %s", rec.Code, rec.Body.String()) + } + }) + + t.Run("顺序③：无 token + 不允许匿名 → 拒绝且带修复指引", func(t *testing.T) { + oldAdmin3, oldAllow3 := adminToken, allowUnauthenticated + adminToken, allowUnauthenticated = "", false + t.Cleanup(func() { adminToken, allowUnauthenticated = oldAdmin3, oldAllow3 }) + + rec := httptest.NewRecorder() + if requireAdminToken(rec, httptest.NewRequest(http.MethodGet, "/api/history", nil)) { + t.Fatal("无 token 且不允许匿名时必须拒绝") + } + if rec.Code != http.StatusUnauthorized { + t.Fatalf("应 401，实际 %d", rec.Code) + } + if !strings.Contains(rec.Body.String(), "NAVI_ADMIN_TOKEN") { + t.Fatalf("拒绝信息应提示配置 NAVI_ADMIN_TOKEN，实际: %s", rec.Body.String()) + } + }) +} + +// 用例 3：静态护栏 —— 两个模块的源码都不得再出现「未配置 token 则不校验」这一类 +// fail-open 注释/写法。第 1 步只在 history-server 里钉了这条，等于护栏只钉了一半仓库。 +func TestNoFailOpenShortCircuitAcrossServers(t *testing.T) { + // 被禁止的字串：注释形态与代码形态都算 + forbidden := []string{ + "未配置 token 则不校验", + "expected == \"\" {\n\t\treturn true", + } + targets := []string{ + "main.go", // admin-server 自己 + "gateway.go", // 同模块里也可能有人写 + "security_test.go", // 防止有人把护栏改成「允许放行」 + } + for _, f := range targets { + src, err := os.ReadFile(f) + if err != nil { + t.Fatalf("读取 %s 失败: %v", f, err) + } + for _, bad := range forbidden { + if strings.Contains(string(src), bad) { + t.Fatalf("%s 里仍有 fail-open 形态 %q：空 token 必须拒绝，不能 return true", f, bad) + } + } + } + + // 正例：requireToken 的空 expected 分支里必须有拒绝（401），而不是放行。 + src, err := os.ReadFile("main.go") + if err != nil { + t.Fatal(err) + } + i := strings.Index(string(src), "func requireToken(") + if i < 0 { + t.Fatal("main.go 里找不到 requireToken") + } + fn := string(src)[i:] + if j := strings.Index(fn, "\n}"); j > 0 { + fn = fn[:j] + } + if !strings.Contains(fn, "StatusUnauthorized") { + t.Fatalf("requireToken 必须在拒绝路径上写 401，实际函数体:\n%s", fn) + } + if strings.Contains(fn, "return true // 未配置") { + t.Fatalf("requireToken 里仍有「未配置就放行」的短路:\n%s", fn) + } +}