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 ea9f3276022714afc87b7cadcb1060f420894b41 Author: Wu Sheng <[email protected]> AuthorDate: Fri Jun 26 02:14:10 2026 +0800 fix(config): review pass 2 — readonly UI gating, friendly config error, tests - Translations page is now read-only in readonly mode like its siblings: the widget editor never opens (openPanel early-returns) and pushToOap guards on readOnly. Previously the operator could translate→stage→push and was only stopped by the server 409 at the end. - Invalid config (e.g. a bool env var set to no/off/0, a non-numeric port, or malformed JSON env) now fails boot with a field-path-annotated fatal message instead of a raw ZodError dump (the crash itself is correct — fail loud). - Tests: the readonly write-deny route hook (isTemplateWriteRoute matches the config-template writes, not runtime-rule / local-state; deny → 409 in readonly, no-op in live); quoted string scalars survive YAML-metachar values; the example-parity test interpolates only the env vars the schema reads inline (a stray HORIZON_* no longer reads as drift). type-check / lint / license / 155 BFF + 116 UI unit tests green. --- apps/bff/src/config/loader.test.ts | 8 +++ apps/bff/src/config/schema.test.ts | 18 +++++-- apps/bff/src/rbac/route-policy.test.ts | 62 ++++++++++++++++++++++ apps/bff/src/rbac/route-policy.ts | 4 +- apps/bff/src/server.ts | 12 +++++ .../admin/translations/TranslationsView.vue | 6 ++- 6 files changed, 102 insertions(+), 8 deletions(-) diff --git a/apps/bff/src/config/loader.test.ts b/apps/bff/src/config/loader.test.ts index 5890194..aa629f8 100644 --- a/apps/bff/src/config/loader.test.ts +++ b/apps/bff/src/config/loader.test.ts @@ -103,6 +103,14 @@ describe('env-native config (horizon.example.yaml + env)', () => { expect(() => load({ HORIZON_AUTH_LOCAL_USERS: '[{bad json' })).toThrow(); }); + it('quoted string scalars survive a value with YAML metacharacters (: and #)', () => { + // The string tokens are quoted (`"${X:default}"`), so a metachar value + // lands safely inside a YAML string instead of breaking the parse. + const cfg = load({ HORIZON_SESSION_COOKIE_NAME: 'weird: name #x', HORIZON_OAP_QUERY_URL: 'http://oap:12800' }); + expect(cfg.session.cookieName).toBe('weird: name #x'); + expect(cfg.oap.queryUrl).toBe('http://oap:12800'); + }); + it('survives newlines and YAML formatting', () => { const raw = `auth:\n ldap:\n bindPassword: "\${HORIZON_LDAP_BIND_PW:dev-only}"\n`; const out = interpolateEnv(raw, {}); diff --git a/apps/bff/src/config/schema.test.ts b/apps/bff/src/config/schema.test.ts index 9aaccc0..9d4a7d0 100644 --- a/apps/bff/src/config/schema.test.ts +++ b/apps/bff/src/config/schema.test.ts @@ -41,11 +41,19 @@ describe('horizon.example.yaml — tokenized default + parity', () => { const raw = readFileSync(examplePath, 'utf8'); it('parses to exactly the schema defaults (token defaults match the schema)', () => { - // Use process.env on BOTH sides: the schema's inline env defaults - // (serverHostDefault, the *_FILE paths, sourcemaps dir, templatesMode) read - // process.env at module load, so the example's tokens must resolve against - // the same env or a stray HORIZON_* in CI would read as drift. - const parsed = stripNullish(YAML.parse(interpolateEnv(raw, process.env)) ?? {}); + // configSchema.parse({}) reads a FIXED set of HORIZON_* env vars inline for + // its defaults (server host/port, the *_FILE paths, sourcemaps dir, + // templates mode). Interpolate the example with ONLY those — so the two + // sides agree on the env-derived defaults, while every OTHER stray HORIZON_* + // in a dev/CI env is ignored (it would otherwise read as drift, since the + // schema default for e.g. oap.queryUrl is a literal, not env-read). + const SCHEMA_ENV = [ + 'HORIZON_SERVER_HOST', 'HORIZON_SERVER_PORT', 'HORIZON_AUDIT_FILE', 'HORIZON_SETUP_FILE', + 'HORIZON_ALARMS_FILE', 'HORIZON_WIRE_LOG_FILE', 'HORIZON_SOURCEMAPS_DIR', 'HORIZON_TEMPLATES_MODE', + ]; + const env: NodeJS.ProcessEnv = {}; + for (const k of SCHEMA_ENV) if (process.env[k] !== undefined) env[k] = process.env[k]; + const parsed = stripNullish(YAML.parse(interpolateEnv(raw, env)) ?? {}); expect(configSchema.parse(parsed)).toEqual(configSchema.parse({})); }); diff --git a/apps/bff/src/rbac/route-policy.test.ts b/apps/bff/src/rbac/route-policy.test.ts new file mode 100644 index 0000000..67f3242 --- /dev/null +++ b/apps/bff/src/rbac/route-policy.test.ts @@ -0,0 +1,62 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import type { FastifyReply, FastifyRequest } from 'fastify'; +import { isTemplateWriteRoute, denyTemplateWriteWhenReadOnly } from './route-policy.js'; +import { setTemplateReadOnly } from '../logic/templates/sync.js'; + +describe('isTemplateWriteRoute — which routes the readonly backstop covers', () => { + it('matches non-GET config-template write routes', () => { + expect(isTemplateWriteRoute('POST', '/api/admin/templates/save')).toBe(true); + expect(isTemplateWriteRoute('POST', '/api/admin/templates/save-translation')).toBe(true); + 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); + }); + it('does NOT match reads, nor non-template writes (runtime-rule / local state stay editable)', () => { + expect(isTemplateWriteRoute('GET', '/api/admin/templates/sync-status')).toBe(false); + expect(isTemplateWriteRoute('HEAD', '/api/admin/templates/sync-status')).toBe(false); + 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 + }); +}); + +describe('denyTemplateWriteWhenReadOnly — the BFF backstop', () => { + afterEach(() => setTemplateReadOnly(false)); + + const fakeReply = (): { reply: FastifyReply; code: ReturnType<typeof vi.fn> } => { + const send = vi.fn(); + const code = vi.fn(() => ({ send })); + return { reply: { code } as unknown as FastifyReply, code }; + }; + + it('rejects with 409 in readonly mode', async () => { + setTemplateReadOnly(true); + const { reply, code } = fakeReply(); + await denyTemplateWriteWhenReadOnly({} as FastifyRequest, reply); + expect(code).toHaveBeenCalledWith(409); + }); + + it('is a no-op in live mode (lets the write proceed)', async () => { + setTemplateReadOnly(false); + const { reply, code } = fakeReply(); + await denyTemplateWriteWhenReadOnly({} as FastifyRequest, reply); + expect(code).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/bff/src/rbac/route-policy.ts b/apps/bff/src/rbac/route-policy.ts index ff0abeb..fb3d617 100644 --- a/apps/bff/src/rbac/route-policy.ts +++ b/apps/bff/src/rbac/route-policy.ts @@ -37,11 +37,11 @@ export type RoutePolicy = 'public' | 'auth' | string; * `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). */ -function isTemplateWriteRoute(method: string, url: string): boolean { +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'); } -async function denyTemplateWriteWhenReadOnly( +export async function denyTemplateWriteWhenReadOnly( _req: FastifyRequest, reply: FastifyReply, ): Promise<void> { diff --git a/apps/bff/src/server.ts b/apps/bff/src/server.ts index 3a2b1a8..eba8ce9 100644 --- a/apps/bff/src/server.ts +++ b/apps/bff/src/server.ts @@ -17,6 +17,7 @@ import { existsSync } from 'node:fs'; import { resolve as resolvePath } from 'node:path'; +import { ZodError } from 'zod'; import Fastify from 'fastify'; import cookie from '@fastify/cookie'; import fastifyStatic from '@fastify/static'; @@ -104,6 +105,17 @@ try { logger.fatal({ err: err.message, configPath }, 'BFF refusing to start: bootstrap validation failed'); process.exit(1); } + if (err instanceof ZodError) { + // A bad value (often a HORIZON_* env override): surface the field path + + // reason instead of a raw zod dump. Booleans accept only true/false, + // numbers must be numeric, and JSON env vars must be valid JSON. + const issues = err.issues.map((i) => `${i.path.join('.') || '(root)'}: ${i.message}`).join('; '); + logger.fatal( + { issues, configPath }, + 'BFF refusing to start: config validation failed — check the value (and any HORIZON_* env override) at each path. Booleans must be true/false; numbers must be numeric; JSON env vars must be valid JSON', + ); + process.exit(1); + } throw err; } logger.info( diff --git a/apps/ui/src/features/admin/translations/TranslationsView.vue b/apps/ui/src/features/admin/translations/TranslationsView.vue index f21ccf9..333881e 100644 --- a/apps/ui/src/features/admin/translations/TranslationsView.vue +++ b/apps/ui/src/features/admin/translations/TranslationsView.vue @@ -398,6 +398,10 @@ interface PanelState { const panel = ref<PanelState>({ open: false, anchor: null, point: null, fields: [], label: '' }); function openPanel(fields: TranslatableField[], label: string, el: HTMLElement, point: { x: number; y: number }): void { + // Read-only mode (templates.mode=readonly or admin unreachable): the canvas + // stays viewable but the editor never opens — no edit can start that could + // only end in a server 409. Sibling admin pages gate the same way. + if (readOnly.value) return; // Widgets with no translatable text (e.g. a topology widget that // only carries `layer`) shouldn't open an empty panel. if (fields.length === 0) return; @@ -600,7 +604,7 @@ async function pushToOap(): Promise<void> { const name = selectedName.value; const loc = target.value; const overlay = draftOverlayForTarget.value; - if (!name || saving.value) return; + if (!name || saving.value || readOnly.value) return; // BFF denies it anyway saving.value = true; saveMsg.value = t('Saving to OAP…'); let elapsed = 0;
