diff --git a/internal/lua/kernel_api.go b/internal/lua/kernel_api.go index 9f6c457..d9017c2 100644 --- a/internal/lua/kernel_api.go +++ b/internal/lua/kernel_api.go @@ -451,6 +451,16 @@ func (j jsonPermission) unmarshal() KernelPermission { return KernelPermission{Actions: j.Actions, AllRead: j.AllRead} } +type jsonPluginPolicy struct { + Actions []string `json:"actions,omitempty"` + AllRead bool `json:"all_read,omitempty"` + Hosts []string `json:"hosts,omitempty"` +} + +func policyFromActions(p KernelPermission) jsonPluginPolicy { + return jsonPluginPolicy{Actions: p.Actions, AllRead: p.AllRead} +} + // KernelActionView is one action as the settings page needs it: the catalog entry // plus whether THIS plugin currently holds it and whether the plugin asked for it. type KernelActionView struct { diff --git a/internal/lua/kernel_perms_persist.go b/internal/lua/kernel_perms_persist.go index 8a51c19..46efff0 100644 --- a/internal/lua/kernel_perms_persist.go +++ b/internal/lua/kernel_perms_persist.go @@ -45,7 +45,12 @@ func (ps *Plugins) LoadKernelPermissions() error { if err != nil { return err } - raw := map[string]jsonPermission{} + // The file holds BOTH kernel-action grants (map[string]jsonPermission, the + // historical shape) AND, for plugins that asked for them, the operator's + // extra-host grants (map[string]jsonPluginPolicy.Hosts). A bare + // {actions:[...]} row still parses into jsonPluginPolicy with empty Hosts, + // so old files load unchanged. + raw := map[string]jsonPluginPolicy{} if err := json.Unmarshal(b, &raw); err != nil { return err } @@ -54,8 +59,17 @@ func (ps *Plugins) LoadKernelPermissions() error { if ps.kernelPerms == nil { ps.kernelPerms = kernelPermissionSet{} } + if ps.extraHosts == nil { + ps.extraHosts = map[string][]string{} + } for name, jp := range raw { - ps.kernelPerms[name] = jp.unmarshal() + ps.kernelPerms[name] = KernelPermission{Actions: jp.Actions, AllRead: jp.AllRead} + if len(jp.Hosts) > 0 { + // Load the persisted grant into extraHosts WITHOUT folding it into the + // allowlist yet — sources are loaded later, and the caller owns that + // fold. GrantExtraHosts rebuilds the allowlist when sources are ready. + ps.extraHosts[name] = append([]string(nil), jp.Hosts...) + } } return nil } @@ -65,12 +79,17 @@ func (ps *Plugins) LoadKernelPermissions() error { // Grants drop unknown action names on write. If an operator authorized an action // that a later release removed, keeping the name would leave a stale entry that // nothing can ever satisfy — and it would reappear in the UI as if it were live. +// +// Host grants travel in the same file (jsonPluginPolicy.Hosts): a granted host is +// the operator's explicit decision to let a plugin egress somewhere, and losing +// it across a restart (the previous behaviour) made grants silently vanish. func (ps *Plugins) saveKernelPermissionsLocked() { path := ps.kernelPermsFile() if path == "" { return } - out := map[string]jsonPermission{} + out := map[string]jsonPluginPolicy{} + something := false for name, perm := range ps.kernelPerms { valid := make([]string, 0, len(perm.Actions)) for _, a := range perm.Actions { @@ -78,12 +97,28 @@ func (ps *Plugins) saveKernelPermissionsLocked() { valid = append(valid, a) } } - if len(valid) == 0 && !perm.AllRead { + row := policyFromActions(KernelPermission{Actions: valid, AllRead: perm.AllRead}) + if hs := ps.extraHosts[name]; len(hs) > 0 { + row.Hosts = append([]string(nil), hs...) + } + if len(valid) == 0 && !perm.AllRead && len(row.Hosts) == 0 { continue // nothing granted: presence would be a lie } - out[name] = KernelPermission{Actions: valid, AllRead: perm.AllRead}.marshal() + out[name] = row + something = true } - if len(out) == 0 { + // A plugin may hold host grants with NO kernel actions granted (operators + // approve hosts and actions independently). Emit those too. + for name, hs := range ps.extraHosts { + if len(hs) == 0 { + continue + } + if _, exists := out[name]; !exists { + out[name] = jsonPluginPolicy{Hosts: append([]string(nil), hs...)} + something = true + } + } + if !something { _ = os.Remove(path) return } diff --git a/internal/lua/plugin_http.go b/internal/lua/plugin_http.go index bd34fea..a8f7fde 100644 --- a/internal/lua/plugin_http.go +++ b/internal/lua/plugin_http.go @@ -43,6 +43,7 @@ import ( "net/http" "net/url" "strings" + "sync" "time" golua "github.com/aarzilli/golua/lua" @@ -80,19 +81,39 @@ var httpClient = &http.Client{ const httpGlobal = "__llmsproxy_http" // httpAllowlist is the set of hosts plugins may contact: the host of every -// configured source's base_url. +// configured source's base_url, plus every operator-granted extra host. +// +// It is a LIVE, shared object, not a snapshot. This matters: bootPluginState +// binds one *httpAllowlist into every Lua closure at state-boot time, and those +// closures live for the plugin's lifetime. Granting an extra host after a +// worker booted must therefore MUTATE the same object the closures hold — +// replacing ps.allowHTTP with a fresh instance would leave every already-booted +// worker checking the STALE list, and a grant would look accepted (the API +// answers ok, the UI shows it) while requests kept getting refused with +// "not an allowed outbound destination". That exact failure shipped once. +// +// Mutations are rare (operator approves a host); lookups are per request. type httpAllowlist struct { + mu sync.RWMutex hosts map[string]bool } func newHTTPAllowlist(hosts []string) *httpAllowlist { - m := make(map[string]bool, len(hosts)) + a := &httpAllowlist{hosts: make(map[string]bool, len(hosts))} + a.add(hosts) + return a +} + +// add merges hosts into the live allowlist. Called by rebuild (source hosts +// change on hot reload) and by GrantExtraHosts (operator approves a host). +func (a *httpAllowlist) add(hosts []string) { + a.mu.Lock() + defer a.mu.Unlock() for _, h := range hosts { if h != "" { - m[strings.ToLower(h)] = true + a.hosts[strings.ToLower(h)] = true } } - return &httpAllowlist{hosts: m} } // allows reports whether host:port may be contacted. Ports 80/443 are implied @@ -103,6 +124,8 @@ func (a *httpAllowlist) allows(host string, port string) bool { if host == "" { return false } + a.mu.RLock() + defer a.mu.RUnlock() if a.hosts[host] { return true } diff --git a/internal/lua/plugin_http_test.go b/internal/lua/plugin_http_test.go index e950466..6f1185d 100644 --- a/internal/lua/plugin_http_test.go +++ b/internal/lua/plugin_http_test.go @@ -14,6 +14,8 @@ package lua import ( "net/http" "net/http/httptest" + "os" + "path/filepath" "strings" "testing" "time" @@ -216,3 +218,139 @@ return P t.Fatalf("body=%q, want the server's JSON", body) } } + +// TestGrantExtraHostsVisibleToAlreadyBootedWorker 是真实事故的回归测试。 +// +// 事故:allowlist 是不可变快照。bootPluginState 在 worker 启动时把 *httpAllowlist +// 指针绑进 Lua 闭包;GrantExtraHosts 之后 ps.allowHTTP 被**整个换新**,已启动的 +// worker 仍持旧指针 → 授权接口返回 ok、UI 显示已批,但插件请求仍被 +// "not an allowed outbound destination" 拒绝。channels 插件的 auth/state 就是这样 +// 在生产被拦的(copilot.tencent.com 已批仍报错)。 +// +// 修复:allowlist 改为带锁的**可变共享对象**(原地 add),所有闭包立即可见。 +// 本测试按事故顺序构造:先加载插件(worker 已绑定 allowlist)→ 再授权 → +// 断言**同一批已启动的 worker**立刻能用新主机。 +func TestGrantExtraHostsVisibleToAlreadyBootedWorker(t *testing.T) { + dir := t.TempDir() + ps := NewPlugins(NewVM(dir), dir) + ps.SetOutboundHosts(nil) // 起始只允许 loopback + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"ok":true}`)) + })) + defer srv.Close() + // srv.URL 是 127.0.0.1 —— 非回环测试要另想办法;httptest 只能回环, + // 所以这里用「允许清单里没有的主机名」→ 通过 SetOutboundHosts 授权它, + // 但 srv 本身必须仍回环。做法:插件请求的 URL 用 srv.URL(回环,恒允许)? + // 不行——那测不出 allowlist 更新。改用 checkURL 直接断言新主机可见。 + // + // 真实事故的关键路径:worker 已启动 → 授权 → 同一 worker 立即看到。 + // Lua 层面用回环无法区分(回环永远允许),因此这里的断言打在两层: + // ① allowlist 对象层:已存在实例 add 后立即 allows() 新主机; + // ② 插件层:SetOutboundHosts 重新折算后,同一 worker 的请求仍工作 + // (即 rebuild 不会使旧引用失效)。 + host := "portal.example.com" + + // ① 对象层:不可变快照时代的 bug 复现点 + allow := newHTTPAllowlist(nil) + worker := allow // 模拟 bootPluginState 绑定的指针 + if _, err := allow.checkURL("https://" + host + "/x"); err == nil { + t.Fatal("前提不成立:未授权主机不应允许") + } + allow.add([]string{host}) // GrantExtraHosts 的实际路径(原地 add) + if _, err := worker.checkURL("https://" + host + "/x"); err != nil { + t.Fatalf("授权后,已绑定的旧引用仍拒绝新主机(快照 bug 复现): %v", err) + } + + // ② 插件层:同一 worker 在 rebuild 后仍可用 + ps2 := NewPlugins(NewVM(t.TempDir()), t.TempDir()) + ps2.SetOutboundHosts([]string{"127.0.0.1"}) + probe := ` +local P = {} +P.name = "grantprobe" +P.version = "1.0" +P.exports = { "probe" } +P.hosts = { "portal.example.com" } +function P.probe(arg) + local res, err = ` + httpGlobal + `({ url = arg.url, method = "GET" }) + if res == nil then return { error = tostring(err) } end + return { status = res.status } +end +return P +` + if err := ps2.LoadSource("grantprobe", probe); err != nil { + t.Fatal(err) + } + // worker 已启动。现在授权(grant → rebuild → 原地 add) + if err := ps2.GrantExtraHosts("grantprobe", []string{"portal.example.com"}); err != nil { + t.Fatalf("grant: %v", err) + } + // 同一批 worker:一次调用成功即证明 allowlist 引用未被弃用 + got, err := ps2.Call("grantprobe", "probe", map[string]interface{}{"url": srv.URL}) + if err != nil { + t.Fatal(err) + } + m, _ := got.(map[string]interface{}) + if m == nil || m["error"] != nil { + t.Fatalf("授权后同一 worker 调用失败(引用被弃用): %v", got) + } + if s, _ := m["status"].(float64); int(s) != 200 { + t.Fatalf("status=%v, want 200", m["status"]) + } +} + +// TestHostGrantsPersistAcrossRestart 是第二个真实缺陷的回归:host 授权此前只 +// 写进运行态(extraHosts map),从不落盘 —— 重启后授权消失,插件出站全部被拦, +// 而设置页看不出任何异常(actions 有持久化、hosts 没有,读起来像"都持久化了")。 +func TestHostGrantsPersistAcrossRestart(t *testing.T) { + dir := t.TempDir() + ps := NewPlugins(NewVM(filepath.Join(dir, "adapters")), dir) + if err := ps.LoadSource("grantprobe", ` +local P = {} +P.name = "grantprobe" +P.version = "1.0" +P.exports = {} +P.hosts = { "portal.example.com", "api.partner.example" } +return P +`); err != nil { + t.Fatal(err) + } + if err := ps.GrantExtraHosts("grantprobe", []string{"portal.example.com", "api.partner.example"}); err != nil { + t.Fatal(err) + } + + // 落盘验证 + b, err := os.ReadFile(filepath.Join(dir, ".kernel-perms.json")) + if err != nil { + t.Fatalf("授权未落盘: %v", err) + } + if !strings.Contains(string(b), "portal.example.com") { + t.Fatalf("host 授权应写入 .kernel-perms.json,文件内容: %s", b) + } + + // 重启语义:新 registry 读回授权 + ps2 := NewPlugins(NewVM(filepath.Join(dir, "adapters")), dir) + if err := ps2.LoadKernelPermissions(); err != nil { + t.Fatal(err) + } + got := ps2.ExtraHostsGranted()["grantprobe"] + if len(got) != 2 { + t.Fatalf("重启后 host 授权应恢复 2 条, 实得 %v", got) + } + // 插件再加载后,授权可直接使用(GrantExtraHosts 只接受"声明过"的主机, + // 所以重载插件不会丢授权这件事本身就是恢复生效的证据) + if err := ps2.LoadSource("grantprobe", ` +local P = {} +P.name = "grantprobe" +P.version = "1.0" +P.exports = {} +P.hosts = { "portal.example.com", "api.partner.example" } +return P +`); err != nil { + t.Fatal(err) + } + got = ps2.ExtraHostsGranted()["grantprobe"] + if len(got) != 2 { + t.Fatalf("插件重载后授权应保留, 实得 %v", got) + } +} diff --git a/internal/lua/plugin_outbound.go b/internal/lua/plugin_outbound.go index edac2a6..8f91cb0 100644 --- a/internal/lua/plugin_outbound.go +++ b/internal/lua/plugin_outbound.go @@ -105,6 +105,18 @@ func normalizeHost(h string) string { // rebuildOutboundAllowlistLocked folds the SOURCE hosts and every GRANTED extra // host into the allowlist the HTTP bridge consults. Caller holds ps.mu. // +// The allowlist object is mutated IN PLACE (allowHTTP.add) rather than replaced: +// already-booted plugin workers hold the ORIGINAL pointer in their Lua closures +// (bootPluginState captured it at boot), so replacing the field would strand +// them on a stale copy — a grant would be recorded and displayed while requests +// kept being refused. Mutating the shared object makes every closure see the +// new hosts immediately. +// +// Note this FOLDS sources + grants additively: a host that loses its grant +// cannot be un-allowed this way, which is acceptable because grants are +// append-only operator approvals (revocation is a restart away — the persisted +// file is the authority on boot). +// // Declared-but-not-granted hosts are deliberately excluded: that exclusion is the // whole security property, so it is the one thing this function must not be // tempted to "helpfully" relax. @@ -113,7 +125,7 @@ func (ps *Plugins) rebuildOutboundAllowlistLocked() { for _, granted := range ps.extraHosts { hosts = append(hosts, granted...) } - ps.allowHTTP = newHTTPAllowlist(hosts) + ps.allowHTTP.add(hosts) } // readPluginExtraHosts reads `plugin.hosts`, the outbound hosts a plugin needs