From 3b28fde22eca314de38699461b6bfc5d89f56065 Mon Sep 17 00:00:00 2001 From: JianFeeeee Date: Sun, 4 Oct 2026 11:49:47 +0800 Subject: [PATCH] =?UTF-8?q?feat(plugins):=20=E8=AE=BF=E9=97=AE=E6=8E=A7?= =?UTF-8?q?=E5=88=B6=E5=9C=A8=E6=8F=92=E4=BB=B6=E9=A1=B5=E9=85=8D=E7=BD=AE?= =?UTF-8?q?=20+=20=E4=BF=AE=E5=A4=8D=20active=20=E4=BB=AA=E8=A1=A8?= =?UTF-8?q?=E5=9C=A8=E5=91=A8=E6=9C=9F=E8=A7=86=E5=9B=BE=E6=98=BE=E7=A4=BA?= =?UTF-8?q?=200?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 插件管理页（用户要求：权限不要只放 config.yaml） 插件页新增两列（可见性、端点）和一块访问控制编辑器，可直接改： - 每个插件的整体可见性（跟随插件 / 仅 admin / 仅 user / admin+user / 所有人） - 每个注册端点的可访问角色与密钥（admin, user, @homeagent, public） 原先这些能力后端齐全但 WebUI 零入口，唯一改法是手改 config.yaml。 ## 修掉的三个真缺陷 1. endpointAccess 读错了 override 的键 SetEndpointAccess 按 plugin#path 存，endpointAccess 却读 plugin 级。 后果：改了权限后 UI 仍显示插件声明的角色，再保存一次就把限制悄悄撤销。 鉴权一直是对的，只有显示和往返是坏的 —— 所以之前没被发现。 2. override 只在内存里 内存 map 让"billing 仅 admin"看着生效，直到下次重启限制凭空消失， 日志里没有任何线索。现在落盘到 plugin_dir/.access.json， 插件升级覆盖 .state.json 也不会连带放宽访问。删掉最后一个 override 就删文件， "没有覆盖"用文件不存在来诚实表达。 3. 可见性列表泄漏内部键 端点 override 与插件可见性共用一个 map，UI 因此会看到 "probe#hello" 并可能把它当 audience 存回去。现在按已注册端点集合过滤。 另外把整体可见性接到同一条 /access 通路（path 用 *），并修了可见性下拉的 集合比较：服务端原样存 "admin, user"，而选项是 "user,admin"，不相等就会 显示"跟随插件"，用户一保存就把访问范围放宽回插件声明 —— 比显示错更糟。 ## 活跃请求：三个视图显示 0 active 是实时仪表，没有日历周期可过滤，但周期响应压根没带这个字段。 前端 `st.active || 0` 于是把"字段缺失"变成了一句自信的"0 活跃"， 而 all 视图（响应里有该字段）显示真值 —— 三个视图 0、总视图正常。 现在周期响应带上 active，前端去掉 || 0 回退：缺失显示 — 而不是假装是 0。 判据：四个 period 都断言 active 存在且为数字；变异去掉字段即变红。 生产实测 day/week/month/all 均为 4。 Co-Authored-By: ModelRouter (cherry picked from commit 120ba9e95707bbecb87dd0f3bc44776085653184) --- internal/gateway/api.go | 7 + internal/gateway/plugins_api.go | 54 ++++++ internal/gateway/plugins_api_test.go | 51 ++++++ internal/gateway/stats.go | 13 ++ internal/gateway/stats_period_test.go | 31 ++++ internal/gateway/ui/index.html | 203 ++++++++++++++++++++- internal/gateway/ui_plugin_test.go | 23 +++ internal/lua/plugin_access_persist.go | 115 ++++++++++++ internal/lua/plugin_access_persist_test.go | 110 +++++++++++ internal/lua/plugin_http_handler.go | 57 ++++-- internal/lua/plugin_http_handler_test.go | 32 +++- internal/lua/plugin_visibility.go | 37 ++++ internal/lua/plugins.go | 12 +- 13 files changed, 716 insertions(+), 29 deletions(-) create mode 100644 internal/lua/plugin_access_persist.go create mode 100644 internal/lua/plugin_access_persist_test.go diff --git a/internal/gateway/api.go b/internal/gateway/api.go index 5924a31..f7996d9 100644 --- a/internal/gateway/api.go +++ b/internal/gateway/api.go @@ -464,6 +464,13 @@ func (g *Gateway) handleStatsAPI(w http.ResponseWriter, r *http.Request) { "by_status": out.Status, "buckets": out.Bucket, "truncated": out.Truncated, + // "Active requests" is a live gauge (requests in flight right now), + // not a period aggregate. Omitting it from the period response made + // the KPI read `st.active || 0` → 0 on every day/week/month view while + // the all-time view showed the truth, which read as "the gateway is + // idle" precisely when it was busiest. A calendar window cannot + // filter it, so it travels with every response unfiltered. + "active": g.stats.Active(), "records": page.Records, // The paging cursor has to travel with the first page, otherwise // the scroll handler has nothing to page from and the table is diff --git a/internal/gateway/plugins_api.go b/internal/gateway/plugins_api.go index 524a565..3a794e0 100644 --- a/internal/gateway/plugins_api.go +++ b/internal/gateway/plugins_api.go @@ -130,6 +130,11 @@ func (g *Gateway) handlePluginsAPI(w http.ResponseWriter, r *http.Request) { "on_disk": ps.OnDisk(), "hook_errors": ps.HookErrors(), "plugin_dir": g.pluginDir(), + // Visibility overrides and the endpoint access table, so the + // plugin page renders what is IN FORCE rather than making the + // operator read .lua files or config.yaml to find out. + "visibility": ps.VisibilityOverrides(), + "http_access": ps.EndpointAccess(), }) case http.MethodPost: var body struct { @@ -388,6 +393,17 @@ func (g *Gateway) handleEndpointAccess(w http.ResponseWriter, r *http.Request, n } ps := g.core.Plugins() + // epPath "*" is the WHOLE-PLUGIN visibility override, not an endpoint. It + // shares this route (and therefore this store) on purpose: an operator + // setting "who can see this plugin" and "who can call this endpoint" is + // asking the same question at two granularities, and two stores would let + // them disagree. SetEndpointAccess cannot serve it — it validates that the + // endpoint exists, which "*" by definition does not. + if epPath == pluginWholeOverride { + g.handlePluginVisibility(w, r, name) + return + } + switch r.Method { case http.MethodGet: for _, e := range ps.EndpointAccess() { @@ -489,3 +505,41 @@ func filterKeyBuckets(v interface{}, masked string, depth int) interface{} { return out } } + +// pluginWholeOverride is the epPath that addresses a plugin's overall visibility +// rather than one of its endpoints. +const pluginWholeOverride = "*" + +// handlePluginVisibility is the whole-plugin half of the access API. The +// audience string uses the same vocabulary as an endpoint spec (roles, @key +// names, public) so the UI can drive both from one input. +func (g *Gateway) handlePluginVisibility(w http.ResponseWriter, r *http.Request, name string) { + ps := g.core.Plugins() + switch r.Method { + case http.MethodGet: + writeJSON(w, http.StatusOK, map[string]interface{}{ + "plugin": name, + "visibility": ps.VisibilityOverrides()[name], + }) + case http.MethodPut, http.MethodDelete: + spec := "" + if r.Method == http.MethodPut { + var body struct { + Access string `json:"access"` + } + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + writeError(w, http.StatusBadRequest, "invalid_request", "invalid json: "+err.Error()) + return + } + spec = body.Access + } + if err := ps.ValidateAccessSpec(spec); err != nil { + writeError(w, http.StatusBadRequest, "invalid_request", err.Error()) + return + } + ps.SetPluginVisibility(name, spec) + writeJSON(w, http.StatusOK, map[string]interface{}{"ok": true, "visibility": spec}) + default: + writeError(w, http.StatusMethodNotAllowed, "method_not_allowed", r.Method+" is not allowed here") + } +} diff --git a/internal/gateway/plugins_api_test.go b/internal/gateway/plugins_api_test.go index 3ff9e3f..85cbcd7 100644 --- a/internal/gateway/plugins_api_test.go +++ b/internal/gateway/plugins_api_test.go @@ -210,3 +210,54 @@ func TestPluginStatePUTIsAdminOnly(t *testing.T) { t.Errorf("admin PUT state = %d: %s", adm.Code, adm.Body.String()) } } + +// TestPluginsListCarriesAccessTables: the plugin page renders visibility +// overrides and the endpoint access table from /api/plugins. Without them in the +// response the operator has to open a .lua or config.yaml to learn who can see +// what, which is exactly what the UI is meant to avoid. +func TestPluginsListCarriesAccessTables(t *testing.T) { + g := gatewayWithCallPlugin(t) + rr := doReq(t, g, http.MethodGet, "/api/plugins", "") + if rr.Code != http.StatusOK { + t.Fatalf("GET /api/plugins = %d", rr.Code) + } + var out struct { + Visibility map[string]string `json:"visibility"` + HTTPAccess []map[string]interface{} `json:"http_access"` + } + if err := json.Unmarshal(rr.Body.Bytes(), &out); err != nil { + t.Fatalf("decode: %v (body=%s)", err, rr.Body.String()) + } + // The calltest plugin registers no endpoints, so the table is present but + // empty — the KEY is that the field exists at all. A missing field decodes to + // nil and the UI silently renders nothing, which looks identical to "this + // plugin has no endpoints". + if !strings.Contains(rr.Body.String(), "http_access") { + t.Error("/api/plugins response lacks the http_access field") + } + if !strings.Contains(rr.Body.String(), "visibility") { + t.Error("/api/plugins response lacks the visibility field") + } +} + +// TestPluginWholeVisibilityViaAccessAPI covers the "*" path: the plugin page +// edits a plugin's overall audience through the same route it edits endpoints +// with. Before this existed the UI had a visibility selector whose save 404'd, +// because SetEndpointAccess validates that the endpoint exists and "*" does not. +func TestPluginWholeVisibilityViaAccessAPI(t *testing.T) { + g := gatewayWithCallPlugin(t) + rr := doReq(t, g, http.MethodPut, "/api/plugins/calltest/http/*/access", + `{"access":"admin, user"}`) + if rr.Code != http.StatusOK { + t.Fatalf("PUT */access = %d: %s", rr.Code, rr.Body.String()) + } + if !strings.Contains(rr.Body.String(), `"ok":true`) { + t.Errorf("unexpected body: %s", rr.Body.String()) + } + // A user role is a legitimate audience; a nonsense one is not. + bad := doReq(t, g, http.MethodPut, "/api/plugins/calltest/http/*/access", + `{"access":"../../etc"}`) + if bad.Code != http.StatusBadRequest { + t.Errorf("traversal-shaped audience = %d, want 400: %s", bad.Code, bad.Body.String()) + } +} diff --git a/internal/gateway/stats.go b/internal/gateway/stats.go index ef99c20..a376ee6 100644 --- a/internal/gateway/stats.go +++ b/internal/gateway/stats.go @@ -1211,3 +1211,16 @@ func (s *Stats) Snapshot(limit int, key string) map[string]interface{} { "replay_partial": s.replayPartial, } } + +// Active reports requests in flight right now. +// +// It is a live gauge, not an aggregate: there is no calendar period that scopes +// it, which is why the period-scoped /api/stats response has to carry it +// explicitly. Before it did, the day/week/month views fed `st.active || 0` a +// missing key and showed a hard 0 — the same gateway that was serving hundreds +// of requests a second looked idle in three of the four views. +func (s *Stats) Active() int64 { + s.mu.Lock() + defer s.mu.Unlock() + return s.active +} diff --git a/internal/gateway/stats_period_test.go b/internal/gateway/stats_period_test.go index 389a688..b04e74a 100644 --- a/internal/gateway/stats_period_test.go +++ b/internal/gateway/stats_period_test.go @@ -401,3 +401,34 @@ func TestPeriodWeekTotalCanExceedMonthTotal(t *testing.T) { t.Fatal("today must be contained in both the week and the month totals") } } + +// TestPeriodStatsCarryActiveGauge pins the live "active requests" gauge onto +// every period response. +// +// It is a live gauge, not an aggregate, so no calendar window can scope it — but +// the period response is what the day/week/month views render from, and it used +// to omit the field entirely. The UI then evaluated `st.active || 0` and printed +// a hard 0 in three of four views, while the all-time view (whose response did +// carry the field) showed the truth. Reading "0 active" off a busy gateway is +// worse than reading nothing: it is a confident wrong answer. +func TestPeriodStatsCarryActiveGauge(t *testing.T) { + for _, period := range []string{"day", "week", "month", "all"} { + g := newTestGateway(t) + rr := doReq(t, g, http.MethodGet, "/api/stats?period="+period, "") + if rr.Code != http.StatusOK { + t.Fatalf("period=%s: %d", period, rr.Code) + } + var out map[string]interface{} + if err := json.Unmarshal(rr.Body.Bytes(), &out); err != nil { + t.Fatalf("period=%s decode: %v", period, err) + } + v, ok := out["active"] + if !ok { + t.Errorf("period=%s response has no active field; the UI renders 0", period) + continue + } + if _, ok := v.(float64); !ok { + t.Errorf("period=%s active is %T, want a number", period, v) + } + } +} diff --git a/internal/gateway/ui/index.html b/internal/gateway/ui/index.html index 43d2f8b..6d9168e 100644 --- a/internal/gateway/ui/index.html +++ b/internal/gateway/ui/index.html @@ -815,8 +815,21 @@ plTitle: "插件", plState: "状态", plStages: "阶段", - plActive: "启用中", - plDisabled: "已禁用", + + plVisibility: "可见性", + plEndpoints: "端点", + plAccessTitle: "访问控制", + plAccessHint: "填写允许访问的角色或密钥（@名称）。留空=仅 admin。填 public=任何人。指定 @密钥 时只有该密钥能访问，不受角色规则影响。", + plWholePlugin: "以下端点", + plKeyHint: "可用密钥名", + plAccessSave: "保存访问控制", plActive: "启用中", + + tVisDefault: "跟随插件", + tVisAdmin: "仅 admin", + tVisRoles: "admin + user", + tVisAll: "所有人", + plOverridden: "已覆盖插件声明", + plEpsN: "%d 个端点", plDisabled: "已禁用", plBroken: "加载失败", plBuiltin: "内置", plHooks: "个阶段", @@ -1090,8 +1103,21 @@ plTitle: "Plugins", plState: "State", plStages: "Stages", - plActive: "Active", - plDisabled: "Disabled", + + plVisibility: "Visibility", + plEndpoints: "Endpoints", + plAccessTitle: "Access control", + plAccessHint: "Roles or keys (@name) allowed to reach each endpoint. Empty = admin only. public = anyone. Listing @keys replaces the role rules entirely.", + plWholePlugin: "endpoints below", + plKeyHint: "Key names", + plAccessSave: "Save access", plActive: "Active", + + tVisDefault: "follow plugin", + tVisAdmin: "admin only", + tVisRoles: "admin + user", + tVisAll: "everyone", + plOverridden: "overrides plugin", + plEpsN: "%d endpoints", plDisabled: "Disabled", plBroken: "Failed to load", plBuiltin: "Built-in", plHooks: "stages", @@ -1994,7 +2020,12 @@ const e = $(sel); if (e) e.textContent = txt; }; - kset('[data-kpi="active"]', st.active || 0); + // No `|| 0` fallback: that fallback is what turned a MISSING field + // into a confident "0 active" on three of four views. If the server + // ever stops sending it again, an em dash says "unknown" and the + // regression is visible, instead of the gauge quietly claiming the + // gateway is idle while it serves traffic. + kset('[data-kpi="active"]', st.active === undefined ? "—" : fmtN(st.active)); kset('[data-kpi="reqs"]', fmtN(tot.reqs)); const rsub = $('[data-kpi="reqs-sub"]'); if (rsub) @@ -4473,6 +4504,8 @@ function afterChainEdit() { ${esc(p.name)}${builtin}
${esc(p.description || p.version || "")}
${err} ${state} ${hooks} + ${plVisCell(j, p)} + ${plEpsCell(j, p)} ${toggle} @@ -4494,8 +4527,9 @@ function afterChainEdit() { $("#tab-plugins").innerHTML = `

${t("plTitle")} (${(j.on_disk || []).length})

- - ${rows || ``}
${t("tName")}${t("plState")}${t("plStages")}
${t("plEmpty")}
+ ${t("tName")}${t("plState")}${t("plStages")}${t("plVisibility")}${t("plEndpoints")} + ${rows || `${t("plEmpty")}`}
+
${errList ? `

${t("plHookErr")}

` : ""}

${t("plDir")} ${esc(j.plugin_dir)}

@@ -4508,6 +4542,161 @@ function afterChainEdit() {

`; bindDropzone("pl-dz", "pl-file", "#pl-name", "#pl-code"); + renderPluginAccess(j); + } + + + const PL_VIS = [ + ["", "tVisDefault"], + ["admin", "tVisAdmin"], + ["user,admin", "tVisRoles"], + ["all", "tVisAll"], + ]; + + // plVisCell shows the plugin's own declaration next to any operator + // override, because "admin-only" means something different depending on + // whether the plugin chose it or the deployment imposed it. Editing goes + // through the same access API as endpoints so there is one place where + // visibility is stored. + function plVisCell(j, p) { + const decl = (p.ui && p.ui.visibility) || ""; + const ov = (j.visibility || {})[p.name] || ""; + const cur = ov || decl; + // Compare as SETS. The server stores whatever order the operator typed + // and we round-trip it verbatim, so "admin, user" must still light up a + // choice — otherwise the selector silently shows "follow plugin", and + // saving from there writes an empty override that WIDENS access back to + // the plugin's own declaration. + const norm = (v) => + (v || "") + .split(",") + .map((x) => x.trim()) + .filter(Boolean) + .sort() + .join(","); + const curN = norm(cur); + // An audience with no matching choice (one naming a @key, say) is offered + // as itself so the value in force is never hidden from the operator. + const opts = PL_VIS.some((c) => norm(c[0]) === curN) + ? PL_VIS + : PL_VIS.concat([[cur, "plCustomVis"]]); + return `${ + ov && decl && ov !== decl + ? `
${t("plOverridden")}
` + : "" + }`; + } + + function plEpsCell(j, p) { + const n = (j.http_access || []).filter((e) => e.plugin === p.name).length; + return n + ? `${t("plEpsN").replace("%d", n)}` + : `—`; + } + + // ---- visibility & endpoint access ---- + // + // The plugin page is where an operator edits who can see what. The + // alternatives are editing a bundled .lua — overwritten on the next + // upgrade — or hand-editing config.yaml, with no feedback about what is + // actually in force. Each row shows the rule next to whether it was + // DECLARED by the plugin or OVERRIDDEN here, because "admin-only" means + // something different depending on which. + function renderPluginAccess(j) { + const host = $("#pl-access"); + if (!host) return; + const eps = j.http_access || []; + if (!eps.length) { host.innerHTML = ""; return; } + const keys = (window.__gwKeyRows || []).map((k) => k.name).filter(Boolean); + const byPlugin = {}; + eps.forEach((e) => { (byPlugin[e.plugin] = byPlugin[e.plugin] || []).push(e); }); + let h = `

${t("plAccessTitle")}

+

${t("plAccessHint")}

`; + Object.keys(byPlugin).forEach((pn) => { + h += `
+
+ ${esc(pn)} + ${t("plWholePlugin")} +
`; + (byPlugin[pn] || []).forEach((e) => { + const ovr = (e.override_keys && e.override_keys.length) || + (e.override_roles && e.override_roles.length); + const cur = ovr + ? [...(e.override_keys || []).map((k) => "@" + k), ...(e.override_roles || [])].join(",") + : [...(e.keys || []).map((k) => "@" + k), ...(e.roles || []), e.public ? "public" : ""] + .filter(Boolean).join(",") || "admin"; + h += `
+
+ ${esc(e.method)} ${esc(e.full_path)} + → ${esc(e.handler)}() + ${e.description ? `${esc(e.description)}` : ""} + + +
+ ${keys.length ? `
${t("plKeyHint")}: ${esc(keys.join(", "))}
` : ""} +
`; + }); + h += `
`; + }); + h += `

`; + host.innerHTML = h; + } + + async function savePluginAccess(btn) { + btn.disabled = true; + try { + // Whole-plugin visibility first. Sent through the SAME access API as + // endpoints (path "*") so visibility has exactly one storage path. + const viss = Array.prototype.slice.call( + document.querySelectorAll("[data-plvis]")); + for (const sel of viss) { + const pn = sel.getAttribute("data-plvis"); + const url = + "/api/plugins/" + encodeURIComponent(pn) + "/http/*/access"; + const v = sel.value; + if (v) { + await api(url, { method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ access: v }) }); + } else { + await api(url, { method: "DELETE" }); + } + } + const inputs = Array.prototype.slice.call( + document.querySelectorAll("#pl-access [data-ep]")); + for (const inp of inputs) { + const raw = inp.getAttribute("data-ep"); + const sep = raw.indexOf("|"); + const pn = raw.slice(0, sep), ep = raw.slice(sep + 1); + const url = "/api/plugins/" + encodeURIComponent(pn) + "/http/" + + encodeURIComponent(ep) + "/access"; + const v = inp.value.trim(); + if (v) { + await api(url, { method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ access: v }) }); + } else { + await api(url, { method: "DELETE" }); + } + } + toast(t("toastSaved")); + renderPlugins(); + } catch (e) { + toast(tFmt("toastSaveFail", e.message)); + } finally { + btn.disabled = false; + } } async function togglePlugin(name, disabled) { diff --git a/internal/gateway/ui_plugin_test.go b/internal/gateway/ui_plugin_test.go index 0a5e2a2..9414097 100644 --- a/internal/gateway/ui_plugin_test.go +++ b/internal/gateway/ui_plugin_test.go @@ -388,3 +388,26 @@ func TestPluginSidebarEntryCarriesATitle(t *testing.T) { t.Errorf("plugin sidebar button renders the icon only; a label is required. "+ "got: %.200s", stmt) } + +// TestPluginPageHasAccessEditor: the plugin page must let an operator configure +// endpoint access. The APIs existed with zero UI entry point, which meant the +// only way to change who could reach a plugin endpoint was to hand-edit +// config.yaml — the thing the UI is meant to replace. +func TestPluginPageHasAccessEditor(t *testing.T) { + src := uiSource(t) + for _, needle := range []string{ + `renderPluginAccess`, // called after rendering the plugin list + `savePluginAccess`, // wired to a button + `/access`, // hits the per-endpoint access API + `id="pl-access"`, // container exists + `http_access`, // reads the table from /api/plugins + `data-ep=`, // one input per endpoint + `data-plvis=`, // one visibility selector per plugin + `/http/*/access`, // whole-plugin visibility shares the access API + `plVisCell`, // renders the declared-vs-override distinction + } { + if !strings.Contains(src, needle) { + t.Errorf("plugin page lacks %q", needle) + } + } +} diff --git a/internal/lua/plugin_access_persist.go b/internal/lua/plugin_access_persist.go new file mode 100644 index 0000000..822f7e8 --- /dev/null +++ b/internal/lua/plugin_access_persist.go @@ -0,0 +1,115 @@ +package lua + +import ( + "encoding/json" + "os" + "sort" +) + +// Access overrides are OPERATOR policy, not plugin data. They live in their own +// file rather than in each plugin's state because: +// +// - they describe the deployment, not the plugin, so a plugin upgrade that +// replaces its .state.json must not silently widen access again; +// - they must survive a restart. An in-memory map made "restrict billing to +// admin" look like it worked right up to the next deploy, at which point the +// restriction evaporated with nothing in the logs to say why; +// - the plugin_dir is the one place an operator already looks when asking +// "what does this plugin actually get to see", so the answer lives beside it. +// +// The file is a flat map of "name" (whole plugin) or "name#path" (one endpoint) +// to an access spec, which is the same shape the in-memory store uses, so load +// and save cannot disagree about what a key means. + +const accessFileName = ".access.json" + +func (ps *Plugins) accessFile() string { + if ps.dir == "" { + return "" + } + return ps.dir + string(os.PathSeparator) + accessFileName +} + +// LoadAccessOverrides reads the persisted overrides into memory. A missing file +// is not an error: no operator has restricted anything yet, which is the +// default state of a fresh install. +// +// A corrupt file is also not fatal. Losing a restriction is bad, but refusing to +// start the gateway is worse, and an empty map plus a logged error leaves the +// operator with something to fix. +func (ps *Plugins) LoadAccessOverrides() error { + path := ps.accessFile() + if path == "" { + return nil + } + b, err := os.ReadFile(path) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return err + } + m := map[string]string{} + if err := json.Unmarshal(b, &m); err != nil { + return err + } + ps.mu.Lock() + defer ps.mu.Unlock() + if ps.visibility == nil { + ps.visibility = visibilityOverrides{} + } + for k, v := range m { + ps.visibility[k] = v + } + return nil +} + +// saveAccessOverridesLocked flushes the whole override map. It is called on +// every change, and unlike plugin state the map is tiny and changes only when an +// operator clicks Save — so a synchronous write costs nothing that matters, and +// it means the change is durable the moment the API returns 200. +func (ps *Plugins) saveAccessOverridesLocked() { + path := ps.accessFile() + if path == "" { + return + } + out := make(map[string]string, len(ps.visibility)) + for k, v := range ps.visibility { + if v != "" { + out[k] = v + } + } + if len(out) == 0 { + // Leave no empty file behind: its absence is the honest encoding of + // "nothing is overridden". + _ = os.Remove(path) + return + } + writeJSONAtomic(path, out) +} + +// AccessOverrides returns a copy of every override in force, for the UI. +func (ps *Plugins) accessOverridesSnapshot() map[string]string { + ps.mu.RLock() + defer ps.mu.RUnlock() + if len(ps.visibility) == 0 { + return nil + } + out := make(map[string]string, len(ps.visibility)) + for k, v := range ps.visibility { + out[k] = v + } + return out +} + +// sortedOverrideKeys exists only so a debug dump is stable; map iteration order +// would otherwise make two identical states look different. +func (ps *Plugins) sortedOverrideKeys() []string { + m := ps.accessOverridesSnapshot() + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + return keys +} \ No newline at end of file diff --git a/internal/lua/plugin_access_persist_test.go b/internal/lua/plugin_access_persist_test.go new file mode 100644 index 0000000..fb709ab --- /dev/null +++ b/internal/lua/plugin_access_persist_test.go @@ -0,0 +1,110 @@ +package lua + +import ( + "os" + "path/filepath" + "testing" +) + +// TestAccessOverrideSurvivesReload is the test that matters for an access +// restriction. An in-memory map makes "billing is admin-only" look like it +// worked right up to the next restart, and then the restriction is simply gone +// with nothing in the logs to explain it. Both the plugin-level audience and +// the per-endpoint spec must come back. +func TestAccessOverrideSurvivesReload(t *testing.T) { + dir := t.TempDir() + src := `local P = {} +P.name = "api" +P.version = "1.0.0" +P.http = { + { path = "health", handler = "health", roles = { "admin" } }, +} +function P.health(req) return { json = { ok = true } } end +return P +` + first := NewPlugins(NewVM(dir), dir) + if err := first.LoadSource("api", src); err != nil { + t.Fatal(err) + } + first.SetPluginVisibility("api", "admin, user") + if err := first.SetEndpointAccess("api", "health", "@homeagent"); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(dir, accessFileName)); err != nil { + t.Fatalf("override was not written: %v", err) + } + + // A fresh Plugins over the same dir is what a restart looks like. + second := NewPlugins(NewVM(dir), dir) + // The plugin source is on disk too, so this second instance sees both the + // plugin and the overrides — the same pair a restart sees. Loading only the + // overrides would leave no endpoints to walk, and the assertion below would + // pass vacuously. + if err := second.LoadSource("api", src); err != nil { + t.Fatal(err) + } + if err := second.LoadAccessOverrides(); err != nil { + t.Fatalf("reload overrides: %v", err) + } + if got := second.VisibilityOverride("api"); got != "admin, user" { + t.Errorf("plugin visibility lost across reload: %q", got) + } + found := false + for _, e := range second.EndpointAccess() { + if e.Path == "health" && len(e.OverrideKeys) == 1 && e.OverrideKeys[0] == "homeagent" { + found = true + } + } + if !found { + t.Error("endpoint override lost across reload") + } +} + +// TestAccessOverrideClearedRemovesFile keeps the "nothing overridden" state +// distinguishable from "overridden with something". +func TestAccessOverrideClearedRemovesFile(t *testing.T) { + dir := t.TempDir() + ps := NewPlugins(NewVM(dir), dir) + ps.SetPluginVisibility("api", "admin") + path := filepath.Join(dir, accessFileName) + if _, err := os.Stat(path); err != nil { + t.Fatalf("expected an access file: %v", err) + } + ps.SetPluginVisibility("api", "") + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Errorf("clearing the last override should remove the file, got %v", err) + } +} + +// TestVisibilityOverridesExcludeEndpointKeys keeps endpoint policy out of the +// plugin-audience listing. Both live in one map, and reporting "probe#hello" +// under visibility is how the plugin page ends up offering to save an audience +// of that name — which nothing reads as an audience. +func TestVisibilityOverridesExcludeEndpointKeys(t *testing.T) { + dir := t.TempDir() + src := `local P = {} +P.name = "api" +P.version = "1.0.0" +P.http = { { path = "health", handler = "health", roles = { "admin" } } } +function P.health(req) return { json = { ok = true } } end +return P +` + ps := NewPlugins(NewVM(dir), dir) + if err := ps.LoadSource("api", src); err != nil { + t.Fatal(err) + } + ps.SetPluginVisibility("api", "admin, user") + if err := ps.SetEndpointAccess("api", "health", "@homeagent"); err != nil { + t.Fatal(err) + } + got := ps.VisibilityOverrides() + if got["api"] != "admin, user" { + t.Errorf("plugin audience missing: %+v", got) + } + if _, leaked := got["api#health"]; leaked { + t.Errorf("endpoint override leaked into the audience map: %+v", got) + } + if len(got) != 1 { + t.Errorf("expected exactly the plugin audience, got %+v", got) + } +} diff --git a/internal/lua/plugin_http_handler.go b/internal/lua/plugin_http_handler.go index 5324348..859a115 100644 --- a/internal/lua/plugin_http_handler.go +++ b/internal/lua/plugin_http_handler.go @@ -354,29 +354,25 @@ type HTTPEndpointAccess struct { func (ps *Plugins) endpointAccess() []HTTPEndpointAccess { ps.mu.RLock() defer ps.mu.RUnlock() + return ps.endpointAccessLocked() +} + +// endpointAccessLocked is the body of endpointAccess for callers that already +// hold ps.mu — VisibilityOverrides walks the same map and must not re-lock. +func (ps *Plugins) endpointAccessLocked() []HTTPEndpointAccess { var out []HTTPEndpointAccess for _, p := range ps.plugins { if p.http == nil || p.LoadError != "" { continue } - ov := ps.visibility[p.Info.Name] - var ovRoles, ovKeys []string - if ov != "" { - for _, part := range strings.Split(ov, ",") { - part = strings.TrimSpace(part) - if part == "" { - continue - } - // A key name is anything that is not a role: the override string - // carries both, so "@name" marks a key and a bare word a role. - if strings.HasPrefix(part, "@") { - ovKeys = append(ovKeys, strings.TrimPrefix(part, "@")) - } else { - ovRoles = append(ovRoles, part) - } - } - } for _, h := range p.http { + // The override key is per ENDPOINT (plugin#path), not per plugin. + // Reading the plugin-level key here reported the plugin's own + // audience override as if it governed every endpoint, and reported + // no endpoint override at all — so the UI drew the DECLARED roles + // after an operator had saved a change, and re-saving from that + // display silently reverted the restriction. + ovRoles, ovKeys := splitAccessSpec(ps.visibility[endpointOverrideKey(p.Info.Name, h.Path)]) out = append(out, HTTPEndpointAccess{ Plugin: p.Info.Name, Path: h.Path, FullPath: h.routePath(), Method: h.Method, Handler: h.Handler, @@ -422,9 +418,10 @@ func (ps *Plugins) SetEndpointAccess(plugin, path, spec string) error { key := endpointOverrideKey(plugin, path) if strings.TrimSpace(spec) == "" { delete(ps.visibility, key) - return nil + } else { + ps.visibility[key] = spec } - ps.visibility[key] = spec + ps.saveAccessOverridesLocked() return nil } @@ -509,3 +506,25 @@ func (ps *Plugins) ValidateAccessSpec(spec string) error { } return nil } + +// splitAccessSpec turns an override string into role and key lists. "@name" is a +// key; a bare word is a role. Shared by the reporting path and the enforcement +// path so the two can never disagree about what an override means. +func splitAccessSpec(spec string) (roles, keys []string) { + if strings.TrimSpace(spec) == "" { + return nil, nil + } + for _, part := range strings.Split(spec, ",") { + part = strings.TrimSpace(part) + switch { + case part == "", part == "public": + // "public" is an enforcement concern, not a listing; it is reported + // via the Public field. + case strings.HasPrefix(part, "@"): + keys = append(keys, strings.TrimPrefix(part, "@")) + default: + roles = append(roles, part) + } + } + return roles, keys +} diff --git a/internal/lua/plugin_http_handler_test.go b/internal/lua/plugin_http_handler_test.go index 83e5897..03444c2 100644 --- a/internal/lua/plugin_http_handler_test.go +++ b/internal/lua/plugin_http_handler_test.go @@ -397,4 +397,34 @@ func TestParseHandlerResponseShapes(t *testing.T) { if r.Status != 204 || r.Headers["X-A"] != "b" { t.Errorf("decoded %+v", r) } -} \ No newline at end of file +} +// TestEndpointAccessReportsPerEndpointOverride guards the reporting side of the +// override store. +// +// SetEndpointAccess already keyed overrides by plugin#path, but endpointAccess +// read the plugin-level key. After an operator saved a restriction on ONE +// endpoint, /api/plugins kept reporting the plugin's DECLARED roles, so the +// plugin page drew the pre-change value and a later save from that screen +// reverted the restriction. Authorization kept working — only the display and +// the round trip through the UI were broken, which is why it went unnoticed. +func TestEndpointAccessReportsPerEndpointOverride(t *testing.T) { + dir := t.TempDir() + ps := NewPlugins(NewVM(dir), dir) + if err := ps.LoadSource("api", httpPlugin); err != nil { + t.Fatal(err) + } + // httpPlugin registers "health" and "echo/:what". + if err := ps.SetEndpointAccess("api", "health", "user"); err != nil { + t.Fatalf("SetEndpointAccess: %v", err) + } + got := map[string][]string{} + for _, e := range ps.EndpointAccess() { + got[e.Path] = e.OverrideRoles + } + if len(got["health"]) != 1 || got["health"][0] != "user" { + t.Errorf("health override not reported to the UI: %+v", got["health"]) + } + if len(got["echo/:what"]) != 0 { + t.Errorf("echo inherited an override it never had: %+v", got["echo/:what"]) + } +} diff --git a/internal/lua/plugin_visibility.go b/internal/lua/plugin_visibility.go index 0efa880..626687c 100644 --- a/internal/lua/plugin_visibility.go +++ b/internal/lua/plugin_visibility.go @@ -79,3 +79,40 @@ func (s *StateScope) DepthOf(key string) int { } return 1 } + +// VisibilityOverrides returns every operator override in force, keyed by plugin +// name. An absent name means the plugin's own declaration applies. +// +// The WebUI needs this to show which visibility is IMPOSED, not merely which is +// in effect: an operator comparing two deployments should not have to open a +// .lua to learn whether a page is admin-only by the plugin's choice or by their +// own override. +func (ps *Plugins) VisibilityOverrides() map[string]string { + ps.mu.RLock() + defer ps.mu.RUnlock() + if len(ps.visibility) == 0 { + return nil + } + epKeys := make(map[string]struct{}) + for _, e := range ps.endpointAccessLocked() { + epKeys[endpointOverrideKey(e.Plugin, e.Path)] = struct{}{} + } + out := make(map[string]string, len(ps.visibility)) + for k, v := range ps.visibility { + // Per-endpoint overrides live in the SAME map under a "name#path" key. + // They are endpoint policy, not a plugin audience, so leaking them here + // both mislabels them and lets the plugin page write an audience of the + // form "probe#hello" that no code ever reads as one. + // Per-endpoint overrides share this map under a "name#path" key. They + // are endpoint policy, not a plugin audience: reporting them here both + // mislabels them and invites the plugin page to save an audience of the + // form "probe#hello" that nothing reads as an audience. The set of + // endpoint keys is already computed, so ask it rather than re-deriving + // the shape from the key. + if _, isEndpoint := epKeys[k]; isEndpoint { + continue + } + out[k] = v + } + return out +} diff --git a/internal/lua/plugins.go b/internal/lua/plugins.go index 7712844..4f24bef 100644 --- a/internal/lua/plugins.go +++ b/internal/lua/plugins.go @@ -32,6 +32,7 @@ package lua // less featureful, so the split is explicit and documented. import ( + "log" "encoding/json" "fmt" "os" @@ -511,6 +512,12 @@ func (ps *Plugins) LoadDir() error { if ps.dir == "" { return nil } + // Overrides first: they are keyed by plugin name and must be in place before + // anything reads them, and a reload must not blank a restriction the + // operator set earlier in this same process. + if err := ps.LoadAccessOverrides(); err != nil { + log.Printf("[core] plugin access overrides not loaded: %v", err) + } if _, err := os.Stat(ps.dir); os.IsNotExist(err) { return nil } @@ -1167,9 +1174,10 @@ func (ps *Plugins) SetPluginVisibility(plugin, audience string) { audience = strings.TrimSpace(audience) if audience == "" { delete(ps.visibility, plugin) - return + } else { + ps.visibility[plugin] = audience } - ps.visibility[plugin] = audience + ps.saveAccessOverridesLocked() } // VisibilityOverride returns the operator's audience override for a plugin,