diff --git a/.changeset/settings-visible-expression-interpreter.md b/.changeset/settings-visible-expression-interpreter.md new file mode 100644 index 0000000000..e4702dd7e0 --- /dev/null +++ b/.changeset/settings-visible-expression-interpreter.md @@ -0,0 +1,7 @@ +--- +'@object-ui/console': patch +--- + +Settings: a setting's `visible` predicate is evaluated with the renderer's expression interpreter, and a predicate that does not evaluate hides the setting. + +The settings page used to compile a manifest's `visible` string into a JavaScript function and show the setting whenever that failed. It now evaluates the string with `ExpressionEvaluator` from `@object-ui/core`, the CSP-safe interpreter every other `visible` predicate in the renderer uses, with the namespace's values bound as `data` (`${data.provider === 'cloudflare'}`). A predicate it cannot evaluate hides the setting, the same way the server refuses a save whose `visible` it cannot parse. Every predicate the shipped manifests declare (comparisons, `&&`, `||`, `!` over `data.`) reads as before. diff --git a/apps/console/src/pages/settings/SettingsView.tsx b/apps/console/src/pages/settings/SettingsView.tsx index 6df6284feb..09e539302f 100644 --- a/apps/console/src/pages/settings/SettingsView.tsx +++ b/apps/console/src/pages/settings/SettingsView.tsx @@ -13,6 +13,7 @@ import { toast } from 'sonner'; import { ConsolePageHeader } from '@object-ui/app-shell'; import { Loader2, ArrowLeft, ChevronRight, RotateCcw, ShieldAlert } from 'lucide-react'; import { Button, Card, CardContent, Skeleton, Badge, getLazyIcon } from '@object-ui/components'; +import { ExpressionEvaluator } from '@object-ui/core'; import { extractFieldErrors } from '@object-ui/react'; import { useObjectTranslation } from '@object-ui/i18n'; import { SettingsField } from './SettingsField'; @@ -27,21 +28,23 @@ import { useSettingsLabel, type SettingsLabelHelpers } from './useSettingsLabel' import { useUnsavedLeaveGuard } from './useUnsavedLeaveGuard'; /** - * Evaluate the manifest's `visible: "${data.foo === 'bar'}"` - * expression against the live value map. Intentionally minimal — a - * full expression engine lives in `@object-ui/core` and can be wired - * in later via plugin. + * Evaluate the manifest's `visible: "${data.foo === 'bar'}"` expression + * against the live value map, through `@object-ui/core`'s `ExpressionEvaluator` + * — the CSP-safe interpreter (`SafeExpressionParser`) every other `visible` + * predicate in the renderer is judged by, never a compiled JS function. The + * manifest's values are bound as `data`, the one namespace the server's + * settings grammar reads too. + * + * Fails CLOSED: a predicate that does not evaluate hides the setting, the way + * the server refuses a save whose `visible` it cannot parse. Showing a field + * the deployment never meant to expose is the worse of the two mistakes. */ function evalVisibility(expr: string | undefined, data: Record): boolean { if (!expr) return true; - const trimmed = expr.trim(); - const m = trimmed.match(/^\$\{(.+)\}$/); - if (!m) return true; try { - const fn = new Function('data', `with (data) { return (${m[1]}); }`); - return Boolean(fn(data)); + return new ExpressionEvaluator({ data }).evaluateCondition(expr, { throwOnError: true }); } catch { - return true; + return false; } } diff --git a/apps/console/src/pages/settings/__tests__/SettingsView.visibleExpression.test.tsx b/apps/console/src/pages/settings/__tests__/SettingsView.visibleExpression.test.tsx new file mode 100644 index 0000000000..a38cc6f8c1 --- /dev/null +++ b/apps/console/src/pages/settings/__tests__/SettingsView.visibleExpression.test.tsx @@ -0,0 +1,105 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * SettingsView — a setting's `visible` predicate is judged by the renderer's + * expression interpreter (`ExpressionEvaluator` from `@object-ui/core`), with + * the manifest values bound as `data`, and a predicate that does not evaluate + * HIDES the setting. + * + * The server judges the same string with its own closed grammar and refuses a + * save whose `visible` it cannot parse, so the console must not show a field + * the server would treat as broken, and must never run the string as code. + */ + +import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest'; +import { render, screen, cleanup } from '@testing-library/react'; +import { MemoryRouter, Route, Routes } from 'react-router-dom'; +import { I18nProvider } from '@object-ui/i18n'; + +const getSettingsNamespace = vi.fn(); + +vi.mock('../api', async () => { + const actual = await vi.importActual('../api'); + return { + ...actual, + getSettingsNamespace: (...a: unknown[]) => getSettingsNamespace(...a), + saveSettingsNamespace: vi.fn(), + runSettingsAction: vi.fn(), + }; +}); + +vi.mock('sonner', () => ({ toast: { error: vi.fn(), success: vi.fn() } })); + +import { SettingsView } from '../SettingsView'; + +const PROBE = '__settingsVisibleProbe'; + +/** One always-visible anchor field, plus a field gated by `visible`. */ +function payload(visible: string, values: Record) { + return { + manifest: { + namespace: 'ai', + label: 'AI', + specifiers: [ + { type: 'text', key: 'provider', label: 'Provider anchor' }, + { type: 'text', key: 'gated', label: 'Gated field', visible }, + ], + }, + values: Object.fromEntries( + Object.entries({ provider: '', gated: '', ...values }).map(([k, v]) => [k, { value: v, source: 'default' }]), + ), + }; +} + +async function gatedFieldShown(visible: string, values: Record = {}): Promise { + getSettingsNamespace.mockResolvedValue(payload(visible, values)); + render( + + + + } /> + + + , + ); + await screen.findByText('Provider anchor'); + return screen.queryByText('Gated field') !== null; +} + +beforeEach(() => { + vi.clearAllMocks(); + delete (globalThis as Record)[PROBE]; +}); +afterEach(() => { + cleanup(); + delete (globalThis as Record)[PROBE]; +}); + +describe('SettingsView — a setting\'s `visible` predicate', () => { + it('shows the setting when the predicate holds and hides it when it does not', async () => { + expect(await gatedFieldShown("${data.provider === 'cloudflare'}", { provider: 'cloudflare' })).toBe(true); + cleanup(); + expect(await gatedFieldShown("${data.provider === 'cloudflare'}", { provider: 'openai' })).toBe(false); + }); + + it('reads the relational and logical operators the shipped manifests use', async () => { + expect(await gatedFieldShown('${data.provider > 0 && data.provider !== 3}', { provider: 5 })).toBe(true); + cleanup(); + expect(await gatedFieldShown('${data.provider > 0}', { provider: 0 })).toBe(false); + }); + + it('hides the setting when the predicate does not parse', async () => { + expect(await gatedFieldShown('${data.provider ===}')).toBe(false); + }); + + it('never runs the predicate as code, and hides a setting whose predicate it cannot evaluate', async () => { + expect(await gatedFieldShown(`\${(globalThis.${PROBE} = 1) && true}`)).toBe(false); + expect((globalThis as Record)[PROBE]).toBeUndefined(); + }); +});