(skywalking-horizon-ui) 09/11: fix(readonly): close alert-page write gap, yellow read-only banner, stronger env parity

[wu...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

wu-sheng pushed a commit to branch feat/template-modes-env-config
in repository https://gitbox.apache.org/repos/asf/skywalking-horizon-ui.git

commit fb614baeeb87ee3fbf8c9fa338b0b96a1a2e3e05
Author: Wu Sheng <wu...@foxmail.com>
AuthorDate: Fri Jun 26 09:59:27 2026 +0800

    fix(readonly): close alert-page write gap, yellow read-only banner, stronger env parity
    
    Three review findings + a UX fix:
    
    - HIGH: the alert page-setup (/api/alarms/config) was still writable in readonly.
      The readonly backstop only covered /api/admin/templates* and
      /api/admin/overview-templates*; /api/alarms/config writes the
      horizon.alert.page-setup template (file-backed) and slipped through. It's now
      in the config-surface write set — a direct POST returns 409 in readonly — and
      GET serves the bundled alert page-setup (like every other template) instead of
      the local store. Validated live: POST 409, GET serves the bundle.
    
    - The read-only SyncStatusBanner was muted gray ("white") and easy to miss next
      to the green per-row sync chips. It's now warning-yellow so it's unmistakable
      that the whole config surface is bundle-served and uneditable. (The per-row
      "synced" chip is correct as-is: in readonly the bundle IS the presented remote,
      so bundled == remote and the diff legitimately reads synced.)
    
    - The env parity guard only checked top-level sections + four hard-coded tokens.
      It now recursively walks every schema-default field and asserts each is covered
      by a ${HORIZON_…} token in horizon.example.yaml (a scalar token, or a JSON-env
      token for a whole subtree). It immediately caught a real gap — oap.mqe shipped
      untokenized — now fixed (HORIZON_OAP_MQE). A future nested field without env
      coverage will now fail the test.
    
    type-check / lint / license / 162 BFF + 116 UI tests / builds green.
---
 apps/bff/src/config/schema.test.ts                 | 36 ++++++++++++++++++++++
 apps/bff/src/http/config/alarms.ts                 |  7 ++++-
 apps/bff/src/rbac/route-policy.test.ts             |  6 ++--
 apps/bff/src/rbac/route-policy.ts                  | 16 +++++++---
 .../features/admin/_shared/SyncStatusBanner.vue    | 11 ++++---
 horizon.example.yaml                               |  3 ++
 6 files changed, 66 insertions(+), 13 deletions(-)

diff --git a/apps/bff/src/config/schema.test.ts b/apps/bff/src/config/schema.test.ts
index 9d4a7d0..74bc26d 100644
--- a/apps/bff/src/config/schema.test.ts
+++ b/apps/bff/src/config/schema.test.ts
@@ -73,4 +73,40 @@ describe('horizon.example.yaml — tokenized default + parity', () => {
     expect(raw).toContain('${HORIZON_TEMPLATES_MODE');
     expect(raw).toContain('${HORIZON_OAP_ADMIN_URL');
   });
+
+  // The real contract: EVERY schema-default field is env-overridable. Walk the
+  // fully-defaulted config and assert each node is covered in the example by a
+  // `${HORIZON_…}` token — either a scalar token at that path, or a JSON-env
+  // token standing in for a whole subtree (performance, rbac.roles, …). Catches
+  // a new nested field shipped without env coverage, which the top-level-section
+  // check above would miss.
+  it('every schema-default field is env-tokenized in the example (recursive)', () => {
+    const defaults = configSchema.parse({}) as Record<string, unknown>;
+    const example = (YAML.parse(raw) ?? {}) as Record<string, unknown>;
+    const TOKEN = /\$\{HORIZON_[A-Z0-9_]+(:[\s\S]*)?\}/;
+    const uncovered: string[] = [];
+    const walk = (schemaNode: unknown, exampleNode: unknown, path: string): void => {
+      // A token string covers this node (scalar token, or a JSON-env token
+      // collapsing a whole subtree to one var).
+      if (typeof exampleNode === 'string' && TOKEN.test(exampleNode)) return;
+      if (schemaNode !== null && typeof schemaNode === 'object' && !Array.isArray(schemaNode)) {
+        if (exampleNode === null || typeof exampleNode !== 'object' || Array.isArray(exampleNode)) {
+          uncovered.push(`${path} (section absent from example)`);
+          return;
+        }
+        for (const key of Object.keys(schemaNode)) {
+          if (path === '' && key === 'infra3d') continue; // deprecated passthrough, never tokenized
+          walk(
+            (schemaNode as Record<string, unknown>)[key],
+            (exampleNode as Record<string, unknown>)[key],
+            path ? `${path}.${key}` : key,
+          );
+        }
+        return;
+      }
+      uncovered.push(`${path} (leaf not tokenized)`);
+    };
+    walk(defaults, example, '');
+    expect(uncovered, `fields lacking an env token in horizon.example.yaml:\n  ${uncovered.join('\n  ')}`).toEqual([]);
+  });
 });
diff --git a/apps/bff/src/http/config/alarms.ts b/apps/bff/src/http/config/alarms.ts
index 1b1e5c3..467259d 100644
--- a/apps/bff/src/http/config/alarms.ts
+++ b/apps/bff/src/http/config/alarms.ts
@@ -30,6 +30,8 @@ import type { ConfigSource } from '../../config/loader.js';
 import type { SessionStore } from '../../user/sessions.js';
 import type { AuditLogger } from '../../audit/logger.js';
 import { requireAuth } from '../../user/middleware.js';
+import { isTemplateReadOnly } from '../../logic/templates/sync.js';
+import { loadBundledAlertPageSetup } from '../../logic/alarms/bundled.js';
 import type { ServiceLayerCatalog } from '../../logic/services/service-layer-catalog.js';
 import {
   ALARMS_WINDOW_OPTIONS,
@@ -77,7 +79,10 @@ export function registerAlarmsConfigRoutes(
     '/api/alarms/config',
     { preHandler: auth },
     async (_req: FastifyRequest, reply: FastifyReply) => {
-      const cfg = await deps.store.load();
+      // Readonly mode serves the alert page-setup from the bundle like every
+      // other template; the local store (a prior live session's edits) is not
+      // the source here. Writes are denied by the route-policy backstop.
+      const cfg = isTemplateReadOnly() ? loadBundledAlertPageSetup() : await deps.store.load();
       return reply.send(cfg);
     },
   );
diff --git a/apps/bff/src/rbac/route-policy.test.ts b/apps/bff/src/rbac/route-policy.test.ts
index 67f3242..750dc3a 100644
--- a/apps/bff/src/rbac/route-policy.test.ts
+++ b/apps/bff/src/rbac/route-policy.test.ts
@@ -27,12 +27,14 @@ describe('isTemplateWriteRoute — which routes the readonly backstop covers', (
     expect(isTemplateWriteRoute('POST', '/api/admin/templates/sync-all')).toBe(true);
     expect(isTemplateWriteRoute('POST', '/api/admin/overview-templates')).toBe(true);
     expect(isTemplateWriteRoute('DELETE', '/api/admin/overview-templates/x')).toBe(true);
+    // The alert page-setup (horizon.alert.page-setup) is config-surface too.
+    expect(isTemplateWriteRoute('POST', '/api/alarms/config')).toBe(true);
   });
-  it('does NOT match reads, nor non-template writes (runtime-rule / local state stay editable)', () => {
+  it('does NOT match reads, nor non-config writes (runtime-rule / live-debug stay editable)', () => {
     expect(isTemplateWriteRoute('GET', '/api/admin/templates/sync-status')).toBe(false);
     expect(isTemplateWriteRoute('HEAD', '/api/admin/templates/sync-status')).toBe(false);
+    expect(isTemplateWriteRoute('GET', '/api/alarms/config')).toBe(false); // read stays open
     expect(isTemplateWriteRoute('POST', '/api/rule')).toBe(false); // runtime rule
-    expect(isTemplateWriteRoute('POST', '/api/alarms/config')).toBe(false); // local state
     expect(isTemplateWriteRoute('POST', '/api/debug/session')).toBe(false); // live-debug
   });
 });
diff --git a/apps/bff/src/rbac/route-policy.ts b/apps/bff/src/rbac/route-policy.ts
index fb3d617..257a74b 100644
--- a/apps/bff/src/rbac/route-policy.ts
+++ b/apps/bff/src/rbac/route-policy.ts
@@ -33,13 +33,19 @@ import { logger } from '../logger.js';
 
 export type RoutePolicy = 'public' | 'auth' | string;
 
-/** A config-template write — it pushes to OAP's ui_template store. In
- *  `templates.mode=readonly` there is no store to write to, so these are denied
- *  at the edge regardless of the verb grant (the UI hides them too, but a direct
- *  request must still fail — the BFF is the authority). */
+/** A config-surface write. The template routes push to OAP's ui_template store;
+ *  `/api/alarms/config` writes the alert page-setup (the `horizon.alert.page-setup`
+ *  template, file-backed). In `templates.mode=readonly` the whole config surface
+ *  is served from the local bundle and read-only, so these are denied at the edge
+ *  regardless of the verb grant (the UI hides them too, but a direct request must
+ *  still fail — the BFF is the authority). */
 export function isTemplateWriteRoute(method: string, url: string): boolean {
   if (method === 'GET' || method === 'HEAD') return false;
-  return url.startsWith('/api/admin/templates') || url.startsWith('/api/admin/overview-templates');
+  return (
+    url.startsWith('/api/admin/templates') ||
+    url.startsWith('/api/admin/overview-templates') ||
+    url.startsWith('/api/alarms/config')
+  );
 }
 export async function denyTemplateWriteWhenReadOnly(
   _req: FastifyRequest,
diff --git a/apps/ui/src/features/admin/_shared/SyncStatusBanner.vue b/apps/ui/src/features/admin/_shared/SyncStatusBanner.vue
index 69a52c6..8f84279 100644
--- a/apps/ui/src/features/admin/_shared/SyncStatusBanner.vue
+++ b/apps/ui/src/features/admin/_shared/SyncStatusBanner.vue
@@ -124,13 +124,14 @@ export default { chipLabel };
   border-color: var(--sw-muted, #4a525c);
   background: var(--sw-bg-elev, #161a20);
 }
-/* Deliberate read-only (templates.mode=readonly) — informational, not an
- * error, so muted rather than the danger red of `unreachable`. */
+/* Deliberate read-only (templates.mode=readonly) — not an error (no red), but
+ * warning-yellow so it's unmistakable that the whole surface is uneditable; a
+ * muted gray strip was too easy to miss next to the green per-row chips. */
 .sbb--readonly {
-  border-color: var(--sw-muted, #4a525c);
-  background: rgba(255, 255, 255, 0.03);
+  border-color: var(--sw-warn, #b88500);
+  background: rgba(184, 133, 0, 0.10);
 }
-.sbb--readonly .sbb__chip { background: var(--sw-muted, #4a525c); color: #fff; }
+.sbb--readonly .sbb__chip { background: var(--sw-warn, #b88500); color: #1a1a1a; }
 .sbb__row {
   display: flex;
   gap: 12px;
diff --git a/horizon.example.yaml b/horizon.example.yaml
index d3c045e..4da1e8f 100644
--- a/horizon.example.yaml
+++ b/horizon.example.yaml
@@ -58,6 +58,9 @@ oap:
   # GraphQL port (typical for the demo / k8s deploys).
   zipkinUrl: "${HORIZON_OAP_ZIPKIN_URL:http://127.0.0.1:9412/zipkin}"
   timeoutMs: ${HORIZON_OAP_TIMEOUT_MS:15000}
+  # Optional MQE host/port override (defaults to the query host). JSON env, e.g.
+  # HORIZON_OAP_MQE='{"host":"mqe.internal","port":12800}'.
+  mqe: ${HORIZON_OAP_MQE:null}
   # Optional basic-auth for outbound OAP calls. JSON env, e.g.
   # HORIZON_OAP_AUTH='{"username":"skywalking","password":"skywalking"}'.
   auth: ${HORIZON_OAP_AUTH:null}