diff --git a/.changeset/settings-crypto-unavailable-refusal-4570.md b/.changeset/settings-crypto-unavailable-refusal-4570.md new file mode 100644 index 0000000000..068b16c8d2 --- /dev/null +++ b/.changeset/settings-crypto-unavailable-refusal-4570.md @@ -0,0 +1,26 @@ +--- +'@object-ui/console': patch +--- + +Settings save: render the fail-closed crypto refusal as its own state instead of a generic save failure + +A deployment with nothing able to encrypt a declared-secret setting refuses the write, and +since objectstack#8396 it says so in its own wire envelope — `SETTINGS_CRYPTO_UNAVAILABLE`, +with `error.details` locating the refused `{ namespace, key }` and `error.message` carrying +the operator prescription. The console read none of it: the code fell through to the generic +error path, where the field-error extractor finds no `details.fields` array and returns null, +so nothing was marked and the whole refusal collapsed into one transient toast reading "save +failed". The admin was told the save did not work; that the DEPLOYMENT cannot encrypt, and +which key it refused, was on the wire and thrown away. + +`SettingsView` now branches on the code the way it already does for `SETTINGS_LOCKED`: it +names the refused key as `namespace.key` from the declared `error.details` slot, and renders +the server's prescription verbatim in a persistent panel — the server owns that copy, so the +console frames the refusal but never restates how to fix it. The draft is kept, so the value +is not lost while the deployment is reconfigured, and the refusal clears when its claim can +actually have become false: a new save attempt, a save that succeeds, a discard, or a reload. + +The value itself is never rendered — the envelope locates the refusal and deliberately does +not carry the secret, and the console does not re-introduce it from the draft it is holding. +`SETTINGS_LOCKED` and `SETTINGS_VALIDATION` are untouched, and an unrecognized code still +takes the generic path. diff --git a/apps/console/src/pages/settings/SettingsView.tsx b/apps/console/src/pages/settings/SettingsView.tsx index 688f05563b..e4c90d18c0 100644 --- a/apps/console/src/pages/settings/SettingsView.tsx +++ b/apps/console/src/pages/settings/SettingsView.tsx @@ -9,7 +9,7 @@ import { useCallback, useEffect, useMemo, useState } from 'react'; import { useParams, useNavigate } from 'react-router-dom'; import { toast } from 'sonner'; -import { Loader2, ArrowLeft, RotateCcw } from 'lucide-react'; +import { Loader2, ArrowLeft, RotateCcw, ShieldAlert } from 'lucide-react'; import { Button, Card, CardContent, Skeleton, Badge } from '@object-ui/components'; import { extractFieldErrors } from '@object-ui/react'; import { getIcon } from '../../utils/getIcon'; @@ -49,6 +49,55 @@ function omitKey(map: Record, key: string): Record { return rest; } +/** + * A `SETTINGS_CRYPTO_UNAVAILABLE` refusal, as this view renders it + * (objectstack#8273, PR objectstack#8396). + * + * The deployment has nothing able to encrypt a key the manifest declares + * encrypted, so the write was REFUSED rather than quietly stored in the clear. + * That is a property of the deployment, not of the value — which is why the + * envelope locates the refusal (`details: { namespace, key }`) and never + * carries the value itself. + */ +interface CryptoRefusal { + /** `namespace.key` — the refused key, as located by `error.details`. */ + subject?: string; + /** + * The server's own sentence, carrying the operator prescription (wire + * `SettingsServicePluginOptions.cryptoProvider`, or configure a real crypto + * adapter). Rendered verbatim and never restated here: the server owns this + * copy, and a second wording is how the two drift into disagreeing about how + * to fix the deployment. Absent when the body carried no message — the + * console renders nothing rather than inventing a prescription. + */ + prescription?: string; +} + +/** + * Read the refusal out of the declared `error.details` slot. + * + * Deliberately inline rather than beside `lockedKeyOf` in `api.ts`: that helper + * lives there because it is a dual-position COMPAT SHIM, reading the declared + * `details.key` *and* the pre-objectstack#4224 `error.key` sibling. This code is + * new in #8396 and has exactly one declared position, so there is no second + * shape to reconcile and nothing for the wire-shape module to own. + */ +function cryptoRefusalOf(apiError: unknown): CryptoRefusal { + const e = (apiError ?? {}) as { + message?: unknown; + details?: { namespace?: unknown; key?: unknown } | null; + }; + const nonEmpty = (v: unknown): string | undefined => + typeof v === 'string' && v.length > 0 ? v : undefined; + const details = e.details && typeof e.details === 'object' ? e.details : undefined; + const namespace = nonEmpty(details?.namespace); + const key = nonEmpty(details?.key); + return { + subject: key ? (namespace ? `${namespace}.${key}` : key) : undefined, + prescription: nonEmpty(e.message), + }; +} + export function SettingsView() { const params = useParams<{ namespace?: string }>(); const navigate = useNavigate(); @@ -70,6 +119,16 @@ export function SettingsView() { * declared `FieldError[]` under `error.details.fields`. */ const [fieldErrors, setFieldErrors] = useState>({}); + /** + * The last save's fail-closed crypto refusal, or null (objectui#4570). + * + * Unlike `fieldErrors` this deliberately does NOT clear when the key is + * edited. A field error describes the VALUE the server saw, so typing + * contradicts it; this describes the DEPLOYMENT's inability to encrypt, which + * typing does not change. It clears when that claim can actually have become + * false: a new save attempt, a save that succeeds, a discard, or a reload. + */ + const [cryptoRefusal, setCryptoRefusal] = useState(null); const load = useCallback(async () => { setLoading(true); @@ -79,6 +138,7 @@ export function SettingsView() { setPayload(p); setDraft({}); setFieldErrors({}); + setCryptoRefusal(null); } catch (err: any) { setError(err?.message ?? 'Failed to load settings'); } finally { @@ -141,6 +201,9 @@ export function SettingsView() { const onSave = async () => { if (dirtyKeys.length === 0) return; setSaving(true); + // Each attempt re-derives the refusal from scratch, so the panel always + // reflects THIS save's verdict rather than a stale one from a previous try. + setCryptoRefusal(null); try { const res = await saveSettingsNamespace(namespace, draft); setPayload({ ...payload, values: { ...values, ...res.values } }); @@ -153,6 +216,23 @@ export function SettingsView() { // `lockedKeyOf` reads both wire positions — see its note (objectstack#4224). const key = lockedKeyOf(apiError); toast.error(key ? `Locked by environment: ${key}` : 'Locked by environment'); + } else if (apiError?.code === 'SETTINGS_CRYPTO_UNAVAILABLE') { + // The deployment cannot encrypt a declared-secret key, so the write was + // refused (objectstack#8396). This is neither a per-field rejection nor + // a transient failure the user can retry their way out of — it stays on + // screen as its own state, carrying the server's prescription, until + // the deployment is reconfigured. + // + // The toast mirrors SETTINGS_LOCKED above: a code-specific sentence + // naming the subject, not the generic `err.message`. It fires for the + // same reason the validation branch's does — the panel can be scrolled + // out of view, and a save that silently does nothing is the worse + // failure. + const refusal = cryptoRefusalOf(apiError); + setCryptoRefusal(refusal); + toast.error( + refusal.subject ? `Cannot encrypt secrets: ${refusal.subject}` : 'Cannot encrypt secrets', + ); } else { // Per-field rejections render against the inputs that caused them // (objectstack#4224). `extractFieldErrors` reads `details.fields`, so it @@ -213,6 +293,36 @@ export function SettingsView() {

{manifest.helpText}

) : null} + {cryptoRefusal ? ( + + +
+ +
+

+ This deployment cannot encrypt secrets +

+

+ {cryptoRefusal.subject ? ( + <> + {cryptoRefusal.subject} is + declared encrypted, so nothing was written. + + ) : ( + 'The declared-encrypted value was refused, so nothing was written.' + )} +

+ {cryptoRefusal.prescription ? ( +

+ {cryptoRefusal.prescription} +

+ ) : null} +
+
+ + + ) : null} +
{manifest.specifiers .filter((spec) => evalVisibility(spec.visible, liveValues)) @@ -260,6 +370,7 @@ export function SettingsView() { // Discarding reverts to the stored values, so rejections of // the edits being thrown away go with them. setFieldErrors({}); + setCryptoRefusal(null); }} disabled={saving} > diff --git a/apps/console/src/pages/settings/__tests__/SettingsView.crypto-unavailable.test.tsx b/apps/console/src/pages/settings/__tests__/SettingsView.crypto-unavailable.test.tsx new file mode 100644 index 0000000000..9e8646a70a --- /dev/null +++ b/apps/console/src/pages/settings/__tests__/SettingsView.crypto-unavailable.test.tsx @@ -0,0 +1,349 @@ +/** + * 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 — the fail-closed crypto refusal is a first-class state + * (objectui#4570, following objectstack#8273 / PR objectstack#8396). + * + * What this closes: a deployment with nothing able to encrypt a declared-secret + * key refuses the write. The framework half of that landed in #8396, which gave + * the refusal its OWN wire envelope instead of the generic one: + * + * status 500 (unchanged) + * code SETTINGS_CRYPTO_UNAVAILABLE (was INTERNAL_ERROR) + * details { namespace, key } the located refusal, NEVER the value + * message the operator prescription (unchanged; the server owns this copy) + * + * The console read none of it. `SETTINGS_CRYPTO_UNAVAILABLE` fell to the `else` + * branch, where `extractFieldErrors` finds no `details.fields` array and returns + * null — so nothing was marked, nothing was rendered, and the whole refusal + * collapsed into one transient toast. An admin was told a save failed; that the + * DEPLOYMENT cannot encrypt, and which key it refused, was on the wire and + * thrown away. + * + * The shape mirrors the `SETTINGS_LOCKED` precedent next to it (SettingsView.tsx + * ~:152): detect on `error.code`, locate the subject from the declared + * `error.details` slot, and frame it in the console's own copy. It differs in + * one deliberate way, and that is the point of the card — LOCKED is a transient + * toast, while this refusal names an operator prescription that has to survive + * long enough to be acted on, so it also renders as a persistent panel. + * + * Two things these cases pin that are easy to get wrong: + * + * - The **value never appears.** The envelope's own rule is that `details` + * locates the refusal and never carries the secret; the console must not + * re-introduce it from the draft it is holding. Note the assertion is + * scoped to the panel's own `textContent` — the secret IS legitimately in + * the input the admin typed it into, so a document-wide query would be the + * wrong assertion, passing or failing for the wrong reason. + * - The **fallback must not narrow.** Adding a branch to a `code` chain is + * exactly how an unknown code stops being handled at all, so an + * unrecognized code is pinned to the generic path. + */ + +import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest'; +import { render, screen, cleanup, waitFor, fireEvent } from '@testing-library/react'; +import { MemoryRouter, Route, Routes } from 'react-router-dom'; + +const getSettingsNamespace = vi.fn(); +const saveSettingsNamespace = vi.fn(); +const runSettingsAction = vi.fn(); + +vi.mock('../api', async () => { + const actual = await vi.importActual('../api'); + return { + ...actual, + getSettingsNamespace: (...a: unknown[]) => getSettingsNamespace(...a), + saveSettingsNamespace: (...a: unknown[]) => saveSettingsNamespace(...a), + runSettingsAction: (...a: unknown[]) => runSettingsAction(...a), + }; +}); + +const toastError = vi.fn(); +const toastSuccess = vi.fn(); +vi.mock('sonner', () => ({ + toast: { + error: (...a: unknown[]) => toastError(...a), + success: (...a: unknown[]) => toastSuccess(...a), + }, +})); + +import { SettingsView } from '../SettingsView'; + +/** + * A declared-secret key beside an ordinary one, so "only the refused key is + * named" is a real assertion rather than the only thing on screen. + */ +const PAYLOAD = { + manifest: { + namespace: 'ai', + label: 'AI', + specifiers: [ + { type: 'password', key: 'api_key', label: 'API key', description: 'Provider credential.' }, + { type: 'text', key: 'account_id', label: 'Account ID', description: 'Cloudflare account.' }, + ], + }, + values: { + api_key: { value: '', source: 'default' }, + account_id: { value: 'acct_1', source: 'default' }, + }, +}; + +/** + * What the admin types. Distinctive on purpose: every "never the value" + * assertion below is a substring search for this, so an accidental echo of the + * draft into the refusal copy cannot hide behind a plausible-looking string. + */ +const SECRET = 'sk-live-DO-NOT-RENDER-4570'; + +/** + * The server's sentence, carrying the operator prescription. The console + * renders this VERBATIM — it is the server's copy to own, and restating it + * client-side is how the two drift into disagreeing about how to fix the + * deployment. + */ +const PRESCRIPTION = + "Settings namespace 'ai' declares 'api_key' as encrypted, but this deployment has no way " + + 'to encrypt it. Wire SettingsServicePluginOptions.cryptoProvider, or configure a real ' + + 'crypto adapter, and retry.'; + +/** The #8396 wire body, as `api.ts`'s `jsonOrThrow` parks it on `err.payload`. */ +function cryptoUnavailableRejection() { + const err = new Error(PRESCRIPTION) as Error & { status?: number; payload?: unknown }; + err.status = 500; + err.payload = { + success: false, + error: { + code: 'SETTINGS_CRYPTO_UNAVAILABLE', + message: PRESCRIPTION, + details: { namespace: 'ai', key: 'api_key' }, + }, + }; + return err; +} + +/** The env-lock refusal, in its post-#4224 declared position. */ +function lockedRejection() { + const err = new Error('Locked by environment.') as Error & { status?: number; payload?: unknown }; + err.status = 409; + err.payload = { + success: false, + error: { + code: 'SETTINGS_LOCKED', + message: 'Locked by environment.', + details: { namespace: 'ai', key: 'api_key' }, + }, + }; + return err; +} + +/** The per-field rejection that feeds `extractFieldErrors`. */ +function validationRejection() { + const err = new Error('Settings for \'ai\' are incomplete.') as Error & { + status?: number; + payload?: unknown; + }; + err.status = 400; + err.payload = { + success: false, + error: { + code: 'SETTINGS_VALIDATION', + message: "Settings for 'ai' are incomplete: account_id — …", + details: { + namespace: 'ai', + fields: [ + { + field: 'account_id', + code: 'invalid_format', + message: 'Account ID does not match the expected format.', + label: 'Account ID', + }, + ], + }, + }, + }; + return err; +} + +function renderView() { + return render( + + + } /> + + , + ); +} + +/** + * The secret input. `password` renders UNCONTROLLED (placeholder only, no + * `value`), so it is reached through its type rather than a display value. + */ +const secretInput = (c: HTMLElement) => c.querySelector('input[type="password"]') as HTMLInputElement; + +/** The input rendered for a given field label — the sibling test's helper. */ +const inputFor = (label: string) => + screen.getByText(label).closest('div')?.parentElement?.querySelector('input') ?? null; + +/** + * The refusal panel, located by its headline rather than a test id. + * + * The `?? null` is load-bearing: optional chaining off a `queryByText` miss + * yields `undefined`, and `expect(undefined).toBeNull()` fails — which would + * make the must-not-change pins below red for a reason that has nothing to do + * with the code under test. + */ +const refusalPanel = (): HTMLElement | null => + screen.queryByText(/cannot encrypt secrets/i)?.closest('[role="alert"]') ?? null; + +/** Type a secret and save. */ +async function typeSecretAndSave(container: HTMLElement) { + fireEvent.change(secretInput(container), { target: { value: SECRET } }); + fireEvent.click(await screen.findByRole('button', { name: /save changes/i })); +} + +beforeEach(() => { + vi.clearAllMocks(); + getSettingsNamespace.mockResolvedValue(structuredClone(PAYLOAD)); +}); +afterEach(cleanup); + +describe('SettingsView — a deployment that cannot encrypt refuses the save', () => { + it('renders the refusal as its own state, naming the refused key from error.details', async () => { + saveSettingsNamespace.mockRejectedValue(cryptoUnavailableRejection()); + const { container } = renderView(); + await screen.findByText('API key'); + + await typeSecretAndSave(container); + + // The dedicated state exists at all — pre-fix this envelope rendered + // nothing, because the generic path only toasts. + const panel = (await screen.findByText(/cannot encrypt secrets/i)).closest( + '[role="alert"]', + ) as HTMLElement; + expect(panel).toBeTruthy(); + + // Both halves of `error.details`, composed into the located refusal. + expect(panel.textContent).toContain('ai.api_key'); + + // The server's prescription, verbatim — not re-worded by the console. + expect(panel.textContent).toContain(PRESCRIPTION); + }); + + it('never renders the value it refused to encrypt', async () => { + saveSettingsNamespace.mockRejectedValue(cryptoUnavailableRejection()); + const { container } = renderView(); + await screen.findByText('API key'); + + await typeSecretAndSave(container); + const panel = (await screen.findByText(/cannot encrypt secrets/i)).closest( + '[role="alert"]', + ) as HTMLElement; + + // Scoped to the panel deliberately: the secret is legitimately inside the + // input the admin typed it into, so a document-wide query would assert the + // wrong thing. What must never happen is the console echoing the draft it + // is holding into copy about the refusal. + expect(panel.textContent).not.toContain(SECRET); + expect(panel.textContent).not.toContain('sk-live'); + }); + + it('toasts the console framing rather than the generic save-failed sentence', async () => { + saveSettingsNamespace.mockRejectedValue(cryptoUnavailableRejection()); + const { container } = renderView(); + await screen.findByText('API key'); + + await typeSecretAndSave(container); + await screen.findByText(/cannot encrypt secrets/i); + + // Mirrors SETTINGS_LOCKED's `Locked by environment: ` — a code-specific + // sentence naming the subject, not `err.message` handed to a generic toast. + expect(toastError).toHaveBeenCalledWith('Cannot encrypt secrets: ai.api_key'); + }); + + it('keeps the draft so the admin does not lose the value while fixing the deployment', async () => { + saveSettingsNamespace.mockRejectedValue(cryptoUnavailableRejection()); + const { container } = renderView(); + await screen.findByText('API key'); + + await typeSecretAndSave(container); + await screen.findByText(/cannot encrypt secrets/i); + + // The save bar is still up: the refusal is about the deployment, and + // discarding the admin's work is not part of reporting it. + expect(screen.queryByRole('button', { name: /save changes/i })).toBeTruthy(); + }); + + it('clears the refusal once a save actually succeeds', async () => { + saveSettingsNamespace.mockRejectedValueOnce(cryptoUnavailableRejection()); + const { container } = renderView(); + await screen.findByText('API key'); + + await typeSecretAndSave(container); + await screen.findByText(/cannot encrypt secrets/i); + + // The deployment got its crypto wired; the same draft now goes through. + saveSettingsNamespace.mockResolvedValueOnce({ + values: { api_key: { value: '***', source: 'tenant' } }, + }); + fireEvent.click(await screen.findByRole('button', { name: /save changes/i })); + + await waitFor(() => expect(toastSuccess).toHaveBeenCalled()); + expect(screen.queryByText(/cannot encrypt secrets/i)).toBeNull(); + }); +}); + +describe('SettingsView — the branches this refusal must not disturb', () => { + it('SETTINGS_LOCKED still renders the env-lock toast and no refusal panel', async () => { + saveSettingsNamespace.mockRejectedValue(lockedRejection()); + const { container } = renderView(); + await screen.findByText('API key'); + + await typeSecretAndSave(container); + + // Byte-for-byte the string the locked branch has always produced. + await waitFor(() => + expect(toastError).toHaveBeenCalledWith('Locked by environment: api_key'), + ); + expect(refusalPanel()).toBeNull(); + }); + + it('SETTINGS_VALIDATION still marks the offending field and no refusal panel', async () => { + saveSettingsNamespace.mockRejectedValue(validationRejection()); + const { container } = renderView(); + await screen.findByText('API key'); + + await typeSecretAndSave(container); + + const msg = await screen.findByText(/does not match the expected format/i); + expect(msg.getAttribute('role')).toBe('alert'); + await waitFor(() => { + expect(inputFor('Account ID')!.getAttribute('aria-invalid')).toBe('true'); + }); + // The field error is itself a `role="alert"`, so this is queried by the + // refusal's own headline rather than by role. + expect(refusalPanel()).toBeNull(); + }); + + it('an unrecognized code still takes the generic path — the fallback must not narrow', async () => { + const err = new Error('Something else went wrong') as Error & { payload?: unknown }; + err.payload = { + success: false, + error: { code: 'SETTINGS_SOME_FUTURE_CODE', message: 'Something else went wrong' }, + }; + saveSettingsNamespace.mockRejectedValue(err); + const { container } = renderView(); + await screen.findByText('API key'); + + await typeSecretAndSave(container); + + await waitFor(() => expect(toastError).toHaveBeenCalledWith('Something else went wrong')); + expect(refusalPanel()).toBeNull(); + expect(inputFor('Account ID')!.getAttribute('aria-invalid')).toBeNull(); + }); +});