fix(admin): requireToken 去掉 fail-open 死分支，空 expected 一律拒绝
审计第 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。
This commit is contained in:
JianFeeeee
@ -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 <token>)")
|
||||
return false
|
||||
}
|
||||
auth := r.Header.Get("Authorization")
|
||||
if strings.HasPrefix(auth, "Bearer ") && strings.TrimPrefix(auth, "Bearer ") == expected {
|
||||
|
||||
195
admin-server/token_failclosed_test.go
Normal file
195
admin-server/token_failclosed_test.go
Normal file
@ -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)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user