fix(rest): dashboard requiresService 组件门禁在默认读取路径上真正执行 (#5881)

.changeset/dashboard-service-gate-default-path.md

@@ -0,0 +1,30 @@
+---
+"@objectstack/rest": patch
+---
+
+fix(rest): dashboard 组件门禁在默认配置下真正执行 (#5881)
+
+ADR-0057 D10 的 `requiresService` 组件门禁 —— 剔除指向未注册可选服务的 dashboard
+磁贴 —— 在默认部署里一次都没跑过。`GET /meta/:type/:name` 的单条读取有一条缓存分支,
+它排除了 `app`(per-user RBAC 过滤)与 `doc` / `book`(per-caller audience),唯独没有
+排除 `dashboard`;而 `enableCache` 默认为 `true`。门禁写在非缓存分支里,于是只有显式
+关掉缓存的部署才会执行到它。
+
+后果正是该 ADR 点名要防的那一幕:在没有某个可选服务的部署里(比如单租户运行时里的
+Organizations KPI,其 `org-scoping` 服务不存在),console 会渲染一块绑定到缺失服务的
+死磁贴 —— 尽管服务端的门禁代码在、测试也在。
+
+**修复**:`dashboard` 与 `app` 同款,从缓存分支排除,两种拼写(`/meta/dashboard/x`
+与规范复数 `/meta/dashboards/x`)都覆盖。其它元数据类型的 ETag 快路径不受影响。
+
+**为什么不是"把门禁提到分支之外、两条路径共用"** —— 那读起来更整齐,但 ETag 无法承载
+门禁结论:validator 是**未过滤文档**的哈希,而 `notModified` 在 protocol 内部就已判定,
+REST 层没有机会重判。共用之后送出的就是"过滤过的正文 + 指向未过滤正文的 validator"。
+一次 boot 之内这没有危害(已注册服务集在 bootstrap 之后不可变),但 `Cache-Control:
+private, no-cache` 意味着客户端**存下正文**、之后只做重验证,而存下的正文比进程活得久:
+一次关掉该可选服务的重新部署并不改变文档,ETag 不变 ⇒ 每次重验证都回 304 ⇒ 那块死磁贴
+恰好在移除其服务的那次部署之后被永久缓存下来。放弃快路径的代价则接近于零:
+`getMetaItemCached` 本就委托给 `getMetaItem`,服务端两条路做的是同样的工作,失去的只是
+304 省下的正文字节。
+
+对调用方的可见变化:dashboard 的单条读取不再返回 ETag / 304,每次都是完整的 200。

packages/rest/src/rest-server.ts

@@ -4246,7 +4246,48 @@ export class RestServer {
                         // `doc` and `book` bypass the shared cache: their §6.7
                         // audience gate is per-caller, and a shared ETag would
                         // leak gated content across viewers.
-                        if (metadata.enableCache && p.getMetaItemCached && !isAppType && !isDraftRead && !previewDrafts && !packageScoped && req.params.type !== 'doc' && req.params.type !== 'book') {
+                        //
+                        // [#5881] `dashboard` bypasses it too, and the reason is
+                        // NOT the one above — worth writing down, because the
+                        // obvious reading says a dashboard needn't bypass at all.
+                        // Its ADR-0057 D10 widget gate (`filterDashboardForUser`,
+                        // below) is per-DEPLOYMENT — it asks which optional kernel
+                        // services are registered — never per-caller, so there is
+                        // no cross-viewer leak to avoid. What rules out sharing
+                        // the cached path is the validator itself: the ETag is
+                        // `simpleHash(locale + JSON.stringify(item))` over the
+                        // UNFILTERED document (metadata-protocol `getMetaItemCached`),
+                        // so it cannot express the gate dimension at all, and
+                        // `notModified` is decided inside the protocol before this
+                        // layer could re-judge it. Gating the cached body would
+                        // therefore ship a filtered body under a validator that
+                        // identifies the unfiltered one.
+                        //
+                        // That mismatch is not academic, because the two have
+                        // different lifetimes. Within one boot the registered-service
+                        // set is fixed (`Kernel.use()` throws once bootstrap has
+                        // started, and no deregistration API exists), so the gate
+                        // verdict is stable per process — but `Cache-Control:
+                        // private, no-cache` means the client STORES the body and
+                        // revalidates, and that stored body outlives the process.
+                        // A redeploy that turns the optional service off does not
+                        // change the document, so the ETag is unchanged, every
+                        // revalidation answers 304, and the stale unfiltered body
+                        // stands: the dead tile D10 exists to prevent, now cached
+                        // indefinitely. Bypassing costs nothing to weigh against
+                        // that — `getMetaItemCached` delegates to `getMetaItem`,
+                        // so the server does identical work either way and only
+                        // the 304's saved body bytes are given up.
+                        //
+                        // Compared on the NORMALIZED type, like `isAppType` and
+                        // unlike the two literals at the end of this condition
+                        // (`/meta/dashboards/x` is the canonical plural spelling
+                        // under Prime Directive #3, and an exclusion it could be
+                        // spelled around would not be an exclusion). The `doc` /
+                        // `book` literals have exactly that hole — measured and
+                        // filed as #6241, deliberately not fixed here.
+                        const isDashboardType = RestServer.metaTypeSingular(req.params.type) === 'dashboard';
+                        if (metadata.enableCache && p.getMetaItemCached && !isAppType && !isDashboardType && !isDraftRead && !previewDrafts && !packageScoped && req.params.type !== 'doc' && req.params.type !== 'book') {
                             const cacheRequest = {
                                 ifNoneMatch: req.headers['if-none-match'] as string,
                                 ifModifiedSince: req.headers['if-modified-since'] as string,
@@ -4367,7 +4408,18 @@ export class RestServer {
                             // ADR-0057 D10: gate dashboard widgets by `requiresService`
                             // (mirrors the app-nav gate above) so the console never
                             // renders a tile bound to an absent optional service.
-                            if (RestServer.metaTypeSingular(req.params.type) === 'dashboard' && visible) {
+                            //
+                            // [#5881] This is now on the DEFAULT path. It reads as
+                            // ordinary code either way, which is exactly why the
+                            // defect was invisible: `enableCache` defaults to true
+                            // and `dashboard` was not excluded above, so every
+                            // default deployment took the cached branch and this
+                            // gate ran only where an operator had turned the cache
+                            // off. Declared, tested, and never executed in
+                            // production — the exclusion above is what makes the
+                            // ADR's "the server is the authoritative gate" true
+                            // rather than merely written down.
+                            if (isDashboardType && visible) {
                                 const ctx = await this.resolveExecCtx(environmentId, req).catch(() => undefined);
                                 const registered = await this.resolveRegisteredServices((ctx as any)?.__kernel, [visible]);
                                 const serviceGate = registered ? (n: string) => registered.has(n) : undefined;

packages/rest/src/rest.test.ts

@@ -3375,9 +3375,12 @@ describe('filterDashboardForUser — ADR-0057 D10 widget requiresService gate',
     const rest: any = new RestServer(
       createMockServer() as any,
       protocol,
-      // The widget gate lives on the uncached read; the cached branch — which
-      // is the DEFAULT — does not run it at all (pre-existing and unrelated to
-      // #5563, filed as #5881), so this pins the gate where it exists.
+      // [#5881] `enableCache: false` is no longer what makes this reachable —
+      // dashboard reads bypass the cache unconditionally now, and the DEFAULT
+      // configuration is pinned separately below. Kept explicit because an
+      // operator who disables the cache must get the same answer, and because
+      // this case is what the fix had to leave untouched: it was the only
+      // green proof the gate worked at all while the default path skipped it.
       { api: { requireAuth: false }, metadata: { enableCache: false } } as any,
     );
     rest.resolveExecCtx = async () => ({ userId: 'u1', systemPermissions: [] });
@@ -3395,6 +3398,114 @@ describe('filterDashboardForUser — ADR-0057 D10 widget requiresService gate',
     expect(ids(body.item)).toContain('widget_total_users');
   });
 
+  // -------------------------------------------------------------------------
+  // [#5881] The gate on the DEFAULT path.
+  //
+  // `enableCache` defaults to true, and the single-item read had a cached
+  // branch that excluded `app` (per-user RBAC) and `doc`/`book` (per-caller
+  // audience) but NOT `dashboard` — so the widget gate above, which lives in
+  // the uncached branch, never ran in a default deployment. Measured on
+  // `origin/main` @ 8e2bbba24 before the fix, against the real RestServer:
+  //
+  //     cachedCalls: 1 | uncachedCalls: 0 | widgetsServed: ["w_users","w_orgs"]
+  //
+  // The fix excludes `dashboard` from the cached branch, as `app` already was.
+  // Why not instead hoist the gate so both branches run it once — which reads
+  // like the tidier shape? Because the ETag cannot carry the gate's verdict:
+  // it is a hash of the UNFILTERED document, and `notModified` is decided
+  // inside the protocol before this layer sees it. Measured, same baseline:
+  //
+  //     etag(unfiltered): 2504e71e | etag(gated body): 75ca17c1 | same? false
+  //     revalidate-with-unfiltered-etag -> notModified: true
+  //     cacheControl: {"directives":["private","no-cache"]}
+  //
+  // So a hoisted gate would ship a filtered body under a validator naming the
+  // unfiltered one. Within one boot that is harmless (the registered-service
+  // set cannot change: `Kernel.use()` throws after bootstrap and nothing
+  // deregisters), but `private, no-cache` means the client stores the body and
+  // only revalidates — and the stored body outlives the process. Turn the
+  // optional service off in a redeploy and the document is unchanged, so every
+  // revalidation answers 304 and the dead tile survives the very deploy that
+  // removed its service. The full-read cost of not caching is nil:
+  // `getMetaItemCached` delegates to `getMetaItem`, so the server does the same
+  // work either way and only the 304's saved bytes are given up.
+  // -------------------------------------------------------------------------
+  /** A protocol offering BOTH reads, so the branch choice is the thing tested. */
+  const bothReads = () => {
+    const protocol: any = createMockProtocol();
+    protocol.getMetaItem = vi.fn().mockResolvedValue({
+      type: 'dashboard', name: 'system_overview', item: dash(),
+    });
+    protocol.getMetaItemCached = vi.fn().mockResolvedValue({
+      data: dash(),
+      etag: { value: 'etag-unfiltered', weak: false },
+      lastModified: new Date().toISOString(),
+      cacheControl: { directives: ['private', 'no-cache'] },
+      notModified: false,
+    });
+    return protocol;
+  };
+  /** Default config — no `metadata` block at all, so `enableCache` is its default. */
+  const defaultServer = (protocol: any) => {
+    const rest: any = new RestServer(createMockServer() as any, protocol, ANON_API as any);
+    rest.resolveExecCtx = async () => ({ userId: 'u1', systemPermissions: [] });
+    rest.serviceExistsProvider = (n: string) => n !== 'org-scoping';
+    rest.registerRoutes();
+    return rest;
+  };
+  const readMeta = async (rest: any, type: string, name: string) => {
+    const route = rest.getRoutes().find(
+      (r: any) => r.method === 'GET' && r.path === '/api/v1/meta/:type/:name',
+    );
+    const res = { json: vi.fn(), status: vi.fn().mockReturnThis(), header: vi.fn(), send: vi.fn() };
+    await route.handler({ params: { type, name }, query: {}, headers: {} }, res);
+    return res;
+  };
+
+  it('runs the gate under the DEFAULT configuration, not only when the cache is off', async () => {
+    const protocol = bothReads();
+    const res = await readMeta(defaultServer(protocol), 'dashboard', 'system_overview');
+
+    const body = res.json.mock.calls.at(-1)![0];
+    expect(ids(body.item)).not.toContain('widget_organizations');
+    expect(ids(body.item)).toContain('widget_total_users');
+    // The envelope the route owes its caller is unchanged by the branch switch.
+    expect(body).toMatchObject({ type: 'dashboard', name: 'system_overview' });
+  });
+
+  it('a dashboard read takes the uncached branch — while other types still cache', async () => {
+    // The positive control matters as much as the assertion above it: excluding
+    // one type from the cached branch is only correct if it excludes exactly
+    // that type. A fix that disabled the cache wholesale would satisfy every
+    // gate assertion here and quietly cost every other metadata read its ETag.
+    const protocol = bothReads();
+    const rest = defaultServer(protocol);
+
+    const dashRes = await readMeta(rest, 'dashboard', 'system_overview');
+    expect(protocol.getMetaItemCached).not.toHaveBeenCalled();
+    expect(protocol.getMetaItem).toHaveBeenCalledTimes(1);
+    // The observable price of the bypass, pinned rather than hidden: a
+    // dashboard read carries no ETag validator now.
+    expect(dashRes.header.mock.calls.map((c: any[]) => c[0])).not.toContain('ETag');
+
+    const viewRes = await readMeta(rest, 'view', 'account_list');
+    expect(protocol.getMetaItemCached).toHaveBeenCalledTimes(1);
+    expect(viewRes.header.mock.calls.map((c: any[]) => c[0])).toContain('ETag');
+  });
+
+  it('the plural spelling cannot spell its way around the exclusion', async () => {
+    // `/meta/dashboards/x` is the CANONICAL REST spelling (Prime Directive #3)
+    // and the route serves both, so an exclusion compared against the literal
+    // `'dashboard'` would leave the default path ungated under the spelling
+    // most callers use — the #3984 defect class, which `metaTypeSingular`'s own
+    // docstring exists to remember.
+    const protocol = bothReads();
+    const res = await readMeta(defaultServer(protocol), 'dashboards', 'system_overview');
+
+    expect(protocol.getMetaItemCached).not.toHaveBeenCalled();
+    expect(ids(res.json.mock.calls.at(-1)![0].item)).not.toContain('widget_organizations');
+  });
+
   it('resolveRegisteredServices discovers requiresService declared on widgets', async () => {
     const rest: any = make();
     const kernel = { getServiceAsync: async (n: string) => { if (n === 'org-scoping') return {}; throw new Error('absent'); } };