Files
ModelRouter/internal/lua/plugin_access_test.go
JianFeeeee 360a2a8644 refactor(plugins): 统一授权词汇 —— AccessRule
## 问题

同一个问题「谁能看到 / 能调用什么」有两套规则、相反的默认值,并排住在两个文件里:

    audienceAllows(want, role):   "" / "all" → 全开      页面可见性
    HTTPHandler.allows(role,key): ""         → 仅 admin   注册端点

两个默认值各自都对:页面没人看得见就等于没渲染,页面不该被未声明的 audience
藏起来;而端点「忘了声明」绝不能等于对公网敞开。

危害不在默认值本身,在于**同一个问题有两套词汇**。运维配「谁能看 billing」和
「谁能调 /balance」得先学会两套规则,才能回答同一个问题。

## 改动

新增 internal/lua/plugin_access.go,把两套规则收敛成一个 AccessRule,
并把**仍然不同的那一点——默认值——写成数据而不是留成隐含**:

- AllowsCapability:空 = 拒绝(端点、函数)
- AllowsVisibility:空 = 继承(页面、元素)

语法也统一成一种,`ParseAccessRule` / `Rule` 双向:

    "admin" / "admin, user" / "@alice, @bob" / "user, @alice" / "public"

`@name` 是密钥、其余是角色,一��字符串表达得完规则编辑器要提供的全部选项。

## 抽取时差点弄丢一个真实语义

`"all"` 是既有插件文件和**已保存的运维 override** 里的写法。统一后的解析器
不认识它,于是一个已发布的「全开」授权静默变成了关闭 —— 已部署的
billing、quota 可见性和所有存量 override 都会受影响。

是既有的 TestAudienceAllows 抓到的,它当场变红。这正是抽取的实质收益:
两个函数并排住着的时候,没人会想到去动那个共用字符串。

`TestAllIsAnAliasForPublic` 把它写成新类型的性质,而不是留给碰巧跑到的那
个套件去守着。

## 顺手加的东西我又撤了

accessRuleFor 第一版把 roles 里写成 `@alice` 的项重分类成密钥,理由是
「这大概是个自然的笔误」。那是在重构里偷带行为变更:会把写了莫名其妙内容的
端点的可达范围悄悄放宽。已撤,注释里写明为什么这个笔误值得修是另一件事、
需要它自己的理由和判据。

## 判据

七条性质,每条一个默认/边界:capability 默认关、visibility 默认继承、
keys 替换而非追加、keys 空时未认证调用者不匹配、admin 恒通、
"all" 别名、Rule() 往返稳定、角色与密钥名大小写/空格无关。

变异自证四次,全部变红:keys 变 OR、capability 默认全开、visibility 默认
拒绝、丢掉 "all" 别名。

头两次跑变异时报「没抓到」,是注入脚本自己写坏了(NameError / 替换串不
匹配)—— 变异没生效不等于判据无力,重写后确认四条都能抓到。

## 一条被记录下来的意外

`TestRoleAndKeyUnionAdmitsBoth` 最初断言「同时列出 role 和 key 时两者都放行」。
debug 跑出来是相反的:keys 非空就整体替换掉 role,carol 被拒。

这个意外正是不把它放在注释里的理由。判据现在断言实际语义(替换),与既有
HTTPHandler.allows 一致,因此没有任何已部署行为改变;但它把「这是我们的选择」
写成了可执行的一句话,而不是默默沿用旧代码碰巧做的事。

Co-Authored-By: ModelRouter <noreply@modelrouter.dev>
2026-10-04 20:37:09 +08:00

153 lines
5.6 KiB
Go

package lua
import (
"strings"
"testing"
)
// The two functions this file replaces had OPPOSITE defaults:
//
// audienceAllows: "" / "all" → everybody
// HTTPHandler.allows: "" → admin ONLY
//
// Both are correct for their surface. The hazard is that they are two
// vocabularies for the same question, so a reader — or an operator — assumes
// the rule that is more familiar applies. AccessRule keeps one rule and states
// the default as data at each call site.
// TestCapabilityDefaultIsClosed: an endpoint that declares nobody must not be
// reachable. This is the property that makes "I forgot" fail safe.
func TestCapabilityDefaultIsClosed(t *testing.T) {
for _, tc := range []struct {
name string
rule string
}{
{"empty", ""},
{"nil-ish", " "},
} {
if ParseAccessRule(tc.rule).AllowsCapability("user", "alice") {
t.Errorf("%s: an undeclared capability is reachable by a user", tc.name)
}
if !ParseAccessRule(tc.rule).AllowsCapability("admin", "alice") {
t.Errorf("%s: admin locked out of a capability they must be able to inspect", tc.name)
}
}
}
// TestVisibilityDefaultIsInherits: the opposite default, on purpose. A page
// nobody can see is not rendered; an undeclared audience must not hide a page
// the author clearly meant to publish.
func TestVisibilityDefaultIsInherits(t *testing.T) {
if !ParseAccessRule("").AllowsVisibility("user") {
t.Error("an undeclared audience hides a page")
}
if ParseAccessRule("admin").AllowsVisibility("user") {
t.Error("an admin-only page was shown to a user")
}
}
// TestNamedKeysReplaceRolesNotAdd: the single most consequential rule, and the
// one an "OR" implementation gets wrong. "allow these two keys" must not also
// admit every user key.
func TestNamedKeysReplaceRolesNotAdd(t *testing.T) {
r := ParseAccessRule("@alice, @bob")
if !r.AllowsCapability("user", "alice") {
t.Error("a named key was not admitted")
}
if r.AllowsCapability("user", "carol") {
t.Error("a key list leaked to every other user — the rule must REPLACE, not extend")
}
if r.AllowsCapability("user", "") {
t.Error("an unauthenticated caller — no key name at all — matched a named key")
}
if !r.AllowsCapability("admin", "") {
t.Error("admin locked out; a typo in a key list would leave nobody able to diagnose it")
}
}
// TestRoleAndKeyUnionAdmitsNeitherSibling: what a rule naming BOTH a role and a
// key actually does.
//
// Written first as "both are admitted", which is the intuitive reading of a
// union — and the debug run showed the opposite: a non-empty key list REPLACES
// the role rule outright, so carol is denied even though "user" is listed.
//
// That surprise is exactly why the rule is named, commented and now tested. The
// question is whether the behaviour is the one we want; asserting it here
// documents the choice rather than quietly preserving whatever the old code
// happened to do. The behaviour matches the pre-existing HTTPHandler.allows, so
// nothing deployed changes.
func TestRoleAndKeyUnionAdmitsNeitherSibling(t *testing.T) {
r := ParseAccessRule("user, @alice")
if !r.AllowsCapability("user", "alice") {
t.Error("the named key was not admitted")
}
if r.AllowsCapability("user", "carol") {
t.Error("a listed role was honoured despite a non-empty key list; " +
"the key list must replace it, or a listed role silently widens " +
"access to every user key")
}
if r.AllowsCapability("user", "mallory") {
t.Error("an unrelated user was admitted")
}
}
// TestAllIsAnAliasForPublic guards a real regression this refactor nearly
// shipped. "all" is what existing plugin files and already-saved operator
// overrides contain; when the unified parser did not know it, a published grant
// silently became a closed one. The pre-existing TestAudienceAllows is what
// caught it — this test states it as a property of the new type so the
// compatibility requirement is not left to whichever suite happens to run.
func TestAllIsAnAliasForPublic(t *testing.T) {
r := ParseAccessRule("all")
if !r.Public {
t.Fatal(`"all" did not parse as public`)
}
if !r.AllowsCapability("user", "carol") {
t.Error(`"all" closed a grant that was published as open`)
}
// It must also render back to the form the editor understands, or the next
// save of an untouched field would rewrite it.
if !strings.Contains(strings.ToLower(r.Rule()), "public") {
t.Errorf("Rule() = %q, want it to render as public", r.Rule())
}
}
// TestRuleRoundTrips: the admin page fills its input from Rule(), so a grant
// that does not round-trip is one an operator cannot edit without translating.
func TestRuleRoundTrips(t *testing.T) {
for _, in := range []string{
"admin",
"admin, user",
"@alice",
"@alice, @bob",
"user, @alice",
"public",
"all",
} {
once := ParseAccessRule(in)
twice := ParseAccessRule(once.Rule())
if once.Rule() != twice.Rule() {
t.Errorf("%q -> %q -> %q, not stable", in, once.Rule(), twice.Rule())
}
if len(twice.Roles) != len(once.Roles) || len(twice.Keys) != len(once.Keys) {
t.Errorf("%q lost data across a round trip: %+v -> %+v", in, once, twice)
}
}
}
// TestRoleMatchingIsCaseAndSpaceInsensitive: grants arrive typed by humans into
// a text input, and "Admin" not matching "admin" would lock an admin out of
// their own plugin.
func TestRoleMatchingIsCaseAndSpaceInsensitive(t *testing.T) {
if !ParseAccessRule(" Admin , User ").AllowsCapability("admin", "x") {
t.Error("casing and spacing defeated a role match")
}
if ParseAccessRule("Admin").AllowsCapability("user", "x") {
t.Error("a user was admitted by a case variant")
}
if !ParseAccessRule(" @Alice ").AllowsCapability("user", "alice") {
t.Error("casing defeated a named-key match")
}
}